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

Tian, Kevin
2021-09-22T03:42:00Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> 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@intel.com>
> > 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. 😊

>
> Eg dma_no_snoop is about creating an IOS with flag NO SNOOP set
>
> TYPE1V2 seems like nonsense

just in case other mapping protocols are introduced in the future

>
> Not sure about the others.
>
> IOW, this should recast to a generic 'query capabilities' IOCTL
>
> Jason