diff mbox series

[3/9] vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic

Message ID 20241216094638.26406-4-avihaih@nvidia.com (mailing list archive)
State New
Headers show
Series migration: Drop/unexport migration_is_device() and migration_is_active() | expand

Commit Message

Avihai Horon Dec. 16, 2024, 9:46 a.m. UTC
During DMA unmap with vIOMMU, vfio_devices_all_running_and_mig_active()
is used to check whether a dirty page log sync of the unmapped pages is
required. Such log sync is needed during migration pre-copy phase, and
the current logic detects it by checking if migration is active and if
the VFIO devices are running.

However, recently there has been an effort to simplify the migration
status API and reduce it to a single migration_is_running() function.

To accommodate this, refactor vfio_devices_all_running_and_mig_active()
logic so it won't use migration_is_active().

Do it by modifying the logic to check if migration is running and dirty
tracking has been started. This should be equivalent to the previous
logic because when the guest is stopped there shouldn't be DMA unmaps
coming from it. Also rename the function properly.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/hw/vfio/vfio-common.h |  3 +--
 hw/vfio/common.c              | 28 ++++------------------------
 hw/vfio/container.c           |  2 +-
 3 files changed, 6 insertions(+), 27 deletions(-)

Comments

Joao Martins Dec. 16, 2024, 12:45 p.m. UTC | #1
On 16/12/2024 09:46, Avihai Horon wrote:
> During DMA unmap with vIOMMU, vfio_devices_all_running_and_mig_active()
> is used to check whether a dirty page log sync of the unmapped pages is
> required. Such log sync is needed during migration pre-copy phase, and
> the current logic detects it by checking if migration is active and if
> the VFIO devices are running.
> 
> However, recently there has been an effort to simplify the migration
> status API and reduce it to a single migration_is_running() function.
> 
> To accommodate this, refactor vfio_devices_all_running_and_mig_active()
> logic so it won't use migration_is_active().
> 
> Do it by modifying the logic to check if migration is running and dirty
> tracking has been started. This should be equivalent to the previous
> logic because when the guest is stopped there shouldn't be DMA unmaps
> coming from it. Also rename the function properly.
> 
> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> ---
>  include/hw/vfio/vfio-common.h |  3 +--
>  hw/vfio/common.c              | 28 ++++------------------------
>  hw/vfio/container.c           |  2 +-
>  3 files changed, 6 insertions(+), 27 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index e0ce6ec3a9..c23ca34871 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -296,8 +296,7 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>  void vfio_migration_exit(VFIODevice *vbasedev);
>  
>  int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
> -bool
> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
> +bool vfio_dma_unmap_dirty_sync_needed(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 a99796403e..81fba81a6f 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -229,34 +229,14 @@ bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
>      return true;
>  }
>  
> -/*
> - * Check if all VFIO devices are running and migration is active, which is
> - * essentially equivalent to the migration being in pre-copy phase.
> - */
> -bool
> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer)
> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
>  {
> -    VFIODevice *vbasedev;
> -
> -    if (!migration_is_active()) {
> +    if (!migration_is_running()) {
>          return false;
>      }
>  
> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
> -        VFIOMigration *migration = vbasedev->migration;
> -
> -        if (!migration) {
> -            return false;
> -        }
> -
> -        if (vfio_device_state_is_running(vbasedev) ||
> -            vfio_device_state_is_precopy(vbasedev)) {
> -            continue;
> -        } else {
> -            return false;
> -        }

Functionally the change implies that even if non-migratable VFIO devices behind
IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK as it
increases the coverage for calc-dirty-rate (provided my comment in an earlier
patch) such that if you try to get a dirty rate included the IOMMU invalidations
marking the bits accordingly.

Just stating the obvious in case this was non intended.

> -    }
> -    return true;
> +    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
> +           bcontainer->dirty_pages_started;
>  }
>  
>  static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 9ccdb639ac..8107873534 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_devices_all_running_and_mig_active(bcontainer)) {
> +    if (iotlb && vfio_dma_unmap_dirty_sync_needed(bcontainer)) {
>          if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
>              bcontainer->dirty_pages_supported) {
>              return vfio_dma_unmap_bitmap(container, iova, size, iotlb);

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Avihai Horon Dec. 16, 2024, 2:55 p.m. UTC | #2
On 16/12/2024 14:45, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> On 16/12/2024 09:46, Avihai Horon wrote:
>> During DMA unmap with vIOMMU, vfio_devices_all_running_and_mig_active()
>> is used to check whether a dirty page log sync of the unmapped pages is
>> required. Such log sync is needed during migration pre-copy phase, and
>> the current logic detects it by checking if migration is active and if
>> the VFIO devices are running.
>>
>> However, recently there has been an effort to simplify the migration
>> status API and reduce it to a single migration_is_running() function.
>>
>> To accommodate this, refactor vfio_devices_all_running_and_mig_active()
>> logic so it won't use migration_is_active().
>>
>> Do it by modifying the logic to check if migration is running and dirty
>> tracking has been started. This should be equivalent to the previous
>> logic because when the guest is stopped there shouldn't be DMA unmaps
>> coming from it. Also rename the function properly.
>>
>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> ---
>>   include/hw/vfio/vfio-common.h |  3 +--
>>   hw/vfio/common.c              | 28 ++++------------------------
>>   hw/vfio/container.c           |  2 +-
>>   3 files changed, 6 insertions(+), 27 deletions(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index e0ce6ec3a9..c23ca34871 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -296,8 +296,7 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
>>   void vfio_migration_exit(VFIODevice *vbasedev);
>>
>>   int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>> -bool
>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
>> +bool vfio_dma_unmap_dirty_sync_needed(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 a99796403e..81fba81a6f 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -229,34 +229,14 @@ bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
>>       return true;
>>   }
>>
>> -/*
>> - * Check if all VFIO devices are running and migration is active, which is
>> - * essentially equivalent to the migration being in pre-copy phase.
>> - */
>> -bool
>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer)
>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
>>   {
>> -    VFIODevice *vbasedev;
>> -
>> -    if (!migration_is_active()) {
>> +    if (!migration_is_running()) {
>>           return false;
>>       }
>>
>> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>> -        VFIOMigration *migration = vbasedev->migration;
>> -
>> -        if (!migration) {
>> -            return false;
>> -        }
>> -
>> -        if (vfio_device_state_is_running(vbasedev) ||
>> -            vfio_device_state_is_precopy(vbasedev)) {
>> -            continue;
>> -        } else {
>> -            return false;
>> -        }
> Functionally the change implies that even if non-migratable VFIO devices behind
> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK as it
> increases the coverage for calc-dirty-rate (provided my comment in an earlier
> patch) such that if you try to get a dirty rate included the IOMMU invalidations
> marking the bits accordingly.

We still have the "if (!migration_is_running())" check above, so 
non-migratable VFIO devices won't sync.
But that's a valid point for when we'll allow VFIO log syncs for 
clac-dirty-rate.

>
> Just stating the obvious in case this was non intended.
>
>> -    }
>> -    return true;
>> +    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
>> +           bcontainer->dirty_pages_started;
>>   }
>>
>>   static bool vfio_listener_skipped_section(MemoryRegionSection *section)
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 9ccdb639ac..8107873534 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_devices_all_running_and_mig_active(bcontainer)) {
>> +    if (iotlb && vfio_dma_unmap_dirty_sync_needed(bcontainer)) {
>>           if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
>>               bcontainer->dirty_pages_supported) {
>>               return vfio_dma_unmap_bitmap(container, iova, size, iotlb);
> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

Thanks!
Joao Martins Dec. 16, 2024, 3:58 p.m. UTC | #3
On 16/12/2024 14:55, Avihai Horon wrote:
> 
> On 16/12/2024 14:45, Joao Martins wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 16/12/2024 09:46, Avihai Horon wrote:
>>> During DMA unmap with vIOMMU, vfio_devices_all_running_and_mig_active()
>>> is used to check whether a dirty page log sync of the unmapped pages is
>>> required. Such log sync is needed during migration pre-copy phase, and
>>> the current logic detects it by checking if migration is active and if
>>> the VFIO devices are running.
>>>
>>> However, recently there has been an effort to simplify the migration
>>> status API and reduce it to a single migration_is_running() function.
>>>
>>> To accommodate this, refactor vfio_devices_all_running_and_mig_active()
>>> logic so it won't use migration_is_active().
>>>
>>> Do it by modifying the logic to check if migration is running and dirty
>>> tracking has been started. This should be equivalent to the previous
>>> logic because when the guest is stopped there shouldn't be DMA unmaps
>>> coming from it. Also rename the function properly.
>>>
>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>> ---
>>>   include/hw/vfio/vfio-common.h |  3 +--
>>>   hw/vfio/common.c              | 28 ++++------------------------
>>>   hw/vfio/container.c           |  2 +-
>>>   3 files changed, 6 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index e0ce6ec3a9..c23ca34871 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -296,8 +296,7 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error
>>> **errp);
>>>   void vfio_migration_exit(VFIODevice *vbasedev);
>>>
>>>   int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>>> -bool
>>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
>>> +bool vfio_dma_unmap_dirty_sync_needed(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 a99796403e..81fba81a6f 100644
>>> --- a/hw/vfio/common.c
>>> +++ b/hw/vfio/common.c
>>> @@ -229,34 +229,14 @@ bool vfio_devices_all_device_dirty_tracking(const
>>> VFIOContainerBase *bcontainer)
>>>       return true;
>>>   }
>>>
>>> -/*
>>> - * Check if all VFIO devices are running and migration is active, which is
>>> - * essentially equivalent to the migration being in pre-copy phase.
>>> - */
>>> -bool
>>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer)
>>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
>>>   {
>>> -    VFIODevice *vbasedev;
>>> -
>>> -    if (!migration_is_active()) {
>>> +    if (!migration_is_running()) {
>>>           return false;
>>>       }
>>>
>>> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>> -        VFIOMigration *migration = vbasedev->migration;
>>> -
>>> -        if (!migration) {
>>> -            return false;
>>> -        }
>>> -
>>> -        if (vfio_device_state_is_running(vbasedev) ||
>>> -            vfio_device_state_is_precopy(vbasedev)) {
>>> -            continue;
>>> -        } else {
>>> -            return false;
>>> -        }
>> Functionally the change implies that even if non-migratable VFIO devices behind
>> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK as it
>> increases the coverage for calc-dirty-rate (provided my comment in an earlier
>> patch) such that if you try to get a dirty rate included the IOMMU invalidations
>> marking the bits accordingly.
> 
> We still have the "if (!migration_is_running())" check above, so non-migratable
> VFIO devices won't sync.
> But that's a valid point for when we'll allow VFIO log syncs for clac-dirty-rate.
> 

It's the other way around :) This change helps calc-dirty-rate because you can
use it and still account for DMA unmap based dirties.

migration_is_running just stops logs if migration is not running. And that
doesn't care about VFIO migation support.

But if migration is running, whether the device supports migration or not... it
will still sync for pages. It won't sync if it has no VF dirty tracking, but
there's still the container dirty tracker.

Whereby previously, you skip checking all together if the VFIO migration state
wasn't initialized and the VF was not in the right VF device-state.
Avihai Horon Dec. 17, 2024, 9:38 a.m. UTC | #4
On 16/12/2024 17:58, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> On 16/12/2024 14:55, Avihai Horon wrote:
>> On 16/12/2024 14:45, Joao Martins wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 16/12/2024 09:46, Avihai Horon wrote:
>>>> During DMA unmap with vIOMMU, vfio_devices_all_running_and_mig_active()
>>>> is used to check whether a dirty page log sync of the unmapped pages is
>>>> required. Such log sync is needed during migration pre-copy phase, and
>>>> the current logic detects it by checking if migration is active and if
>>>> the VFIO devices are running.
>>>>
>>>> However, recently there has been an effort to simplify the migration
>>>> status API and reduce it to a single migration_is_running() function.
>>>>
>>>> To accommodate this, refactor vfio_devices_all_running_and_mig_active()
>>>> logic so it won't use migration_is_active().
>>>>
>>>> Do it by modifying the logic to check if migration is running and dirty
>>>> tracking has been started. This should be equivalent to the previous
>>>> logic because when the guest is stopped there shouldn't be DMA unmaps
>>>> coming from it. Also rename the function properly.
>>>>
>>>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>>>> ---
>>>>    include/hw/vfio/vfio-common.h |  3 +--
>>>>    hw/vfio/common.c              | 28 ++++------------------------
>>>>    hw/vfio/container.c           |  2 +-
>>>>    3 files changed, 6 insertions(+), 27 deletions(-)
>>>>
>>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>>> index e0ce6ec3a9..c23ca34871 100644
>>>> --- a/include/hw/vfio/vfio-common.h
>>>> +++ b/include/hw/vfio/vfio-common.h
>>>> @@ -296,8 +296,7 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error
>>>> **errp);
>>>>    void vfio_migration_exit(VFIODevice *vbasedev);
>>>>
>>>>    int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
>>>> -bool
>>>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
>>>> +bool vfio_dma_unmap_dirty_sync_needed(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 a99796403e..81fba81a6f 100644
>>>> --- a/hw/vfio/common.c
>>>> +++ b/hw/vfio/common.c
>>>> @@ -229,34 +229,14 @@ bool vfio_devices_all_device_dirty_tracking(const
>>>> VFIOContainerBase *bcontainer)
>>>>        return true;
>>>>    }
>>>>
>>>> -/*
>>>> - * Check if all VFIO devices are running and migration is active, which is
>>>> - * essentially equivalent to the migration being in pre-copy phase.
>>>> - */
>>>> -bool
>>>> -vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer)
>>>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
>>>>    {
>>>> -    VFIODevice *vbasedev;
>>>> -
>>>> -    if (!migration_is_active()) {
>>>> +    if (!migration_is_running()) {
>>>>            return false;
>>>>        }
>>>>
>>>> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>>> -        VFIOMigration *migration = vbasedev->migration;
>>>> -
>>>> -        if (!migration) {
>>>> -            return false;
>>>> -        }
>>>> -
>>>> -        if (vfio_device_state_is_running(vbasedev) ||
>>>> -            vfio_device_state_is_precopy(vbasedev)) {
>>>> -            continue;
>>>> -        } else {
>>>> -            return false;
>>>> -        }
>>> Functionally the change implies that even if non-migratable VFIO devices behind
>>> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK as it
>>> increases the coverage for calc-dirty-rate (provided my comment in an earlier
>>> patch) such that if you try to get a dirty rate included the IOMMU invalidations
>>> marking the bits accordingly.
>> We still have the "if (!migration_is_running())" check above, so non-migratable
>> VFIO devices won't sync.
>> But that's a valid point for when we'll allow VFIO log syncs for clac-dirty-rate.
>>
> It's the other way around :) This change helps calc-dirty-rate because you can
> use it and still account for DMA unmap based dirties.
>
> migration_is_running just stops logs if migration is not running. And that
> doesn't care about VFIO migation support.
>
> But if migration is running, whether the device supports migration or not...

If the device doesn't support migration then migration can't run, no?
But anyway, as we talked in the other thread, I can untie this from 
migration and then, as you said above, it may also dirty sync for 
non-migratable devices which will make calc-dirty-rate more accurate.

>   it
> will still sync for pages. It won't sync if it has no VF dirty tracking, but
> there's still the container dirty tracker.
>
> Whereby previously, you skip checking all together if the VFIO migration state
> wasn't initialized and the VF was not in the right VF device-state.
Joao Martins Dec. 17, 2024, 9:49 a.m. UTC | #5
On 17/12/2024 09:38, Avihai Horon wrote:
>>>>> +bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
>>>>>    {
>>>>> -    VFIODevice *vbasedev;
>>>>> -
>>>>> -    if (!migration_is_active()) {
>>>>> +    if (!migration_is_running()) {
>>>>>            return false;
>>>>>        }
>>>>>
>>>>> -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>>>>> -        VFIOMigration *migration = vbasedev->migration;
>>>>> -
>>>>> -        if (!migration) {
>>>>> -            return false;
>>>>> -        }
>>>>> -
>>>>> -        if (vfio_device_state_is_running(vbasedev) ||
>>>>> -            vfio_device_state_is_precopy(vbasedev)) {
>>>>> -            continue;
>>>>> -        } else {
>>>>> -            return false;
>>>>> -        }
>>>> Functionally the change implies that even if non-migratable VFIO devices behind
>>>> IOMMUs with dirty tracking would still sync DMA bitmap. I think this is OK
>>>> as it
>>>> increases the coverage for calc-dirty-rate (provided my comment in an earlier
>>>> patch) such that if you try to get a dirty rate included the IOMMU
>>>> invalidations
>>>> marking the bits accordingly.
>>> We still have the "if (!migration_is_running())" check above, so non-migratable
>>> VFIO devices won't sync.
>>> But that's a valid point for when we'll allow VFIO log syncs for clac-dirty-
>>> rate.
>>>
>> It's the other way around :) This change helps calc-dirty-rate because you can
>> use it and still account for DMA unmap based dirties.
>>
>> migration_is_running just stops logs if migration is not running. And that
>> doesn't care about VFIO migation support.
>>
>> But if migration is running, whether the device supports migration or not...
> 
> If the device doesn't support migration then migration can't run, no?

/facepalm yes :D

We still have migration blockers

> But anyway, as we talked in the other thread, I can untie this from migration
> and then, as you said above, it may also dirty sync for non-migratable devices
> which will make calc-dirty-rate more accurate.
> 

Yeap
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index e0ce6ec3a9..c23ca34871 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -296,8 +296,7 @@  bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp);
 void vfio_migration_exit(VFIODevice *vbasedev);
 
 int vfio_bitmap_alloc(VFIOBitmap *vbmap, hwaddr size);
-bool
-vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer);
+bool vfio_dma_unmap_dirty_sync_needed(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 a99796403e..81fba81a6f 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -229,34 +229,14 @@  bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
     return true;
 }
 
-/*
- * Check if all VFIO devices are running and migration is active, which is
- * essentially equivalent to the migration being in pre-copy phase.
- */
-bool
-vfio_devices_all_running_and_mig_active(const VFIOContainerBase *bcontainer)
+bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
 {
-    VFIODevice *vbasedev;
-
-    if (!migration_is_active()) {
+    if (!migration_is_running()) {
         return false;
     }
 
-    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
-        VFIOMigration *migration = vbasedev->migration;
-
-        if (!migration) {
-            return false;
-        }
-
-        if (vfio_device_state_is_running(vbasedev) ||
-            vfio_device_state_is_precopy(vbasedev)) {
-            continue;
-        } else {
-            return false;
-        }
-    }
-    return true;
+    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+           bcontainer->dirty_pages_started;
 }
 
 static bool vfio_listener_skipped_section(MemoryRegionSection *section)
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 9ccdb639ac..8107873534 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_devices_all_running_and_mig_active(bcontainer)) {
+    if (iotlb && vfio_dma_unmap_dirty_sync_needed(bcontainer)) {
         if (!vfio_devices_all_device_dirty_tracking(bcontainer) &&
             bcontainer->dirty_pages_supported) {
             return vfio_dma_unmap_bitmap(container, iova, size, iotlb);