>-----Original Message----- >From: Liu, Yi L <[email protected]> >Subject: Re: [PATCH v2 7/8] vfio/migration: Add migration blocker if VM >memory is too large to cause unmap_bitmap failure > >On 2025/10/21 16:25, Duan, Zhenzhong wrote: >> >> >>> -----Original Message----- >>> From: Liu, Yi L <[email protected]> >>> Subject: Re: [PATCH v2 7/8] vfio/migration: Add migration blocker if VM >>> memory is too large to cause unmap_bitmap failure >>> >>> On 2025/10/17 16:22, Zhenzhong Duan wrote: >>>> With default config, kernel VFIO 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 8TB 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 <[email protected]> >>>> Signed-off-by: Zhenzhong Duan <[email protected]> >>>> --- >>>> hw/vfio/migration.c | 37 >+++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 37 insertions(+) >>>> >>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >>>> index 4c06e3db93..1106ca7857 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,35 @@ 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; >>>> + >>>> + if (!object_dynamic_cast(OBJECT(bcontainer), >>> TYPE_VFIO_IOMMU_LEGACY)) { >>>> + return false; >>>> + } >>>> + >>>> + if (!bcontainer->dirty_pages_supported) { >>>> + return true; >>>> + } >>>> + /* >>>> + * VFIO type1 driver has a limitation of bitmap size on >unmap_bitmap >>>> + * ioctl(), calculate the limit and compare with guest memory size >to >>>> + * catch dirty tracking failure early. >>>> + * >>>> + * This limit is 8TB with default kernel and QEMU config, we are a >bit >>>> + * conservative here as VM memory layout may be nonconsecutive >>> or VM >>>> + * can run with vIOMMU enabled so the limitation could be >relaxed. >>> One >>>> + * can also switch to use IOMMUFD backend if there is a need to >>> migrate >>>> + * large VM. >>>> + */ >>>> + page_size = 1 << ctz64(bcontainer->dirty_pgsizes); >>> >>> Should use qemu_real_host_page_size() here? >> >> hmm, I think it's host mmu page size which is not as accurate as the iommu >page sizes? here we want the iommu ones. > >I saw vfio_legacy_query_dirty_bitmap() uses qemu_real_host_page_size() >though kernel enforces min iommu page size. Shall we let >qemu_real_host_page_size() use iommu page size instead of cpu page >size?
qemu_real_host_page_size() is used in vfio_legacy_query_dirty_bitmap() because physical_memory_set_dirty_lebitmap() only supports that size, so we shouldn't change it. bcontainer->dirty_pgsizes should always contains qemu_real_host_page_size() or else ioctl(VFIO_IOMMU_DIRTY_PAGES) will fail. So above code is same effect as using qemu_real_host_page_size(). But I think it's clearer for readers using iommu information for calculating here. Thanks Zhenzhong
