* [PATCH v3 01/16] iommu: introduce bind_pasid_table API function
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:54 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:54 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Yi L, Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY,
Jean Delvare
Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
use in the guest:
https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
As part of the proposed architecture, when an SVM capable PCI
device is assigned to a guest, nested mode is turned on. Guest owns the
first level page tables (request with PASID) which performs GVA->GPA
translation. Second level page tables are owned by the host for GPA->HPA
translation for both request with and without PASID.
A new IOMMU driver interface is therefore needed to perform tasks as
follows:
* Enable nested translation and appropriate translation type
* Assign guest PASID table pointer (in GPA) and size to host IOMMU
This patch introduces new API functions to perform bind/unbind guest PASID
tables. Based on common data, model specific IOMMU drivers can be extended
to perform the specific steps for binding pasid table of assigned devices.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/iommu.c | 19 +++++++++++++++++++
include/linux/iommu.h | 24 ++++++++++++++++++++++++
include/uapi/linux/iommu.h | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+)
create mode 100644 include/uapi/linux/iommu.h
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3de5c0b..c7e0d64 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1322,6 +1322,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 device *dev,
+ struct pasid_table_config *pasidt_binfo)
+{
+ if (unlikely(!domain->ops->bind_pasid_table))
+ return -ENODEV;
+
+ return domain->ops->bind_pasid_table(domain, dev, pasidt_binfo);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
+
+void iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
+{
+ if (unlikely(!domain->ops->unbind_pasid_table))
+ return;
+
+ domain->ops->unbind_pasid_table(domain, dev);
+}
+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 41b8c57..0f6f6c5 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)
@@ -187,6 +188,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 pointer for guest SVM
+ * @unbind_pasid_table: unbind pasid table pointer and restore defaults
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -233,8 +236,14 @@ struct iommu_ops {
u32 (*domain_get_windows)(struct iommu_domain *domain);
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 device *dev,
+ struct pasid_table_config *pasidt_binfo);
+ void (*unbind_pasid_table)(struct iommu_domain *domain,
+ struct device *dev);
+
unsigned long pgsize_bitmap;
};
@@ -296,6 +305,10 @@ 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 device *dev, struct pasid_table_config *pasidt_binfo);
+extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
+ struct device *dev);
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);
@@ -696,6 +709,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 device *dev,
+ struct pasid_table_config *pasidt_binfo)
+{
+ return -EINVAL;
+}
+static inline
+void iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
+{
+}
+
#endif /* CONFIG_IOMMU_API */
#endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
new file mode 100644
index 0000000..651ad5d
--- /dev/null
+++ b/include/uapi/linux/iommu.h
@@ -0,0 +1,39 @@
+/*
+ * 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>
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU. This will
+ * enable guest managed first level page tables.
+ * @version: for future extensions and identification of the data format
+ * @bytes: size of this structure
+ * @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.
+ */
+struct pasid_table_config {
+ __u32 version;
+#define PASID_TABLE_CFG_VERSION 1
+ __u32 bytes;
+ __u64 base_ptr;
+ __u8 pasid_bits;
+ /* reserved for extension of vendor specific config */
+ union {
+ struct {
+ /* ARM specific fields */
+ bool pasid0_dma_no_pasid;
+ } arm;
+ };
+};
+
+#endif /* _UAPI_IOMMU_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 01/16] iommu: introduce bind_pasid_table API function
@ 2017-11-17 18:54 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:54 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan, Liu, Yi L
Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
use in the guest:
https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
As part of the proposed architecture, when an SVM capable PCI
device is assigned to a guest, nested mode is turned on. Guest owns the
first level page tables (request with PASID) which performs GVA->GPA
translation. Second level page tables are owned by the host for GPA->HPA
translation for both request with and without PASID.
A new IOMMU driver interface is therefore needed to perform tasks as
follows:
* Enable nested translation and appropriate translation type
* Assign guest PASID table pointer (in GPA) and size to host IOMMU
This patch introduces new API functions to perform bind/unbind guest PASID
tables. Based on common data, model specific IOMMU drivers can be extended
to perform the specific steps for binding pasid table of assigned devices.
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: Ashok Raj <ashok.raj@intel.com>
---
drivers/iommu/iommu.c | 19 +++++++++++++++++++
include/linux/iommu.h | 24 ++++++++++++++++++++++++
include/uapi/linux/iommu.h | 39 +++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+)
create mode 100644 include/uapi/linux/iommu.h
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3de5c0b..c7e0d64 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1322,6 +1322,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 device *dev,
+ struct pasid_table_config *pasidt_binfo)
+{
+ if (unlikely(!domain->ops->bind_pasid_table))
+ return -ENODEV;
+
+ return domain->ops->bind_pasid_table(domain, dev, pasidt_binfo);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_pasid_table);
+
+void iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
+{
+ if (unlikely(!domain->ops->unbind_pasid_table))
+ return;
+
+ domain->ops->unbind_pasid_table(domain, dev);
+}
+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 41b8c57..0f6f6c5 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)
@@ -187,6 +188,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 pointer for guest SVM
+ * @unbind_pasid_table: unbind pasid table pointer and restore defaults
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -233,8 +236,14 @@ struct iommu_ops {
u32 (*domain_get_windows)(struct iommu_domain *domain);
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 device *dev,
+ struct pasid_table_config *pasidt_binfo);
+ void (*unbind_pasid_table)(struct iommu_domain *domain,
+ struct device *dev);
+
unsigned long pgsize_bitmap;
};
@@ -296,6 +305,10 @@ 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 device *dev, struct pasid_table_config *pasidt_binfo);
+extern void iommu_unbind_pasid_table(struct iommu_domain *domain,
+ struct device *dev);
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);
@@ -696,6 +709,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 device *dev,
+ struct pasid_table_config *pasidt_binfo)
+{
+ return -EINVAL;
+}
+static inline
+void iommu_unbind_pasid_table(struct iommu_domain *domain, struct device *dev)
+{
+}
+
#endif /* CONFIG_IOMMU_API */
#endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
new file mode 100644
index 0000000..651ad5d
--- /dev/null
+++ b/include/uapi/linux/iommu.h
@@ -0,0 +1,39 @@
+/*
+ * 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>
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU. This will
+ * enable guest managed first level page tables.
+ * @version: for future extensions and identification of the data format
+ * @bytes: size of this structure
+ * @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.
+ */
+struct pasid_table_config {
+ __u32 version;
+#define PASID_TABLE_CFG_VERSION 1
+ __u32 bytes;
+ __u64 base_ptr;
+ __u8 pasid_bits;
+ /* reserved for extension of vendor specific config */
+ union {
+ struct {
+ /* ARM specific fields */
+ bool pasid0_dma_no_pasid;
+ } arm;
+ };
+};
+
+#endif /* _UAPI_IOMMU_H */
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* Re: [PATCH v3 01/16] iommu: introduce bind_pasid_table API function
2017-11-17 18:54 ` Jacob Pan
(?)
@ 2017-11-24 12:04 ` Jean-Philippe Brucker
2017-11-29 22:01 ` Jacob Pan
-1 siblings, 1 reply; 94+ messages in thread
From: Jean-Philippe Brucker @ 2017-11-24 12:04 UTC (permalink / raw)
To: Jacob Pan, iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Yi L, Liu@mail.linuxfoundation.org, Jean Delvare
On 17/11/17 18:54, Jacob Pan wrote:
> Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
> use in the guest:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
>
> As part of the proposed architecture, when an SVM capable PCI
> device is assigned to a guest, nested mode is turned on. Guest owns the
> first level page tables (request with PASID) which performs GVA->GPA
> translation. Second level page tables are owned by the host for GPA->HPA
> translation for both request with and without PASID.
>
> A new IOMMU driver interface is therefore needed to perform tasks as
> follows:
> * Enable nested translation and appropriate translation type
> * Assign guest PASID table pointer (in GPA) and size to host IOMMU
>
> This patch introduces new API functions to perform bind/unbind guest PASID
> tables. Based on common data, model specific IOMMU drivers can be extended
> to perform the specific steps for binding pasid table of assigned devices.
>
[...]
>
> #define IOMMU_READ (1 << 0)
> #define IOMMU_WRITE (1 << 1)
> @@ -187,6 +188,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 pointer for guest SVM
> + * @unbind_pasid_table: unbind pasid table pointer and restore defaults
I was wondering, are you planning on using the IOMMU_DOMAIN_NESTING
attribute? It differentiates a domain that supports
bind/unbind_pasid_table and map/unmap GPA (virt SVM), from the domain that
supports bind/unbind individual PASIDs and map/unmap IOVA (host SVM)?
Users can set this attribute by using the VFIO_TYPE1_NESTING_IOMMU type
instead of VFIO_TYPE1v2_IOMMU, which seems ideal for what we're trying to do.
[...]
> +/**
> + * PASID table data used to bind guest PASID table to the host IOMMU. This will
> + * enable guest managed first level page tables.
> + * @version: for future extensions and identification of the data format
> + * @bytes: size of this structure
> + * @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.
Why remove the @model parameter?
> + */
> +struct pasid_table_config {
> + __u32 version;
> +#define PASID_TABLE_CFG_VERSION 1
> + __u32 bytes;
> + __u64 base_ptr;
> + __u8 pasid_bits;
> + /* reserved for extension of vendor specific config */
> + union {
> + struct {
> + /* ARM specific fields */
> + bool pasid0_dma_no_pasid;
> + } arm;
I think @model is still required for sanity check, but could you remove
the whole union for the moment? Other parameters will be needed and I'm
still thinking about it, so I'll add the arm struct back in a future patch.
Thanks,
Jean
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 01/16] iommu: introduce bind_pasid_table API function
2017-11-24 12:04 ` Jean-Philippe Brucker
@ 2017-11-29 22:01 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-29 22:01 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson, Lan Tianyu, Yi L, Liu@mail.linuxfoundation.org,
Jean Delvare, jacob.jun.pan
On Fri, 24 Nov 2017 12:04:08 +0000
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
> On 17/11/17 18:54, Jacob Pan wrote:
> > Virtual IOMMU was proposed to support Shared Virtual Memory (SVM)
> > use in the guest:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg05311.html
> >
> > As part of the proposed architecture, when an SVM capable PCI
> > device is assigned to a guest, nested mode is turned on. Guest owns
> > the first level page tables (request with PASID) which performs
> > GVA->GPA translation. Second level page tables are owned by the
> > host for GPA->HPA translation for both request with and without
> > PASID.
> >
> > A new IOMMU driver interface is therefore needed to perform tasks as
> > follows:
> > * Enable nested translation and appropriate translation type
> > * Assign guest PASID table pointer (in GPA) and size to host IOMMU
> >
> > This patch introduces new API functions to perform bind/unbind
> > guest PASID tables. Based on common data, model specific IOMMU
> > drivers can be extended to perform the specific steps for binding
> > pasid table of assigned devices.
> [...]
> >
> > #define IOMMU_READ (1 << 0)
> > #define IOMMU_WRITE (1 << 1)
> > @@ -187,6 +188,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 pointer for guest SVM
> > + * @unbind_pasid_table: unbind pasid table pointer and restore
> > defaults
>
> I was wondering, are you planning on using the IOMMU_DOMAIN_NESTING
> attribute? It differentiates a domain that supports
> bind/unbind_pasid_table and map/unmap GPA (virt SVM), from the domain
> that supports bind/unbind individual PASIDs and map/unmap IOVA (host
> SVM)?
>
> Users can set this attribute by using the VFIO_TYPE1_NESTING_IOMMU
> type instead of VFIO_TYPE1v2_IOMMU, which seems ideal for what we're
> trying to do.
>
Hmmm, I am not sure. I think the bind/unbind is strictly a per device
attribute.
Yi, could you comment on the use via VFIO or QEMU?
> [...]
> > +/**
> > + * PASID table data used to bind guest PASID table to the host
> > IOMMU. This will
> > + * enable guest managed first level page tables.
> > + * @version: for future extensions and identification of the data
> > format
> > + * @bytes: size of this structure
> > + * @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.
>
> Why remove the @model parameter?
>
We removed it because we want the config data to be model agnostic. Any
sanity check should be done via some query interface, e.g. sysfs, to
ensure model matching. Once set up, there is no need to embed model info
in every bind operation.
> > + */
> > +struct pasid_table_config {
> > + __u32 version;
> > +#define PASID_TABLE_CFG_VERSION 1
> > + __u32 bytes;
> > + __u64 base_ptr;
> > + __u8 pasid_bits;
> > + /* reserved for extension of vendor specific config */
> > + union {
> > + struct {
> > + /* ARM specific fields */
> > + bool pasid0_dma_no_pasid;
> > + } arm;
>
> I think @model is still required for sanity check, but could you
> remove the whole union for the moment? Other parameters will be
> needed and I'm still thinking about it, so I'll add the arm struct
> back in a future patch.
>
sure, I will remove it for now.
Thanks,
Jacob
> Thanks,
> Jean
>
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 02/16] iommu/vt-d: add bind_pasid_table function
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Yi L, Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY,
Jean Delvare
Add Intel VT-d ops to the generic iommu_bind_pasid_table API
functions.
The primary use case is for direct assignment of SVM capable
device. Originated from emulated IOMMU in the guest, the request goes
through many layers (e.g. VFIO). Upon calling host IOMMU driver, caller
passes guest PASID table pointer (GPA) and size.
Device context table entry is modified by Intel IOMMU specific
bind_pasid_table function. This will turn on nesting mode and matching
translation type.
The unbind operation restores default context mapping.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/intel-iommu.c | 107 ++++++++++++++++++++++++++++++++++++++++++
include/linux/dma_remapping.h | 1 +
2 files changed, 108 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 2087cd8..3d1901d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5176,6 +5176,7 @@ static void intel_iommu_put_resv_regions(struct device *dev,
#ifdef CONFIG_INTEL_IOMMU_SVM
#define MAX_NR_PASID_BITS (20)
+#define MIN_NR_PASID_BITS (5)
static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
{
/*
@@ -5302,6 +5303,108 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
return iommu;
}
+
+static int intel_iommu_bind_pasid_table(struct iommu_domain *domain,
+ struct device *dev, struct pasid_table_config *pasidt_binfo)
+{
+ struct intel_iommu *iommu;
+ struct context_entry *context;
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct device_domain_info *info;
+ struct pci_dev *pdev;
+ u8 bus, devfn, host_table_pasid_bits;
+ u16 did, sid;
+ int ret = 0;
+ unsigned long flags;
+ u64 ctx_lo;
+
+ iommu = device_to_iommu(dev, &bus, &devfn);
+ if (!iommu)
+ return -ENODEV;
+ /* VT-d spec 9.4 says pasid table size is encoded as 2^(x+5) */
+ host_table_pasid_bits = intel_iommu_get_pts(iommu) + MIN_NR_PASID_BITS;
+ if (!pasidt_binfo || pasidt_binfo->pasid_bits > host_table_pasid_bits ||
+ pasidt_binfo->pasid_bits < MIN_NR_PASID_BITS) {
+ pr_err("Invalid gPASID bits %d, host range %d - %d\n",
+ pasidt_binfo->pasid_bits,
+ MIN_NR_PASID_BITS, host_table_pasid_bits);
+ return -ERANGE;
+ }
+
+ pdev = to_pci_dev(dev);
+ sid = PCI_DEVID(bus, devfn);
+ info = dev->archdata.iommu;
+
+ if (!info) {
+ dev_err(dev, "Invalid device domain info\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ if (!info->pasid_enabled) {
+ ret = pci_enable_pasid(pdev, info->pasid_supported & ~1);
+ if (ret) {
+ dev_err(dev, "Failed to enable PASID\n");
+ goto out;
+ }
+ }
+ if (!device_context_mapped(iommu, bus, devfn)) {
+ pr_warn("ctx not mapped for bus devfn %x:%x\n", bus, devfn);
+ ret = -EINVAL;
+ goto out;
+ }
+ spin_lock_irqsave(&iommu->lock, flags);
+ context = iommu_context_addr(iommu, bus, devfn, 0);
+ if (!context) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ /* Anticipate guest to use SVM and owns the first level, so we turn
+ * nested mode on
+ */
+ ctx_lo = context[0].lo;
+ ctx_lo |= CONTEXT_NESTE | CONTEXT_PRS | CONTEXT_PASIDE;
+ ctx_lo &= ~CONTEXT_TT_MASK;
+ ctx_lo |= CONTEXT_TT_DEV_IOTLB << 2;
+ context[0].lo = ctx_lo;
+
+ /* Assign guest PASID table pointer and size order */
+ ctx_lo = (pasidt_binfo->base_ptr & VTD_PAGE_MASK) |
+ (pasidt_binfo->pasid_bits - MIN_NR_PASID_BITS);
+ context[1].lo = ctx_lo;
+ /* make sure context entry is updated before flushing */
+ wmb();
+ did = dmar_domain->iommu_did[iommu->seq_id];
+ iommu->flush.flush_context(iommu, did,
+ (((u16)bus) << 8) | devfn,
+ DMA_CCMD_MASK_NOBIT,
+ DMA_CCMD_DEVICE_INVL);
+ iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
+
+out_unlock:
+ spin_unlock_irqrestore(&iommu->lock, flags);
+out:
+ return ret;
+}
+
+static void intel_iommu_unbind_pasid_table(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct intel_iommu *iommu;
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ u8 bus, devfn;
+
+ assert_spin_locked(&device_domain_lock);
+ iommu = device_to_iommu(dev, &bus, &devfn);
+ if (!iommu) {
+ dev_err(dev, "No IOMMU for device to unbind PASID table\n");
+ return;
+ }
+
+ domain_context_clear(iommu, dev);
+
+ domain_context_mapping_one(dmar_domain, iommu, bus, devfn);
+}
#endif /* CONFIG_INTEL_IOMMU_SVM */
const struct iommu_ops intel_iommu_ops = {
@@ -5310,6 +5413,10 @@ const struct iommu_ops intel_iommu_ops = {
.domain_free = intel_iommu_domain_free,
.attach_dev = intel_iommu_attach_device,
.detach_dev = intel_iommu_detach_device,
+#ifdef CONFIG_INTEL_IOMMU_SVM
+ .bind_pasid_table = intel_iommu_bind_pasid_table,
+ .unbind_pasid_table = intel_iommu_unbind_pasid_table,
+#endif
.map = intel_iommu_map,
.unmap = intel_iommu_unmap,
.map_sg = default_iommu_map_sg,
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 21b3e7d..db290b2 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -28,6 +28,7 @@
#define CONTEXT_DINVE (1ULL << 8)
#define CONTEXT_PRS (1ULL << 9)
+#define CONTEXT_NESTE (1ULL << 10)
#define CONTEXT_PASIDE (1ULL << 11)
struct intel_iommu;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 02/16] iommu/vt-d: add bind_pasid_table function
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan, Liu, Yi L
Add Intel VT-d ops to the generic iommu_bind_pasid_table API
functions.
The primary use case is for direct assignment of SVM capable
device. Originated from emulated IOMMU in the guest, the request goes
through many layers (e.g. VFIO). Upon calling host IOMMU driver, caller
passes guest PASID table pointer (GPA) and size.
Device context table entry is modified by Intel IOMMU specific
bind_pasid_table function. This will turn on nesting mode and matching
translation type.
The unbind operation restores default context mapping.
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: Ashok Raj <ashok.raj@intel.com>
---
drivers/iommu/intel-iommu.c | 107 ++++++++++++++++++++++++++++++++++++++++++
include/linux/dma_remapping.h | 1 +
2 files changed, 108 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 2087cd8..3d1901d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5176,6 +5176,7 @@ static void intel_iommu_put_resv_regions(struct device *dev,
#ifdef CONFIG_INTEL_IOMMU_SVM
#define MAX_NR_PASID_BITS (20)
+#define MIN_NR_PASID_BITS (5)
static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
{
/*
@@ -5302,6 +5303,108 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
return iommu;
}
+
+static int intel_iommu_bind_pasid_table(struct iommu_domain *domain,
+ struct device *dev, struct pasid_table_config *pasidt_binfo)
+{
+ struct intel_iommu *iommu;
+ struct context_entry *context;
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct device_domain_info *info;
+ struct pci_dev *pdev;
+ u8 bus, devfn, host_table_pasid_bits;
+ u16 did, sid;
+ int ret = 0;
+ unsigned long flags;
+ u64 ctx_lo;
+
+ iommu = device_to_iommu(dev, &bus, &devfn);
+ if (!iommu)
+ return -ENODEV;
+ /* VT-d spec 9.4 says pasid table size is encoded as 2^(x+5) */
+ host_table_pasid_bits = intel_iommu_get_pts(iommu) + MIN_NR_PASID_BITS;
+ if (!pasidt_binfo || pasidt_binfo->pasid_bits > host_table_pasid_bits ||
+ pasidt_binfo->pasid_bits < MIN_NR_PASID_BITS) {
+ pr_err("Invalid gPASID bits %d, host range %d - %d\n",
+ pasidt_binfo->pasid_bits,
+ MIN_NR_PASID_BITS, host_table_pasid_bits);
+ return -ERANGE;
+ }
+
+ pdev = to_pci_dev(dev);
+ sid = PCI_DEVID(bus, devfn);
+ info = dev->archdata.iommu;
+
+ if (!info) {
+ dev_err(dev, "Invalid device domain info\n");
+ ret = -EINVAL;
+ goto out;
+ }
+ if (!info->pasid_enabled) {
+ ret = pci_enable_pasid(pdev, info->pasid_supported & ~1);
+ if (ret) {
+ dev_err(dev, "Failed to enable PASID\n");
+ goto out;
+ }
+ }
+ if (!device_context_mapped(iommu, bus, devfn)) {
+ pr_warn("ctx not mapped for bus devfn %x:%x\n", bus, devfn);
+ ret = -EINVAL;
+ goto out;
+ }
+ spin_lock_irqsave(&iommu->lock, flags);
+ context = iommu_context_addr(iommu, bus, devfn, 0);
+ if (!context) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
+ /* Anticipate guest to use SVM and owns the first level, so we turn
+ * nested mode on
+ */
+ ctx_lo = context[0].lo;
+ ctx_lo |= CONTEXT_NESTE | CONTEXT_PRS | CONTEXT_PASIDE;
+ ctx_lo &= ~CONTEXT_TT_MASK;
+ ctx_lo |= CONTEXT_TT_DEV_IOTLB << 2;
+ context[0].lo = ctx_lo;
+
+ /* Assign guest PASID table pointer and size order */
+ ctx_lo = (pasidt_binfo->base_ptr & VTD_PAGE_MASK) |
+ (pasidt_binfo->pasid_bits - MIN_NR_PASID_BITS);
+ context[1].lo = ctx_lo;
+ /* make sure context entry is updated before flushing */
+ wmb();
+ did = dmar_domain->iommu_did[iommu->seq_id];
+ iommu->flush.flush_context(iommu, did,
+ (((u16)bus) << 8) | devfn,
+ DMA_CCMD_MASK_NOBIT,
+ DMA_CCMD_DEVICE_INVL);
+ iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
+
+out_unlock:
+ spin_unlock_irqrestore(&iommu->lock, flags);
+out:
+ return ret;
+}
+
+static void intel_iommu_unbind_pasid_table(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct intel_iommu *iommu;
+ struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ u8 bus, devfn;
+
+ assert_spin_locked(&device_domain_lock);
+ iommu = device_to_iommu(dev, &bus, &devfn);
+ if (!iommu) {
+ dev_err(dev, "No IOMMU for device to unbind PASID table\n");
+ return;
+ }
+
+ domain_context_clear(iommu, dev);
+
+ domain_context_mapping_one(dmar_domain, iommu, bus, devfn);
+}
#endif /* CONFIG_INTEL_IOMMU_SVM */
const struct iommu_ops intel_iommu_ops = {
@@ -5310,6 +5413,10 @@ const struct iommu_ops intel_iommu_ops = {
.domain_free = intel_iommu_domain_free,
.attach_dev = intel_iommu_attach_device,
.detach_dev = intel_iommu_detach_device,
+#ifdef CONFIG_INTEL_IOMMU_SVM
+ .bind_pasid_table = intel_iommu_bind_pasid_table,
+ .unbind_pasid_table = intel_iommu_unbind_pasid_table,
+#endif
.map = intel_iommu_map,
.unmap = intel_iommu_unmap,
.map_sg = default_iommu_map_sg,
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 21b3e7d..db290b2 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -28,6 +28,7 @@
#define CONTEXT_DINVE (1ULL << 8)
#define CONTEXT_PRS (1ULL << 9)
+#define CONTEXT_NESTE (1ULL << 10)
#define CONTEXT_PASIDE (1ULL << 11)
struct intel_iommu;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread
* [PATCH v3 04/16] iommu/vt-d: move device_domain_info to header
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
Allow both intel-iommu.c and dmar.c to access device_domain_info.
Prepare for additional per device arch data used in TLB flush function
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/intel-iommu.c | 18 ------------------
include/linux/intel-iommu.h | 19 +++++++++++++++++++
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3d1901d..399b504 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -391,24 +391,6 @@ struct dmar_domain {
iommu core */
};
-/* PCI domain-device relationship */
-struct device_domain_info {
- struct list_head link; /* link to domain siblings */
- struct list_head global; /* link to global list */
- u8 bus; /* PCI bus number */
- u8 devfn; /* PCI devfn number */
- u8 pasid_supported:3;
- u8 pasid_enabled:1;
- u8 pri_supported:1;
- u8 pri_enabled:1;
- u8 ats_supported:1;
- u8 ats_enabled:1;
- u8 ats_qdep;
- struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
- struct intel_iommu *iommu; /* IOMMU used by this device */
- struct dmar_domain *domain; /* pointer to domain */
-};
-
struct dmar_rmrr_unit {
struct list_head list; /* list of rmrr units */
struct acpi_dmar_header *hdr; /* ACPI header */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 77ea056..8d38e24 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -458,6 +458,25 @@ struct intel_iommu {
u32 flags; /* Software defined flags */
};
+/* PCI domain-device relationship */
+struct device_domain_info {
+ struct list_head link; /* link to domain siblings */
+ struct list_head global; /* link to global list */
+ u8 bus; /* PCI bus number */
+ u8 devfn; /* PCI devfn number */
+ u8 pasid_supported:3;
+ u8 pasid_enabled:1;
+ u8 pri_supported:1;
+ u8 pri_enabled:1;
+ u8 ats_supported:1;
+ u8 ats_enabled:1;
+ u8 ats_qdep;
+ u64 fault_mask; /* selected IOMMU faults to be reported */
+ struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
+ struct intel_iommu *iommu; /* IOMMU used by this device */
+ struct dmar_domain *domain; /* pointer to domain */
+};
+
static inline void __iommu_flush_cache(
struct intel_iommu *iommu, void *addr, int size)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 04/16] iommu/vt-d: move device_domain_info to header
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
Allow both intel-iommu.c and dmar.c to access device_domain_info.
Prepare for additional per device arch data used in TLB flush function
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/intel-iommu.c | 18 ------------------
include/linux/intel-iommu.h | 19 +++++++++++++++++++
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 3d1901d..399b504 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -391,24 +391,6 @@ struct dmar_domain {
iommu core */
};
-/* PCI domain-device relationship */
-struct device_domain_info {
- struct list_head link; /* link to domain siblings */
- struct list_head global; /* link to global list */
- u8 bus; /* PCI bus number */
- u8 devfn; /* PCI devfn number */
- u8 pasid_supported:3;
- u8 pasid_enabled:1;
- u8 pri_supported:1;
- u8 pri_enabled:1;
- u8 ats_supported:1;
- u8 ats_enabled:1;
- u8 ats_qdep;
- struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
- struct intel_iommu *iommu; /* IOMMU used by this device */
- struct dmar_domain *domain; /* pointer to domain */
-};
-
struct dmar_rmrr_unit {
struct list_head list; /* list of rmrr units */
struct acpi_dmar_header *hdr; /* ACPI header */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 77ea056..8d38e24 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -458,6 +458,25 @@ struct intel_iommu {
u32 flags; /* Software defined flags */
};
+/* PCI domain-device relationship */
+struct device_domain_info {
+ struct list_head link; /* link to domain siblings */
+ struct list_head global; /* link to global list */
+ u8 bus; /* PCI bus number */
+ u8 devfn; /* PCI devfn number */
+ u8 pasid_supported:3;
+ u8 pasid_enabled:1;
+ u8 pri_supported:1;
+ u8 pri_enabled:1;
+ u8 ats_supported:1;
+ u8 ats_enabled:1;
+ u8 ats_qdep;
+ u64 fault_mask; /* selected IOMMU faults to be reported */
+ struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
+ struct intel_iommu *iommu; /* IOMMU used by this device */
+ struct dmar_domain *domain; /* pointer to domain */
+};
+
static inline void __iommu_flush_cache(
struct intel_iommu *iommu, void *addr, int size)
{
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread
* [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Yi L, Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY,
Jean Delvare
With shared virtual memory vitualization, extended IOTLB invalidation
may be passed down from outside IOMMU subsystems. This patch adds
invalidation functions that can be used for each IOTLB types.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/dmar.c | 54 ++++++++++++++++++++++++++++++++++++++++++---
drivers/iommu/intel-iommu.c | 3 ++-
include/linux/intel-iommu.h | 10 +++++++--
3 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 57c920c..f69f6ee 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1336,11 +1336,25 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
qi_submit_sync(&desc, iommu);
}
-void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask)
+void qi_flush_eiotlb(struct intel_iommu *iommu, u16 did, u64 addr, u32 pasid,
+ unsigned int size_order, u64 granu, bool global)
{
struct qi_desc desc;
+ desc.low = QI_EIOTLB_PASID(pasid) | QI_EIOTLB_DID(did) |
+ QI_EIOTLB_GRAN(granu) | QI_EIOTLB_TYPE;
+ desc.high = QI_EIOTLB_ADDR(addr) | QI_EIOTLB_GL(global) |
+ QI_EIOTLB_IH(0) | QI_EIOTLB_AM(size_order);
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask)
+{
+ struct qi_desc desc;
+
+ pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr %llx, mask %d\n",
+ __func__, sid, pfsid, qdep, addr, mask);
if (mask) {
BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
@@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
qdep = 0;
desc.low = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
- QI_DIOTLB_TYPE;
+ QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
+
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_dev_eiotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep, u64 addr, unsigned size, u64 granu)
+{
+ struct qi_desc desc;
+
+ desc.low = QI_DEV_EIOTLB_PASID(pasid) | QI_DEV_EIOTLB_SID(sid) |
+ QI_DEV_EIOTLB_QDEP(qdep) | QI_DEIOTLB_TYPE |
+ QI_DEV_EIOTLB_PFSID(pfsid);
+ desc.high |= QI_DEV_EIOTLB_GLOB(granu);
+
+ /* If S bit is 0, we only flush a single page. If S bit is set,
+ * The least significant zero bit indicates the size. VT-d spec
+ * 6.5.2.6
+ */
+ if (!size)
+ desc.high = QI_DEV_EIOTLB_ADDR(addr) & ~QI_DEV_EIOTLB_SIZE;
+ else {
+ unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size);
+
+ desc.high = QI_DEV_EIOTLB_ADDR(addr & ~mask) | QI_DEV_EIOTLB_SIZE;
+ }
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_pasid(struct intel_iommu *iommu, u16 did, u64 granu, int pasid)
+{
+ struct qi_desc desc;
+
+ desc.high = 0;
+ desc.low = QI_PC_TYPE | QI_PC_DID(did) | QI_PC_GRAN(granu) | QI_PC_PASID(pasid);
qi_submit_sync(&desc, iommu);
}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 399b504..556bdd2 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1524,7 +1524,8 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
- qi_flush_dev_iotlb(info->iommu, sid, qdep, addr, mask);
+ qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+ qdep, addr, mask);
}
spin_unlock_irqrestore(&device_domain_lock, flags);
}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8d38e24..3c83f7e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -305,6 +305,7 @@ enum {
#define QI_DEV_EIOTLB_PASID(p) (((u64)p) << 32)
#define QI_DEV_EIOTLB_SID(sid) ((u64)((sid) & 0xffff) << 16)
#define QI_DEV_EIOTLB_QDEP(qd) ((u64)((qd) & 0x1f) << 4)
+#define QI_DEV_EIOTLB_PFSID(pfsid) (((u64)(pfsid & 0xf) << 12) | ((u64)(pfsid & 0xff0) << 48))
#define QI_DEV_EIOTLB_MAX_INVS 32
#define QI_PGRP_IDX(idx) (((u64)(idx)) << 55)
@@ -496,8 +497,13 @@ extern void qi_flush_context(struct intel_iommu *iommu, u16 did, u16 sid,
u8 fm, u64 type);
extern void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
unsigned int size_order, u64 type);
-extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask);
+extern void qi_flush_eiotlb(struct intel_iommu *iommu, u16 did, u64 addr,
+ u32 pasid, unsigned int size_order, u64 type, bool global);
+extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask);
+extern void qi_flush_dev_eiotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep, u64 addr, unsigned size);
+extern void qi_flush_pasid(struct intel_iommu *iommu, u16 did, u64 granu, int pasid);
extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan, Liu, Yi L
With shared virtual memory vitualization, extended IOTLB invalidation
may be passed down from outside IOMMU subsystems. This patch adds
invalidation functions that can be used for each IOTLB types.
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: Ashok Raj <ashok.raj@intel.com>
---
drivers/iommu/dmar.c | 54 ++++++++++++++++++++++++++++++++++++++++++---
drivers/iommu/intel-iommu.c | 3 ++-
include/linux/intel-iommu.h | 10 +++++++--
3 files changed, 61 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 57c920c..f69f6ee 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1336,11 +1336,25 @@ void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
qi_submit_sync(&desc, iommu);
}
-void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask)
+void qi_flush_eiotlb(struct intel_iommu *iommu, u16 did, u64 addr, u32 pasid,
+ unsigned int size_order, u64 granu, bool global)
{
struct qi_desc desc;
+ desc.low = QI_EIOTLB_PASID(pasid) | QI_EIOTLB_DID(did) |
+ QI_EIOTLB_GRAN(granu) | QI_EIOTLB_TYPE;
+ desc.high = QI_EIOTLB_ADDR(addr) | QI_EIOTLB_GL(global) |
+ QI_EIOTLB_IH(0) | QI_EIOTLB_AM(size_order);
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask)
+{
+ struct qi_desc desc;
+
+ pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr %llx, mask %d\n",
+ __func__, sid, pfsid, qdep, addr, mask);
if (mask) {
BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
@@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
qdep = 0;
desc.low = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
- QI_DIOTLB_TYPE;
+ QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
+
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_dev_eiotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep, u64 addr, unsigned size, u64 granu)
+{
+ struct qi_desc desc;
+
+ desc.low = QI_DEV_EIOTLB_PASID(pasid) | QI_DEV_EIOTLB_SID(sid) |
+ QI_DEV_EIOTLB_QDEP(qdep) | QI_DEIOTLB_TYPE |
+ QI_DEV_EIOTLB_PFSID(pfsid);
+ desc.high |= QI_DEV_EIOTLB_GLOB(granu);
+
+ /* If S bit is 0, we only flush a single page. If S bit is set,
+ * The least significant zero bit indicates the size. VT-d spec
+ * 6.5.2.6
+ */
+ if (!size)
+ desc.high = QI_DEV_EIOTLB_ADDR(addr) & ~QI_DEV_EIOTLB_SIZE;
+ else {
+ unsigned long mask = 1UL << (VTD_PAGE_SHIFT + size);
+
+ desc.high = QI_DEV_EIOTLB_ADDR(addr & ~mask) | QI_DEV_EIOTLB_SIZE;
+ }
+ qi_submit_sync(&desc, iommu);
+}
+
+void qi_flush_pasid(struct intel_iommu *iommu, u16 did, u64 granu, int pasid)
+{
+ struct qi_desc desc;
+
+ desc.high = 0;
+ desc.low = QI_PC_TYPE | QI_PC_DID(did) | QI_PC_GRAN(granu) | QI_PC_PASID(pasid);
qi_submit_sync(&desc, iommu);
}
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 399b504..556bdd2 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1524,7 +1524,8 @@ static void iommu_flush_dev_iotlb(struct dmar_domain *domain,
sid = info->bus << 8 | info->devfn;
qdep = info->ats_qdep;
- qi_flush_dev_iotlb(info->iommu, sid, qdep, addr, mask);
+ qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
+ qdep, addr, mask);
}
spin_unlock_irqrestore(&device_domain_lock, flags);
}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8d38e24..3c83f7e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -305,6 +305,7 @@ enum {
#define QI_DEV_EIOTLB_PASID(p) (((u64)p) << 32)
#define QI_DEV_EIOTLB_SID(sid) ((u64)((sid) & 0xffff) << 16)
#define QI_DEV_EIOTLB_QDEP(qd) ((u64)((qd) & 0x1f) << 4)
+#define QI_DEV_EIOTLB_PFSID(pfsid) (((u64)(pfsid & 0xf) << 12) | ((u64)(pfsid & 0xff0) << 48))
#define QI_DEV_EIOTLB_MAX_INVS 32
#define QI_PGRP_IDX(idx) (((u64)(idx)) << 55)
@@ -496,8 +497,13 @@ extern void qi_flush_context(struct intel_iommu *iommu, u16 did, u16 sid,
u8 fm, u64 type);
extern void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
unsigned int size_order, u64 type);
-extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
- u64 addr, unsigned mask);
+extern void qi_flush_eiotlb(struct intel_iommu *iommu, u16 did, u64 addr,
+ u32 pasid, unsigned int size_order, u64 type, bool global);
+extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u16 qdep, u64 addr, unsigned mask);
+extern void qi_flush_dev_eiotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
+ u32 pasid, u16 qdep, u64 addr, unsigned size);
+extern void qi_flush_pasid(struct intel_iommu *iommu, u16 did, u64 granu, int pasid);
extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread[parent not found: <1510944914-54430-6-git-send-email-jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
2017-11-17 18:55 ` Jacob Pan
@ 2017-11-20 14:20 ` Lukoshkov, Maksim
-1 siblings, 0 replies; 94+ messages in thread
From: Lukoshkov, Maksim @ 2017-11-20 14:20 UTC (permalink / raw)
To: Jacob Pan,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Wysocki, Rafael J, Alex Williamson
Cc: Lan, Tianyu, Yi L, Jean Delvare,
Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY@public.gmane.org
On 11/17/2017 18:55, Jacob Pan wrote:
> +void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
> + u16 qdep, u64 addr, unsigned mask)
> +{
> + struct qi_desc desc;
> +
> + pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr %llx, mask %d\n",
> + __func__, sid, pfsid, qdep, addr, mask);
> if (mask) {
> BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
> addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
> @@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
> qdep = 0;
>
> desc.low = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
> - QI_DIOTLB_TYPE;
> + QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
QI_DEV_IOTLB_SID(pfsid) -> QI_DEV_EIOTLB_PFSID(pfsid)?
> +
> + qi_submit_sync(&desc, iommu);
> +}
> +
Regards,
Maksim Lukoshkov
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
@ 2017-11-20 14:20 ` Lukoshkov, Maksim
0 siblings, 0 replies; 94+ messages in thread
From: Lukoshkov, Maksim @ 2017-11-20 14:20 UTC (permalink / raw)
To: Jacob Pan, iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Wysocki, Rafael J,
Alex Williamson
Cc: Lan, Tianyu, Yi L, Liu@mail.linuxfoundation.org, Jean Delvare
On 11/17/2017 18:55, Jacob Pan wrote:
> +void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
> + u16 qdep, u64 addr, unsigned mask)
> +{
> + struct qi_desc desc;
> +
> + pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr %llx, mask %d\n",
> + __func__, sid, pfsid, qdep, addr, mask);
> if (mask) {
> BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) - 1));
> addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
> @@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
> qdep = 0;
>
> desc.low = QI_DEV_IOTLB_SID(sid) | QI_DEV_IOTLB_QDEP(qdep) |
> - QI_DIOTLB_TYPE;
> + QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
QI_DEV_IOTLB_SID(pfsid) -> QI_DEV_EIOTLB_PFSID(pfsid)?
> +
> + qi_submit_sync(&desc, iommu);
> +}
> +
Regards,
Maksim Lukoshkov
^ permalink raw reply [flat|nested] 94+ messages in thread[parent not found: <04788a00-2075-7975-2f13-b9e032688305-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
2017-11-20 14:20 ` Lukoshkov, Maksim
@ 2017-11-20 18:40 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-20 18:40 UTC (permalink / raw)
To: Lukoshkov, Maksim
Cc: Lan, Tianyu, Yi L,
Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY@public.gmane.org,
Greg Kroah-Hartman, Wysocki, Rafael J, LKML,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Jean Delvare, David Woodhouse
On Mon, 20 Nov 2017 14:20:31 +0000
"Lukoshkov, Maksim" <maksim.lukoshkov-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On 11/17/2017 18:55, Jacob Pan wrote:
> > +void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16
> > pfsid,
> > + u16 qdep, u64 addr, unsigned mask)
> > +{
> > + struct qi_desc desc;
> > +
> > + pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr
> > %llx, mask %d\n",
> > + __func__, sid, pfsid, qdep, addr, mask);
> > if (mask) {
> > BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) -
> > 1)); addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
> > @@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu
> > *iommu, u16 sid, u16 qdep, qdep = 0;
> >
> > desc.low = QI_DEV_IOTLB_SID(sid) |
> > QI_DEV_IOTLB_QDEP(qdep) |
> > - QI_DIOTLB_TYPE;
> > + QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
>
> QI_DEV_IOTLB_SID(pfsid) -> QI_DEV_EIOTLB_PFSID(pfsid)?
>
good catch! thank you.
> > +
> > + qi_submit_sync(&desc, iommu);
> > +}
> > +
>
> Regards,
> Maksim Lukoshkov
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 05/16] iommu/vt-d: support flushing more TLB types
@ 2017-11-20 18:40 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-20 18:40 UTC (permalink / raw)
To: Lukoshkov, Maksim
Cc: iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Wysocki, Rafael J,
Alex Williamson, Lan, Tianyu, Yi L, Liu@mail.linuxfoundation.org,
Jean Delvare, jacob.jun.pan
On Mon, 20 Nov 2017 14:20:31 +0000
"Lukoshkov, Maksim" <maksim.lukoshkov@intel.com> wrote:
> On 11/17/2017 18:55, Jacob Pan wrote:
> > +void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16
> > pfsid,
> > + u16 qdep, u64 addr, unsigned mask)
> > +{
> > + struct qi_desc desc;
> > +
> > + pr_debug_ratelimited("%s: sid %d, pfsid %d, qdep %d, addr
> > %llx, mask %d\n",
> > + __func__, sid, pfsid, qdep, addr, mask);
> > if (mask) {
> > BUG_ON(addr & ((1 << (VTD_PAGE_SHIFT + mask)) -
> > 1)); addr |= (1ULL << (VTD_PAGE_SHIFT + mask - 1)) - 1;
> > @@ -1352,7 +1366,41 @@ void qi_flush_dev_iotlb(struct intel_iommu
> > *iommu, u16 sid, u16 qdep, qdep = 0;
> >
> > desc.low = QI_DEV_IOTLB_SID(sid) |
> > QI_DEV_IOTLB_QDEP(qdep) |
> > - QI_DIOTLB_TYPE;
> > + QI_DIOTLB_TYPE | QI_DEV_IOTLB_SID(pfsid);
>
> QI_DEV_IOTLB_SID(pfsid) -> QI_DEV_EIOTLB_PFSID(pfsid)?
>
good catch! thank you.
> > +
> > + qi_submit_sync(&desc, iommu);
> > +}
> > +
>
> Regards,
> Maksim Lukoshkov
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 10/16] iommu: introduce device fault report API
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/iommu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/iommu.h | 36 +++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 829e9e9..97b7990 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
goto err_free_name;
}
+ dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
+ if (!dev->iommu_param) {
+ ret = -ENOMEM;
+ goto err_free_name;
+ }
+
kobject_get(group->devices_kobj);
dev->iommu_group = group;
@@ -657,7 +663,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;
@@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
}
EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
+int iommu_register_device_fault_handler(struct device *dev,
+ iommu_dev_fault_handler_t handler,
+ void *data)
+{
+ struct iommu_param *idata = dev->iommu_param;
+
+ /*
+ * Device iommu_param should have been allocated when device is
+ * added to its iommu_group.
+ */
+ if (!idata)
+ return -EINVAL;
+ /* Only allow one fault handler registered for each device */
+ if (idata->fault_param)
+ return -EBUSY;
+ get_device(dev);
+ idata->fault_param =
+ kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
+ if (!idata->fault_param)
+ return -ENOMEM;
+ idata->fault_param->handler = handler;
+ idata->fault_param->data = data;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
+
+int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ struct iommu_param *idata = dev->iommu_param;
+
+ if (!idata)
+ return -EINVAL;
+
+ kfree(idata->fault_param);
+ idata->fault_param = NULL;
+ put_device(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
+
+
+int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ /* we only report device fault if there is a handler registered */
+ if (!dev->iommu_param || !dev->iommu_param->fault_param ||
+ !dev->iommu_param->fault_param->handler)
+ return -ENOSYS;
+
+ return dev->iommu_param->fault_param->handler(evt,
+ dev->iommu_param->fault_param->data);
+}
+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 dfda89b..841c044 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -463,6 +463,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 *);
@@ -481,6 +489,12 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags);
+static inline bool iommu_has_device_fault_handler(struct device *dev)
+{
+ return dev->iommu_param && dev->iommu_param->fault_param &&
+ dev->iommu_param->fault_param->handler;
+}
+
static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
{
if (domain->ops->flush_iotlb_all)
@@ -734,6 +748,28 @@ 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 0;
+}
+
+static inline int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ return 0;
+}
+
+static inline bool iommu_has_device_fault_handler(struct device *dev)
+{
+ return false;
+}
+
+static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ return 0;
+}
+
static inline int iommu_group_id(struct iommu_group *group)
{
return -ENODEV;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 10/16] iommu: introduce device fault report API
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
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>
---
drivers/iommu/iommu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/iommu.h | 36 +++++++++++++++++++++++++++++
2 files changed, 98 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 829e9e9..97b7990 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
goto err_free_name;
}
+ dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
+ if (!dev->iommu_param) {
+ ret = -ENOMEM;
+ goto err_free_name;
+ }
+
kobject_get(group->devices_kobj);
dev->iommu_group = group;
@@ -657,7 +663,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;
@@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
}
EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
+int iommu_register_device_fault_handler(struct device *dev,
+ iommu_dev_fault_handler_t handler,
+ void *data)
+{
+ struct iommu_param *idata = dev->iommu_param;
+
+ /*
+ * Device iommu_param should have been allocated when device is
+ * added to its iommu_group.
+ */
+ if (!idata)
+ return -EINVAL;
+ /* Only allow one fault handler registered for each device */
+ if (idata->fault_param)
+ return -EBUSY;
+ get_device(dev);
+ idata->fault_param =
+ kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
+ if (!idata->fault_param)
+ return -ENOMEM;
+ idata->fault_param->handler = handler;
+ idata->fault_param->data = data;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
+
+int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ struct iommu_param *idata = dev->iommu_param;
+
+ if (!idata)
+ return -EINVAL;
+
+ kfree(idata->fault_param);
+ idata->fault_param = NULL;
+ put_device(dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
+
+
+int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ /* we only report device fault if there is a handler registered */
+ if (!dev->iommu_param || !dev->iommu_param->fault_param ||
+ !dev->iommu_param->fault_param->handler)
+ return -ENOSYS;
+
+ return dev->iommu_param->fault_param->handler(evt,
+ dev->iommu_param->fault_param->data);
+}
+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 dfda89b..841c044 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -463,6 +463,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 *);
@@ -481,6 +489,12 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
unsigned long iova, int flags);
+static inline bool iommu_has_device_fault_handler(struct device *dev)
+{
+ return dev->iommu_param && dev->iommu_param->fault_param &&
+ dev->iommu_param->fault_param->handler;
+}
+
static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
{
if (domain->ops->flush_iotlb_all)
@@ -734,6 +748,28 @@ 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 0;
+}
+
+static inline int iommu_unregister_device_fault_handler(struct device *dev)
+{
+ return 0;
+}
+
+static inline bool iommu_has_device_fault_handler(struct device *dev)
+{
+ return false;
+}
+
+static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
+{
+ return 0;
+}
+
static inline int iommu_group_id(struct iommu_group *group)
{
return -ENODEV;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-11-17 18:55 ` Jacob Pan
(?)
@ 2017-12-05 6:22 ` Lu Baolu
[not found] ` <5A263B31.6050808-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
-1 siblings, 1 reply; 94+ messages in thread
From: Lu Baolu @ 2017-12-05 6:22 UTC (permalink / raw)
To: Jacob Pan, iommu, LKML, Joerg Roedel, David Woodhouse,
Greg Kroah-Hartman, Rafael Wysocki, Alex Williamson
Cc: Lan Tianyu, Jean Delvare
Hi,
On 11/18/2017 02:55 AM, Jacob Pan wrote:
> 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>
> ---
> drivers/iommu/iommu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/iommu.h | 36 +++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 829e9e9..97b7990 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> goto err_free_name;
> }
>
> + dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!dev->iommu_param) {
> + ret = -ENOMEM;
> + goto err_free_name;
> + }
> +
> kobject_get(group->devices_kobj);
>
> dev->iommu_group = group;
> @@ -657,7 +663,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;
> @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
> }
> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
>
> +int iommu_register_device_fault_handler(struct device *dev,
> + iommu_dev_fault_handler_t handler,
> + void *data)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + /*
> + * Device iommu_param should have been allocated when device is
> + * added to its iommu_group.
> + */
> + if (!idata)
> + return -EINVAL;
> + /* Only allow one fault handler registered for each device */
> + if (idata->fault_param)
> + return -EBUSY;
> + get_device(dev);
> + idata->fault_param =
> + kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!idata->fault_param)
> + return -ENOMEM;
> + idata->fault_param->handler = handler;
> + idata->fault_param->data = data;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> +
> +int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + if (!idata)
> + return -EINVAL;
> +
> + kfree(idata->fault_param);
> + idata->fault_param = NULL;
> + put_device(dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> +
> +
> +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + /* we only report device fault if there is a handler registered */
> + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> + !dev->iommu_param->fault_param->handler)
Can this replaced by:
if (!iommu_has_device_fault_handler(dev))
?
Best regards,
Lu Baolu
> + return -ENOSYS;
> +
> + return dev->iommu_param->fault_param->handler(evt,
> + dev->iommu_param->fault_param->data);
> +}
> +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 dfda89b..841c044 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -463,6 +463,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 *);
> @@ -481,6 +489,12 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
> extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> unsigned long iova, int flags);
>
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return dev->iommu_param && dev->iommu_param->fault_param &&
> + dev->iommu_param->fault_param->handler;
> +}
> +
> static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> {
> if (domain->ops->flush_iotlb_all)
> @@ -734,6 +748,28 @@ 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 0;
> +}
> +
> +static inline int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return false;
> +}
> +
> +static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + return 0;
> +}
> +
> static inline int iommu_group_id(struct iommu_group *group)
> {
> return -ENODEV;
^ permalink raw reply [flat|nested] 94+ messages in thread[parent not found: <1510944914-54430-11-git-send-email-jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-11-17 18:55 ` Jacob Pan
@ 2017-12-07 21:27 ` Alex Williamson
-1 siblings, 0 replies; 94+ messages in thread
From: Alex Williamson @ 2017-12-07 21:27 UTC (permalink / raw)
To: Jacob Pan
Cc: Lan Tianyu, Greg Kroah-Hartman, Rafael Wysocki, LKML,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Jean Delvare,
David Woodhouse
On Fri, 17 Nov 2017 10:55:08 -0800
Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/iommu/iommu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/iommu.h | 36 +++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 829e9e9..97b7990 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> goto err_free_name;
> }
>
> + dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!dev->iommu_param) {
> + ret = -ENOMEM;
> + goto err_free_name;
> + }
> +
> kobject_get(group->devices_kobj);
>
> dev->iommu_group = group;
> @@ -657,7 +663,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;
> @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
> }
> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
>
> +int iommu_register_device_fault_handler(struct device *dev,
> + iommu_dev_fault_handler_t handler,
> + void *data)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + /*
> + * Device iommu_param should have been allocated when device is
> + * added to its iommu_group.
> + */
> + if (!idata)
> + return -EINVAL;
> + /* Only allow one fault handler registered for each device */
> + if (idata->fault_param)
> + return -EBUSY;
> + get_device(dev);
> + idata->fault_param =
> + kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!idata->fault_param)
> + return -ENOMEM;
> + idata->fault_param->handler = handler;
> + idata->fault_param->data = data;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> +
> +int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + if (!idata)
> + return -EINVAL;
> +
> + kfree(idata->fault_param);
> + idata->fault_param = NULL;
> + put_device(dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> +
> +
> +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + /* we only report device fault if there is a handler registered */
> + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> + !dev->iommu_param->fault_param->handler)
> + return -ENOSYS;
> +
> + return dev->iommu_param->fault_param->handler(evt,
> + dev->iommu_param->fault_param->data);
> +}
> +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> +
Isn't this all rather racy? I see that we can have multiple callers to
register racing. Unregister is buggy, allowing any caller to decrement
the device reference regardless of whether there's one outstanding
through this interface. The reporting callout can also race with an
unregistration. Might need a mutex on iommu_param to avoid.
> /**
> * 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 dfda89b..841c044 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -463,6 +463,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 *);
> @@ -481,6 +489,12 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
> extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> unsigned long iova, int flags);
>
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return dev->iommu_param && dev->iommu_param->fault_param &&
> + dev->iommu_param->fault_param->handler;
> +}
> +
This interface is racy by design, there's no guarantee that the
handler isn't immediately unregistered after this check. Thanks,
Alex
> static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> {
> if (domain->ops->flush_iotlb_all)
> @@ -734,6 +748,28 @@ 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 0;
> +}
> +
> +static inline int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return false;
> +}
> +
> +static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + return 0;
> +}
> +
> static inline int iommu_group_id(struct iommu_group *group)
> {
> return -ENODEV;
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
@ 2017-12-07 21:27 ` Alex Williamson
0 siblings, 0 replies; 94+ messages in thread
From: Alex Williamson @ 2017-12-07 21:27 UTC (permalink / raw)
To: Jacob Pan
Cc: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok,
Jean Delvare, Christoph Hellwig
On Fri, 17 Nov 2017 10:55:08 -0800
Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
> 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>
> ---
> drivers/iommu/iommu.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> include/linux/iommu.h | 36 +++++++++++++++++++++++++++++
> 2 files changed, 98 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 829e9e9..97b7990 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> goto err_free_name;
> }
>
> + dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!dev->iommu_param) {
> + ret = -ENOMEM;
> + goto err_free_name;
> + }
> +
> kobject_get(group->devices_kobj);
>
> dev->iommu_group = group;
> @@ -657,7 +663,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;
> @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
> }
> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
>
> +int iommu_register_device_fault_handler(struct device *dev,
> + iommu_dev_fault_handler_t handler,
> + void *data)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + /*
> + * Device iommu_param should have been allocated when device is
> + * added to its iommu_group.
> + */
> + if (!idata)
> + return -EINVAL;
> + /* Only allow one fault handler registered for each device */
> + if (idata->fault_param)
> + return -EBUSY;
> + get_device(dev);
> + idata->fault_param =
> + kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!idata->fault_param)
> + return -ENOMEM;
> + idata->fault_param->handler = handler;
> + idata->fault_param->data = data;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> +
> +int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + if (!idata)
> + return -EINVAL;
> +
> + kfree(idata->fault_param);
> + idata->fault_param = NULL;
> + put_device(dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> +
> +
> +int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + /* we only report device fault if there is a handler registered */
> + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> + !dev->iommu_param->fault_param->handler)
> + return -ENOSYS;
> +
> + return dev->iommu_param->fault_param->handler(evt,
> + dev->iommu_param->fault_param->data);
> +}
> +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> +
Isn't this all rather racy? I see that we can have multiple callers to
register racing. Unregister is buggy, allowing any caller to decrement
the device reference regardless of whether there's one outstanding
through this interface. The reporting callout can also race with an
unregistration. Might need a mutex on iommu_param to avoid.
> /**
> * 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 dfda89b..841c044 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -463,6 +463,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 *);
> @@ -481,6 +489,12 @@ extern void iommu_domain_window_disable(struct iommu_domain *domain, u32 wnd_nr)
> extern int report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> unsigned long iova, int flags);
>
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return dev->iommu_param && dev->iommu_param->fault_param &&
> + dev->iommu_param->fault_param->handler;
> +}
> +
This interface is racy by design, there's no guarantee that the
handler isn't immediately unregistered after this check. Thanks,
Alex
> static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> {
> if (domain->ops->flush_iotlb_all)
> @@ -734,6 +748,28 @@ 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 0;
> +}
> +
> +static inline int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return false;
> +}
> +
> +static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + return 0;
> +}
> +
> static inline int iommu_group_id(struct iommu_group *group)
> {
> return -ENODEV;
^ permalink raw reply [flat|nested] 94+ messages in thread[parent not found: <20171207142725.731695ea-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org>]
* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-12-07 21:27 ` Alex Williamson
@ 2017-12-08 20:23 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-12-08 20:23 UTC (permalink / raw)
To: Alex Williamson
Cc: Lan Tianyu, Greg Kroah-Hartman, Rafael Wysocki, LKML,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Jean Delvare,
David Woodhouse
On Thu, 7 Dec 2017 14:27:25 -0700
Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 17 Nov 2017 10:55:08 -0800
> Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>
> > 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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/iommu/iommu.c | 63
> > ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > include/linux/iommu.h | 36 +++++++++++++++++++++++++++++ 2 files
> > changed, 98 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 829e9e9..97b7990 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group
> > *group, struct device *dev) goto err_free_name;
> > }
> >
> > + dev->iommu_param = kzalloc(sizeof(struct
> > iommu_fault_param), GFP_KERNEL);
> > + if (!dev->iommu_param) {
> > + ret = -ENOMEM;
> > + goto err_free_name;
> > + }
> > +
> > kobject_get(group->devices_kobj);
> >
> > dev->iommu_group = group;
> > @@ -657,7 +663,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;
> > @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct
> > iommu_group *group, }
> > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
> >
> > +int iommu_register_device_fault_handler(struct device *dev,
> > + iommu_dev_fault_handler_t
> > handler,
> > + void *data)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + /*
> > + * Device iommu_param should have been allocated when
> > device is
> > + * added to its iommu_group.
> > + */
> > + if (!idata)
> > + return -EINVAL;
> > + /* Only allow one fault handler registered for each device
> > */
> > + if (idata->fault_param)
> > + return -EBUSY;
> > + get_device(dev);
> > + idata->fault_param =
> > + kzalloc(sizeof(struct iommu_fault_param),
> > GFP_KERNEL);
> > + if (!idata->fault_param)
> > + return -ENOMEM;
> > + idata->fault_param->handler = handler;
> > + idata->fault_param->data = data;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> > +
> > +int iommu_unregister_device_fault_handler(struct device *dev)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + if (!idata)
> > + return -EINVAL;
> > +
> > + kfree(idata->fault_param);
> > + idata->fault_param = NULL;
> > + put_device(dev);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> > +
> > +
> > +int iommu_report_device_fault(struct device *dev, struct
> > iommu_fault_event *evt) +{
> > + /* we only report device fault if there is a handler
> > registered */
> > + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> > + !dev->iommu_param->fault_param->handler)
> > + return -ENOSYS;
> > +
> > + return dev->iommu_param->fault_param->handler(evt,
> > +
> > dev->iommu_param->fault_param->data); +}
> > +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> > +
>
> Isn't this all rather racy? I see that we can have multiple callers
> to register racing.
I agree, should use a lock here to prevent unregister. For multiple
caller race, it won't happen since there is only one caller can
register handler.
> Unregister is buggy, allowing any caller to
> decrement the device reference regardless of whether there's one
> outstanding through this interface. The reporting callout can also
> race with an unregistration. Might need a mutex on iommu_param to
> avoid.
>
you are right, forgot to check outstanding handler. will add mutex also.
Thanks,
> > /**
> > * 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 dfda89b..841c044 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -463,6 +463,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 *); @@ -481,6
> > +489,12 @@ extern void iommu_domain_window_disable(struct
> > iommu_domain *domain, u32 wnd_nr) extern int
> > report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> > unsigned long iova, int flags); +static inline bool
> > iommu_has_device_fault_handler(struct device *dev) +{
> > + return dev->iommu_param && dev->iommu_param->fault_param &&
> > + dev->iommu_param->fault_param->handler;
> > +}
> > +
>
> This interface is racy by design, there's no guarantee that the
> handler isn't immediately unregistered after this check. Thanks,
>
right, I will fold this check into report function and protect by a
lock. I was trying to save some cycles but it would not work with the
race condition.
> Alex
>
> > static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> > {
> > if (domain->ops->flush_iotlb_all)
> > @@ -734,6 +748,28 @@ 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 0;
> > +}
> > +
> > +static inline int iommu_unregister_device_fault_handler(struct
> > device *dev) +{
> > + return 0;
> > +}
> > +
> > +static inline bool iommu_has_device_fault_handler(struct device
> > *dev) +{
> > + return false;
> > +}
> > +
> > +static inline int iommu_report_device_fault(struct device *dev,
> > struct iommu_fault_event *evt) +{
> > + return 0;
> > +}
> > +
> > static inline int iommu_group_id(struct iommu_group *group)
> > {
> > return -ENODEV;
>
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
@ 2017-12-08 20:23 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-12-08 20:23 UTC (permalink / raw)
To: Alex Williamson
Cc: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok,
Jean Delvare, Christoph Hellwig, jacob.jun.pan
On Thu, 7 Dec 2017 14:27:25 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> On Fri, 17 Nov 2017 10:55:08 -0800
> Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
>
> > 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>
> > ---
> > drivers/iommu/iommu.c | 63
> > ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > include/linux/iommu.h | 36 +++++++++++++++++++++++++++++ 2 files
> > changed, 98 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 829e9e9..97b7990 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group
> > *group, struct device *dev) goto err_free_name;
> > }
> >
> > + dev->iommu_param = kzalloc(sizeof(struct
> > iommu_fault_param), GFP_KERNEL);
> > + if (!dev->iommu_param) {
> > + ret = -ENOMEM;
> > + goto err_free_name;
> > + }
> > +
> > kobject_get(group->devices_kobj);
> >
> > dev->iommu_group = group;
> > @@ -657,7 +663,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;
> > @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct
> > iommu_group *group, }
> > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
> >
> > +int iommu_register_device_fault_handler(struct device *dev,
> > + iommu_dev_fault_handler_t
> > handler,
> > + void *data)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + /*
> > + * Device iommu_param should have been allocated when
> > device is
> > + * added to its iommu_group.
> > + */
> > + if (!idata)
> > + return -EINVAL;
> > + /* Only allow one fault handler registered for each device
> > */
> > + if (idata->fault_param)
> > + return -EBUSY;
> > + get_device(dev);
> > + idata->fault_param =
> > + kzalloc(sizeof(struct iommu_fault_param),
> > GFP_KERNEL);
> > + if (!idata->fault_param)
> > + return -ENOMEM;
> > + idata->fault_param->handler = handler;
> > + idata->fault_param->data = data;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> > +
> > +int iommu_unregister_device_fault_handler(struct device *dev)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + if (!idata)
> > + return -EINVAL;
> > +
> > + kfree(idata->fault_param);
> > + idata->fault_param = NULL;
> > + put_device(dev);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> > +
> > +
> > +int iommu_report_device_fault(struct device *dev, struct
> > iommu_fault_event *evt) +{
> > + /* we only report device fault if there is a handler
> > registered */
> > + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> > + !dev->iommu_param->fault_param->handler)
> > + return -ENOSYS;
> > +
> > + return dev->iommu_param->fault_param->handler(evt,
> > +
> > dev->iommu_param->fault_param->data); +}
> > +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> > +
>
> Isn't this all rather racy? I see that we can have multiple callers
> to register racing.
I agree, should use a lock here to prevent unregister. For multiple
caller race, it won't happen since there is only one caller can
register handler.
> Unregister is buggy, allowing any caller to
> decrement the device reference regardless of whether there's one
> outstanding through this interface. The reporting callout can also
> race with an unregistration. Might need a mutex on iommu_param to
> avoid.
>
you are right, forgot to check outstanding handler. will add mutex also.
Thanks,
> > /**
> > * 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 dfda89b..841c044 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -463,6 +463,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 *); @@ -481,6
> > +489,12 @@ extern void iommu_domain_window_disable(struct
> > iommu_domain *domain, u32 wnd_nr) extern int
> > report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> > unsigned long iova, int flags); +static inline bool
> > iommu_has_device_fault_handler(struct device *dev) +{
> > + return dev->iommu_param && dev->iommu_param->fault_param &&
> > + dev->iommu_param->fault_param->handler;
> > +}
> > +
>
> This interface is racy by design, there's no guarantee that the
> handler isn't immediately unregistered after this check. Thanks,
>
right, I will fold this check into report function and protect by a
lock. I was trying to save some cycles but it would not work with the
race condition.
> Alex
>
> > static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> > {
> > if (domain->ops->flush_iotlb_all)
> > @@ -734,6 +748,28 @@ 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 0;
> > +}
> > +
> > +static inline int iommu_unregister_device_fault_handler(struct
> > device *dev) +{
> > + return 0;
> > +}
> > +
> > +static inline bool iommu_has_device_fault_handler(struct device
> > *dev) +{
> > + return false;
> > +}
> > +
> > +static inline int iommu_report_device_fault(struct device *dev,
> > struct iommu_fault_event *evt) +{
> > + return 0;
> > +}
> > +
> > static inline int iommu_group_id(struct iommu_group *group)
> > {
> > return -ENODEV;
>
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-12-08 20:23 ` Jacob Pan
@ 2017-12-08 20:59 ` Alex Williamson
-1 siblings, 0 replies; 94+ messages in thread
From: Alex Williamson @ 2017-12-08 20:59 UTC (permalink / raw)
To: Jacob Pan
Cc: Lan Tianyu, Greg Kroah-Hartman, Rafael Wysocki, LKML,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Jean Delvare,
David Woodhouse
On Fri, 8 Dec 2017 12:23:58 -0800
Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Thu, 7 Dec 2017 14:27:25 -0700
> Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> > On Fri, 17 Nov 2017 10:55:08 -0800
> > Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> >
> > > 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-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > > Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > > ---
> > > drivers/iommu/iommu.c | 63
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > > include/linux/iommu.h | 36 +++++++++++++++++++++++++++++ 2 files
> > > changed, 98 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 829e9e9..97b7990 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group
> > > *group, struct device *dev) goto err_free_name;
> > > }
> > >
> > > + dev->iommu_param = kzalloc(sizeof(struct
> > > iommu_fault_param), GFP_KERNEL);
> > > + if (!dev->iommu_param) {
> > > + ret = -ENOMEM;
> > > + goto err_free_name;
> > > + }
> > > +
> > > kobject_get(group->devices_kobj);
> > >
> > > dev->iommu_group = group;
> > > @@ -657,7 +663,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;
> > > @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct
> > > iommu_group *group, }
> > > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
> > >
> > > +int iommu_register_device_fault_handler(struct device *dev,
> > > + iommu_dev_fault_handler_t
> > > handler,
> > > + void *data)
> > > +{
> > > + struct iommu_param *idata = dev->iommu_param;
> > > +
> > > + /*
> > > + * Device iommu_param should have been allocated when
> > > device is
> > > + * added to its iommu_group.
> > > + */
> > > + if (!idata)
> > > + return -EINVAL;
> > > + /* Only allow one fault handler registered for each device
> > > */
> > > + if (idata->fault_param)
> > > + return -EBUSY;
> > > + get_device(dev);
> > > + idata->fault_param =
> > > + kzalloc(sizeof(struct iommu_fault_param),
> > > GFP_KERNEL);
> > > + if (!idata->fault_param)
> > > + return -ENOMEM;
> > > + idata->fault_param->handler = handler;
> > > + idata->fault_param->data = data;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> > > +
> > > +int iommu_unregister_device_fault_handler(struct device *dev)
> > > +{
> > > + struct iommu_param *idata = dev->iommu_param;
> > > +
> > > + if (!idata)
> > > + return -EINVAL;
> > > +
> > > + kfree(idata->fault_param);
> > > + idata->fault_param = NULL;
> > > + put_device(dev);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> > > +
> > > +
> > > +int iommu_report_device_fault(struct device *dev, struct
> > > iommu_fault_event *evt) +{
> > > + /* we only report device fault if there is a handler
> > > registered */
> > > + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> > > + !dev->iommu_param->fault_param->handler)
> > > + return -ENOSYS;
> > > +
> > > + return dev->iommu_param->fault_param->handler(evt,
> > > +
> > > dev->iommu_param->fault_param->data); +}
> > > +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> > > +
> >
> > Isn't this all rather racy? I see that we can have multiple callers
> > to register racing.
> I agree, should use a lock here to prevent unregister. For multiple
> caller race, it won't happen since there is only one caller can
> register handler.
If you have multiple simultaneous callers to
iommu_register_device_fault_handler, they can all get past the test
for fault_param (testing and setting is not atomic), then it's
indeterminate which handler gets installed. Thanks,
Alex
> > Unregister is buggy, allowing any caller to
> > decrement the device reference regardless of whether there's one
> > outstanding through this interface. The reporting callout can also
> > race with an unregistration. Might need a mutex on iommu_param to
> > avoid.
> >
> you are right, forgot to check outstanding handler. will add mutex also.
>
> Thanks,
> > > /**
> > > * 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 dfda89b..841c044 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -463,6 +463,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 *); @@ -481,6
> > > +489,12 @@ extern void iommu_domain_window_disable(struct
> > > iommu_domain *domain, u32 wnd_nr) extern int
> > > report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> > > unsigned long iova, int flags); +static inline bool
> > > iommu_has_device_fault_handler(struct device *dev) +{
> > > + return dev->iommu_param && dev->iommu_param->fault_param &&
> > > + dev->iommu_param->fault_param->handler;
> > > +}
> > > +
> >
> > This interface is racy by design, there's no guarantee that the
> > handler isn't immediately unregistered after this check. Thanks,
> >
> right, I will fold this check into report function and protect by a
> lock. I was trying to save some cycles but it would not work with the
> race condition.
> > Alex
> >
> > > static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> > > {
> > > if (domain->ops->flush_iotlb_all)
> > > @@ -734,6 +748,28 @@ 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 0;
> > > +}
> > > +
> > > +static inline int iommu_unregister_device_fault_handler(struct
> > > device *dev) +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline bool iommu_has_device_fault_handler(struct device
> > > *dev) +{
> > > + return false;
> > > +}
> > > +
> > > +static inline int iommu_report_device_fault(struct device *dev,
> > > struct iommu_fault_event *evt) +{
> > > + return 0;
> > > +}
> > > +
> > > static inline int iommu_group_id(struct iommu_group *group)
> > > {
> > > return -ENODEV;
> >
>
> [Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
@ 2017-12-08 20:59 ` Alex Williamson
0 siblings, 0 replies; 94+ messages in thread
From: Alex Williamson @ 2017-12-08 20:59 UTC (permalink / raw)
To: Jacob Pan
Cc: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok,
Jean Delvare, Christoph Hellwig
On Fri, 8 Dec 2017 12:23:58 -0800
Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
> On Thu, 7 Dec 2017 14:27:25 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
> > On Fri, 17 Nov 2017 10:55:08 -0800
> > Jacob Pan <jacob.jun.pan@linux.intel.com> wrote:
> >
> > > 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>
> > > ---
> > > drivers/iommu/iommu.c | 63
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++-
> > > include/linux/iommu.h | 36 +++++++++++++++++++++++++++++ 2 files
> > > changed, 98 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 829e9e9..97b7990 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group
> > > *group, struct device *dev) goto err_free_name;
> > > }
> > >
> > > + dev->iommu_param = kzalloc(sizeof(struct
> > > iommu_fault_param), GFP_KERNEL);
> > > + if (!dev->iommu_param) {
> > > + ret = -ENOMEM;
> > > + goto err_free_name;
> > > + }
> > > +
> > > kobject_get(group->devices_kobj);
> > >
> > > dev->iommu_group = group;
> > > @@ -657,7 +663,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;
> > > @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct
> > > iommu_group *group, }
> > > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
> > >
> > > +int iommu_register_device_fault_handler(struct device *dev,
> > > + iommu_dev_fault_handler_t
> > > handler,
> > > + void *data)
> > > +{
> > > + struct iommu_param *idata = dev->iommu_param;
> > > +
> > > + /*
> > > + * Device iommu_param should have been allocated when
> > > device is
> > > + * added to its iommu_group.
> > > + */
> > > + if (!idata)
> > > + return -EINVAL;
> > > + /* Only allow one fault handler registered for each device
> > > */
> > > + if (idata->fault_param)
> > > + return -EBUSY;
> > > + get_device(dev);
> > > + idata->fault_param =
> > > + kzalloc(sizeof(struct iommu_fault_param),
> > > GFP_KERNEL);
> > > + if (!idata->fault_param)
> > > + return -ENOMEM;
> > > + idata->fault_param->handler = handler;
> > > + idata->fault_param->data = data;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> > > +
> > > +int iommu_unregister_device_fault_handler(struct device *dev)
> > > +{
> > > + struct iommu_param *idata = dev->iommu_param;
> > > +
> > > + if (!idata)
> > > + return -EINVAL;
> > > +
> > > + kfree(idata->fault_param);
> > > + idata->fault_param = NULL;
> > > + put_device(dev);
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
> > > +
> > > +
> > > +int iommu_report_device_fault(struct device *dev, struct
> > > iommu_fault_event *evt) +{
> > > + /* we only report device fault if there is a handler
> > > registered */
> > > + if (!dev->iommu_param || !dev->iommu_param->fault_param ||
> > > + !dev->iommu_param->fault_param->handler)
> > > + return -ENOSYS;
> > > +
> > > + return dev->iommu_param->fault_param->handler(evt,
> > > +
> > > dev->iommu_param->fault_param->data); +}
> > > +EXPORT_SYMBOL_GPL(iommu_report_device_fault);
> > > +
> >
> > Isn't this all rather racy? I see that we can have multiple callers
> > to register racing.
> I agree, should use a lock here to prevent unregister. For multiple
> caller race, it won't happen since there is only one caller can
> register handler.
If you have multiple simultaneous callers to
iommu_register_device_fault_handler, they can all get past the test
for fault_param (testing and setting is not atomic), then it's
indeterminate which handler gets installed. Thanks,
Alex
> > Unregister is buggy, allowing any caller to
> > decrement the device reference regardless of whether there's one
> > outstanding through this interface. The reporting callout can also
> > race with an unregistration. Might need a mutex on iommu_param to
> > avoid.
> >
> you are right, forgot to check outstanding handler. will add mutex also.
>
> Thanks,
> > > /**
> > > * 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 dfda89b..841c044 100644
> > > --- a/include/linux/iommu.h
> > > +++ b/include/linux/iommu.h
> > > @@ -463,6 +463,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 *); @@ -481,6
> > > +489,12 @@ extern void iommu_domain_window_disable(struct
> > > iommu_domain *domain, u32 wnd_nr) extern int
> > > report_iommu_fault(struct iommu_domain *domain, struct device *dev,
> > > unsigned long iova, int flags); +static inline bool
> > > iommu_has_device_fault_handler(struct device *dev) +{
> > > + return dev->iommu_param && dev->iommu_param->fault_param &&
> > > + dev->iommu_param->fault_param->handler;
> > > +}
> > > +
> >
> > This interface is racy by design, there's no guarantee that the
> > handler isn't immediately unregistered after this check. Thanks,
> >
> right, I will fold this check into report function and protect by a
> lock. I was trying to save some cycles but it would not work with the
> race condition.
> > Alex
> >
> > > static inline void iommu_flush_tlb_all(struct iommu_domain *domain)
> > > {
> > > if (domain->ops->flush_iotlb_all)
> > > @@ -734,6 +748,28 @@ 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 0;
> > > +}
> > > +
> > > +static inline int iommu_unregister_device_fault_handler(struct
> > > device *dev) +{
> > > + return 0;
> > > +}
> > > +
> > > +static inline bool iommu_has_device_fault_handler(struct device
> > > *dev) +{
> > > + return false;
> > > +}
> > > +
> > > +static inline int iommu_report_device_fault(struct device *dev,
> > > struct iommu_fault_event *evt) +{
> > > + return 0;
> > > +}
> > > +
> > > static inline int iommu_group_id(struct iommu_group *group)
> > > {
> > > return -ENODEV;
> >
>
> [Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread[parent not found: <20171208135909.0ee02d16-1yVPhWWZRC1BDLzU/O5InQ@public.gmane.org>]
* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-12-08 20:59 ` Alex Williamson
@ 2017-12-08 21:22 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-12-08 21:22 UTC (permalink / raw)
To: Alex Williamson
Cc: Lan Tianyu, Greg Kroah-Hartman, Rafael Wysocki, LKML,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Jean Delvare,
David Woodhouse
On Fri, 8 Dec 2017 13:59:09 -0700
Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > >
> > > Isn't this all rather racy? I see that we can have multiple
> > > callers to register racing.
> > I agree, should use a lock here to prevent unregister. For multiple
> > caller race, it won't happen since there is only one caller can
> > register handler.
>
> If you have multiple simultaneous callers to
> iommu_register_device_fault_handler, they can all get past the test
> for fault_param (testing and setting is not atomic), then it's
> indeterminate which handler gets installed. Thanks,
>
I see, having the mutex would prevent it. Later callers would get
-EBUSY.
Thanks a lot!
> Alex
^ permalink raw reply [flat|nested] 94+ messages in thread
* Re: [PATCH v3 10/16] iommu: introduce device fault report API
@ 2017-12-08 21:22 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-12-08 21:22 UTC (permalink / raw)
To: Alex Williamson
Cc: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok,
Jean Delvare, Christoph Hellwig, jacob.jun.pan
On Fri, 8 Dec 2017 13:59:09 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:
> > >
> > > Isn't this all rather racy? I see that we can have multiple
> > > callers to register racing.
> > I agree, should use a lock here to prevent unregister. For multiple
> > caller race, it won't happen since there is only one caller can
> > register handler.
>
> If you have multiple simultaneous callers to
> iommu_register_device_fault_handler, they can all get past the test
> for fault_param (testing and setting is not atomic), then it's
> indeterminate which handler gets installed. Thanks,
>
I see, having the mutex would prevent it. Later callers would get
-EBUSY.
Thanks a lot!
> Alex
^ permalink raw reply [flat|nested] 94+ messages in thread
* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-11-17 18:55 ` Jacob Pan
` (2 preceding siblings ...)
(?)
@ 2018-01-10 12:39 ` Jean-Philippe Brucker
-1 siblings, 0 replies; 94+ messages in thread
From: Jean-Philippe Brucker @ 2018-01-10 12:39 UTC (permalink / raw)
To: Jacob Pan, iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
On 17/11/17 18:55, Jacob Pan wrote:
[...]
> +static inline int iommu_register_device_fault_handler(struct device *dev,
> + iommu_dev_fault_handler_t handler,
> + void *data)
> +{
> + return 0;> +}
> +
> +static inline int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static inline bool iommu_has_device_fault_handler(struct device *dev)
> +{
> + return false;
> +}
> +
> +static inline int iommu_report_device_fault(struct device *dev, struct iommu_fault_event *evt)
> +{
> + return 0;
> +}
Not too important but these stubs, when CONFIG_IOMMU_API is disabled,
usually return an error value (-ENODEV) instead of 0.
Thanks,
Jean
> +
> static inline int iommu_group_id(struct iommu_group *group)
> {
> return -ENODEV;
>
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2017-11-17 18:55 ` Jacob Pan
` (3 preceding siblings ...)
(?)
@ 2018-01-18 19:24 ` Jean-Philippe Brucker
2018-01-23 20:01 ` Jacob Pan
-1 siblings, 1 reply; 94+ messages in thread
From: Jean-Philippe Brucker @ 2018-01-18 19:24 UTC (permalink / raw)
To: Jacob Pan, iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu
Hi Jacob,
I've got minor comments after working with this patch, sorry for the
multiple replies
On 17/11/17 18:55, Jacob Pan wrote:
[...]
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 829e9e9..97b7990 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group *group, struct device *dev)
> goto err_free_name;
> }
>
> + dev->iommu_param = kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
This should be "sizeof(struct iommu_param)" or maybe
"sizeof(*dev->iommu_param)".
> + if (!dev->iommu_param) {
> + ret = -ENOMEM;
> + goto err_free_name;
> + }
> +
> kobject_get(group->devices_kobj);
>
> dev->iommu_group = group;
> @@ -657,7 +663,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;
> @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct iommu_group *group,
> }
> EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
>
> +int iommu_register_device_fault_handler(struct device *dev,
> + iommu_dev_fault_handler_t handler,
> + void *data)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + /*
> + * Device iommu_param should have been allocated when device is
> + * added to its iommu_group.
> + */
> + if (!idata)
> + return -EINVAL;
> + /* Only allow one fault handler registered for each device */
> + if (idata->fault_param)
> + return -EBUSY;
> + get_device(dev);
> + idata->fault_param =
> + kzalloc(sizeof(struct iommu_fault_param), GFP_KERNEL);
> + if (!idata->fault_param)
> + return -ENOMEM;
> + idata->fault_param->handler = handler;
> + idata->fault_param->data = data;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> +
> +int iommu_unregister_device_fault_handler(struct device *dev)
> +{
> + struct iommu_param *idata = dev->iommu_param;
> +
> + if (!idata)
> + return -EINVAL;
> +
> + kfree(idata->fault_param);
> + idata->fault_param = NULL;
> + put_device(dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
We should probably document register() and unregister() functions since
they are part of the device driver API. If it helps I came up with:
/**
* 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. If the fault is
* recoverable (IOMMU_FAULT_PAGE_REQ), the handler must 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.
*/
/**
* 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.
*/
Thanks,
Jean
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 10/16] iommu: introduce device fault report API
2018-01-18 19:24 ` Jean-Philippe Brucker
@ 2018-01-23 20:01 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2018-01-23 20:01 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson, Lan Tianyu, jacob.jun.pan
On Thu, 18 Jan 2018 19:24:52 +0000
Jean-Philippe Brucker <jean-philippe.brucker@arm.com> wrote:
> Hi Jacob,
>
> I've got minor comments after working with this patch, sorry for the
> multiple replies
>
> On 17/11/17 18:55, Jacob Pan wrote:
> [...]
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 829e9e9..97b7990 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -581,6 +581,12 @@ int iommu_group_add_device(struct iommu_group
> > *group, struct device *dev) goto err_free_name;
> > }
> >
> > + dev->iommu_param = kzalloc(sizeof(struct
> > iommu_fault_param), GFP_KERNEL);
>
> This should be "sizeof(struct iommu_param)" or maybe
> "sizeof(*dev->iommu_param)".
>
good catch, thanks,
> > + if (!dev->iommu_param) {
> > + ret = -ENOMEM;
> > + goto err_free_name;
> > + }
> > +
> > kobject_get(group->devices_kobj);
> >
> > dev->iommu_group = group;
> > @@ -657,7 +663,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;
> > @@ -791,6 +797,61 @@ int iommu_group_unregister_notifier(struct
> > iommu_group *group, }
> > EXPORT_SYMBOL_GPL(iommu_group_unregister_notifier);
> >
> > +int iommu_register_device_fault_handler(struct device *dev,
> > + iommu_dev_fault_handler_t
> > handler,
> > + void *data)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + /*
> > + * Device iommu_param should have been allocated when
> > device is
> > + * added to its iommu_group.
> > + */
> > + if (!idata)
> > + return -EINVAL;
> > + /* Only allow one fault handler registered for each device
> > */
> > + if (idata->fault_param)
> > + return -EBUSY;
> > + get_device(dev);
> > + idata->fault_param =
> > + kzalloc(sizeof(struct iommu_fault_param),
> > GFP_KERNEL);
> > + if (!idata->fault_param)
> > + return -ENOMEM;
> > + idata->fault_param->handler = handler;
> > + idata->fault_param->data = data;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_register_device_fault_handler);
> > +
> > +int iommu_unregister_device_fault_handler(struct device *dev)
> > +{
> > + struct iommu_param *idata = dev->iommu_param;
> > +
> > + if (!idata)
> > + return -EINVAL;
> > +
> > + kfree(idata->fault_param);
> > + idata->fault_param = NULL;
> > + put_device(dev);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(iommu_unregister_device_fault_handler);
>
> We should probably document register() and unregister() functions
> since they are part of the device driver API. If it helps I came up
> with:
>
> /**
> * 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. If the fault is
> * recoverable (IOMMU_FAULT_PAGE_REQ), the handler must 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. */
>
> /**
> * 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.
> */
>
agreed. thanks. sorry about the delay.
> Thanks,
> Jean
[Jacob Pan]
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 11/16] iommu/vt-d: use threaded irq for dmar_fault
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
Currently, dmar fault IRQ handler does nothing more than rate
limited printk, no critical hardware handling need to be done
in IRQ context.
Convert it to threaded IRQ would allow fault processing that
requires process context. e.g. find out offending device based
on source ID in the fault rasons.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/dmar.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index f69f6ee..38ee91b 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1749,7 +1749,8 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
return -EINVAL;
}
- ret = request_irq(irq, dmar_fault, IRQF_NO_THREAD, iommu->name, iommu);
+ ret = request_threaded_irq(irq, NULL, dmar_fault,
+ IRQF_ONESHOT, iommu->name, iommu);
if (ret)
pr_err("Can't request irq\n");
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 11/16] iommu/vt-d: use threaded irq for dmar_fault
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
Currently, dmar fault IRQ handler does nothing more than rate
limited printk, no critical hardware handling need to be done
in IRQ context.
Convert it to threaded IRQ would allow fault processing that
requires process context. e.g. find out offending device based
on source ID in the fault rasons.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/dmar.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index f69f6ee..38ee91b 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1749,7 +1749,8 @@ int dmar_set_interrupt(struct intel_iommu *iommu)
return -EINVAL;
}
- ret = request_irq(irq, dmar_fault, IRQF_NO_THREAD, iommu->name, iommu);
+ ret = request_threaded_irq(irq, NULL, dmar_fault,
+ IRQF_ONESHOT, iommu->name, iommu);
if (ret)
pr_err("Can't request irq\n");
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread
* [PATCH v3 12/16] iommu/vt-d: report unrecoverable device faults
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Yi L, Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY,
Jean Delvare
Currently, when device DMA faults are detected by IOMMU the fault
reasons are printed but the driver of the offending device is
involved in fault handling.
This patch uses per device fault reporting API to send fault event
data for further processing.
Offending device is identified by the source ID in VT-d fault reason
report registers.
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/dmar.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 38ee91b..b1f67fc2 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1555,6 +1555,31 @@ static const char *irq_remap_fault_reasons[] =
"Blocked an interrupt request due to source-id verification failure",
};
+/* fault data and status */
+enum intel_iommu_fault_reason {
+ INTEL_IOMMU_FAULT_REASON_SW,
+ INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID,
+ INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH,
+ INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS,
+ INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS,
+ INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID,
+ INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_RTP,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_CTP,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_PTE,
+ NR_INTEL_IOMMU_FAULT_REASON,
+};
+
+/* fault reasons that are allowed to be reported outside IOMMU subsystem */
+#define INTEL_IOMMU_FAULT_REASON_ALLOWED \
+ ((1ULL << INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH) | \
+ (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS) | \
+ (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS))
+
+
static const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
{
if (fault_reason >= 0x20 && (fault_reason - 0x20 <
@@ -1635,6 +1660,69 @@ void dmar_msi_read(int irq, struct msi_msg *msg)
raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
}
+static enum iommu_fault_reason to_iommu_fault_reason(u8 reason)
+{
+ if (reason >= NR_INTEL_IOMMU_FAULT_REASON) {
+ pr_warn("unknown DMAR fault reason %d\n", reason);
+ return IOMMU_FAULT_REASON_UNKNOWN;
+ }
+ switch (reason) {
+ case INTEL_IOMMU_FAULT_REASON_SW:
+ case INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH:
+ case INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID:
+ return IOMMU_FAULT_REASON_INTERNAL;
+ case INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS:
+ case INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS:
+ return IOMMU_FAULT_REASON_PERMISSION;
+ default:
+ return IOMMU_FAULT_REASON_UNKNOWN;
+ }
+}
+
+static void report_fault_to_device(struct intel_iommu *iommu, u64 addr, int type,
+ int fault_type, enum intel_iommu_fault_reason reason, u16 sid)
+{
+ struct iommu_fault_event event;
+ struct pci_dev *pdev;
+ u8 bus, devfn;
+
+ /* check if fault reason is worth reporting outside IOMMU */
+ if (!((1 << reason) & INTEL_IOMMU_FAULT_REASON_ALLOWED)) {
+ pr_debug("Fault reason %d not allowed to report to device\n",
+ reason);
+ return;
+ }
+
+ bus = PCI_BUS_NUM(sid);
+ devfn = PCI_DEVFN(PCI_SLOT(sid), PCI_FUNC(sid));
+ /*
+ * we need to check if the fault reporting is requested for the
+ * offending device.
+ */
+ pdev = pci_get_bus_and_slot(bus, devfn);
+ if (!pdev) {
+ pr_warn("No PCI device found for source ID %x\n", sid);
+ return;
+ }
+ /*
+ * unrecoverable fault is reported per IOMMU, notifier handler can
+ * resolve PCI device based on source ID.
+ */
+ event.reason = to_iommu_fault_reason(reason);
+ event.addr = addr;
+ event.type = IOMMU_FAULT_DMA_UNRECOV;
+ event.prot = type ? IOMMU_READ : IOMMU_WRITE;
+ dev_warn(&pdev->dev, "report device unrecoverable fault: %d, %x, %d\n",
+ event.reason, sid, event.type);
+ iommu_report_device_fault(&pdev->dev, &event);
+ pci_dev_put(pdev);
+}
+
static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
u8 fault_reason, u16 source_id, unsigned long long addr)
{
@@ -1648,11 +1736,15 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
source_id >> 8, PCI_SLOT(source_id & 0xFF),
PCI_FUNC(source_id & 0xFF), addr >> 48,
fault_reason, reason);
- else
+ else {
pr_err("[%s] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02d] %s\n",
type ? "DMA Read" : "DMA Write",
source_id >> 8, PCI_SLOT(source_id & 0xFF),
PCI_FUNC(source_id & 0xFF), addr, fault_reason, reason);
+ }
+ report_fault_to_device(iommu, addr, type, fault_type,
+ fault_reason, source_id);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 12/16] iommu/vt-d: report unrecoverable device faults
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan, Liu, Yi L
Currently, when device DMA faults are detected by IOMMU the fault
reasons are printed but the driver of the offending device is
involved in fault handling.
This patch uses per device fault reporting API to send fault event
data for further processing.
Offending device is identified by the source ID in VT-d fault reason
report registers.
Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
drivers/iommu/dmar.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 93 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 38ee91b..b1f67fc2 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1555,6 +1555,31 @@ static const char *irq_remap_fault_reasons[] =
"Blocked an interrupt request due to source-id verification failure",
};
+/* fault data and status */
+enum intel_iommu_fault_reason {
+ INTEL_IOMMU_FAULT_REASON_SW,
+ INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID,
+ INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH,
+ INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS,
+ INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS,
+ INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID,
+ INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID,
+ INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_RTP,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_CTP,
+ INTEL_IOMMU_FAULT_REASON_NONE_ZERO_PTE,
+ NR_INTEL_IOMMU_FAULT_REASON,
+};
+
+/* fault reasons that are allowed to be reported outside IOMMU subsystem */
+#define INTEL_IOMMU_FAULT_REASON_ALLOWED \
+ ((1ULL << INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH) | \
+ (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS) | \
+ (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS))
+
+
static const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
{
if (fault_reason >= 0x20 && (fault_reason - 0x20 <
@@ -1635,6 +1660,69 @@ void dmar_msi_read(int irq, struct msi_msg *msg)
raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
}
+static enum iommu_fault_reason to_iommu_fault_reason(u8 reason)
+{
+ if (reason >= NR_INTEL_IOMMU_FAULT_REASON) {
+ pr_warn("unknown DMAR fault reason %d\n", reason);
+ return IOMMU_FAULT_REASON_UNKNOWN;
+ }
+ switch (reason) {
+ case INTEL_IOMMU_FAULT_REASON_SW:
+ case INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH:
+ case INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID:
+ return IOMMU_FAULT_REASON_INTERNAL;
+ case INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID:
+ case INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS:
+ case INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS:
+ return IOMMU_FAULT_REASON_PERMISSION;
+ default:
+ return IOMMU_FAULT_REASON_UNKNOWN;
+ }
+}
+
+static void report_fault_to_device(struct intel_iommu *iommu, u64 addr, int type,
+ int fault_type, enum intel_iommu_fault_reason reason, u16 sid)
+{
+ struct iommu_fault_event event;
+ struct pci_dev *pdev;
+ u8 bus, devfn;
+
+ /* check if fault reason is worth reporting outside IOMMU */
+ if (!((1 << reason) & INTEL_IOMMU_FAULT_REASON_ALLOWED)) {
+ pr_debug("Fault reason %d not allowed to report to device\n",
+ reason);
+ return;
+ }
+
+ bus = PCI_BUS_NUM(sid);
+ devfn = PCI_DEVFN(PCI_SLOT(sid), PCI_FUNC(sid));
+ /*
+ * we need to check if the fault reporting is requested for the
+ * offending device.
+ */
+ pdev = pci_get_bus_and_slot(bus, devfn);
+ if (!pdev) {
+ pr_warn("No PCI device found for source ID %x\n", sid);
+ return;
+ }
+ /*
+ * unrecoverable fault is reported per IOMMU, notifier handler can
+ * resolve PCI device based on source ID.
+ */
+ event.reason = to_iommu_fault_reason(reason);
+ event.addr = addr;
+ event.type = IOMMU_FAULT_DMA_UNRECOV;
+ event.prot = type ? IOMMU_READ : IOMMU_WRITE;
+ dev_warn(&pdev->dev, "report device unrecoverable fault: %d, %x, %d\n",
+ event.reason, sid, event.type);
+ iommu_report_device_fault(&pdev->dev, &event);
+ pci_dev_put(pdev);
+}
+
static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
u8 fault_reason, u16 source_id, unsigned long long addr)
{
@@ -1648,11 +1736,15 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
source_id >> 8, PCI_SLOT(source_id & 0xFF),
PCI_FUNC(source_id & 0xFF), addr >> 48,
fault_reason, reason);
- else
+ else {
pr_err("[%s] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02d] %s\n",
type ? "DMA Read" : "DMA Write",
source_id >> 8, PCI_SLOT(source_id & 0xFF),
PCI_FUNC(source_id & 0xFF), addr, fault_reason, reason);
+ }
+ report_fault_to_device(iommu, addr, type, fault_type,
+ fault_reason, source_id);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread[parent not found: <1510944914-54430-13-git-send-email-jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v3 12/16] iommu/vt-d: report unrecoverable device faults
2017-11-17 18:55 ` Jacob Pan
@ 2017-12-05 6:34 ` Lu Baolu
-1 siblings, 0 replies; 94+ messages in thread
From: Lu Baolu @ 2017-12-05 6:34 UTC (permalink / raw)
To: Jacob Pan, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Lan Tianyu, Yi L, Jean Delvare,
Liu-i9wRM+HIrmnmtl4Z8vJ8Kg761KYD1DLY
Hi,
On 11/18/2017 02:55 AM, Jacob Pan wrote:
> Currently, when device DMA faults are detected by IOMMU the fault
> reasons are printed but the driver of the offending device is
"... but the driver of the offending device is not involved in ..."
Best regards,
Lu Baolu
> involved in fault handling.
> This patch uses per device fault reporting API to send fault event
> data for further processing.
> Offending device is identified by the source ID in VT-d fault reason
> report registers.
>
> Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/iommu/dmar.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 38ee91b..b1f67fc2 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1555,6 +1555,31 @@ static const char *irq_remap_fault_reasons[] =
> "Blocked an interrupt request due to source-id verification failure",
> };
>
> +/* fault data and status */
> +enum intel_iommu_fault_reason {
> + INTEL_IOMMU_FAULT_REASON_SW,
> + INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID,
> + INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH,
> + INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS,
> + INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS,
> + INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID,
> + INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_RTP,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_CTP,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_PTE,
> + NR_INTEL_IOMMU_FAULT_REASON,
> +};
> +
> +/* fault reasons that are allowed to be reported outside IOMMU subsystem */
> +#define INTEL_IOMMU_FAULT_REASON_ALLOWED \
> + ((1ULL << INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH) | \
> + (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS) | \
> + (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS))
> +
> +
> static const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
> {
> if (fault_reason >= 0x20 && (fault_reason - 0x20 <
> @@ -1635,6 +1660,69 @@ void dmar_msi_read(int irq, struct msi_msg *msg)
> raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
> }
>
> +static enum iommu_fault_reason to_iommu_fault_reason(u8 reason)
> +{
> + if (reason >= NR_INTEL_IOMMU_FAULT_REASON) {
> + pr_warn("unknown DMAR fault reason %d\n", reason);
> + return IOMMU_FAULT_REASON_UNKNOWN;
> + }
> + switch (reason) {
> + case INTEL_IOMMU_FAULT_REASON_SW:
> + case INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH:
> + case INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID:
> + return IOMMU_FAULT_REASON_INTERNAL;
> + case INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS:
> + case INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS:
> + return IOMMU_FAULT_REASON_PERMISSION;
> + default:
> + return IOMMU_FAULT_REASON_UNKNOWN;
> + }
> +}
> +
> +static void report_fault_to_device(struct intel_iommu *iommu, u64 addr, int type,
> + int fault_type, enum intel_iommu_fault_reason reason, u16 sid)
> +{
> + struct iommu_fault_event event;
> + struct pci_dev *pdev;
> + u8 bus, devfn;
> +
> + /* check if fault reason is worth reporting outside IOMMU */
> + if (!((1 << reason) & INTEL_IOMMU_FAULT_REASON_ALLOWED)) {
> + pr_debug("Fault reason %d not allowed to report to device\n",
> + reason);
> + return;
> + }
> +
> + bus = PCI_BUS_NUM(sid);
> + devfn = PCI_DEVFN(PCI_SLOT(sid), PCI_FUNC(sid));
> + /*
> + * we need to check if the fault reporting is requested for the
> + * offending device.
> + */
> + pdev = pci_get_bus_and_slot(bus, devfn);
> + if (!pdev) {
> + pr_warn("No PCI device found for source ID %x\n", sid);
> + return;
> + }
> + /*
> + * unrecoverable fault is reported per IOMMU, notifier handler can
> + * resolve PCI device based on source ID.
> + */
> + event.reason = to_iommu_fault_reason(reason);
> + event.addr = addr;
> + event.type = IOMMU_FAULT_DMA_UNRECOV;
> + event.prot = type ? IOMMU_READ : IOMMU_WRITE;
> + dev_warn(&pdev->dev, "report device unrecoverable fault: %d, %x, %d\n",
> + event.reason, sid, event.type);
> + iommu_report_device_fault(&pdev->dev, &event);
> + pci_dev_put(pdev);
> +}
> +
> static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
> u8 fault_reason, u16 source_id, unsigned long long addr)
> {
> @@ -1648,11 +1736,15 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
> source_id >> 8, PCI_SLOT(source_id & 0xFF),
> PCI_FUNC(source_id & 0xFF), addr >> 48,
> fault_reason, reason);
> - else
> + else {
> pr_err("[%s] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02d] %s\n",
> type ? "DMA Read" : "DMA Write",
> source_id >> 8, PCI_SLOT(source_id & 0xFF),
> PCI_FUNC(source_id & 0xFF), addr, fault_reason, reason);
> + }
> + report_fault_to_device(iommu, addr, type, fault_type,
> + fault_reason, source_id);
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 12/16] iommu/vt-d: report unrecoverable device faults
@ 2017-12-05 6:34 ` Lu Baolu
0 siblings, 0 replies; 94+ messages in thread
From: Lu Baolu @ 2017-12-05 6:34 UTC (permalink / raw)
To: Jacob Pan, iommu, LKML, Joerg Roedel, David Woodhouse,
Greg Kroah-Hartman, Rafael Wysocki, Alex Williamson
Cc: Lan Tianyu, Yi L, Liu, Jean Delvare
Hi,
On 11/18/2017 02:55 AM, Jacob Pan wrote:
> Currently, when device DMA faults are detected by IOMMU the fault
> reasons are printed but the driver of the offending device is
"... but the driver of the offending device is not involved in ..."
Best regards,
Lu Baolu
> involved in fault handling.
> This patch uses per device fault reporting API to send fault event
> data for further processing.
> Offending device is identified by the source ID in VT-d fault reason
> report registers.
>
> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> ---
> drivers/iommu/dmar.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
> index 38ee91b..b1f67fc2 100644
> --- a/drivers/iommu/dmar.c
> +++ b/drivers/iommu/dmar.c
> @@ -1555,6 +1555,31 @@ static const char *irq_remap_fault_reasons[] =
> "Blocked an interrupt request due to source-id verification failure",
> };
>
> +/* fault data and status */
> +enum intel_iommu_fault_reason {
> + INTEL_IOMMU_FAULT_REASON_SW,
> + INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID,
> + INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH,
> + INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS,
> + INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS,
> + INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID,
> + INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID,
> + INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_RTP,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_CTP,
> + INTEL_IOMMU_FAULT_REASON_NONE_ZERO_PTE,
> + NR_INTEL_IOMMU_FAULT_REASON,
> +};
> +
> +/* fault reasons that are allowed to be reported outside IOMMU subsystem */
> +#define INTEL_IOMMU_FAULT_REASON_ALLOWED \
> + ((1ULL << INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH) | \
> + (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS) | \
> + (1ULL << INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS))
> +
> +
> static const char *dmar_get_fault_reason(u8 fault_reason, int *fault_type)
> {
> if (fault_reason >= 0x20 && (fault_reason - 0x20 <
> @@ -1635,6 +1660,69 @@ void dmar_msi_read(int irq, struct msi_msg *msg)
> raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
> }
>
> +static enum iommu_fault_reason to_iommu_fault_reason(u8 reason)
> +{
> + if (reason >= NR_INTEL_IOMMU_FAULT_REASON) {
> + pr_warn("unknown DMAR fault reason %d\n", reason);
> + return IOMMU_FAULT_REASON_UNKNOWN;
> + }
> + switch (reason) {
> + case INTEL_IOMMU_FAULT_REASON_SW:
> + case INTEL_IOMMU_FAULT_REASON_ROOT_NOT_PRESENT:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_NOT_PRESENT:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_BEYOND_ADDR_WIDTH:
> + case INTEL_IOMMU_FAULT_REASON_ROOT_ADDR_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_CONTEXT_PTR_INVALID:
> + return IOMMU_FAULT_REASON_INTERNAL;
> + case INTEL_IOMMU_FAULT_REASON_NEXT_PT_INVALID:
> + case INTEL_IOMMU_FAULT_REASON_PTE_WRITE_ACCESS:
> + case INTEL_IOMMU_FAULT_REASON_PTE_READ_ACCESS:
> + return IOMMU_FAULT_REASON_PERMISSION;
> + default:
> + return IOMMU_FAULT_REASON_UNKNOWN;
> + }
> +}
> +
> +static void report_fault_to_device(struct intel_iommu *iommu, u64 addr, int type,
> + int fault_type, enum intel_iommu_fault_reason reason, u16 sid)
> +{
> + struct iommu_fault_event event;
> + struct pci_dev *pdev;
> + u8 bus, devfn;
> +
> + /* check if fault reason is worth reporting outside IOMMU */
> + if (!((1 << reason) & INTEL_IOMMU_FAULT_REASON_ALLOWED)) {
> + pr_debug("Fault reason %d not allowed to report to device\n",
> + reason);
> + return;
> + }
> +
> + bus = PCI_BUS_NUM(sid);
> + devfn = PCI_DEVFN(PCI_SLOT(sid), PCI_FUNC(sid));
> + /*
> + * we need to check if the fault reporting is requested for the
> + * offending device.
> + */
> + pdev = pci_get_bus_and_slot(bus, devfn);
> + if (!pdev) {
> + pr_warn("No PCI device found for source ID %x\n", sid);
> + return;
> + }
> + /*
> + * unrecoverable fault is reported per IOMMU, notifier handler can
> + * resolve PCI device based on source ID.
> + */
> + event.reason = to_iommu_fault_reason(reason);
> + event.addr = addr;
> + event.type = IOMMU_FAULT_DMA_UNRECOV;
> + event.prot = type ? IOMMU_READ : IOMMU_WRITE;
> + dev_warn(&pdev->dev, "report device unrecoverable fault: %d, %x, %d\n",
> + event.reason, sid, event.type);
> + iommu_report_device_fault(&pdev->dev, &event);
> + pci_dev_put(pdev);
> +}
> +
> static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
> u8 fault_reason, u16 source_id, unsigned long long addr)
> {
> @@ -1648,11 +1736,15 @@ static int dmar_fault_do_one(struct intel_iommu *iommu, int type,
> source_id >> 8, PCI_SLOT(source_id & 0xFF),
> PCI_FUNC(source_id & 0xFF), addr >> 48,
> fault_reason, reason);
> - else
> + else {
> pr_err("[%s] Request device [%02x:%02x.%d] fault addr %llx [fault reason %02d] %s\n",
> type ? "DMA Read" : "DMA Write",
> source_id >> 8, PCI_SLOT(source_id & 0xFF),
> PCI_FUNC(source_id & 0xFF), addr, fault_reason, reason);
> + }
> + report_fault_to_device(iommu, addr, type, fault_type,
> + fault_reason, source_id);
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 13/16] iommu/intel-svm: notify page request to guest
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
If the source device of a page request has its PASID table pointer
bond to a guest, the first level page tables are owned by the guest.
In this case, we shall let guest OS to manage page fault.
This patch uses the IOMMU fault notification API to send notifications,
possibly via VFIO, to the guest OS. Once guest pages are fault in, guest
will issue page response which will be passed down via the invalidation
passdown APIs.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
drivers/iommu/intel-svm.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
include/linux/iommu.h | 1 +
2 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index f6697e5..77c25d8 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -555,6 +555,71 @@ static bool is_canonical_address(u64 addr)
return (((saddr << shift) >> shift) == saddr);
}
+static int prq_to_iommu_prot(struct page_req_dsc *req)
+{
+ int prot = 0;
+
+ if (req->rd_req)
+ prot |= IOMMU_FAULT_READ;
+ if (req->wr_req)
+ prot |= IOMMU_FAULT_WRITE;
+ if (req->exe_req)
+ prot |= IOMMU_FAULT_EXEC;
+ if (req->priv_req)
+ prot |= IOMMU_FAULT_PRIV;
+
+ return prot;
+}
+
+static int intel_svm_prq_report(struct device *dev, struct page_req_dsc *desc)
+{
+ int ret = 0;
+ struct iommu_fault_event event;
+ struct pci_dev *pdev;
+
+ /**
+ * If caller does not provide struct device, this is the case where
+ * guest PASID table is bound to the device. So we need to retrieve
+ * struct device from the page request descriptor then proceed.
+ */
+ if (!dev) {
+ pdev = pci_get_bus_and_slot(desc->bus, desc->devfn);
+ if (!pdev) {
+ pr_err("No PCI device found for PRQ [%02x:%02x.%d]\n",
+ desc->bus, PCI_SLOT(desc->devfn),
+ PCI_FUNC(desc->devfn));
+ return -ENODEV;
+ }
+ dev = &pdev->dev;
+ } else if (dev_is_pci(dev)) {
+ pdev = to_pci_dev(dev);
+ pci_dev_get(pdev);
+ } else
+ return -ENODEV;
+
+ pr_debug("Notify PRQ device [%02x:%02x.%d]\n",
+ desc->bus, PCI_SLOT(desc->devfn),
+ PCI_FUNC(desc->devfn));
+
+ /* invoke device fault handler if registered */
+ if (iommu_has_device_fault_handler(dev)) {
+ /* Fill in event data for device specific processing */
+ event.type = IOMMU_FAULT_PAGE_REQ;
+ event.addr = desc->addr;
+ event.pasid = desc->pasid;
+ event.page_req_group_id = desc->prg_index;
+ event.prot = prq_to_iommu_prot(desc);
+ event.last_req = desc->lpig;
+ event.pasid_valid = 1;
+ event.iommu_private = desc->private;
+ ret = iommu_report_device_fault(&pdev->dev, &event);
+ }
+
+ pci_dev_put(pdev);
+
+ return ret;
+}
+
static irqreturn_t prq_event_thread(int irq, void *d)
{
struct intel_iommu *iommu = d;
@@ -578,7 +643,12 @@ static irqreturn_t prq_event_thread(int irq, void *d)
handled = 1;
req = &iommu->prq[head / sizeof(*req)];
-
+ /**
+ * If prq is to be handled outside iommu driver via receiver of
+ * the fault notifiers, we skip the page response here.
+ */
+ if (!intel_svm_prq_report(NULL, req))
+ goto prq_advance;
result = QI_RESP_FAILURE;
address = (u64)req->addr << VTD_PAGE_SHIFT;
if (!req->pasid_present) {
@@ -649,11 +719,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
if (WARN_ON(&sdev->list == &svm->devs))
sdev = NULL;
- if (sdev && sdev->ops && sdev->ops->fault_cb) {
- int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
- (req->exe_req << 1) | (req->priv_req);
- sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result);
- }
+ intel_svm_prq_report(sdev->dev, req);
/* We get here in the error case where the PASID lookup failed,
and these can be NULL. Do not use them below this point! */
sdev = NULL;
@@ -679,7 +745,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
qi_submit_sync(&resp, iommu);
}
-
+ prq_advance:
head = (head + sizeof(*req)) & PRQ_RING_MASK;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 841c044..3083796b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -42,6 +42,7 @@
* if the IOMMU page table format is equivalent.
*/
#define IOMMU_PRIV (1 << 5)
+#define IOMMU_EXEC (1 << 6)
struct iommu_ops;
struct iommu_group;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 13/16] iommu/intel-svm: notify page request to guest
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
If the source device of a page request has its PASID table pointer
bond to a guest, the first level page tables are owned by the guest.
In this case, we shall let guest OS to manage page fault.
This patch uses the IOMMU fault notification API to send notifications,
possibly via VFIO, to the guest OS. Once guest pages are fault in, guest
will issue page response which will be passed down via the invalidation
passdown APIs.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Ashok Raj <ashok.raj@intel.com>
---
drivers/iommu/intel-svm.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
include/linux/iommu.h | 1 +
2 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index f6697e5..77c25d8 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -555,6 +555,71 @@ static bool is_canonical_address(u64 addr)
return (((saddr << shift) >> shift) == saddr);
}
+static int prq_to_iommu_prot(struct page_req_dsc *req)
+{
+ int prot = 0;
+
+ if (req->rd_req)
+ prot |= IOMMU_FAULT_READ;
+ if (req->wr_req)
+ prot |= IOMMU_FAULT_WRITE;
+ if (req->exe_req)
+ prot |= IOMMU_FAULT_EXEC;
+ if (req->priv_req)
+ prot |= IOMMU_FAULT_PRIV;
+
+ return prot;
+}
+
+static int intel_svm_prq_report(struct device *dev, struct page_req_dsc *desc)
+{
+ int ret = 0;
+ struct iommu_fault_event event;
+ struct pci_dev *pdev;
+
+ /**
+ * If caller does not provide struct device, this is the case where
+ * guest PASID table is bound to the device. So we need to retrieve
+ * struct device from the page request descriptor then proceed.
+ */
+ if (!dev) {
+ pdev = pci_get_bus_and_slot(desc->bus, desc->devfn);
+ if (!pdev) {
+ pr_err("No PCI device found for PRQ [%02x:%02x.%d]\n",
+ desc->bus, PCI_SLOT(desc->devfn),
+ PCI_FUNC(desc->devfn));
+ return -ENODEV;
+ }
+ dev = &pdev->dev;
+ } else if (dev_is_pci(dev)) {
+ pdev = to_pci_dev(dev);
+ pci_dev_get(pdev);
+ } else
+ return -ENODEV;
+
+ pr_debug("Notify PRQ device [%02x:%02x.%d]\n",
+ desc->bus, PCI_SLOT(desc->devfn),
+ PCI_FUNC(desc->devfn));
+
+ /* invoke device fault handler if registered */
+ if (iommu_has_device_fault_handler(dev)) {
+ /* Fill in event data for device specific processing */
+ event.type = IOMMU_FAULT_PAGE_REQ;
+ event.addr = desc->addr;
+ event.pasid = desc->pasid;
+ event.page_req_group_id = desc->prg_index;
+ event.prot = prq_to_iommu_prot(desc);
+ event.last_req = desc->lpig;
+ event.pasid_valid = 1;
+ event.iommu_private = desc->private;
+ ret = iommu_report_device_fault(&pdev->dev, &event);
+ }
+
+ pci_dev_put(pdev);
+
+ return ret;
+}
+
static irqreturn_t prq_event_thread(int irq, void *d)
{
struct intel_iommu *iommu = d;
@@ -578,7 +643,12 @@ static irqreturn_t prq_event_thread(int irq, void *d)
handled = 1;
req = &iommu->prq[head / sizeof(*req)];
-
+ /**
+ * If prq is to be handled outside iommu driver via receiver of
+ * the fault notifiers, we skip the page response here.
+ */
+ if (!intel_svm_prq_report(NULL, req))
+ goto prq_advance;
result = QI_RESP_FAILURE;
address = (u64)req->addr << VTD_PAGE_SHIFT;
if (!req->pasid_present) {
@@ -649,11 +719,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
if (WARN_ON(&sdev->list == &svm->devs))
sdev = NULL;
- if (sdev && sdev->ops && sdev->ops->fault_cb) {
- int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
- (req->exe_req << 1) | (req->priv_req);
- sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result);
- }
+ intel_svm_prq_report(sdev->dev, req);
/* We get here in the error case where the PASID lookup failed,
and these can be NULL. Do not use them below this point! */
sdev = NULL;
@@ -679,7 +745,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
qi_submit_sync(&resp, iommu);
}
-
+ prq_advance:
head = (head + sizeof(*req)) & PRQ_RING_MASK;
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 841c044..3083796b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -42,6 +42,7 @@
* if the IOMMU page table format is equivalent.
*/
#define IOMMU_PRIV (1 << 5)
+#define IOMMU_EXEC (1 << 6)
struct iommu_ops;
struct iommu_group;
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread[parent not found: <1510944914-54430-14-git-send-email-jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: [PATCH v3 13/16] iommu/intel-svm: notify page request to guest
2017-11-17 18:55 ` Jacob Pan
@ 2017-12-05 7:37 ` Lu Baolu
-1 siblings, 0 replies; 94+ messages in thread
From: Lu Baolu @ 2017-12-05 7:37 UTC (permalink / raw)
To: Jacob Pan, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Lan Tianyu, Jean Delvare
Hi,
On 11/18/2017 02:55 AM, Jacob Pan wrote:
> If the source device of a page request has its PASID table pointer
> bond to a guest, the first level page tables are owned by the guest.
> In this case, we shall let guest OS to manage page fault.
>
> This patch uses the IOMMU fault notification API to send notifications,
> possibly via VFIO, to the guest OS. Once guest pages are fault in, guest
> will issue page response which will be passed down via the invalidation
> passdown APIs.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> drivers/iommu/intel-svm.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
> include/linux/iommu.h | 1 +
> 2 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index f6697e5..77c25d8 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
> @@ -555,6 +555,71 @@ static bool is_canonical_address(u64 addr)
> return (((saddr << shift) >> shift) == saddr);
> }
>
> +static int prq_to_iommu_prot(struct page_req_dsc *req)
> +{
> + int prot = 0;
> +
> + if (req->rd_req)
> + prot |= IOMMU_FAULT_READ;
> + if (req->wr_req)
> + prot |= IOMMU_FAULT_WRITE;
> + if (req->exe_req)
> + prot |= IOMMU_FAULT_EXEC;
> + if (req->priv_req)
> + prot |= IOMMU_FAULT_PRIV;
> +
> + return prot;
> +}
> +
> +static int intel_svm_prq_report(struct device *dev, struct page_req_dsc *desc)
> +{
> + int ret = 0;
It seems that "ret" should be initialized as -EINVAL. Otherwise, this function
will return 0 for devices which have no fault handlers, and all page requests
will be ignored by iommu driver.
> + struct iommu_fault_event event;
> + struct pci_dev *pdev;
> +
> + /**
> + * If caller does not provide struct device, this is the case where
> + * guest PASID table is bound to the device. So we need to retrieve
> + * struct device from the page request descriptor then proceed.
> + */
> + if (!dev) {
> + pdev = pci_get_bus_and_slot(desc->bus, desc->devfn);
> + if (!pdev) {
> + pr_err("No PCI device found for PRQ [%02x:%02x.%d]\n",
> + desc->bus, PCI_SLOT(desc->devfn),
> + PCI_FUNC(desc->devfn));
> + return -ENODEV;
> + }
> + dev = &pdev->dev;
> + } else if (dev_is_pci(dev)) {
> + pdev = to_pci_dev(dev);
> + pci_dev_get(pdev);
> + } else
> + return -ENODEV;
> +
> + pr_debug("Notify PRQ device [%02x:%02x.%d]\n",
> + desc->bus, PCI_SLOT(desc->devfn),
> + PCI_FUNC(desc->devfn));
> +
> + /* invoke device fault handler if registered */
> + if (iommu_has_device_fault_handler(dev)) {
> + /* Fill in event data for device specific processing */
> + event.type = IOMMU_FAULT_PAGE_REQ;
> + event.addr = desc->addr;
> + event.pasid = desc->pasid;
> + event.page_req_group_id = desc->prg_index;
> + event.prot = prq_to_iommu_prot(desc);
> + event.last_req = desc->lpig;
> + event.pasid_valid = 1;
> + event.iommu_private = desc->private;
> + ret = iommu_report_device_fault(&pdev->dev, &event);
> + }
> +
> + pci_dev_put(pdev);
> +
> + return ret;
> +}
> +
> static irqreturn_t prq_event_thread(int irq, void *d)
> {
> struct intel_iommu *iommu = d;
> @@ -578,7 +643,12 @@ static irqreturn_t prq_event_thread(int irq, void *d)
> handled = 1;
>
> req = &iommu->prq[head / sizeof(*req)];
> -
> + /**
> + * If prq is to be handled outside iommu driver via receiver of
> + * the fault notifiers, we skip the page response here.
> + */
> + if (!intel_svm_prq_report(NULL, req))
> + goto prq_advance;
> result = QI_RESP_FAILURE;
> address = (u64)req->addr << VTD_PAGE_SHIFT;
> if (!req->pasid_present) {
> @@ -649,11 +719,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
> if (WARN_ON(&sdev->list == &svm->devs))
> sdev = NULL;
>
> - if (sdev && sdev->ops && sdev->ops->fault_cb) {
> - int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
> - (req->exe_req << 1) | (req->priv_req);
> - sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result);
> - }
> + intel_svm_prq_report(sdev->dev, req);
Do you mind explaining why we need to report this request twice?
Best regards,
Lu Baolu
> /* We get here in the error case where the PASID lookup failed,
> and these can be NULL. Do not use them below this point! */
> sdev = NULL;
> @@ -679,7 +745,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
>
> qi_submit_sync(&resp, iommu);
> }
> -
> + prq_advance:
> head = (head + sizeof(*req)) & PRQ_RING_MASK;
> }
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 841c044..3083796b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -42,6 +42,7 @@
> * if the IOMMU page table format is equivalent.
> */
> #define IOMMU_PRIV (1 << 5)
> +#define IOMMU_EXEC (1 << 6)
>
> struct iommu_ops;
> struct iommu_group;
^ permalink raw reply [flat|nested] 94+ messages in thread* Re: [PATCH v3 13/16] iommu/intel-svm: notify page request to guest
@ 2017-12-05 7:37 ` Lu Baolu
0 siblings, 0 replies; 94+ messages in thread
From: Lu Baolu @ 2017-12-05 7:37 UTC (permalink / raw)
To: Jacob Pan, iommu, LKML, Joerg Roedel, David Woodhouse,
Greg Kroah-Hartman, Rafael Wysocki, Alex Williamson
Cc: Lan Tianyu, Jean Delvare
Hi,
On 11/18/2017 02:55 AM, Jacob Pan wrote:
> If the source device of a page request has its PASID table pointer
> bond to a guest, the first level page tables are owned by the guest.
> In this case, we shall let guest OS to manage page fault.
>
> This patch uses the IOMMU fault notification API to send notifications,
> possibly via VFIO, to the guest OS. Once guest pages are fault in, guest
> will issue page response which will be passed down via the invalidation
> passdown APIs.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> ---
> drivers/iommu/intel-svm.c | 80 ++++++++++++++++++++++++++++++++++++++++++-----
> include/linux/iommu.h | 1 +
> 2 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index f6697e5..77c25d8 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
> @@ -555,6 +555,71 @@ static bool is_canonical_address(u64 addr)
> return (((saddr << shift) >> shift) == saddr);
> }
>
> +static int prq_to_iommu_prot(struct page_req_dsc *req)
> +{
> + int prot = 0;
> +
> + if (req->rd_req)
> + prot |= IOMMU_FAULT_READ;
> + if (req->wr_req)
> + prot |= IOMMU_FAULT_WRITE;
> + if (req->exe_req)
> + prot |= IOMMU_FAULT_EXEC;
> + if (req->priv_req)
> + prot |= IOMMU_FAULT_PRIV;
> +
> + return prot;
> +}
> +
> +static int intel_svm_prq_report(struct device *dev, struct page_req_dsc *desc)
> +{
> + int ret = 0;
It seems that "ret" should be initialized as -EINVAL. Otherwise, this function
will return 0 for devices which have no fault handlers, and all page requests
will be ignored by iommu driver.
> + struct iommu_fault_event event;
> + struct pci_dev *pdev;
> +
> + /**
> + * If caller does not provide struct device, this is the case where
> + * guest PASID table is bound to the device. So we need to retrieve
> + * struct device from the page request descriptor then proceed.
> + */
> + if (!dev) {
> + pdev = pci_get_bus_and_slot(desc->bus, desc->devfn);
> + if (!pdev) {
> + pr_err("No PCI device found for PRQ [%02x:%02x.%d]\n",
> + desc->bus, PCI_SLOT(desc->devfn),
> + PCI_FUNC(desc->devfn));
> + return -ENODEV;
> + }
> + dev = &pdev->dev;
> + } else if (dev_is_pci(dev)) {
> + pdev = to_pci_dev(dev);
> + pci_dev_get(pdev);
> + } else
> + return -ENODEV;
> +
> + pr_debug("Notify PRQ device [%02x:%02x.%d]\n",
> + desc->bus, PCI_SLOT(desc->devfn),
> + PCI_FUNC(desc->devfn));
> +
> + /* invoke device fault handler if registered */
> + if (iommu_has_device_fault_handler(dev)) {
> + /* Fill in event data for device specific processing */
> + event.type = IOMMU_FAULT_PAGE_REQ;
> + event.addr = desc->addr;
> + event.pasid = desc->pasid;
> + event.page_req_group_id = desc->prg_index;
> + event.prot = prq_to_iommu_prot(desc);
> + event.last_req = desc->lpig;
> + event.pasid_valid = 1;
> + event.iommu_private = desc->private;
> + ret = iommu_report_device_fault(&pdev->dev, &event);
> + }
> +
> + pci_dev_put(pdev);
> +
> + return ret;
> +}
> +
> static irqreturn_t prq_event_thread(int irq, void *d)
> {
> struct intel_iommu *iommu = d;
> @@ -578,7 +643,12 @@ static irqreturn_t prq_event_thread(int irq, void *d)
> handled = 1;
>
> req = &iommu->prq[head / sizeof(*req)];
> -
> + /**
> + * If prq is to be handled outside iommu driver via receiver of
> + * the fault notifiers, we skip the page response here.
> + */
> + if (!intel_svm_prq_report(NULL, req))
> + goto prq_advance;
> result = QI_RESP_FAILURE;
> address = (u64)req->addr << VTD_PAGE_SHIFT;
> if (!req->pasid_present) {
> @@ -649,11 +719,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
> if (WARN_ON(&sdev->list == &svm->devs))
> sdev = NULL;
>
> - if (sdev && sdev->ops && sdev->ops->fault_cb) {
> - int rwxp = (req->rd_req << 3) | (req->wr_req << 2) |
> - (req->exe_req << 1) | (req->priv_req);
> - sdev->ops->fault_cb(sdev->dev, req->pasid, req->addr, req->private, rwxp, result);
> - }
> + intel_svm_prq_report(sdev->dev, req);
Do you mind explaining why we need to report this request twice?
Best regards,
Lu Baolu
> /* We get here in the error case where the PASID lookup failed,
> and these can be NULL. Do not use them below this point! */
> sdev = NULL;
> @@ -679,7 +745,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
>
> qi_submit_sync(&resp, iommu);
> }
> -
> + prq_advance:
> head = (head + sizeof(*req)) & PRQ_RING_MASK;
> }
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 841c044..3083796b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -42,6 +42,7 @@
> * if the IOMMU page table format is equivalent.
> */
> #define IOMMU_PRIV (1 << 5)
> +#define IOMMU_EXEC (1 << 6)
>
> struct iommu_ops;
> struct iommu_group;
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 14/16] iommu/intel-svm: replace dev ops with fault report API
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
With the introduction of generic IOMMU device fault reporting API, we
can replace the private fault callback functions with standard function
and event data.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/intel-svm.c | 7 +------
include/linux/intel-svm.h | 20 +++-----------------
2 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 77c25d8..93b1849 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -283,7 +283,7 @@ static const struct mmu_notifier_ops intel_mmuops = {
static DEFINE_MUTEX(pasid_mutex);
-int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
+int intel_svm_bind_mm(struct device *dev, int *pasid, int flags)
{
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
struct intel_svm_dev *sdev;
@@ -329,10 +329,6 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
list_for_each_entry(sdev, &svm->devs, list) {
if (dev == sdev->dev) {
- if (sdev->ops != ops) {
- ret = -EBUSY;
- goto out;
- }
sdev->users++;
goto success;
}
@@ -358,7 +354,6 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
}
/* Finish the setup now we know we're keeping it */
sdev->users = 1;
- sdev->ops = ops;
init_rcu_head(&sdev->rcu);
if (!svm) {
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 99bc5b3..a39a502 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -18,18 +18,6 @@
struct device;
-struct svm_dev_ops {
- void (*fault_cb)(struct device *dev, int pasid, u64 address,
- u32 private, int rwxp, int response);
-};
-
-/* Values for rxwp in fault_cb callback */
-#define SVM_REQ_READ (1<<3)
-#define SVM_REQ_WRITE (1<<2)
-#define SVM_REQ_EXEC (1<<1)
-#define SVM_REQ_PRIV (1<<0)
-
-
/*
* The SVM_FLAG_PRIVATE_PASID flag requests a PASID which is *not* the "main"
* PASID for the current process. Even if a PASID already exists, a new one
@@ -60,7 +48,6 @@ struct svm_dev_ops {
* @dev: Device to be granted acccess
* @pasid: Address for allocated PASID
* @flags: Flags. Later for requesting supervisor mode, etc.
- * @ops: Callbacks to device driver
*
* This function attempts to enable PASID support for the given device.
* If the @pasid argument is non-%NULL, a PASID is allocated for access
@@ -82,8 +69,7 @@ struct svm_dev_ops {
* Multiple calls from the same process may result in the same PASID
* being re-used. A reference count is kept.
*/
-extern int intel_svm_bind_mm(struct device *dev, int *pasid, int flags,
- struct svm_dev_ops *ops);
+extern int intel_svm_bind_mm(struct device *dev, int *pasid, int flags);
/**
* intel_svm_unbind_mm() - Unbind a specified PASID
@@ -120,7 +106,7 @@ extern int intel_svm_is_pasid_valid(struct device *dev, int pasid);
#else /* CONFIG_INTEL_IOMMU_SVM */
static inline int intel_svm_bind_mm(struct device *dev, int *pasid,
- int flags, struct svm_dev_ops *ops)
+ int flags)
{
return -ENOSYS;
}
@@ -136,6 +122,6 @@ static int intel_svm_is_pasid_valid(struct device *dev, int pasid)
}
#endif /* CONFIG_INTEL_IOMMU_SVM */
-#define intel_svm_available(dev) (!intel_svm_bind_mm((dev), NULL, 0, NULL))
+#define intel_svm_available(dev) (!intel_svm_bind_mm((dev), NULL, 0))
#endif /* __INTEL_SVM_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 14/16] iommu/intel-svm: replace dev ops with fault report API
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
With the introduction of generic IOMMU device fault reporting API, we
can replace the private fault callback functions with standard function
and event data.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/intel-svm.c | 7 +------
include/linux/intel-svm.h | 20 +++-----------------
2 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 77c25d8..93b1849 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -283,7 +283,7 @@ static const struct mmu_notifier_ops intel_mmuops = {
static DEFINE_MUTEX(pasid_mutex);
-int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
+int intel_svm_bind_mm(struct device *dev, int *pasid, int flags)
{
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
struct intel_svm_dev *sdev;
@@ -329,10 +329,6 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
list_for_each_entry(sdev, &svm->devs, list) {
if (dev == sdev->dev) {
- if (sdev->ops != ops) {
- ret = -EBUSY;
- goto out;
- }
sdev->users++;
goto success;
}
@@ -358,7 +354,6 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
}
/* Finish the setup now we know we're keeping it */
sdev->users = 1;
- sdev->ops = ops;
init_rcu_head(&sdev->rcu);
if (!svm) {
diff --git a/include/linux/intel-svm.h b/include/linux/intel-svm.h
index 99bc5b3..a39a502 100644
--- a/include/linux/intel-svm.h
+++ b/include/linux/intel-svm.h
@@ -18,18 +18,6 @@
struct device;
-struct svm_dev_ops {
- void (*fault_cb)(struct device *dev, int pasid, u64 address,
- u32 private, int rwxp, int response);
-};
-
-/* Values for rxwp in fault_cb callback */
-#define SVM_REQ_READ (1<<3)
-#define SVM_REQ_WRITE (1<<2)
-#define SVM_REQ_EXEC (1<<1)
-#define SVM_REQ_PRIV (1<<0)
-
-
/*
* The SVM_FLAG_PRIVATE_PASID flag requests a PASID which is *not* the "main"
* PASID for the current process. Even if a PASID already exists, a new one
@@ -60,7 +48,6 @@ struct svm_dev_ops {
* @dev: Device to be granted acccess
* @pasid: Address for allocated PASID
* @flags: Flags. Later for requesting supervisor mode, etc.
- * @ops: Callbacks to device driver
*
* This function attempts to enable PASID support for the given device.
* If the @pasid argument is non-%NULL, a PASID is allocated for access
@@ -82,8 +69,7 @@ struct svm_dev_ops {
* Multiple calls from the same process may result in the same PASID
* being re-used. A reference count is kept.
*/
-extern int intel_svm_bind_mm(struct device *dev, int *pasid, int flags,
- struct svm_dev_ops *ops);
+extern int intel_svm_bind_mm(struct device *dev, int *pasid, int flags);
/**
* intel_svm_unbind_mm() - Unbind a specified PASID
@@ -120,7 +106,7 @@ extern int intel_svm_is_pasid_valid(struct device *dev, int pasid);
#else /* CONFIG_INTEL_IOMMU_SVM */
static inline int intel_svm_bind_mm(struct device *dev, int *pasid,
- int flags, struct svm_dev_ops *ops)
+ int flags)
{
return -ENOSYS;
}
@@ -136,6 +122,6 @@ static int intel_svm_is_pasid_valid(struct device *dev, int pasid)
}
#endif /* CONFIG_INTEL_IOMMU_SVM */
-#define intel_svm_available(dev) (!intel_svm_bind_mm((dev), NULL, 0, NULL))
+#define intel_svm_available(dev) (!intel_svm_bind_mm((dev), NULL, 0))
#endif /* __INTEL_SVM_H__ */
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread
* [PATCH v3 15/16] iommu: introduce page response function
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
When nested translation is turned on and guest owns the
first level page tables, device page request can be forwared
to the guest for handling faults. As the page response returns
by the guest, IOMMU driver on the host need to process the
response which informs the device and completes the page request
transaction.
This patch introduces generic API function for page response
passing from the guest or other in-kernel users. The definitions of
the generic data is based on PCI ATS specification not limited to
any vendor.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/iommu.c | 14 ++++++++++++++
include/linux/iommu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 97b7990..7aefb40 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1416,6 +1416,20 @@ int iommu_sva_invalidate(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_sva_invalidate);
+int iommu_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg)
+{
+ int ret = 0;
+
+ if (unlikely(!domain->ops->page_response))
+ return -ENODEV;
+
+ ret = domain->ops->page_response(domain, dev, msg);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_page_response);
+
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 3083796b..17f698b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -163,6 +163,43 @@ struct iommu_resv_region {
#ifdef CONFIG_IOMMU_API
+enum page_response_type {
+ IOMMU_PAGE_STREAM_RESP = 1,
+ IOMMU_PAGE_GROUP_RESP,
+};
+
+/**
+ * Generic page response information based on PCI ATS and PASID spec.
+ * @paddr: servicing page address
+ * @pasid: contains process address space ID, used in shared virtual memory(SVM)
+ * @rid: requestor ID
+ * @did: destination device ID
+ * @last_req: last request in a page request group
+ * @resp_code: response code
+ * @page_req_group_id: page request group index
+ * @prot: page access protection flag, e.g. IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
+ * @type: group or stream response
+ * @private_data: uniquely identify device-specific private data for an
+ * individual page response
+
+ */
+struct page_response_msg {
+ u64 paddr;
+ u32 pasid;
+ u32 rid:16;
+ u32 did:16;
+ u32 resp_code:4;
+ u32 last_req:1;
+ u32 pasid_present:1;
+#define IOMMU_PAGE_RESP_SUCCESS 0
+#define IOMMU_PAGE_RESP_INVALID 1
+#define IOMMU_PAGE_RESP_FAILURE 0xF
+ u32 page_req_group_id : 9;
+ u32 prot;
+ enum page_response_type type;
+ u32 private_data;
+};
+
/**
* struct iommu_ops - iommu ops and capabilities
* @capable: check capability
@@ -196,6 +233,7 @@ struct iommu_resv_region {
* @bind_pasid_table: bind pasid table pointer for guest SVM
* @unbind_pasid_table: unbind pasid table pointer and restore defaults
* @sva_invalidate: invalidate translation caches of shared virtual address
+ * @page_response: handle page request response
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -251,6 +289,8 @@ struct iommu_ops {
struct device *dev);
int (*sva_invalidate)(struct iommu_domain *domain,
struct device *dev, struct tlb_invalidate_info *inv_info);
+ int (*page_response)(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg);
unsigned long pgsize_bitmap;
};
@@ -472,6 +512,8 @@ 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_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg);
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 *);
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 15/16] iommu: introduce page response function
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
When nested translation is turned on and guest owns the
first level page tables, device page request can be forwared
to the guest for handling faults. As the page response returns
by the guest, IOMMU driver on the host need to process the
response which informs the device and completes the page request
transaction.
This patch introduces generic API function for page response
passing from the guest or other in-kernel users. The definitions of
the generic data is based on PCI ATS specification not limited to
any vendor.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/iommu.c | 14 ++++++++++++++
include/linux/iommu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 56 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 97b7990..7aefb40 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1416,6 +1416,20 @@ int iommu_sva_invalidate(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_sva_invalidate);
+int iommu_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg)
+{
+ int ret = 0;
+
+ if (unlikely(!domain->ops->page_response))
+ return -ENODEV;
+
+ ret = domain->ops->page_response(domain, dev, msg);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_page_response);
+
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 3083796b..17f698b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -163,6 +163,43 @@ struct iommu_resv_region {
#ifdef CONFIG_IOMMU_API
+enum page_response_type {
+ IOMMU_PAGE_STREAM_RESP = 1,
+ IOMMU_PAGE_GROUP_RESP,
+};
+
+/**
+ * Generic page response information based on PCI ATS and PASID spec.
+ * @paddr: servicing page address
+ * @pasid: contains process address space ID, used in shared virtual memory(SVM)
+ * @rid: requestor ID
+ * @did: destination device ID
+ * @last_req: last request in a page request group
+ * @resp_code: response code
+ * @page_req_group_id: page request group index
+ * @prot: page access protection flag, e.g. IOMMU_FAULT_READ, IOMMU_FAULT_WRITE
+ * @type: group or stream response
+ * @private_data: uniquely identify device-specific private data for an
+ * individual page response
+
+ */
+struct page_response_msg {
+ u64 paddr;
+ u32 pasid;
+ u32 rid:16;
+ u32 did:16;
+ u32 resp_code:4;
+ u32 last_req:1;
+ u32 pasid_present:1;
+#define IOMMU_PAGE_RESP_SUCCESS 0
+#define IOMMU_PAGE_RESP_INVALID 1
+#define IOMMU_PAGE_RESP_FAILURE 0xF
+ u32 page_req_group_id : 9;
+ u32 prot;
+ enum page_response_type type;
+ u32 private_data;
+};
+
/**
* struct iommu_ops - iommu ops and capabilities
* @capable: check capability
@@ -196,6 +233,7 @@ struct iommu_resv_region {
* @bind_pasid_table: bind pasid table pointer for guest SVM
* @unbind_pasid_table: unbind pasid table pointer and restore defaults
* @sva_invalidate: invalidate translation caches of shared virtual address
+ * @page_response: handle page request response
*/
struct iommu_ops {
bool (*capable)(enum iommu_cap);
@@ -251,6 +289,8 @@ struct iommu_ops {
struct device *dev);
int (*sva_invalidate)(struct iommu_domain *domain,
struct device *dev, struct tlb_invalidate_info *inv_info);
+ int (*page_response)(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg);
unsigned long pgsize_bitmap;
};
@@ -472,6 +512,8 @@ 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_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg);
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 *);
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* Re: [PATCH v3 15/16] iommu: introduce page response function
2017-11-17 18:55 ` Jacob Pan
(?)
@ 2017-11-24 12:03 ` Jean-Philippe Brucker
[not found] ` <93661c1c-2d3b-295f-0b9d-52e50ea9e1d0-5wv7dgnIgG8@public.gmane.org>
-1 siblings, 1 reply; 94+ messages in thread
From: Jean-Philippe Brucker @ 2017-11-24 12:03 UTC (permalink / raw)
To: Jacob Pan, iommu@lists.linux-foundation.org, LKML, Joerg Roedel,
David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
On 17/11/17 18:55, Jacob Pan wrote:
> When nested translation is turned on and guest owns the
> first level page tables, device page request can be forwared
> to the guest for handling faults. As the page response returns
> by the guest, IOMMU driver on the host need to process the
> response which informs the device and completes the page request
> transaction.
>
> This patch introduces generic API function for page response
> passing from the guest or other in-kernel users. The definitions of
> the generic data is based on PCI ATS specification not limited to
> any vendor.>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
> drivers/iommu/iommu.c | 14 ++++++++++++++
> include/linux/iommu.h | 42 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 97b7990..7aefb40 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1416,6 +1416,20 @@ int iommu_sva_invalidate(struct iommu_domain *domain,
> }
> EXPORT_SYMBOL_GPL(iommu_sva_invalidate);
>
> +int iommu_page_response(struct iommu_domain *domain, struct device *dev,
> + struct page_response_msg *msg)
I think it's simpler, both for IOMMU and device drivers, to pass the exact
structure received in the fault handler back to the IOMMU driver, along
with a separate response status. So maybe
int iommu_page_response(struct iommu_domain *domain, struct device *dev,
struct iommu_fault_event *event, u32 response)
And then you'd just need to define the IOMMU_PAGE_RESPONSE_* values.
Thanks,
Jean
^ permalink raw reply [flat|nested] 94+ messages in thread
* [PATCH v3 16/16] iommu/vt-d: add intel iommu page response function
2017-11-17 18:54 ` Jacob Pan
@ 2017-11-17 18:55 ` Jacob Pan
-1 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, LKML,
Joerg Roedel, David Woodhouse, Greg Kroah-Hartman, Rafael Wysocki,
Alex Williamson
Cc: Lan Tianyu, Jean Delvare
This patch adds page response support for Intel VT-d.
Generic response data is taken from the IOMMU API
then parsed into VT-d specific response descriptor format.
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/intel-iommu.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e1bd219..7f95827 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5171,6 +5171,35 @@ static int intel_iommu_sva_invalidate(struct iommu_domain *domain,
return ret;
}
+int intel_iommu_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg)
+{
+ struct qi_desc resp;
+ struct intel_iommu *iommu = dev_to_intel_iommu(dev);
+
+ /* TODO: sanitize response message */
+ if (msg->last_req) {
+ /* Page Group Response */
+ resp.low = QI_PGRP_PASID(msg->pasid) |
+ QI_PGRP_DID(msg->did) |
+ QI_PGRP_PASID_P(msg->pasid_present) |
+ QI_PGRP_RESP_TYPE;
+ /* REVISIT: allow private data passing from device prq */
+ resp.high = QI_PGRP_IDX(msg->page_req_group_id) |
+ QI_PGRP_PRIV(msg->private_data) | QI_PGRP_RESP_CODE(msg->resp_code);
+ } else {
+ /* Page Stream Response */
+ resp.low = QI_PSTRM_IDX(msg->page_req_group_id) |
+ QI_PSTRM_PRIV(msg->private_data) | QI_PSTRM_BUS(PCI_BUS_NUM(msg->did)) |
+ QI_PSTRM_PASID(msg->pasid) | QI_PSTRM_RESP_TYPE;
+ resp.high = QI_PSTRM_ADDR(msg->paddr) | QI_PSTRM_DEVFN(msg->did & 0xff) |
+ QI_PSTRM_RESP_CODE(msg->resp_code);
+ }
+ qi_submit_sync(&resp, iommu);
+
+ return 0;
+}
+
static int intel_iommu_map(struct iommu_domain *domain,
unsigned long iova, phys_addr_t hpa,
size_t size, int iommu_prot)
@@ -5606,6 +5635,7 @@ const struct iommu_ops intel_iommu_ops = {
.bind_pasid_table = intel_iommu_bind_pasid_table,
.unbind_pasid_table = intel_iommu_unbind_pasid_table,
.sva_invalidate = intel_iommu_sva_invalidate,
+ .page_response = intel_iommu_page_response,
#endif
.map = intel_iommu_map,
.unmap = intel_iommu_unmap,
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread* [PATCH v3 16/16] iommu/vt-d: add intel iommu page response function
@ 2017-11-17 18:55 ` Jacob Pan
0 siblings, 0 replies; 94+ messages in thread
From: Jacob Pan @ 2017-11-17 18:55 UTC (permalink / raw)
To: iommu, LKML, Joerg Roedel, David Woodhouse, Greg Kroah-Hartman,
Rafael Wysocki, Alex Williamson
Cc: Liu, Yi L, Lan Tianyu, Tian, Kevin, Raj Ashok, Jean Delvare,
Christoph Hellwig, Jacob Pan
This patch adds page response support for Intel VT-d.
Generic response data is taken from the IOMMU API
then parsed into VT-d specific response descriptor format.
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
drivers/iommu/intel-iommu.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e1bd219..7f95827 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5171,6 +5171,35 @@ static int intel_iommu_sva_invalidate(struct iommu_domain *domain,
return ret;
}
+int intel_iommu_page_response(struct iommu_domain *domain, struct device *dev,
+ struct page_response_msg *msg)
+{
+ struct qi_desc resp;
+ struct intel_iommu *iommu = dev_to_intel_iommu(dev);
+
+ /* TODO: sanitize response message */
+ if (msg->last_req) {
+ /* Page Group Response */
+ resp.low = QI_PGRP_PASID(msg->pasid) |
+ QI_PGRP_DID(msg->did) |
+ QI_PGRP_PASID_P(msg->pasid_present) |
+ QI_PGRP_RESP_TYPE;
+ /* REVISIT: allow private data passing from device prq */
+ resp.high = QI_PGRP_IDX(msg->page_req_group_id) |
+ QI_PGRP_PRIV(msg->private_data) | QI_PGRP_RESP_CODE(msg->resp_code);
+ } else {
+ /* Page Stream Response */
+ resp.low = QI_PSTRM_IDX(msg->page_req_group_id) |
+ QI_PSTRM_PRIV(msg->private_data) | QI_PSTRM_BUS(PCI_BUS_NUM(msg->did)) |
+ QI_PSTRM_PASID(msg->pasid) | QI_PSTRM_RESP_TYPE;
+ resp.high = QI_PSTRM_ADDR(msg->paddr) | QI_PSTRM_DEVFN(msg->did & 0xff) |
+ QI_PSTRM_RESP_CODE(msg->resp_code);
+ }
+ qi_submit_sync(&resp, iommu);
+
+ return 0;
+}
+
static int intel_iommu_map(struct iommu_domain *domain,
unsigned long iova, phys_addr_t hpa,
size_t size, int iommu_prot)
@@ -5606,6 +5635,7 @@ const struct iommu_ops intel_iommu_ops = {
.bind_pasid_table = intel_iommu_bind_pasid_table,
.unbind_pasid_table = intel_iommu_unbind_pasid_table,
.sva_invalidate = intel_iommu_sva_invalidate,
+ .page_response = intel_iommu_page_response,
#endif
.map = intel_iommu_map,
.unmap = intel_iommu_unmap,
--
2.7.4
^ permalink raw reply related [flat|nested] 94+ messages in thread