* [RFC v2 01/20] iommu: Introduce bind_pasid_table API
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-20 17:21 ` Jacob Pan
2018-09-18 14:24 ` [RFC v2 02/20] iommu: Introduce cache_invalidate API Eric Auger
` (18 subsequent siblings)
19 siblings, 1 reply; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
In virtualization use case, when a guest is assigned
a PCI host device, protected by a virtual IOMMU on a guest,
the physical IOMMU must be programmed to be consistent with
the guest mappings. If the physical IOMMU supports two
translation stages it makes sense to program guest mappings
onto the first stage/level (ARM/VTD terminology) while to host
owns the stage/level 2.
In that case, it is mandated to trap on guest configuration
settings and pass those to the physical iommu driver.
This patch adds a new API to the iommu subsystem that allows
to bind and unbind the guest configuration data to the host.
A generic iommu_pasid_table_config struct is introduced in
a new iommu.h uapi header. This is going to be used by the VFIO
user API. We foresee at least two specializations of this struct,
for PASID table passing and ARM SMMUv3.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
In practice, I think it would be simpler to have a single
set_pasid_table function instead of bind/unbind. The "bypass" field
tells the stage 1 is bypassed (equivalent to the unbind actually).
On userspace we have notifications that the device context has changed.
Calling either bind or unbind requires to have an understand of what
was the previous state and call different notifiers. So to me the
bind/unbind complexifies the user integration while not bring much
benefits.
This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/
v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3
---
drivers/iommu/iommu.c | 19 ++++++++++++++
include/linux/iommu.h | 21 +++++++++++++++
include/uapi/linux/iommu.h | 52 ++++++++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+)
create mode 100644 include/uapi/linux/iommu.h
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8c15c5980299..db2c7c9502ae 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1362,6 +1362,25 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
}
EXPORT_SYMBOL_GPL(iommu_attach_device);
+int iommu_bind_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ if (unlikely(!domain->ops->bind_pasid_table))
+ return -ENODEV;
+
+ return domain->ops->bind_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
+
+void iommu_unbind_pasid_table(struct iommu_domain *domain)
+{
+ if (unlikely(!domain->ops->unbind_pasid_table))
+ return;
+
+ domain->ops->unbind_pasid_table(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
+
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 87994c265bf5..e56cad4863f7 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -25,6 +25,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/of.h>
+#include <uapi/linux/iommu.h>
#define IOMMU_READ (1 << 0)
#define IOMMU_WRITE (1 << 1)
@@ -185,6 +186,8 @@ struct iommu_resv_region {
* @domain_get_windows: Return the number of windows for a domain
* @of_xlate: add OF master IDs to iommu grouping
* @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @bind_pasid_table: bind pasid table
+ * @unbind_pasid_table: unbind pasid table and restore defaults
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -231,6 +234,10 @@ struct iommu_ops {
int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
+ int (*bind_pasid_table)(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+ void (*unbind_pasid_table)(struct iommu_domain *domain);
+
unsigned long pgsize_bitmap;
};
@@ -292,6 +299,9 @@ extern int iommu_attach_device(struct iommu_domain *domain,
struct device *dev);
extern void iommu_detach_device(struct iommu_domain *domain,
struct device *dev);
+extern int iommu_bind_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg);
+extern void iommu_unbind_pasid_table(struct iommu_domain *domain);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
@@ -684,6 +694,17 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
return NULL;
}
+static inline
+int iommu_bind_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ return -ENODEV;
+}
+static inline
+void iommu_unbind_pasid_table(struct iommu_domain *domain)
+{
+}
+
#endif /* CONFIG_IOMMU_API */
#ifdef CONFIG_IOMMU_DEBUGFS
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
new file mode 100644
index 000000000000..babec91ae7e1
--- /dev/null
+++ b/include/uapi/linux/iommu.h
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * IOMMU user API definitions
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information
+ * @s1contextptr: Context Descriptor Table GPA
+ * @abort: shall the STE lead to abort
+ * @s1fmt: STE s1fmt field as set by the guest
+ * @s1cdmax: STE s1cdmax as set by the guest
+ * @s1dss: STE s1dss as set by the guest
+ * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)
+ */
+struct iommu_pasid_smmuv3 {
+ __u64 s1contextptr;
+ __u8 bypass;
+ __u8 abort;
+ __u8 s1fmt;
+ __u8 s1cdmax;
+ __u8 s1dss;
+};
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */
+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1
+ __u32 version;
+#define IOMMU_PASID_FORMAT_SMMUV3 (1 << 0)
+ __u32 format;
+ union {
+ struct iommu_pasid_smmuv3 smmuv3;
+ };
+};
+
+#endif /* _UAPI_IOMMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC v2 01/20] iommu: Introduce bind_pasid_table API
2018-09-18 14:24 ` [RFC v2 01/20] iommu: Introduce bind_pasid_table API Eric Auger
@ 2018-09-20 17:21 ` Jacob Pan
2018-09-21 9:45 ` Auger Eric
0 siblings, 1 reply; 33+ messages in thread
From: Jacob Pan @ 2018-09-20 17:21 UTC (permalink / raw)
To: Eric Auger
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell, jacob.jun.pan
On Tue, 18 Sep 2018 16:24:38 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>
> In virtualization use case, when a guest is assigned
> a PCI host device, protected by a virtual IOMMU on a guest,
> the physical IOMMU must be programmed to be consistent with
> the guest mappings. If the physical IOMMU supports two
> translation stages it makes sense to program guest mappings
> onto the first stage/level (ARM/VTD terminology) while to host
> owns the stage/level 2.
>
> In that case, it is mandated to trap on guest configuration
> settings and pass those to the physical iommu driver.
>
> This patch adds a new API to the iommu subsystem that allows
> to bind and unbind the guest configuration data to the host.
>
> A generic iommu_pasid_table_config struct is introduced in
> a new iommu.h uapi header. This is going to be used by the VFIO
> user API. We foresee at least two specializations of this struct,
> for PASID table passing and ARM SMMUv3.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> In practice, I think it would be simpler to have a single
> set_pasid_table function instead of bind/unbind. The "bypass" field
> tells the stage 1 is bypassed (equivalent to the unbind actually).
> On userspace we have notifications that the device context has
> changed. Calling either bind or unbind requires to have an understand
> of what was the previous state and call different notifiers. So to me
> the bind/unbind complexifies the user integration while not bring much
> benefits.
>
I don't have strong preference and I think having a single function
makes sense. In VT-d2, the bind/unbind operation is a result of PASID
cache invalidation from the guest. So there is no symmetrical
bind/unbin user calls.
> This patch generalizes the API introduced by Jacob & co-authors in
> https://lwn.net/Articles/754331/
>
> v1 -> v2:
> - restore the original pasid table name
> - remove the struct device * parameter in the API
> - reworked iommu_pasid_smmuv3
> ---
> drivers/iommu/iommu.c | 19 ++++++++++++++
> include/linux/iommu.h | 21 +++++++++++++++
> include/uapi/linux/iommu.h | 52
> ++++++++++++++++++++++++++++++++++++++ 3 files changed, 92
> insertions(+) create mode 100644 include/uapi/linux/iommu.h
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 8c15c5980299..db2c7c9502ae 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1362,6 +1362,25 @@ int iommu_attach_device(struct iommu_domain
> *domain, struct device *dev) }
> EXPORT_SYMBOL_GPL(iommu_attach_device);
>
> +int iommu_bind_pasid_table(struct iommu_domain *domain,
> + struct iommu_pasid_table_config *cfg)
> +{
> + if (unlikely(!domain->ops->bind_pasid_table))
> + return -ENODEV;
> +
> + return domain->ops->bind_pasid_table(domain, cfg);
> +}
> +EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
> +
> +void iommu_unbind_pasid_table(struct iommu_domain *domain)
> +{
> + if (unlikely(!domain->ops->unbind_pasid_table))
> + return;
> +
> + domain->ops->unbind_pasid_table(domain);
> +}
> +EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
> +
> static void __iommu_detach_device(struct iommu_domain *domain,
> struct device *dev)
> {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 87994c265bf5..e56cad4863f7 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -25,6 +25,7 @@
> #include <linux/errno.h>
> #include <linux/err.h>
> #include <linux/of.h>
> +#include <uapi/linux/iommu.h>
>
> #define IOMMU_READ (1 << 0)
> #define IOMMU_WRITE (1 << 1)
> @@ -185,6 +186,8 @@ struct iommu_resv_region {
> * @domain_get_windows: Return the number of windows for a domain
> * @of_xlate: add OF master IDs to iommu grouping
> * @pgsize_bitmap: bitmap of all possible supported page sizes
> + * @bind_pasid_table: bind pasid table
> + * @unbind_pasid_table: unbind pasid table and restore defaults
> */
> struct iommu_ops {
> bool (*capable)(enum iommu_cap);
> @@ -231,6 +234,10 @@ struct iommu_ops {
> int (*of_xlate)(struct device *dev, struct of_phandle_args
> *args); bool (*is_attach_deferred)(struct iommu_domain *domain,
> struct device *dev);
> + int (*bind_pasid_table)(struct iommu_domain *domain,
> + struct iommu_pasid_table_config
> *cfg);
> + void (*unbind_pasid_table)(struct iommu_domain *domain);
> +
> unsigned long pgsize_bitmap;
> };
>
> @@ -292,6 +299,9 @@ extern int iommu_attach_device(struct
> iommu_domain *domain, struct device *dev);
> extern void iommu_detach_device(struct iommu_domain *domain,
> struct device *dev);
> +extern int iommu_bind_pasid_table(struct iommu_domain *domain,
> + struct iommu_pasid_table_config
> *cfg); +extern void iommu_unbind_pasid_table(struct iommu_domain
> *domain); extern struct iommu_domain *iommu_get_domain_for_dev(struct
> device *dev); extern int iommu_map(struct iommu_domain *domain,
> unsigned long iova, phys_addr_t paddr, size_t size, int prot);
> @@ -684,6 +694,17 @@ const struct iommu_ops
> *iommu_ops_from_fwnode(struct fwnode_handle *fwnode) return NULL;
> }
>
> +static inline
> +int iommu_bind_pasid_table(struct iommu_domain *domain,
> + struct iommu_pasid_table_config *cfg)
> +{
> + return -ENODEV;
> +}
> +static inline
> +void iommu_unbind_pasid_table(struct iommu_domain *domain)
> +{
> +}
> +
> #endif /* CONFIG_IOMMU_API */
>
> #ifdef CONFIG_IOMMU_DEBUGFS
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> new file mode 100644
> index 000000000000..babec91ae7e1
> --- /dev/null
> +++ b/include/uapi/linux/iommu.h
> @@ -0,0 +1,52 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * IOMMU user API definitions
> + *
> + *
> + * This program is free software; you can redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _UAPI_IOMMU_H
> +#define _UAPI_IOMMU_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * SMMUv3 Stream Table Entry stage 1 related information
> + * @s1contextptr: Context Descriptor Table GPA
> + * @abort: shall the STE lead to abort
> + * @s1fmt: STE s1fmt field as set by the guest
> + * @s1cdmax: STE s1cdmax as set by the guest
> + * @s1dss: STE s1dss as set by the guest
> + * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)
> + */
> +struct iommu_pasid_smmuv3 {
> + __u64 s1contextptr;
> + __u8 bypass;
> + __u8 abort;
> + __u8 s1fmt;
> + __u8 s1cdmax;
> + __u8 s1dss;
> +};
> +
> +/**
> + * PASID table data used to bind guest PASID table to the host IOMMU
> + * Note PASID table corresponds to the Context Table on ARM SMMUv3.
> + *
> + * @version: API version to prepare for future extensions
> + * @format: format of the PASID table
> + *
> + */
> +struct iommu_pasid_table_config {
don;t you need some vendor neutral data such as
* @base_ptr: PASID table pointer
* @pasid_bits: number of bits supported in the guest PASID table, must be less
* or equal than the host supported PASID size.
> +#define PASID_TABLE_CFG_VERSION_1 1
> + __u32 version;
> +#define IOMMU_PASID_FORMAT_SMMUV3 (1 << 0)
> + __u32 format;
> + union {
> + struct iommu_pasid_smmuv3 smmuv3;
> + };
> +};
> +
> +#endif /* _UAPI_IOMMU_H */
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 01/20] iommu: Introduce bind_pasid_table API
2018-09-20 17:21 ` Jacob Pan
@ 2018-09-21 9:45 ` Auger Eric
0 siblings, 0 replies; 33+ messages in thread
From: Auger Eric @ 2018-09-21 9:45 UTC (permalink / raw)
To: Jacob Pan
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell
Hi Jacob,
On 9/20/18 7:21 PM, Jacob Pan wrote:
> On Tue, 18 Sep 2018 16:24:38 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>
>> In virtualization use case, when a guest is assigned
>> a PCI host device, protected by a virtual IOMMU on a guest,
>> the physical IOMMU must be programmed to be consistent with
>> the guest mappings. If the physical IOMMU supports two
>> translation stages it makes sense to program guest mappings
>> onto the first stage/level (ARM/VTD terminology) while to host
>> owns the stage/level 2.
>>
>> In that case, it is mandated to trap on guest configuration
>> settings and pass those to the physical iommu driver.
>>
>> This patch adds a new API to the iommu subsystem that allows
>> to bind and unbind the guest configuration data to the host.
>>
>> A generic iommu_pasid_table_config struct is introduced in
>> a new iommu.h uapi header. This is going to be used by the VFIO
>> user API. We foresee at least two specializations of this struct,
>> for PASID table passing and ARM SMMUv3.
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> In practice, I think it would be simpler to have a single
>> set_pasid_table function instead of bind/unbind. The "bypass" field
>> tells the stage 1 is bypassed (equivalent to the unbind actually).
>> On userspace we have notifications that the device context has
>> changed. Calling either bind or unbind requires to have an understand
>> of what was the previous state and call different notifiers. So to me
>> the bind/unbind complexifies the user integration while not bring much
>> benefits.
>>
> I don't have strong preference and I think having a single function
> makes sense. In VT-d2, the bind/unbind operation is a result of PASID
> cache invalidation from the guest. So there is no symmetrical
> bind/unbin user calls.
OK thank you for the feedback.
>
>> This patch generalizes the API introduced by Jacob & co-authors in
>> https://lwn.net/Articles/754331/
>>
>> v1 -> v2:
>> - restore the original pasid table name
>> - remove the struct device * parameter in the API
>> - reworked iommu_pasid_smmuv3
>> ---
>> drivers/iommu/iommu.c | 19 ++++++++++++++
>> include/linux/iommu.h | 21 +++++++++++++++
>> include/uapi/linux/iommu.h | 52
>> ++++++++++++++++++++++++++++++++++++++ 3 files changed, 92
>> insertions(+) create mode 100644 include/uapi/linux/iommu.h
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 8c15c5980299..db2c7c9502ae 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1362,6 +1362,25 @@ int iommu_attach_device(struct iommu_domain
>> *domain, struct device *dev) }
>> EXPORT_SYMBOL_GPL(iommu_attach_device);
>>
>> +int iommu_bind_pasid_table(struct iommu_domain *domain,
>> + struct iommu_pasid_table_config *cfg)
>> +{
>> + if (unlikely(!domain->ops->bind_pasid_table))
>> + return -ENODEV;
>> +
>> + return domain->ops->bind_pasid_table(domain, cfg);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
>> +
>> +void iommu_unbind_pasid_table(struct iommu_domain *domain)
>> +{
>> + if (unlikely(!domain->ops->unbind_pasid_table))
>> + return;
>> +
>> + domain->ops->unbind_pasid_table(domain);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
>> +
>> static void __iommu_detach_device(struct iommu_domain *domain,
>> struct device *dev)
>> {
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 87994c265bf5..e56cad4863f7 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -25,6 +25,7 @@
>> #include <linux/errno.h>
>> #include <linux/err.h>
>> #include <linux/of.h>
>> +#include <uapi/linux/iommu.h>
>>
>> #define IOMMU_READ (1 << 0)
>> #define IOMMU_WRITE (1 << 1)
>> @@ -185,6 +186,8 @@ struct iommu_resv_region {
>> * @domain_get_windows: Return the number of windows for a domain
>> * @of_xlate: add OF master IDs to iommu grouping
>> * @pgsize_bitmap: bitmap of all possible supported page sizes
>> + * @bind_pasid_table: bind pasid table
>> + * @unbind_pasid_table: unbind pasid table and restore defaults
>> */
>> struct iommu_ops {
>> bool (*capable)(enum iommu_cap);
>> @@ -231,6 +234,10 @@ struct iommu_ops {
>> int (*of_xlate)(struct device *dev, struct of_phandle_args
>> *args); bool (*is_attach_deferred)(struct iommu_domain *domain,
>> struct device *dev);
>> + int (*bind_pasid_table)(struct iommu_domain *domain,
>> + struct iommu_pasid_table_config
>> *cfg);
>> + void (*unbind_pasid_table)(struct iommu_domain *domain);
>> +
>> unsigned long pgsize_bitmap;
>> };
>>
>> @@ -292,6 +299,9 @@ extern int iommu_attach_device(struct
>> iommu_domain *domain, struct device *dev);
>> extern void iommu_detach_device(struct iommu_domain *domain,
>> struct device *dev);
>> +extern int iommu_bind_pasid_table(struct iommu_domain *domain,
>> + struct iommu_pasid_table_config
>> *cfg); +extern void iommu_unbind_pasid_table(struct iommu_domain
>> *domain); extern struct iommu_domain *iommu_get_domain_for_dev(struct
>> device *dev); extern int iommu_map(struct iommu_domain *domain,
>> unsigned long iova, phys_addr_t paddr, size_t size, int prot);
>> @@ -684,6 +694,17 @@ const struct iommu_ops
>> *iommu_ops_from_fwnode(struct fwnode_handle *fwnode) return NULL;
>> }
>>
>> +static inline
>> +int iommu_bind_pasid_table(struct iommu_domain *domain,
>> + struct iommu_pasid_table_config *cfg)
>> +{
>> + return -ENODEV;
>> +}
>> +static inline
>> +void iommu_unbind_pasid_table(struct iommu_domain *domain)
>> +{
>> +}
>> +
>> #endif /* CONFIG_IOMMU_API */
>>
>> #ifdef CONFIG_IOMMU_DEBUGFS
>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>> new file mode 100644
>> index 000000000000..babec91ae7e1
>> --- /dev/null
>> +++ b/include/uapi/linux/iommu.h
>> @@ -0,0 +1,52 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * IOMMU user API definitions
>> + *
>> + *
>> + * This program is free software; you can redistribute it and/or
>> modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _UAPI_IOMMU_H
>> +#define _UAPI_IOMMU_H
>> +
>> +#include <linux/types.h>
>> +
>> +/**
>> + * SMMUv3 Stream Table Entry stage 1 related information
>> + * @s1contextptr: Context Descriptor Table GPA
>> + * @abort: shall the STE lead to abort
>> + * @s1fmt: STE s1fmt field as set by the guest
>> + * @s1cdmax: STE s1cdmax as set by the guest
>> + * @s1dss: STE s1dss as set by the guest
>> + * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)
>> + */
>> +struct iommu_pasid_smmuv3 {
>> + __u64 s1contextptr;
>> + __u8 bypass;
>> + __u8 abort;
>> + __u8 s1fmt;
>> + __u8 s1cdmax;
>> + __u8 s1dss;
>> +};
>> +
>> +/**
>> + * PASID table data used to bind guest PASID table to the host IOMMU
>> + * Note PASID table corresponds to the Context Table on ARM SMMUv3.
>> + *
>> + * @version: API version to prepare for future extensions
>> + * @format: format of the PASID table
>> + *
>> + */
>> +struct iommu_pasid_table_config {
> don;t you need some vendor neutral data such as
> * @base_ptr: PASID table pointer
> * @pasid_bits: number of bits supported in the guest PASID table, must be less
> * or equal than the host supported PASID size.
At the moment I put those info in the vendor specific struct, ie.
iommu_pasid_smmuv3
s1contextptr = base_ptr whereas s1cdmax corresponds to the number of
entries pointed by S1contextptr.
I am open to moving those fields back to the generic part if their
semantic is shared by all the archs.
Thanks
Eric
>
>
>> +#define PASID_TABLE_CFG_VERSION_1 1
>> + __u32 version;
>> +#define IOMMU_PASID_FORMAT_SMMUV3 (1 << 0)
>> + __u32 format;
>> + union {
>> + struct iommu_pasid_smmuv3 smmuv3;
>> + };
>> +};
>> +
>> +#endif /* _UAPI_IOMMU_H */
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC v2 02/20] iommu: Introduce cache_invalidate API
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
2018-09-18 14:24 ` [RFC v2 01/20] iommu: Introduce bind_pasid_table API Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 03/20] iommu: Introduce bind_guest_msi Eric Auger
` (17 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
In any virtualization use case, when the first translation stage
is "owned" by the guest OS, the host IOMMU driver has no knowledge
of caching structure updates unless the guest invalidation activities
are trapped by the virtualizer and passed down to the host.
Since the invalidation data are obtained from user space and will be
written into physical IOMMU, we must allow security check at various
layers. Therefore, generic invalidation data format are proposed here,
model specific IOMMU drivers need to convert them into their own format.
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows
to invalidate context caches on top of IOTLBs
v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in
header. Commit message reworded.
---
drivers/iommu/iommu.c | 14 ++++++
include/linux/iommu.h | 14 ++++++
include/uapi/linux/iommu.h | 95 ++++++++++++++++++++++++++++++++++++++
3 files changed, 123 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index db2c7c9502ae..1442a6c640af 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1381,6 +1381,20 @@ void iommu_unbind_pasid_table(struct iommu_domain *domain)
}
EXPORT_SYMBOL_GPL(iommu_unbind_pasid_table);
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+ int ret = 0;
+
+ if (unlikely(!domain->ops->cache_invalidate))
+ return -ENODEV;
+
+ ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
static void __iommu_detach_device(struct iommu_domain *domain,
struct device *dev)
{
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index e56cad4863f7..3e5f6eb1f04a 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -188,6 +188,7 @@ struct iommu_resv_region {
* @pgsize_bitmap: bitmap of all possible supported page sizes
* @bind_pasid_table: bind pasid table
* @unbind_pasid_table: unbind pasid table and restore defaults
+ * @cache_invalidate: invalidate translation caches
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -238,6 +239,9 @@ struct iommu_ops {
struct iommu_pasid_table_config *cfg);
void (*unbind_pasid_table)(struct iommu_domain *domain);
+ int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info);
+
unsigned long pgsize_bitmap;
};
@@ -302,6 +306,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
extern int iommu_bind_pasid_table(struct iommu_domain *domain,
struct iommu_pasid_table_config *cfg);
extern void iommu_unbind_pasid_table(struct iommu_domain *domain);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info);
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
@@ -704,6 +711,13 @@ static inline
void iommu_unbind_pasid_table(struct iommu_domain *domain)
{
}
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+ struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+ return -ENODEV;
+}
#endif /* CONFIG_IOMMU_API */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index babec91ae7e1..4283e0334baf 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -49,4 +49,99 @@ struct iommu_pasid_table_config {
};
};
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID: TLB entries or PASID caches of all
+ * PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL: TLB entries or PASID cache associated
+ * with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID: TLB entries of selected page range
+ * within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ * type | DTLB | TLB | PASID |
+ * granule | | | cache |
+ * -----------------+-----------+-----------+-----------+
+ * DN_ALL_PASID | Y | Y | Y |
+ * PASID_SEL | Y | Y | Y |
+ * PAGE_PASID | Y | Y | N/A |
+ *
+ */
+enum iommu_inv_granularity {
+ IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+ IOMMU_INV_GRANU_PASID_SEL,
+ IOMMU_INV_GRANU_PAGE_PASID,
+ IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB: device IOTLB
+ * @IOMMU_INV_TYPE_TLB: IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID: PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+ IOMMU_INV_TYPE_DTLB,
+ IOMMU_INV_TYPE_TLB,
+ IOMMU_INV_TYPE_PASID,
+ IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version: info format version, expecting future extesions
+ * @type: type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+ __u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+ enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.
+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity: requested invalidation granularity, type dependent
+ * @size: 2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.
+ * @nr_pages: number of pages to invalidate
+ * @pasid: processor address space ID value per PCI spec.
+ * @arch_id: architecture dependent id characterizing a context
+ * and tagging the caches, ie. domain Identfier on VTD,
+ * asid on ARM SMMU
+ * @addr: page address to be invalidated
+ * @flags IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ * IOMMU_INVALIDATE_GLOBAL_PAGE: global pages
+ *
+ */
+struct iommu_cache_invalidate_info {
+ struct iommu_cache_invalidate_hdr hdr;
+ enum iommu_inv_granularity granularity;
+ __u32 flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF (1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE (1 << 1)
+ __u8 size;
+ __u64 nr_pages;
+ __u32 pasid;
+ __u64 arch_id;
+ __u64 addr;
+};
#endif /* _UAPI_IOMMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 03/20] iommu: Introduce bind_guest_msi
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
2018-09-18 14:24 ` [RFC v2 01/20] iommu: Introduce bind_pasid_table API Eric Auger
2018-09-18 14:24 ` [RFC v2 02/20] iommu: Introduce cache_invalidate API Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 04/20] vfio: VFIO_IOMMU_BIND_PASID_TABLE Eric Auger
` (16 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
On ARM, MSI are translated by the SMMU. An IOVA is allocated
for each MSI doorbell. If both the host and the guest are exposed
with SMMUs, we end up with 2 different IOVAs allocated by each.
guest allocates an IOVA (gIOVA) to map onto the guest MSI
doorbell (gDB). The Host allocates another IOVA (hIOVA) to map
onto the physical doorbell (hDB).
So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB
hIOVA -> gDB
Currently the PCI device is programmed by the host with hIOVA
as MSI doorbell. So this does not work.
This patch introduces an API to pass gIOVA/gDB to the host so
that gIOVA can be reused by the host instead of re-allocating
a new IOVA. So the goal is to create the following nested mapping:
S1 S2
gIOVA -> gDB -> hDB
and program the PCI device with gIOVA MSI doorbell.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/iommu.c | 10 ++++++++++
include/linux/iommu.h | 13 +++++++++++++
include/uapi/linux/iommu.h | 7 +++++++
3 files changed, 30 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1442a6c640af..b2b8f375b169 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1409,6 +1409,16 @@ static void __iommu_detach_device(struct iommu_domain *domain,
trace_detach_device_from_domain(dev);
}
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding)
+{
+ if (unlikely(!domain->ops->bind_guest_msi))
+ return -ENODEV;
+
+ return domain->ops->bind_guest_msi(domain, binding);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
{
struct iommu_group *group;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 3e5f6eb1f04a..9bd3e63d562b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -242,6 +242,9 @@ struct iommu_ops {
int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
struct iommu_cache_invalidate_info *inv_info);
+ int (*bind_guest_msi)(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding);
+
unsigned long pgsize_bitmap;
};
@@ -309,6 +312,9 @@ extern void iommu_unbind_pasid_table(struct iommu_domain *domain);
extern int iommu_cache_invalidate(struct iommu_domain *domain,
struct device *dev,
struct iommu_cache_invalidate_info *inv_info);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding);
+
extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot);
@@ -719,6 +725,13 @@ iommu_cache_invalidate(struct iommu_domain *domain,
return -ENODEV;
}
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding)
+{
+ return -ENODEV;
+}
+
#endif /* CONFIG_IOMMU_API */
#ifdef CONFIG_IOMMU_DEBUGFS
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 4283e0334baf..21adb2a964e5 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -144,4 +144,11 @@ struct iommu_cache_invalidate_info {
__u64 arch_id;
__u64 addr;
};
+
+struct iommu_guest_msi_binding {
+ __u64 iova;
+ __u64 gpa;
+ __u32 granule;
+};
#endif /* _UAPI_IOMMU_H */
+
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 04/20] vfio: VFIO_IOMMU_BIND_PASID_TABLE
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (2 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 03/20] iommu: Introduce bind_guest_msi Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 05/20] vfio: VFIO_IOMMU_CACHE_INVALIDATE Eric Auger
` (15 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
This patch adds VFIO_IOMMU_BIND_PASID_TABLE ioctl which aims at
passing the virtual iommu guest configuration to the VFIO driver
downto to the iommu subsystem.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- s/BIND_GUEST_STAGE/BIND_PASID_TABLE
- remove the struct device arg
---
drivers/vfio/vfio_iommu_type1.c | 31 +++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 8 ++++++++
2 files changed, 39 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index d9fd3188615d..044d3a046125 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1673,6 +1673,24 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
return ret;
}
+static int
+vfio_bind_pasid_table(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_bind_pasid_table *ustruct)
+{
+ struct vfio_domain *d;
+ int ret = 0;
+
+ mutex_lock(&iommu->lock);
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ ret = iommu_bind_pasid_table(d->domain, &ustruct->config);
+ if (ret)
+ break;
+ }
+ mutex_unlock(&iommu->lock);
+ return ret;
+}
+
static long vfio_iommu_type1_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
@@ -1743,6 +1761,19 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return copy_to_user((void __user *)arg, &unmap, minsz) ?
-EFAULT : 0;
+ } else if (cmd == VFIO_IOMMU_BIND_PASID_TABLE) {
+ struct vfio_iommu_type1_bind_pasid_table ustruct;
+
+ minsz = offsetofend(struct vfio_iommu_type1_bind_pasid_table,
+ config);
+
+ if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (ustruct.argsz < minsz || ustruct.flags)
+ return -EINVAL;
+
+ return vfio_bind_pasid_table(iommu, &ustruct);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 1aa7b82e8169..f81ce0a75c01 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -14,6 +14,7 @@
#include <linux/types.h>
#include <linux/ioctl.h>
+#include <linux/iommu.h>
#define VFIO_API_VERSION 0
@@ -665,6 +666,13 @@ struct vfio_iommu_type1_dma_unmap {
#define VFIO_IOMMU_ENABLE _IO(VFIO_TYPE, VFIO_BASE + 15)
#define VFIO_IOMMU_DISABLE _IO(VFIO_TYPE, VFIO_BASE + 16)
+struct vfio_iommu_type1_bind_pasid_table {
+ __u32 argsz;
+ __u32 flags;
+ struct iommu_pasid_table_config config;
+};
+#define VFIO_IOMMU_BIND_PASID_TABLE _IO(VFIO_TYPE, VFIO_BASE + 22)
+
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 05/20] vfio: VFIO_IOMMU_CACHE_INVALIDATE
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (3 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 04/20] vfio: VFIO_IOMMU_BIND_PASID_TABLE Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 06/20] vfio: VFIO_IOMMU_BIND_MSI Eric Auger
` (14 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: "Liu, Yi L" <yi.l.liu@linux.intel.com>
When the guest "owns" the stage 1 translation structures, the host
IOMMU driver has no knowledge of caching structure updates unless
the guest invalidation requests are trapped and passed down to the
host.
This patch adds the VFIO_IOMMU_CACHE_INVALIDATE ioctl with aims
at propagating guest stage1 IOMMU cache invalidations to the host.
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- s/TLB/CACHE
- remove vfio_iommu_task usage
- commit message rewording
---
drivers/vfio/vfio_iommu_type1.c | 44 +++++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 7 ++++++
2 files changed, 51 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 044d3a046125..14529233f774 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1673,6 +1673,37 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
return ret;
}
+static int vfio_cache_inv_fn(struct device *dev, void *data)
+{
+ struct vfio_iommu_type1_cache_invalidate *ustruct =
+ (struct vfio_iommu_type1_cache_invalidate *)data;
+ struct iommu_domain *d = iommu_get_domain_for_dev(dev);
+
+ return iommu_cache_invalidate(d, dev, &ustruct->info);
+}
+
+static int
+vfio_cache_invalidate(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_cache_invalidate *ustruct)
+{
+ struct vfio_domain *d;
+ struct vfio_group *g;
+ int ret = 0;
+
+ mutex_lock(&iommu->lock);
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ list_for_each_entry(g, &d->group_list, next) {
+ ret = iommu_group_for_each_dev(g->iommu_group, ustruct,
+ vfio_cache_inv_fn);
+ if (ret)
+ break;
+ }
+ }
+ mutex_unlock(&iommu->lock);
+ return ret;
+}
+
static int
vfio_bind_pasid_table(struct vfio_iommu *iommu,
struct vfio_iommu_type1_bind_pasid_table *ustruct)
@@ -1774,6 +1805,19 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return -EINVAL;
return vfio_bind_pasid_table(iommu, &ustruct);
+ } else if (cmd == VFIO_IOMMU_CACHE_INVALIDATE) {
+ struct vfio_iommu_type1_cache_invalidate ustruct;
+
+ minsz = offsetofend(struct vfio_iommu_type1_cache_invalidate,
+ info);
+
+ if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (ustruct.argsz < minsz || ustruct.flags)
+ return -EINVAL;
+
+ return vfio_cache_invalidate(iommu, &ustruct);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index f81ce0a75c01..2a38d0bca0ca 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -673,6 +673,13 @@ struct vfio_iommu_type1_bind_pasid_table {
};
#define VFIO_IOMMU_BIND_PASID_TABLE _IO(VFIO_TYPE, VFIO_BASE + 22)
+struct vfio_iommu_type1_cache_invalidate {
+ __u32 argsz;
+ __u32 flags;
+ struct iommu_cache_invalidate_info info;
+};
+#define VFIO_IOMMU_CACHE_INVALIDATE _IO(VFIO_TYPE, VFIO_BASE + 23)
+
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 06/20] vfio: VFIO_IOMMU_BIND_MSI
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (4 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 05/20] vfio: VFIO_IOMMU_CACHE_INVALIDATE Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 07/20] iommu/arm-smmu-v3: Link domains and devices Eric Auger
` (13 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
This patch adds the VFIO_IOMMU_BIND_MSI ioctl which aims at
passing the guest MSI binding to the host.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- s/vfio_iommu_type1_guest_msi_binding/vfio_iommu_type1_bind_guest_msi
---
drivers/vfio/vfio_iommu_type1.c | 31 +++++++++++++++++++++++++++++++
include/uapi/linux/vfio.h | 7 +++++++
2 files changed, 38 insertions(+)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 14529233f774..2ffd46c26dc3 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1704,6 +1704,24 @@ vfio_cache_invalidate(struct vfio_iommu *iommu,
return ret;
}
+static int
+vfio_iommu_bind_guest_msi(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_bind_guest_msi *ustruct)
+{
+ struct vfio_domain *d;
+ int ret;
+
+ mutex_lock(&iommu->lock);
+
+ list_for_each_entry(d, &iommu->domain_list, next) {
+ ret = iommu_bind_guest_msi(d->domain, &ustruct->binding);
+ if (ret)
+ break;
+ }
+ mutex_unlock(&iommu->lock);
+ return ret;
+}
+
static int
vfio_bind_pasid_table(struct vfio_iommu *iommu,
struct vfio_iommu_type1_bind_pasid_table *ustruct)
@@ -1818,6 +1836,19 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return -EINVAL;
return vfio_cache_invalidate(iommu, &ustruct);
+ } else if (cmd == VFIO_IOMMU_BIND_MSI) {
+ struct vfio_iommu_type1_bind_guest_msi ustruct;
+
+ minsz = offsetofend(struct vfio_iommu_type1_bind_guest_msi,
+ binding);
+
+ if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (ustruct.argsz < minsz || ustruct.flags)
+ return -EINVAL;
+
+ return vfio_iommu_bind_guest_msi(iommu, &ustruct);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 2a38d0bca0ca..6fb5e944e73c 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -680,6 +680,13 @@ struct vfio_iommu_type1_cache_invalidate {
};
#define VFIO_IOMMU_CACHE_INVALIDATE _IO(VFIO_TYPE, VFIO_BASE + 23)
+struct vfio_iommu_type1_bind_guest_msi {
+ __u32 argsz;
+ __u32 flags;
+ struct iommu_guest_msi_binding binding;
+};
+#define VFIO_IOMMU_BIND_MSI _IO(VFIO_TYPE, VFIO_BASE + 24)
+
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 07/20] iommu/arm-smmu-v3: Link domains and devices
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (5 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 06/20] vfio: VFIO_IOMMU_BIND_MSI Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 08/20] iommu/arm-smmu-v3: Maintain a SID->device structure Eric Auger
` (12 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
When removing a mapping from a domain, we need to send an invalidation to
all devices that might have stored it in their Address Translation Cache
(ATC). In addition with SVM, we'll need to invalidate context descriptors
of all devices attached to a live domain.
Maintain a list of devices in each domain, protected by a spinlock. It is
updated every time we attach or detach devices to and from domains.
It needs to be a spinlock because we'll invalidate ATC entries from
within hardirq-safe contexts, but it may be possible to relax the read
side with RCU later.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 5059d09f3202..5d0d080f0a02 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -596,6 +596,11 @@ struct arm_smmu_device {
struct arm_smmu_master_data {
struct arm_smmu_device *smmu;
struct arm_smmu_strtab_ent ste;
+
+ struct arm_smmu_domain *domain;
+ struct list_head list; /* domain->devices */
+
+ struct device *dev;
};
/* SMMU private data for an IOMMU domain */
@@ -619,6 +624,9 @@ struct arm_smmu_domain {
};
struct iommu_domain domain;
+
+ struct list_head devices;
+ spinlock_t devices_lock;
};
struct arm_smmu_option_prop {
@@ -1472,6 +1480,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
}
mutex_init(&smmu_domain->init_mutex);
+ INIT_LIST_HEAD(&smmu_domain->devices);
+ spin_lock_init(&smmu_domain->devices_lock);
+
return &smmu_domain->domain;
}
@@ -1687,7 +1698,17 @@ static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
static void arm_smmu_detach_dev(struct device *dev)
{
+ unsigned long flags;
struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
+ struct arm_smmu_domain *smmu_domain = master->domain;
+
+ if (smmu_domain) {
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_del(&master->list);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+ master->domain = NULL;
+ }
master->ste.assigned = false;
arm_smmu_install_ste_for_dev(dev->iommu_fwspec);
@@ -1696,6 +1717,7 @@ static void arm_smmu_detach_dev(struct device *dev)
static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
{
int ret = 0;
+ unsigned long flags;
struct arm_smmu_device *smmu;
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_master_data *master;
@@ -1731,6 +1753,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
}
ste->assigned = true;
+ master->domain = smmu_domain;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_add(&master->list, &smmu_domain->devices);
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
ste->s1_cfg = NULL;
@@ -1849,6 +1876,7 @@ static int arm_smmu_add_device(struct device *dev)
return -ENOMEM;
master->smmu = smmu;
+ master->dev = dev;
fwspec->iommu_priv = master;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 08/20] iommu/arm-smmu-v3: Maintain a SID->device structure
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (6 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 07/20] iommu/arm-smmu-v3: Link domains and devices Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 09/20] iommu/smmuv3: Get prepared for nested stage support Eric Auger
` (11 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
When handling faults from the event or PRI queue, we need to find the
struct device associated to a SID. Add a rb_tree to keep track of SIDs.
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/arm-smmu-v3.c | 136 ++++++++++++++++++++++++++++++++++--
1 file changed, 132 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 5d0d080f0a02..80bb93b43a2e 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -590,6 +590,16 @@ struct arm_smmu_device {
/* IOMMU core code handle */
struct iommu_device iommu;
+
+ struct rb_root streams;
+ struct mutex streams_mutex;
+
+};
+
+struct arm_smmu_stream {
+ u32 id;
+ struct arm_smmu_master_data *master;
+ struct rb_node node;
};
/* SMMU private data for each master */
@@ -599,6 +609,7 @@ struct arm_smmu_master_data {
struct arm_smmu_domain *domain;
struct list_head list; /* domain->devices */
+ struct arm_smmu_stream *streams;
struct device *dev;
};
@@ -1224,6 +1235,32 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
+__maybe_unused
+static struct arm_smmu_master_data *
+arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
+{
+ struct rb_node *node;
+ struct arm_smmu_stream *stream;
+ struct arm_smmu_master_data *master = NULL;
+
+ mutex_lock(&smmu->streams_mutex);
+ node = smmu->streams.rb_node;
+ while (node) {
+ stream = rb_entry(node, struct arm_smmu_stream, node);
+ if (stream->id < sid) {
+ node = node->rb_right;
+ } else if (stream->id > sid) {
+ node = node->rb_left;
+ } else {
+ master = stream->master;
+ break;
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return master;
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
@@ -1847,6 +1884,71 @@ static bool arm_smmu_sid_in_range(struct arm_smmu_device *smmu, u32 sid)
return sid < limit;
}
+static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_data *master)
+{
+ int i;
+ int ret = 0;
+ struct arm_smmu_stream *new_stream, *cur_stream;
+ struct rb_node **new_node, *parent_node = NULL;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ master->streams = kcalloc(fwspec->num_ids,
+ sizeof(struct arm_smmu_stream), GFP_KERNEL);
+ if (!master->streams)
+ return -ENOMEM;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < fwspec->num_ids && !ret; i++) {
+ new_stream = &master->streams[i];
+ new_stream->id = fwspec->ids[i];
+ new_stream->master = master;
+
+ new_node = &(smmu->streams.rb_node);
+ while (*new_node) {
+ cur_stream = rb_entry(*new_node, struct arm_smmu_stream,
+ node);
+ parent_node = *new_node;
+ if (cur_stream->id > new_stream->id) {
+ new_node = &((*new_node)->rb_left);
+ } else if (cur_stream->id < new_stream->id) {
+ new_node = &((*new_node)->rb_right);
+ } else {
+ dev_warn(master->dev,
+ "stream %u already in tree\n",
+ cur_stream->id);
+ ret = -EINVAL;
+ break;
+ }
+ }
+
+ if (!ret) {
+ rb_link_node(&new_stream->node, parent_node, new_node);
+ rb_insert_color(&new_stream->node, &smmu->streams);
+ }
+ }
+ mutex_unlock(&smmu->streams_mutex);
+
+ return ret;
+}
+
+static void arm_smmu_remove_master(struct arm_smmu_device *smmu,
+ struct arm_smmu_master_data *master)
+{
+ int i;
+ struct iommu_fwspec *fwspec = master->dev->iommu_fwspec;
+
+ if (!master->streams)
+ return;
+
+ mutex_lock(&smmu->streams_mutex);
+ for (i = 0; i < fwspec->num_ids; i++)
+ rb_erase(&master->streams[i].node, &smmu->streams);
+ mutex_unlock(&smmu->streams_mutex);
+
+ kfree(master->streams);
+}
+
static struct iommu_ops arm_smmu_ops;
static int arm_smmu_add_device(struct device *dev)
@@ -1895,13 +1997,35 @@ static int arm_smmu_add_device(struct device *dev)
}
}
+ ret = iommu_device_link(&smmu->iommu, dev);
+ if (ret)
+ goto err_free_master;
+
+ ret = arm_smmu_insert_master(smmu, master);
+ if (ret)
+ goto err_unlink;
+
group = iommu_group_get_for_dev(dev);
- if (!IS_ERR(group)) {
- iommu_group_put(group);
- iommu_device_link(&smmu->iommu, dev);
+ if (IS_ERR(group)) {
+ ret = PTR_ERR(group);
+ goto err_remove_master;
}
- return PTR_ERR_OR_ZERO(group);
+ iommu_group_put(group);
+
+ return 0;
+
+err_remove_master:
+ arm_smmu_remove_master(smmu, master);
+
+err_unlink:
+ iommu_device_unlink(&smmu->iommu, dev);
+
+err_free_master:
+ kfree(master);
+ fwspec->iommu_priv = NULL;
+
+ return ret;
}
static void arm_smmu_remove_device(struct device *dev)
@@ -1918,6 +2042,7 @@ static void arm_smmu_remove_device(struct device *dev)
if (master && master->ste.assigned)
arm_smmu_detach_dev(dev);
iommu_group_remove_device(dev);
+ arm_smmu_remove_master(smmu, master);
iommu_device_unlink(&smmu->iommu, dev);
kfree(master);
iommu_fwspec_free(dev);
@@ -2209,6 +2334,9 @@ static int arm_smmu_init_structures(struct arm_smmu_device *smmu)
int ret;
atomic_set(&smmu->sync_nr, 0);
+ mutex_init(&smmu->streams_mutex);
+ smmu->streams = RB_ROOT;
+
ret = arm_smmu_init_queues(smmu);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 09/20] iommu/smmuv3: Get prepared for nested stage support
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (7 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 08/20] iommu/arm-smmu-v3: Maintain a SID->device structure Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 10/20] iommu/smmuv3: Implement bind_pasid_table Eric Auger
` (10 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
To allow nested stage support, we need to store both
stage 1 and stage 2 configurations (and remove the former
union).
arm_smmu_write_strtab_ent() is modified to write both stage
fields in the STE.
We add a nested_bypass field to the S1 configuration as the first
stage can be bypassed. Also the guest may force the STE to abort:
this information gets stored into the nested_abort field.
Only S2 stage is "finalized" as the host does not configure
S1 CD, guest does.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- invalidate the STE before moving from a live STE config to another
- add the nested_abort and nested_bypass fields
---
drivers/iommu/arm-smmu-v3.c | 43 ++++++++++++++++++++++++++++---------
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 80bb93b43a2e..9749c36208f3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -222,6 +222,7 @@
#define STRTAB_STE_0_CFG_BYPASS 4
#define STRTAB_STE_0_CFG_S1_TRANS 5
#define STRTAB_STE_0_CFG_S2_TRANS 6
+#define STRTAB_STE_0_CFG_NESTED 7
#define STRTAB_STE_0_S1FMT GENMASK_ULL(5, 4)
#define STRTAB_STE_0_S1FMT_LINEAR 0
@@ -497,6 +498,10 @@ struct arm_smmu_strtab_l1_desc {
struct arm_smmu_s1_cfg {
__le64 *cdptr;
dma_addr_t cdptr_dma;
+ /* in nested mode, tells s1 must be bypassed */
+ bool nested_bypass;
+ /* in nested mode, abort is forced by guest */
+ bool nested_abort;
struct arm_smmu_ctx_desc {
u16 asid;
@@ -521,6 +526,7 @@ struct arm_smmu_strtab_ent {
* configured according to the domain type.
*/
bool assigned;
+ bool nested;
struct arm_smmu_s1_cfg *s1_cfg;
struct arm_smmu_s2_cfg *s2_cfg;
};
@@ -629,10 +635,8 @@ struct arm_smmu_domain {
struct io_pgtable_ops *pgtbl_ops;
enum arm_smmu_domain_stage stage;
- union {
- struct arm_smmu_s1_cfg s1_cfg;
- struct arm_smmu_s2_cfg s2_cfg;
- };
+ struct arm_smmu_s1_cfg s1_cfg;
+ struct arm_smmu_s2_cfg s2_cfg;
struct iommu_domain domain;
@@ -1119,10 +1123,11 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
break;
case STRTAB_STE_0_CFG_S1_TRANS:
case STRTAB_STE_0_CFG_S2_TRANS:
+ case STRTAB_STE_0_CFG_NESTED:
ste_live = true;
break;
case STRTAB_STE_0_CFG_ABORT:
- if (disable_bypass)
+ if (disable_bypass || ste->nested)
break;
default:
BUG(); /* STE corruption */
@@ -1134,7 +1139,8 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
/* Bypass/fault */
if (!ste->assigned || !(ste->s1_cfg || ste->s2_cfg)) {
- if (!ste->assigned && disable_bypass)
+ if ((!ste->assigned && disable_bypass) ||
+ (ste->s1_cfg && ste->s1_cfg->nested_abort))
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_ABORT);
else
val |= FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_BYPASS);
@@ -1152,8 +1158,17 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
return;
}
+ if (ste->nested && ste_live) {
+ /*
+ * When enabling nested, the STE may be transitionning from
+ * s2 to nested and back. Invalidate the STE before changing it.
+ */
+ dst[0] = cpu_to_le64(0);
+ arm_smmu_sync_ste_for_sid(smmu, sid);
+ val = STRTAB_STE_0_V;
+ }
+
if (ste->s1_cfg) {
- BUG_ON(ste_live);
dst[1] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
@@ -1167,12 +1182,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
!(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
- val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
- FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
+ if (!ste->s1_cfg->nested_bypass)
+ val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
+ FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
}
if (ste->s2_cfg) {
- BUG_ON(ste_live);
dst[2] = cpu_to_le64(
FIELD_PREP(STRTAB_STE_2_S2VMID, ste->s2_cfg->vmid) |
FIELD_PREP(STRTAB_STE_2_VTCR, ste->s2_cfg->vtcr) |
@@ -1438,6 +1453,10 @@ static void arm_smmu_tlb_inv_context(void *cookie)
cmd.opcode = CMDQ_OP_TLBI_NH_ASID;
cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
cmd.tlbi.vmid = 0;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
+ cmd.opcode = CMDQ_OP_TLBI_NH_ASID;
+ cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
} else {
cmd.opcode = CMDQ_OP_TLBI_S12_VMALL;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
@@ -1462,6 +1481,10 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
cmd.opcode = CMDQ_OP_TLBI_NH_VA;
cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
+ } else if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
+ cmd.opcode = CMDQ_OP_TLBI_NH_VA;
+ cmd.tlbi.asid = smmu_domain->s1_cfg.cd.asid;
+ cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
} else {
cmd.opcode = CMDQ_OP_TLBI_S2_IPA;
cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 10/20] iommu/smmuv3: Implement bind_pasid_table
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (8 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 09/20] iommu/smmuv3: Get prepared for nested stage support Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 11/20] iommu/smmuv3: Implement cache_invalidate Eric Auger
` (9 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
On bind_pasid_table() we program STE S1 related info set
by the guest into the actual physical STEs. At minimum
we need to program the context descriptor GPA and compute
whether the guest wanted to bypass the stage 1 or induce
aborts for this STE.
On unbind, the STE stage 1 fields are reset.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- invalidate the STE before changing them
- hold init_mutex
- handle new fields
---
drivers/iommu/arm-smmu-v3.c | 85 +++++++++++++++++++++++++++++++++++++
1 file changed, 85 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 9749c36208f3..c7e05c0b93e1 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2168,6 +2168,89 @@ static void arm_smmu_put_resv_regions(struct device *dev,
kfree(entry);
}
+static int arm_smmu_bind_pasid_table(struct iommu_domain *domain,
+ struct iommu_pasid_table_config *cfg)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_master_data *entry;
+ struct arm_smmu_s1_cfg *s1_cfg;
+ struct arm_smmu_device *smmu;
+ unsigned long flags;
+ int ret = -EINVAL;
+
+ if (cfg->format != IOMMU_PASID_FORMAT_SMMUV3)
+ return -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ smmu = smmu_domain->smmu;
+
+ if (!smmu)
+ goto out;
+
+ if (!((smmu->features & ARM_SMMU_FEAT_TRANS_S1) &&
+ (smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+ dev_info(smmu_domain->smmu->dev,
+ "does not implement two stages\n");
+ goto out;
+ }
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ /* we currently support a single CD. S1DSS and S1FMT are ignored */
+ if (cfg->smmuv3.s1cdmax)
+ goto out;
+
+ s1_cfg = &smmu_domain->s1_cfg;
+ s1_cfg->nested_bypass = cfg->smmuv3.bypass;
+ s1_cfg->nested_abort = cfg->smmuv3.abort;
+ s1_cfg->cdptr_dma = cfg->smmuv3.s1contextptr;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(entry, &smmu_domain->devices, list) {
+ entry->ste.s1_cfg = &smmu_domain->s1_cfg;
+ entry->ste.nested = true;
+ arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+ }
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+ ret = 0;
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
+static void arm_smmu_unbind_pasid_table(struct iommu_domain *domain)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct arm_smmu_master_data *entry;
+ struct arm_smmu_s1_cfg *s1_cfg;
+ unsigned long flags;
+
+ if (!smmu)
+ return;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ return;
+
+ mutex_lock(&smmu_domain->init_mutex);
+
+ s1_cfg = &smmu_domain->s1_cfg;
+
+ spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+ list_for_each_entry(entry, &smmu_domain->devices, list) {
+ entry->ste.s1_cfg = NULL;
+ entry->ste.nested = false;
+ arm_smmu_install_ste_for_dev(entry->dev->iommu_fwspec);
+ }
+ spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+ s1_cfg->nested_abort = false;
+ s1_cfg->nested_bypass = false;
+ mutex_unlock(&smmu_domain->init_mutex);
+}
+
static struct iommu_ops arm_smmu_ops = {
.capable = arm_smmu_capable,
.domain_alloc = arm_smmu_domain_alloc,
@@ -2186,6 +2269,8 @@ static struct iommu_ops arm_smmu_ops = {
.of_xlate = arm_smmu_of_xlate,
.get_resv_regions = arm_smmu_get_resv_regions,
.put_resv_regions = arm_smmu_put_resv_regions,
+ .bind_pasid_table = arm_smmu_bind_pasid_table,
+ .unbind_pasid_table = arm_smmu_unbind_pasid_table,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 11/20] iommu/smmuv3: Implement cache_invalidate
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (9 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 10/20] iommu/smmuv3: Implement bind_pasid_table Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie Eric Auger
` (8 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Implement IOMMU_INV_TYPE_TLB invalidations. When
nr_pages is null we interpret this as a context
invalidation.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
The user API needs to be refined to discriminate context
invalidations from NH_VA invalidations. Also the leaf attribute
is not yet properly handled.
v1 -> v2:
- properly pass the asid
---
drivers/iommu/arm-smmu-v3.c | 40 +++++++++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index c7e05c0b93e1..4054da756a41 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2251,6 +2251,45 @@ static void arm_smmu_unbind_pasid_table(struct iommu_domain *domain)
mutex_unlock(&smmu_domain->init_mutex);
}
+static int
+arm_smmu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+ struct iommu_cache_invalidate_info *inv_info)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ return -EINVAL;
+
+ if (!smmu)
+ return -EINVAL;
+
+ switch (inv_info->hdr.type) {
+ case IOMMU_INV_TYPE_TLB:
+ /*
+ * TODO: On context invalidation, the userspace sets nr_pages
+ * to 0. Refine the API to add a dedicated flags and also
+ * properly handle the leaf parameter.
+ */
+ if (!inv_info->nr_pages) {
+ smmu_domain->s1_cfg.cd.asid = inv_info->arch_id;
+ arm_smmu_tlb_inv_context(smmu_domain);
+ } else {
+ size_t granule = 1 << (inv_info->size + 12);
+ size_t size = inv_info->nr_pages * granule;
+
+ smmu_domain->s1_cfg.cd.asid = inv_info->arch_id;
+ arm_smmu_tlb_inv_range_nosync(inv_info->addr, size,
+ granule, false,
+ smmu_domain);
+ __arm_smmu_tlb_sync(smmu);
+ }
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
static struct iommu_ops arm_smmu_ops = {
.capable = arm_smmu_capable,
.domain_alloc = arm_smmu_domain_alloc,
@@ -2271,6 +2310,7 @@ static struct iommu_ops arm_smmu_ops = {
.put_resv_regions = arm_smmu_put_resv_regions,
.bind_pasid_table = arm_smmu_bind_pasid_table,
.unbind_pasid_table = arm_smmu_unbind_pasid_table,
+ .cache_invalidate = arm_smmu_cache_invalidate,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (10 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 11/20] iommu/smmuv3: Implement cache_invalidate Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-10-24 18:02 ` Robin Murphy
2018-09-18 14:24 ` [RFC v2 13/20] iommu/smmuv3: Implement bind_guest_msi Eric Auger
` (7 subsequent siblings)
19 siblings, 1 reply; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a range provided by the user.
This does not work in nested mode.
If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).
So we end up with 2 unrelated mappings, at S1 and S2:
S1 S2
gIOVA -> gDB
hIOVA -> hDB
The PCI device would be programmed with hIOVA.
iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
so that gIOVA can be used by the host instead of re-allocating
a new IOVA. That way the host can create the following nested
mapping:
S1 S2
gIOVA -> gDB -> hDB
this time, the PCI device will be programmed with the gIOVA MSI
doorbell which is correctly map through the 2 stages.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
v1 -> v2:
- unmap stage2 on put()
---
drivers/iommu/dma-iommu.c | 97 +++++++++++++++++++++++++++++++++++++--
include/linux/dma-iommu.h | 11 +++++
2 files changed, 105 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 511ff9a1d6d9..53444c3e8f2f 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -37,12 +37,14 @@
struct iommu_dma_msi_page {
struct list_head list;
dma_addr_t iova;
+ dma_addr_t ipa;
phys_addr_t phys;
};
enum iommu_dma_cookie_type {
IOMMU_DMA_IOVA_COOKIE,
IOMMU_DMA_MSI_COOKIE,
+ IOMMU_DMA_NESTED_MSI_COOKIE,
};
struct iommu_dma_cookie {
@@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
*
* Users who manage their own IOVA allocation and do not want DMA API support,
* but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
* contiguous IOVA region, starting at @base, large enough to accommodate the
* number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
+ * use case)
*/
int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
{
struct iommu_dma_cookie *cookie;
+ int nesting, ret;
if (domain->type != IOMMU_DOMAIN_UNMANAGED)
return -EINVAL;
@@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
if (domain->iova_cookie)
return -EEXIST;
- cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+ ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
+ if (!ret && nesting)
+ cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+ else
+ cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
if (!cookie)
return -ENOMEM;
@@ -145,6 +155,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
{
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iommu_dma_msi_page *msi, *tmp;
+ bool s2_unmap = false;
if (!cookie)
return;
@@ -152,7 +163,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
put_iova_domain(&cookie->iovad);
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
+ s2_unmap = true;
+
list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
+ if (s2_unmap && msi->phys) {
+ size_t size = cookie_msi_granule(cookie);
+
+ WARN_ON(iommu_unmap(domain, msi->ipa, size) != size);
+ }
list_del(&msi->list);
kfree(msi);
}
@@ -161,6 +180,50 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
}
EXPORT_SYMBOL(iommu_put_dma_cookie);
+/**
+ * iommu_dma_bind_doorbell - Allows to provide a usable IOVA page
+ * @domain: domain handle
+ * @binding: IOVA/IPA binding
+ *
+ * In nested stage use case, the user can provide IOVA/IPA bindings
+ * corresponding to a guest MSI stage 1 mapping. When the host needs
+ * to map its own MSI doorbells, it can use the IPA as stage 2 input
+ * and map it onto the physical MSI doorbell.
+ */
+int iommu_dma_bind_doorbell(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding)
+{
+ struct iommu_dma_cookie *cookie = domain->iova_cookie;
+ struct iommu_dma_msi_page *msi;
+ dma_addr_t ipa, iova;
+ size_t size;
+
+ if (!cookie)
+ return -EINVAL;
+
+ if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+ return -EINVAL;
+
+ size = 1 << binding->granule;
+ iova = binding->iova & ~(phys_addr_t)(size - 1);
+ ipa = binding->gpa & ~(phys_addr_t)(size - 1);
+
+ list_for_each_entry(msi, &cookie->msi_page_list, list) {
+ if (msi->iova == iova)
+ return 0; /* this page is already registered */
+ }
+
+ msi = kzalloc(sizeof(*msi), GFP_KERNEL);
+ if (!msi)
+ return -ENOMEM;
+
+ msi->iova = iova;
+ msi->ipa = ipa;
+ list_add(&msi->list, &cookie->msi_page_list);
+ return 0;
+}
+EXPORT_SYMBOL(iommu_dma_bind_doorbell);
+
/**
* iommu_dma_get_resv_regions - Reserved region driver helper
* @dev: Device from iommu_get_resv_regions()
@@ -846,6 +909,34 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
if (msi_page->phys == msi_addr)
return msi_page;
+ /*
+ * In nested stage mode, we do not allocate an MSI page in
+ * a range provided by the user. Instead, IOVA/IPA bindings are
+ * individually provided. We reuse thise IOVAs to build the
+ * IOVA -> IPA -> MSI PA nested stage mapping.
+ */
+ if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE) {
+ list_for_each_entry(msi_page, &cookie->msi_page_list, list)
+ if (!msi_page->phys) { /* this binding is free to use */
+ dma_addr_t ipa = msi_page->ipa;
+ int ret;
+
+ msi_page->phys = msi_addr;
+
+ /* do the stage 2 mapping */
+ ret = iommu_map(domain, ipa, msi_addr, size,
+ IOMMU_MMIO | IOMMU_WRITE);
+ if (ret) {
+ pr_warn("MSI S2 mapping failed (%d)\n",
+ ret);
+ return NULL;
+ }
+ return msi_page;
+ }
+ pr_warn("%s no MSI binding found\n", __func__);
+ return NULL;
+ }
+
msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
if (!msi_page)
return NULL;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index e8ca5e654277..324745eef644 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -24,6 +24,7 @@
#include <linux/dma-mapping.h>
#include <linux/iommu.h>
#include <linux/msi.h>
+#include <uapi/linux/iommu.h>
int iommu_dma_init(void);
@@ -74,12 +75,15 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
/* The DMA API isn't _quite_ the whole story, though... */
void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
+int iommu_dma_bind_doorbell(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding);
#else
struct iommu_domain;
struct msi_msg;
struct device;
+struct iommu_guest_msi_binding;
static inline int iommu_dma_init(void)
{
@@ -104,6 +108,13 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
{
}
+static inline int
+iommu_dma_bind_doorbell(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding)
+{
+ return -ENODEV;
+}
+
static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
{
}
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
2018-09-18 14:24 ` [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie Eric Auger
@ 2018-10-24 18:02 ` Robin Murphy
2018-10-24 18:44 ` Auger Eric
0 siblings, 1 reply; 33+ messages in thread
From: Robin Murphy @ 2018-10-24 18:02 UTC (permalink / raw)
To: Eric Auger, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Hi Eric,
On 2018-09-18 3:24 pm, Eric Auger wrote:
> Up to now, when the type was UNMANAGED, we used to
> allocate IOVA pages within a range provided by the user.
> This does not work in nested mode.
>
> If both the host and the guest are exposed with SMMUs, each
> would allocate an IOVA. The guest allocates an IOVA (gIOVA)
> to map onto the guest MSI doorbell (gDB). The Host allocates
> another IOVA (hIOVA) to map onto the physical doorbell (hDB).
>
> So we end up with 2 unrelated mappings, at S1 and S2:
> S1 S2
> gIOVA -> gDB
> hIOVA -> hDB
>
> The PCI device would be programmed with hIOVA.
>
> iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
> so that gIOVA can be used by the host instead of re-allocating
> a new IOVA. That way the host can create the following nested
> mapping:
>
> S1 S2
> gIOVA -> gDB -> hDB
>
> this time, the PCI device will be programmed with the gIOVA MSI
> doorbell which is correctly map through the 2 stages.
If I'm understanding things correctly, this plus a couple of the
preceding patches all add up to a rather involved way of coercing an
automatic allocator to only "allocate" predetermined addresses in an
entirely known-ahead-of-time manner. Given that the guy calling
iommu_dma_bind_doorbell() could seemingly just as easily call
iommu_map() at that point and not bother with an allocator cookie and
all this machinery at all, what am I missing?
Robin.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
>
> v1 -> v2:
> - unmap stage2 on put()
> ---
> drivers/iommu/dma-iommu.c | 97 +++++++++++++++++++++++++++++++++++++--
> include/linux/dma-iommu.h | 11 +++++
> 2 files changed, 105 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 511ff9a1d6d9..53444c3e8f2f 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -37,12 +37,14 @@
> struct iommu_dma_msi_page {
> struct list_head list;
> dma_addr_t iova;
> + dma_addr_t ipa;
> phys_addr_t phys;
> };
>
> enum iommu_dma_cookie_type {
> IOMMU_DMA_IOVA_COOKIE,
> IOMMU_DMA_MSI_COOKIE,
> + IOMMU_DMA_NESTED_MSI_COOKIE,
> };
>
> struct iommu_dma_cookie {
> @@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
> *
> * Users who manage their own IOVA allocation and do not want DMA API support,
> * but would still like to take advantage of automatic MSI remapping, can use
> - * this to initialise their own domain appropriately. Users should reserve a
> + * this to initialise their own domain appropriately. Users may reserve a
> * contiguous IOVA region, starting at @base, large enough to accommodate the
> * number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
> - * used by the devices attached to @domain.
> + * used by the devices attached to @domain. The other way round is to provide
> + * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
> + * use case)
> */
> int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
> {
> struct iommu_dma_cookie *cookie;
> + int nesting, ret;
>
> if (domain->type != IOMMU_DOMAIN_UNMANAGED)
> return -EINVAL;
> @@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
> if (domain->iova_cookie)
> return -EEXIST;
>
> - cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
> + ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
> + if (!ret && nesting)
> + cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
> + else
> + cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
> +
> if (!cookie)
> return -ENOMEM;
>
> @@ -145,6 +155,7 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
> {
> struct iommu_dma_cookie *cookie = domain->iova_cookie;
> struct iommu_dma_msi_page *msi, *tmp;
> + bool s2_unmap = false;
>
> if (!cookie)
> return;
> @@ -152,7 +163,15 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
> if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
> put_iova_domain(&cookie->iovad);
>
> + if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
> + s2_unmap = true;
> +
> list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
> + if (s2_unmap && msi->phys) {
> + size_t size = cookie_msi_granule(cookie);
> +
> + WARN_ON(iommu_unmap(domain, msi->ipa, size) != size);
> + }
> list_del(&msi->list);
> kfree(msi);
> }
> @@ -161,6 +180,50 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
> }
> EXPORT_SYMBOL(iommu_put_dma_cookie);
>
> +/**
> + * iommu_dma_bind_doorbell - Allows to provide a usable IOVA page
> + * @domain: domain handle
> + * @binding: IOVA/IPA binding
> + *
> + * In nested stage use case, the user can provide IOVA/IPA bindings
> + * corresponding to a guest MSI stage 1 mapping. When the host needs
> + * to map its own MSI doorbells, it can use the IPA as stage 2 input
> + * and map it onto the physical MSI doorbell.
> + */
> +int iommu_dma_bind_doorbell(struct iommu_domain *domain,
> + struct iommu_guest_msi_binding *binding)
> +{
> + struct iommu_dma_cookie *cookie = domain->iova_cookie;
> + struct iommu_dma_msi_page *msi;
> + dma_addr_t ipa, iova;
> + size_t size;
> +
> + if (!cookie)
> + return -EINVAL;
> +
> + if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
> + return -EINVAL;
> +
> + size = 1 << binding->granule;
> + iova = binding->iova & ~(phys_addr_t)(size - 1);
> + ipa = binding->gpa & ~(phys_addr_t)(size - 1);
> +
> + list_for_each_entry(msi, &cookie->msi_page_list, list) {
> + if (msi->iova == iova)
> + return 0; /* this page is already registered */
> + }
> +
> + msi = kzalloc(sizeof(*msi), GFP_KERNEL);
> + if (!msi)
> + return -ENOMEM;
> +
> + msi->iova = iova;
> + msi->ipa = ipa;
> + list_add(&msi->list, &cookie->msi_page_list);
> + return 0;
> +}
> +EXPORT_SYMBOL(iommu_dma_bind_doorbell);
> +
> /**
> * iommu_dma_get_resv_regions - Reserved region driver helper
> * @dev: Device from iommu_get_resv_regions()
> @@ -846,6 +909,34 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
> if (msi_page->phys == msi_addr)
> return msi_page;
>
> + /*
> + * In nested stage mode, we do not allocate an MSI page in
> + * a range provided by the user. Instead, IOVA/IPA bindings are
> + * individually provided. We reuse thise IOVAs to build the
> + * IOVA -> IPA -> MSI PA nested stage mapping.
> + */
> + if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE) {
> + list_for_each_entry(msi_page, &cookie->msi_page_list, list)
> + if (!msi_page->phys) { /* this binding is free to use */
> + dma_addr_t ipa = msi_page->ipa;
> + int ret;
> +
> + msi_page->phys = msi_addr;
> +
> + /* do the stage 2 mapping */
> + ret = iommu_map(domain, ipa, msi_addr, size,
> + IOMMU_MMIO | IOMMU_WRITE);
> + if (ret) {
> + pr_warn("MSI S2 mapping failed (%d)\n",
> + ret);
> + return NULL;
> + }
> + return msi_page;
> + }
> + pr_warn("%s no MSI binding found\n", __func__);
> + return NULL;
> + }
> +
> msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
> if (!msi_page)
> return NULL;
> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
> index e8ca5e654277..324745eef644 100644
> --- a/include/linux/dma-iommu.h
> +++ b/include/linux/dma-iommu.h
> @@ -24,6 +24,7 @@
> #include <linux/dma-mapping.h>
> #include <linux/iommu.h>
> #include <linux/msi.h>
> +#include <uapi/linux/iommu.h>
>
> int iommu_dma_init(void);
>
> @@ -74,12 +75,15 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
> /* The DMA API isn't _quite_ the whole story, though... */
> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
> void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
> +int iommu_dma_bind_doorbell(struct iommu_domain *domain,
> + struct iommu_guest_msi_binding *binding);
>
> #else
>
> struct iommu_domain;
> struct msi_msg;
> struct device;
> +struct iommu_guest_msi_binding;
>
> static inline int iommu_dma_init(void)
> {
> @@ -104,6 +108,13 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
> {
> }
>
> +static inline int
> +iommu_dma_bind_doorbell(struct iommu_domain *domain,
> + struct iommu_guest_msi_binding *binding)
> +{
> + return -ENODEV;
> +}
> +
> static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
> {
> }
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
2018-10-24 18:02 ` Robin Murphy
@ 2018-10-24 18:44 ` Auger Eric
2018-10-24 22:05 ` Robin Murphy
0 siblings, 1 reply; 33+ messages in thread
From: Auger Eric @ 2018-10-24 18:44 UTC (permalink / raw)
To: Robin Murphy, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Hi Robin,
On 10/24/18 8:02 PM, Robin Murphy wrote:
> Hi Eric,
>
> On 2018-09-18 3:24 pm, Eric Auger wrote:
>> Up to now, when the type was UNMANAGED, we used to
>> allocate IOVA pages within a range provided by the user.
>> This does not work in nested mode.
>>
>> If both the host and the guest are exposed with SMMUs, each
>> would allocate an IOVA. The guest allocates an IOVA (gIOVA)
>> to map onto the guest MSI doorbell (gDB). The Host allocates
>> another IOVA (hIOVA) to map onto the physical doorbell (hDB).
>>
>> So we end up with 2 unrelated mappings, at S1 and S2:
>> S1 S2
>> gIOVA -> gDB
>> hIOVA -> hDB
>>
>> The PCI device would be programmed with hIOVA.
>>
>> iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
>> so that gIOVA can be used by the host instead of re-allocating
>> a new IOVA. That way the host can create the following nested
>> mapping:
>>
>> S1 S2
>> gIOVA -> gDB -> hDB
>>
>> this time, the PCI device will be programmed with the gIOVA MSI
>> doorbell which is correctly map through the 2 stages.
>
> If I'm understanding things correctly, this plus a couple of the
> preceding patches all add up to a rather involved way of coercing an
> automatic allocator to only "allocate" predetermined addresses in an
> entirely known-ahead-of-time manner.
agreed
Given that the guy calling
> iommu_dma_bind_doorbell() could seemingly just as easily call
> iommu_map() at that point and not bother with an allocator cookie and
> all this machinery at all, what am I missing?
Well iommu_dma_map_msi_msg() gets called and is part of this existing
MSI mapping machinery. If we do not do anything this function allocates
an hIOVA that is not involved in any nested setup. So either we coerce
the allocator in place (which is what this series does) or we unplug the
allocator to replace this latter with a simple S2 mapping, as you
suggest, ie. iommu_map(gDB, hDB). Assuming we unplug the allocator, the
guy who actually calls iommu_dma_bind_doorbell() knows gDB but does not
know hDB. So I don't really get how we can simplify things.
Thanks
Eric
>
> Robin.
>
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - unmap stage2 on put()
>> ---
>> drivers/iommu/dma-iommu.c | 97 +++++++++++++++++++++++++++++++++++++--
>> include/linux/dma-iommu.h | 11 +++++
>> 2 files changed, 105 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 511ff9a1d6d9..53444c3e8f2f 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -37,12 +37,14 @@
>> struct iommu_dma_msi_page {
>> struct list_head list;
>> dma_addr_t iova;
>> + dma_addr_t ipa;
>> phys_addr_t phys;
>> };
>> enum iommu_dma_cookie_type {
>> IOMMU_DMA_IOVA_COOKIE,
>> IOMMU_DMA_MSI_COOKIE,
>> + IOMMU_DMA_NESTED_MSI_COOKIE,
>> };
>> struct iommu_dma_cookie {
>> @@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
>> *
>> * Users who manage their own IOVA allocation and do not want DMA
>> API support,
>> * but would still like to take advantage of automatic MSI
>> remapping, can use
>> - * this to initialise their own domain appropriately. Users should
>> reserve a
>> + * this to initialise their own domain appropriately. Users may
>> reserve a
>> * contiguous IOVA region, starting at @base, large enough to
>> accommodate the
>> * number of PAGE_SIZE mappings necessary to cover every MSI
>> doorbell address
>> - * used by the devices attached to @domain.
>> + * used by the devices attached to @domain. The other way round is to
>> provide
>> + * usable iova pages through the iommu_dma_bind_doorbell API (nested
>> stages
>> + * use case)
>> */
>> int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>> {
>> struct iommu_dma_cookie *cookie;
>> + int nesting, ret;
>> if (domain->type != IOMMU_DOMAIN_UNMANAGED)
>> return -EINVAL;
>> @@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain
>> *domain, dma_addr_t base)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> + ret = iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
>> + if (!ret && nesting)
>> + cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
>> + else
>> + cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> +
>> if (!cookie)
>> return -ENOMEM;
>> @@ -145,6 +155,7 @@ void iommu_put_dma_cookie(struct iommu_domain
>> *domain)
>> {
>> struct iommu_dma_cookie *cookie = domain->iova_cookie;
>> struct iommu_dma_msi_page *msi, *tmp;
>> + bool s2_unmap = false;
>> if (!cookie)
>> return;
>> @@ -152,7 +163,15 @@ void iommu_put_dma_cookie(struct iommu_domain
>> *domain)
>> if (cookie->type == IOMMU_DMA_IOVA_COOKIE && cookie->iovad.granule)
>> put_iova_domain(&cookie->iovad);
>> + if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE)
>> + s2_unmap = true;
>> +
>> list_for_each_entry_safe(msi, tmp, &cookie->msi_page_list, list) {
>> + if (s2_unmap && msi->phys) {
>> + size_t size = cookie_msi_granule(cookie);
>> +
>> + WARN_ON(iommu_unmap(domain, msi->ipa, size) != size);
>> + }
>> list_del(&msi->list);
>> kfree(msi);
>> }
>> @@ -161,6 +180,50 @@ void iommu_put_dma_cookie(struct iommu_domain
>> *domain)
>> }
>> EXPORT_SYMBOL(iommu_put_dma_cookie);
>> +/**
>> + * iommu_dma_bind_doorbell - Allows to provide a usable IOVA page
>> + * @domain: domain handle
>> + * @binding: IOVA/IPA binding
>> + *
>> + * In nested stage use case, the user can provide IOVA/IPA bindings
>> + * corresponding to a guest MSI stage 1 mapping. When the host needs
>> + * to map its own MSI doorbells, it can use the IPA as stage 2 input
>> + * and map it onto the physical MSI doorbell.
>> + */
>> +int iommu_dma_bind_doorbell(struct iommu_domain *domain,
>> + struct iommu_guest_msi_binding *binding)
>> +{
>> + struct iommu_dma_cookie *cookie = domain->iova_cookie;
>> + struct iommu_dma_msi_page *msi;
>> + dma_addr_t ipa, iova;
>> + size_t size;
>> +
>> + if (!cookie)
>> + return -EINVAL;
>> +
>> + if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
>> + return -EINVAL;
>> +
>> + size = 1 << binding->granule;
>> + iova = binding->iova & ~(phys_addr_t)(size - 1);
>> + ipa = binding->gpa & ~(phys_addr_t)(size - 1);
>> +
>> + list_for_each_entry(msi, &cookie->msi_page_list, list) {
>> + if (msi->iova == iova)
>> + return 0; /* this page is already registered */
>> + }
>> +
>> + msi = kzalloc(sizeof(*msi), GFP_KERNEL);
>> + if (!msi)
>> + return -ENOMEM;
>> +
>> + msi->iova = iova;
>> + msi->ipa = ipa;
>> + list_add(&msi->list, &cookie->msi_page_list);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(iommu_dma_bind_doorbell);
>> +
>> /**
>> * iommu_dma_get_resv_regions - Reserved region driver helper
>> * @dev: Device from iommu_get_resv_regions()
>> @@ -846,6 +909,34 @@ static struct iommu_dma_msi_page
>> *iommu_dma_get_msi_page(struct device *dev,
>> if (msi_page->phys == msi_addr)
>> return msi_page;
>> + /*
>> + * In nested stage mode, we do not allocate an MSI page in
>> + * a range provided by the user. Instead, IOVA/IPA bindings are
>> + * individually provided. We reuse thise IOVAs to build the
>> + * IOVA -> IPA -> MSI PA nested stage mapping.
>> + */
>> + if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE) {
>> + list_for_each_entry(msi_page, &cookie->msi_page_list, list)
>> + if (!msi_page->phys) { /* this binding is free to use */
>> + dma_addr_t ipa = msi_page->ipa;
>> + int ret;
>> +
>> + msi_page->phys = msi_addr;
>> +
>> + /* do the stage 2 mapping */
>> + ret = iommu_map(domain, ipa, msi_addr, size,
>> + IOMMU_MMIO | IOMMU_WRITE);
>> + if (ret) {
>> + pr_warn("MSI S2 mapping failed (%d)\n",
>> + ret);
>> + return NULL;
>> + }
>> + return msi_page;
>> + }
>> + pr_warn("%s no MSI binding found\n", __func__);
>> + return NULL;
>> + }
>> +
>> msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
>> if (!msi_page)
>> return NULL;
>> diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
>> index e8ca5e654277..324745eef644 100644
>> --- a/include/linux/dma-iommu.h
>> +++ b/include/linux/dma-iommu.h
>> @@ -24,6 +24,7 @@
>> #include <linux/dma-mapping.h>
>> #include <linux/iommu.h>
>> #include <linux/msi.h>
>> +#include <uapi/linux/iommu.h>
>> int iommu_dma_init(void);
>> @@ -74,12 +75,15 @@ int iommu_dma_mapping_error(struct device *dev,
>> dma_addr_t dma_addr);
>> /* The DMA API isn't _quite_ the whole story, though... */
>> void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
>> void iommu_dma_get_resv_regions(struct device *dev, struct list_head
>> *list);
>> +int iommu_dma_bind_doorbell(struct iommu_domain *domain,
>> + struct iommu_guest_msi_binding *binding);
>> #else
>> struct iommu_domain;
>> struct msi_msg;
>> struct device;
>> +struct iommu_guest_msi_binding;
>> static inline int iommu_dma_init(void)
>> {
>> @@ -104,6 +108,13 @@ static inline void iommu_dma_map_msi_msg(int irq,
>> struct msi_msg *msg)
>> {
>> }
>> +static inline int
>> +iommu_dma_bind_doorbell(struct iommu_domain *domain,
>> + struct iommu_guest_msi_binding *binding)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> static inline void iommu_dma_get_resv_regions(struct device *dev,
>> struct list_head *list)
>> {
>> }
>>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
2018-10-24 18:44 ` Auger Eric
@ 2018-10-24 22:05 ` Robin Murphy
2018-10-27 9:24 ` Auger Eric
0 siblings, 1 reply; 33+ messages in thread
From: Robin Murphy @ 2018-10-24 22:05 UTC (permalink / raw)
To: Auger Eric, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
On 2018-10-24 7:44 pm, Auger Eric wrote:
> Hi Robin,
>
> On 10/24/18 8:02 PM, Robin Murphy wrote:
>> Hi Eric,
>>
>> On 2018-09-18 3:24 pm, Eric Auger wrote:
>>> Up to now, when the type was UNMANAGED, we used to
>>> allocate IOVA pages within a range provided by the user.
>>> This does not work in nested mode.
>>>
>>> If both the host and the guest are exposed with SMMUs, each
>>> would allocate an IOVA. The guest allocates an IOVA (gIOVA)
>>> to map onto the guest MSI doorbell (gDB). The Host allocates
>>> another IOVA (hIOVA) to map onto the physical doorbell (hDB).
>>>
>>> So we end up with 2 unrelated mappings, at S1 and S2:
>>> S1 S2
>>> gIOVA -> gDB
>>> hIOVA -> hDB
>>>
>>> The PCI device would be programmed with hIOVA.
>>>
>>> iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
>>> so that gIOVA can be used by the host instead of re-allocating
>>> a new IOVA. That way the host can create the following nested
>>> mapping:
>>>
>>> S1 S2
>>> gIOVA -> gDB -> hDB
>>>
>>> this time, the PCI device will be programmed with the gIOVA MSI
>>> doorbell which is correctly map through the 2 stages.
>>
>> If I'm understanding things correctly, this plus a couple of the
>> preceding patches all add up to a rather involved way of coercing an
>> automatic allocator to only "allocate" predetermined addresses in an
>> entirely known-ahead-of-time manner.
> agreed
> Given that the guy calling
>> iommu_dma_bind_doorbell() could seemingly just as easily call
>> iommu_map() at that point and not bother with an allocator cookie and
>> all this machinery at all, what am I missing?
> Well iommu_dma_map_msi_msg() gets called and is part of this existing
> MSI mapping machinery. If we do not do anything this function allocates
> an hIOVA that is not involved in any nested setup. So either we coerce
> the allocator in place (which is what this series does) or we unplug the
> allocator to replace this latter with a simple S2 mapping, as you
> suggest, ie. iommu_map(gDB, hDB). Assuming we unplug the allocator, the
> guy who actually calls iommu_dma_bind_doorbell() knows gDB but does not
> know hDB. So I don't really get how we can simplify things.
OK, there's what I was missing :D
But that then seems to reveal a somewhat bigger problem - if the callers
are simply registering IPAs, and relying on the ITS driver to grab an
entry and fill in a PA later, then how does either one know *which* PA
is supposed to belong to a given IPA in the case where you have multiple
devices with different ITS targets assigned to the same guest? (and if
it's possible to assume a guest will use per-device stage 1 mappings and
present it with a single vITS backed by multiple pITSes, I think things
start breaking even harder.)
Other than allowing arbitrary disjoint IOVA pages, I'm not sure this
really works any differently from the existing MSI cookie now that I
look more closely :/
Robin.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie
2018-10-24 22:05 ` Robin Murphy
@ 2018-10-27 9:24 ` Auger Eric
0 siblings, 0 replies; 33+ messages in thread
From: Auger Eric @ 2018-10-27 9:24 UTC (permalink / raw)
To: Robin Murphy, eric.auger.pro, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Hi Robin,
On 10/25/18 12:05 AM, Robin Murphy wrote:
> On 2018-10-24 7:44 pm, Auger Eric wrote:
>> Hi Robin,
>>
>> On 10/24/18 8:02 PM, Robin Murphy wrote:
>>> Hi Eric,
>>>
>>> On 2018-09-18 3:24 pm, Eric Auger wrote:
>>>> Up to now, when the type was UNMANAGED, we used to
>>>> allocate IOVA pages within a range provided by the user.
>>>> This does not work in nested mode.
>>>>
>>>> If both the host and the guest are exposed with SMMUs, each
>>>> would allocate an IOVA. The guest allocates an IOVA (gIOVA)
>>>> to map onto the guest MSI doorbell (gDB). The Host allocates
>>>> another IOVA (hIOVA) to map onto the physical doorbell (hDB).
>>>>
>>>> So we end up with 2 unrelated mappings, at S1 and S2:
>>>> S1 S2
>>>> gIOVA -> gDB
>>>> hIOVA -> hDB
>>>>
>>>> The PCI device would be programmed with hIOVA.
>>>>
>>>> iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
>>>> so that gIOVA can be used by the host instead of re-allocating
>>>> a new IOVA. That way the host can create the following nested
>>>> mapping:
>>>>
>>>> S1 S2
>>>> gIOVA -> gDB -> hDB
>>>>
>>>> this time, the PCI device will be programmed with the gIOVA MSI
>>>> doorbell which is correctly map through the 2 stages.
>>>
>>> If I'm understanding things correctly, this plus a couple of the
>>> preceding patches all add up to a rather involved way of coercing an
>>> automatic allocator to only "allocate" predetermined addresses in an
>>> entirely known-ahead-of-time manner.
>> agreed
>> Given that the guy calling
>>> iommu_dma_bind_doorbell() could seemingly just as easily call
>>> iommu_map() at that point and not bother with an allocator cookie and
>>> all this machinery at all, what am I missing?
>> Well iommu_dma_map_msi_msg() gets called and is part of this existing
>> MSI mapping machinery. If we do not do anything this function allocates
>> an hIOVA that is not involved in any nested setup. So either we coerce
>> the allocator in place (which is what this series does) or we unplug the
>> allocator to replace this latter with a simple S2 mapping, as you
>> suggest, ie. iommu_map(gDB, hDB). Assuming we unplug the allocator, the
>> guy who actually calls iommu_dma_bind_doorbell() knows gDB but does not
>> know hDB. So I don't really get how we can simplify things.
>
> OK, there's what I was missing :D
>
> But that then seems to reveal a somewhat bigger problem - if the callers
> are simply registering IPAs, and relying on the ITS driver to grab an
> entry and fill in a PA later, then how does either one know *which* PA
> is supposed to belong to a given IPA in the case where you have multiple
> devices with different ITS targets assigned to the same guest?
You're definitively right here. I think this can be resolved by passing
the struct device handle along with the stage1 mapping and storing the
info together. Then when the host MSI controller looks for a free
unmapped iova, it must also check whether the device belongs to its MSI
domain.
(and if
> it's possible to assume a guest will use per-device stage 1 mappings and
> present it with a single vITS backed by multiple pITSes, I think things
> start breaking even harder.)
I don't really get your point here. Assigned devices on guest side
should be in separate iommu domain because we want them to get isolated
from each other. There is a single vITS as of now and I don't think we
will change that anytime soon. The vITS driver is allocating a gIOVA for
each separate domain and I currently "trap" the gIOVA/gPA mapping on
irqfd routing setup. This mapping gets associated to a VFIO IOMMU, one
per assigned device, so we have different vfio containers for each of
them. If I then enumerate all the devices attached to the containers and
pass this stage1 binding along with the device struct, I think we should
be OK?
Thanks
Eric
>
> Other than allowing arbitrary disjoint IOVA pages, I'm not sure this
> really works any differently from the existing MSI cookie now that I
> look more closely :/
>
> Robin.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC v2 13/20] iommu/smmuv3: Implement bind_guest_msi
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (11 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 12/20] dma-iommu: Implement NESTED_MSI cookie Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 14/20] iommu: introduce device fault data Eric Auger
` (6 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
The bind_guest_msi() callback checks the domain
is NESTED and redirect to the dma-iommu implementation.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm-smmu-v3.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4054da756a41..d9d300ab62a5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2168,6 +2168,27 @@ static void arm_smmu_put_resv_regions(struct device *dev,
kfree(entry);
}
+static int arm_smmu_bind_guest_msi(struct iommu_domain *domain,
+ struct iommu_guest_msi_binding *binding)
+{
+ struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+ struct arm_smmu_device *smmu;
+ int ret = -EINVAL;
+
+ mutex_lock(&smmu_domain->init_mutex);
+ smmu = smmu_domain->smmu;
+ if (!smmu)
+ goto out;
+
+ if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+ goto out;
+
+ ret = iommu_dma_bind_doorbell(domain, binding);
+out:
+ mutex_unlock(&smmu_domain->init_mutex);
+ return ret;
+}
+
static int arm_smmu_bind_pasid_table(struct iommu_domain *domain,
struct iommu_pasid_table_config *cfg)
{
@@ -2311,6 +2332,7 @@ static struct iommu_ops arm_smmu_ops = {
.bind_pasid_table = arm_smmu_bind_pasid_table,
.unbind_pasid_table = arm_smmu_unbind_pasid_table,
.cache_invalidate = arm_smmu_cache_invalidate,
+ .bind_guest_msi = arm_smmu_bind_guest_msi,
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 14/20] iommu: introduce device fault data
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (12 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 13/20] iommu/smmuv3: Implement bind_guest_msi Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-20 22:06 ` Jacob Pan
2018-09-18 14:24 ` [RFC v2 15/20] driver core: add per device iommu param Eric Auger
` (5 subsequent siblings)
19 siblings, 1 reply; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
Device faults detected by IOMMU can be reported outside IOMMU
subsystem for further processing. This patch intends to provide
a generic device fault data such that device drivers can be
communicated with IOMMU faults without model specific knowledge.
The proposed format is the result of discussion at:
https://lkml.org/lkml/2017/11/10/291
Part of the code is based on Jean-Philippe Brucker's patchset
(https://patchwork.kernel.org/patch/9989315/).
The assumption is that model specific IOMMU driver can filter and
handle most of the internal faults if the cause is within IOMMU driver
control. Therefore, the fault reasons can be reported are grouped
and generalized based common specifications such as PCI ATS.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
[moved part of the iommu_fault_event struct in the uapi, enriched
the fault reasons to be able to map unrecoverable SMMUv3 errors]
---
include/linux/iommu.h | 55 ++++++++++++++++++++++++-
include/uapi/linux/iommu.h | 83 ++++++++++++++++++++++++++++++++++++++
2 files changed, 136 insertions(+), 2 deletions(-)
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9bd3e63d562b..7529c14ff506 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -49,13 +49,17 @@ struct bus_type;
struct device;
struct iommu_domain;
struct notifier_block;
+struct iommu_fault_event;
/* iommu fault flags */
-#define IOMMU_FAULT_READ 0x0
-#define IOMMU_FAULT_WRITE 0x1
+#define IOMMU_FAULT_READ (1 << 0)
+#define IOMMU_FAULT_WRITE (1 << 1)
+#define IOMMU_FAULT_EXEC (1 << 2)
+#define IOMMU_FAULT_PRIV (1 << 3)
typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
struct device *, unsigned long, int, void *);
+typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *, void *);
struct iommu_domain_geometry {
dma_addr_t aperture_start; /* First address that can be mapped */
@@ -262,6 +266,52 @@ struct iommu_device {
struct device *dev;
};
+/**
+ * struct iommu_fault_event - Generic per device fault data
+ *
+ * - PCI and non-PCI devices
+ * - Recoverable faults (e.g. page request), information based on PCI ATS
+ * and PASID spec.
+ * - Un-recoverable faults of device interest
+ * - DMA remapping and IRQ remapping faults
+ *
+ * @fault: fault descriptor
+ * @device_private: if present, uniquely identify device-specific
+ * private data for an individual page request.
+ * @iommu_private: used by the IOMMU driver for storing fault-specific
+ * data. Users should not modify this field before
+ * sending the fault response.
+ */
+struct iommu_fault_event {
+ struct iommu_fault fault;
+ u64 device_private;
+ u64 iommu_private;
+};
+
+/**
+ * struct iommu_fault_param - per-device IOMMU fault data
+ * @dev_fault_handler: Callback function to handle IOMMU faults at device level
+ * @data: handler private data
+ *
+ */
+struct iommu_fault_param {
+ iommu_dev_fault_handler_t handler;
+ void *data;
+};
+
+/**
+ * struct iommu_param - collection of per-device IOMMU data
+ *
+ * @fault_param: IOMMU detected device fault reporting data
+ *
+ * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
+ * struct iommu_group *iommu_group;
+ * struct iommu_fwspec *iommu_fwspec;
+ */
+struct iommu_param {
+ struct iommu_fault_param *fault_param;
+};
+
int iommu_device_register(struct iommu_device *iommu);
void iommu_device_unregister(struct iommu_device *iommu);
int iommu_device_sysfs_add(struct iommu_device *iommu,
@@ -429,6 +479,7 @@ struct iommu_ops {};
struct iommu_group {};
struct iommu_fwspec {};
struct iommu_device {};
+struct iommu_fault_param {};
static inline bool iommu_present(struct bus_type *bus)
{
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 21adb2a964e5..a0fe5c2fb236 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
__u64 gpa;
__u32 granule;
};
+
+/* Generic fault types, can be expanded IRQ remapping fault */
+enum iommu_fault_type {
+ IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
+ IOMMU_FAULT_PAGE_REQ, /* page request fault */
+};
+
+enum iommu_fault_reason {
+ IOMMU_FAULT_REASON_UNKNOWN = 0,
+
+ /* IOMMU internal error, no specific reason to report out */
+ IOMMU_FAULT_REASON_INTERNAL,
+
+ /* Could not access the PASID table (fetch caused external abort) */
+ IOMMU_FAULT_REASON_PASID_FETCH,
+
+ /* could not access the device context (fetch caused external abort) */
+ IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
+
+ /* pasid entry is invalid or has configuration errors */
+ IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
+
+ /* device context entry is invalid or has configuration errors */
+ IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
+ /*
+ * PASID is out of range (e.g. exceeds the maximum PASID
+ * supported by the IOMMU) or disabled.
+ */
+ IOMMU_FAULT_REASON_PASID_INVALID,
+
+ /* source id is out of range */
+ IOMMU_FAULT_REASON_SOURCEID_INVALID,
+
+ /*
+ * An external abort occurred fetching (or updating) a translation
+ * table descriptor
+ */
+ IOMMU_FAULT_REASON_WALK_EABT,
+
+ /*
+ * Could not access the page table entry (Bad address),
+ * actual translation fault
+ */
+ IOMMU_FAULT_REASON_PTE_FETCH,
+
+ /* Protection flag check failed */
+ IOMMU_FAULT_REASON_PERMISSION,
+
+ /* access flag check failed */
+ IOMMU_FAULT_REASON_ACCESS,
+
+ /* Output address of a translation stage caused Address Size fault */
+ IOMMU_FAULT_REASON_OOR_ADDRESS
+};
+
+/**
+ * struct iommu_fault - Generic fault data
+ *
+ * @type contains fault type
+ * @reason fault reasons if relevant outside IOMMU driver.
+ * IOMMU driver internal faults are not reported.
+ * @addr: tells the offending page address
+ * @fetch_addr: tells the address that caused an abort, if any
+ * @pasid: contains process address space ID, used in shared virtual memory
+ * @page_req_group_id: page request group index
+ * @last_req: last request in a page request group
+ * @pasid_valid: indicates if the PRQ has a valid PASID
+ * @prot: page access protection flag:
+ * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
+ */
+
+struct iommu_fault {
+ __u32 type; /* enum iommu_fault_type */
+ __u32 reason; /* enum iommu_fault_reason */
+ __u64 addr;
+ __u64 fetch_addr;
+ __u32 pasid;
+ __u32 page_req_group_id;
+ __u32 last_req;
+ __u32 pasid_valid;
+ __u32 prot;
+ __u32 access;
+};
#endif /* _UAPI_IOMMU_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-09-18 14:24 ` [RFC v2 14/20] iommu: introduce device fault data Eric Auger
@ 2018-09-20 22:06 ` Jacob Pan
2018-09-21 9:54 ` Auger Eric
2018-12-12 8:21 ` Auger Eric
0 siblings, 2 replies; 33+ messages in thread
From: Jacob Pan @ 2018-09-20 22:06 UTC (permalink / raw)
To: Eric Auger
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell, jacob.jun.pan
On Tue, 18 Sep 2018 16:24:51 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>
> Device faults detected by IOMMU can be reported outside IOMMU
> subsystem for further processing. This patch intends to provide
> a generic device fault data such that device drivers can be
> communicated with IOMMU faults without model specific knowledge.
>
> The proposed format is the result of discussion at:
> https://lkml.org/lkml/2017/11/10/291
> Part of the code is based on Jean-Philippe Brucker's patchset
> (https://patchwork.kernel.org/patch/9989315/).
>
> The assumption is that model specific IOMMU driver can filter and
> handle most of the internal faults if the cause is within IOMMU driver
> control. Therefore, the fault reasons can be reported are grouped
> and generalized based common specifications such as PCI ATS.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> [moved part of the iommu_fault_event struct in the uapi, enriched
> the fault reasons to be able to map unrecoverable SMMUv3 errors]
Sounds good to me.
There are also other "enrichment" we need to do to support mdev or
finer granularity fault reporting below physical device. e.g. PASID
level.
The current scheme works for PCIe physical device level, where each
device registers a single handler only once. When device fault is
detected by the IOMMU, it will find the matching handler and private
data to report back. However, for devices partitioned by PASID and
represented by mdev this may not work. Since IOMMU is not mdev aware
and only works at physical device level.
So I am thinking we should allow multiple registration of fault handler
with different data and ID. i.e.
int iommu_register_device_fault_handler(struct device *dev,
iommu_dev_fault_handler_t handler,
int id,
void *data)
where the new "id field" is
* @id: Identification of the handler private data, will be used by fault
* reporting code to match the handler data to be returned. For page
* request, this can be the PASID. ID must be unique per device, i.e.
* each ID can only be registered once per device.
* - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault reporting
* w/o ID. e.g. unrecoverable faults.
I am still testing, but just wanted to have feedback on this idea.
Thanks,
Jacob
> ---
> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
> include/uapi/linux/iommu.h | 83
> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
> insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 9bd3e63d562b..7529c14ff506 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -49,13 +49,17 @@ struct bus_type;
> struct device;
> struct iommu_domain;
> struct notifier_block;
> +struct iommu_fault_event;
>
> /* iommu fault flags */
> -#define IOMMU_FAULT_READ 0x0
> -#define IOMMU_FAULT_WRITE 0x1
> +#define IOMMU_FAULT_READ (1 << 0)
> +#define IOMMU_FAULT_WRITE (1 << 1)
> +#define IOMMU_FAULT_EXEC (1 << 2)
> +#define IOMMU_FAULT_PRIV (1 << 3)
>
> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
> struct device *, unsigned long, int, void *);
> +typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *,
> void *);
> struct iommu_domain_geometry {
> dma_addr_t aperture_start; /* First address that can be
> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
> struct device *dev;
> };
>
> +/**
> + * struct iommu_fault_event - Generic per device fault data
> + *
> + * - PCI and non-PCI devices
> + * - Recoverable faults (e.g. page request), information based on
> PCI ATS
> + * and PASID spec.
> + * - Un-recoverable faults of device interest
> + * - DMA remapping and IRQ remapping faults
> + *
> + * @fault: fault descriptor
> + * @device_private: if present, uniquely identify device-specific
> + * private data for an individual page request.
> + * @iommu_private: used by the IOMMU driver for storing
> fault-specific
> + * data. Users should not modify this field before
> + * sending the fault response.
> + */
> +struct iommu_fault_event {
> + struct iommu_fault fault;
> + u64 device_private;
> + u64 iommu_private;
> +};
> +
> +/**
> + * struct iommu_fault_param - per-device IOMMU fault data
> + * @dev_fault_handler: Callback function to handle IOMMU faults at
> device level
> + * @data: handler private data
> + *
> + */
> +struct iommu_fault_param {
> + iommu_dev_fault_handler_t handler;
> + void *data;
> +};
> +
> +/**
> + * struct iommu_param - collection of per-device IOMMU data
> + *
> + * @fault_param: IOMMU detected device fault reporting data
> + *
> + * TODO: migrate other per device data pointers under
> iommu_dev_data, e.g.
> + * struct iommu_group *iommu_group;
> + * struct iommu_fwspec *iommu_fwspec;
> + */
> +struct iommu_param {
> + struct iommu_fault_param *fault_param;
> +};
> +
> int iommu_device_register(struct iommu_device *iommu);
> void iommu_device_unregister(struct iommu_device *iommu);
> int iommu_device_sysfs_add(struct iommu_device *iommu,
> @@ -429,6 +479,7 @@ struct iommu_ops {};
> struct iommu_group {};
> struct iommu_fwspec {};
> struct iommu_device {};
> +struct iommu_fault_param {};
>
> static inline bool iommu_present(struct bus_type *bus)
> {
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> index 21adb2a964e5..a0fe5c2fb236 100644
> --- a/include/uapi/linux/iommu.h
> +++ b/include/uapi/linux/iommu.h
> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
> __u64 gpa;
> __u32 granule;
> };
> +
> +/* Generic fault types, can be expanded IRQ remapping fault */
> +enum iommu_fault_type {
> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
> + IOMMU_FAULT_PAGE_REQ, /* page request fault */
> +};
> +
> +enum iommu_fault_reason {
> + IOMMU_FAULT_REASON_UNKNOWN = 0,
> +
> + /* IOMMU internal error, no specific reason to report out */
> + IOMMU_FAULT_REASON_INTERNAL,
> +
> + /* Could not access the PASID table (fetch caused external
> abort) */
> + IOMMU_FAULT_REASON_PASID_FETCH,
> +
> + /* could not access the device context (fetch caused
> external abort) */
> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
> +
> + /* pasid entry is invalid or has configuration errors */
> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
> +
> + /* device context entry is invalid or has configuration
> errors */
> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
> + /*
> + * PASID is out of range (e.g. exceeds the maximum PASID
> + * supported by the IOMMU) or disabled.
> + */
> + IOMMU_FAULT_REASON_PASID_INVALID,
> +
> + /* source id is out of range */
> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
> +
> + /*
> + * An external abort occurred fetching (or updating) a
> translation
> + * table descriptor
> + */
> + IOMMU_FAULT_REASON_WALK_EABT,
> +
> + /*
> + * Could not access the page table entry (Bad address),
> + * actual translation fault
> + */
> + IOMMU_FAULT_REASON_PTE_FETCH,
> +
> + /* Protection flag check failed */
> + IOMMU_FAULT_REASON_PERMISSION,
> +
> + /* access flag check failed */
> + IOMMU_FAULT_REASON_ACCESS,
> +
> + /* Output address of a translation stage caused Address Size
> fault */
> + IOMMU_FAULT_REASON_OOR_ADDRESS
> +};
> +
> +/**
> + * struct iommu_fault - Generic fault data
> + *
> + * @type contains fault type
> + * @reason fault reasons if relevant outside IOMMU driver.
> + * IOMMU driver internal faults are not reported.
> + * @addr: tells the offending page address
> + * @fetch_addr: tells the address that caused an abort, if any
> + * @pasid: contains process address space ID, used in shared virtual
> memory
> + * @page_req_group_id: page request group index
> + * @last_req: last request in a page request group
> + * @pasid_valid: indicates if the PRQ has a valid PASID
> + * @prot: page access protection flag:
> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
> + */
> +
> +struct iommu_fault {
> + __u32 type; /* enum iommu_fault_type */
> + __u32 reason; /* enum iommu_fault_reason */
> + __u64 addr;
> + __u64 fetch_addr;
> + __u32 pasid;
> + __u32 page_req_group_id;
> + __u32 last_req;
> + __u32 pasid_valid;
> + __u32 prot;
> + __u32 access;
> +};
> #endif /* _UAPI_IOMMU_H */
>
[Jacob Pan]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-09-20 22:06 ` Jacob Pan
@ 2018-09-21 9:54 ` Auger Eric
2018-09-21 16:18 ` Jacob Pan
2018-12-12 8:21 ` Auger Eric
1 sibling, 1 reply; 33+ messages in thread
From: Auger Eric @ 2018-09-21 9:54 UTC (permalink / raw)
To: Jacob Pan
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell
Hi Jacob,
On 9/21/18 12:06 AM, Jacob Pan wrote:
> On Tue, 18 Sep 2018 16:24:51 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>
>> Device faults detected by IOMMU can be reported outside IOMMU
>> subsystem for further processing. This patch intends to provide
>> a generic device fault data such that device drivers can be
>> communicated with IOMMU faults without model specific knowledge.
>>
>> The proposed format is the result of discussion at:
>> https://lkml.org/lkml/2017/11/10/291
>> Part of the code is based on Jean-Philippe Brucker's patchset
>> (https://patchwork.kernel.org/patch/9989315/).
>>
>> The assumption is that model specific IOMMU driver can filter and
>> handle most of the internal faults if the cause is within IOMMU driver
>> control. Therefore, the fault reasons can be reported are grouped
>> and generalized based common specifications such as PCI ATS.
>>
>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> [moved part of the iommu_fault_event struct in the uapi, enriched
>> the fault reasons to be able to map unrecoverable SMMUv3 errors]
> Sounds good to me.
> There are also other "enrichment" we need to do to support mdev or
> finer granularity fault reporting below physical device. e.g. PASID
> level.
Actually I intended to send you an email about those iommu_fault_reason
enum value changes. To attach this discussion to your original series, I
will send a separate email in the "[PATCH v5 00/23] IOMMU and VT-d
driver support for Shared Virtual Address (SVA)" thread.
>
> The current scheme works for PCIe physical device level, where each
> device registers a single handler only once. When device fault is
> detected by the IOMMU, it will find the matching handler and private
> data to report back. However, for devices partitioned by PASID and
> represented by mdev this may not work. Since IOMMU is not mdev aware
> and only works at physical device level.
> So I am thinking we should allow multiple registration of fault handler
> with different data and ID. i.e.
>
> int iommu_register_device_fault_handler(struct device *dev,
> iommu_dev_fault_handler_t handler,
> int id,
> void *data)
>
> where the new "id field" is
> * @id: Identification of the handler private data, will be used by fault
> * reporting code to match the handler data to be returned. For page
> * request, this can be the PASID. ID must be unique per device, i.e.
> * each ID can only be registered once per device.
> * - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault reporting
> * w/o ID. e.g. unrecoverable faults.
I don't get this last sentence. Don't you need the feature also for
unrecoverable faults, ie. isn't it requested to report an unrecoverable
fault on a specific id?
Otherwise looks OK; but I still need to carefully review "[RFC PATCH v2
00/10] vfio/mdev: IOMMU aware mediated device".
Thanks
Eric
>
> I am still testing, but just wanted to have feedback on this idea.
>
> Thanks,
>
> Jacob
>
>
>> ---
>> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
>> include/uapi/linux/iommu.h | 83
>> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
>> insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 9bd3e63d562b..7529c14ff506 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -49,13 +49,17 @@ struct bus_type;
>> struct device;
>> struct iommu_domain;
>> struct notifier_block;
>> +struct iommu_fault_event;
>>
>> /* iommu fault flags */
>> -#define IOMMU_FAULT_READ 0x0
>> -#define IOMMU_FAULT_WRITE 0x1
>> +#define IOMMU_FAULT_READ (1 << 0)
>> +#define IOMMU_FAULT_WRITE (1 << 1)
>> +#define IOMMU_FAULT_EXEC (1 << 2)
>> +#define IOMMU_FAULT_PRIV (1 << 3)
>>
>> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
>> struct device *, unsigned long, int, void *);
>> +typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *,
>> void *);
>> struct iommu_domain_geometry {
>> dma_addr_t aperture_start; /* First address that can be
>> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
>> struct device *dev;
>> };
>>
>> +/**
>> + * struct iommu_fault_event - Generic per device fault data
>> + *
>> + * - PCI and non-PCI devices
>> + * - Recoverable faults (e.g. page request), information based on
>> PCI ATS
>> + * and PASID spec.
>> + * - Un-recoverable faults of device interest
>> + * - DMA remapping and IRQ remapping faults
>> + *
>> + * @fault: fault descriptor
>> + * @device_private: if present, uniquely identify device-specific
>> + * private data for an individual page request.
>> + * @iommu_private: used by the IOMMU driver for storing
>> fault-specific
>> + * data. Users should not modify this field before
>> + * sending the fault response.
>> + */
>> +struct iommu_fault_event {
>> + struct iommu_fault fault;
>> + u64 device_private;
>> + u64 iommu_private;
>> +};
>> +
>> +/**
>> + * struct iommu_fault_param - per-device IOMMU fault data
>> + * @dev_fault_handler: Callback function to handle IOMMU faults at
>> device level
>> + * @data: handler private data
>> + *
>> + */
>> +struct iommu_fault_param {
>> + iommu_dev_fault_handler_t handler;
>> + void *data;
>> +};
>> +
>> +/**
>> + * struct iommu_param - collection of per-device IOMMU data
>> + *
>> + * @fault_param: IOMMU detected device fault reporting data
>> + *
>> + * TODO: migrate other per device data pointers under
>> iommu_dev_data, e.g.
>> + * struct iommu_group *iommu_group;
>> + * struct iommu_fwspec *iommu_fwspec;
>> + */
>> +struct iommu_param {
>> + struct iommu_fault_param *fault_param;
>> +};
>> +
>> int iommu_device_register(struct iommu_device *iommu);
>> void iommu_device_unregister(struct iommu_device *iommu);
>> int iommu_device_sysfs_add(struct iommu_device *iommu,
>> @@ -429,6 +479,7 @@ struct iommu_ops {};
>> struct iommu_group {};
>> struct iommu_fwspec {};
>> struct iommu_device {};
>> +struct iommu_fault_param {};
>>
>> static inline bool iommu_present(struct bus_type *bus)
>> {
>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>> index 21adb2a964e5..a0fe5c2fb236 100644
>> --- a/include/uapi/linux/iommu.h
>> +++ b/include/uapi/linux/iommu.h
>> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
>> __u64 gpa;
>> __u32 granule;
>> };
>> +
>> +/* Generic fault types, can be expanded IRQ remapping fault */
>> +enum iommu_fault_type {
>> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
>> + IOMMU_FAULT_PAGE_REQ, /* page request fault */
>> +};
>> +
>> +enum iommu_fault_reason {
>> + IOMMU_FAULT_REASON_UNKNOWN = 0,
>> +
>> + /* IOMMU internal error, no specific reason to report out */
>> + IOMMU_FAULT_REASON_INTERNAL,
>> +
>> + /* Could not access the PASID table (fetch caused external
>> abort) */
>> + IOMMU_FAULT_REASON_PASID_FETCH,
>> +
>> + /* could not access the device context (fetch caused
>> external abort) */
>> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
>> +
>> + /* pasid entry is invalid or has configuration errors */
>> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
>> +
>> + /* device context entry is invalid or has configuration
>> errors */
>> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
>> + /*
>> + * PASID is out of range (e.g. exceeds the maximum PASID
>> + * supported by the IOMMU) or disabled.
>> + */
>> + IOMMU_FAULT_REASON_PASID_INVALID,
>> +
>> + /* source id is out of range */
>> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
>> +
>> + /*
>> + * An external abort occurred fetching (or updating) a
>> translation
>> + * table descriptor
>> + */
>> + IOMMU_FAULT_REASON_WALK_EABT,
>> +
>> + /*
>> + * Could not access the page table entry (Bad address),
>> + * actual translation fault
>> + */
>> + IOMMU_FAULT_REASON_PTE_FETCH,
>> +
>> + /* Protection flag check failed */
>> + IOMMU_FAULT_REASON_PERMISSION,
>> +
>> + /* access flag check failed */
>> + IOMMU_FAULT_REASON_ACCESS,
>> +
>> + /* Output address of a translation stage caused Address Size
>> fault */
>> + IOMMU_FAULT_REASON_OOR_ADDRESS
>> +};
>> +
>> +/**
>> + * struct iommu_fault - Generic fault data
>> + *
>> + * @type contains fault type
>> + * @reason fault reasons if relevant outside IOMMU driver.
>> + * IOMMU driver internal faults are not reported.
>> + * @addr: tells the offending page address
>> + * @fetch_addr: tells the address that caused an abort, if any
>> + * @pasid: contains process address space ID, used in shared virtual
>> memory
>> + * @page_req_group_id: page request group index
>> + * @last_req: last request in a page request group
>> + * @pasid_valid: indicates if the PRQ has a valid PASID
>> + * @prot: page access protection flag:
>> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
>> + */
>> +
>> +struct iommu_fault {
>> + __u32 type; /* enum iommu_fault_type */
>> + __u32 reason; /* enum iommu_fault_reason */
>> + __u64 addr;
>> + __u64 fetch_addr;
>> + __u32 pasid;
>> + __u32 page_req_group_id;
>> + __u32 last_req;
>> + __u32 pasid_valid;
>> + __u32 prot;
>> + __u32 access;
>> +};
>> #endif /* _UAPI_IOMMU_H */
>>
>
> [Jacob Pan]
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-09-21 9:54 ` Auger Eric
@ 2018-09-21 16:18 ` Jacob Pan
0 siblings, 0 replies; 33+ messages in thread
From: Jacob Pan @ 2018-09-21 16:18 UTC (permalink / raw)
To: Auger Eric
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell, jacob.jun.pan
On Fri, 21 Sep 2018 11:54:56 +0200
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Jacob,
>
> On 9/21/18 12:06 AM, Jacob Pan wrote:
> > On Tue, 18 Sep 2018 16:24:51 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >
> >> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >>
> >> Device faults detected by IOMMU can be reported outside IOMMU
> >> subsystem for further processing. This patch intends to provide
> >> a generic device fault data such that device drivers can be
> >> communicated with IOMMU faults without model specific knowledge.
> >>
> >> The proposed format is the result of discussion at:
> >> https://lkml.org/lkml/2017/11/10/291
> >> Part of the code is based on Jean-Philippe Brucker's patchset
> >> (https://patchwork.kernel.org/patch/9989315/).
> >>
> >> The assumption is that model specific IOMMU driver can filter and
> >> handle most of the internal faults if the cause is within IOMMU
> >> driver control. Therefore, the fault reasons can be reported are
> >> grouped and generalized based common specifications such as PCI
> >> ATS.
> >>
> >> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >> Signed-off-by: Jean-Philippe Brucker
> >> <jean-philippe.brucker@arm.com> Signed-off-by: Liu, Yi L
> >> <yi.l.liu@linux.intel.com> Signed-off-by: Ashok Raj
> >> <ashok.raj@intel.com> Signed-off-by: Eric Auger
> >> <eric.auger@redhat.com> [moved part of the iommu_fault_event
> >> struct in the uapi, enriched the fault reasons to be able to map
> >> unrecoverable SMMUv3 errors]
> > Sounds good to me.
> > There are also other "enrichment" we need to do to support mdev or
> > finer granularity fault reporting below physical device. e.g. PASID
> > level.
>
> Actually I intended to send you an email about those
> iommu_fault_reason enum value changes. To attach this discussion to
> your original series, I will send a separate email in the "[PATCH v5
> 00/23] IOMMU and VT-d driver support for Shared Virtual Address
> (SVA)" thread.
> >
> > The current scheme works for PCIe physical device level, where each
> > device registers a single handler only once. When device fault is
> > detected by the IOMMU, it will find the matching handler and private
> > data to report back. However, for devices partitioned by PASID and
> > represented by mdev this may not work. Since IOMMU is not mdev aware
> > and only works at physical device level.
> > So I am thinking we should allow multiple registration of fault
> > handler with different data and ID. i.e.
> >
> > int iommu_register_device_fault_handler(struct device *dev,
> > iommu_dev_fault_handler_t
> > handler, int id,
> > void *data)
> >
> > where the new "id field" is
> > * @id: Identification of the handler private data, will be used by
> > fault
> > * reporting code to match the handler data to be returned.
> > For page
> > * request, this can be the PASID. ID must be unique per
> > device, i.e.
> > * each ID can only be registered once per device.
> > * - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault
> > reporting
> > * w/o ID. e.g. unrecoverable faults.
> I don't get this last sentence. Don't you need the feature also for
> unrecoverable faults, ie. isn't it requested to report an
> unrecoverable fault on a specific id?
>
For unrecoverable faults which are not associated with a specific PASID,
we reserve a range of special IDs for them. Let me rewrite the
comments.
The usage would be:
For Handler registration by vfio or device driver
1. PRQ of PASID1
iommu_register_device_fault_handler(pdev, handler, pasid1, data1);
2. PRQ of PASID2
iommu_register_device_fault_handler(pdev, handler, pasid2, data2);
3. unrecoverable fault
iommu_register_device_fault_handler(pdev, handler,
IOMMU_DEV_FAULT_ID_UNRECOVERY, NULL);
For IOMMU driver reporting fault back to vfio or kernel driver:
1. PRQ of PASID1
iommu_report_device_fault(dev, evt1)
where evt1->data = data1, evt1->pasid = pasid1
2. PRQ of PASID2
iommu_report_device_fault(dev, evt2)
where evt2->data = data2, evt2->pasid = pasid2
3. unrecoverable fault
iommu_report_device_fault(dev, evt)
where evt2->data = NULL, evt->pasid = IOMMU_DEV_FAULT_ID_UNRECOVERY
where evt is of struct iommu_fault_event.
> Otherwise looks OK; but I still need to carefully review "[RFC PATCH
> v2 00/10] vfio/mdev: IOMMU aware mediated device".
>
> Thanks
>
> Eric
> >
> > I am still testing, but just wanted to have feedback on this idea.
> >
> > Thanks,
> >
> > Jacob
> >
> >
> >> ---
> >> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
> >> include/uapi/linux/iommu.h | 83
> >> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
> >> insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >> index 9bd3e63d562b..7529c14ff506 100644
> >> --- a/include/linux/iommu.h
> >> +++ b/include/linux/iommu.h
> >> @@ -49,13 +49,17 @@ struct bus_type;
> >> struct device;
> >> struct iommu_domain;
> >> struct notifier_block;
> >> +struct iommu_fault_event;
> >>
> >> /* iommu fault flags */
> >> -#define IOMMU_FAULT_READ 0x0
> >> -#define IOMMU_FAULT_WRITE 0x1
> >> +#define IOMMU_FAULT_READ (1 << 0)
> >> +#define IOMMU_FAULT_WRITE (1 << 1)
> >> +#define IOMMU_FAULT_EXEC (1 << 2)
> >> +#define IOMMU_FAULT_PRIV (1 << 3)
> >>
> >> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
> >> struct device *, unsigned long, int, void
> >> *); +typedef int (*iommu_dev_fault_handler_t)(struct
> >> iommu_fault_event *, void *);
> >> struct iommu_domain_geometry {
> >> dma_addr_t aperture_start; /* First address that can be
> >> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
> >> struct device *dev;
> >> };
> >>
> >> +/**
> >> + * struct iommu_fault_event - Generic per device fault data
> >> + *
> >> + * - PCI and non-PCI devices
> >> + * - Recoverable faults (e.g. page request), information based on
> >> PCI ATS
> >> + * and PASID spec.
> >> + * - Un-recoverable faults of device interest
> >> + * - DMA remapping and IRQ remapping faults
> >> + *
> >> + * @fault: fault descriptor
> >> + * @device_private: if present, uniquely identify device-specific
> >> + * private data for an individual page request.
> >> + * @iommu_private: used by the IOMMU driver for storing
> >> fault-specific
> >> + * data. Users should not modify this field before
> >> + * sending the fault response.
> >> + */
> >> +struct iommu_fault_event {
> >> + struct iommu_fault fault;
> >> + u64 device_private;
> >> + u64 iommu_private;
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_fault_param - per-device IOMMU fault data
> >> + * @dev_fault_handler: Callback function to handle IOMMU faults at
> >> device level
> >> + * @data: handler private data
> >> + *
> >> + */
> >> +struct iommu_fault_param {
> >> + iommu_dev_fault_handler_t handler;
> >> + void *data;
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_param - collection of per-device IOMMU data
> >> + *
> >> + * @fault_param: IOMMU detected device fault reporting data
> >> + *
> >> + * TODO: migrate other per device data pointers under
> >> iommu_dev_data, e.g.
> >> + * struct iommu_group *iommu_group;
> >> + * struct iommu_fwspec *iommu_fwspec;
> >> + */
> >> +struct iommu_param {
> >> + struct iommu_fault_param *fault_param;
> >> +};
> >> +
> >> int iommu_device_register(struct iommu_device *iommu);
> >> void iommu_device_unregister(struct iommu_device *iommu);
> >> int iommu_device_sysfs_add(struct iommu_device *iommu,
> >> @@ -429,6 +479,7 @@ struct iommu_ops {};
> >> struct iommu_group {};
> >> struct iommu_fwspec {};
> >> struct iommu_device {};
> >> +struct iommu_fault_param {};
> >>
> >> static inline bool iommu_present(struct bus_type *bus)
> >> {
> >> diff --git a/include/uapi/linux/iommu.h
> >> b/include/uapi/linux/iommu.h index 21adb2a964e5..a0fe5c2fb236
> >> 100644 --- a/include/uapi/linux/iommu.h
> >> +++ b/include/uapi/linux/iommu.h
> >> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
> >> __u64 gpa;
> >> __u32 granule;
> >> };
> >> +
> >> +/* Generic fault types, can be expanded IRQ remapping fault */
> >> +enum iommu_fault_type {
> >> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable
> >> fault */
> >> + IOMMU_FAULT_PAGE_REQ, /* page request
> >> fault */ +};
> >> +
> >> +enum iommu_fault_reason {
> >> + IOMMU_FAULT_REASON_UNKNOWN = 0,
> >> +
> >> + /* IOMMU internal error, no specific reason to report out
> >> */
> >> + IOMMU_FAULT_REASON_INTERNAL,
> >> +
> >> + /* Could not access the PASID table (fetch caused external
> >> abort) */
> >> + IOMMU_FAULT_REASON_PASID_FETCH,
> >> +
> >> + /* could not access the device context (fetch caused
> >> external abort) */
> >> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
> >> +
> >> + /* pasid entry is invalid or has configuration errors */
> >> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
> >> +
> >> + /* device context entry is invalid or has configuration
> >> errors */
> >> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
> >> + /*
> >> + * PASID is out of range (e.g. exceeds the maximum PASID
> >> + * supported by the IOMMU) or disabled.
> >> + */
> >> + IOMMU_FAULT_REASON_PASID_INVALID,
> >> +
> >> + /* source id is out of range */
> >> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
> >> +
> >> + /*
> >> + * An external abort occurred fetching (or updating) a
> >> translation
> >> + * table descriptor
> >> + */
> >> + IOMMU_FAULT_REASON_WALK_EABT,
> >> +
> >> + /*
> >> + * Could not access the page table entry (Bad address),
> >> + * actual translation fault
> >> + */
> >> + IOMMU_FAULT_REASON_PTE_FETCH,
> >> +
> >> + /* Protection flag check failed */
> >> + IOMMU_FAULT_REASON_PERMISSION,
> >> +
> >> + /* access flag check failed */
> >> + IOMMU_FAULT_REASON_ACCESS,
> >> +
> >> + /* Output address of a translation stage caused Address
> >> Size fault */
> >> + IOMMU_FAULT_REASON_OOR_ADDRESS
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_fault - Generic fault data
> >> + *
> >> + * @type contains fault type
> >> + * @reason fault reasons if relevant outside IOMMU driver.
> >> + * IOMMU driver internal faults are not reported.
> >> + * @addr: tells the offending page address
> >> + * @fetch_addr: tells the address that caused an abort, if any
> >> + * @pasid: contains process address space ID, used in shared
> >> virtual memory
> >> + * @page_req_group_id: page request group index
> >> + * @last_req: last request in a page request group
> >> + * @pasid_valid: indicates if the PRQ has a valid PASID
> >> + * @prot: page access protection flag:
> >> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
> >> + */
> >> +
> >> +struct iommu_fault {
> >> + __u32 type; /* enum iommu_fault_type */
> >> + __u32 reason; /* enum iommu_fault_reason */
> >> + __u64 addr;
> >> + __u64 fetch_addr;
> >> + __u32 pasid;
> >> + __u32 page_req_group_id;
> >> + __u32 last_req;
> >> + __u32 pasid_valid;
> >> + __u32 prot;
> >> + __u32 access;
> >> +};
> >> #endif /* _UAPI_IOMMU_H */
> >>
> >
> > [Jacob Pan]
> >
[Jacob Pan]
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-09-20 22:06 ` Jacob Pan
2018-09-21 9:54 ` Auger Eric
@ 2018-12-12 8:21 ` Auger Eric
2018-12-15 0:30 ` Jacob Pan
1 sibling, 1 reply; 33+ messages in thread
From: Auger Eric @ 2018-12-12 8:21 UTC (permalink / raw)
To: Jacob Pan
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell
Hi Jacob,
On 9/21/18 12:06 AM, Jacob Pan wrote:
> On Tue, 18 Sep 2018 16:24:51 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>
>> Device faults detected by IOMMU can be reported outside IOMMU
>> subsystem for further processing. This patch intends to provide
>> a generic device fault data such that device drivers can be
>> communicated with IOMMU faults without model specific knowledge.
>>
>> The proposed format is the result of discussion at:
>> https://lkml.org/lkml/2017/11/10/291
>> Part of the code is based on Jean-Philippe Brucker's patchset
>> (https://patchwork.kernel.org/patch/9989315/).
>>
>> The assumption is that model specific IOMMU driver can filter and
>> handle most of the internal faults if the cause is within IOMMU driver
>> control. Therefore, the fault reasons can be reported are grouped
>> and generalized based common specifications such as PCI ATS.
>>
>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> [moved part of the iommu_fault_event struct in the uapi, enriched
>> the fault reasons to be able to map unrecoverable SMMUv3 errors]
> Sounds good to me.
> There are also other "enrichment" we need to do to support mdev or
> finer granularity fault reporting below physical device. e.g. PASID
> level.
>
> The current scheme works for PCIe physical device level, where each
> device registers a single handler only once. When device fault is
> detected by the IOMMU, it will find the matching handler and private
> data to report back. However, for devices partitioned by PASID and
> represented by mdev this may not work. Since IOMMU is not mdev aware
> and only works at physical device level.
> So I am thinking we should allow multiple registration of fault handler
> with different data and ID. i.e.
>
> int iommu_register_device_fault_handler(struct device *dev,
> iommu_dev_fault_handler_t handler,
> int id,
> void *data)
>
> where the new "id field" is
> * @id: Identification of the handler private data, will be used by fault
> * reporting code to match the handler data to be returned. For page
> * request, this can be the PASID. ID must be unique per device, i.e.
> * each ID can only be registered once per device.
> * - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault reporting
> * w/o ID. e.g. unrecoverable faults.
>
> I am still testing, but just wanted to have feedback on this idea.
I am currently respinning this series. Do you have a respin for this
patch including iommu_register_device_fault_handler with the @id param
as you suggested above? Otherwise 2 solutions: I keep the code as is or
I do the modification myself implementing a list of fault_params?
Besides do you plans for "[PATCH v5 00/23] IOMMU and VT-d driver support
for Shared Virtual Address (SVA)" respin - hope I didn't miss anything? - ?
Thanks
Eric
>
> Thanks,
>
> Jacob
>
>
>> ---
>> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
>> include/uapi/linux/iommu.h | 83
>> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
>> insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 9bd3e63d562b..7529c14ff506 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -49,13 +49,17 @@ struct bus_type;
>> struct device;
>> struct iommu_domain;
>> struct notifier_block;
>> +struct iommu_fault_event;
>>
>> /* iommu fault flags */
>> -#define IOMMU_FAULT_READ 0x0
>> -#define IOMMU_FAULT_WRITE 0x1
>> +#define IOMMU_FAULT_READ (1 << 0)
>> +#define IOMMU_FAULT_WRITE (1 << 1)
>> +#define IOMMU_FAULT_EXEC (1 << 2)
>> +#define IOMMU_FAULT_PRIV (1 << 3)
>>
>> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
>> struct device *, unsigned long, int, void *);
>> +typedef int (*iommu_dev_fault_handler_t)(struct iommu_fault_event *,
>> void *);
>> struct iommu_domain_geometry {
>> dma_addr_t aperture_start; /* First address that can be
>> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
>> struct device *dev;
>> };
>>
>> +/**
>> + * struct iommu_fault_event - Generic per device fault data
>> + *
>> + * - PCI and non-PCI devices
>> + * - Recoverable faults (e.g. page request), information based on
>> PCI ATS
>> + * and PASID spec.
>> + * - Un-recoverable faults of device interest
>> + * - DMA remapping and IRQ remapping faults
>> + *
>> + * @fault: fault descriptor
>> + * @device_private: if present, uniquely identify device-specific
>> + * private data for an individual page request.
>> + * @iommu_private: used by the IOMMU driver for storing
>> fault-specific
>> + * data. Users should not modify this field before
>> + * sending the fault response.
>> + */
>> +struct iommu_fault_event {
>> + struct iommu_fault fault;
>> + u64 device_private;
>> + u64 iommu_private;
>> +};
>> +
>> +/**
>> + * struct iommu_fault_param - per-device IOMMU fault data
>> + * @dev_fault_handler: Callback function to handle IOMMU faults at
>> device level
>> + * @data: handler private data
>> + *
>> + */
>> +struct iommu_fault_param {
>> + iommu_dev_fault_handler_t handler;
>> + void *data;
>> +};
>> +
>> +/**
>> + * struct iommu_param - collection of per-device IOMMU data
>> + *
>> + * @fault_param: IOMMU detected device fault reporting data
>> + *
>> + * TODO: migrate other per device data pointers under
>> iommu_dev_data, e.g.
>> + * struct iommu_group *iommu_group;
>> + * struct iommu_fwspec *iommu_fwspec;
>> + */
>> +struct iommu_param {
>> + struct iommu_fault_param *fault_param;
>> +};
>> +
>> int iommu_device_register(struct iommu_device *iommu);
>> void iommu_device_unregister(struct iommu_device *iommu);
>> int iommu_device_sysfs_add(struct iommu_device *iommu,
>> @@ -429,6 +479,7 @@ struct iommu_ops {};
>> struct iommu_group {};
>> struct iommu_fwspec {};
>> struct iommu_device {};
>> +struct iommu_fault_param {};
>>
>> static inline bool iommu_present(struct bus_type *bus)
>> {
>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>> index 21adb2a964e5..a0fe5c2fb236 100644
>> --- a/include/uapi/linux/iommu.h
>> +++ b/include/uapi/linux/iommu.h
>> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
>> __u64 gpa;
>> __u32 granule;
>> };
>> +
>> +/* Generic fault types, can be expanded IRQ remapping fault */
>> +enum iommu_fault_type {
>> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable fault */
>> + IOMMU_FAULT_PAGE_REQ, /* page request fault */
>> +};
>> +
>> +enum iommu_fault_reason {
>> + IOMMU_FAULT_REASON_UNKNOWN = 0,
>> +
>> + /* IOMMU internal error, no specific reason to report out */
>> + IOMMU_FAULT_REASON_INTERNAL,
>> +
>> + /* Could not access the PASID table (fetch caused external
>> abort) */
>> + IOMMU_FAULT_REASON_PASID_FETCH,
>> +
>> + /* could not access the device context (fetch caused
>> external abort) */
>> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
>> +
>> + /* pasid entry is invalid or has configuration errors */
>> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
>> +
>> + /* device context entry is invalid or has configuration
>> errors */
>> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
>> + /*
>> + * PASID is out of range (e.g. exceeds the maximum PASID
>> + * supported by the IOMMU) or disabled.
>> + */
>> + IOMMU_FAULT_REASON_PASID_INVALID,
>> +
>> + /* source id is out of range */
>> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
>> +
>> + /*
>> + * An external abort occurred fetching (or updating) a
>> translation
>> + * table descriptor
>> + */
>> + IOMMU_FAULT_REASON_WALK_EABT,
>> +
>> + /*
>> + * Could not access the page table entry (Bad address),
>> + * actual translation fault
>> + */
>> + IOMMU_FAULT_REASON_PTE_FETCH,
>> +
>> + /* Protection flag check failed */
>> + IOMMU_FAULT_REASON_PERMISSION,
>> +
>> + /* access flag check failed */
>> + IOMMU_FAULT_REASON_ACCESS,
>> +
>> + /* Output address of a translation stage caused Address Size
>> fault */
>> + IOMMU_FAULT_REASON_OOR_ADDRESS
>> +};
>> +
>> +/**
>> + * struct iommu_fault - Generic fault data
>> + *
>> + * @type contains fault type
>> + * @reason fault reasons if relevant outside IOMMU driver.
>> + * IOMMU driver internal faults are not reported.
>> + * @addr: tells the offending page address
>> + * @fetch_addr: tells the address that caused an abort, if any
>> + * @pasid: contains process address space ID, used in shared virtual
>> memory
>> + * @page_req_group_id: page request group index
>> + * @last_req: last request in a page request group
>> + * @pasid_valid: indicates if the PRQ has a valid PASID
>> + * @prot: page access protection flag:
>> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
>> + */
>> +
>> +struct iommu_fault {
>> + __u32 type; /* enum iommu_fault_type */
>> + __u32 reason; /* enum iommu_fault_reason */
>> + __u64 addr;
>> + __u64 fetch_addr;
>> + __u32 pasid;
>> + __u32 page_req_group_id;
>> + __u32 last_req;
>> + __u32 pasid_valid;
>> + __u32 prot;
>> + __u32 access;
>> +};
>> #endif /* _UAPI_IOMMU_H */
>>
>
> [Jacob Pan]
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-12-12 8:21 ` Auger Eric
@ 2018-12-15 0:30 ` Jacob Pan
2018-12-17 9:04 ` Auger Eric
0 siblings, 1 reply; 33+ messages in thread
From: Jacob Pan @ 2018-12-15 0:30 UTC (permalink / raw)
To: Auger Eric
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell, jacob.jun.pan
On Wed, 12 Dec 2018 09:21:43 +0100
Auger Eric <eric.auger@redhat.com> wrote:
> Hi Jacob,
>
> On 9/21/18 12:06 AM, Jacob Pan wrote:
> > On Tue, 18 Sep 2018 16:24:51 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >
> >> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >>
> >> Device faults detected by IOMMU can be reported outside IOMMU
> >> subsystem for further processing. This patch intends to provide
> >> a generic device fault data such that device drivers can be
> >> communicated with IOMMU faults without model specific knowledge.
> >>
> >> The proposed format is the result of discussion at:
> >> https://lkml.org/lkml/2017/11/10/291
> >> Part of the code is based on Jean-Philippe Brucker's patchset
> >> (https://patchwork.kernel.org/patch/9989315/).
> >>
> >> The assumption is that model specific IOMMU driver can filter and
> >> handle most of the internal faults if the cause is within IOMMU
> >> driver control. Therefore, the fault reasons can be reported are
> >> grouped and generalized based common specifications such as PCI
> >> ATS.
> >>
> >> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >> Signed-off-by: Jean-Philippe Brucker
> >> <jean-philippe.brucker@arm.com> Signed-off-by: Liu, Yi L
> >> <yi.l.liu@linux.intel.com> Signed-off-by: Ashok Raj
> >> <ashok.raj@intel.com> Signed-off-by: Eric Auger
> >> <eric.auger@redhat.com> [moved part of the iommu_fault_event
> >> struct in the uapi, enriched the fault reasons to be able to map
> >> unrecoverable SMMUv3 errors]
> > Sounds good to me.
> > There are also other "enrichment" we need to do to support mdev or
> > finer granularity fault reporting below physical device. e.g. PASID
> > level.
> >
> > The current scheme works for PCIe physical device level, where each
> > device registers a single handler only once. When device fault is
> > detected by the IOMMU, it will find the matching handler and private
> > data to report back. However, for devices partitioned by PASID and
> > represented by mdev this may not work. Since IOMMU is not mdev aware
> > and only works at physical device level.
> > So I am thinking we should allow multiple registration of fault
> > handler with different data and ID. i.e.
> >
> > int iommu_register_device_fault_handler(struct device *dev,
> > iommu_dev_fault_handler_t
> > handler, int id,
> > void *data)
> >
> > where the new "id field" is
> > * @id: Identification of the handler private data, will be used by
> > fault
> > * reporting code to match the handler data to be returned.
> > For page
> > * request, this can be the PASID. ID must be unique per
> > device, i.e.
> > * each ID can only be registered once per device.
> > * - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault
> > reporting
> > * w/o ID. e.g. unrecoverable faults.
> >
> > I am still testing, but just wanted to have feedback on this idea.
>
> I am currently respinning this series. Do you have a respin for this
> patch including iommu_register_device_fault_handler with the @id param
> as you suggested above? Otherwise 2 solutions: I keep the code as is
> or I do the modification myself implementing a list of fault_params?
>
you can keep the code as is if it fits your current needs. Yi and I
have thought of some new cases for supporting mdev. We are thinking to
support many to many handler vs PASID relationship. i.e. allow
registration of many fault handlers per device, each associated with an
ID and data. The use case is that a physical device may register a
fault handler for its own PASID or non-PASID related faults. Such
physical device can also be partitioned into sub-device, e.g. mdev, but
fault handler registration is at physical device level in that IOMMU is
not mdev aware.
Anyway, still need some work to flush out the details.
> Besides do you plans for "[PATCH v5 00/23] IOMMU and VT-d driver
> support for Shared Virtual Address (SVA)" respin - hope I didn't miss
> anything? - ?
>
You did not miss anything. Yes, we are still working on some internal
integration issues. It should not affect the common interface much. Or
I can send out a common API spin first once we have the functionality
tested.
Thanks for checking.
> Thanks
>
> Eric
> >
> > Thanks,
> >
> > Jacob
> >
> >
> >> ---
> >> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
> >> include/uapi/linux/iommu.h | 83
> >> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
> >> insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> >> index 9bd3e63d562b..7529c14ff506 100644
> >> --- a/include/linux/iommu.h
> >> +++ b/include/linux/iommu.h
> >> @@ -49,13 +49,17 @@ struct bus_type;
> >> struct device;
> >> struct iommu_domain;
> >> struct notifier_block;
> >> +struct iommu_fault_event;
> >>
> >> /* iommu fault flags */
> >> -#define IOMMU_FAULT_READ 0x0
> >> -#define IOMMU_FAULT_WRITE 0x1
> >> +#define IOMMU_FAULT_READ (1 << 0)
> >> +#define IOMMU_FAULT_WRITE (1 << 1)
> >> +#define IOMMU_FAULT_EXEC (1 << 2)
> >> +#define IOMMU_FAULT_PRIV (1 << 3)
> >>
> >> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
> >> struct device *, unsigned long, int, void
> >> *); +typedef int (*iommu_dev_fault_handler_t)(struct
> >> iommu_fault_event *, void *);
> >> struct iommu_domain_geometry {
> >> dma_addr_t aperture_start; /* First address that can be
> >> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
> >> struct device *dev;
> >> };
> >>
> >> +/**
> >> + * struct iommu_fault_event - Generic per device fault data
> >> + *
> >> + * - PCI and non-PCI devices
> >> + * - Recoverable faults (e.g. page request), information based on
> >> PCI ATS
> >> + * and PASID spec.
> >> + * - Un-recoverable faults of device interest
> >> + * - DMA remapping and IRQ remapping faults
> >> + *
> >> + * @fault: fault descriptor
> >> + * @device_private: if present, uniquely identify device-specific
> >> + * private data for an individual page request.
> >> + * @iommu_private: used by the IOMMU driver for storing
> >> fault-specific
> >> + * data. Users should not modify this field before
> >> + * sending the fault response.
> >> + */
> >> +struct iommu_fault_event {
> >> + struct iommu_fault fault;
> >> + u64 device_private;
> >> + u64 iommu_private;
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_fault_param - per-device IOMMU fault data
> >> + * @dev_fault_handler: Callback function to handle IOMMU faults at
> >> device level
> >> + * @data: handler private data
> >> + *
> >> + */
> >> +struct iommu_fault_param {
> >> + iommu_dev_fault_handler_t handler;
> >> + void *data;
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_param - collection of per-device IOMMU data
> >> + *
> >> + * @fault_param: IOMMU detected device fault reporting data
> >> + *
> >> + * TODO: migrate other per device data pointers under
> >> iommu_dev_data, e.g.
> >> + * struct iommu_group *iommu_group;
> >> + * struct iommu_fwspec *iommu_fwspec;
> >> + */
> >> +struct iommu_param {
> >> + struct iommu_fault_param *fault_param;
> >> +};
> >> +
> >> int iommu_device_register(struct iommu_device *iommu);
> >> void iommu_device_unregister(struct iommu_device *iommu);
> >> int iommu_device_sysfs_add(struct iommu_device *iommu,
> >> @@ -429,6 +479,7 @@ struct iommu_ops {};
> >> struct iommu_group {};
> >> struct iommu_fwspec {};
> >> struct iommu_device {};
> >> +struct iommu_fault_param {};
> >>
> >> static inline bool iommu_present(struct bus_type *bus)
> >> {
> >> diff --git a/include/uapi/linux/iommu.h
> >> b/include/uapi/linux/iommu.h index 21adb2a964e5..a0fe5c2fb236
> >> 100644 --- a/include/uapi/linux/iommu.h
> >> +++ b/include/uapi/linux/iommu.h
> >> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
> >> __u64 gpa;
> >> __u32 granule;
> >> };
> >> +
> >> +/* Generic fault types, can be expanded IRQ remapping fault */
> >> +enum iommu_fault_type {
> >> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable
> >> fault */
> >> + IOMMU_FAULT_PAGE_REQ, /* page request
> >> fault */ +};
> >> +
> >> +enum iommu_fault_reason {
> >> + IOMMU_FAULT_REASON_UNKNOWN = 0,
> >> +
> >> + /* IOMMU internal error, no specific reason to report out
> >> */
> >> + IOMMU_FAULT_REASON_INTERNAL,
> >> +
> >> + /* Could not access the PASID table (fetch caused external
> >> abort) */
> >> + IOMMU_FAULT_REASON_PASID_FETCH,
> >> +
> >> + /* could not access the device context (fetch caused
> >> external abort) */
> >> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
> >> +
> >> + /* pasid entry is invalid or has configuration errors */
> >> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
> >> +
> >> + /* device context entry is invalid or has configuration
> >> errors */
> >> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
> >> + /*
> >> + * PASID is out of range (e.g. exceeds the maximum PASID
> >> + * supported by the IOMMU) or disabled.
> >> + */
> >> + IOMMU_FAULT_REASON_PASID_INVALID,
> >> +
> >> + /* source id is out of range */
> >> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
> >> +
> >> + /*
> >> + * An external abort occurred fetching (or updating) a
> >> translation
> >> + * table descriptor
> >> + */
> >> + IOMMU_FAULT_REASON_WALK_EABT,
> >> +
> >> + /*
> >> + * Could not access the page table entry (Bad address),
> >> + * actual translation fault
> >> + */
> >> + IOMMU_FAULT_REASON_PTE_FETCH,
> >> +
> >> + /* Protection flag check failed */
> >> + IOMMU_FAULT_REASON_PERMISSION,
> >> +
> >> + /* access flag check failed */
> >> + IOMMU_FAULT_REASON_ACCESS,
> >> +
> >> + /* Output address of a translation stage caused Address
> >> Size fault */
> >> + IOMMU_FAULT_REASON_OOR_ADDRESS
> >> +};
> >> +
> >> +/**
> >> + * struct iommu_fault - Generic fault data
> >> + *
> >> + * @type contains fault type
> >> + * @reason fault reasons if relevant outside IOMMU driver.
> >> + * IOMMU driver internal faults are not reported.
> >> + * @addr: tells the offending page address
> >> + * @fetch_addr: tells the address that caused an abort, if any
> >> + * @pasid: contains process address space ID, used in shared
> >> virtual memory
> >> + * @page_req_group_id: page request group index
> >> + * @last_req: last request in a page request group
> >> + * @pasid_valid: indicates if the PRQ has a valid PASID
> >> + * @prot: page access protection flag:
> >> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
> >> + */
> >> +
> >> +struct iommu_fault {
> >> + __u32 type; /* enum iommu_fault_type */
> >> + __u32 reason; /* enum iommu_fault_reason */
> >> + __u64 addr;
> >> + __u64 fetch_addr;
> >> + __u32 pasid;
> >> + __u32 page_req_group_id;
> >> + __u32 last_req;
> >> + __u32 pasid_valid;
> >> + __u32 prot;
> >> + __u32 access;
> >> +};
> >> #endif /* _UAPI_IOMMU_H */
> >>
> >
> > [Jacob Pan]
> >
[Jacob Pan]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [RFC v2 14/20] iommu: introduce device fault data
2018-12-15 0:30 ` Jacob Pan
@ 2018-12-17 9:04 ` Auger Eric
0 siblings, 0 replies; 33+ messages in thread
From: Auger Eric @ 2018-12-17 9:04 UTC (permalink / raw)
To: Jacob Pan
Cc: eric.auger.pro, iommu, linux-kernel, kvm, kvmarm, joro,
alex.williamson, yi.l.liu, jean-philippe.brucker, will.deacon,
robin.murphy, tianyu.lan, ashok.raj, marc.zyngier,
christoffer.dall, peter.maydell
Hi Jacob,
On 12/15/18 1:30 AM, Jacob Pan wrote:
> On Wed, 12 Dec 2018 09:21:43 +0100
> Auger Eric <eric.auger@redhat.com> wrote:
>
>> Hi Jacob,
>>
>> On 9/21/18 12:06 AM, Jacob Pan wrote:
>>> On Tue, 18 Sep 2018 16:24:51 +0200
>>> Eric Auger <eric.auger@redhat.com> wrote:
>>>
>>>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>>>
>>>> Device faults detected by IOMMU can be reported outside IOMMU
>>>> subsystem for further processing. This patch intends to provide
>>>> a generic device fault data such that device drivers can be
>>>> communicated with IOMMU faults without model specific knowledge.
>>>>
>>>> The proposed format is the result of discussion at:
>>>> https://lkml.org/lkml/2017/11/10/291
>>>> Part of the code is based on Jean-Philippe Brucker's patchset
>>>> (https://patchwork.kernel.org/patch/9989315/).
>>>>
>>>> The assumption is that model specific IOMMU driver can filter and
>>>> handle most of the internal faults if the cause is within IOMMU
>>>> driver control. Therefore, the fault reasons can be reported are
>>>> grouped and generalized based common specifications such as PCI
>>>> ATS.
>>>>
>>>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>>> Signed-off-by: Jean-Philippe Brucker
>>>> <jean-philippe.brucker@arm.com> Signed-off-by: Liu, Yi L
>>>> <yi.l.liu@linux.intel.com> Signed-off-by: Ashok Raj
>>>> <ashok.raj@intel.com> Signed-off-by: Eric Auger
>>>> <eric.auger@redhat.com> [moved part of the iommu_fault_event
>>>> struct in the uapi, enriched the fault reasons to be able to map
>>>> unrecoverable SMMUv3 errors]
>>> Sounds good to me.
>>> There are also other "enrichment" we need to do to support mdev or
>>> finer granularity fault reporting below physical device. e.g. PASID
>>> level.
>>>
>>> The current scheme works for PCIe physical device level, where each
>>> device registers a single handler only once. When device fault is
>>> detected by the IOMMU, it will find the matching handler and private
>>> data to report back. However, for devices partitioned by PASID and
>>> represented by mdev this may not work. Since IOMMU is not mdev aware
>>> and only works at physical device level.
>>> So I am thinking we should allow multiple registration of fault
>>> handler with different data and ID. i.e.
>>>
>>> int iommu_register_device_fault_handler(struct device *dev,
>>> iommu_dev_fault_handler_t
>>> handler, int id,
>>> void *data)
>>>
>>> where the new "id field" is
>>> * @id: Identification of the handler private data, will be used by
>>> fault
>>> * reporting code to match the handler data to be returned.
>>> For page
>>> * request, this can be the PASID. ID must be unique per
>>> device, i.e.
>>> * each ID can only be registered once per device.
>>> * - IOMMU_DEV_FAULT_ID_UNRECOVERY (~0U) is reserved for fault
>>> reporting
>>> * w/o ID. e.g. unrecoverable faults.
>>>
>>> I am still testing, but just wanted to have feedback on this idea.
>>
>> I am currently respinning this series. Do you have a respin for this
>> patch including iommu_register_device_fault_handler with the @id param
>> as you suggested above? Otherwise 2 solutions: I keep the code as is
>> or I do the modification myself implementing a list of fault_params?
>>
> you can keep the code as is if it fits your current needs. Yi and I
> have thought of some new cases for supporting mdev. We are thinking to
> support many to many handler vs PASID relationship. i.e. allow
> registration of many fault handlers per device, each associated with an
> ID and data. The use case is that a physical device may register a
> fault handler for its own PASID or non-PASID related faults. Such
> physical device can also be partitioned into sub-device, e.g. mdev, but
> fault handler registration is at physical device level in that IOMMU is
> not mdev aware.
> Anyway, still need some work to flush out the details.
>> Besides do you plans for "[PATCH v5 00/23] IOMMU and VT-d driver
>> support for Shared Virtual Address (SVA)" respin - hope I didn't miss
>> anything? - ?
Thank you for your reply. OK I will see how much time I can dedicate to
this API change. At the moment I am working on the vfio-pci side and
exposing a fault region as initiated by Yi.
>>
> You did not miss anything. Yes, we are still working on some internal
> integration issues. It should not affect the common interface much. Or
> I can send out a common API spin first once we have the functionality
> tested.
No worries. I can live without at the moment
Thanks
Eric
>
> Thanks for checking.
>
>> Thanks
>>
>> Eric
>>>
>>> Thanks,
>>>
>>> Jacob
>>>
>>>
>>>> ---
>>>> include/linux/iommu.h | 55 ++++++++++++++++++++++++-
>>>> include/uapi/linux/iommu.h | 83
>>>> ++++++++++++++++++++++++++++++++++++++ 2 files changed, 136
>>>> insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>>> index 9bd3e63d562b..7529c14ff506 100644
>>>> --- a/include/linux/iommu.h
>>>> +++ b/include/linux/iommu.h
>>>> @@ -49,13 +49,17 @@ struct bus_type;
>>>> struct device;
>>>> struct iommu_domain;
>>>> struct notifier_block;
>>>> +struct iommu_fault_event;
>>>>
>>>> /* iommu fault flags */
>>>> -#define IOMMU_FAULT_READ 0x0
>>>> -#define IOMMU_FAULT_WRITE 0x1
>>>> +#define IOMMU_FAULT_READ (1 << 0)
>>>> +#define IOMMU_FAULT_WRITE (1 << 1)
>>>> +#define IOMMU_FAULT_EXEC (1 << 2)
>>>> +#define IOMMU_FAULT_PRIV (1 << 3)
>>>>
>>>> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
>>>> struct device *, unsigned long, int, void
>>>> *); +typedef int (*iommu_dev_fault_handler_t)(struct
>>>> iommu_fault_event *, void *);
>>>> struct iommu_domain_geometry {
>>>> dma_addr_t aperture_start; /* First address that can be
>>>> mapped */ @@ -262,6 +266,52 @@ struct iommu_device {
>>>> struct device *dev;
>>>> };
>>>>
>>>> +/**
>>>> + * struct iommu_fault_event - Generic per device fault data
>>>> + *
>>>> + * - PCI and non-PCI devices
>>>> + * - Recoverable faults (e.g. page request), information based on
>>>> PCI ATS
>>>> + * and PASID spec.
>>>> + * - Un-recoverable faults of device interest
>>>> + * - DMA remapping and IRQ remapping faults
>>>> + *
>>>> + * @fault: fault descriptor
>>>> + * @device_private: if present, uniquely identify device-specific
>>>> + * private data for an individual page request.
>>>> + * @iommu_private: used by the IOMMU driver for storing
>>>> fault-specific
>>>> + * data. Users should not modify this field before
>>>> + * sending the fault response.
>>>> + */
>>>> +struct iommu_fault_event {
>>>> + struct iommu_fault fault;
>>>> + u64 device_private;
>>>> + u64 iommu_private;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct iommu_fault_param - per-device IOMMU fault data
>>>> + * @dev_fault_handler: Callback function to handle IOMMU faults at
>>>> device level
>>>> + * @data: handler private data
>>>> + *
>>>> + */
>>>> +struct iommu_fault_param {
>>>> + iommu_dev_fault_handler_t handler;
>>>> + void *data;
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct iommu_param - collection of per-device IOMMU data
>>>> + *
>>>> + * @fault_param: IOMMU detected device fault reporting data
>>>> + *
>>>> + * TODO: migrate other per device data pointers under
>>>> iommu_dev_data, e.g.
>>>> + * struct iommu_group *iommu_group;
>>>> + * struct iommu_fwspec *iommu_fwspec;
>>>> + */
>>>> +struct iommu_param {
>>>> + struct iommu_fault_param *fault_param;
>>>> +};
>>>> +
>>>> int iommu_device_register(struct iommu_device *iommu);
>>>> void iommu_device_unregister(struct iommu_device *iommu);
>>>> int iommu_device_sysfs_add(struct iommu_device *iommu,
>>>> @@ -429,6 +479,7 @@ struct iommu_ops {};
>>>> struct iommu_group {};
>>>> struct iommu_fwspec {};
>>>> struct iommu_device {};
>>>> +struct iommu_fault_param {};
>>>>
>>>> static inline bool iommu_present(struct bus_type *bus)
>>>> {
>>>> diff --git a/include/uapi/linux/iommu.h
>>>> b/include/uapi/linux/iommu.h index 21adb2a964e5..a0fe5c2fb236
>>>> 100644 --- a/include/uapi/linux/iommu.h
>>>> +++ b/include/uapi/linux/iommu.h
>>>> @@ -150,5 +150,88 @@ struct iommu_guest_msi_binding {
>>>> __u64 gpa;
>>>> __u32 granule;
>>>> };
>>>> +
>>>> +/* Generic fault types, can be expanded IRQ remapping fault */
>>>> +enum iommu_fault_type {
>>>> + IOMMU_FAULT_DMA_UNRECOV = 1, /* unrecoverable
>>>> fault */
>>>> + IOMMU_FAULT_PAGE_REQ, /* page request
>>>> fault */ +};
>>>> +
>>>> +enum iommu_fault_reason {
>>>> + IOMMU_FAULT_REASON_UNKNOWN = 0,
>>>> +
>>>> + /* IOMMU internal error, no specific reason to report out
>>>> */
>>>> + IOMMU_FAULT_REASON_INTERNAL,
>>>> +
>>>> + /* Could not access the PASID table (fetch caused external
>>>> abort) */
>>>> + IOMMU_FAULT_REASON_PASID_FETCH,
>>>> +
>>>> + /* could not access the device context (fetch caused
>>>> external abort) */
>>>> + IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH,
>>>> +
>>>> + /* pasid entry is invalid or has configuration errors */
>>>> + IOMMU_FAULT_REASON_BAD_PASID_ENTRY,
>>>> +
>>>> + /* device context entry is invalid or has configuration
>>>> errors */
>>>> + IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY,
>>>> + /*
>>>> + * PASID is out of range (e.g. exceeds the maximum PASID
>>>> + * supported by the IOMMU) or disabled.
>>>> + */
>>>> + IOMMU_FAULT_REASON_PASID_INVALID,
>>>> +
>>>> + /* source id is out of range */
>>>> + IOMMU_FAULT_REASON_SOURCEID_INVALID,
>>>> +
>>>> + /*
>>>> + * An external abort occurred fetching (or updating) a
>>>> translation
>>>> + * table descriptor
>>>> + */
>>>> + IOMMU_FAULT_REASON_WALK_EABT,
>>>> +
>>>> + /*
>>>> + * Could not access the page table entry (Bad address),
>>>> + * actual translation fault
>>>> + */
>>>> + IOMMU_FAULT_REASON_PTE_FETCH,
>>>> +
>>>> + /* Protection flag check failed */
>>>> + IOMMU_FAULT_REASON_PERMISSION,
>>>> +
>>>> + /* access flag check failed */
>>>> + IOMMU_FAULT_REASON_ACCESS,
>>>> +
>>>> + /* Output address of a translation stage caused Address
>>>> Size fault */
>>>> + IOMMU_FAULT_REASON_OOR_ADDRESS
>>>> +};
>>>> +
>>>> +/**
>>>> + * struct iommu_fault - Generic fault data
>>>> + *
>>>> + * @type contains fault type
>>>> + * @reason fault reasons if relevant outside IOMMU driver.
>>>> + * IOMMU driver internal faults are not reported.
>>>> + * @addr: tells the offending page address
>>>> + * @fetch_addr: tells the address that caused an abort, if any
>>>> + * @pasid: contains process address space ID, used in shared
>>>> virtual memory
>>>> + * @page_req_group_id: page request group index
>>>> + * @last_req: last request in a page request group
>>>> + * @pasid_valid: indicates if the PRQ has a valid PASID
>>>> + * @prot: page access protection flag:
>>>> + * IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
>>>> + */
>>>> +
>>>> +struct iommu_fault {
>>>> + __u32 type; /* enum iommu_fault_type */
>>>> + __u32 reason; /* enum iommu_fault_reason */
>>>> + __u64 addr;
>>>> + __u64 fetch_addr;
>>>> + __u32 pasid;
>>>> + __u32 page_req_group_id;
>>>> + __u32 last_req;
>>>> + __u32 pasid_valid;
>>>> + __u32 prot;
>>>> + __u32 access;
>>>> +};
>>>> #endif /* _UAPI_IOMMU_H */
>>>>
>>>
>>> [Jacob Pan]
>>>
>
> [Jacob Pan]
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [RFC v2 15/20] driver core: add per device iommu param
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (13 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 14/20] iommu: introduce device fault data Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 16/20] iommu: introduce device fault report API Eric Auger
` (4 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
DMA faults can be detected by IOMMU at device level. Adding a pointer
to struct device allows IOMMU subsystem to report relevant faults
back to the device driver for further handling.
For direct assigned device (or user space drivers), guest OS holds
responsibility to handle and respond per device IOMMU fault.
Therefore we need fault reporting mechanism to propagate faults beyond
IOMMU subsystem.
There are two other IOMMU data pointers under struct device today, here
we introduce iommu_param as a parent pointer such that all device IOMMU
data can be consolidated here. The idea was suggested here by Greg KH
and Joerg. The name iommu_param is chosen here since iommu_data has been used.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Link: https://lkml.org/lkml/2017/10/6/81
---
include/linux/device.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..2daf4a94bd31 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -42,6 +42,7 @@ struct iommu_ops;
struct iommu_group;
struct iommu_fwspec;
struct dev_pin_info;
+struct iommu_param;
struct bus_attribute {
struct attribute attr;
@@ -922,6 +923,7 @@ struct dev_links_info {
* device (i.e. the bus driver that discovered the device).
* @iommu_group: IOMMU group the device belongs to.
* @iommu_fwspec: IOMMU-specific properties supplied by firmware.
+ * @iommu_param: Per device generic IOMMU runtime data
*
* @offline_disabled: If set, the device is permanently online.
* @offline: Set after successful invocation of bus type's .offline().
@@ -1012,6 +1014,7 @@ struct device {
void (*release)(struct device *dev);
struct iommu_group *iommu_group;
struct iommu_fwspec *iommu_fwspec;
+ struct iommu_param *iommu_param;
bool offline_disabled:1;
bool offline:1;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 16/20] iommu: introduce device fault report API
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (14 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 15/20] driver core: add per device iommu param Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 17/20] vfio: VFIO_IOMMU_SET_FAULT_EVENTFD Eric Auger
` (3 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
From: Jacob Pan <jacob.jun.pan@linux.intel.com>
Traditionally, device specific faults are detected and handled within
their own device drivers. When IOMMU is enabled, faults such as DMA
related transactions are detected by IOMMU. There is no generic
reporting mechanism to report faults back to the in-kernel device
driver or the guest OS in case of assigned devices.
Faults detected by IOMMU is based on the transaction's source ID which
can be reported at per device basis, regardless of the device type is a
PCI device or not.
The fault types include recoverable (e.g. page request) and
unrecoverable faults(e.g. access error). In most cases, faults can be
handled by IOMMU drivers internally. The primary use cases are as
follows:
1. page request fault originated from an SVM capable device that is
assigned to guest via vIOMMU. In this case, the first level page tables
are owned by the guest. Page request must be propagated to the guest to
let guest OS fault in the pages then send page response. In this
mechanism, the direct receiver of IOMMU fault notification is VFIO,
which can relay notification events to QEMU or other user space
software.
2. faults need more subtle handling by device drivers. Other than
simply invoke reset function, there are needs to let device driver
handle the fault with a smaller impact.
This patchset is intended to create a generic fault report API such
that it can scale as follows:
- all IOMMU types
- PCI and non-PCI devices
- recoverable and unrecoverable faults
- VFIO and other other in kernel users
- DMA & IRQ remapping (TBD)
The original idea was brought up by David Woodhouse and discussions
summarized at https://lwn.net/Articles/608914/.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
[adapt to new iommu_fault fault field, test fault_param on
iommu_unregister_device_fault_handler]
---
drivers/iommu/iommu.c | 153 +++++++++++++++++++++++++++++++++++++++++-
include/linux/iommu.h | 33 ++++++++-
2 files changed, 184 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index b2b8f375b169..dd4f3677a9ac 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -618,6 +618,13 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
goto err_free_name;
}
+ dev->iommu_param = kzalloc(sizeof(*dev->iommu_param), GFP_KERNEL);
+ if (!dev->iommu_param) {
+ ret = -ENOMEM;
+ goto err_free_name;
+ }
+ mutex_init(&dev->iommu_param->lock);
+
kobject_get(group->devices_kobj);
dev->iommu_group = group;
@@ -648,6 +655,7 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
mutex_unlock(&group->mutex);
dev->iommu_group = NULL;
kobject_put(group->devices_kobj);
+ kfree(dev->iommu_param);
err_free_name:
kfree(device->name);
err_remove_link:
@@ -694,7 +702,7 @@ void iommu_group_remove_device(struct device *dev)
sysfs_remove_link(&dev->kobj, "iommu_group");
trace_remove_device_from_group(group->id, dev);
-
+ kfree(dev->iommu_param);
kfree(device->name);
kfree(device);
dev->iommu_group = NULL;
@@ -828,6 +836,149 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
}
EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
+/**
+ * iommu_register_device_fault_handler() - Register a device fault handler
+ * @dev: the device
+ * @handler: the fault handler
+ * @data: private data passed as argument to the handler
+ *
+ * When an IOMMU fault event is received, call this handler with the fault event
+ * and data as argument. The handler should return 0 on success. If the fault is
+ * recoverable (IOMMU_FAULT_PAGE_REQ), the handler can also complete
+ * the fault by calling iommu_page_response() with one of the following
+ * response code:
+ * - IOMMU_PAGE_RESP_SUCCESS: retry the translation
+ * - IOMMU_PAGE_RESP_INVALID: terminate the fault
+ * - IOMMU_PAGE_RESP_FAILURE: terminate the fault and stop reporting
+ * page faults if possible.
+ *
+ * Return 0 if the fault handler was installed successfully, or an error.
+ */
+int iommu_register_device_fault_handler(struct device *dev,
+ iommu_dev_fault_handler_t handler,
+ void *data)
+{
+ struct iommu_param *param = dev->iommu_param;
+ int ret = 0;
+
+ /*
+ * Device iommu_param should have been allocated when device is
+ * added to its iommu_group.
+ */
+ if (!param)
+ return -EINVAL;
+
+ mutex_lock(¶m->lock);
+ /* Only allow one fault handler registered for each device */
+ if (param->fault_param) {
+ ret = -EBUSY;
+ goto done_unlock;
+ }
+
+ get_device(dev);
+ param->fault_param =
+ kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
+ if (!param->fault_param) {
+ put_device(dev);
+ ret = -ENOMEM;
+ goto done_unlock;
+ }
+ mutex_init(¶m->fault_param->lock);
+ param->fault_param->handler = handler;
+ param->fault_param->data = data;
+ INIT_LIST_HEAD(¶m->fault_param->faults);
+
+done_unlock:
+ mutex_unlock(¶m->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
+
+/**
+ * iommu_unregister_device_fault_handler() - Unregister the device fault handler
+ * @dev: the device
+ *
+ * Remove the device fault handler installed with
+ * iommu_register_device_fault_handler().
+ *
+ * Return 0 on success, or an error.
+ */
+int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ struct iommu_param *param = dev->iommu_param;
+ int ret = 0;
+
+ if (!param)
+ return -EINVAL;
+
+ mutex_lock(¶m->lock);
+
+ if (!param->fault_param)
+ goto unlock;
+
+ /* we cannot unregister handler if there are pending faults */
+ if (!list_empty(¶m->fault_param->faults)) {
+ ret = -EBUSY;
+ goto unlock;
+ }
+
+ kfree(param->fault_param);
+ param->fault_param = NULL;
+ put_device(dev);
+unlock:
+ mutex_unlock(¶m->lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
+
+
+/**
+ * iommu_report_device_fault() - Report fault event to device
+ * @dev: the device
+ * @evt: fault event data
+ *
+ * Called by IOMMU model specific drivers when fault is detected, typically
+ * in a threaded IRQ handler.
+ *
+ * Return 0 on success, or an error.
+ */
+int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ int ret = 0;
+ struct iommu_fault_event *evt_pending;
+ struct iommu_fault_param *fparam;
+
+ /* iommu_param is allocated when device is added to group */
+ if (!dev->iommu_param | !evt)
+ return -EINVAL;
+ /* we only report device fault if there is a handler registered */
+ mutex_lock(&dev->iommu_param->lock);
+ if (!dev->iommu_param->fault_param ||
+ !dev->iommu_param->fault_param->handler) {
+ ret = -EINVAL;
+ goto done_unlock;
+ }
+ fparam = dev->iommu_param->fault_param;
+ if (evt->fault.type == IOMMU_FAULT_PAGE_REQ && evt->fault.last_req) {
+ evt_pending = kmemdup(evt, sizeof(struct iommu_fault_event),
+ GFP_KERNEL);
+ if (!evt_pending) {
+ ret = -ENOMEM;
+ goto done_unlock;
+ }
+ mutex_lock(&fparam->lock);
+ list_add_tail(&evt_pending->list, &fparam->faults);
+ mutex_unlock(&fparam->lock);
+ }
+ ret = fparam->handler(evt, fparam->data);
+done_unlock:
+ mutex_unlock(&dev->iommu_param->lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_report_device_fault);
+
/**
* iommu_group_id - Return ID for a group
* @group: the group to ID
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7529c14ff506..760752300763 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -276,6 +276,7 @@ struct iommu_device {
* - DMA remapping and IRQ remapping faults
*
* @fault: fault descriptor
+ * @list pending fault event list, used for tracking responses
* @device_private: if present, uniquely identify device-specific
* private data for an individual page request.
* @iommu_private: used by the IOMMU driver for storing fault-specific
@@ -283,6 +284,7 @@ struct iommu_device {
* sending the fault response.
*/
struct iommu_fault_event {
+ struct list_head list;
struct iommu_fault fault;
u64 device_private;
u64 iommu_private;
@@ -292,10 +294,13 @@ struct iommu_fault_event {
* struct iommu_fault_param - per-device IOMMU fault data
* @dev_fault_handler: Callback function to handle IOMMU faults at device level
* @data: handler private data
- *
+ * @faults: holds the pending faults which needs response, e.g. page response.
+ * @lock: protect pending PRQ event list
*/
struct iommu_fault_param {
iommu_dev_fault_handler_t handler;
+ struct list_head faults;
+ struct mutex lock;
void *data;
};
@@ -309,6 +314,7 @@ struct iommu_fault_param {
* struct iommu_fwspec *iommu_fwspec;
*/
struct iommu_param {
+ struct mutex lock;
struct iommu_fault_param *fault_param;
};
@@ -409,6 +415,14 @@ extern int iommu_group_register_notifier(struct iommu_group *group,
struct notifier_block *nb);
extern int iommu_group_unregister_notifier(struct iommu_group *group,
struct notifier_block *nb);
+extern int iommu_register_device_fault_handler(struct device *dev,
+ iommu_dev_fault_handler_t handler,
+ void *data);
+
+extern int iommu_unregister_device_fault_handler(struct device *dev);
+
+extern int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt);
+
extern int iommu_group_id(struct iommu_group *group);
extern struct iommu_group *iommu_group_get_for_dev(struct device *dev);
extern struct iommu_domain *iommu_group_default_domain(struct iommu_group *);
@@ -673,6 +687,23 @@ static inline int iommu_group_unregister_notifier(struct iommu_group *group,
return 0;
}
+static inline int iommu_register_device_fault_handler(struct device *dev,
+ iommu_dev_fault_handler_t handler,
+ void *data)
+{
+ return -ENODEV;
+}
+
+static inline int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ return 0;
+}
+
+static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ return -ENODEV;
+}
+
static inline int iommu_group_id(struct iommu_group *group)
{
return -ENODEV;
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 17/20] vfio: VFIO_IOMMU_SET_FAULT_EVENTFD
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (15 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 16/20] iommu: introduce device fault report API Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 18/20] vfio: VFIO_IOMMU_GET_FAULT_EVENTS Eric Auger
` (2 subsequent siblings)
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
VFIO_IOMMU_SET_FAULT_EVENTFD Allows to associate a fault
event handler to a container. This is useful when the
guest owns the first translation stage and the host uses
the second stage. As the guest translation is fully handled
by the physical IOMMU, if any fault happens, this latter needs
to be propagated to the guest.
The userspace passes an eventfd and a queue size. Each time
a fault occurs on physical IOMMU side, the fault is pushed to
a kfifo and the eventfd is signalled. The userspace handler,
awaken on eventfd signalling then reads the populated fifo
and can inject the faults to the virtual IOMMU.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
original API proposed by Lan in
[RFC PATCH 0/3] VFIO: Report IOMMU fault event to userspace
(https://www.spinics.net/lists/kvm/msg145280.html)
A current issue with the current implementation of
iommu_register_device_fault_handler() is that is expects
responses to void the list of pending faults. This looks
overkill for unrecoverable as responses are not mandated.
---
drivers/vfio/vfio_iommu_type1.c | 150 ++++++++++++++++++++++++++++++--
include/uapi/linux/vfio.h | 16 ++++
2 files changed, 157 insertions(+), 9 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 2ffd46c26dc3..e52cbeb479c3 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -41,6 +41,8 @@
#include <linux/notifier.h>
#include <linux/dma-iommu.h>
#include <linux/irqdomain.h>
+#include <linux/eventfd.h>
+#include <linux/kfifo.h>
#define DRIVER_VERSION "0.2"
#define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
@@ -66,6 +68,9 @@ struct vfio_iommu {
struct blocking_notifier_head notifier;
bool v2;
bool nesting;
+ struct eventfd_ctx *fault_ctx;
+ DECLARE_KFIFO_PTR(fault_fifo, struct iommu_fault);
+ struct mutex fault_lock;
};
struct vfio_domain {
@@ -114,6 +119,7 @@ struct vfio_regions {
(!list_empty(&iommu->domain_list))
static int put_pfn(unsigned long pfn, int prot);
+static int vfio_iommu_teardown_fault_eventfd(struct vfio_iommu *iommu);
/*
* This code handles mapping and unmapping of user data buffers
@@ -1527,15 +1533,26 @@ static void vfio_sanity_check_pfn_list(struct vfio_iommu *iommu)
WARN_ON(iommu->notifier.head);
}
+static inline int
+vfio_unregister_fault_handler_fn(struct device *dev, void *data)
+{
+ return iommu_unregister_device_fault_handler(dev);
+}
+
static void vfio_iommu_type1_detach_group(void *iommu_data,
struct iommu_group *iommu_group)
{
struct vfio_iommu *iommu = iommu_data;
struct vfio_domain *domain;
struct vfio_group *group;
+ int ret;
mutex_lock(&iommu->lock);
+ ret = iommu_group_for_each_dev(iommu_group, NULL,
+ vfio_unregister_fault_handler_fn);
+ WARN_ON(ret);
+
if (iommu->external_domain) {
group = find_iommu_group(iommu->external_domain, iommu_group);
if (group) {
@@ -1613,6 +1630,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
INIT_LIST_HEAD(&iommu->domain_list);
iommu->dma_list = RB_ROOT;
mutex_init(&iommu->lock);
+ mutex_init(&iommu->fault_lock);
BLOCKING_INIT_NOTIFIER_HEAD(&iommu->notifier);
return iommu;
@@ -1682,25 +1700,22 @@ static int vfio_cache_inv_fn(struct device *dev, void *data)
return iommu_cache_invalidate(d, dev, &ustruct->info);
}
-static int
-vfio_cache_invalidate(struct vfio_iommu *iommu,
- struct vfio_iommu_type1_cache_invalidate *ustruct)
+/* iommu->lock must be held */
+static int vfio_iommu_for_each_dev(struct vfio_iommu *iommu, void *data,
+ int (*fn)(struct device *, void *))
{
struct vfio_domain *d;
struct vfio_group *g;
int ret = 0;
- mutex_lock(&iommu->lock);
-
list_for_each_entry(d, &iommu->domain_list, next) {
list_for_each_entry(g, &d->group_list, next) {
- ret = iommu_group_for_each_dev(g->iommu_group, ustruct,
- vfio_cache_inv_fn);
+ ret = iommu_group_for_each_dev(g->iommu_group,
+ data, fn);
if (ret)
break;
}
}
- mutex_unlock(&iommu->lock);
return ret;
}
@@ -1740,6 +1755,102 @@ vfio_bind_pasid_table(struct vfio_iommu *iommu,
return ret;
}
+static int
+vfio_iommu_fault_handler(struct iommu_fault_event *event, void *data)
+{
+ struct vfio_iommu *iommu = (struct vfio_iommu *)data;
+ int ret;
+
+ mutex_lock(&iommu->fault_lock);
+ if (!iommu->fault_ctx)
+ goto out;
+ ret = kfifo_put(&iommu->fault_fifo, event->fault);
+ if (ret)
+ eventfd_signal(iommu->fault_ctx, 1);
+out:
+ mutex_unlock(&iommu->fault_lock);
+ return 0;
+}
+
+static inline int
+vfio_register_fault_handler_fn(struct device *dev, void *data)
+{
+ return iommu_register_device_fault_handler(dev,
+ vfio_iommu_fault_handler,
+ data);
+}
+
+static int vfio_iommu_teardown_fault_eventfd(struct vfio_iommu *iommu)
+{
+ int ret = -EINVAL;
+
+ mutex_lock(&iommu->fault_lock);
+
+ if (!iommu->fault_ctx)
+ goto out;
+
+ ret = vfio_iommu_for_each_dev(iommu, NULL,
+ vfio_unregister_fault_handler_fn);
+ WARN_ON(ret);
+
+ eventfd_ctx_put(iommu->fault_ctx);
+ iommu->fault_ctx = NULL;
+ kfifo_free(&iommu->fault_fifo);
+out:
+ mutex_unlock(&iommu->fault_lock);
+ return ret;
+}
+
+static int
+vfio_iommu_set_fault_eventfd(struct vfio_iommu *iommu,
+ struct vfio_iommu_type1_set_fault_eventfd *ustruct)
+{
+ struct eventfd_ctx *ctx;
+ int eventfd = ustruct->eventfd;
+ int qs = ustruct->config.qs;
+ int ret;
+
+ if (eventfd == -1)
+ return vfio_iommu_teardown_fault_eventfd(iommu);
+
+ if (qs <= 0)
+ return -EINVAL;
+
+ ctx = eventfd_ctx_fdget(eventfd);
+ if (IS_ERR(ctx))
+ return PTR_ERR(ctx);
+
+ mutex_lock(&iommu->fault_lock);
+ if (iommu->fault_ctx) {
+ eventfd_ctx_put(ctx);
+ return -EEXIST;
+ }
+
+ ret = kfifo_alloc(&iommu->fault_fifo,
+ (1 << qs) * sizeof(struct iommu_fault),
+ GFP_KERNEL);
+ if (ret)
+ goto out;
+
+ iommu->fault_ctx = ctx;
+ ret = vfio_iommu_for_each_dev(iommu, iommu,
+ vfio_register_fault_handler_fn);
+ if (ret)
+ goto unregister;
+
+ mutex_unlock(&iommu->fault_lock);
+ return 0;
+unregister:
+ vfio_iommu_for_each_dev(iommu, NULL,
+ vfio_unregister_fault_handler_fn);
+ iommu->fault_ctx = NULL;
+ kfifo_free(&iommu->fault_fifo);
+out:
+ eventfd_ctx_put(ctx);
+ mutex_unlock(&iommu->fault_lock);
+ return ret;
+}
+
static long vfio_iommu_type1_ioctl(void *iommu_data,
unsigned int cmd, unsigned long arg)
{
@@ -1825,6 +1936,7 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return vfio_bind_pasid_table(iommu, &ustruct);
} else if (cmd == VFIO_IOMMU_CACHE_INVALIDATE) {
struct vfio_iommu_type1_cache_invalidate ustruct;
+ int ret;
minsz = offsetofend(struct vfio_iommu_type1_cache_invalidate,
info);
@@ -1835,7 +1947,11 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
if (ustruct.argsz < minsz || ustruct.flags)
return -EINVAL;
- return vfio_cache_invalidate(iommu, &ustruct);
+ mutex_lock(&iommu->lock);
+ ret = vfio_iommu_for_each_dev(iommu,
+ &ustruct, vfio_cache_inv_fn);
+ mutex_unlock(&iommu->lock);
+ return ret;
} else if (cmd == VFIO_IOMMU_BIND_MSI) {
struct vfio_iommu_type1_bind_guest_msi ustruct;
@@ -1849,6 +1965,22 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return -EINVAL;
return vfio_iommu_bind_guest_msi(iommu, &ustruct);
+ } else if (cmd == VFIO_IOMMU_SET_FAULT_EVENTFD) {
+ struct vfio_iommu_type1_set_fault_eventfd ustruct;
+
+ minsz = offsetofend(struct vfio_iommu_type1_set_fault_eventfd,
+ config);
+
+ if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (ustruct.argsz < minsz || ustruct.flags)
+ return -EINVAL;
+
+ if (ustruct.eventfd < 1)
+ return -EINVAL;
+
+ return vfio_iommu_set_fault_eventfd(iommu, &ustruct);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 6fb5e944e73c..0d62598c818a 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -687,6 +687,22 @@ struct vfio_iommu_type1_bind_guest_msi {
};
#define VFIO_IOMMU_BIND_MSI _IO(VFIO_TYPE, VFIO_BASE + 24)
+struct vfio_iommu_type1_guest_fault_config {
+#define VFIO_IOMMU_FAULT_UNRECOVERABLE (1 << 0)
+ __u32 flags;
+ union {
+ __u8 qs; /* queue size, log2(entries) */
+ };
+};
+
+struct vfio_iommu_type1_set_fault_eventfd {
+ __u32 argsz;
+ __u32 flags;
+ __u32 eventfd;
+ struct vfio_iommu_type1_guest_fault_config config;
+};
+#define VFIO_IOMMU_SET_FAULT_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 25)
+
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 18/20] vfio: VFIO_IOMMU_GET_FAULT_EVENTS
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (16 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 17/20] vfio: VFIO_IOMMU_SET_FAULT_EVENTFD Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 19/20] vfio: Document nested stage control Eric Auger
2018-09-18 14:24 ` [RFC v2 20/20] iommu/smmuv3: Report non recoverable faults Eric Auger
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
Introduce an IOTCL that allows the userspace to consume fault
events that may have occurred. The userspace buffer gets filled
by pending faults, if any. The number of filled faults is
reported to the userspace. Read faults are removed from the kfifo.
the kernel does not expect any response from the userspace.
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
the fault_lock may not be needed as kfifo documentation says:
"Note that with only one concurrent reader and one concurrent
writer, you don't need extra locking to use these macro."
---
drivers/vfio/vfio_iommu_type1.c | 33 ++++++++++++++++++++++++++++++++-
include/uapi/linux/vfio.h | 10 ++++++++++
2 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index e52cbeb479c3..917bb8f9c9ae 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1769,7 +1769,7 @@ vfio_iommu_fault_handler(struct iommu_fault_event *event, void *data)
eventfd_signal(iommu->fault_ctx, 1);
out:
mutex_unlock(&iommu->fault_lock);
- return 0;
+ return ret;
}
static inline int
@@ -1981,6 +1981,37 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
return -EINVAL;
return vfio_iommu_set_fault_eventfd(iommu, &ustruct);
+ } else if (cmd == VFIO_IOMMU_GET_FAULT_EVENTS) {
+ struct vfio_iommu_type1_get_fault_events ustruct;
+ size_t buf_size, len, elem_size;
+ int copied, max_events, ret;
+
+ minsz = offsetofend(struct vfio_iommu_type1_get_fault_events,
+ reserved);
+
+ if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+ return -EFAULT;
+
+ if (ustruct.argsz < minsz || ustruct.flags)
+ return -EINVAL;
+
+ elem_size = sizeof(struct iommu_fault);
+ buf_size = ustruct.argsz - minsz;
+ max_events = buf_size / elem_size;
+ len = max_events * elem_size;
+
+ mutex_lock(&iommu->fault_lock);
+
+ ret = kfifo_to_user(&iommu->fault_fifo,
+ (void __user *)(arg + minsz), len, &copied);
+
+ mutex_unlock(&iommu->fault_lock);
+ if (ret)
+ return ret;
+
+ ustruct.count = copied / elem_size;
+
+ return copy_to_user((void __user *)arg, &ustruct, minsz);
}
return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 0d62598c818a..5b9165b7db8d 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -703,6 +703,16 @@ struct vfio_iommu_type1_set_fault_eventfd {
};
#define VFIO_IOMMU_SET_FAULT_EVENTFD _IO(VFIO_TYPE, VFIO_BASE + 25)
+struct vfio_iommu_type1_get_fault_events {
+ __u32 argsz;
+ __u32 flags;
+ __u32 count; /* number of faults returned */
+ __u32 reserved;
+ struct iommu_fault events[];
+};
+
+#define VFIO_IOMMU_GET_FAULT_EVENTS _IO(VFIO_TYPE, VFIO_BASE + 26)
+
/* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 19/20] vfio: Document nested stage control
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (17 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 18/20] vfio: VFIO_IOMMU_GET_FAULT_EVENTS Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
2018-09-18 14:24 ` [RFC v2 20/20] iommu/smmuv3: Report non recoverable faults Eric Auger
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
New iotcls were introduced to pass information about guest stage1
to the host through VFIO. Let's document the nested stage control.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
fault reporting is current missing to the picture
v1 -> v2:
- use the new ioctl names
- add doc related to fault handling
---
Documentation/vfio.txt | 60 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index f1a4d3c3ba0b..d4611759d306 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -239,6 +239,66 @@ group and can access them as follows::
/* Gratuitous device reset and go... */
ioctl(device, VFIO_DEVICE_RESET);
+IOMMU Dual Stage Control
+------------------------
+
+Some IOMMUs support 2 stages/levels of translation. "Stage" corresponds to
+the ARM terminology while "level" corresponds to Intel's VTD terminology. In
+the following text we use either without distinction.
+
+This is useful when the guest is exposed with a virtual IOMMU and some
+devices are assigned to the guest through VFIO. Then the guest OS can use
+stage 1 (IOVA -> GPA), while the hypervisor uses stage 2 for VM isolation
+(GPA -> HPA).
+
+The guest gets ownership of the stage 1 page tables and also owns stage 1
+configuration structures. The hypervisor owns the root configuration structure
+(for security reason), including stage 2 configuration. This works as long
+configuration structures and page table format are compatible between the
+virtual IOMMU and the physical IOMMU.
+
+Assuming the HW supports it, this nested mode is selected by choosing the
+VFIO_TYPE1_NESTING_IOMMU type through:
+
+ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU);
+
+This forces the hypervisor to use the stage 2, leaving stage 1 available for
+guest usage.
+
+Once groups are attached to the container, the guest stage 1 translation
+configuration data can be passed to VFIO by using
+
+ioctl(container, VFIO_IOMMU_BIND_PASID_TABLE, &pasid_table_info);
+
+This allows to combine guest stage 1 configuration structure along with
+hypervisor stage 2 configuration structure. stage 1 configuration structures
+are dependent on the IOMMU type.
+
+As the stage 1 translation is fully delegated to the HW, physical events that
+may occur (especially translation faults), need to be propagated up to
+the virtualizer and re-injected into the guest.
+
+By calling: ioctl(container, VFIO_IOMMU_SET_FAULT_EVENTFD &fault_config),
+the virtualizer can register an eventfd. This latter will be signalled
+whenever an event is detected at physical level. The fault handler,
+executed at userspace level has to call:
+ioctl(container, VFIO_IOMMU_GET_FAULT_EVENTS, &fault_info) to retrieve
+the pending fault events.
+
+When the guest invalidates stage 1 related caches, invalidations must be
+forwarded to the host through
+ioctl(container, VFIO_IOMMU_CACHE_INVALIDATE, &inv_data);
+Those invalidations can happen at various granularity levels, page, context, ...
+
+The ARM SMMU specification introduces another challenge: MSIs are translated by
+both the virtual SMMU and the physical SMMU. To build a nested mapping for the
+IOVA programmed into the assigned device, the guest needs to pass its IOVA/MSI
+doorbell GPA binding to the host. Then the hypervisor can build a nested stage 2
+binding eventually translating into the physical MSI doorbell.
+
+This is achieved by
+ioctl(container, VFIO_IOMMU_BIND_MSI, &guest_binding);
+
VFIO User API
-------------------------------------------------------------------------------
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread* [RFC v2 20/20] iommu/smmuv3: Report non recoverable faults
2018-09-18 14:24 [RFC v2 00/20] SMMUv3 Nested Stage Setup Eric Auger
` (18 preceding siblings ...)
2018-09-18 14:24 ` [RFC v2 19/20] vfio: Document nested stage control Eric Auger
@ 2018-09-18 14:24 ` Eric Auger
19 siblings, 0 replies; 33+ messages in thread
From: Eric Auger @ 2018-09-18 14:24 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, iommu, linux-kernel, kvm, kvmarm,
joro, alex.williamson, jacob.jun.pan, yi.l.liu,
jean-philippe.brucker, will.deacon, robin.murphy
Cc: tianyu.lan, ashok.raj, marc.zyngier, christoffer.dall, peter.maydell
When a stage 1 related fault event is read from the event queue,
let's propagate it to potential external fault listeners, ie. users
who registered a fault handler.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
drivers/iommu/arm-smmu-v3.c | 124 ++++++++++++++++++++++++++++++++----
1 file changed, 113 insertions(+), 11 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index d9d300ab62a5..948fc82fc4ce 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -178,6 +178,26 @@
#define ARM_SMMU_PRIQ_IRQ_CFG1 0xd8
#define ARM_SMMU_PRIQ_IRQ_CFG2 0xdc
+/* Events */
+#define ARM_SMMU_EVT_F_UUT 0x01
+#define ARM_SMMU_EVT_C_BAD_STREAMID 0x02
+#define ARM_SMMU_EVT_F_STE_FETCH 0x03
+#define ARM_SMMU_EVT_C_BAD_STE 0x04
+#define ARM_SMMU_EVT_F_BAD_ATS_TREQ 0x05
+#define ARM_SMMU_EVT_F_STREAM_DISABLED 0x06
+#define ARM_SMMU_EVT_F_TRANSL_FORBIDDEN 0x07
+#define ARM_SMMU_EVT_C_BAD_SUBSTREAMID 0x08
+#define ARM_SMMU_EVT_F_CD_FETCH 0x09
+#define ARM_SMMU_EVT_C_BAD_CD 0x0a
+#define ARM_SMMU_EVT_F_WALK_EABT 0x0b
+#define ARM_SMMU_EVT_F_TRANSLATION 0x10
+#define ARM_SMMU_EVT_F_ADDR_SIZE 0x11
+#define ARM_SMMU_EVT_F_ACCESS 0x12
+#define ARM_SMMU_EVT_F_PERMISSION 0x13
+#define ARM_SMMU_EVT_F_TLB_CONFLICT 0x20
+#define ARM_SMMU_EVT_F_CFG_CONFLICT 0x21
+#define ARM_SMMU_EVT_E_PAGE_REQUEST 0x24
+
/* Common MSI config fields */
#define MSI_CFG0_ADDR_MASK GENMASK_ULL(51, 2)
#define MSI_CFG2_SH GENMASK(5, 4)
@@ -343,6 +363,11 @@
#define EVTQ_MAX_SZ_SHIFT 7
#define EVTQ_0_ID GENMASK_ULL(7, 0)
+#define EVTQ_0_SUBSTREAMID GENMASK_ULL(31, 12)
+#define EVTQ_0_STREAMID GENMASK_ULL(63, 32)
+#define EVTQ_1_S2 GENMASK_ULL(39, 39)
+#define EVTQ_1_CLASS GENMASK_ULL(40, 41)
+#define EVTQ_3_FETCH_ADDR GENMASK_ULL(51, 3)
/* PRI queue */
#define PRIQ_ENT_DWORDS 2
@@ -1250,7 +1275,6 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
return 0;
}
-__maybe_unused
static struct arm_smmu_master_data *
arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
{
@@ -1276,24 +1300,102 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
return master;
}
+static void arm_smmu_report_event(struct arm_smmu_device *smmu, u64 *evt)
+{
+ u64 fetch_addr = FIELD_GET(EVTQ_3_FETCH_ADDR, evt[3]);
+ u32 sid = FIELD_GET(EVTQ_0_STREAMID, evt[0]);
+ bool s1 = !FIELD_GET(EVTQ_1_S2, evt[1]);
+ u8 type = FIELD_GET(EVTQ_0_ID, evt[0]);
+ struct arm_smmu_master_data *master;
+ struct iommu_fault_event event;
+ bool propagate = true;
+ u64 addr = evt[2];
+ int i;
+
+ master = arm_smmu_find_master(smmu, sid);
+ if (WARN_ON(!master))
+ return;
+
+ event.fault.type = IOMMU_FAULT_DMA_UNRECOV;
+
+ switch (type) {
+ case ARM_SMMU_EVT_C_BAD_STREAMID:
+ event.fault.reason = IOMMU_FAULT_REASON_SOURCEID_INVALID;
+ break;
+ case ARM_SMMU_EVT_F_STREAM_DISABLED:
+ case ARM_SMMU_EVT_C_BAD_SUBSTREAMID:
+ event.fault.reason = IOMMU_FAULT_REASON_PASID_INVALID;
+ break;
+ case ARM_SMMU_EVT_F_CD_FETCH:
+ event.fault.reason = IOMMU_FAULT_REASON_PASID_FETCH;
+ break;
+ case ARM_SMMU_EVT_F_WALK_EABT:
+ event.fault.reason = IOMMU_FAULT_REASON_WALK_EABT;
+ event.fault.addr = addr;
+ event.fault.fetch_addr = fetch_addr;
+ propagate = s1;
+ break;
+ case ARM_SMMU_EVT_F_TRANSLATION:
+ event.fault.reason = IOMMU_FAULT_REASON_PTE_FETCH;
+ event.fault.addr = addr;
+ event.fault.fetch_addr = fetch_addr;
+ propagate = s1;
+ break;
+ case ARM_SMMU_EVT_F_PERMISSION:
+ event.fault.reason = IOMMU_FAULT_REASON_PERMISSION;
+ event.fault.addr = addr;
+ propagate = s1;
+ break;
+ case ARM_SMMU_EVT_F_ACCESS:
+ event.fault.reason = IOMMU_FAULT_REASON_ACCESS;
+ event.fault.addr = addr;
+ propagate = s1;
+ break;
+ case ARM_SMMU_EVT_C_BAD_STE:
+ event.fault.reason = IOMMU_FAULT_REASON_BAD_DEVICE_CONTEXT_ENTRY;
+ break;
+ case ARM_SMMU_EVT_C_BAD_CD:
+ event.fault.reason = IOMMU_FAULT_REASON_BAD_PASID_ENTRY;
+ break;
+ case ARM_SMMU_EVT_F_ADDR_SIZE:
+ event.fault.reason = IOMMU_FAULT_REASON_OOR_ADDRESS;
+ propagate = s1;
+ break;
+ case ARM_SMMU_EVT_F_STE_FETCH:
+ event.fault.reason = IOMMU_FAULT_REASON_DEVICE_CONTEXT_FETCH;
+ event.fault.fetch_addr = fetch_addr;
+ break;
+ /* End of addition */
+ case ARM_SMMU_EVT_E_PAGE_REQUEST:
+ case ARM_SMMU_EVT_F_TLB_CONFLICT:
+ case ARM_SMMU_EVT_F_CFG_CONFLICT:
+ case ARM_SMMU_EVT_F_BAD_ATS_TREQ:
+ case ARM_SMMU_EVT_F_TRANSL_FORBIDDEN:
+ case ARM_SMMU_EVT_F_UUT:
+ default:
+ event.fault.reason = IOMMU_FAULT_REASON_UNKNOWN;
+ }
+ /* only propagate the error if it relates to stage 1 */
+ if (s1)
+ iommu_report_device_fault(master->dev, &event);
+
+ dev_info(smmu->dev, "event 0x%02x received:\n", type);
+ for (i = 0; i < EVTQ_ENT_DWORDS; ++i) {
+ dev_info(smmu->dev, "\t0x%016llx\n",
+ (unsigned long long)evt[i]);
+ }
+}
+
/* IRQ and event handlers */
static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
{
- int i;
struct arm_smmu_device *smmu = dev;
struct arm_smmu_queue *q = &smmu->evtq.q;
u64 evt[EVTQ_ENT_DWORDS];
do {
- while (!queue_remove_raw(q, evt)) {
- u8 id = FIELD_GET(EVTQ_0_ID, evt[0]);
-
- dev_info(smmu->dev, "event 0x%02x received:\n", id);
- for (i = 0; i < ARRAY_SIZE(evt); ++i)
- dev_info(smmu->dev, "\t0x%016llx\n",
- (unsigned long long)evt[i]);
-
- }
+ while (!queue_remove_raw(q, evt))
+ arm_smmu_report_event(smmu, evt);
/*
* Not much we can do on overflow, so scream and pretend we're
--
2.17.1
^ permalink raw reply related [flat|nested] 33+ messages in thread