On 4/23/25 19:17, John Levon wrote:
On Wed, Apr 23, 2025 at 07:01:23PM +0200, Cédric Le Goater wrote:
On 4/9/25 15:48, John Levon wrote:
Use the new flags parameter to indicate when we want to unmap
everything; no functional change is intended.
I find these changes confusing. Most likely there are not well presented
or I am missing something. Some more below.
I don't see any way to further break up the change unfortunately.
+/*
+ * DMA - Mapping and unmapping for the "type1" IOMMU interface used on x86
+ */
+static int vfio_legacy_dma_unmap(const VFIOContainerBase *bcontainer,
+ hwaddr iova, ram_addr_t size,
+ IOMMUTLBEntry *iotlb, int flags)
+{
+ int ret;
+
+ if ((flags & ~(VFIO_DMA_UNMAP_FLAG_ALL)) != 0) {
VFIO_DMA_UNMAP_FLAG_ALL is a kernel uapi flag. It should be used only with
the corresponding ioctl(VFIO_IOMMU_UNMAP_DMA) and not internally between
QEMU routines.
Happy to use a different define for the flags if you like, but surely it's
better to have a flags field so it's extendable and it's always clear what the
meaning is? Problem with a boolean is you just see "true" or "false" in the
caller and have no real idea what it means until you look it up.
I think adding a 'bool unmap_all' paremeter to vfio_legacy_dma_unmap() would
make more sense.
Having said that I'm OK with going back to just a simple boolean if you'd really
prefer.
yes. VFIO_DMA_UNMAP_FLAG_ALL is a kernel interface and we don't
need more than one flag today.
}
- ret = vfio_container_dma_unmap(bcontainer, iova,
- int128_get64(llsize), NULL, 0);
+ ret = vfio_container_dma_unmap(bcontainer, iova, int128_get64(llsize),
+ NULL, flags);
Why not unmap the halves here instead of in the backends ?
The whole point of the change is that right now the generic listener.c code has
a workaround that is specific to one particular backend.
It's due to the ARM IO space size AFAICT.
vfio-user doesn't have
any need to unmap in halves and in fact *has* to pass an "unmap all" flag.
OK. So this flag is a vfio-user requirement. Why can't we call
vfio_container_dma_unmap() twice from vfio_listener_region_del() ?
Thanks,
C.
In theory, neither does vfio if the flag is supported, but I dropped that patch
as I couldn't figure out a clean way to use it WRT the dirty tracking code.
regards
john
