RE: [RFC 12/20] iommu/iommufd: Add IOMMU_CHECK_EXTENSION

[Tian, Kevin]

> From: Jason Gunthorpe
> Sent: Wednesday, September 22, 2021 8:55 PM
> 
> On Wed, Sep 22, 2021 at 03:41:50AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > > Sent: Wednesday, September 22, 2021 1:47 AM
> > >
> > > On Sun, Sep 19, 2021 at 02:38:40PM +0800, Liu Yi L wrote:
> > > > As aforementioned, userspace should check extension for what formats
> > > > can be specified when allocating an IOASID. This patch adds such
> > > > interface for userspace. In this RFC, iommufd reports
> EXT_MAP_TYPE1V2
> > > > support and no no-snoop support yet.
> > > >
> > > > Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
> > > >  drivers/iommu/iommufd/iommufd.c |  7 +++++++
> > > >  include/uapi/linux/iommu.h      | 27 +++++++++++++++++++++++++++
> > > >  2 files changed, 34 insertions(+)
> > > >
> > > > diff --git a/drivers/iommu/iommufd/iommufd.c
> > > b/drivers/iommu/iommufd/iommufd.c
> > > > index 4839f128b24a..e45d76359e34 100644
> > > > +++ b/drivers/iommu/iommufd/iommufd.c
> > > > @@ -306,6 +306,13 @@ static long iommufd_fops_unl_ioctl(struct file
> > > *filep,
> > > >  		return ret;
> > > >
> > > >  	switch (cmd) {
> > > > +	case IOMMU_CHECK_EXTENSION:
> > > > +		switch (arg) {
> > > > +		case EXT_MAP_TYPE1V2:
> > > > +			return 1;
> > > > +		default:
> > > > +			return 0;
> > > > +		}
> > > >  	case IOMMU_DEVICE_GET_INFO:
> > > >  		ret = iommufd_get_device_info(ictx, arg);
> > > >  		break;
> > > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> > > > index 5cbd300eb0ee..49731be71213 100644
> > > > +++ b/include/uapi/linux/iommu.h
> > > > @@ -14,6 +14,33 @@
> > > >  #define IOMMU_TYPE	(';')
> > > >  #define IOMMU_BASE	100
> > > >
> > > > +/*
> > > > + * IOMMU_CHECK_EXTENSION - _IO(IOMMU_TYPE, IOMMU_BASE + 0)
> > > > + *
> > > > + * Check whether an uAPI extension is supported.
> > > > + *
> > > > + * It's unlikely that all planned capabilities in IOMMU fd will be ready
> > > > + * in one breath. User should check which uAPI extension is supported
> > > > + * according to its intended usage.
> > > > + *
> > > > + * A rough list of possible extensions may include:
> > > > + *
> > > > + *	- EXT_MAP_TYPE1V2 for vfio type1v2 map semantics;
> > > > + *	- EXT_DMA_NO_SNOOP for no-snoop DMA support;
> > > > + *	- EXT_MAP_NEWTYPE for an enhanced map semantics;
> > > > + *	- EXT_MULTIDEV_GROUP for 1:N iommu group;
> > > > + *	- EXT_IOASID_NESTING for what the name stands;
> > > > + *	- EXT_USER_PAGE_TABLE for user managed page table;
> > > > + *	- EXT_USER_PASID_TABLE for user managed PASID table;
> > > > + *	- EXT_DIRTY_TRACKING for tracking pages dirtied by DMA;
> > > > + *	- ...
> > > > + *
> > > > + * Return: 0 if not supported, 1 if supported.
> > > > + */
> > > > +#define EXT_MAP_TYPE1V2		1
> > > > +#define EXT_DMA_NO_SNOOP	2
> > > > +#define IOMMU_CHECK_EXTENSION	_IO(IOMMU_TYPE,
> > > IOMMU_BASE + 0)
> > >
> > > I generally advocate for a 'try and fail' approach to discovering
> > > compatibility.
> > >
> > > If that doesn't work for the userspace then a query to return a
> > > generic capability flag is the next best idea. Each flag should
> > > clearly define what 'try and fail' it is talking about
> >
> > We don't have strong preference here. Just follow what vfio does
> > today. So Alex's opinion is appreciated here. 😊
> 
> This is a uAPI design, it should follow the current mainstream
> thinking on how to build these things. There is a lot of old stuff in
> vfio that doesn't match the modern thinking. IMHO.
> 
> > > TYPE1V2 seems like nonsense
> >
> > just in case other mapping protocols are introduced in the future
> 
> Well, we should never, ever do that. Allowing PPC and evrything else
> to split in VFIO has created a compelte disaster in userspace. HW
> specific extensions should be modeled as extensions not a wholesale
> replacement of everything.
> 
> I'd say this is part of the modern thinking on uAPI design.
> 
> What I want to strive for is the basic API is usable with all HW - and
> is what something like DPDK can exclusively use.
> 
> An extended API with HW specific facets exists for qemu to use to
> build a HW backed accelereated and featureful vIOMMU emulation.
> 
> The needs of qmeu should not trump the requirement for a universal
> basic API.
> 
> Eg if we can't figure out a basic API version of the PPC range issue
> then that should be punted to a PPC specific API.
> 

sounds good. I may keep an wrong memory on the multiple mapping
protocols thing. 😊