SMMUv3 Nested Stage Setup

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +0 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c

include/uapi/linux/iommu.h new file +13 -0

+ */
+struct iommu_pasid_smmuv3 {
+	__u8 abort;
+	__u8 s1fmt;
+	__u8 s1dss;
+};
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table

include/uapi/linux/iommu.h new file +6 -0

+	union {
+		struct iommu_pasid_smmuv3 smmuv3;
+	};
+};
+
+#endif /* _UAPI_IOMMU_H */

Quoted 16 lines · Eric Auger

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

Quoted 4 lines

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Last sentence is a bit confusing. With SMMUv3 it is also used for the PASID table, even when it only has one entry and PASID is disabled.

Quoted 109 lines

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +2 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*

include/uapi/linux/iommu.h line 8

Quoted 6 lines

include/uapi/linux/iommu.h new file +6 -0

+ * 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.

I don't think we need both the boilerplate and the SPDX header

Quoted 10 lines

include/uapi/linux/iommu.h new file +9 -0

+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information

include/uapi/linux/iommu.h line 21

Quoted 4 lines

include/uapi/linux/iommu.h new file +4 -0

+ * @abort: shall the STE lead to abort
+ * @s1fmt: STE s1fmt field as set by the guest
+ * @s1dss: STE s1dss as set by the guest
+ * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)

Not really the case for @abort. Could you clarify whether @abort is valid in combination with @bypass?
abort corresponds to !Config[2]. In that case the spec says "report

Quoted 15 lines

include/uapi/linux/iommu.h new file +15 -0

+ */
+struct iommu_pasid_smmuv3 {
+	__u8 abort;
+	__u8 s1fmt;
+	__u8 s1dss;
+};
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */

include/uapi/linux/iommu.h

Quoted 8 lines

include/uapi/linux/iommu.h new file +8 -0

+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1
+	__u32	version;
+#define IOMMU_PASID_FORMAT_SMMUV3	(1 << 0)
+	__u32	format;
+	__u64	base_ptr;
+	__u8	pasid_bits;
+	__u8	bypass

We need some padding, in case someone adds a new struct to the union that requires 64-byte alignment

And 'bypass' might not be the right name if we're making it common, maybe 'reset' would be clearer? Or we just need to explain that bypass is the initial state of a nesting domain

Thanks,
Jean

Quoted 7 lines

include/uapi/linux/iommu.h new file +6 -0

+	union {
+		struct iommu_pasid_smmuv3 smmuv3;
+	};
+};
+
+#endif /* _UAPI_IOMMU_H */

Hi Jean-Philippe,

Quoted 17 lines · Jean-Philippe Brucker

Quoted 16 lines · Eric Auger

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

Quoted 7 lines

Quoted 4 lines

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Last sentence is a bit confusing. With SMMUv3 it is also used for the PASID table, even when it only has one entry and PASID is disabled.

OK removed

Quoted 109 lines

Quoted 109 lines

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +2 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*

Quoted 8 lines

include/uapi/linux/iommu.h line 8

Quoted 6 lines

include/uapi/linux/iommu.h new file +6 -0

+ * 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.

I don't think we need both the boilerplate and the SPDX header

OK I only kept the SPDX header.

Quoted 10 lines

Quoted 10 lines

include/uapi/linux/iommu.h new file +9 -0

+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information

Quoted 8 lines

include/uapi/linux/iommu.h line 21

Quoted 4 lines

include/uapi/linux/iommu.h new file +4 -0

+ * @abort: shall the STE lead to abort
+ * @s1fmt: STE s1fmt field as set by the guest
+ * @s1dss: STE s1dss as set by the guest
+ * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)

Not really the case for @abort. Could you clarify whether @abort is valid in combination with @bypass?
abort corresponds to !Config[2]. In that case the spec says "report

abort to device, no event recorded". S1 bypass corresponds to Config=0b1x0. What about removing abort in the SMMUv3 specific part and encode the stage state in the generic part. See below proposal ...

Quoted 19 lines

Quoted 19 lines

include/uapi/linux/iommu.h new file +19 -0

+ */
+struct iommu_pasid_smmuv3 {
+	__u8 abort;
+	__u8 s1fmt;
+	__u8 s1dss;
+};
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */
+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1
+	__u32	version;
+#define IOMMU_PASID_FORMAT_SMMUV3	(1 << 0)

Quoted 4 lines

include/uapi/linux/iommu.h

Quoted 4 lines

include/uapi/linux/iommu.h new file +4 -0

+	__u32	format;
+	__u64	base_ptr;
+	__u8	pasid_bits;
+	__u8	bypass

#define IOMMU_PASID_STREAM_ABORT (1 << 0)
#define IOMMU_PASID_STREAM_BYPASS (1 << 1)
#define IOMMU_PASID_STREAM_TRANSLATE (1 << 2)
__u8 config;

Quoted 3 lines

We need some padding, in case someone adds a new struct to the union that requires 64-byte alignment

OK

Quoted 4 lines

And 'bypass' might not be the right name if we're making it common, maybe 'reset' would be clearer? Or we just need to explain that bypass is the initial state of a nesting domain

I will add such comment. To me the "bypass" terminology sounds clearer than "reset"

Quoted 12 lines

Thanks,
Jean

Quoted 7 lines

include/uapi/linux/iommu.h new file +6 -0

+	union {
+		struct iommu_pasid_smmuv3 smmuv3;
+	};
+};
+
+#endif /* _UAPI_IOMMU_H */

Hi Jean-Philippe,

Quoted 186 lines · Auger Eric

Hi Jean-Philippe,

Quoted 17 lines · Jean-Philippe Brucker

Quoted 16 lines · Eric Auger

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

Quoted 7 lines

Quoted 4 lines

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Last sentence is a bit confusing. With SMMUv3 it is also used for the PASID table, even when it only has one entry and PASID is disabled.

OK removed

Quoted 109 lines

Quoted 109 lines

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +2 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*

Quoted 8 lines

include/uapi/linux/iommu.h line 8

Quoted 6 lines

include/uapi/linux/iommu.h new file +6 -0

+ * 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.

I don't think we need both the boilerplate and the SPDX header

OK I only kept the SPDX header.

Quoted 10 lines

Quoted 10 lines

include/uapi/linux/iommu.h new file +9 -0

+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information

Quoted 8 lines

include/uapi/linux/iommu.h line 21

Quoted 4 lines

include/uapi/linux/iommu.h new file +4 -0

+ * @abort: shall the STE lead to abort
+ * @s1fmt: STE s1fmt field as set by the guest
+ * @s1dss: STE s1dss as set by the guest
+ * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)

Not really the case for @abort. Could you clarify whether @abort is valid in combination with @bypass?
abort corresponds to !Config[2]. In that case the spec says "report

abort to device, no event recorded". S1 bypass corresponds to Config=0b1x0. What about removing abort in the SMMUv3 specific part and encode the stage state in the generic part. See below proposal ...

Quoted 19 lines

Quoted 19 lines

include/uapi/linux/iommu.h new file +19 -0

+ */
+struct iommu_pasid_smmuv3 {
+	__u8 abort;
+	__u8 s1fmt;
+	__u8 s1dss;
+};
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */
+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1
+	__u32	version;
+#define IOMMU_PASID_FORMAT_SMMUV3	(1 << 0)

Quoted 8 lines

Quoted 4 lines

include/uapi/linux/iommu.h

Quoted 4 lines

include/uapi/linux/iommu.h new file +4 -0

+	__u32	format;
+	__u64	base_ptr;
+	__u8	pasid_bits;
+	__u8	bypass

#define IOMMU_PASID_STREAM_ABORT (1 << 0)
#define IOMMU_PASID_STREAM_BYPASS (1 << 1)
#define IOMMU_PASID_STREAM_TRANSLATE (1 << 2)
__u8 config;

Sorry for the confusion, we don't want a bitfield here as those values are exclusive.

What about:
struct iommu_pasid_table_config {

#define PASID_TABLE_CFG_VERSION_1 1
        __u32   version;
#define IOMMU_PASID_FORMAT_SMMUV3       (1 << 0)
        __u32   format;
        __u64   base_ptr;
        __u8    pasid_bits;
#define IOMMU_PASID_CONFIG_BYPASS       1
#define IOMMU_PASID_CONFIG_ABORT        2
#define IOMMU_PASID_CONFIG_TRANSLATE    3

        __u8    config;
        __u8    padding[6];
        union {
                struct iommu_pasid_smmuv3 smmuv3;
        };
};

Thanks

Eric

Quoted 22 lines

Quoted 3 lines

We need some padding, in case someone adds a new struct to the union that requires 64-byte alignment

OK

Quoted 4 lines

And 'bypass' might not be the right name if we're making it common, maybe 'reset' would be clearer? Or we just need to explain that bypass is the initial state of a nesting domain

I will add such comment. To me the "bypass" terminology sounds clearer than "reset"

Quoted 12 lines

Thanks,
Jean

Quoted 7 lines

include/uapi/linux/iommu.h new file +6 -0

+	union {
+		struct iommu_pasid_smmuv3 smmuv3;
+	};
+};
+
+#endif /* _UAPI_IOMMU_H */

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +18 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * IOMMU user API definitions
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information
+ * @abort: shall the STE lead to abort

include/uapi/linux/iommu.h new file +10 -0

+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */
+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1

include/uapi/linux/iommu.h new file +2 -0

+
+#endif /* _UAPI_IOMMU_H */

Quoted 152 lines · On Tue, 8 Jan 2019 11:26:13 +0100 Eric Auger <[email protected]> wrote:

From: Jacob Pan <[email protected]>

In virtualization use case, when a guest is assigned a PCI host device, protected by a virtual IOMMU on a guest, the physical IOMMU must be programmed to be consistent with the guest mappings. If the physical IOMMU supports two translation stages it makes sense to program guest mappings onto the first stage/level (ARM/VTD terminology) while to host owns the stage/level 2.

In that case, it is mandated to trap on guest configuration settings and pass those to the physical iommu driver.

This patch adds a new API to the iommu subsystem that allows to set the pasid table information.

A generic iommu_pasid_table_config struct is introduced in a new iommu.h uapi header. This is going to be used by the VFIO user API. We foresee at least two specializations of this struct, for PASID table passing and ARM SMMUv3.

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/

v2 -> v3:
- replace unbind/bind by set_pasid_table
- move table pointer and pasid bits in the generic part of the struct

v1 -> v2:
- restore the original pasid table name
- remove the struct device * parameter in the API
- reworked iommu_pasid_smmuv3

---
 drivers/iommu/iommu.c      | 10 ++++++++
 include/linux/iommu.h      | 14 +++++++++++
 include/uapi/linux/iommu.h | 50 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

drivers/iommu/iommu.c +10 -0

index 3ed4db334341..0f2b7f1fc7c8 100644
@@ -1393,6 +1393,16 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	if (unlikely(!domain->ops->set_pasid_table))
+		return -ENODEV;
+
+	return domain->ops->set_pasid_table(domain, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index e90da6b6f3d1..1da2a2357ea4 100644
@@ -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)

@@ -184,6 +185,7 @@ struct iommu_resv_region {
  * @domain_window_disable: Disable a particular window for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @set_pasid_table: set pasid table
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -226,6 +228,9 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*set_pasid_table)(struct iommu_domain *domain,
+			       struct iommu_pasid_table_config *cfg);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -287,6 +292,8 @@ 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_set_pasid_table(struct iommu_domain *domain,
+				 struct iommu_pasid_table_config *cfg);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -696,6 +703,13 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_set_pasid_table(struct iommu_domain *domain,
+			  struct iommu_pasid_table_config *cfg)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h new file +25 -0

new file mode 100644
index 000000000000..7a7cf7a3de7c
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * IOMMU user API definitions
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _UAPI_IOMMU_H
+#define _UAPI_IOMMU_H
+
+#include <linux/types.h>
+
+/**
+ * SMMUv3 Stream Table Entry stage 1 related information
+ * @abort: shall the STE lead to abort
+ * @s1fmt: STE s1fmt field as set by the guest
+ * @s1dss: STE s1dss as set by the guest
+ * All field names match the smmu 3.0/3.1 spec (ARM IHI 0070A)
+ */
+struct iommu_pasid_smmuv3 {
+	__u8 abort;
+	__u8 s1fmt;

include/uapi/linux/iommu.h line 28

Quoted 3 lines

include/uapi/linux/iommu.h new file +3 -0

+	__u8 s1dss;
+};
+

I can find STE.S1DSS and STE.S1FMT in the spec, but not STE.ABORT, is this something to do with Config[2:0]? Are we allowed to describe what these fields are beyond their name and why they're necessary here vs the other fields or do the spec restrictions preclude that?

Quoted 11 lines

include/uapi/linux/iommu.h new file +10 -0

+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU
+ * Note PASID table corresponds to the Context Table on ARM SMMUv3.
+ *
+ * @version: API version to prepare for future extensions
+ * @format: format of the PASID table
+ *
+ */
+struct iommu_pasid_table_config {
+#define PASID_TABLE_CFG_VERSION_1 1

include/uapi/linux/iommu.h line 48

Quoted 10 lines

include/uapi/linux/iommu.h new file +10 -0

+	__u32	version;
+#define IOMMU_PASID_FORMAT_SMMUV3	(1 << 0)
+	__u32	format;
+	__u64	base_ptr;
+	__u8	pasid_bits;
+	__u8	bypass;
+	union {
+		struct iommu_pasid_smmuv3 smmuv3;
+	};
+};

Structure is not naturally aligned or explicitly aligned for interchange with userspace. It might work for smmuv3 since the structure is only composed of bytes, but looks troublesome in general. Should each format type also contain a version? Is format intended to be a bit-field or a signature? It seems we only need a signature, but only having a single format defined, it looks like a bit-field, which makes me worry what we do when we exhaust the bits.

should be better defined, is it 0/1? zero/non-zero? more selective?

Thanks,

Alex

Quoted 2 lines

include/uapi/linux/iommu.h new file +2 -0

+
+#endif /* _UAPI_IOMMU_H */

From: "Liu, Yi L" <[email protected]>

In any virtualization use case, when the first translation stage is "owned" by the guest OS, the host IOMMU driver has no knowledge of caching structure updates unless the guest invalidation activities are trapped by the virtualizer and passed down to the host.

Since the invalidation data are obtained from user space and will be written into physical IOMMU, we must allow security check at various layers. Therefore, generic invalidation data format are proposed here, model specific IOMMU drivers need to convert them into their own format.

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---
v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows to invalidate context caches on top of IOTLBs

v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in header. Commit message reworded.

---
 drivers/iommu/iommu.c      | 14 ++++++
 include/linux/iommu.h      | 14 ++++++
 include/uapi/linux/iommu.h | 95 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)

drivers/iommu/iommu.c +14 -0

index 0f2b7f1fc7c8..b2e248770508 100644
@@ -1403,6 +1403,20 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
 
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_cache_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->cache_invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index 1da2a2357ea4..96d59886f230 100644
@@ -186,6 +186,7 @@ struct iommu_resv_region {
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @set_pasid_table: set pasid table
+ * @cache_invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -231,6 +232,9 @@ struct iommu_ops {
 	int (*set_pasid_table)(struct iommu_domain *domain,
 			       struct iommu_pasid_table_config *cfg);
 
+	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -294,6 +298,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
 extern int iommu_set_pasid_table(struct iommu_domain *domain,
 				 struct iommu_pasid_table_config *cfg);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+				struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -709,6 +716,13 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 {
 	return -ENODEV;
 }
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+		       struct device *dev,
+		       struct iommu_cache_invalidate_info *inv_info)
+{
+	return -ENODEV;
+}
 
 #endif /* CONFIG_IOMMU_API */
 

include/uapi/linux/iommu.h +63 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};

include/uapi/linux/iommu.h +3 -0

+	__u64		arch_id;
+	__u64		addr;
+};
 #endif /* _UAPI_IOMMU_H */

Quoted 179 lines · On Tue, 8 Jan 2019 11:26:14 +0100 Eric Auger <[email protected]> wrote:

From: "Liu, Yi L" <[email protected]>

In any virtualization use case, when the first translation stage is "owned" by the guest OS, the host IOMMU driver has no knowledge of caching structure updates unless the guest invalidation activities are trapped by the virtualizer and passed down to the host.

Since the invalidation data are obtained from user space and will be written into physical IOMMU, we must allow security check at various layers. Therefore, generic invalidation data format are proposed here, model specific IOMMU drivers need to convert them into their own format.

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---
v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows to invalidate context caches on top of IOTLBs

v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in header. Commit message reworded.

---
 drivers/iommu/iommu.c      | 14 ++++++
 include/linux/iommu.h      | 14 ++++++
 include/uapi/linux/iommu.h | 95 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)

drivers/iommu/iommu.c +14 -0

index 0f2b7f1fc7c8..b2e248770508 100644
@@ -1403,6 +1403,20 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
 
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_cache_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->cache_invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index 1da2a2357ea4..96d59886f230 100644
@@ -186,6 +186,7 @@ struct iommu_resv_region {
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @set_pasid_table: set pasid table
+ * @cache_invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -231,6 +232,9 @@ struct iommu_ops {
 	int (*set_pasid_table)(struct iommu_domain *domain,
 			       struct iommu_pasid_table_config *cfg);
 
+	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -294,6 +298,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
 extern int iommu_set_pasid_table(struct iommu_domain *domain,
 				 struct iommu_pasid_table_config *cfg);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+				struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -709,6 +716,13 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 {
 	return -ENODEV;
 }
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+		       struct device *dev,
+		       struct iommu_cache_invalidate_info *inv_info)
+{
+	return -ENODEV;
+}
 
 #endif /* CONFIG_IOMMU_API */
 

include/uapi/linux/iommu.h +67 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 122

Quoted 6 lines

include/uapi/linux/iommu.h +6 -0

+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.

Why is this a 4K page centric interface?

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 130

Quoted 9 lines

include/uapi/linux/iommu.h +8 -0

+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @arch_id:		architecture dependent id characterizing a context
+ *			and tagging the caches, ie. domain Identfier on VTD,
+ *			asid on ARM SMMU
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages

Shouldn't some of these be tied the the granularity of the invalidation? It seems like this should be more similar to iommu_pasid_table_config where the granularity of the invalidation defines which entry within a union at the end of the structure is valid and populated. Otherwise we have fields that don't make sense for certain invalidations.

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 135

Quoted 6 lines

include/uapi/linux/iommu.h +5 -0

+ *
+ */
+struct iommu_cache_invalidate_info {
+	struct iommu_cache_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;

A separate structure for hdr seems a little pointless.

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 139

Quoted 5 lines

include/uapi/linux/iommu.h +4 -0

+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;

Really need some padding or packing here for any hope of having consistency with userspace.

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 141

Quoted 2 lines

include/uapi/linux/iommu.h +2 -0

+	__u64		nr_pages;
+	__u32		pasid;

Sub-optimal ordering for packing/padding. Thanks,

Alex

Quoted 4 lines

include/uapi/linux/iommu.h +3 -0

+	__u64		arch_id;
+	__u64		addr;
+};
 #endif /* _UAPI_IOMMU_H */

Quoted 75 lines

Quoted 75 lines

include/uapi/linux/iommu.h +67 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 122

Quoted 6 lines

include/uapi/linux/iommu.h +6 -0

+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.

Why is this a 4K page centric interface?

This matches the vt-d Address Mask (AM) field of the IOTLB Invalidate Descriptor. We can pass a log2size instead.

Quoted 10 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 130

Quoted 9 lines

include/uapi/linux/iommu.h +8 -0

+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @arch_id:		architecture dependent id characterizing a context
+ *			and tagging the caches, ie. domain Identfier on VTD,
+ *			asid on ARM SMMU
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages

Quoted 6 lines

Shouldn't some of these be tied the the granularity of the invalidation? It seems like this should be more similar to iommu_pasid_table_config where the granularity of the invalidation defines which entry within a union at the end of the structure is valid and populated. Otherwise we have fields that don't make sense for certain invalidations.

I am a little bit embarrassed here as this API version is the outcome of long discussions held by Jacob, jean-Philippe and many others. I don't want to hijack that work as I am "simply" reusing this API. Nevertheless I am willing to help on this. So following your recommendation above I dare to propose an updated API:

struct iommu_device_iotlb_inv_info {
        __u32   version;
#define IOMMU_DEV_IOTLB_INV_GLOBAL   0
#define IOMMU_DEV_IOTLB_INV_SOURCEID (1 << 0)
#define IOMMU_DEV_IOTLB_INV_PASID    (1 << 1)
        __u8    granularity;
        __u64   addr;
        __u8    log2size;
        __u64   sourceid;
        __u64   pasid;
        __u8    padding[2];
};

struct iommu_iotlb_inv_info {
        __u32   version;

#define IOMMU_IOTLB_INV_GLOBAL 0

#define IOMMU_IOTLB_INV_ARCHID  (1 << 0)
#define IOMMU_IOTLB_INV_PASID   (1 << 1)
#define IOMMU_IOTLB_INV_PAGE    (1 << 2)

        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   addr;
        __u8    log2size;
        __u8    padding[2];

};

struct iommu_pasid_inv_info {
        __u32   version;
#define IOMMU_PASID_INV_GLOBAL     0
#define IOMMU_PASID_INV_ARCHID     (1 << 0)
#define IOMMU_PASID_INV_PASID      (1 << 1)
#define IOMMU_PASID_INV_SOURCEID   (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   sourceid;
        __u8    padding[3];

};
/**
* Translation cache invalidation information, contains generic IOMMU
* data which can be parsed based on model ID by model specific drivers.
* Since the invalidation of second level page tables are included in
* the unmap operation, this info is only applicable to the first level
* translation caches, i.e. DMA request with PASID.
*
*/
struct iommu_cache_invalidate_info {

#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
        __u32 version;
#define IOMMU_INV_TYPE_IOTLB        1 /* IOMMU paging structure cache */
#define IOMMU_INV_TYPE_DEV_IOTLB    2 /* Device IOTLB */
#define IOMMU_INV_TYPE_PASID        3 /* PASID cache */
        __u8 type;
        union {
                struct iommu_iotlb_invalidate_info iotlb_inv_info;
                struct iommu_dev_iotlb_invalidate_info dev_iotlb_inv_info;
                struct iommu_pasid_inv_info pasid_inv_info;
        };
};

At the moment I ignore the leaf bool parameter used on ARM for PASID invalidation and TLB invalidation. Maybe we can just invalidate more that the leaf cache structures at the moment?

on ARM the PASID table can be invalidated per streamid. On vt-d, as far as I understand the sourceid does not tag the entries.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 135

Quoted 6 lines

include/uapi/linux/iommu.h +5 -0

+ *
+ */
+struct iommu_cache_invalidate_info {
+	struct iommu_cache_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;

A separate structure for hdr seems a little pointless.

removed

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 139

Quoted 5 lines

include/uapi/linux/iommu.h +4 -0

+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;

Really need some padding or packing here for any hope of having consistency with userspace.

Quoted 4 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 141

Quoted 2 lines

include/uapi/linux/iommu.h +2 -0

+	__u64		nr_pages;
+	__u32		pasid;

Sub-optimal ordering for packing/padding. Thanks,

I introduced some padding above. Is that OK?

Again if this introduces more noise than it helps I will simply rely on initial contributors for the respin of their series according to your comments. Also we if can't define generic enough structures for ARM and x86

Thanks

Eric

Quoted 8 lines

Alex

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 145

Quoted 4 lines

include/uapi/linux/iommu.h +3 -0

+	__u64		arch_id;
+	__u64		addr;
+};
 #endif /* _UAPI_IOMMU_H */

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Hi Eric,

On 25/01/2019 16:49, Auger Eric wrote:
[...]

Quoted 102 lines

Quoted 75 lines

Quoted 75 lines

include/uapi/linux/iommu.h +67 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 122

Quoted 6 lines

include/uapi/linux/iommu.h +6 -0

+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.

Why is this a 4K page centric interface?

This matches the vt-d Address Mask (AM) field of the IOTLB Invalidate Descriptor. We can pass a log2size instead.

Quoted 10 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 130

Quoted 9 lines

include/uapi/linux/iommu.h +8 -0

+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @arch_id:		architecture dependent id characterizing a context
+ *			and tagging the caches, ie. domain Identfier on VTD,
+ *			asid on ARM SMMU
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages

Quoted 6 lines

Shouldn't some of these be tied the the granularity of the invalidation? It seems like this should be more similar to iommu_pasid_table_config where the granularity of the invalidation defines which entry within a union at the end of the structure is valid and populated. Otherwise we have fields that don't make sense for certain invalidations.

Quoted 5 lines

I am a little bit embarrassed here as this API version is the outcome of long discussions held by Jacob, jean-Philippe and many others. I don't want to hijack that work as I am "simply" reusing this API. Nevertheless I am willing to help on this. So following your recommendation above I dare to propose an updated API:

Discussing this again is completely fine by me. I have some concerns with this proposal though, some of which apply to our previous versions as well.

Quoted 13 lines

struct iommu_device_iotlb_inv_info {
        __u32   version;
#define IOMMU_DEV_IOTLB_INV_GLOBAL   0
#define IOMMU_DEV_IOTLB_INV_SOURCEID (1 << 0)
#define IOMMU_DEV_IOTLB_INV_PASID    (1 << 1)
        __u8    granularity;
        __u64   addr;
        __u8    log2size;
        __u64   sourceid;
        __u64   pasid;
        __u8    padding[2];
};

Quoted 3 lines

struct iommu_iotlb_inv_info {
        __u32   version;

#define IOMMU_IOTLB_INV_GLOBAL 0

Using "global" for granularity=0 will be confusing, let's call this "domain" instead. In the SMMU and ATS specifications (and I think VT-d as well), the global flag is used to invalidate VA ranges that are cached for all PASIDs. In the Arm architecture these TLB entries are created from PTE entries without the "nG" bit. So "global" usually means granularity=IOMMU_IOTLB_INV_PAGE.

Quoted 4 lines

#define IOMMU_IOTLB_INV_ARCHID  (1 << 0)
#define IOMMU_IOTLB_INV_PASID   (1 << 1)
#define IOMMU_IOTLB_INV_PAGE    (1 << 2)

We might as well call this bit "INV_ADDR" to make it clear that it describes the validity of field @addr.

Quoted 30 lines

        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   addr;
        __u8    log2size;
        __u8    padding[2];

};

struct iommu_pasid_inv_info {
        __u32   version;
#define IOMMU_PASID_INV_GLOBAL     0
#define IOMMU_PASID_INV_ARCHID     (1 << 0)
#define IOMMU_PASID_INV_PASID      (1 << 1)
#define IOMMU_PASID_INV_SOURCEID   (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   sourceid;
        __u8    padding[3];

};
/**
* Translation cache invalidation information, contains generic IOMMU
* data which can be parsed based on model ID by model specific drivers.
* Since the invalidation of second level page tables are included in
* the unmap operation, this info is only applicable to the first level
* translation caches, i.e. DMA request with PASID.
*
*/
struct iommu_cache_invalidate_info {

Quoted 12 lines

#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
        __u32 version;
#define IOMMU_INV_TYPE_IOTLB        1 /* IOMMU paging structure cache */
#define IOMMU_INV_TYPE_DEV_IOTLB    2 /* Device IOTLB */
#define IOMMU_INV_TYPE_PASID        3 /* PASID cache */
        __u8 type;
        union {
                struct iommu_iotlb_invalidate_info iotlb_inv_info;
                struct iommu_dev_iotlb_invalidate_info dev_iotlb_inv_info;
                struct iommu_pasid_inv_info pasid_inv_info;
        };
};

Although I find the new structure and field names clearer in general, I believe we're losing something by splitting structures this way.

The one concept I'd like to keep is the possibility to multiplex ATC and IOTLB invalidation. That is, when unmapping a range, the guest (using a pvIOMMU) shouldn't need to send both iotlb_inv and device_iotlb_inv to the host - it's completely redundant. Instead the host IOMMU driver should receive a single invalidation packet, and send both TLB and ATC invalidation to the hardware.

So in my opinion the invalidation type needs to be a bitfield: userspace can select either TYPE_IOTLB, TYPE_DEV_IOTLB, or both. And if type is a bitfield, the content that follows has to be a single structure.

Same for the PASID cache: when the guest changes the config for a PASID, it shouldn't have to also send TLB and ATC invalidations. A single packet should be enough. However config changes won't be a fast path, so optimizing the API is less important here.

We could say that IOMMU_INV_TYPE_IOTLB implies IOMMU_INV_TYPE_DEV_IOTLB, and that IOMMU_INV_TYPE_PASID implies the others. In fact I think we do, in this patch. But that in turn would be suboptimal with vSMMU and other emulated solutions, which will receive both TLB and ATC invalidation from the guest, and have to signal both separately to the kernel. So for @type, a bitfield would be best.

If we want to split the structure, I think splitting by @granularity might make more sense. It might require at least 4 structures:
* domain invalidation: granule = 0
* pasid invalidation: granule = pasid|archid
* global va invalidation: granule = addr> * va invalidation granule = pasid|archid|addr

Quoted 4 lines

At the moment I ignore the leaf bool parameter used on ARM for PASID invalidation and TLB invalidation. Maybe we can just invalidate more that the leaf cache structures at the moment?

Sure, though we could add a flags field and leave it unused for now, which is easier to extend than introducing a new version. I wonder if Intel's Invalidation Hint (IH) does the same as Arm's leaf flag?

Quoted 3 lines

on ARM the PASID table can be invalidated per streamid. On vt-d, as far as I understand the sourceid does not tag the entries.

I don't think sourceid is the right thing to use in this interface. The device ID (streamid or sourceid) that the IOMMU sees in DMA transactions isn't really made visible to userspace. And for mdev it doesn't really exist, VFIO will have to pass the parent device's handle to IOMMU.

To userspace a device is identified by the fd provided by VFIO. So if we do want to have device-scope in the invalidation (as opposed to the current domain-scope) I think userspace needs to provide a device fd to VFIO (outside the structures defined here), and then VFIO would pass a struct device to iommu_cache_inval().

Thanks,
Jean

Quoted 44 lines

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 135

Quoted 6 lines

include/uapi/linux/iommu.h +5 -0

+ *
+ */
+struct iommu_cache_invalidate_info {
+	struct iommu_cache_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;

A separate structure for hdr seems a little pointless.

removed

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 139

Quoted 5 lines

include/uapi/linux/iommu.h +4 -0

+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;

Really need some padding or packing here for any hope of having consistency with userspace.

Quoted 4 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 141

Quoted 2 lines

include/uapi/linux/iommu.h +2 -0

+	__u64		nr_pages;
+	__u32		pasid;

Sub-optimal ordering for packing/padding. Thanks,

I introduced some padding above. Is that OK?

Again if this introduces more noise than it helps I will simply rely on initial contributors for the respin of their series according to your comments. Also we if can't define generic enough structures for ARM and x86

Thanks

Eric

Quoted 8 lines

Alex

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 145

Quoted 4 lines

include/uapi/linux/iommu.h +3 -0

+	__u64		arch_id;
+	__u64		addr;
+};
 #endif /* _UAPI_IOMMU_H */

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Hi Alex,

Quoted 182 lines · Alex Williamson

Quoted 179 lines · On Tue, 8 Jan 2019 11:26:14 +0100 Eric Auger <[email protected]> wrote:

From: "Liu, Yi L" <[email protected]>

In any virtualization use case, when the first translation stage is "owned" by the guest OS, the host IOMMU driver has no knowledge of caching structure updates unless the guest invalidation activities are trapped by the virtualizer and passed down to the host.

Since the invalidation data are obtained from user space and will be written into physical IOMMU, we must allow security check at various layers. Therefore, generic invalidation data format are proposed here, model specific IOMMU drivers need to convert them into their own format.

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---
v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows to invalidate context caches on top of IOTLBs

v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in header. Commit message reworded.

---
 drivers/iommu/iommu.c      | 14 ++++++
 include/linux/iommu.h      | 14 ++++++
 include/uapi/linux/iommu.h | 95 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)

drivers/iommu/iommu.c +14 -0

index 0f2b7f1fc7c8..b2e248770508 100644
@@ -1403,6 +1403,20 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
 
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_cache_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->cache_invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index 1da2a2357ea4..96d59886f230 100644
@@ -186,6 +186,7 @@ struct iommu_resv_region {
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @set_pasid_table: set pasid table
+ * @cache_invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -231,6 +232,9 @@ struct iommu_ops {
 	int (*set_pasid_table)(struct iommu_domain *domain,
 			       struct iommu_pasid_table_config *cfg);
 
+	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -294,6 +298,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
 extern int iommu_set_pasid_table(struct iommu_domain *domain,
 				 struct iommu_pasid_table_config *cfg);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+				struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -709,6 +716,13 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 {
 	return -ENODEV;
 }
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+		       struct device *dev,
+		       struct iommu_cache_invalidate_info *inv_info)
+{
+	return -ENODEV;
+}
 
 #endif /* CONFIG_IOMMU_API */
 

include/uapi/linux/iommu.h +67 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 122

Quoted 6 lines

include/uapi/linux/iommu.h +6 -0

+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.  

Why is this a 4K page centric interface?

This matches the vt-d Address Mask (AM) field of the IOTLB Invalidate Descriptor. We can pass a log2size instead.

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 130

Quoted 8 lines

include/uapi/linux/iommu.h +8 -0

+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @arch_id:		architecture dependent id characterizing a context
+ *			and tagging the caches, ie. domain Identfier on VTD,
+ *			asid on ARM SMMU
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages  

Quoted 6 lines

Shouldn't some of these be tied the the granularity of the invalidation? It seems like this should be more similar to iommu_pasid_table_config where the granularity of the invalidation defines which entry within a union at the end of the structure is valid and populated. Otherwise we have fields that don't make sense for certain invalidations.

I am a little bit embarrassed here as this API version is the outcome of long discussions held by Jacob, jean-Philippe and many others. I don't want to hijack that work as I am "simply" reusing this API. Nevertheless I am willing to help on this. So following your recommendation above I dare to propose an updated API:

struct iommu_device_iotlb_inv_info {
        __u32   version;
#define IOMMU_DEV_IOTLB_INV_GLOBAL   0
#define IOMMU_DEV_IOTLB_INV_SOURCEID (1 << 0)
#define IOMMU_DEV_IOTLB_INV_PASID    (1 << 1)
        __u8    granularity;
        __u64   addr;
        __u8    log2size;
        __u64   sourceid;
        __u64   pasid;
        __u8    padding[2];
};

struct iommu_iotlb_inv_info {
        __u32   version;
#define IOMMU_IOTLB_INV_GLOBAL  0
#define IOMMU_IOTLB_INV_ARCHID  (1 << 0)
#define IOMMU_IOTLB_INV_PASID   (1 << 1)
#define IOMMU_IOTLB_INV_PAGE    (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   addr;
        __u8    log2size;
        __u8    padding[2];

};

struct iommu_pasid_inv_info {
        __u32   version;
#define IOMMU_PASID_INV_GLOBAL     0
#define IOMMU_PASID_INV_ARCHID     (1 << 0)
#define IOMMU_PASID_INV_PASID      (1 << 1)
#define IOMMU_PASID_INV_SOURCEID   (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   sourceid;
        __u8    padding[3];

};
/**
* Translation cache invalidation information, contains generic IOMMU
* data which can be parsed based on model ID by model specific drivers.
* Since the invalidation of second level page tables are included in
* the unmap operation, this info is only applicable to the first level
* translation caches, i.e. DMA request with PASID.
*
*/
struct iommu_cache_invalidate_info {

#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
        __u32 version;
#define IOMMU_INV_TYPE_IOTLB        1 /* IOMMU paging structure cache */
#define IOMMU_INV_TYPE_DEV_IOTLB    2 /* Device IOTLB */
#define IOMMU_INV_TYPE_PASID        3 /* PASID cache */
        __u8 type;
        union {
                struct iommu_iotlb_invalidate_info iotlb_inv_info;
                struct iommu_dev_iotlb_invalidate_info dev_iotlb_inv_info;
                struct iommu_pasid_inv_info pasid_inv_info;
        };
};

At the moment I ignore the leaf bool parameter used on ARM for PASID invalidation and TLB invalidation. Maybe we can just invalidate more that the leaf cache structures at the moment?

on ARM the PASID table can be invalidated per streamid. On vt-d, as far as I understand the sourceid does not tag the entries.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 135

Quoted 6 lines

include/uapi/linux/iommu.h +5 -0

+ *
+ */
+struct iommu_cache_invalidate_info {
+	struct iommu_cache_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;  

A separate structure for hdr seems a little pointless.

removed

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 139

Quoted 5 lines

include/uapi/linux/iommu.h +4 -0

+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;  

Really need some padding or packing here for any hope of having consistency with userspace.

Quoted 4 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 141

Quoted 2 lines

include/uapi/linux/iommu.h +2 -0

+	__u64		nr_pages;
+	__u32		pasid;  

Sub-optimal ordering for packing/padding. Thanks,

I introduced some padding above. Is that OK?

Quoted 193 lines · On Fri, 25 Jan 2019 17:49:20 +0100 Auger Eric <[email protected]> wrote:

Hi Alex,

Quoted 182 lines · Alex Williamson

Quoted 179 lines · On Tue, 8 Jan 2019 11:26:14 +0100 Eric Auger <[email protected]> wrote:

From: "Liu, Yi L" <[email protected]>

In any virtualization use case, when the first translation stage is "owned" by the guest OS, the host IOMMU driver has no knowledge of caching structure updates unless the guest invalidation activities are trapped by the virtualizer and passed down to the host.

Since the invalidation data are obtained from user space and will be written into physical IOMMU, we must allow security check at various layers. Therefore, generic invalidation data format are proposed here, model specific IOMMU drivers need to convert them into their own format.

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Jean-Philippe Brucker <[email protected]>

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Ashok Raj <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---
v1 -> v2:
- add arch_id field
- renamed tlb_invalidate into cache_invalidate as this API allows to invalidate context caches on top of IOTLBs

v1:
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in header. Commit message reworded.

---
 drivers/iommu/iommu.c      | 14 ++++++
 include/linux/iommu.h      | 14 ++++++
 include/uapi/linux/iommu.h | 95 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)

drivers/iommu/iommu.c +14 -0

index 0f2b7f1fc7c8..b2e248770508 100644
@@ -1403,6 +1403,20 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 }
 EXPORT_SYMBOL_GPL(iommu_set_pasid_table);
 
+int iommu_cache_invalidate(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_cache_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->cache_invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->cache_invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_cache_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {

include/linux/iommu.h +14 -0

index 1da2a2357ea4..96d59886f230 100644
@@ -186,6 +186,7 @@ struct iommu_resv_region {
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @set_pasid_table: set pasid table
+ * @cache_invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);

@@ -231,6 +232,9 @@ struct iommu_ops {
 	int (*set_pasid_table)(struct iommu_domain *domain,
 			       struct iommu_pasid_table_config *cfg);
 
+	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -294,6 +298,9 @@ extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
 extern int iommu_set_pasid_table(struct iommu_domain *domain,
 				 struct iommu_pasid_table_config *cfg);
+extern int iommu_cache_invalidate(struct iommu_domain *domain,
+				struct device *dev,
+				struct iommu_cache_invalidate_info *inv_info);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -709,6 +716,13 @@ int iommu_set_pasid_table(struct iommu_domain *domain,
 {
 	return -ENODEV;
 }
+static inline int
+iommu_cache_invalidate(struct iommu_domain *domain,
+		       struct device *dev,
+		       struct iommu_cache_invalidate_info *inv_info)
+{
+	return -ENODEV;
+}
 
 #endif /* CONFIG_IOMMU_API */
 

include/uapi/linux/iommu.h +67 -0

index 7a7cf7a3de7c..4605f5cfac84 100644
@@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
 	};
 };
 
+/**
+ * enum iommu_inv_granularity - Generic invalidation granularity
+ * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID:	TLB entries or PASID caches of all
+ *					PASIDs associated with a domain ID
+ * @IOMMU_INV_GRANU_PASID_SEL:		TLB entries or PASID cache associated
+ *					with a PASID and a domain
+ * @IOMMU_INV_GRANU_PAGE_PASID:		TLB entries of selected page range
+ *					within a PASID
+ *
+ * When an invalidation request is passed down to IOMMU to flush translation
+ * caches, it may carry different granularity levels, which can be specific
+ * to certain types of translation caches.
+ * This enum is a collection of granularities for all types of translation
+ * caches. The idea is to make it easy for IOMMU model specific driver to
+ * convert from generic to model specific value. Each IOMMU driver
+ * can enforce check based on its own conversion table. The conversion is
+ * based on 2D look-up with inputs as follows:
+ * - translation cache types
+ * - granularity
+ *
+ *             type |   DTLB    |    TLB    |   PASID   |
+ *  granule         |           |           |   cache   |
+ * -----------------+-----------+-----------+-----------+
+ *  DN_ALL_PASID    |   Y       |   Y       |   Y       |
+ *  PASID_SEL       |   Y       |   Y       |   Y       |
+ *  PAGE_PASID      |   Y       |   Y       |   N/A     |
+ *
+ */
+enum iommu_inv_granularity {
+	IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
+	IOMMU_INV_GRANU_PASID_SEL,
+	IOMMU_INV_GRANU_PAGE_PASID,
+	IOMMU_INV_NR_GRANU,
+};
+
+/**
+ * enum iommu_inv_type - Generic translation cache types for invalidation
+ *
+ * @IOMMU_INV_TYPE_DTLB:	device IOTLB
+ * @IOMMU_INV_TYPE_TLB:		IOMMU paging structure cache
+ * @IOMMU_INV_TYPE_PASID:	PASID cache
+ * Invalidation requests sent to IOMMU for a given device need to indicate
+ * which type of translation cache to be operated on. Combined with enum
+ * iommu_inv_granularity, model specific driver can do a simple lookup to
+ * convert from generic to model specific value.
+ */
+enum iommu_inv_type {
+	IOMMU_INV_TYPE_DTLB,
+	IOMMU_INV_TYPE_TLB,
+	IOMMU_INV_TYPE_PASID,
+	IOMMU_INV_NR_TYPE
+};
+
+/**
+ * Translation cache invalidation header that contains mandatory meta data.
+ * @version:	info format version, expecting future extesions
+ * @type:	type of translation cache to be invalidated
+ */
+struct iommu_cache_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 122

Quoted 6 lines

include/uapi/linux/iommu.h +6 -0

+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.  

Why is this a 4K page centric interface?

Quoted 2 lines

This matches the vt-d Address Mask (AM) field of the IOTLB Invalidate Descriptor. We can pass a log2size instead.

Could some options not require a power of two size?

Quoted 108 lines

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 130

Quoted 8 lines

include/uapi/linux/iommu.h +8 -0

+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @arch_id:		architecture dependent id characterizing a context
+ *			and tagging the caches, ie. domain Identfier on VTD,
+ *			asid on ARM SMMU
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages  

Quoted 6 lines

Shouldn't some of these be tied the the granularity of the invalidation? It seems like this should be more similar to iommu_pasid_table_config where the granularity of the invalidation defines which entry within a union at the end of the structure is valid and populated. Otherwise we have fields that don't make sense for certain invalidations.

I am a little bit embarrassed here as this API version is the outcome of long discussions held by Jacob, jean-Philippe and many others. I don't want to hijack that work as I am "simply" reusing this API. Nevertheless I am willing to help on this. So following your recommendation above I dare to propose an updated API:

struct iommu_device_iotlb_inv_info {
        __u32   version;
#define IOMMU_DEV_IOTLB_INV_GLOBAL   0
#define IOMMU_DEV_IOTLB_INV_SOURCEID (1 << 0)
#define IOMMU_DEV_IOTLB_INV_PASID    (1 << 1)
        __u8    granularity;
        __u64   addr;
        __u8    log2size;
        __u64   sourceid;
        __u64   pasid;
        __u8    padding[2];
};

struct iommu_iotlb_inv_info {
        __u32   version;
#define IOMMU_IOTLB_INV_GLOBAL  0
#define IOMMU_IOTLB_INV_ARCHID  (1 << 0)
#define IOMMU_IOTLB_INV_PASID   (1 << 1)
#define IOMMU_IOTLB_INV_PAGE    (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   addr;
        __u8    log2size;
        __u8    padding[2];

};

struct iommu_pasid_inv_info {
        __u32   version;
#define IOMMU_PASID_INV_GLOBAL     0
#define IOMMU_PASID_INV_ARCHID     (1 << 0)
#define IOMMU_PASID_INV_PASID      (1 << 1)
#define IOMMU_PASID_INV_SOURCEID   (1 << 2)
        __u8    granularity;
        __u64   archid;
        __u64   pasid;
        __u64   sourceid;
        __u8    padding[3];

};
/**
* Translation cache invalidation information, contains generic IOMMU
* data which can be parsed based on model ID by model specific drivers.
* Since the invalidation of second level page tables are included in
* the unmap operation, this info is only applicable to the first level
* translation caches, i.e. DMA request with PASID.
*
*/
struct iommu_cache_invalidate_info {

#define IOMMU_CACHE_INVALIDATE_INFO_VERSION_1 1
        __u32 version;
#define IOMMU_INV_TYPE_IOTLB        1 /* IOMMU paging structure cache */
#define IOMMU_INV_TYPE_DEV_IOTLB    2 /* Device IOTLB */
#define IOMMU_INV_TYPE_PASID        3 /* PASID cache */
        __u8 type;
        union {
                struct iommu_iotlb_invalidate_info iotlb_inv_info;
                struct iommu_dev_iotlb_invalidate_info dev_iotlb_inv_info;
                struct iommu_pasid_inv_info pasid_inv_info;
        };
};

At the moment I ignore the leaf bool parameter used on ARM for PASID invalidation and TLB invalidation. Maybe we can just invalidate more that the leaf cache structures at the moment?

on ARM the PASID table can be invalidated per streamid. On vt-d, as far as I understand the sourceid does not tag the entries.

Quoted 8 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 135

Quoted 6 lines

include/uapi/linux/iommu.h +5 -0

+ *
+ */
+struct iommu_cache_invalidate_info {
+	struct iommu_cache_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;  

A separate structure for hdr seems a little pointless.

removed

Quoted 9 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 139

Quoted 5 lines

include/uapi/linux/iommu.h +4 -0

+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;  

Really need some padding or packing here for any hope of having consistency with userspace.

Quoted 5 lines

Quoted 4 lines

include/uapi/linux/iommu.h struct iommu_pasid_table_config { · line 141

Quoted 2 lines

include/uapi/linux/iommu.h +2 -0

+	__u64		nr_pages;
+	__u32		pasid;  

Sub-optimal ordering for packing/padding. Thanks,

I introduced some padding above. Is that OK?

No, you're not taking field alignment into account, processors don't like unaligned data. If we have:

struct foo {
	uint32_t	a;
	uint8_t		b;
	uint64_t	c;
	uint8_t		d;
	uint64_t	e;
};

In memory on a 64 bit system, that would look like:

aaaab...ccccccccd.......eeeeeeee

While on a 32 bit system, it would look like:

aaaab...ccccccccd...eeeeeeee

In this example we have 22 bytes of data (4 + 1 + 8 + 1 + 8), but the structure is 32 bytes when provided by a 64 bit userspace or 28 bytes when provided by a 32 bit userspace and the start address of the 'e' field changes. A 64 bit kernel would process the latter structure incorrectly or fault trying to copy the expected length from userspace. Adding padding to the end doesn't solve this. If we instead reconstruct the structure as:

struct foo {
	uint32_t	a;
	uint8_t		b;
	uint8_t		d;
	uint8_t		pad[2];
	uint64_t	c;
	uint64_t	e;
};

Then we create a structure that looks the same from either a 32 bit or 64 bit userspace, is only 24 bytes in memory, and works for any reasonable compiler, though we might choose to add a packed attribute to make sure the compiler doesn't do anything screwy if we were paranoid. Thanks,

Alex

On ARM, MSI are translated by the SMMU. An IOVA is allocated for each MSI doorbell. If both the host and the guest are exposed with SMMUs, we end up with 2 different IOVAs allocated by each. guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB).

So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB

              hIOVA    ->    gDB

Currently the PCI device is programmed by the host with hIOVA as MSI doorbell. So this does not work.

This patch introduces an API to pass gIOVA/gDB to the host so that gIOVA can be reused by the host instead of re-allocating a new IOVA. So the goal is to create the following nested mapping:

S1 S2
gIOVA -> gDB -> hDB

and program the PCI device with gIOVA MSI doorbell.

Signed-off-by Eric Auger <[email protected]>

---

v2 -> v3:
- add a struct device handle

---
 drivers/iommu/iommu.c      | 10 ++++++++++
 include/linux/iommu.h      | 13 +++++++++++++
 include/uapi/linux/iommu.h |  6 ++++++
 3 files changed, 29 insertions(+)

drivers/iommu/iommu.c +10 -0

index b2e248770508..ea11442e7054 100644
@@ -1431,6 +1431,16 @@ static void __iommu_detach_device(struct iommu_domain *domain,
 	trace_detach_device_from_domain(dev);
 }
 
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	if (unlikely(!domain->ops->bind_guest_msi))
+		return -ENODEV;
+
+	return domain->ops->bind_guest_msi(domain, dev, binding);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	struct iommu_group *group;

include/linux/iommu.h +13 -0

index 96d59886f230..244c1a3d5989 100644
@@ -235,6 +235,9 @@ struct iommu_ops {
 	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
 
+	int (*bind_guest_msi)(struct iommu_domain *domain, struct device *dev,
+			      struct iommu_guest_msi_binding *binding);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -301,6 +304,9 @@ extern int iommu_set_pasid_table(struct iommu_domain *domain,
 extern int iommu_cache_invalidate(struct iommu_domain *domain,
 				struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+				struct iommu_guest_msi_binding *binding);
+
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -724,6 +730,13 @@ iommu_cache_invalidate(struct iommu_domain *domain,
 	return -ENODEV;
 }
 
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h +0 -0

index 4605f5cfac84..f28cd9a1aa96 100644

Quoted 7 lines · On Tue, 8 Jan 2019 11:26:15 +0100 Eric Auger <[email protected]> wrote:

On ARM, MSI are translated by the SMMU. An IOVA is allocated for each MSI doorbell. If both the host and the guest are exposed with SMMUs, we end up with 2 different IOVAs allocated by each. guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB).

Quoted 4 lines

So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB

              hIOVA    ->    gDB

                               ^^^ hDB

Quoted 93 lines

Currently the PCI device is programmed by the host with hIOVA as MSI doorbell. So this does not work.

This patch introduces an API to pass gIOVA/gDB to the host so that gIOVA can be reused by the host instead of re-allocating a new IOVA. So the goal is to create the following nested mapping:

S1 S2
gIOVA -> gDB -> hDB

and program the PCI device with gIOVA MSI doorbell.

Signed-off-by Eric Auger <[email protected]>

---

v2 -> v3:
- add a struct device handle

---
 drivers/iommu/iommu.c      | 10 ++++++++++
 include/linux/iommu.h      | 13 +++++++++++++
 include/uapi/linux/iommu.h |  6 ++++++
 3 files changed, 29 insertions(+)

drivers/iommu/iommu.c +10 -0

index b2e248770508..ea11442e7054 100644
@@ -1431,6 +1431,16 @@ static void __iommu_detach_device(struct iommu_domain *domain,
 	trace_detach_device_from_domain(dev);
 }
 
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	if (unlikely(!domain->ops->bind_guest_msi))
+		return -ENODEV;
+
+	return domain->ops->bind_guest_msi(domain, dev, binding);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	struct iommu_group *group;

include/linux/iommu.h +13 -0

index 96d59886f230..244c1a3d5989 100644
@@ -235,6 +235,9 @@ struct iommu_ops {
 	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
 
+	int (*bind_guest_msi)(struct iommu_domain *domain, struct device *dev,
+			      struct iommu_guest_msi_binding *binding);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -301,6 +304,9 @@ extern int iommu_set_pasid_table(struct iommu_domain *domain,
 extern int iommu_cache_invalidate(struct iommu_domain *domain,
 				struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+				struct iommu_guest_msi_binding *binding);
+
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -724,6 +730,13 @@ iommu_cache_invalidate(struct iommu_domain *domain,
 	return -ENODEV;
 }
 
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h +1 -0

index 4605f5cfac84..f28cd9a1aa96 100644
@@ -142,4 +142,10 @@ struct iommu_cache_invalidate_info {
 	__u64		arch_id;
 	__u64		addr;
 };
+

include/uapi/linux/iommu.h struct iommu_cache_invalidate_info { · line 149

Quoted 4 lines

include/uapi/linux/iommu.h +4 -0

+struct iommu_guest_msi_binding {
+	__u64		iova;
+	__u64		gpa;
+	__u32		granule;

What's granule? The size? This looks a lot like just a stage 1 mapping interface, I can't really figure out from the description how this matches to any specific MSI mapping.

or headers here about how this is supposed to work. Thanks,

Alex

Quoted 11 lines · On Tue, 8 Jan 2019 11:26:15 +0100 Eric Auger <[email protected]> wrote:

On ARM, MSI are translated by the SMMU. An IOVA is allocated for each MSI doorbell. If both the host and the guest are exposed with SMMUs, we end up with 2 different IOVAs allocated by each. guest allocates an IOVA (gIOVA) to map onto the guest MSI doorbell (gDB). The Host allocates another IOVA (hIOVA) to map onto the physical doorbell (hDB).

So we end up with 2 untied mappings:
S1 S2
gIOVA -> gDB

              hIOVA    ->    gDB

                               ^^^ hDB

Quoted 93 lines

Currently the PCI device is programmed by the host with hIOVA as MSI doorbell. So this does not work.

This patch introduces an API to pass gIOVA/gDB to the host so that gIOVA can be reused by the host instead of re-allocating a new IOVA. So the goal is to create the following nested mapping:

S1 S2
gIOVA -> gDB -> hDB

and program the PCI device with gIOVA MSI doorbell.

Signed-off-by Eric Auger <[email protected]>

---

v2 -> v3:
- add a struct device handle

---
 drivers/iommu/iommu.c      | 10 ++++++++++
 include/linux/iommu.h      | 13 +++++++++++++
 include/uapi/linux/iommu.h |  6 ++++++
 3 files changed, 29 insertions(+)

drivers/iommu/iommu.c +10 -0

index b2e248770508..ea11442e7054 100644
@@ -1431,6 +1431,16 @@ static void __iommu_detach_device(struct iommu_domain *domain,
 	trace_detach_device_from_domain(dev);
 }
 
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	if (unlikely(!domain->ops->bind_guest_msi))
+		return -ENODEV;
+
+	return domain->ops->bind_guest_msi(domain, dev, binding);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
 void iommu_detach_device(struct iommu_domain *domain, struct device *dev)
 {
 	struct iommu_group *group;

include/linux/iommu.h +13 -0

index 96d59886f230..244c1a3d5989 100644
@@ -235,6 +235,9 @@ struct iommu_ops {
 	int (*cache_invalidate)(struct iommu_domain *domain, struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
 
+	int (*bind_guest_msi)(struct iommu_domain *domain, struct device *dev,
+			      struct iommu_guest_msi_binding *binding);
+
 	unsigned long pgsize_bitmap;
 };
 

@@ -301,6 +304,9 @@ extern int iommu_set_pasid_table(struct iommu_domain *domain,
 extern int iommu_cache_invalidate(struct iommu_domain *domain,
 				struct device *dev,
 				struct iommu_cache_invalidate_info *inv_info);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+				struct iommu_guest_msi_binding *binding);
+
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern struct iommu_domain *iommu_get_dma_domain(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,

@@ -724,6 +730,13 @@ iommu_cache_invalidate(struct iommu_domain *domain,
 	return -ENODEV;
 }
 
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_guest_msi_binding *binding)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

include/uapi/linux/iommu.h +2 -0

index 4605f5cfac84..f28cd9a1aa96 100644
@@ -142,4 +142,10 @@ struct iommu_cache_invalidate_info {
 	__u64		arch_id;
 	__u64		addr;
 };
+
+struct iommu_guest_msi_binding {

include/uapi/linux/iommu.h struct iommu_cache_invalidate_info { · line 149

Quoted 3 lines

include/uapi/linux/iommu.h +3 -0

+	__u64		iova;
+	__u64		gpa;
+	__u32		granule;

What's granule? The size? This looks a lot like just a stage 1 mapping interface, I can't really figure out from the description how this matches to any specific MSI mapping. Zero comments in the code or headers here about how this is supposed to work. Thanks,

Alex

From: "Liu, Yi L" <[email protected]>

This patch adds VFIO_IOMMU_SET_PASID_TABLE ioctl which aims at passing the virtual iommu guest configuration to the VFIO driver downto to the iommu subsystem.

Signed-off-by Jacob Pan <[email protected]>

Signed-off-by Liu, Yi L <[email protected]>

Signed-off-by Eric Auger <[email protected]>

---
v2 -> v3:
- s/BIND_PASID_TABLE/SET_PASID_TABLE

v1 -> v2:
- s/BIND_GUEST_STAGE/BIND_PASID_TABLE
- remove the struct device arg

---
 drivers/vfio/vfio_iommu_type1.c | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h       |  8 ++++++++
 2 files changed, 39 insertions(+)

drivers/vfio/vfio_iommu_type1.c +4 -0

index 7651cfb14836..d9dd23f64f00 100644
@@ -1644,6 +1644,24 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
 	return ret;
 }
 
+static int
+vfio_set_pasid_table(struct vfio_iommu *iommu,
+		      struct vfio_iommu_type1_set_pasid_table *ustruct)
+{