On 5/2/22 23:04, Jason Gunthorpe wrote: > 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. >> Though one knows (in the interim abstractions) at start of guest, the underlying memory map e.g. that you can only have <this much> for hotpluggable memory. Albeit I can't remember (need to check) if a memory listener infra gets propagated with everything at start (including the non present stuff like hotplug MR). >>> 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. It allows a bitmap big enough to marshal 64G of IOVA space to it, albeit it only does a section at a time (128M) i.e. one 4K page or iow 32K bits at a time. However the callers will only set bits in the bitmap for a given PTE page size, so we are always limited by PTE page size being recorded at the bitmap page-size granularity. The limit is implicitly bound by the IOVA range being passed in and bitmap page size. So this means it's the bitmap size necessary to record an iova range of length ULONG_MAX.
