Re: [PATCH RFC] vfio: Introduce DMA logging uAPIs for VFIO device

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2 May 2022 16:25:41 -0300
Jason Gunthorpe <jgg@nvidia.com> wrote:

> On Mon, May 02, 2022 at 01:07:01PM -0600, Alex Williamson wrote:
> 
> > > +/*
> > > + * Upon VFIO_DEVICE_FEATURE_SET stop device DMA logging that was started
> > > + * by VFIO_DEVICE_FEATURE_DMA_LOGGING_START
> > > + */
> > > +#define VFIO_DEVICE_FEATURE_DMA_LOGGING_STOP 4  
> > 
> > This seems difficult to use from a QEMU perspective, where a vfio
> > device typically operates on a MemoryListener and we only have
> > visibility to one range at a time.  I don't see any indication that
> > LOGGING_START is meant to be cumulative such that userspace could
> > incrementally add ranges to be watched, nor clearly does LOGGING_STOP
> > appear to have any sort of IOVA range granularity.    
> 
> Correct, at least mlx5 HW just cannot do a change tracking operation,
> so userspace must pre-select some kind of IOVA range to monitor based
> on the current VM configuration.
> 
> > Is userspace intended to pass the full vCPU physical address range
> > here, and if so would a single min/max IOVA be sufficient?    
> 
> At least mlx5 doesn't have enough capacity for that. Some reasonable
> in-between of the current address space, and maybe a speculative extra
> for hot plug.

Ah great, implicit limitations not reported to the user that I hadn't
even guessed!  How does a user learn about any limitations in the
number of ranges or size of each range?
 
> Multi-range is needed to support some arch cases that have a big
> discontinuity in their IOVA space, like PPC for instance.
> 
> > I'm not sure how else we could support memory hotplug while this was
> > enabled.  
> 
> Most likely memory hot plug events will have to take a 'everything got
> dirty' hit during pre-copy - not much else we can do with this HW.
> 
> > How does this work with IOMMU based tracking, I assume that if devices
> > share an IOAS we wouldn't be able to exclude devices supporting
> > device-level tracking from the IOAS log.  
> 
> Exclusion is possible, the userspace would have to manually create
> iommu_domains and attach devices to them with the idea that only
> iommu_domains for devices it wants to track would have dma dirty
> tracking turned on.

Well yeah, but that's the separate IOAS solution.

> But I'm not sure it makes much sense, if the iommu will track dirties,
> and you have to use it for another devices, then it is unlikely there
> will be a performance win to inject a device tracker as well.
> 
> In any case the two interfaces are orthogonal, I would not expect
> userspace to want to track with both, but if it does everything does
> work.
> 
> > Is there a bitmap size limit?    
> 
> Joao's code doesn't have a limit, it pulls user pages incrementally as
> it goes.
> 
> I'm expecting VFIO devices to use the same bitmap library as the IOMMU
> drivers so we have a consistent reporting.

I haven't reviewed that series in any detail yet, but it seems to
impose the same bitmap size and reporting to userspace features as
type1 based in internal limits of bitmap_set().  Thanks,

Alex