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@xxxxxxxxxx> 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