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.
