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

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, May 02, 2022 at 01:58:37PM -0600, Alex Williamson wrote:
> 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?

There is some limit of number of ranges and total aggregate address
space, you think we should report rather than try-and-fail?

I guess total address space and total number of ranges is easy to
report, but I don't quite know what userspace will do with it?

> > > 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.

Sure, you can't disable tracking done at the iommu_domain level
without creating different iommu_domains. The IOAS can be shared,
userspace just has to be aware of, or perhaps explicitly control, the
assignment of iommu_domains to devices under the IOAS.

> > 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,

It goes page by page, so the bitmap_set() can't see more than 4k of
bitmap at a time, IIRC.

Jason