>-----Original Message----- >From: Joao Martins <[email protected]> >Subject: Re: [PATCH v4 08/10] vfio/migration: Fix a check on >vbasedev->iommu_dirty_tracking > >On 30/10/2025 13:06, Duan, Zhenzhong wrote: >>> -----Original Message----- >>> From: Joao Martins <[email protected]> >>> Subject: Re: [PATCH v4 08/10] vfio/migration: Fix a check on >>> vbasedev->iommu_dirty_tracking >>> >>> On 30/10/2025 09:10, Duan, Zhenzhong wrote: >>>>> From: Avihai Horon <[email protected]> >>>>> On 29/10/2025 11:53, Zhenzhong Duan wrote: >>>>> >>>>> 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. >>>> >>> >>> I think the real question is why you are using type1 backend overall for >>> purposes of dirty tracking. >> >> Just because we had set bcontainer->dirty_pages_supported = true, the >dirty tracking >> by query through kernel VFIO type1 interface is used during migration, even >though it's 'perpectual'. >> > >Right but that's what iommu_dirty_tracking is for. iommu_dirty_tracking >should >only be set for something that actually uses IOMMU for dirty tracking. And >this >patch does the opposite of that. > >See below on the perpectual part. > >>> >>> type1 dirty tracking just returns everything in the bitmap as 1s. There's no >>> actual dirty tracking and we usually call 'perpectual' dirty tracking >>> because >>> everything DMA mapped as write is always returned as dirty no matter >what >>> you >>> do. It doesn't look at pagetable neither for the unmap get dirty. >>> >>> It's only (known) use has been for testing (in the lack of IOMMU HW + >>> IOMMUFD) >> >> You mean testing live migration or the legacy VFIO type1 dirty tracking >interface? >> If you mean the first, we can force enable it with 'enable-migration=on' and >set >> bcontainer->dirty_pages_supported = false. >> > >I'm aware. I meant testing of dirty_pages_supported code path before >IOMMUFD >existed, or in the lack of IOMMU hardware. > >>> >>> But reading your statement in a different way: you are saying that you use >>> *two* >>> backends at the same time? Why would you do that? >> >> I mean patch7 is needed no matter legacy backend or IOMMUFD backend. >> >> btw: we do support a qemu cmdline with mixed backend, e.g., two devices, >> one backed by legacy backend and the other backed by IOMMUFD. But I'm >not mean that. >patch 7 I don't disagree. But this path 8 doesn't make sense, and it's not >fixing anything but rather introducing wrong behavior I think (also the Fixes: >wouild be wrong if you're code is setting iommu_dirty_tracking). It is setting >iommu_dirty_tracking on something that does not anything iommu dirty >tracking >related. For type1 container it should be false. > >iommu dirty tracking is purposedly there to make the distinct difference that >type1 dirty tracking is *not* actual IOMMU dirty tracking. And that the >migration *only* goes/is-allowed automatically if either you have device dirty >page tracking or IOMMU dirty tracking, but not on type1 legacy dirty >tracking .. >unless you force it (with enable-migration=on). > >Imagine type1 dirty tracking actually getting treated like >iommu_dirty_tracking: >it means you are telling users that 1TB guests migration with VFs that don't >have device dirty tracking without IOMMU HW can be migrated *by default*. >And in >pratice that means they transfer 1TB of RAM during stop-copy. And have >them like >1min downtime.
Clear, so it's intentional to have dirty_pages_supported = true and iommu dirty tracking = false for legacy backend, I'll drop this patch. Thank you all. BRs, Zhenzhong
