Series comparison

-[PATCH 0/9] migration: Drop/unexport migration_is_device() and migration_is_active()
+[PATCH v2 0/7] migration: Drop/unexport migration_is_device() and migration_is_active()
 ...
 - VFIO device dirty tracking.
 - Legacy VFIO iommu dirty tracking.
 - vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is
   currently blocked, so I used a patched QEMU to allow it).
+I also tested calc-dirty-rate as now VFIO dirty pages should be included
+in its report, and indeed they are.
 I didn't test it with iommu DPT as I don't have access to such HW.
-Cedric, I remember you said that you have such HW, it would be very
+It would be great if someone with the proper HW could test it.
 helpful if you could test it.
 Patch structure:
--6: Refactor and clean up VFIO dirty page tracking helpers.
+-4: Refactor and clean up VFIO dirty page tracking helpers.
 : Refactor dirty limit code.
--9: Drop/unexport migration_is_device() and migration_is_running().
+-7: Drop/unexport migration_is_device() and migration_is_running().
 Changes from v1 [2]:
 * Bail out early in vfio_container_set_dirty_page_tracking() if dirty
   tracking has already been started/stopped. (Joao)
 * Untied the dirty tracking helpers from migration (i.e., removed
   migration_is_running() check) so VFIO dirty pages will be included in
   calc-dirty-rate. (Joao)
 * Added comment to VFIODevice->dirty_tracking that states it's protected
   by BQL.
 * Added R-b/A-b tags.
 Thanks.
 [1]
 https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/
-Avihai Horon (9):
+[2]
 https://lore.kernel.org/qemu-devel/20241216094638.26406-1-avihaih@nvidia.com/
 Avihai Horon (7):
   vfio/container: Add dirty tracking started flag
   vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic
   vfio/migration: Refactor vfio_devices_all_running_and_mig_active()
     logic
-  vfio/migration: Add vfio_devices_all_dirty_tracking_started() helper
-  vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
   vfio/migration: Rename vfio_devices_all_dirty_tracking()
   system/dirtylimit: Don't use migration_is_active()
   migration: Drop migration_is_device()
   migration: Unexport migration_is_active()
- include/hw/vfio/vfio-common.h         |  4 +-
+ include/hw/vfio/vfio-common.h         |  6 +--
  include/hw/vfio/vfio-container-base.h |  1 +
  include/migration/misc.h              |  2 -
- hw/vfio/common.c                      | 61 ++++++++++++---------------
+ hw/vfio/common.c                      | 57 +++++++++++----------------
- hw/vfio/container-base.c              |  8 +++-
+ hw/vfio/container-base.c              | 12 +++++-
  hw/vfio/container.c                   |  2 +-
- migration/migration.c                 | 23 ++++------
+ migration/migration.c                 | 23 ++++-------
  system/dirtylimit.c                   |  3 +-
-files changed, 48 insertions(+), 56 deletions(-)
+files changed, 49 insertions(+), 57 deletions(-)
 --
 .40.1

-[PATCH 1/9] vfio/container: Add dirty tracking started flag
+[PATCH v2 1/7] vfio/container: Add dirty tracking started flag
 ...
 This will be used in the following patches to allow dirty page syncs
 only if dirty tracking has been started.
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
+Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
 ---
- include/hw/vfio/vfio-container-base.h | 1 +
+ include/hw/vfio/vfio-container-base.h |  1 +
- hw/vfio/container-base.c              | 8 +++++++-
+ hw/vfio/container-base.c              | 12 +++++++++++-
-files changed, 8 insertions(+), 1 deletion(-)
+files changed, 12 insertions(+), 1 deletion(-)
 diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/hw/vfio/vfio-container-base.h
 +++ b/include/hw/vfio/vfio-container-base.h
 ...
          return 0;
      }
      g_assert(vioc->set_dirty_page_tracking);
 -    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
++    if (bcontainer->dirty_pages_started == start) {
++        return 0;
++    }
++
 +    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
 +    if (!ret) {
 +        bcontainer->dirty_pages_started = start;
 +    }
 +
 +    return ret;
  }
  int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
 --
 .40.1

-[PATCH 2/9] vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic
+[PATCH v2 2/7] vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic
 ...
 To accommodate this, refactor vfio_devices_all_dirty_tracking() logic so
 it won't use migration_is_active() and migration_is_device(). Instead,
 use internal VFIO dirty tracking flags.
+As a side effect, now that migration status is no longer used to detect
+dirty tracking status, VFIO log syncs are untied from migration. This
+will make calc-dirty-rate more accurate as now it will also include VFIO
+dirty pages.
+While at it, as VFIODevice->dirty_tracking is now used to detect dirty
+tracking status, add a comment that states how it's protected.
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
 ---
- hw/vfio/common.c | 21 ++++++++++++++++++++-
+ include/hw/vfio/vfio-common.h |  2 +-
-file changed, 20 insertions(+), 1 deletion(-)
+ hw/vfio/common.c              | 17 ++++++++++++++++-
 files changed, 17 insertions(+), 2 deletions(-)
+diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
+index XXXXXXX..XXXXXXX 100644
+--- a/include/hw/vfio/vfio-common.h
++++ b/include/hw/vfio/vfio-common.h
+@@ -XXX,XX +XXX,XX @@ typedef struct VFIODevice {
+     OnOffAuto pre_copy_dirty_page_tracking;
+     OnOffAuto device_dirty_page_tracking;
+     bool dirty_pages_supported;
+-    bool dirty_tracking;
++    bool dirty_tracking; /* Protected by BQL */
+     bool iommu_dirty_tracking;
+     HostIOMMUDevice *hiod;
+     int devid;
 diff --git a/hw/vfio/common.c b/hw/vfio/common.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/vfio/common.c
 +++ b/hw/vfio/common.c
 @@ -XXX,XX +XXX,XX @@ bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
 ...
  static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
  {
      VFIODevice *vbasedev;
 -    if (!migration_is_active() && !migration_is_device()) {
-+    if (!migration_is_running()) {
-+        return false;
-+    }
-+
 +    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
 +          bcontainer->dirty_pages_started)) {
          return false;
      }
 --
 .40.1

-[PATCH 3/9] vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic
+[PATCH v2 3/7] vfio/migration: Refactor vfio_devices_all_running_and_mig_active() logic
 ...
 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().
+logic so it won't use migration_is_active(). Do it by simply checking if
 dirty tracking has been started using internal VFIO flags.
-Do it by modifying the logic to check if migration is running and dirty
+This should be equivalent to the previous logic as during migration
-tracking has been started. This should be equivalent to the previous
+dirty tracking is active and when the guest is stopped there shouldn't
-logic because when the guest is stopped there shouldn't be DMA unmaps
+be DMA unmaps coming from it.
-coming from it. Also rename the function properly.
 As a side effect, now that migration status is no longer used, DMA unmap
 log syncs are untied from migration. This will make calc-dirty-rate more
 accurate as now it will also include VFIO dirty pages that were DMA
 unmapped.
 Also rename the function to properly reflect its new logic and extract
 common code from vfio_devices_all_dirty_tracking().
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
 ---
- include/hw/vfio/vfio-common.h |  3 +--
+ include/hw/vfio/vfio-common.h |  4 ++--
- hw/vfio/common.c              | 28 ++++------------------------
+ hw/vfio/common.c              | 40 +++++++----------------------------
  hw/vfio/container.c           |  2 +-
-files changed, 6 insertions(+), 27 deletions(-)
+files changed, 11 insertions(+), 35 deletions(-)
 diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
 index XXXXXXX..XXXXXXX 100644
 --- a/include/hw/vfio/vfio-common.h
 +++ b/include/hw/vfio/vfio-common.h
 @@ -XXX,XX +XXX,XX @@ 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_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 XXXXXXX..XXXXXXX 100644
 --- a/hw/vfio/common.c
 +++ b/hw/vfio/common.c
+@@ -XXX,XX +XXX,XX @@ static bool vfio_devices_all_device_dirty_tracking_started(
+     return true;
+ }
++bool vfio_devices_all_dirty_tracking_started(
++    const VFIOContainerBase *bcontainer)
++{
++    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
++           bcontainer->dirty_pages_started;
++}
++
+ static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
+ {
+     VFIODevice *vbasedev;
+-    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+-          bcontainer->dirty_pages_started)) {
++    if (!vfio_devices_all_dirty_tracking_started(bcontainer)) {
+         return false;
+     }
 @@ -XXX,XX +XXX,XX @@ 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;
-         return false;
+-    }
-     }
+-
 -    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
 -        VFIOMigration *migration = vbasedev->migration;
 -
 -        if (!migration) {
 -            return false;
 ...
 -        } 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)
+ {
+     return (!memory_region_is_ram(section->mr) &&
 diff --git a/hw/vfio/container.c b/hw/vfio/container.c
 index XXXXXXX..XXXXXXX 100644
 --- a/hw/vfio/container.c
 +++ b/hw/vfio/container.c
 @@ -XXX,XX +XXX,XX @@ 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 (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);
 --
 .40.1

-[PATCH 4/9] vfio/migration: Add vfio_devices_all_dirty_tracking_started() helper
+Deleted patch
-vfio_devices_all_dirty_tracking() and vfio_dma_unmap_dirty_sync_needed()
-share the same code that checks if dirty page tracking has been started.
-Extract it to a common helper vfio_devices_all_dirty_tracking_started().
-Signed-off-by: Avihai Horon <avihaih@nvidia.com>
----
- hw/vfio/common.c | 25 +++++++++++++------------
-file changed, 13 insertions(+), 12 deletions(-)
-diff --git a/hw/vfio/common.c b/hw/vfio/common.c
-index XXXXXXX..XXXXXXX 100644
---- a/hw/vfio/common.c
-+++ b/hw/vfio/common.c
-@@ -XXX,XX +XXX,XX @@ static bool vfio_devices_all_device_dirty_tracking_started(
-     return true;
- }
-+static bool
-+vfio_devices_all_dirty_tracking_started(const VFIOContainerBase *bcontainer)
-+{
-+    if (!migration_is_running()) {
-+        return false;
-+    }
-+
-+    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
-+           bcontainer->dirty_pages_started;
-+}
-+
- static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
- {
-     VFIODevice *vbasedev;
--    if (!migration_is_running()) {
--        return false;
--    }
--
--    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
--          bcontainer->dirty_pages_started)) {
-+    if (!vfio_devices_all_dirty_tracking_started(bcontainer)) {
-         return false;
-     }
-@@ -XXX,XX +XXX,XX @@ bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
- bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
- {
--    if (!migration_is_running()) {
--        return false;
--    }
--
--    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
--           bcontainer->dirty_pages_started;
-+    return vfio_devices_all_dirty_tracking_started(bcontainer);
- }
- static bool vfio_listener_skipped_section(MemoryRegionSection *section)
---
-.40.1

-[PATCH 5/9] vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
+Deleted patch
-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 +-
-files changed, 5 insertions(+), 9 deletions(-)
-diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
-index XXXXXXX..XXXXXXX 100644
---- a/include/hw/vfio/vfio-common.h
-+++ b/include/hw/vfio/vfio-common.h
-@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
---- a/hw/vfio/common.c
-+++ b/hw/vfio/common.c
-@@ -XXX,XX +XXX,XX @@ 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;
-@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
---- a/hw/vfio/container.c
-+++ b/hw/vfio/container.c
-@@ -XXX,XX +XXX,XX @@ 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);
---
-.40.1

-[PATCH 6/9] vfio/migration: Rename vfio_devices_all_dirty_tracking()
+[PATCH v2 4/7] vfio/migration: Rename vfio_devices_all_dirty_tracking()
 ...
 Rename it to vfio_devices_log_sync_needed() which reflects its purpose
 more accurately and makes the code clearer as there are already several
 helpers with similar names.
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
+Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
 ---
  hw/vfio/common.c | 4 ++--
 file changed, 2 insertions(+), 2 deletions(-)
 diff --git a/hw/vfio/common.c b/hw/vfio/common.c
 ...

-[PATCH 7/9] system/dirtylimit: Don't use migration_is_active()
+[PATCH v2 5/7] system/dirtylimit: Don't use migration_is_active()
 ...
 To accommodate this, and since the same functionality can be achieved
 with migration_is_running(), use it instead of migration_is_active().
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
+Reviewed-by: Hyman Huang <yong.huang@smartx.com>
 ---
  system/dirtylimit.c | 3 +--
 file changed, 1 insertion(+), 2 deletions(-)
 diff --git a/system/dirtylimit.c b/system/dirtylimit.c
 ...

-[PATCH 8/9] migration: Drop migration_is_device()
+[PATCH v2 6/7] migration: Drop migration_is_device()
 After being removed from VFIO, migration_is_device() no longer has any
 users. Drop it.
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
+Reviewed-by: Cédric Le Goater <clg@redhat.com>
+Acked-by: Peter Xu <peterx@redhat.com>
 ---
  include/migration/misc.h | 1 -
  migration/migration.c    | 7 -------
 files changed, 8 deletions(-)
 ...
  bool migration_thread_is_self(void)
  {
      MigrationState *s = current_migration;
 --
 .40.1

-[PATCH 9/9] migration: Unexport migration_is_active()
+[PATCH v2 7/7] migration: Unexport migration_is_active()
 ...
 only used in migration.c.
 Unexport it and also relocate it so it can be made static.
 Signed-off-by: Avihai Horon <avihaih@nvidia.com>
+Reviewed-by: Cédric Le Goater <clg@redhat.com>
+Acked-by: Peter Xu <peterx@redhat.com>
 ---
  include/migration/misc.h |  1 -
  migration/migration.c    | 16 ++++++++--------
 files changed, 8 insertions(+), 9 deletions(-)
 ...
  bool migration_thread_is_self(void)
  {
      MigrationState *s = current_migration;
 --
 .40.1

Hello,

This follows up on Peter's series [1] to simplify migration status API
to a single migration_is_running() function.

Peter's series tried to drop migration_is_device() and
migration_is_active(), however VFIO used them to check if dirty page
tracking has been started in order to avoid errors in log sync, so they
couldn't simply be dropped without some preliminary cleanups.

This series handles these preliminary cleanups and eventually drops
migration_is_device() and unexports migration_is_active().

The series has been migration tested with the following:
- VFIO device dirty tracking.
- Legacy VFIO iommu dirty tracking.
- vIOMMU + Legacy VFIO iommu dirty tracking (migration with vIOMMU is
  currently blocked, so I used a patched QEMU to allow it).

I didn't test it with iommu DPT as I don't have access to such HW.
Cedric, I remember you said that you have such HW, it would be very
helpful if you could test it.

Patch structure:
1-6: Refactor and clean up VFIO dirty page tracking helpers.
7: Refactor dirty limit code.
8-9: Drop/unexport migration_is_device() and migration_is_running().

Thanks.

[1]
https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/

Avihai Horon (9):
  vfio/container: Add dirty tracking started flag
  vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic
  vfio/migration: Refactor vfio_devices_all_running_and_mig_active()
    logic
  vfio/migration: Add vfio_devices_all_dirty_tracking_started() helper
  vfio/migration: Drop vfio_dma_unmap_dirty_sync_needed()
  vfio/migration: Rename vfio_devices_all_dirty_tracking()
  system/dirtylimit: Don't use migration_is_active()
  migration: Drop migration_is_device()
  migration: Unexport migration_is_active()

-- 
2.40.1

Add a flag to VFIOContainerBase that indicates whether dirty tracking
has been started for the container or not.

This will be used in the following patches to allow dirty page syncs
only if dirty tracking has been started.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/hw/vfio/vfio-container-base.h | 1 +
 hw/vfio/container-base.c              | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -XXX,XX +XXX,XX @@ typedef struct VFIOContainerBase {
     unsigned long pgsizes;
     unsigned int dma_max_mappings;
     bool dirty_pages_supported;
+    bool dirty_pages_started; /* Protected by BQL */
     QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
     QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
     QLIST_ENTRY(VFIOContainerBase) next;
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -XXX,XX +XXX,XX @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
                                            bool start, Error **errp)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    int ret;
 
     if (!bcontainer->dirty_pages_supported) {
         return 0;
     }
 
     g_assert(vioc->set_dirty_page_tracking);
-    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    if (!ret) {
+        bcontainer->dirty_pages_started = start;
+    }
+
+    return ret;
 }
 
 int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
-- 
2.40.1

During dirty page log sync, vfio_devices_all_dirty_tracking() is used to
check if dirty tracking has been started in order to avoid errors. The
current logic checks if migration is in ACTIVE or DEVICE states to
ensure dirty tracking has been started.

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_dirty_tracking() logic so
it won't use migration_is_active() and migration_is_device(). Instead,
use internal VFIO dirty tracking flags.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 hw/vfio/common.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
            migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
 }
 
+static bool vfio_devices_all_device_dirty_tracking_started(
+    const VFIOContainerBase *bcontainer)
+{
+    VFIODevice *vbasedev;
+
+    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
+        if (!vbasedev->dirty_tracking) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
-    if (!migration_is_active() && !migration_is_device()) {
+    if (!migration_is_running()) {
+        return false;
+    }
+
+    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+          bcontainer->dirty_pages_started)) {
         return false;
     }
 
-- 
2.40.1

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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -XXX,XX +XXX,XX @@ 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);
-- 
2.40.1

vfio_devices_all_dirty_tracking() and vfio_dma_unmap_dirty_sync_needed()
share the same code that checks if dirty page tracking has been started.

Extract it to a common helper vfio_devices_all_dirty_tracking_started().

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 hw/vfio/common.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ static bool vfio_devices_all_device_dirty_tracking_started(
     return true;
 }
 
+static bool
+vfio_devices_all_dirty_tracking_started(const VFIOContainerBase *bcontainer)
+{
+    if (!migration_is_running()) {
+        return false;
+    }
+
+    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+           bcontainer->dirty_pages_started;
+}
+
 static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
-    if (!migration_is_running()) {
-        return false;
-    }
-
-    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
-          bcontainer->dirty_pages_started)) {
+    if (!vfio_devices_all_dirty_tracking_started(bcontainer)) {
         return false;
     }
 
@@ -XXX,XX +XXX,XX @@ bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
 
 bool vfio_dma_unmap_dirty_sync_needed(const VFIOContainerBase *bcontainer)
 {
-    if (!migration_is_running()) {
-        return false;
-    }
-
-    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
-           bcontainer->dirty_pages_started;
+    return vfio_devices_all_dirty_tracking_started(bcontainer);
 }
 
 static bool vfio_listener_skipped_section(MemoryRegionSection *section)
-- 
2.40.1

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 XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ 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;
@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -XXX,XX +XXX,XX @@ 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

vfio_devices_all_dirty_tracking() is used to check if dirty page log
sync is needed. However, besides checking the dirty page tracking
status, it also checks the pre_copy_dirty_page_tracking flag.

Rename it to vfio_devices_log_sync_needed() which reflects its purpose
more accurately and makes the code clearer as there are already several
helpers with similar names.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 hw/vfio/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ bool vfio_devices_all_dirty_tracking_started(
            bcontainer->dirty_pages_started;
 }
 
-static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
+static bool vfio_log_sync_needed(const VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
@@ -XXX,XX +XXX,XX @@ static void vfio_listener_log_sync(MemoryListener *listener,
         return;
     }
 
-    if (vfio_devices_all_dirty_tracking(bcontainer)) {
+    if (vfio_log_sync_needed(bcontainer)) {
         ret = vfio_sync_dirty_bitmap(bcontainer, section, &local_err);
         if (ret) {
             error_report_err(local_err);
-- 
2.40.1

vcpu_dirty_rate_stat_collect() uses migration_is_active() to detect
whether migration is running or not, in order to get the correct dirty
rate period value.

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, and since the same functionality can be achieved
with migration_is_running(), use it instead of migration_is_active().

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 system/dirtylimit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/system/dirtylimit.c b/system/dirtylimit.c
index XXXXXXX..XXXXXXX 100644
--- a/system/dirtylimit.c
+++ b/system/dirtylimit.c
@@ -XXX,XX +XXX,XX @@ static void vcpu_dirty_rate_stat_collect(void)
     int i = 0;
     int64_t period = DIRTYLIMIT_CALC_TIME_MS;
 
-    if (migrate_dirty_limit() &&
-        migration_is_active()) {
+    if (migrate_dirty_limit() && migration_is_running()) {
         period = migrate_vcpu_dirty_limit_period();
     }
 
-- 
2.40.1

After being removed from VFIO, migration_is_device() no longer has any
users. Drop it.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/migration/misc.h | 1 -
 migration/migration.c    | 7 -------
 2 files changed, 8 deletions(-)

After being removed from VFIO and dirty limit, migration_is_active() no
longer has any users outside the migration subsystem, and in fact, it's
only used in migration.c.

Unexport it and also relocate it so it can be made static.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
---
 include/migration/misc.h |  1 -
 migration/migration.c    | 16 ++++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index XXXXXXX..XXXXXXX 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -XXX,XX +XXX,XX @@ void dump_vmstate_json_to_file(FILE *out_fp);
 void migration_object_init(void);
 void migration_shutdown(void);
 
-bool migration_is_active(void);
 bool migration_is_running(void);
 bool migration_thread_is_self(void);
 
diff --git a/migration/migration.c b/migration/migration.c
index XXXXXXX..XXXXXXX 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -XXX,XX +XXX,XX @@ bool migration_is_running(void)
     }
 }
 
+static bool migration_is_active(void)
+{
+    MigrationState *s = current_migration;
+
+    return (s->state == MIGRATION_STATUS_ACTIVE ||
+            s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
+}
+
 static bool migrate_show_downtime(MigrationState *s)
 {
     return (s->state == MIGRATION_STATUS_COMPLETED) || migration_in_postcopy();
@@ -XXX,XX +XXX,XX @@ bool migration_in_bg_snapshot(void)
     return migrate_background_snapshot() && migration_is_running();
 }
 
-bool migration_is_active(void)
-{
-    MigrationState *s = current_migration;
-
-    return (s->state == MIGRATION_STATUS_ACTIVE ||
-            s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
-}
-
 bool migration_thread_is_self(void)
 {
     MigrationState *s = current_migration;
-- 
2.40.1

Hello,

This follows up on Peter's series [1] to simplify migration status API
to a single migration_is_running() function.

This series handles these preliminary cleanups and eventually drops
migration_is_device() and unexports migration_is_active().

I also tested calc-dirty-rate as now VFIO dirty pages should be included
in its report, and indeed they are.

I didn't test it with iommu DPT as I don't have access to such HW.
It would be great if someone with the proper HW could test it.

Patch structure:
1-4: Refactor and clean up VFIO dirty page tracking helpers.
5: Refactor dirty limit code.
6-7: Drop/unexport migration_is_device() and migration_is_running().

Changes from v1 [2]:
* Bail out early in vfio_container_set_dirty_page_tracking() if dirty
  tracking has already been started/stopped. (Joao)
* Untied the dirty tracking helpers from migration (i.e., removed
  migration_is_running() check) so VFIO dirty pages will be included in
  calc-dirty-rate. (Joao)
* Added comment to VFIODevice->dirty_tracking that states it's protected
  by BQL.
* Added R-b/A-b tags.

Thanks.

[1]
https://lore.kernel.org/qemu-devel/20241024213056.1395400-1-peterx@redhat.com/

[2]
https://lore.kernel.org/qemu-devel/20241216094638.26406-1-avihaih@nvidia.com/

Avihai Horon (7):
  vfio/container: Add dirty tracking started flag
  vfio/migration: Refactor vfio_devices_all_dirty_tracking() logic
  vfio/migration: Refactor vfio_devices_all_running_and_mig_active()
    logic
  vfio/migration: Rename vfio_devices_all_dirty_tracking()
  system/dirtylimit: Don't use migration_is_active()
  migration: Drop migration_is_device()
  migration: Unexport migration_is_active()

-- 
2.40.1

Add a flag to VFIOContainerBase that indicates whether dirty tracking
has been started for the container or not.

This will be used in the following patches to allow dirty page syncs
only if dirty tracking has been started.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
---
 include/hw/vfio/vfio-container-base.h |  1 +
 hw/vfio/container-base.c              | 12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/hw/vfio/vfio-container-base.h b/include/hw/vfio/vfio-container-base.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-container-base.h
+++ b/include/hw/vfio/vfio-container-base.h
@@ -XXX,XX +XXX,XX @@ typedef struct VFIOContainerBase {
     unsigned long pgsizes;
     unsigned int dma_max_mappings;
     bool dirty_pages_supported;
+    bool dirty_pages_started; /* Protected by BQL */
     QLIST_HEAD(, VFIOGuestIOMMU) giommu_list;
     QLIST_HEAD(, VFIORamDiscardListener) vrdl_list;
     QLIST_ENTRY(VFIOContainerBase) next;
diff --git a/hw/vfio/container-base.c b/hw/vfio/container-base.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/container-base.c
+++ b/hw/vfio/container-base.c
@@ -XXX,XX +XXX,XX @@ int vfio_container_set_dirty_page_tracking(VFIOContainerBase *bcontainer,
                                            bool start, Error **errp)
 {
     VFIOIOMMUClass *vioc = VFIO_IOMMU_GET_CLASS(bcontainer);
+    int ret;
 
     if (!bcontainer->dirty_pages_supported) {
         return 0;
     }
 
     g_assert(vioc->set_dirty_page_tracking);
-    return vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    if (bcontainer->dirty_pages_started == start) {
+        return 0;
+    }
+
+    ret = vioc->set_dirty_page_tracking(bcontainer, start, errp);
+    if (!ret) {
+        bcontainer->dirty_pages_started = start;
+    }
+
+    return ret;
 }
 
 int vfio_container_query_dirty_bitmap(const VFIOContainerBase *bcontainer,
-- 
2.40.1

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_dirty_tracking() logic so
it won't use migration_is_active() and migration_is_device(). Instead,
use internal VFIO dirty tracking flags.

As a side effect, now that migration status is no longer used to detect
dirty tracking status, VFIO log syncs are untied from migration. This
will make calc-dirty-rate more accurate as now it will also include VFIO
dirty pages.

While at it, as VFIODevice->dirty_tracking is now used to detect dirty
tracking status, add a comment that states how it's protected.

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

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -XXX,XX +XXX,XX @@ typedef struct VFIODevice {
     OnOffAuto pre_copy_dirty_page_tracking;
     OnOffAuto device_dirty_page_tracking;
     bool dirty_pages_supported;
-    bool dirty_tracking;
+    bool dirty_tracking; /* Protected by BQL */
     bool iommu_dirty_tracking;
     HostIOMMUDevice *hiod;
     int devid;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ bool vfio_device_state_is_precopy(VFIODevice *vbasedev)
            migration->device_state == VFIO_DEVICE_STATE_PRE_COPY_P2P;
 }
 
+static bool vfio_devices_all_device_dirty_tracking_started(
+    const VFIOContainerBase *bcontainer)
+{
+    VFIODevice *vbasedev;
+
+    QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
+        if (!vbasedev->dirty_tracking) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
-    if (!migration_is_active() && !migration_is_device()) {
+    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+          bcontainer->dirty_pages_started)) {
         return false;
     }
 
-- 
2.40.1

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 simply checking if
dirty tracking has been started using internal VFIO flags.

This should be equivalent to the previous logic as during migration
dirty tracking is active and when the guest is stopped there shouldn't
be DMA unmaps coming from it.

As a side effect, now that migration status is no longer used, DMA unmap
log syncs are untied from migration. This will make calc-dirty-rate more
accurate as now it will also include VFIO dirty pages that were DMA
unmapped.

Also rename the function to properly reflect its new logic and extract
common code from vfio_devices_all_dirty_tracking().

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

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index XXXXXXX..XXXXXXX 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -XXX,XX +XXX,XX @@ 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_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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -XXX,XX +XXX,XX @@ static bool vfio_devices_all_device_dirty_tracking_started(
     return true;
 }
 
+bool vfio_devices_all_dirty_tracking_started(
+    const VFIOContainerBase *bcontainer)
+{
+    return vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
+           bcontainer->dirty_pages_started;
+}
+
 static bool vfio_devices_all_dirty_tracking(VFIOContainerBase *bcontainer)
 {
     VFIODevice *vbasedev;
 
-    if (!(vfio_devices_all_device_dirty_tracking_started(bcontainer) ||
-          bcontainer->dirty_pages_started)) {
+    if (!vfio_devices_all_dirty_tracking_started(bcontainer)) {
         return false;
     }
 
@@ -XXX,XX +XXX,XX @@ 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)
-{
-    VFIODevice *vbasedev;
-
-    if (!migration_is_active()) {
-        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;
-}
-
 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 XXXXXXX..XXXXXXX 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -XXX,XX +XXX,XX @@ 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_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

vfio_devices_all_dirty_tracking() is used to check if dirty page log
sync is needed. However, besides checking the dirty page tracking
status, it also checks the pre_copy_dirty_page_tracking flag.

Rename it to vfio_devices_log_sync_needed() which reflects its purpose
more accurately and makes the code clearer as there are already several
helpers with similar names.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/vfio/common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

vcpu_dirty_rate_stat_collect() uses migration_is_active() to detect
whether migration is running or not, in order to get the correct dirty
rate period value.

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, and since the same functionality can be achieved
with migration_is_running(), use it instead of migration_is_active().

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Hyman Huang <yong.huang@smartx.com>
---
 system/dirtylimit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

After being removed from VFIO, migration_is_device() no longer has any
users. Drop it.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h | 1 -
 migration/migration.c    | 7 -------
 2 files changed, 8 deletions(-)

After being removed from VFIO and dirty limit, migration_is_active() no
longer has any users outside the migration subsystem, and in fact, it's
only used in migration.c.

Unexport it and also relocate it so it can be made static.

Signed-off-by: Avihai Horon <avihaih@nvidia.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
---
 include/migration/misc.h |  1 -
 migration/migration.c    | 16 ++++++++--------
 2 files changed, 8 insertions(+), 9 deletions(-)