[v3,01/17] iommu: Add new iommu op to create domains owned by userspace

Message ID 20230724110406.107212-2-yi.l.liu@intel.com
State New
Headers
Series iommufd: Add nesting infrastructure |

Commit Message

Yi Liu July 24, 2023, 11:03 a.m. UTC
  From: Lu Baolu <baolu.lu@linux.intel.com>

Introduce a new iommu_domain op to create domains owned by userspace,
e.g. through iommufd. These domains have a few different properties
compares to kernel owned domains:

 - They may be UNMANAGED domains, but created with special parameters.
   For instance aperture size changes/number of levels, different
   IOPTE formats, or other things necessary to make a vIOMMU work

 - We have to track all the memory allocations with GFP_KERNEL_ACCOUNT
   to make the cgroup sandbox stronger

 - Device-specialty domains, such as NESTED domains can be created by
   iommufd.

The new op clearly says the domain is being created by IOMMUFD, that
the domain is intended for userspace use, and it provides a way to pass
a driver specific uAPI structure to customize the created domain to
exactly what the vIOMMU userspace driver requires.

iommu drivers that cannot support VFIO/IOMMUFD should not support this
op. This includes any driver that cannot provide a fully functional
UNMANAGED domain.

This op chooses to make the special parameters opaque to the core. This
suits the current usage model where accessing any of the IOMMU device
special parameters does require a userspace driver that matches the
kernel driver. If a need for common parameters, implemented similarly
by several drivers, arises then there is room in the design to grow a
generic parameter set as well.

This new op for now is only supposed to be used by iommufd, hence no
wrapper for it. iommufd would call the callback directly. As for domain
free, iommufd would use iommu_domain_free().

enum iommu_hwpt_type is defined to differentiate the user parameters use
by different usages. For the allocations that don't require user parameter,
IOMMU_HWPT_TYPE_DEFAULT is defined for backward compatibility. Other types
would be added by future iommu vendor driver extensions.

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Co-developed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 include/linux/iommu.h        | 20 ++++++++++++++++++++
 include/uapi/linux/iommufd.h |  8 ++++++++
 2 files changed, 28 insertions(+)
  

Comments

Tian, Kevin July 28, 2023, 9:37 a.m. UTC | #1
> From: Yi Liu <yi.l.liu@intel.com>
> Sent: Monday, July 24, 2023 7:04 PM
>
> + * @domain_alloc_user: allocate a user iommu domain corresponding to
> the input
> + *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
> + *                     include/uapi/linux/iommufd.h. A returning domain will be
> + *                     set to an IOMMU_DOMAIN_NESTED type, upon valid
> @user_data
> + *                     and @parent that is a kernel-managed domain. Otherwise,
> + *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type.
> Return
> + *                     ERR_PTR on allocation failure.

"If @user_data is valid and @parent points to a kernel-managed domain,
the returning domain is set to IOMMU_DOMAIN_NESTED type. Otherwise
it is set to IOMMU_DOMAIN_UNMANAGED type."

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
  
Jason Gunthorpe July 28, 2023, 4:56 p.m. UTC | #2
On Fri, Jul 28, 2023 at 09:37:21AM +0000, Tian, Kevin wrote:
> > From: Yi Liu <yi.l.liu@intel.com>
> > Sent: Monday, July 24, 2023 7:04 PM
> >
> > + * @domain_alloc_user: allocate a user iommu domain corresponding to
> > the input
> > + *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
> > + *                     include/uapi/linux/iommufd.h. A returning domain will be
> > + *                     set to an IOMMU_DOMAIN_NESTED type, upon valid
> > @user_data
> > + *                     and @parent that is a kernel-managed domain. Otherwise,
> > + *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type.
> > Return
> > + *                     ERR_PTR on allocation failure.
> 
> "If @user_data is valid and @parent points to a kernel-managed domain,
> the returning domain is set to IOMMU_DOMAIN_NESTED type. Otherwise
> it is set to IOMMU_DOMAIN_UNMANAGED type."

 "If @user_data is valid and @parent points to a kernel-managed domain,
 then the returned domain must be the IOMMU_DOMAIN_NESTED type. Otherwise
 the returned domain is IOMMU_DOMAIN_UNMANAGED."

Notice the detail that this API expects the driver to set the type and
fully initialize the domain, including the generic iommu_domain
struct, which is different than alloc_domain.

When we implement this in drivers we should tidy this so all the alloc
flows fully initialize the domain internally.

Jason
  
Yi Liu July 31, 2023, 12:44 p.m. UTC | #3
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Saturday, July 29, 2023 12:56 AM
> 
> On Fri, Jul 28, 2023 at 09:37:21AM +0000, Tian, Kevin wrote:
> > > From: Yi Liu <yi.l.liu@intel.com>
> > > Sent: Monday, July 24, 2023 7:04 PM
> > >
> > > + * @domain_alloc_user: allocate a user iommu domain corresponding to
> > > the input
> > > + *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
> > > + *                     include/uapi/linux/iommufd.h. A returning domain will be
> > > + *                     set to an IOMMU_DOMAIN_NESTED type, upon valid
> > > @user_data
> > > + *                     and @parent that is a kernel-managed domain. Otherwise,
> > > + *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type.
> > > Return
> > > + *                     ERR_PTR on allocation failure.
> >
> > "If @user_data is valid and @parent points to a kernel-managed domain,
> > the returning domain is set to IOMMU_DOMAIN_NESTED type. Otherwise
> > it is set to IOMMU_DOMAIN_UNMANAGED type."
> 
>  "If @user_data is valid and @parent points to a kernel-managed domain,
>  then the returned domain must be the IOMMU_DOMAIN_NESTED type. Otherwise
>  the returned domain is IOMMU_DOMAIN_UNMANAGED."
> 
> Notice the detail that this API expects the driver to set the type and
> fully initialize the domain, including the generic iommu_domain
> struct, which is different than alloc_domain.
> 
> When we implement this in drivers we should tidy this so all the alloc
> flows fully initialize the domain internally.

Yes. this should be documented in the kdoc. Is it?

Regards,
Yi Liu
  
Jason Gunthorpe July 31, 2023, 1:19 p.m. UTC | #4
On Mon, Jul 31, 2023 at 12:44:25PM +0000, Liu, Yi L wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Saturday, July 29, 2023 12:56 AM
> > 
> > On Fri, Jul 28, 2023 at 09:37:21AM +0000, Tian, Kevin wrote:
> > > > From: Yi Liu <yi.l.liu@intel.com>
> > > > Sent: Monday, July 24, 2023 7:04 PM
> > > >
> > > > + * @domain_alloc_user: allocate a user iommu domain corresponding to
> > > > the input
> > > > + *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
> > > > + *                     include/uapi/linux/iommufd.h. A returning domain will be
> > > > + *                     set to an IOMMU_DOMAIN_NESTED type, upon valid
> > > > @user_data
> > > > + *                     and @parent that is a kernel-managed domain. Otherwise,
> > > > + *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type.
> > > > Return
> > > > + *                     ERR_PTR on allocation failure.
> > >
> > > "If @user_data is valid and @parent points to a kernel-managed domain,
> > > the returning domain is set to IOMMU_DOMAIN_NESTED type. Otherwise
> > > it is set to IOMMU_DOMAIN_UNMANAGED type."
> > 
> >  "If @user_data is valid and @parent points to a kernel-managed domain,
> >  then the returned domain must be the IOMMU_DOMAIN_NESTED type. Otherwise
> >  the returned domain is IOMMU_DOMAIN_UNMANAGED."
> > 
> > Notice the detail that this API expects the driver to set the type and
> > fully initialize the domain, including the generic iommu_domain
> > struct, which is different than alloc_domain.
> > 
> > When we implement this in drivers we should tidy this so all the alloc
> > flows fully initialize the domain internally.
> 
> Yes. this should be documented in the kdoc. Is it?

Yeah, maybe it should be mentioned

Jason
  
Nicolin Chen Aug. 3, 2023, 2:28 a.m. UTC | #5
On Fri, Jul 28, 2023 at 01:56:05PM -0300, Jason Gunthorpe wrote:
> On Fri, Jul 28, 2023 at 09:37:21AM +0000, Tian, Kevin wrote:
> > > From: Yi Liu <yi.l.liu@intel.com>
> > > Sent: Monday, July 24, 2023 7:04 PM
> > >
> > > + * @domain_alloc_user: allocate a user iommu domain corresponding to
> > > the input
> > > + *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
> > > + *                     include/uapi/linux/iommufd.h. A returning domain will be
> > > + *                     set to an IOMMU_DOMAIN_NESTED type, upon valid
> > > @user_data
> > > + *                     and @parent that is a kernel-managed domain. Otherwise,
> > > + *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type.
> > > Return
> > > + *                     ERR_PTR on allocation failure.
> > 
> > "If @user_data is valid and @parent points to a kernel-managed domain,
> > the returning domain is set to IOMMU_DOMAIN_NESTED type. Otherwise
> > it is set to IOMMU_DOMAIN_UNMANAGED type."
> 
>  "If @user_data is valid and @parent points to a kernel-managed domain,
>  then the returned domain must be the IOMMU_DOMAIN_NESTED type. Otherwise
>  the returned domain is IOMMU_DOMAIN_UNMANAGED."
> 
> Notice the detail that this API expects the driver to set the type and
> fully initialize the domain, including the generic iommu_domain
> struct, which is different than alloc_domain.
> 
> When we implement this in drivers we should tidy this so all the alloc
> flows fully initialize the domain internally.

Changed to:

+ * @domain_alloc_user: allocate a user iommu domain corresponding to the input
+ *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
+ *                     include/uapi/linux/iommufd.h. Different from domain_alloc
+ *                     it requires iommu driver to fully initialize a new domain
+ *                     including the generic iommu_domain struct. Upon success,
+ *                     if the @user_data is valid and the @parent points to a
+ *                     kernel-managed domain, the type of the new domain must be
+ *                     IOMMU_DOMAIN_NESTED, otherwise be IOMMU_DOMAIN_UNMANAGED.
+ *                     Upon failure, ERR_PTR must be returned.

Thanks
Nic
  

Patch

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 4199e13b34e6..ecbec2627b63 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -226,6 +226,15 @@  struct iommu_iotlb_gather {
 	bool			queued;
 };
 
+/*
+ * The user data to allocate a specific type user iommu domain
+ *
+ * This includes the corresponding driver data structures in
+ * include/uapi/linux/iommufd.h.
+ */
+union iommu_domain_user_data {
+};
+
 /**
  * struct iommu_ops - iommu ops and capabilities
  * @capable: check capability
@@ -235,6 +244,13 @@  struct iommu_iotlb_gather {
  *           after use. Return the data buffer if success, or ERR_PTR on
  *           failure.
  * @domain_alloc: allocate iommu domain
+ * @domain_alloc_user: allocate a user iommu domain corresponding to the input
+ *                     @hwpt_type that is defined as enum iommu_hwpt_type in the
+ *                     include/uapi/linux/iommufd.h. A returning domain will be
+ *                     set to an IOMMU_DOMAIN_NESTED type, upon valid @user_data
+ *                     and @parent that is a kernel-managed domain. Otherwise,
+ *                     it will be set to an IOMMU_DOMAIN_UNMANAGED type. Return
+ *                     ERR_PTR on allocation failure.
  * @probe_device: Add device to iommu driver handling
  * @release_device: Remove device from iommu driver handling
  * @probe_finalize: Do final setup work after the device is added to an IOMMU
@@ -275,6 +291,10 @@  struct iommu_ops {
 
 	/* Domain allocation and freeing by the iommu driver */
 	struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
+	struct iommu_domain *(*domain_alloc_user)(struct device *dev,
+						  enum iommu_hwpt_type hwpt_type,
+						  struct iommu_domain *parent,
+						  const union iommu_domain_user_data *user_data);
 
 	struct iommu_device *(*probe_device)(struct device *dev);
 	void (*release_device)(struct device *dev);
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 4295362e7b44..d38bc54fd5f2 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -347,6 +347,14 @@  struct iommu_vfio_ioas {
 };
 #define IOMMU_VFIO_IOAS _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VFIO_IOAS)
 
+/**
+ * enum iommu_hwpt_type - IOMMU HWPT Type
+ * @IOMMU_HWPT_TYPE_DEFAULT: default
+ */
+enum iommu_hwpt_type {
+	IOMMU_HWPT_TYPE_DEFAULT,
+};
+
 /**
  * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC)
  * @size: sizeof(struct iommu_hwpt_alloc)