Re: [PATCH v4 08/10] vfio/migration: Fix a check on vbasedev->iommu_dirty_tracking

[Avihai Horon]

On 29/10/2025 11:53, Zhenzhong Duan wrote:
External email: Use caution opening links or attachments


VFIO IOMMU type1 claims to support IOMMU dirty tracking by setting
bcontainer->dirty_pages_supported, but in vfio_migration_realize()
vbasedev->iommu_dirty_tracking is checked, we should pass
bcontainer->dirty_pages_supported to vbasedev->iommu_dirty_tracking
in legacy backend so that the check is accurate.

Fixes: 30b916778517 ("vfio/common: Allow disabling device dirty page tracking")
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
  hw/vfio/container-legacy.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/container-legacy.c b/hw/vfio/container-legacy.c
index dd9c4a6a5a..fa726a2733 100644
--- a/hw/vfio/container-legacy.c
+++ b/hw/vfio/container-legacy.c
@@ -845,6 +845,7 @@ static bool vfio_device_get(VFIOGroup *group, const char 
*name,
                              VFIODevice *vbasedev, Error **errp)
  {
      g_autofree struct vfio_device_info *info = NULL;
+    VFIOContainer *bcontainer = VFIO_IOMMU(group->container);
      int fd;

      fd = vfio_cpr_group_get_device_fd(group->fd, name);
@@ -883,7 +884,8 @@ static bool vfio_device_get(VFIOGroup *group, const char 
*name,
          }
      }

-    vfio_device_prepare(vbasedev, VFIO_IOMMU(group->container), info);
+    vfio_device_prepare(vbasedev, bcontainer, info);
+    vbasedev->iommu_dirty_tracking = bcontainer->dirty_pages_supported;

IIRC, we intentionally don't consider VFIO IOMMU type1 dirty tracking as 
a real dirty tracker, because all it does is mark the whole tracked 
address range as dirty (which is the same as not having dirty tracking 
support at all).
Thus, when vbasedev->iommu_dirty_tracking was introduced, we 
intentionally set it only if IOMMUFD dirty tracking was supported.

So, I don't think this patch is needed.

BTW, do you have a real production use case for migration with VFIO 
IOMMU type1 dirty tracking? I mean, is the scenario you described in 
patch #7 a real use case or you just fixed it for completeness?
If there is no use case, maybe patch #9 is not really needed?

Thanks.