Re: [PATCH 5/9] vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()

[Joao Martins]

On 16/12/2024 09:46, Avihai Horon wrote:
> There is no need for vfio_dma_unmap_dirty_sync_needed(), as it simply
> calls vfio_devices_all_dirty_tracking_started().
> 
> Drop vfio_dma_unmap_dirty_sync_needed(), export
> vfio_devices_all_dirty_tracking_started() and use it instead.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>  include/hw/vfio/vfio-common.h | 3 ++-
>  hw/vfio/common.c              | 9 ++-------
>  hw/vfio/container.c           | 2 +-
>  3 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index c23ca34871..c5aa606890 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -296,7 +296,8 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error 
> **errp);
>  void vfio_migration_exit(VFIODevice *vbasedev);
>  
>  int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
> -bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer);
> +bool vfio_devices_all_dirty_tracking_started(
> +    const VFIOContainerBase *bcontainer);
>  bool
>  vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer);
>  int vfio_devices_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 45783982c9..6e4654218a 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -184,8 +184,8 @@ static bool 
> vfio_devices_all_device_dirty_tracking_started(
>      return true;
>  }
>  
> -static bool
> -vfio_devices_all_dirty_tracking_started(const VFIOContainerBase *bcontainer)
> +bool vfio_devices_all_dirty_tracking_started(
> +    const VFIOContainerBase *bcontainer)
>  {
>      if (!migration_is_running()) {
>          return false;
> @@ -235,11 +235,6 @@ bool vfio_devices_all_device_dirty_tracking(const 
> VFIOContainerBase *bcontainer)
>      return true;
>  }
>  
> -bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
> -{
> -    return vfio_devices_all_dirty_tracking_started(bcontainer);
> -}
> -

Why not just renaming vfio_dma_unmap_dirty_sync_needed to
vfio_devices_all_dirty_tracking_started() off from the start? We are introducing
that helper name to just remove it again. And this patch wouldn't be needed 
anymore

>  static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>  {
>      return (!memory_region_is_ram(section->mr) &&
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 8107873534..15deffe3e4 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -131,7 +131,7 @@ static int vfio_legacy_dma_unmap(const VFIOContainerBase 
> *bcontainer,
>      int ret;
>      Error *local_err = NULL;
>  
> -    if (iotlb && vfio_dma_unmap_dirty_sync_needed(bcontainer)) {
> +    if (iotlb && vfio_devices_all_dirty_tracking_started(bcontainer)) {
>          if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
>              bcontainer->dirty_pages_supported) {
>              return vfio_dma_unmap_bitmap(container, iova, size, iotlb);