1. Patchew
  2. QEMU
  3. [PATCH 0/9] migration: Drop/unexport migration_is_device() and migration_is_active()
  4. View patch

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

Avihai Horon posted 9 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH 5/9] vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
Posted by Avihai Horon 3 months, 3 weeks ago 
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);
-}
-
 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);
-- 
2.40.1
Re: [PATCH 5/9] vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
Posted by Joao Martins 3 months, 3 weeks ago 
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);
Re: [PATCH 5/9] vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
Posted by Avihai Horon 3 months, 3 weeks ago 
On 16/12/2024 14:57, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> 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

That's what I did at first, however there are several DPT helpers with 
similar names that check similar things but with subtle differences, so 
I thought that having a few changes in a single commit might be confusing.
To ease review and make it super clear I split it to be a step-by-step 
cleanup.

I like it this way, but if you think it's redundant I can merge it again.

Thanks.

>
>>   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);

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.