>-----Original Message----- >From: Peter Xu <pet...@redhat.com> >Sent: Thursday, June 8, 2023 11:41 PM >To: Jason Gunthorpe <j...@nvidia.com>; Liu, Yi L <yi.l....@intel.com>; Duan, >Zhenzhong <zhenzhong.d...@intel.com> >Cc: Duan, Zhenzhong <zhenzhong.d...@intel.com>; qemu- >de...@nongnu.org; m...@redhat.com; jasow...@redhat.com; >pbonz...@redhat.com; richard.hender...@linaro.org; edua...@habkost.net; >marcel.apfelb...@gmail.com; alex.william...@redhat.com; >c...@redhat.com; da...@redhat.com; phi...@linaro.org; >kwankh...@nvidia.com; c...@nvidia.com; Liu, Yi L <yi.l....@intel.com>; Peng, >Chao P <chao.p.p...@intel.com> >Subject: Re: [PATCH v3 5/5] intel_iommu: Optimize out some unnecessary >UNMAP calls > >On Thu, Jun 08, 2023 at 11:11:15AM -0300, Jason Gunthorpe wrote: >> On Thu, Jun 08, 2023 at 10:05:08AM -0400, Peter Xu wrote: >> >> > IIUC what VFIO does here is it returns succeed if unmap over nothing >rather >> > than failing like iommufd. Curious (like JasonW) on why that retval? I'd >> > assume for returning "how much unmapped" we can at least still return 0 >for >> > nothing. >> >> In iommufd maps are objects, you can only map or unmap entire >> objects. The ability to batch unmap objects by specifying an range >> that spans many is something that was easy to do and that VFIO had, >> but I'm not sure it is actually usefull.. >> >> So asking to unmap an object that is already known not to be mapped is >> actually possibly racy, especially if you consider iommufd's support >> for kernel-side IOVA allocation. It should not be done, or if it is >> done, with user space locking to protect it. >> >> For VFIO, long long ago, VFIO could unmap IOVA page at a time - ie it >> wasn't objects. In this world it made some sense that the unmap would >> 'succeed' as the end result was unmapped. >> >> > Are you probably suggesting that we can probably handle that in QEMU >side >> > on -ENOENT here for iommufd only (a question to Yi?). >> >> Yes, this can be done, ENOENT is reliably returned and qemu doesn't >> use the kernel-side IOVA allocator. >> >> But if there is the proper locks to prevent a map/unmap race, then >> there should also be the proper locks to check that there is no map in >> the first place and avoid the kernel call.. > >The problem is IIRC guest iommu driver can do smart things like batching >invalidations, it means when QEMU gets it from the guest OS it may already >not matching one mapped objects. > >We can definitely lookup every single object and explicitly unmap, but it >loses partial of the point of batching that guest OS does. Logically QEMU >can redirect that batched invalidation into one ioctl() to the host, rather >than a lot of smaller ones. > >While for this specific patch - Zhenzhong/Yi, do you agree that we should >just handle -ENOENT in the iommufd series (I assume it's still under work), >then for this specific patch it's only about performance difference?
Yes, that make sense. Thanks Zhenzhong
