On Mon, May 02, 2022 at 12:11:07PM -0600, Alex Williamson wrote: > On Fri, 29 Apr 2022 05:45:20 +0000 > "Tian, Kevin" <kevin.tian@xxxxxxxxx> wrote: > > > From: Joao Martins <joao.m.martins@xxxxxxxxxx> > > > 3) Unmapping an IOVA range while returning its dirty bit prior to > > > unmap. This case is specific for non-nested vIOMMU case where an > > > erronous guest (or device) DMAing to an address being unmapped at the > > > same time. > > > > an erroneous attempt like above cannot anticipate which DMAs can > > succeed in that window thus the end behavior is undefined. For an > > undefined behavior nothing will be broken by losing some bits dirtied > > in the window between reading back dirty bits of the range and > > actually calling unmap. From guest p.o.v. all those are black-box > > hardware logic to serve a virtual iotlb invalidation request which just > > cannot be completed in one cycle. > > > > Hence in reality probably this is not required except to meet vfio > > compat requirement. Just in concept returning dirty bits at unmap > > is more accurate. > > > > I'm slightly inclined to abandon it in iommufd uAPI. > > Sorry, I'm not following why an unmap with returned dirty bitmap > operation is specific to a vIOMMU case, or in fact indicative of some > sort of erroneous, racy behavior of guest or device. It is being compared against the alternative which is to explicitly query dirty then do a normal unmap as two system calls and permit a race. The only case with any difference is if the guest is racing DMA with the unmap - in which case it is already indeterminate for the guest if the DMA will be completed or not. eg on the vIOMMU case if the guest races DMA with unmap then we are already fine with throwing away that DMA because that is how the race resolves during non-migration situations, so resovling it as throwing away the DMA during migration is OK too. > We need the flexibility to support memory hot-unplug operations > during migration, I would have thought that hotplug during migration would simply discard all the data - how does it use the dirty bitmap? > This was implemented as a single operation specifically to avoid > races where ongoing access may be available after retrieving a > snapshot of the bitmap. Thanks, The issue is the cost. On a real iommu elminating the race is expensive as we have to write protect the pages before query dirty, which seems to be an extra IOTLB flush. It is not clear if paying this cost to become atomic is actually something any use case needs. So, I suggest we think about a 3rd op 'write protect and clear dirties' that will be followed by a normal unmap - the extra op will have the extra oveheard and userspace can decide if it wants to pay or not vs the non-atomic read dirties operation. And lets have a use case where this must be atomic before we implement it.. The downside is we loose a little bit of efficiency by unbundling these steps, the upside is that it doesn't require quite as many special iommu_domain/etc paths. (Also Joao, you should probably have a read and do not clear dirty operation with the idea that the next operation will be unmap - then maybe we can avoid IOTLB flushing..) Jason
