| Message ID | 20230209041642.9346-2-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommufd: Add iommu capability reporting | expand |
> 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.
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
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
> 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.
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
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 --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