Re: [PATCH v3 5/5] intel_iommu: Optimize out some unnecessary UNMAP calls

[Peter Xu]

On Thu, Jun 08, 2023 at 01:27:50PM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 08, 2023 at 11:40:55AM -0400, Peter Xu wrote:
> 
> > > 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.
> 
> qemu has to fix it. The kernel API is object based, not paged
> based. You cannot unmap partions of a prior mapping.
> 
> I assume for this kind of emulation it is doing 4k objects because
> it has no idea what size of mapping the client will use?

MAP is fine, before notify() to VFIO or anything, qemu scans the pgtable
and handles it in page size or huge page size, so it can be >4K but always
guest iommu pgsize aligned.

I think we rely on guest behaving right, so it should also always operate
on that size minimum when mapped huge.  It shouldn't violate the
"per-object" protocol of iommufd.

IIUC the same to vfio type1v2 from that aspect.

It's more about UNMAP batching, but I assume iommufd is fine if it's fine
with holes inside for that case.  The only difference of "not exist" of
-ENOENT seems to be just same as before as long as QEMU treats it as 0 like
before.

Though that does look slightly special, because the whole empty UNMAP
region can be seen as a hole too; not sure when that -ENOENT will be useful
if qemu should always bypass it anyway.  Indeed not a problem to qemu.

> 
> > We can definitely lookup every single object and explicitly unmap, but it
> > loses partial of the point of batching that guest OS does.  
> 
> You don't need every single object, but it would be faster to check
> where things are mapped and then call the kernel correctly instead of
> trying to iterate with the unmapped reults.

Maybe yes.  If so, It'll be great if Zhenzhong could just attach some proof
on that, meanwhile drop the "iommufd UNMAP warnings" section in the commit
message.

Thanks,

-- 
Peter Xu