From: Lu Baolu <[email protected]>
This provides an interface for upper layers to get the per-device iommu
attributes.
int iommu_device_get_info(struct device *dev,
enum iommu_devattr attr, void *data);
The first attribute (IOMMU_DEV_INFO_FORCE_SNOOP) is added. It tells if
the iommu can force DMA to snoop cache. At this stage, only PCI devices
which have this attribute set could use the iommufd, this is due to
supporting no-snoop DMA requires additional refactoring work on the
current kvm-vfio contract. The following patch will have vfio check this
attribute to decide whether a pci device can be exposed through
/dev/vfio/devices.
Signed-off-by: Lu Baolu <[email protected]>
---
drivers/iommu/iommu.c | 16 ++++++++++++++++
include/linux/iommu.h | 19 +++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 63f0af10c403..5ea3a007fd7c 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3260,3 +3260,19 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
return ret;
}
+
+/* Expose per-device iommu attributes. */
+int iommu_device_get_info(struct device *dev, enum iommu_devattr attr, void *data)
+{
+ const struct iommu_ops *ops;
+
+ if (!dev->bus || !dev->bus->iommu_ops)
+ return -EINVAL;
+
+ ops = dev->bus->iommu_ops;
+ if (unlikely(!ops->device_info))
+ return -ENODEV;
+
+ return ops->device_info(dev, attr, data);
+}
+EXPORT_SYMBOL_GPL(iommu_device_get_info);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 32d448050bf7..52a6d33c82dc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -150,6 +150,14 @@ enum iommu_dev_features {
IOMMU_DEV_FEAT_IOPF,
};
+/**
+ * enum iommu_devattr - Per device IOMMU attributes
+ * @IOMMU_DEV_INFO_FORCE_SNOOP [bool]: IOMMU can force DMA to be snooped.
+ */
+enum iommu_devattr {
+ IOMMU_DEV_INFO_FORCE_SNOOP,
+};
+
#define IOMMU_PASID_INVALID (-1U)
#ifdef CONFIG_IOMMU_API
@@ -215,6 +223,7 @@ struct iommu_iotlb_gather {
* - IOMMU_DOMAIN_IDENTITY: must use an identity domain
* - IOMMU_DOMAIN_DMA: must use a dma domain
* - 0: use the default setting
+ * @device_info: query per-device iommu attributes
* @pgsize_bitmap: bitmap of all possible supported page sizes
* @owner: Driver module providing these ops
*/
@@ -283,6 +292,8 @@ struct iommu_ops {
int (*def_domain_type)(struct device *dev);
+ int (*device_info)(struct device *dev, enum iommu_devattr attr, void *data);
+
unsigned long pgsize_bitmap;
struct module *owner;
};
@@ -604,6 +615,8 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
void iommu_sva_unbind_device(struct iommu_sva *handle);
u32 iommu_sva_get_pasid(struct iommu_sva *handle);
+int iommu_device_get_info(struct device *dev, enum iommu_devattr attr, void *data);
+
#else /* CONFIG_IOMMU_API */
struct iommu_ops {};
@@ -999,6 +1012,12 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
{
return NULL;
}
+
+static inline int iommu_device_get_info(struct device *dev,
+ enum iommu_devattr type, void *data)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_IOMMU_API */
/**
--
2.25.1
On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
> From: Lu Baolu <[email protected]>
>
> This provides an interface for upper layers to get the per-device iommu
> attributes.
>
> int iommu_device_get_info(struct device *dev,
> enum iommu_devattr attr, void *data);
Can't we use properly typed ops and functions here instead of a void
*data?
get_snoop()
get_page_size()
get_addr_width()
?
Jason
Hi Jason,
On 9/22/21 12:19 AM, Jason Gunthorpe wrote:
> On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
>> From: Lu Baolu <[email protected]>
>>
>> This provides an interface for upper layers to get the per-device iommu
>> attributes.
>>
>> int iommu_device_get_info(struct device *dev,
>> enum iommu_devattr attr, void *data);
>
> Can't we use properly typed ops and functions here instead of a void
> *data?
>
> get_snoop()
> get_page_size()
> get_addr_width()
Yeah! Above are more friendly to the upper layer callers.
>
> ?
>
> Jason
>
Best regards,
baolu
On Wed, Sep 22, 2021 at 10:31:47AM +0800, Lu Baolu wrote:
> Hi Jason,
>
> On 9/22/21 12:19 AM, Jason Gunthorpe wrote:
>> On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
>>> From: Lu Baolu <[email protected]>
>>>
>>> This provides an interface for upper layers to get the per-device iommu
>>> attributes.
>>>
>>> int iommu_device_get_info(struct device *dev,
>>> enum iommu_devattr attr, void *data);
>>
>> Can't we use properly typed ops and functions here instead of a void
>> *data?
>>
>> get_snoop()
>> get_page_size()
>> get_addr_width()
>
> Yeah! Above are more friendly to the upper layer callers.
The other option would be a struct with all the attributes. Still
type safe, but not as many methods. It'll require a little boilerplate
in the callers, though.
On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
> From: Lu Baolu <[email protected]>
>
> This provides an interface for upper layers to get the per-device iommu
> attributes.
>
> int iommu_device_get_info(struct device *dev,
> enum iommu_devattr attr, void *data);
That fact that this interface doesn't let you know how to size the
data buffer, other than by just knowing the right size for each attr
concerns me.
>
> The first attribute (IOMMU_DEV_INFO_FORCE_SNOOP) is added. It tells if
> the iommu can force DMA to snoop cache. At this stage, only PCI devices
> which have this attribute set could use the iommufd, this is due to
> supporting no-snoop DMA requires additional refactoring work on the
> current kvm-vfio contract. The following patch will have vfio check this
> attribute to decide whether a pci device can be exposed through
> /dev/vfio/devices.
>
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> drivers/iommu/iommu.c | 16 ++++++++++++++++
> include/linux/iommu.h | 19 +++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 63f0af10c403..5ea3a007fd7c 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3260,3 +3260,19 @@ static ssize_t iommu_group_store_type(struct iommu_group *group,
>
> return ret;
> }
> +
> +/* Expose per-device iommu attributes. */
> +int iommu_device_get_info(struct device *dev, enum iommu_devattr attr, void *data)
> +{
> + const struct iommu_ops *ops;
> +
> + if (!dev->bus || !dev->bus->iommu_ops)
> + return -EINVAL;
> +
> + ops = dev->bus->iommu_ops;
> + if (unlikely(!ops->device_info))
> + return -ENODEV;
> +
> + return ops->device_info(dev, attr, data);
> +}
> +EXPORT_SYMBOL_GPL(iommu_device_get_info);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 32d448050bf7..52a6d33c82dc 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -150,6 +150,14 @@ enum iommu_dev_features {
> IOMMU_DEV_FEAT_IOPF,
> };
>
> +/**
> + * enum iommu_devattr - Per device IOMMU attributes
> + * @IOMMU_DEV_INFO_FORCE_SNOOP [bool]: IOMMU can force DMA to be snooped.
> + */
> +enum iommu_devattr {
> + IOMMU_DEV_INFO_FORCE_SNOOP,
> +};
> +
> #define IOMMU_PASID_INVALID (-1U)
>
> #ifdef CONFIG_IOMMU_API
> @@ -215,6 +223,7 @@ struct iommu_iotlb_gather {
> * - IOMMU_DOMAIN_IDENTITY: must use an identity domain
> * - IOMMU_DOMAIN_DMA: must use a dma domain
> * - 0: use the default setting
> + * @device_info: query per-device iommu attributes
> * @pgsize_bitmap: bitmap of all possible supported page sizes
> * @owner: Driver module providing these ops
> */
> @@ -283,6 +292,8 @@ struct iommu_ops {
>
> int (*def_domain_type)(struct device *dev);
>
> + int (*device_info)(struct device *dev, enum iommu_devattr attr, void *data);
> +
> unsigned long pgsize_bitmap;
> struct module *owner;
> };
> @@ -604,6 +615,8 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev,
> void iommu_sva_unbind_device(struct iommu_sva *handle);
> u32 iommu_sva_get_pasid(struct iommu_sva *handle);
>
> +int iommu_device_get_info(struct device *dev, enum iommu_devattr attr, void *data);
> +
> #else /* CONFIG_IOMMU_API */
>
> struct iommu_ops {};
> @@ -999,6 +1012,12 @@ static inline struct iommu_fwspec *dev_iommu_fwspec_get(struct device *dev)
> {
> return NULL;
> }
> +
> +static inline int iommu_device_get_info(struct device *dev,
> + enum iommu_devattr type, void *data)
> +{
> + return -ENODEV;
> +}
> #endif /* CONFIG_IOMMU_API */
>
> /**
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 2021/9/29 17:25, Lu Baolu wrote:
> Hi David,
>
> On 2021/9/29 10:52, David Gibson wrote:
>> On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
>>> From: Lu Baolu<[email protected]>
>>>
>>> This provides an interface for upper layers to get the per-device iommu
>>> attributes.
>>>
>>> ???? int iommu_device_get_info(struct device *dev,
>>> ?????????????????????????????? enum iommu_devattr attr, void *data);
>> That fact that this interface doesn't let you know how to size the
>> data buffer, other than by just knowing the right size for each attr
>> concerns me.
>>
>
> We plan to address this by following the comments here.
>
> https://lore.kernel.org/linux-iommu/[email protected]/
And Christoph gave another option as well.
https://lore.kernel.org/linux-iommu/[email protected]/
Best regards,
baolu
Hi David,
On 2021/9/29 10:52, David Gibson wrote:
> On Sun, Sep 19, 2021 at 02:38:32PM +0800, Liu Yi L wrote:
>> From: Lu Baolu<[email protected]>
>>
>> This provides an interface for upper layers to get the per-device iommu
>> attributes.
>>
>> int iommu_device_get_info(struct device *dev,
>> enum iommu_devattr attr, void *data);
> That fact that this interface doesn't let you know how to size the
> data buffer, other than by just knowing the right size for each attr
> concerns me.
>
We plan to address this by following the comments here.
https://lore.kernel.org/linux-iommu/[email protected]/
Best regards,
baolu