RE: [PATCH v3 7/8] vfio/migration: Add migration blocker if VM memory is too large to cause unmap_bitmap failure

[Duan, Zhenzhong]

>-----Original Message-----
>From: Avihai Horon <avih...@nvidia.com>
>Subject: Re: [PATCH v3 7/8] vfio/migration: Add migration blocker if VM
>memory is too large to cause unmap_bitmap failure
>
>
>On 28/10/2025 11:32, Duan, Zhenzhong wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> Hi
>>
>>> -----Original Message-----
>>> From: Avihai Horon <avih...@nvidia.com>
>>> Subject: Re: [PATCH v3 7/8] vfio/migration: Add migration blocker if VM
>>> memory is too large to cause unmap_bitmap failure
>>>
>>> Hi,
>>>
>>> On 24/10/2025 5:09, Zhenzhong Duan wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> With default config, kernel VFIO IOMMU type1 driver limits dirty bitmap
>to
>>>> 256MB for unmap_bitmap ioctl so the maximum guest memory region is
>no
>>> more
>>>> than 8TB size for the ioctl to succeed.
>>>>
>>>> Be conservative here to limit total guest memory to max value supported
>>>> by unmap_bitmap ioctl or else add a migration blocker. IOMMUFD
>backend
>>>> doesn't have such limit, one can use IOMMUFD backed device if there is a
>>>> need to migration such large VM.
>>>>
>>>> Suggested-by: Yi Liu <yi.l....@intel.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
>>>> ---
>>>>    hw/vfio/iommufd.c   |  5 +++++
>>>>    hw/vfio/migration.c | 33 +++++++++++++++++++++++++++++++++
>>>>    2 files changed, 38 insertions(+)
>>>>
>>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>>> index ba5c6b6586..8de765c769 100644
>>>> --- a/hw/vfio/iommufd.c
>>>> +++ b/hw/vfio/iommufd.c
>>>> @@ -633,6 +633,11 @@ skip_ioas_alloc:
>>>>        QLIST_INIT(&container->hwpt_list);
>>>>
>>>>        bcontainer = VFIO_IOMMU(container);
>>>> +
>>>> +    /* There is no limitation on dirty bitmap size in IOMMUFD */
>>>> +    bcontainer->max_dirty_bitmap_size = UINT64_MAX;
>>>> +    bcontainer->dirty_pgsizes = qemu_real_host_page_size();
>>>> +
>>>>        vfio_address_space_insert(space, bcontainer);
>>>>
>>>>        if (!iommufd_cdev_attach_container(vbasedev, container, errp))
>{
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> index 4c06e3db93..44bab024b7 100644
>>>> --- a/hw/vfio/migration.c
>>>> +++ b/hw/vfio/migration.c
>>>> @@ -16,6 +16,7 @@
>>>>    #include <sys/ioctl.h>
>>>>
>>>>    #include "system/runstate.h"
>>>> +#include "hw/boards.h"
>>>>    #include "hw/vfio/vfio-device.h"
>>>>    #include "hw/vfio/vfio-migration.h"
>>>>    #include "migration/misc.h"
>>>> @@ -1152,6 +1153,31 @@ static bool vfio_viommu_preset(VFIODevice
>>> *vbasedev)
>>>>        return vbasedev->bcontainer->space->as !=
>>> &address_space_memory;
>>>>    }
>>>>
>>>> +static bool vfio_dirty_tracking_exceed_limit(VFIODevice *vbasedev)
>>>> +{
>>>> +    VFIOContainer *bcontainer = vbasedev->bcontainer;
>>>> +    uint64_t max_size, page_size;
>>>> +
>>> IIUC, this check is not relevant when using VFIO device dirty tracking,
>>> so maybe bail early if VFIO device dirty tracking is used?
>>> E.g.:
>>>
>>> if (vbasedev->dirty_pages_supported
>>> && vbasedev->device_dirty_page_tracking != ON_OFF_AUTO_OFF) {
>>>      return false;
>>> }
>>>
>>> And replace this with vfio_device_dirty_pages_disabled() in patch #8?
>> OK, will do.
>> Previously, I thought 8TB is a size of super large that device dirty tracking
>will never support due to hardware limit.
>
>Yes, it could be, but what confused me is that we check
>bcontainer->dirty_pages_supported also when using device DPT, although
>it's not relevant for it AFAIU.
>So even with memory size < 8TB, if device DPT is used and
>bcontainer->dirty_pages_supported == false, migration would be blocked.

OK, that's an issue, will fix it.

Thanks
Zhenzhong