Re: [RFC 10/20] iommu/iommufd: Add IOMMU_DEVICE_GET_INFO

From: david@xxxxxxxxxxxxxxxxxxxxx
Date: Wed Sep 29 2021 - 02:27:59 EST


On Wed, Sep 22, 2021 at 09:41:50AM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 22, 2021 at 03:30:09AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > Sent: Wednesday, September 22, 2021 1:41 AM
> > >
> > > On Sun, Sep 19, 2021 at 02:38:38PM +0800, Liu Yi L wrote:
> > > > After a device is bound to the iommufd, userspace can use this interface
> > > > to query the underlying iommu capability and format info for this device.
> > > > Based on this information the user then creates I/O address space in a
> > > > compatible format with the to-be-attached devices.
> > > >
> > > > Device cookie which is registered at binding time is used to mark the
> > > > device which is being queried here.
> > > >
> > > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> > > > drivers/iommu/iommufd/iommufd.c | 68
> > > +++++++++++++++++++++++++++++++++
> > > > include/uapi/linux/iommu.h | 49 ++++++++++++++++++++++++
> > > > 2 files changed, 117 insertions(+)
> > > >
> > > > diff --git a/drivers/iommu/iommufd/iommufd.c
> > > b/drivers/iommu/iommufd/iommufd.c
> > > > index e16ca21e4534..641f199f2d41 100644
> > > > +++ b/drivers/iommu/iommufd/iommufd.c
> > > > @@ -117,6 +117,71 @@ static int iommufd_fops_release(struct inode
> > > *inode, struct file *filep)
> > > > return 0;
> > > > }
> > > >
> > > > +static struct device *
> > > > +iommu_find_device_from_cookie(struct iommufd_ctx *ictx, u64
> > > dev_cookie)
> > > > +{
> > >
> > > We have an xarray ID for the device, why are we allowing userspace to
> > > use the dev_cookie as input?
> > >
> > > Userspace should always pass in the ID. The only place dev_cookie
> > > should appear is if the kernel generates an event back to
> > > userspace. Then the kernel should return both the ID and the
> > > dev_cookie in the event to allow userspace to correlate it.
> > >
> >
> > A little background.
> >
> > In earlier design proposal we discussed two options. One is to return
> > an kernel-allocated ID (label) to userspace. The other is to have user
> > register a cookie and use it in iommufd uAPI. At that time the two
> > options were discussed exclusively and the cookie one is preferred.
> >
> > Now you instead recommended a mixed option. We can follow it for
> > sure if nobody objects.
>
> Either or for the return is fine, I'd return both just because it is
> more flexable
>
> But the cookie should never be an input from userspace, and the kernel
> should never search for it. Locating the kernel object is what the ID
> and xarray is for.

Why do we need two IDs at all? Can't we just use the cookie as the
sole ID?

--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature