>-----Original Message-----
>From: Avihai Horon <[email protected]>
>Subject: Re: [PATCH v4 08/10] vfio/migration: Fix a check on
>vbasedev->iommu_dirty_tracking
>
>
>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 <[email protected]>
>> ---
>> 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).
Yes.
>Thus, when vbasedev->iommu_dirty_tracking was introduced, we
>intentionally set it only if IOMMUFD dirty tracking was supported.
Then we have conflict setting of vbasedev->iommu_dirty_tracking and
bcontainer->dirty_pages_supported.
We had set bcontainer->dirty_pages_supported = true in
vfio_get_iommu_info_migration().
bcontainer->dirty_pages_supported is checked in dirty tracking related
functions of container scope,
e.g., vfio_container_set_dirty_page_tracking() and
vfio_container_query_dirty_bitmap(). If we take the
legacy backend ditry tracking as not supported, we should set it to false, is
that work for you?
>
>So, I don't think this patch needed.
Yes, if we set bcontainer->dirty_pages_supported = false, this patch and patch9
are both not 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?
patch7 is a real use case, in guest, we switch device from IOMMU domain to
block domain and see the issue.
We need to send accurate unmap notification with actual mapping during
migration, for both backend.
patch9 is specifically for unmap_bitmap(), it's needed as long as we set
bcontainer->dirty_pages_supported = true for legacy backend,
because vfio_legacy_dma_unmap_one checks bcontainer->dirty_pages_supported.
Thanks
Zhenzhong
