diff mbox series

[v6,02/10] iommu: Pass in parent domain with user_data to domain_alloc_user op

Message ID 20231024150609.46884-3-yi.l.liu@intel.com
State New
Headers
Series iommufd: Add nesting infrastructure (part 1/2) |

Commit Message

Yi Liu Oct. 24, 2023, 3:06 p.m. UTC 
  domain_alloc_user op already accepts user flags for domain allocation, add
a parent domain pointer and a driver specific user data support as well.

Add a struct iommu_user_data as a bundle of data_ptr/data_len/type from an
iommufd core uAPI structure. Make the user data opaque to the core, since
a userspace driver must match the kernel driver. In the future, if drivers
share some common parameter, there would be a generic parameter as well.

Define an enum iommu_hwpt_data_type (with IOMMU_HWPT_DATA_NONE type) for
iommu drivers to add their own driver specific user data per hw_pagetable.

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>
---
 drivers/iommu/intel/iommu.c          |  7 ++++++-
 drivers/iommu/iommufd/hw_pagetable.c |  3 ++-
 drivers/iommu/iommufd/selftest.c     |  7 ++++++-
 include/linux/iommu.h                | 27 ++++++++++++++++++++++++---
 include/uapi/linux/iommufd.h         |  8 ++++++++
 5 files changed, 46 insertions(+), 6 deletions(-)

Comments

Joao Martins Oct. 24, 2023, 3:56 p.m. UTC | #1 
On 24/10/2023 16:06, Yi Liu wrote:
> domain_alloc_user op already accepts user flags for domain allocation, add
> a parent domain pointer and a driver specific user data support as well.
> 
> Add a struct iommu_user_data as a bundle of data_ptr/data_len/type from an
> iommufd core uAPI structure. Make the user data opaque to the core, since
> a userspace driver must match the kernel driver. In the future, if drivers
> share some common parameter, there would be a generic parameter as well.
> 
> Define an enum iommu_hwpt_data_type (with IOMMU_HWPT_DATA_NONE type) for
> iommu drivers to add their own driver specific user data per hw_pagetable.
> 
> 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>
> ---
>  drivers/iommu/intel/iommu.c          |  7 ++++++-

You are sadly missing AMD IOMMU

This would fix the build and nack the op should parent or user_data be passed:

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index caad10f9cee3..bc747513afcb 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2220,13 +2220,17 @@ static struct iommu_domain
*amd_iommu_domain_alloc(unsigned int type)
 }

 static struct iommu_domain *amd_iommu_domain_alloc_user(struct device *dev,
-                                                       u32 flags)
+                                       u32 flags, struct iommu_domain *parent,
+                                       const struct iommu_user_data *user_data)
 {
        unsigned int type = IOMMU_DOMAIN_UNMANAGED;

        if (flags & ~IOMMU_HWPT_ALLOC_DIRTY_TRACKING)
                return ERR_PTR(-EOPNOTSUPP);

+       if (parent || user_data)
+               return ERR_PTR(-EOPNOTSUPP);
+
        return do_iommu_domain_alloc(type, dev, flags);
 }
Yi Liu Oct. 24, 2023, 4:14 p.m. UTC | #2 
On 2023/10/24 23:56, Joao Martins wrote:
> On 24/10/2023 16:06, Yi Liu wrote:
>> domain_alloc_user op already accepts user flags for domain allocation, add
>> a parent domain pointer and a driver specific user data support as well.
>>
>> Add a struct iommu_user_data as a bundle of data_ptr/data_len/type from an
>> iommufd core uAPI structure. Make the user data opaque to the core, since
>> a userspace driver must match the kernel driver. In the future, if drivers
>> share some common parameter, there would be a generic parameter as well.
>>
>> Define an enum iommu_hwpt_data_type (with IOMMU_HWPT_DATA_NONE type) for
>> iommu drivers to add their own driver specific user data per hw_pagetable.
>>
>> 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>
>> ---
>>   drivers/iommu/intel/iommu.c          |  7 ++++++-
> 
> You are sadly missing AMD IOMMU

good catch.

> 
> This would fix the build and nack the op should parent or user_data be passed:
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index caad10f9cee3..bc747513afcb 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -2220,13 +2220,17 @@ static struct iommu_domain
> *amd_iommu_domain_alloc(unsigned int type)
>   }
> 
>   static struct iommu_domain *amd_iommu_domain_alloc_user(struct device *dev,
> -                                                       u32 flags)
> +                                       u32 flags, struct iommu_domain *parent,
> +                                       const struct iommu_user_data *user_data)
>   {
>          unsigned int type = IOMMU_DOMAIN_UNMANAGED;
> 
>          if (flags & ~IOMMU_HWPT_ALLOC_DIRTY_TRACKING)
>                  return ERR_PTR(-EOPNOTSUPP);
> 
> +       if (parent || user_data)
> +               return ERR_PTR(-EOPNOTSUPP);
> +
>          return do_iommu_domain_alloc(type, dev, flags);
>   }

yes, this should work. @Jason, one more version or just this one with the
above diff from Joao?
Jason Gunthorpe Oct. 24, 2023, 5:23 p.m. UTC | #3 
On Tue, Oct 24, 2023 at 08:06:01AM -0700, Yi Liu wrote:
> domain_alloc_user op already accepts user flags for domain allocation, add
> a parent domain pointer and a driver specific user data support as well.
> 
> Add a struct iommu_user_data as a bundle of data_ptr/data_len/type from an
> iommufd core uAPI structure. Make the user data opaque to the core, since
> a userspace driver must match the kernel driver. In the future, if drivers
> share some common parameter, there would be a generic parameter as well.
> 
> Define an enum iommu_hwpt_data_type (with IOMMU_HWPT_DATA_NONE type) for
> iommu drivers to add their own driver specific user data per hw_pagetable.
> 
> 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>
> ---
>  drivers/iommu/intel/iommu.c          |  7 ++++++-
>  drivers/iommu/iommufd/hw_pagetable.c |  3 ++-
>  drivers/iommu/iommufd/selftest.c     |  7 ++++++-
>  include/linux/iommu.h                | 27 ++++++++++++++++++++++++---
>  include/uapi/linux/iommufd.h         |  8 ++++++++
>  5 files changed, 46 insertions(+), 6 deletions(-)

This patch should be immediately before "iommufd: Add a nested HW
pagetable object"

Since it is basically preperation for that patch

> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index c44eecf5d318..fccc6315a520 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -361,6 +361,14 @@ enum iommufd_hwpt_alloc_flags {
>  	IOMMU_HWPT_ALLOC_DIRTY_TRACKING = 1 << 1,
>  };
>  
> +/**
> + * enum iommu_hwpt_data_type - IOMMU HWPT Data Type
> + * @IOMMU_HWPT_DATA_NONE: no data
> + */
> +enum iommu_hwpt_data_type {
> +	IOMMU_HWPT_DATA_NONE,
> +};
> +

And this hunk should go in "iommufd: Add a nested HW pagetable object"

With the rest of the uapi

Jason
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 65f46f1347e6..cb64759b3d95 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4076,7 +4076,9 @@  static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 }
 
 static struct iommu_domain *
-intel_iommu_domain_alloc_user(struct device *dev, u32 flags)
+intel_iommu_domain_alloc_user(struct device *dev, u32 flags,
+			      struct iommu_domain *parent,
+			      const struct iommu_user_data *user_data)
 {
 	struct iommu_domain *domain;
 	struct intel_iommu *iommu;
@@ -4086,6 +4088,9 @@  intel_iommu_domain_alloc_user(struct device *dev, u32 flags)
 	    (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING)))
 		return ERR_PTR(-EOPNOTSUPP);
 
+	if (parent || user_data)
+		return ERR_PTR(-EOPNOTSUPP);
+
 	iommu = device_to_iommu(dev, NULL, NULL);
 	if (!iommu)
 		return ERR_PTR(-ENODEV);
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index c2b742ffeb0c..97b84d681e09 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -96,7 +96,8 @@  iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 	hwpt->ioas = ioas;
 
 	if (ops->domain_alloc_user) {
-		hwpt->domain = ops->domain_alloc_user(idev->dev, flags);
+		hwpt->domain = ops->domain_alloc_user(idev->dev, flags,
+						      NULL, NULL);
 		if (IS_ERR(hwpt->domain)) {
 			rc = PTR_ERR(hwpt->domain);
 			hwpt->domain = NULL;
diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c
index d8551c9d5b6c..a01a38332dc1 100644
--- a/drivers/iommu/iommufd/selftest.c
+++ b/drivers/iommu/iommufd/selftest.c
@@ -241,7 +241,9 @@  static struct iommu_domain *mock_domain_alloc(unsigned int iommu_domain_type)
 }
 
 static struct iommu_domain *
-mock_domain_alloc_user(struct device *dev, u32 flags)
+mock_domain_alloc_user(struct device *dev, u32 flags,
+		       struct iommu_domain *parent,
+		       const struct iommu_user_data *user_data)
 {
 	struct mock_dev *mdev = container_of(dev, struct mock_dev, dev);
 	struct iommu_domain *domain;
@@ -250,6 +252,9 @@  mock_domain_alloc_user(struct device *dev, u32 flags)
 	    (~(IOMMU_HWPT_ALLOC_NEST_PARENT | IOMMU_HWPT_ALLOC_DIRTY_TRACKING)))
 		return ERR_PTR(-EOPNOTSUPP);
 
+	if (parent || user_data)
+		return ERR_PTR(-EOPNOTSUPP);
+
 	if ((flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING) &&
 	    (mdev->flags & MOCK_FLAGS_DEVICE_NO_DIRTY))
 		return ERR_PTR(-EOPNOTSUPP);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index becfc8b725de..a86ce9cb3547 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -265,6 +265,21 @@  struct iommu_dirty_ops {
 				    struct iommu_dirty_bitmap *dirty);
 };
 
+/**
+ * struct iommu_user_data - iommu driver specific user space data info
+ * @type: The data type of the user buffer
+ * @uptr: Pointer to the user buffer for copy_from_user()
+ * @len: The length of the user buffer in bytes
+ *
+ * A user space data is an uAPI that is defined in include/uapi/linux/iommufd.h
+ * @type, @uptr and @len should be just copied from an iommufd core uAPI struct.
+ */
+struct iommu_user_data {
+	unsigned int type;
+	void __user *uptr;
+	size_t len;
+};
+
 /**
  * struct iommu_ops - iommu ops and capabilities
  * @capable: check capability
@@ -279,8 +294,12 @@  struct iommu_dirty_ops {
  *                     parameters as defined in include/uapi/linux/iommufd.h.
  *                     Unlike @domain_alloc, it is called only by IOMMUFD and
  *                     must fully initialize the new domain before return.
- *                     Upon success, a domain is returned. Upon failure,
- *                     ERR_PTR must be returned.
+ *                     Upon success, if the @user_data is valid and the @parent
+ *                     points to a kernel-managed domain, the new domain must be
+ *                     IOMMU_DOMAIN_NESTED type; otherwise, the @parent must be
+ *                     NULL while the @user_data can be optionally provided, the
+ *                     new domain must support __IOMMU_DOMAIN_PAGING.
+ *                     Upon failure, ERR_PTR must be returned.
  * @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
@@ -313,7 +332,9 @@  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, u32 flags);
+	struct iommu_domain *(*domain_alloc_user)(struct device *dev, u32 flags,
+						  struct iommu_domain *parent,
+						  const struct iommu_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 c44eecf5d318..fccc6315a520 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -361,6 +361,14 @@  enum iommufd_hwpt_alloc_flags {
 	IOMMU_HWPT_ALLOC_DIRTY_TRACKING = 1 << 1,
 };
 
+/**
+ * enum iommu_hwpt_data_type - IOMMU HWPT Data Type
+ * @IOMMU_HWPT_DATA_NONE: no data
+ */
+enum iommu_hwpt_data_type {
+	IOMMU_HWPT_DATA_NONE,
+};
+
 /**
  * struct iommu_hwpt_alloc - ioctl(IOMMU_HWPT_ALLOC)
  * @size: sizeof(struct iommu_hwpt_alloc)