diff mbox series

[1/6] iommu: Add new iommu op to get iommu hardware information

Message ID 20230209041642.9346-2-yi.l.liu@intel.com
State New
Headers
Series iommufd: Add iommu capability reporting |

Commit Message

Yi Liu Feb. 9, 2023, 4:16 a.m. UTC 
  From: Lu Baolu <baolu.lu@linux.intel.com>

Introduce a new iommu op get the IOMMU hardware capabilities for iommufd.
This information will be used by any vIOMMU driver which is owned by
userspace.

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. No warpper API is added as it is supposed to be used by
iommufd only.

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

Comments

Tian, Kevin Feb. 10, 2023, 7:28 a.m. UTC | #1 
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Thursday, February 9, 2023 12:17 PM
> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
>  /**
>   * struct iommu_ops - iommu ops and capabilities
>   * @capable: check capability
> + * @hw_info: IOMMU hardware capabilities. The type of the returned data
> is

is it 'info' or 'capability'?

> + *           defined in include/uapi/linux/iommufd.h. The data buffer is
> + *           allocated in the IOMMU driver and the caller should free it
> + *           after use. Return the data buffer if success, or ERR_PTR on
> + *	      failure.
>   * @domain_alloc: allocate iommu domain
>   * @probe_device: Add device to iommu driver handling
>   * @release_device: Remove device from iommu driver handling
> @@ -252,6 +258,7 @@ struct iommu_iotlb_gather {
>   */
>  struct iommu_ops {
>  	bool (*capable)(struct device *dev, enum iommu_cap);
> +	void *(*hw_info)(struct device *dev, u32 *length);
> 
>  	/* Domain allocation and freeing by the iommu driver */
>  	struct iommu_domain *(*domain_alloc)(unsigned
> iommu_domain_type);
> @@ -280,6 +287,7 @@ struct iommu_ops {
>  	void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);
> 
>  	const struct iommu_domain_ops *default_domain_ops;
> +	enum iommu_device_data_type driver_type;

the enum is called 'device_data_type' while the field is called 'driver_type'.

btw this new field is not documented above.
Baolu Lu Feb. 11, 2023, 3:38 a.m. UTC | #2 
On 2023/2/10 15:28, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Thursday, February 9, 2023 12:17 PM
>> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
>>   /**
>>    * struct iommu_ops - iommu ops and capabilities
>>    * @capable: check capability
>> + * @hw_info: IOMMU hardware capabilities. The type of the returned data
>> is
> 
> is it 'info' or 'capability'?
> 
>> + *           defined in include/uapi/linux/iommufd.h. The data buffer is
>> + *           allocated in the IOMMU driver and the caller should free it
>> + *           after use. Return the data buffer if success, or ERR_PTR on
>> + *	      failure.
>>    * @domain_alloc: allocate iommu domain
>>    * @probe_device: Add device to iommu driver handling
>>    * @release_device: Remove device from iommu driver handling
>> @@ -252,6 +258,7 @@ struct iommu_iotlb_gather {
>>    */
>>   struct iommu_ops {
>>   	bool (*capable)(struct device *dev, enum iommu_cap);
>> +	void *(*hw_info)(struct device *dev, u32 *length);
>>
>>   	/* Domain allocation and freeing by the iommu driver */
>>   	struct iommu_domain *(*domain_alloc)(unsigned
>> iommu_domain_type);
>> @@ -280,6 +287,7 @@ struct iommu_ops {
>>   	void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);
>>
>>   	const struct iommu_domain_ops *default_domain_ops;
>> +	enum iommu_device_data_type driver_type;
> 
> the enum is called 'device_data_type' while the field is called 'driver_type'.

The enum is named from uAPI's p-o-v and driver_type is from IOMMU's.

> btw this new field is not documented above.

Yes. Will do that.

Best regards,
baolu
Baolu Lu Feb. 11, 2023, 3:42 a.m. UTC | #3 
On 2023/2/10 15:28, Tian, Kevin wrote:
>> From: Liu, Yi L<yi.l.liu@intel.com>
>> Sent: Thursday, February 9, 2023 12:17 PM
>> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
>>   /**
>>    * struct iommu_ops - iommu ops and capabilities
>>    * @capable: check capability
>> + * @hw_info: IOMMU hardware capabilities. The type of the returned data
>> is
> is it 'info' or 'capability'?

hw_info. IOMMU core does not care about specific content, so it is not
necessary to define it as capability or anything else.

Perhaps we need to change the comments a bit, say, "IOMMU hardware
information"?

Best regards,
baolu
Tian, Kevin Feb. 13, 2023, 1:54 a.m. UTC | #4 
> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Saturday, February 11, 2023 11:43 AM
> 
> On 2023/2/10 15:28, Tian, Kevin wrote:
> >> From: Liu, Yi L<yi.l.liu@intel.com>
> >> Sent: Thursday, February 9, 2023 12:17 PM
> >> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
> >>   /**
> >>    * struct iommu_ops - iommu ops and capabilities
> >>    * @capable: check capability
> >> + * @hw_info: IOMMU hardware capabilities. The type of the returned
> data
> >> is
> > is it 'info' or 'capability'?
> 
> hw_info. IOMMU core does not care about specific content, so it is not
> necessary to define it as capability or anything else.
> 
> Perhaps we need to change the comments a bit, say, "IOMMU hardware
> information"?
> 

yes, that is probably more consistent.
Binbin Wu Feb. 13, 2023, 2:36 a.m. UTC | #5 
On 2/9/2023 12:16 PM, Yi Liu wrote:
> From: Lu Baolu <baolu.lu@linux.intel.com>
>
> Introduce a new iommu op get

get -> to get


> the IOMMU hardware capabilities for iommufd.
> This information will be used by any vIOMMU driver which is owned by
> userspace.
>
> 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. No warpper

warpper -> wrapper


>   API is added as it is supposed to be used by
> iommufd only.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> ---
>   include/linux/iommu.h        | 8 ++++++++
>   include/uapi/linux/iommufd.h | 6 ++++++
>   2 files changed, 14 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index cb586d054c57..97b398d19fd2 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -15,6 +15,7 @@
>   #include <linux/of.h>
>   #include <linux/ioasid.h>
>   #include <uapi/linux/iommu.h>
> +#include <uapi/linux/iommufd.h>
>   
>   #define IOMMU_READ	(1 << 0)
>   #define IOMMU_WRITE	(1 << 1)
> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
>   /**
>    * struct iommu_ops - iommu ops and capabilities
>    * @capable: check capability
> + * @hw_info: IOMMU hardware capabilities. The type of the returned data is
> + *           defined in include/uapi/linux/iommufd.h. The data buffer is
> + *           allocated in the IOMMU driver and the caller should free it
> + *           after use. Return the data buffer if success, or ERR_PTR on
> + *	      failure.
>    * @domain_alloc: allocate iommu domain
>    * @probe_device: Add device to iommu driver handling
>    * @release_device: Remove device from iommu driver handling
> @@ -252,6 +258,7 @@ struct iommu_iotlb_gather {
>    */
>   struct iommu_ops {
>   	bool (*capable)(struct device *dev, enum iommu_cap);
> +	void *(*hw_info)(struct device *dev, u32 *length);
>   
>   	/* Domain allocation and freeing by the iommu driver */
>   	struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
> @@ -280,6 +287,7 @@ struct iommu_ops {
>   	void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);
>   
>   	const struct iommu_domain_ops *default_domain_ops;
> +	enum iommu_device_data_type driver_type;


How to understand the name "iommu_device_data_type"?
Is it just refer to the driver types or it has a more generic meaning?


>   	unsigned long pgsize_bitmap;
>   	struct module *owner;
>   };
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index 98ebba80cfa1..2309edb55028 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -344,4 +344,10 @@ struct iommu_vfio_ioas {
>   	__u16 __reserved;
>   };
>   #define IOMMU_VFIO_IOAS _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VFIO_IOAS)
> +
> +/**
> + * enum iommu_device_data_type - IOMMU hardware Data types
> + */
> +enum iommu_device_data_type {
> +};
>   #endif
Baolu Lu Feb. 13, 2023, 8:46 a.m. UTC | #6 
On 2023/2/13 10:36, Binbin Wu wrote:
> 
> On 2/9/2023 12:16 PM, Yi Liu wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>
>> Introduce a new iommu op get
> 
> get -> to get
> 
> 
>> the IOMMU hardware capabilities for iommufd.
>> This information will be used by any vIOMMU driver which is owned by
>> userspace.
>>
>> 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. No warpper
> 
> warpper -> wrapper

Thanks, will update.

> 
>>   API is added as it is supposed to be used by
>> iommufd only.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
>> ---
>>   include/linux/iommu.h        | 8 ++++++++
>>   include/uapi/linux/iommufd.h | 6 ++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index cb586d054c57..97b398d19fd2 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -15,6 +15,7 @@
>>   #include <linux/of.h>
>>   #include <linux/ioasid.h>
>>   #include <uapi/linux/iommu.h>
>> +#include <uapi/linux/iommufd.h>
>>   #define IOMMU_READ    (1 << 0)
>>   #define IOMMU_WRITE    (1 << 1)
>> @@ -223,6 +224,11 @@ struct iommu_iotlb_gather {
>>   /**
>>    * struct iommu_ops - iommu ops and capabilities
>>    * @capable: check capability
>> + * @hw_info: IOMMU hardware capabilities. The type of the returned 
>> data is
>> + *           defined in include/uapi/linux/iommufd.h. The data buffer is
>> + *           allocated in the IOMMU driver and the caller should free it
>> + *           after use. Return the data buffer if success, or ERR_PTR on
>> + *          failure.
>>    * @domain_alloc: allocate iommu domain
>>    * @probe_device: Add device to iommu driver handling
>>    * @release_device: Remove device from iommu driver handling
>> @@ -252,6 +258,7 @@ struct iommu_iotlb_gather {
>>    */
>>   struct iommu_ops {
>>       bool (*capable)(struct device *dev, enum iommu_cap);
>> +    void *(*hw_info)(struct device *dev, u32 *length);
>>       /* Domain allocation and freeing by the iommu driver */
>>       struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
>> @@ -280,6 +287,7 @@ struct iommu_ops {
>>       void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);
>>       const struct iommu_domain_ops *default_domain_ops;
>> +    enum iommu_device_data_type driver_type;
> 
> 
> How to understand the name "iommu_device_data_type"?

This is to tell userspace which type of IOMMU the returned information
comes from, Intel VT-d, ARM or others...

> Is it just refer to the driver types or it has a more generic meaning?

IOMMU driver sets this field to distinguish different IOMMU hardware.

Best regards,
baolu
diff mbox series

Patch

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index cb586d054c57..97b398d19fd2 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -15,6 +15,7 @@ 
 #include <linux/of.h>
 #include <linux/ioasid.h>
 #include <uapi/linux/iommu.h>
+#include <uapi/linux/iommufd.h>
 
 #define IOMMU_READ	(1 << 0)
 #define IOMMU_WRITE	(1 << 1)
@@ -223,6 +224,11 @@  struct iommu_iotlb_gather {
 /**
  * struct iommu_ops - iommu ops and capabilities
  * @capable: check capability
+ * @hw_info: IOMMU hardware capabilities. The type of the returned data is
+ *           defined in include/uapi/linux/iommufd.h. The data buffer is
+ *           allocated in the IOMMU driver and the caller should free it
+ *           after use. Return the data buffer if success, or ERR_PTR on
+ *	      failure.
  * @domain_alloc: allocate iommu domain
  * @probe_device: Add device to iommu driver handling
  * @release_device: Remove device from iommu driver handling
@@ -252,6 +258,7 @@  struct iommu_iotlb_gather {
  */
 struct iommu_ops {
 	bool (*capable)(struct device *dev, enum iommu_cap);
+	void *(*hw_info)(struct device *dev, u32 *length);
 
 	/* Domain allocation and freeing by the iommu driver */
 	struct iommu_domain *(*domain_alloc)(unsigned iommu_domain_type);
@@ -280,6 +287,7 @@  struct iommu_ops {
 	void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);
 
 	const struct iommu_domain_ops *default_domain_ops;
+	enum iommu_device_data_type driver_type;
 	unsigned long pgsize_bitmap;
 	struct module *owner;
 };
diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
index 98ebba80cfa1..2309edb55028 100644
--- a/include/uapi/linux/iommufd.h
+++ b/include/uapi/linux/iommufd.h
@@ -344,4 +344,10 @@  struct iommu_vfio_ioas {
 	__u16 __reserved;
 };
 #define IOMMU_VFIO_IOAS _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VFIO_IOAS)
+
+/**
+ * enum iommu_device_data_type - IOMMU hardware Data types
+ */
+enum iommu_device_data_type {
+};
 #endif