[RFC 00/13] SMMUv3 Nested Stage Setup

[RFC 00/13] SMMUv3 Nested Stage Setup

[Eric Auger]

This series allows a virtualizer to program the nested stage mode.
This is useful when both the host and the guest are exposed with
an SMMUv3 and a PCI device is assigned to the guest using VFIO.

In this mode, the physical IOMMU must be programmed to translate
the two stages: the one set up by the guest (IOVA -> GPA) and the
one set up by the host VFIO driver as part of the assignment process
(GPA -> HPA).

On Intel, this is traditionnaly achieved by combining the 2 stages
into a single physical stage. However this relies on the capability
to trap on each guest translation structure update. This is possible
by using the VTD Caching Mode. Unfortunately the ARM SMMUv3 does
not offer a similar mechanism.

However, the ARM SMMUv3 architecture supports 2 physical stages! Those
were devised exactly with that use case in mind. Assuming the HW
implements both stages (optional), the guest now can use stage 1
while the host uses stage 2.

This assumes the virtualizer has means to propagate guest settings
to the host SMMUv3 driver. This series brings this VFIO/IOMMU
infrastructure.  Those services are:
- bind the guest stage 1 configuration to the stream table entry
- propagate guest TLB invalidations
- bind MSI IOVAs

This series largely reuses the infrastructure devised for SVA/SVM
and patches submitted by Jacob and Liu in [1] and [2] and
Jean-Philippe [3], with some generalizations to adapt to this use case.

At the moment, this series does not implement fault reporting to
the guest. This will be added later on.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/linux/tree/v4.18-2stage-rfc

This was tested on Qualcomm HW featuring SMMUv3 and with adapted QEMU
vSMMUv3.

References:
[1] [PATCH v5 00/23] IOMMU and VT-d driver support for Shared Virtual
    Address (SVA)
    https://lwn.net/Articles/754331/
[2] [RFC PATCH 0/8] Shared Virtual Memory virtualization for VT-d
    (VFIO part)
    https://lists.linuxfoundation.org/pipermail/iommu/2017-April/021475.html
[3] [v2,17/40] iommu/arm-smmu-v3: Link domains and devices
    https://patchwork.kernel.org/patch/10395187/

Eric Auger (8):
  iommu: Introduce bind_guest_msi
  vfio: VFIO_IOMMU_BIND_MSI
  vfio: Document nested stage control
  iommu/smmuv3: Get prepared for nested stage support
  iommu/smmuv3: Implement bind_guest_stage
  iommu/smmuv3: Implement tlb_invalidate
  dma-iommu: Implement NESTED_MSI cookie
  iommu/smmuv3: Implement bind_guest_msi

Jacob Pan (1):
  iommu: Introduce bind_guest_stage API

Jean-Philippe Brucker (1):
  iommu/arm-smmu-v3: Link domains and devices

Liu, Yi L (3):
  iommu: Introduce tlb_invalidate API
  vfio: VFIO_IOMMU_BIND_GUEST_STAGE
  vfio: VFIO_IOMMU_TLB_INVALIDATE

 Documentation/vfio.txt          |  45 ++++++++++
 drivers/iommu/arm-smmu-v3.c     | 179 ++++++++++++++++++++++++++++++++++++++--
 drivers/iommu/dma-iommu.c       |  88 +++++++++++++++++++-
 drivers/iommu/iommu.c           |  43 ++++++++++
 drivers/vfio/vfio_iommu_type1.c | 110 ++++++++++++++++++++++++
 include/linux/dma-iommu.h       |  11 +++
 include/linux/iommu.h           |  48 +++++++++++
 include/uapi/linux/iommu.h      | 157 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h       |  22 +++++
 9 files changed, 692 insertions(+), 11 deletions(-)
 create mode 100644 include/uapi/linux/iommu.h

-- 
2.5.5

[RFC 01/13] iommu: Introduce bind_guest_stage API

[Eric Auger]

From: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

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 while to host owns the stage 2.

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

This patch adds a new API to the iommu subsystem that allows
to bind and unbind the guest configuration data to the host.

A generic iommu_guest_stage_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 2 specialization of this struct,
for PASID table passing and ARM SMMUv3.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

This patch generalizes the API introduced by Jacob & co-authors in
https://lwn.net/Articles/754331/
---
 drivers/iommu/iommu.c      | 19 +++++++++++++++
 include/linux/iommu.h      | 23 ++++++++++++++++++
 include/uapi/linux/iommu.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 include/uapi/linux/iommu.h

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 63b3756..5156172 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1326,6 +1326,25 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_attach_device);
 
+int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_guest_stage_config *cfg)
+{
+	if (unlikely(!domain->ops->bind_guest_stage))
+		return -ENODEV;
+
+	return domain->ops->bind_guest_stage(domain, dev, cfg);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_stage);
+
+void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
+{
+	if (unlikely(!domain->ops->unbind_guest_stage))
+		return;
+
+	domain->ops->unbind_guest_stage(domain, dev);
+}
+EXPORT_SYMBOL_GPL(iommu_unbind_guest_stage);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee..aec853d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -25,6 +25,7 @@
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/of.h>
+#include <uapi/linux/iommu.h>
 
 #define IOMMU_READ	(1 << 0)
 #define IOMMU_WRITE	(1 << 1)
@@ -187,6 +188,8 @@ struct iommu_resv_region {
  * @domain_get_windows: Return the number of windows for a domain
  * @of_xlate: add OF master IDs to iommu grouping
  * @pgsize_bitmap: bitmap of all possible supported page sizes
+ * @bind_guest_stage: program the guest stage configuration
+ * @unbind_guest_stage: unbind the guest stage and restore defaults
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);
@@ -235,6 +238,11 @@ struct iommu_ops {
 	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
 	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
 
+	int (*bind_guest_stage)(struct iommu_domain *domain, struct device *dev,
+				struct iommu_guest_stage_config *cfg);
+	void (*unbind_guest_stage)(struct iommu_domain *domain,
+				   struct device *dev);
+
 	unsigned long pgsize_bitmap;
 };
 
@@ -296,6 +304,10 @@ extern int iommu_attach_device(struct iommu_domain *domain,
 			       struct device *dev);
 extern void iommu_detach_device(struct iommu_domain *domain,
 				struct device *dev);
+extern int iommu_bind_guest_stage(struct iommu_domain *domain,
+		struct device *dev, struct iommu_guest_stage_config *cfg);
+extern void iommu_unbind_guest_stage(struct iommu_domain *domain,
+				     struct device *dev);
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		     phys_addr_t paddr, size_t size, int prot);
@@ -696,6 +708,17 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
 	return NULL;
 }
 
+static inline
+int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
+			   struct iommu_guest_stage_config *cfg)
+{
+	return -ENODEV;
+}
+static inline
+void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
+{
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
new file mode 100644
index 0000000..2d1593c
--- /dev/null
+++ b/include/uapi/linux/iommu.h
@@ -0,0 +1,59 @@
+/* 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>
+
+/**
+ * PASID table data used to bind guest PASID table to the host IOMMU. This will
+ * enable guest managed first level page tables.
+ * @version: for future extensions and identification of the data format
+ * @bytes: size of this structure
+ * @base_ptr:	PASID table pointer
+ * @pasid_bits:	number of bits supported in the guest PASID table, must be less
+ *		or equal than the host supported PASID size.
+ */
+struct iommu_pasid_table_config {
+	__u32 version;
+#define PASID_TABLE_CFG_VERSION_1 1
+	__u32 bytes;
+	__u64 base_ptr;
+	__u8 pasid_bits;
+};
+
+/**
+ * Stream Table Entry S1 info
+ * @disabled: the smmu is disabled
+ * @bypassed: stage 1 is bypassed
+ * @aborted: aborted
+ * @cdptr_dma: GPA of the Context Descriptor
+ * @asid: address space identified associated to the CD
+ */
+struct iommu_smmu_s1_config {
+	__u8 disabled;
+	__u8 bypassed;
+	__u8 aborted;
+	__u64 cdptr_dma;
+	__u16 asid;
+};
+
+struct iommu_guest_stage_config {
+#define PASID_TABLE	(1 << 0)
+#define SMMUV3_S1_CFG	(1 << 1)
+	__u32 flags;
+	union {
+		struct iommu_pasid_table_config pasidt;
+		struct iommu_smmu_s1_config smmu_s1;
+	};
+};
+
+#endif /* _UAPI_IOMMU_H */
-- 
2.5.5

[RFC 02/13] iommu: Introduce tlb_invalidate API

[Eric Auger]

From: "Liu, Yi L" <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

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

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

Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---
renamed sva_invalidate into tlb_invalidate and add iommu_ prefix in
header. Commit message reworded.
---
 drivers/iommu/iommu.c      | 14 +++++++
 include/linux/iommu.h      | 13 +++++++
 include/uapi/linux/iommu.h | 91 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5156172..0bc05a7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1345,6 +1345,20 @@ void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
 }
 EXPORT_SYMBOL_GPL(iommu_unbind_guest_stage);
 
+int iommu_tlb_invalidate(struct iommu_domain *domain, struct device *dev,
+			 struct iommu_tlb_invalidate_info *inv_info)
+{
+	int ret = 0;
+
+	if (unlikely(!domain->ops->tlb_invalidate))
+		return -ENODEV;
+
+	ret = domain->ops->tlb_invalidate(domain, dev, inv_info);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_tlb_invalidate);
+
 static void __iommu_detach_device(struct iommu_domain *domain,
 				  struct device *dev)
 {
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index aec853d..b13cf2b 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -190,6 +190,7 @@ struct iommu_resv_region {
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  * @bind_guest_stage: program the guest stage configuration
  * @unbind_guest_stage: unbind the guest stage and restore defaults
+ * @tlb_invalidate: invalidate translation caches
  */
 struct iommu_ops {
 	bool (*capable)(enum iommu_cap);
@@ -243,6 +244,9 @@ struct iommu_ops {
 	void (*unbind_guest_stage)(struct iommu_domain *domain,
 				   struct device *dev);
 
+	int (*tlb_invalidate)(struct iommu_domain *domain, struct device *dev,
+			      struct iommu_tlb_invalidate_info *inv_info);
+
 	unsigned long pgsize_bitmap;
 };
 
@@ -308,6 +312,10 @@ extern int iommu_bind_guest_stage(struct iommu_domain *domain,
 		struct device *dev, struct iommu_guest_stage_config *cfg);
 extern void iommu_unbind_guest_stage(struct iommu_domain *domain,
 				     struct device *dev);
+extern int iommu_tlb_invalidate(struct iommu_domain *domain,
+				struct device *dev,
+				struct iommu_tlb_invalidate_info *inv_info);
+
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
 		     phys_addr_t paddr, size_t size, int prot);
@@ -718,6 +726,11 @@ static inline
 void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
 {
 }
+static inline int iommu_tlb_invalidate(struct iommu_domain *domain,
+		struct device *dev, struct iommu_tlb_invalidate_info *inv_info)
+{
+	return -ENODEV;
+}
 
 #endif /* CONFIG_IOMMU_API */
 
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 2d1593c..7c88d80 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -56,4 +56,95 @@ struct iommu_guest_stage_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_tlb_invalidate_hdr {
+	__u32 version;
+#define TLB_INV_HDR_VERSION_1 1
+	enum iommu_inv_type type;
+};
+
+/**
+ * Translation cache invalidation information, contains generic IOMMU
+ * data which can be parsed based on model ID by model specific drivers.
+ * Since the invalidation of second level page tables are included in the
+ * unmap operation, this info is only applicable to the first level
+ * translation caches, i.e. DMA request with PASID.
+ *
+ * @granularity:	requested invalidation granularity, type dependent
+ * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.
+ * @nr_pages:		number of pages to invalidate
+ * @pasid:		processor address space ID value per PCI spec.
+ * @addr:		page address to be invalidated
+ * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
+ *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages
+ *
+ */
+struct iommu_tlb_invalidate_info {
+	struct iommu_tlb_invalidate_hdr	hdr;
+	enum iommu_inv_granularity	granularity;
+	__u32		flags;
+#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
+#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
+	__u8		size;
+	__u64		nr_pages;
+	__u32		pasid;
+	__u64		addr;
+};
 #endif /* _UAPI_IOMMU_H */
-- 
2.5.5

[RFC 03/13] iommu: Introduce bind_guest_msi

[Eric Auger]

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

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

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

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

         S1            S2
gIOVA    ->    gDB     ->    hDB

and program the PCI device with gIOVA MSI doorbell.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/iommu.c      | 10 ++++++++++
 include/linux/iommu.h      | 12 ++++++++++++
 include/uapi/linux/iommu.h |  7 +++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0bc05a7..ab8968f 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1336,6 +1336,16 @@ int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
 }
 EXPORT_SYMBOL_GPL(iommu_bind_guest_stage);
 
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+			 struct iommu_guest_msi_binding *binding)
+{
+	if (unlikely(!domain->ops->bind_guest_msi))
+		return -ENODEV;
+
+	return domain->ops->bind_guest_msi(domain, binding);
+}
+EXPORT_SYMBOL_GPL(iommu_bind_guest_msi);
+
 void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
 {
 	if (unlikely(!domain->ops->unbind_guest_stage))
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b13cf2b..ff8ae5f0 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -247,6 +247,9 @@ struct iommu_ops {
 	int (*tlb_invalidate)(struct iommu_domain *domain, struct device *dev,
 			      struct iommu_tlb_invalidate_info *inv_info);
 
+	int (*bind_guest_msi)(struct iommu_domain *domain,
+			      struct iommu_guest_msi_binding *binding);
+
 	unsigned long pgsize_bitmap;
 };
 
@@ -315,6 +318,8 @@ extern void iommu_unbind_guest_stage(struct iommu_domain *domain,
 extern int iommu_tlb_invalidate(struct iommu_domain *domain,
 				struct device *dev,
 				struct iommu_tlb_invalidate_info *inv_info);
+extern int iommu_bind_guest_msi(struct iommu_domain *domain,
+				struct iommu_guest_msi_binding *binding);
 
 extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
 extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
@@ -732,6 +737,13 @@ static inline int iommu_tlb_invalidate(struct iommu_domain *domain,
 	return -ENODEV;
 }
 
+static inline
+int iommu_bind_guest_msi(struct iommu_domain *domain,
+			 struct iommu_guest_msi_binding *binding)
+{
+	return -ENODEV;
+}
+
 #endif /* CONFIG_IOMMU_API */
 
 #endif /* __LINUX_IOMMU_H */
diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
index 7c88d80..7de9ab1 100644
--- a/include/uapi/linux/iommu.h
+++ b/include/uapi/linux/iommu.h
@@ -147,4 +147,11 @@ struct iommu_tlb_invalidate_info {
 	__u32		pasid;
 	__u64		addr;
 };
+
+struct iommu_guest_msi_binding {
+	__u64		iova;
+	__u64		gpa;
+	__u32		granule;
+};
 #endif /* _UAPI_IOMMU_H */
+
-- 
2.5.5

[RFC 04/13] vfio: VFIO_IOMMU_BIND_GUEST_STAGE

[Eric Auger]

From: "Liu, Yi L" <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

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

Signed-off-by: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

for SMMUV3 S1 config data passing the device parameter is not needed.
---
 drivers/vfio/vfio_iommu_type1.c | 56 +++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h       |  8 ++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index d9fd318..83c7f96 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1673,6 +1673,48 @@ static int vfio_domains_have_iommu_cache(struct vfio_iommu *iommu)
 	return ret;
 }
 
+struct vfio_iommu_task {
+	struct iommu_domain *domain;
+	void *payload;
+};
+
+static int bind_guest_stage_fn(struct device *dev, void *data)
+{
+	struct vfio_iommu_type1_bind_guest_stage *ustruct;
+	struct vfio_iommu_task *task = data;
+
+	ustruct = task->payload;
+	return iommu_bind_guest_stage(task->domain, dev, &ustruct->config);
+}
+
+static int vfio_iommu_dispatch_task(struct vfio_iommu *iommu, void *data,
+				    int (*fn)(struct device *, void *))
+{
+	int ret = 0;
+	struct vfio_domain *d;
+	struct vfio_group *g;
+	struct vfio_iommu_task task;
+
+	task.payload = data;
+
+	mutex_lock(&iommu->lock);
+
+	list_for_each_entry(d, &iommu->domain_list, next) {
+		list_for_each_entry(g, &d->group_list, next) {
+			if (g->iommu_group != NULL) {
+				task.domain = d->domain;
+				ret = iommu_group_for_each_dev(
+					g->iommu_group, &task, fn);
+				if (ret != 0)
+					break;
+			}
+		}
+	}
+
+	mutex_unlock(&iommu->lock);
+	return ret;
+}
+
 static long vfio_iommu_type1_ioctl(void *iommu_data,
 				   unsigned int cmd, unsigned long arg)
 {
@@ -1743,6 +1785,20 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 
 		return copy_to_user((void __user *)arg, &unmap, minsz) ?
 			-EFAULT : 0;
+	} else if (cmd == VFIO_IOMMU_BIND_GUEST_STAGE) {
+		struct vfio_iommu_type1_bind_guest_stage ustruct;
+
+		minsz = offsetofend(struct vfio_iommu_type1_bind_guest_stage,
+				    config);
+
+		if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (ustruct.argsz < minsz || ustruct.flags)
+			return -EINVAL;
+
+		return vfio_iommu_dispatch_task(iommu, (u8 *)&ustruct,
+						bind_guest_stage_fn);
 	}
 
 	return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 1aa7b82..a5801ed 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -14,6 +14,7 @@
 
 #include <linux/types.h>
 #include <linux/ioctl.h>
+#include <linux/iommu.h>
 
 #define VFIO_API_VERSION	0
 
@@ -665,6 +666,13 @@ struct vfio_iommu_type1_dma_unmap {
 #define VFIO_IOMMU_ENABLE	_IO(VFIO_TYPE, VFIO_BASE + 15)
 #define VFIO_IOMMU_DISABLE	_IO(VFIO_TYPE, VFIO_BASE + 16)
 
+struct vfio_iommu_type1_bind_guest_stage {
+	__u32	argsz;
+	__u32	flags;
+	struct iommu_guest_stage_config config;
+};
+#define VFIO_IOMMU_BIND_GUEST_STAGE	_IO(VFIO_TYPE, VFIO_BASE + 22)
+
 /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
 
 /*
-- 
2.5.5

[RFC 05/13] vfio: VFIO_IOMMU_TLB_INVALIDATE

[Eric Auger]

From: "Liu, Yi L" <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

This patch adds the VFIO_IOMMU_TLB_INVALIDATE ioctl with aims
at propagating stage1 IOMMU TLB invalidations from guest to host.

In the case of SVM virtualization on VT-d, host IOMMU driver has
no knowledge of caching structure updates unless the guest
invalidation activities are passed down to the host. So a new
IOCTL is needed to propagate the guest cache invalidation through
VFIO.

Signed-off-by: Liu, Yi L <yi.l.liu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

For SMMUv3 invalidations, the device parameter is not needed
---
 drivers/vfio/vfio_iommu_type1.c | 23 +++++++++++++++++++++++
 include/uapi/linux/vfio.h       |  7 +++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 83c7f96..973bb4d 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1687,6 +1687,15 @@ static int bind_guest_stage_fn(struct device *dev, void *data)
 	return iommu_bind_guest_stage(task->domain, dev, &ustruct->config);
 }
 
+static int do_tlb_inv_fn(struct device *dev, void *data)
+{
+	struct vfio_iommu_type1_tlb_invalidate *ustruct;
+	struct vfio_iommu_task *task = data;
+
+	ustruct = task->payload;
+	return iommu_tlb_invalidate(task->domain, dev, &ustruct->info);
+}
+
 static int vfio_iommu_dispatch_task(struct vfio_iommu *iommu, void *data,
 				    int (*fn)(struct device *, void *))
 {
@@ -1799,6 +1808,20 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 
 		return vfio_iommu_dispatch_task(iommu, (u8 *)&ustruct,
 						bind_guest_stage_fn);
+	} else if (cmd == VFIO_IOMMU_TLB_INVALIDATE) {
+		struct vfio_iommu_type1_tlb_invalidate ustruct;
+
+		minsz = offsetofend(struct vfio_iommu_type1_tlb_invalidate,
+				    info);
+
+		if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (ustruct.argsz < minsz || ustruct.flags)
+			return -EINVAL;
+
+		return vfio_iommu_dispatch_task(iommu, (u8 *)&ustruct,
+						do_tlb_inv_fn);
 	}
 
 	return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index a5801ed..b59f9fc 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -673,6 +673,13 @@ struct vfio_iommu_type1_bind_guest_stage {
 };
 #define VFIO_IOMMU_BIND_GUEST_STAGE	_IO(VFIO_TYPE, VFIO_BASE + 22)
 
+struct vfio_iommu_type1_tlb_invalidate {
+	__u32   argsz;
+	__u32   flags;
+	struct iommu_tlb_invalidate_info info;
+};
+#define VFIO_IOMMU_TLB_INVALIDATE      _IO(VFIO_TYPE, VFIO_BASE + 23)
+
 /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
 
 /*
-- 
2.5.5

[RFC 06/13] vfio: VFIO_IOMMU_BIND_MSI

[Eric Auger]

This patch adds the VFIO_IOMMU_BIND_MSI ioctl which aims at
passing the guest MSI binding to the host.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/vfio/vfio_iommu_type1.c | 31 +++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h       |  7 +++++++
 2 files changed, 38 insertions(+)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 973bb4d..2962cb9 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -1696,6 +1696,24 @@ static int do_tlb_inv_fn(struct device *dev, void *data)
 	return iommu_tlb_invalidate(task->domain, dev, &ustruct->info);
 }
 
+static int
+vfio_iommu_bind_guest_msi(struct vfio_iommu *iommu,
+			  struct vfio_iommu_type1_guest_msi_binding *ustruct)
+{
+	struct vfio_domain *d;
+	int ret;
+
+	mutex_lock(&iommu->lock);
+	list_for_each_entry(d, &iommu->domain_list, next) {
+		ret = iommu_bind_guest_msi(d->domain, &ustruct->binding);
+		if (ret)
+			break;
+	}
+	mutex_unlock(&iommu->lock);
+	return ret;
+}
+
+
 static int vfio_iommu_dispatch_task(struct vfio_iommu *iommu, void *data,
 				    int (*fn)(struct device *, void *))
 {
@@ -1822,6 +1840,19 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
 
 		return vfio_iommu_dispatch_task(iommu, (u8 *)&ustruct,
 						do_tlb_inv_fn);
+	} else if (cmd == VFIO_IOMMU_BIND_MSI) {
+		struct vfio_iommu_type1_guest_msi_binding ustruct;
+
+		minsz = offsetofend(struct vfio_iommu_type1_guest_msi_binding,
+				    binding);
+
+		if (copy_from_user(&ustruct, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (ustruct.argsz < minsz || ustruct.flags)
+			return -EINVAL;
+
+		return vfio_iommu_bind_guest_msi(iommu, &ustruct);
 	}
 
 	return -ENOTTY;
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index b59f9fc..b7a9943 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -680,6 +680,13 @@ struct vfio_iommu_type1_tlb_invalidate {
 };
 #define VFIO_IOMMU_TLB_INVALIDATE      _IO(VFIO_TYPE, VFIO_BASE + 23)
 
+struct vfio_iommu_type1_guest_msi_binding {
+	__u32   argsz;
+	__u32   flags;
+	struct iommu_guest_msi_binding binding;
+};
+#define VFIO_IOMMU_BIND_MSI      _IO(VFIO_TYPE, VFIO_BASE + 24)
+
 /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */
 
 /*
-- 
2.5.5

[RFC 07/13] vfio: Document nested stage control

[Eric Auger]

New iotcls were introduced to pass information about guest stage1
to the host through VFIO. Let's document the nested stage control.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

fault reporting is current missing to the picture
---
 Documentation/vfio.txt | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index f1a4d3c..858a363 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -239,6 +239,51 @@ group and can access them as follows::
 	/* Gratuitous device reset and go... */
 	ioctl(device, VFIO_DEVICE_RESET);
 
+IOMMU Dual Stage Control
+------------------------
+
+Some IOMMUs support 2 stages of translation. This is useful when the
+guest is exposed with a virtual IOMMU and some devices are assigned
+to the guest through VFIO. Then the guest OS can use stage 1 (IOVA -> GPA),
+while the hypervisor uses stage 2 for VM isolation (GPA -> HPA).
+
+The guest gets ownership of the stage 1 page tables and also owns stage 1
+configuration structures. The hypervisor owns the root configuration structure
+(for security reason), including stage 2 configuration. This works as long
+configuration structures and page table format are compatible between the
+virtual IOMMU and the physical IOMMU.
+
+Assuming the HW supports it, this nested mode is selected by choosing the
+VFIO_TYPE1_NESTING_IOMMU type through:
+
+ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_NESTING_IOMMU);
+
+This forces the hypervisor to use the stage 2, leaving stage 1 available for
+guest usage.
+
+Once groups are attached to the container, the guest stage 1 translation
+configuration data can be passed to VFIO by using
+
+ioctl(container, VFIO_IOMMU_BIND_GUEST_STAGE, &guest_stage_info);
+
+This allows to combine guest stage1 configuration structure along with hypervisor
+stage 2 configuration structure. stage 1 configuration structures are dependent
+on the IOMMU type.
+
+When the guest invalidates stage 1 entries, IOTLB invalidations must be forwarded
+to the host through
+ioctl(container, VFIO_IOMMU_TLB_INVALIDATE, &inv_data);
+Those invalidations can happen at various granularity levels, page, context, ...
+
+The ARM SMMU specification introduces another challenge: MSIs are translated by
+both the virtual SMMU and the physical SMMU. To build a nested mapping for the
+IOVA programmed into the assigned device, the guest needs to pass its IOVA/MSI
+doorbell GPA binding to the host. Then the hypervisor can build a nested stage 2
+binding eventually translating into the physical MSI doorbell.
+
+This is achieved by
+ioctl(container, VFIO_IOMMU_BIND_MSI, &guest_binding);
+
 VFIO User API
 -------------------------------------------------------------------------------
 
-- 
2.5.5

[RFC 08/13] iommu/arm-smmu-v3: Link domains and devices

[Eric Auger]

From: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>

When removing a mapping from a domain, we need to send an invalidation to
all devices that might have stored it in their Address Translation Cache
(ATC). In addition with SVM, we'll need to invalidate context descriptors
of all devices attached to a live domain.

Maintain a list of devices in each domain, protected by a spinlock. It is
updated every time we attach or detach devices to and from domains.

It needs to be a spinlock because we'll invalidate ATC entries from
within hardirq-safe contexts, but it may be possible to relax the read
side with RCU later.

Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker-5wv7dgnIgG8@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 22bdabd..19c085d 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -595,6 +595,11 @@ struct arm_smmu_device {
 struct arm_smmu_master_data {
 	struct arm_smmu_device		*smmu;
 	struct arm_smmu_strtab_ent	ste;
+
+	struct arm_smmu_domain		*domain;
+	struct list_head		list; /* domain->devices */
+
+	struct device			*dev;
 };
 
 /* SMMU private data for an IOMMU domain */
@@ -618,6 +623,9 @@ struct arm_smmu_domain {
 	};
 
 	struct iommu_domain		domain;
+
+	struct list_head		devices;
+	spinlock_t			devices_lock;
 };
 
 struct arm_smmu_option_prop {
@@ -1470,6 +1478,9 @@ static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
 	}
 
 	mutex_init(&smmu_domain->init_mutex);
+	INIT_LIST_HEAD(&smmu_domain->devices);
+	spin_lock_init(&smmu_domain->devices_lock);
+
 	return &smmu_domain->domain;
 }
 
@@ -1685,7 +1696,17 @@ static void arm_smmu_install_ste_for_dev(struct iommu_fwspec *fwspec)
 
 static void arm_smmu_detach_dev(struct device *dev)
 {
+	unsigned long flags;
 	struct arm_smmu_master_data *master = dev->iommu_fwspec->iommu_priv;
+	struct arm_smmu_domain *smmu_domain = master->domain;
+
+	if (smmu_domain) {
+		spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+		list_del(&master->list);
+		spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+		master->domain = NULL;
+	}
 
 	master->ste.assigned = false;
 	arm_smmu_install_ste_for_dev(dev->iommu_fwspec);
@@ -1694,6 +1715,7 @@ static void arm_smmu_detach_dev(struct device *dev)
 static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 {
 	int ret = 0;
+	unsigned long flags;
 	struct arm_smmu_device *smmu;
 	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
 	struct arm_smmu_master_data *master;
@@ -1729,6 +1751,11 @@ static int arm_smmu_attach_dev(struct iommu_domain *domain, struct device *dev)
 	}
 
 	ste->assigned = true;
+	master->domain = smmu_domain;
+
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_add(&master->list, &smmu_domain->devices);
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_BYPASS) {
 		ste->s1_cfg = NULL;
@@ -1847,6 +1874,7 @@ static int arm_smmu_add_device(struct device *dev)
 			return -ENOMEM;
 
 		master->smmu = smmu;
+		master->dev = dev;
 		fwspec->iommu_priv = master;
 	}
 
-- 
2.5.5

[RFC 09/13] iommu/smmuv3: Get prepared for nested stage support

[Eric Auger]

To allow nested stage support, we need to store both
stage 1 and stage 2 configurations (and remove the former
union).

arm_smmu_write_strtab_ent() is modified to write both stage
fields in the STE.

We add a bypass field to the S1 configuration as the first
stage can be bypassed.

Only S2 stage is "finalized" as the host does not configure
S1 CD, guest does.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 19c085d..3a33979 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -496,6 +496,7 @@ struct arm_smmu_strtab_l1_desc {
 struct arm_smmu_s1_cfg {
 	__le64				*cdptr;
 	dma_addr_t			cdptr_dma;
+	bool bypass;
 
 	struct arm_smmu_ctx_desc {
 		u16	asid;
@@ -617,10 +618,8 @@ struct arm_smmu_domain {
 	struct io_pgtable_ops		*pgtbl_ops;
 
 	enum arm_smmu_domain_stage	stage;
-	union {
-		struct arm_smmu_s1_cfg	s1_cfg;
-		struct arm_smmu_s2_cfg	s2_cfg;
-	};
+	struct arm_smmu_s1_cfg	s1_cfg;
+	struct arm_smmu_s2_cfg	s2_cfg;
 
 	struct iommu_domain		domain;
 
@@ -1107,6 +1106,7 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 			break;
 		case STRTAB_STE_0_CFG_S1_TRANS:
 		case STRTAB_STE_0_CFG_S2_TRANS:
+		case STRTAB_STE_0_CFG_S1_TRANS | STRTAB_STE_0_CFG_S2_TRANS:
 			ste_live = true;
 			break;
 		case STRTAB_STE_0_CFG_ABORT:
@@ -1141,7 +1141,6 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 	}
 
 	if (ste->s1_cfg) {
-		BUG_ON(ste_live);
 		dst[1] = cpu_to_le64(
 			 FIELD_PREP(STRTAB_STE_1_S1CIR, STRTAB_STE_1_S1C_CACHE_WBRA) |
 			 FIELD_PREP(STRTAB_STE_1_S1COR, STRTAB_STE_1_S1C_CACHE_WBRA) |
@@ -1155,12 +1154,12 @@ static void arm_smmu_write_strtab_ent(struct arm_smmu_device *smmu, u32 sid,
 		   !(smmu->features & ARM_SMMU_FEAT_STALL_FORCE))
 			dst[1] |= cpu_to_le64(STRTAB_STE_1_S1STALLD);
 
-		val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
-			FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
+		if (!ste->s1_cfg->bypass)
+			val |= (ste->s1_cfg->cdptr_dma & STRTAB_STE_0_S1CTXPTR_MASK) |
+				FIELD_PREP(STRTAB_STE_0_CFG, STRTAB_STE_0_CFG_S1_TRANS);
 	}
 
 	if (ste->s2_cfg) {
-		BUG_ON(ste_live);
 		dst[2] = cpu_to_le64(
 			 FIELD_PREP(STRTAB_STE_2_S2VMID, ste->s2_cfg->vmid) |
 			 FIELD_PREP(STRTAB_STE_2_VTCR, ste->s2_cfg->vtcr) |
@@ -1399,6 +1398,10 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 		cmd.opcode	= CMDQ_OP_TLBI_NH_ASID;
 		cmd.tlbi.asid	= smmu_domain->s1_cfg.cd.asid;
 		cmd.tlbi.vmid	= 0;
+	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
+		cmd.opcode      = CMDQ_OP_TLBI_NH_ASID;
+		cmd.tlbi.asid   = smmu_domain->s1_cfg.cd.asid;
+		cmd.tlbi.vmid   = smmu_domain->s2_cfg.vmid;
 	} else {
 		cmd.opcode	= CMDQ_OP_TLBI_S12_VMALL;
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
@@ -1423,6 +1426,10 @@ static void arm_smmu_tlb_inv_range_nosync(unsigned long iova, size_t size,
 	if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1) {
 		cmd.opcode	= CMDQ_OP_TLBI_NH_VA;
 		cmd.tlbi.asid	= smmu_domain->s1_cfg.cd.asid;
+	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED) {
+		cmd.opcode      = CMDQ_OP_TLBI_NH_VA;
+		cmd.tlbi.asid   = smmu_domain->s1_cfg.cd.asid;
+		cmd.tlbi.vmid   = smmu_domain->s2_cfg.vmid;
 	} else {
 		cmd.opcode	= CMDQ_OP_TLBI_S2_IPA;
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
-- 
2.5.5

[RFC 10/13] iommu/smmuv3: Implement bind_guest_stage

[Eric Auger]

On bind_guest_stage() we set the minimal information in s1_cfg:
- the asid (allocated by the guest)
- the context descriptor GPA

On unbind, the STE stage 1 fields are reset.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 78 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 3a33979..26afafa 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2025,6 +2025,82 @@ static void arm_smmu_put_resv_regions(struct device *dev,
 		kfree(entry);
 }
 
+static int arm_smmu_bind_guest_stage(struct iommu_domain *domain,
+				     struct device *dev,
+				     struct iommu_guest_stage_config *cfg)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_master_data *entry;
+	unsigned long flags;
+
+	if (cfg->flags != SMMUV3_S1_CFG)
+		return -EINVAL;
+
+	if (!smmu)
+		return -EINVAL;
+
+	if (!((smmu->features & ARM_SMMU_FEAT_TRANS_S1) &&
+	      (smmu->features & ARM_SMMU_FEAT_TRANS_S2))) {
+		dev_info(smmu_domain->smmu->dev,
+			 "does not implement two stages\n");
+		return -EINVAL;
+	}
+
+	if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+		return -EINVAL;
+
+	if (cfg->smmu_s1.asid >= (1 << smmu->asid_bits))
+		return -EINVAL;
+
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_for_each_entry(entry, &smmu_domain->devices, list) {
+		struct arm_smmu_s1_cfg *s1_cfg;
+
+		if (dev != entry->dev)
+			continue;
+
+		s1_cfg = entry->ste.s1_cfg = &smmu_domain->s1_cfg;
+		s1_cfg->bypass = cfg->smmu_s1.disabled || cfg->smmu_s1.bypassed;
+		s1_cfg->cd.asid = cfg->smmu_s1.asid;
+
+		if (!s1_cfg->bypass)
+			s1_cfg->cdptr_dma = cfg->smmu_s1.cdptr_dma;
+
+		arm_smmu_install_ste_for_dev(dev->iommu_fwspec);
+		break;
+	}
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+
+	return 0;
+}
+
+static void arm_smmu_unbind_guest_stage(struct iommu_domain *domain,
+					struct device *dev)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	struct arm_smmu_master_data *entry;
+	unsigned long flags;
+
+	if (!smmu)
+		return;
+
+	if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+		return;
+
+	spin_lock_irqsave(&smmu_domain->devices_lock, flags);
+	list_for_each_entry(entry, &smmu_domain->devices, list) {
+		if (dev != entry->dev)
+			continue;
+
+		entry->ste.s1_cfg = NULL;
+		arm_smmu_install_ste_for_dev(dev->iommu_fwspec);
+		break;
+	}
+	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
+}
+
 static struct iommu_ops arm_smmu_ops = {
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
@@ -2044,6 +2120,8 @@ static struct iommu_ops arm_smmu_ops = {
 	.of_xlate		= arm_smmu_of_xlate,
 	.get_resv_regions	= arm_smmu_get_resv_regions,
 	.put_resv_regions	= arm_smmu_put_resv_regions,
+	.bind_guest_stage	= arm_smmu_bind_guest_stage,
+	.unbind_guest_stage	= arm_smmu_unbind_guest_stage,
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 };
 
-- 
2.5.5

[RFC 11/13] iommu/smmuv3: Implement tlb_invalidate

[Eric Auger]

Implement IOMMU_INV_TYPE_TLB invalidations. When
nr_pages is null we interpret this as a context
invalidation.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

---

The user API needs to be refined to discriminate context
invalidations from NH_VA invalidations. Also the leaf attribute
is not yet properly handled.
---
 drivers/iommu/arm-smmu-v3.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 26afafa..e4c7ae8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2101,6 +2101,39 @@ static void arm_smmu_unbind_guest_stage(struct iommu_domain *domain,
 	spin_unlock_irqrestore(&smmu_domain->devices_lock, flags);
 }
 
+static int arm_smmu_tlb_invalidate(struct iommu_domain *domain,
+				   struct device *dev,
+				   struct iommu_tlb_invalidate_info *inv_info)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+	size_t granule = 1 << (inv_info->size + 12);
+
+	if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+		return -EINVAL;
+
+	if (!smmu)
+		return -EINVAL;
+
+	if (inv_info->hdr.type != IOMMU_INV_TYPE_TLB)
+		return -EINVAL;
+
+	/*
+	 * TODO: On context invalidation, the userspace sets nr_pages to 0.
+	 * Refine the API to add a dedicated flags and also properly handle
+	 * the leaf parameter.
+	 */
+	if (!inv_info->nr_pages) {
+		arm_smmu_tlb_inv_context(smmu_domain);
+	} else {
+		arm_smmu_tlb_inv_range_nosync(inv_info->addr,
+					      inv_info->nr_pages * granule,
+					      granule, false, smmu_domain);
+		__arm_smmu_tlb_sync(smmu);
+	}
+	return 0;
+}
+
 static struct iommu_ops arm_smmu_ops = {
 	.capable		= arm_smmu_capable,
 	.domain_alloc		= arm_smmu_domain_alloc,
@@ -2122,6 +2155,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.put_resv_regions	= arm_smmu_put_resv_regions,
 	.bind_guest_stage	= arm_smmu_bind_guest_stage,
 	.unbind_guest_stage	= arm_smmu_unbind_guest_stage,
+	.tlb_invalidate		= arm_smmu_tlb_invalidate,
 	.pgsize_bitmap		= -1UL, /* Restricted during device attach */
 };
 
-- 
2.5.5

[RFC 12/13] dma-iommu: Implement NESTED_MSI cookie

[Eric Auger]

Up to now, when the type was UNMANAGED, we used to
allocate IOVA pages within a range provided by the user.
This does not work in nested mode.

If both the host and the guest are exposed with SMMUs, each
would allocate an IOVA. The guest allocates an IOVA (gIOVA)
to map onto the guest MSI doorbell (gDB). The Host allocates
another IOVA (hIOVA) to map onto the physical doorbell (hDB).

So we end up with 2 unrelated mappings, at S1 and S2:
         S1             S2
gIOVA    ->     gDB
               hIOVA    ->    hDB

The PCI device would be programmed with hIOVA.

iommu_dma_bind_doorbell allows to pass gIOVA/gDB to the host
so that gIOVA can be used by the host instead of re-allocating
a new IOVA. That way the host can create the following nested
mapping:

         S1           S2
gIOVA    ->    gDB    ->    hDB

this time, the PCI device will be programmed with the gIOVA MSI
doorbell which is correctly map through the 2 stages.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/dma-iommu.c | 88 +++++++++++++++++++++++++++++++++++++++++++++--
 include/linux/dma-iommu.h | 11 ++++++
 2 files changed, 96 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 511ff9a..ffce2b2 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -37,12 +37,14 @@
 struct iommu_dma_msi_page {
 	struct list_head	list;
 	dma_addr_t		iova;
+	dma_addr_t		ipa;
 	phys_addr_t		phys;
 };
 
 enum iommu_dma_cookie_type {
 	IOMMU_DMA_IOVA_COOKIE,
 	IOMMU_DMA_MSI_COOKIE,
+	IOMMU_DMA_NESTED_MSI_COOKIE,
 };
 
 struct iommu_dma_cookie {
@@ -109,14 +111,17 @@ EXPORT_SYMBOL(iommu_get_dma_cookie);
  *
  * Users who manage their own IOVA allocation and do not want DMA API support,
  * but would still like to take advantage of automatic MSI remapping, can use
- * this to initialise their own domain appropriately. Users should reserve a
+ * this to initialise their own domain appropriately. Users may reserve a
  * contiguous IOVA region, starting at @base, large enough to accommodate the
  * number of PAGE_SIZE mappings necessary to cover every MSI doorbell address
- * used by the devices attached to @domain.
+ * used by the devices attached to @domain. The other way round is to provide
+ * usable iova pages through the iommu_dma_bind_doorbell API (nested stages
+ * use case)
  */
 int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 {
 	struct iommu_dma_cookie *cookie;
+	int nesting, ret;
 
 	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
 		return -EINVAL;
@@ -124,7 +129,12 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 	if (domain->iova_cookie)
 		return -EEXIST;
 
-	cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+	ret =  iommu_domain_get_attr(domain, DOMAIN_ATTR_NESTING, &nesting);
+	if (!ret && nesting)
+		cookie = cookie_alloc(IOMMU_DMA_NESTED_MSI_COOKIE);
+	else
+		cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
+
 	if (!cookie)
 		return -ENOMEM;
 
@@ -162,6 +172,50 @@ void iommu_put_dma_cookie(struct iommu_domain *domain)
 EXPORT_SYMBOL(iommu_put_dma_cookie);
 
 /**
+ * iommu_dma_bind_doorbell - Allows to provide a usable IOVA page
+ * @domain: domain handle
+ * @binding: IOVA/IPA binding
+ *
+ * In nested stage use case, the user can provide IOVA/IPA bindings
+ * corresponding to a guest MSI stage 1 mapping. When the host needs
+ * to map its own MSI doorbells, it can use the IPA as stage 2 input
+ * and map it onto the physical MSI doorbell.
+ */
+int iommu_dma_bind_doorbell(struct iommu_domain *domain,
+			    struct iommu_guest_msi_binding *binding)
+{
+	struct iommu_dma_cookie *cookie = domain->iova_cookie;
+	struct iommu_dma_msi_page *msi;
+	dma_addr_t ipa, iova;
+	size_t size;
+
+	if (!cookie)
+		return -EINVAL;
+
+	if (cookie->type != IOMMU_DMA_NESTED_MSI_COOKIE)
+		return -EINVAL;
+
+	size = 1 << binding->granule;
+	iova = binding->iova & ~(phys_addr_t)(size - 1);
+	ipa = binding->gpa & ~(phys_addr_t)(size - 1);
+
+	list_for_each_entry(msi, &cookie->msi_page_list, list) {
+		if (msi->iova == iova)
+			return 0; /* this page is already registered */
+	}
+
+	msi = kzalloc(sizeof(*msi), GFP_KERNEL);
+	if (!msi)
+		return -ENOMEM;
+
+	msi->iova = iova;
+	msi->ipa = ipa;
+	list_add(&msi->list, &cookie->msi_page_list);
+	return 0;
+}
+EXPORT_SYMBOL(iommu_dma_bind_doorbell);
+
+/**
  * iommu_dma_get_resv_regions - Reserved region driver helper
  * @dev: Device from iommu_get_resv_regions()
  * @list: Reserved region list from iommu_get_resv_regions()
@@ -846,6 +900,34 @@ static struct iommu_dma_msi_page *iommu_dma_get_msi_page(struct device *dev,
 		if (msi_page->phys == msi_addr)
 			return msi_page;
 
+	/*
+	 * In nested stage mode, we do not allocate an MSI page in
+	 * a range provided by the user. Instead, IOVA/IPA bindings are
+	 * individually provided. We reuse thise IOVAs to build the
+	 * IOVA -> IPA -> MSI PA nested stage mapping.
+	 */
+	if (cookie->type == IOMMU_DMA_NESTED_MSI_COOKIE) {
+		list_for_each_entry(msi_page, &cookie->msi_page_list, list)
+			if (!msi_page->phys) { /* this binding is free to use */
+				dma_addr_t ipa = msi_page->ipa;
+				int ret;
+
+				msi_page->phys = msi_addr;
+
+				/* do the stage 2 mapping */
+				ret = iommu_map(domain, ipa, msi_addr, size,
+						IOMMU_MMIO | IOMMU_WRITE);
+				if (ret) {
+					pr_warn("MSI S2 mapping failed (%d)\n",
+						ret);
+					return NULL;
+				}
+				return msi_page;
+			}
+		pr_warn("%s no MSI binding found\n", __func__);
+		return NULL;
+	}
+
 	msi_page = kzalloc(sizeof(*msi_page), GFP_ATOMIC);
 	if (!msi_page)
 		return NULL;
diff --git a/include/linux/dma-iommu.h b/include/linux/dma-iommu.h
index e8ca5e6..324745e 100644
--- a/include/linux/dma-iommu.h
+++ b/include/linux/dma-iommu.h
@@ -24,6 +24,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/iommu.h>
 #include <linux/msi.h>
+#include <uapi/linux/iommu.h>
 
 int iommu_dma_init(void);
 
@@ -74,12 +75,15 @@ int iommu_dma_mapping_error(struct device *dev, dma_addr_t dma_addr);
 /* The DMA API isn't _quite_ the whole story, though... */
 void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg);
 void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list);
+int iommu_dma_bind_doorbell(struct iommu_domain *domain,
+			    struct iommu_guest_msi_binding *binding);
 
 #else
 
 struct iommu_domain;
 struct msi_msg;
 struct device;
+struct iommu_guest_msi_binding;
 
 static inline int iommu_dma_init(void)
 {
@@ -104,6 +108,13 @@ static inline void iommu_dma_map_msi_msg(int irq, struct msi_msg *msg)
 {
 }
 
+static inline int
+iommu_dma_bind_doorbell(struct iommu_domain *domain,
+			struct iommu_guest_msi_binding *binding)
+{
+	return -ENODEV;
+}
+
 static inline void iommu_dma_get_resv_regions(struct device *dev, struct list_head *list)
 {
 }
-- 
2.5.5

[RFC 13/13] iommu/smmuv3: Implement bind_guest_msi

[Eric Auger]

The bind_guest_msi() callback checks the domain
is NESTED and redirect to the dma-iommu implementation.

Signed-off-by: Eric Auger <eric.auger-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/arm-smmu-v3.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e4c7ae8..ccd55d3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2025,6 +2025,21 @@ static void arm_smmu_put_resv_regions(struct device *dev,
 		kfree(entry);
 }
 
+static int arm_smmu_bind_guest_msi(struct iommu_domain *domain,
+				   struct iommu_guest_msi_binding *binding)
+{
+	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
+
+	if (!smmu)
+		return -EINVAL;
+
+	if (smmu_domain->stage != ARM_SMMU_DOMAIN_NESTED)
+		return -EINVAL;
+
+	return iommu_dma_bind_doorbell(domain, binding);
+}
+
 static int arm_smmu_bind_guest_stage(struct iommu_domain *domain,
 				     struct device *dev,
 				     struct iommu_guest_stage_config *cfg)
@@ -2153,6 +2168,7 @@ static struct iommu_ops arm_smmu_ops = {
 	.of_xlate		= arm_smmu_of_xlate,
 	.get_resv_regions	= arm_smmu_get_resv_regions,
 	.put_resv_regions	= arm_smmu_put_resv_regions,
+	.bind_guest_msi		= arm_smmu_bind_guest_msi,
 	.bind_guest_stage	= arm_smmu_bind_guest_stage,
 	.unbind_guest_stage	= arm_smmu_unbind_guest_stage,
 	.tlb_invalidate		= arm_smmu_tlb_invalidate,
-- 
2.5.5

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Auger Eric]

Hi,

On 08/23/2018 02:17 PM, Eric Auger wrote:
> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
> 
> In virtualization use case, when a guest is assigned
> a PCI host device, protected by a virtual IOMMU on a guest,
> the physical IOMMU must be programmed to be consistent with
> the guest mappings. If the physical IOMMU supports two
> translation stages it makes sense to program guest mappings
> onto the first stage while to host owns the stage 2.
> 
> In that case, it is mandated to trap on guest configuration
> settings and pass those to the physical iommu driver.
> 
> This patch adds a new API to the iommu subsystem that allows
> to bind and unbind the guest configuration data to the host.
> 
> A generic iommu_guest_stage_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 2 specialization of this struct,
> for PASID table passing and ARM SMMUv3.
> 
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> This patch generalizes the API introduced by Jacob & co-authors in
> https://lwn.net/Articles/754331/
> ---
>  drivers/iommu/iommu.c      | 19 +++++++++++++++
>  include/linux/iommu.h      | 23 ++++++++++++++++++
>  include/uapi/linux/iommu.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+)
>  create mode 100644 include/uapi/linux/iommu.h
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 63b3756..5156172 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1326,6 +1326,25 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
>  }
>  EXPORT_SYMBOL_GPL(iommu_attach_device);
>  
> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
> +			   struct iommu_guest_stage_config *cfg)
> +{
> +	if (unlikely(!domain->ops->bind_guest_stage))
> +		return -ENODEV;
> +
> +	return domain->ops->bind_guest_stage(domain, dev, cfg);
> +}
> +EXPORT_SYMBOL_GPL(iommu_bind_guest_stage);
> +
> +void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
> +{
> +	if (unlikely(!domain->ops->unbind_guest_stage))
> +		return;
> +
> +	domain->ops->unbind_guest_stage(domain, dev);
> +}
> +EXPORT_SYMBOL_GPL(iommu_unbind_guest_stage);
> +
>  static void __iommu_detach_device(struct iommu_domain *domain,
>  				  struct device *dev)
>  {
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 19938ee..aec853d 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -25,6 +25,7 @@
>  #include <linux/errno.h>
>  #include <linux/err.h>
>  #include <linux/of.h>
> +#include <uapi/linux/iommu.h>
>  
>  #define IOMMU_READ	(1 << 0)
>  #define IOMMU_WRITE	(1 << 1)
> @@ -187,6 +188,8 @@ struct iommu_resv_region {
>   * @domain_get_windows: Return the number of windows for a domain
>   * @of_xlate: add OF master IDs to iommu grouping
>   * @pgsize_bitmap: bitmap of all possible supported page sizes
> + * @bind_guest_stage: program the guest stage configuration
> + * @unbind_guest_stage: unbind the guest stage and restore defaults
>   */
>  struct iommu_ops {
>  	bool (*capable)(enum iommu_cap);
> @@ -235,6 +238,11 @@ struct iommu_ops {
>  	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>  	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device *dev);
>  
> +	int (*bind_guest_stage)(struct iommu_domain *domain, struct device *dev,
> +				struct iommu_guest_stage_config *cfg);
> +	void (*unbind_guest_stage)(struct iommu_domain *domain,
> +				   struct device *dev);
> +
>  	unsigned long pgsize_bitmap;
>  };
>  
> @@ -296,6 +304,10 @@ extern int iommu_attach_device(struct iommu_domain *domain,
>  			       struct device *dev);
>  extern void iommu_detach_device(struct iommu_domain *domain,
>  				struct device *dev);
> +extern int iommu_bind_guest_stage(struct iommu_domain *domain,
> +		struct device *dev, struct iommu_guest_stage_config *cfg);
> +extern void iommu_unbind_guest_stage(struct iommu_domain *domain,
> +				     struct device *dev);
>  extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
>  extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>  		     phys_addr_t paddr, size_t size, int prot);
> @@ -696,6 +708,17 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct fwnode_handle *fwnode)
>  	return NULL;
>  }
>  
> +static inline
> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
> +			   struct iommu_guest_stage_config *cfg)
> +{
> +	return -ENODEV;
> +}
> +static inline
> +void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device *dev)
> +{
> +}
> +
>  #endif /* CONFIG_IOMMU_API */
>  
>  #endif /* __LINUX_IOMMU_H */
> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
> new file mode 100644
> index 0000000..2d1593c
> --- /dev/null
> +++ b/include/uapi/linux/iommu.h
> @@ -0,0 +1,59 @@
> +/* 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>
> +
> +/**
> + * PASID table data used to bind guest PASID table to the host IOMMU. This will
> + * enable guest managed first level page tables.
> + * @version: for future extensions and identification of the data format
> + * @bytes: size of this structure
> + * @base_ptr:	PASID table pointer
> + * @pasid_bits:	number of bits supported in the guest PASID table, must be less
> + *		or equal than the host supported PASID size.
> + */
> +struct iommu_pasid_table_config {
> +	__u32 version;
> +#define PASID_TABLE_CFG_VERSION_1 1
> +	__u32 bytes;
> +	__u64 base_ptr;
> +	__u8 pasid_bits;
> +};
> +
> +/**
> + * Stream Table Entry S1 info
> + * @disabled: the smmu is disabled
> + * @bypassed: stage 1 is bypassed
> + * @aborted: aborted
> + * @cdptr_dma: GPA of the Context Descriptor
> + * @asid: address space identified associated to the CD
> + */
> +struct iommu_smmu_s1_config {
> +	__u8 disabled;
> +	__u8 bypassed;
> +	__u8 aborted;
> +	__u64 cdptr_dma;
> +	__u16 asid;
This asid field is a leftover. asid is part of the CD (owned by the
guest) and cannot be conveyed through this API. What is relevant is to
pass is the guest asid_bits (vSMMU IDR1 info), to be compared with the
host one. So we end up having something similar to the base_ptr and
pasid_bits of iommu_pasid_table_config.

Otherwise, the other fields (disable, bypassed, aborted) can be packed
as masks in a _u8 flag.

Anyway this API still is at the stage of proof of concept. At least it
shows the similarities between that use case and the PASID one and
should encourage to discuss about the relevance to have a unified API to
pass the guest stage info.

Thanks

Eric
> +};
> +
> +struct iommu_guest_stage_config {
> +#define PASID_TABLE	(1 << 0)
> +#define SMMUV3_S1_CFG	(1 << 1)
> +	__u32 flags;
> +	union {
> +		struct iommu_pasid_table_config pasidt;
> +		struct iommu_smmu_s1_config smmu_s1;
> +	};
> +};
> +
> +#endif /* _UAPI_IOMMU_H */
> 

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Auger Eric]

Hi Yi Liu,

On 08/24/2018 02:53 PM, Liu, Yi L wrote:
> Hi Eric,
> 
>> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
>> bounces@lists.linux-foundation.org] On Behalf Of Eric Auger
>> Sent: Thursday, August 23, 2018 8:17 PM
>> Subject: [RFC 01/13] iommu: Introduce bind_guest_stage API
>>
>> From: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>
>> In virtualization use case, when a guest is assigned
>> a PCI host device, protected by a virtual IOMMU on a guest,
>> the physical IOMMU must be programmed to be consistent with
>> the guest mappings. If the physical IOMMU supports two
>> translation stages it makes sense to program guest mappings
>> onto the first stage while to host owns the stage 2.
>>
>> In that case, it is mandated to trap on guest configuration
>> settings and pass those to the physical iommu driver.
>>
>> This patch adds a new API to the iommu subsystem that allows
>> to bind and unbind the guest configuration data to the host.
>>
>> A generic iommu_guest_stage_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 2 specialization of this struct,
>> for PASID table passing and ARM SMMUv3.
>>
>> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
>> Signed-off-by: Liu, Yi L <yi.l.liu@linux.intel.com>
>> Signed-off-by: Ashok Raj <ashok.raj@intel.com>
>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> This patch generalizes the API introduced by Jacob & co-authors in
>> https://lwn.net/Articles/754331/
>> ---
>>  drivers/iommu/iommu.c      | 19 +++++++++++++++
>>  include/linux/iommu.h      | 23 ++++++++++++++++++
>>  include/uapi/linux/iommu.h | 59
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 101 insertions(+)
>>  create mode 100644 include/uapi/linux/iommu.h
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 63b3756..5156172 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1326,6 +1326,25 @@ int iommu_attach_device(struct iommu_domain
>> *domain, struct device *dev)
>>  }
>>  EXPORT_SYMBOL_GPL(iommu_attach_device);
>>
>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
>> +			   struct iommu_guest_stage_config *cfg)
>> +{
>> +	if (unlikely(!domain->ops->bind_guest_stage))
>> +		return -ENODEV;
>> +
>> +	return domain->ops->bind_guest_stage(domain, dev, cfg);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_bind_guest_stage);
>> +
>> +void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device
>> *dev)
>> +{
>> +	if (unlikely(!domain->ops->unbind_guest_stage))
>> +		return;
>> +
>> +	domain->ops->unbind_guest_stage(domain, dev);
>> +}
>> +EXPORT_SYMBOL_GPL(iommu_unbind_guest_stage);
>> +
>>  static void __iommu_detach_device(struct iommu_domain *domain,
>>  				  struct device *dev)
>>  {
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 19938ee..aec853d 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -25,6 +25,7 @@
>>  #include <linux/errno.h>
>>  #include <linux/err.h>
>>  #include <linux/of.h>
>> +#include <uapi/linux/iommu.h>
>>
>>  #define IOMMU_READ	(1 << 0)
>>  #define IOMMU_WRITE	(1 << 1)
>> @@ -187,6 +188,8 @@ struct iommu_resv_region {
>>   * @domain_get_windows: Return the number of windows for a domain
>>   * @of_xlate: add OF master IDs to iommu grouping
>>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>> + * @bind_guest_stage: program the guest stage configuration
>> + * @unbind_guest_stage: unbind the guest stage and restore defaults
>>   */
>>  struct iommu_ops {
>>  	bool (*capable)(enum iommu_cap);
>> @@ -235,6 +238,11 @@ struct iommu_ops {
>>  	int (*of_xlate)(struct device *dev, struct of_phandle_args *args);
>>  	bool (*is_attach_deferred)(struct iommu_domain *domain, struct device
>> *dev);
>>
>> +	int (*bind_guest_stage)(struct iommu_domain *domain, struct device *dev,
>> +				struct iommu_guest_stage_config *cfg);
>> +	void (*unbind_guest_stage)(struct iommu_domain *domain,
>> +				   struct device *dev);
>> +
>>  	unsigned long pgsize_bitmap;
>>  };
>>
>> @@ -296,6 +304,10 @@ extern int iommu_attach_device(struct iommu_domain
>> *domain,
>>  			       struct device *dev);
>>  extern void iommu_detach_device(struct iommu_domain *domain,
>>  				struct device *dev);
>> +extern int iommu_bind_guest_stage(struct iommu_domain *domain,
>> +		struct device *dev, struct iommu_guest_stage_config *cfg);
>> +extern void iommu_unbind_guest_stage(struct iommu_domain *domain,
>> +				     struct device *dev);
>>  extern struct iommu_domain *iommu_get_domain_for_dev(struct device *dev);
>>  extern int iommu_map(struct iommu_domain *domain, unsigned long iova,
>>  		     phys_addr_t paddr, size_t size, int prot);
>> @@ -696,6 +708,17 @@ const struct iommu_ops *iommu_ops_from_fwnode(struct
>> fwnode_handle *fwnode)
>>  	return NULL;
>>  }
>>
>> +static inline
>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
>> +			   struct iommu_guest_stage_config *cfg)
>> +{
>> +	return -ENODEV;
>> +}
>> +static inline
>> +void iommu_unbind_guest_stage(struct iommu_domain *domain, struct device
>> *dev)
>> +{
>> +}
>> +
>>  #endif /* CONFIG_IOMMU_API */
>>
>>  #endif /* __LINUX_IOMMU_H */
>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>> new file mode 100644
>> index 0000000..2d1593c
>> --- /dev/null
>> +++ b/include/uapi/linux/iommu.h
>> @@ -0,0 +1,59 @@
>> +/* 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>
>> +
>> +/**
>> + * PASID table data used to bind guest PASID table to the host IOMMU. This will
>> + * enable guest managed first level page tables.
>> + * @version: for future extensions and identification of the data format
>> + * @bytes: size of this structure
>> + * @base_ptr:	PASID table pointer
>> + * @pasid_bits:	number of bits supported in the guest PASID table, must be
>> less
>> + *		or equal than the host supported PASID size.
>> + */
>> +struct iommu_pasid_table_config {
>> +	__u32 version;
>> +#define PASID_TABLE_CFG_VERSION_1 1
>> +	__u32 bytes;
>> +	__u64 base_ptr;
>> +	__u8 pasid_bits;
>> +};
>> +
>> +/**
>> + * Stream Table Entry S1 info
>> + * @disabled: the smmu is disabled
>> + * @bypassed: stage 1 is bypassed
>> + * @aborted: aborted
>> + * @cdptr_dma: GPA of the Context Descriptor
>> + * @asid: address space identified associated to the CD
>> + */
>> +struct iommu_smmu_s1_config {
>> +	__u8 disabled;
>> +	__u8 bypassed;
>> +	__u8 aborted;
>> +	__u64 cdptr_dma;
> 
> If I'm getting it correctly, so with cdptr_dma configed on
> host, hardware can access guest's CD table when nested
> stage is setup on host. Is it? If yes, I think it is similar with
> the base_ptr field in struct iommu_pasid_table_config.

Yes you're right, this is similar to base_ptr.

Thanks

Eric
> 
>> +	__u16 asid;
>> +};
>> +
>> +struct iommu_guest_stage_config {
>> +#define PASID_TABLE	(1 << 0)
>> +#define SMMUV3_S1_CFG	(1 << 1)
>> +	__u32 flags;
>> +	union {
>> +		struct iommu_pasid_table_config pasidt;
>> +		struct iommu_smmu_s1_config smmu_s1;
>> +	};
>> +};
>> +
>> +#endif /* _UAPI_IOMMU_H */
> 
> Thanks,
> Yi Liu
> 

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Jean-Philippe Brucker]

Hi Eric,

On 23/08/18 16:25, Auger Eric wrote:
>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
>> +			   struct iommu_guest_stage_config *cfg)

About the name change from iommu_bind_pasid_table: is the intent to
reuse this API for SMMUv2, which supports nested but not PASID? Seems
like a good idea but "iommu_bind_table" may be better since "stage" is
only used by Arm.

>> +/**
>> + * PASID table data used to bind guest PASID table to the host IOMMU. This will
>> + * enable guest managed first level page tables.
>> + * @version: for future extensions and identification of the data format
>> + * @bytes: size of this structure
>> + * @base_ptr:	PASID table pointer
>> + * @pasid_bits:	number of bits supported in the guest PASID table, must be less
>> + *		or equal than the host supported PASID size.
>> + */
>> +struct iommu_pasid_table_config {
>> +	__u32 version;
>> +#define PASID_TABLE_CFG_VERSION_1 1
>> +	__u32 bytes;
>> +	__u64 base_ptr;
>> +	__u8 pasid_bits;
>> +};
>> +
>> +/**
>> + * Stream Table Entry S1 info
>> + * @disabled: the smmu is disabled
>> + * @bypassed: stage 1 is bypassed
>> + * @aborted: aborted
>> + * @cdptr_dma: GPA of the Context Descriptor
>> + * @asid: address space identified associated to the CD
>> + */
>> +struct iommu_smmu_s1_config {
>> +	__u8 disabled;
>> +	__u8 bypassed;
>> +	__u8 aborted;
>> +	__u64 cdptr_dma;
>> +	__u16 asid;
> This asid field is a leftover. asid is part of the CD (owned by the
> guest) and cannot be conveyed through this API. What is relevant is to
> pass is the guest asid_bits (vSMMU IDR1 info), to be compared with the
> host one. So we end up having something similar to the base_ptr and
> pasid_bits of iommu_pasid_table_config.

That part seems strange. Userspace wouldn't try arbitrary ASID sizes
until one fits, especially since ASID size isn't the only parameter:
there will be SSID, IOVA, IPA sizes, HTTU features and I guess any other
SMMU_IDRx bit that corresponds to a field in the CD or STE. Doesn't
vSMMU need to tailor its IDR registers to whatever is supported by the
host SMMU, in order to use nested translation?

Before creating the virtual ID registers, userspace will likely want to
know more about the physical IOMMU, by querying the kernel. I wrote
something about that interface recently:
https://www.spinics.net/lists/iommu/msg28039.html

And if you provide this interface, checking the parameters again in
BIND_GUEST_STAGE isn't very useful, it only tells you that userspace
read your values. Once the guest writes the CD, it could still use the
wrong ASID size or some other invalid config. That would be caught by
the SMMU walker and cause a C_BAD_CD error report.

> Otherwise, the other fields (disable, bypassed, aborted) can be packed
> as masks in a _u8 flag.

What's the difference between aborted and disabled? I'm also not sure we
should give such fine tweaks to userspace, since the STE code is already
pretty complicated and has to deal with several state transitions.
Existing behavior (1) (2) (5) probably needs to be preserved:

(1) Initially no container is set, s1-translate s2-bypass with the
default domain (non-VFIO)
(2) A VFIO_TYPE1_NESTING_IOMMU container is set, s1-bypass s2-translate
(3) BIND_GUEST_STAGE, s1-translate s2-translate
(4) UNBIND_GUEST_STAGE, s1-bypass s2-translate
(5) Remove container, s1-translate s2-bypass with the default domain

That said, when instantiating a vSMMU, QEMU may want to switch from (2)
to an intermediate "s1-abort s2-translate" state. At the moment with
virtio-iommu I only create the stage-2 mappings at (3). This ensures
that transactions abort until the guest configures the vIOMMU and it
keeps the host driver simple, but it has the downside of pinning guest
memory lazily (necessary with virtio-iommu since the guest falls back to
the map/unmap interface, which doesn't pin all memory upfront, if it
can't support nested).

> Anyway this API still is at the stage of proof of concept. At least it
> shows the similarities between that use case and the PASID one and
> should encourage to discuss about the relevance to have a unified API to
> pass the guest stage info.
Other required fields in the BIND_GUEST_STAGE ioctl are at least s1dss,
s1cdmax and s1fmt. It's not for sanity-check, they describe the guest
configuration. The guest selects a PASID table format (1-level, 2-level
4k/64k) which is written into STE.s1fmt. It also selects the behavior of
requests-without-pasid, which is written into STE.s1dss. s1cdmax
corresponds to pasid_bits.

Thanks,
Jean

Re: [RFC 02/13] iommu: Introduce tlb_invalidate API

[Jean-Philippe Brucker]

On 23/08/18 13:17, Eric Auger wrote:
> +/**
> + * Translation cache invalidation information, contains generic IOMMU
> + * data which can be parsed based on model ID by model specific drivers.
> + * Since the invalidation of second level page tables are included in the
> + * unmap operation, this info is only applicable to the first level
> + * translation caches, i.e. DMA request with PASID.
> + *
> + * @granularity:	requested invalidation granularity, type dependent
> + * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.
> + * @nr_pages:		number of pages to invalidate
> + * @pasid:		processor address space ID value per PCI spec.
> + * @addr:		page address to be invalidated
> + * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
> + *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages
> + *
> + */
> +struct iommu_tlb_invalidate_info {
> +	struct iommu_tlb_invalidate_hdr	hdr;
> +	enum iommu_inv_granularity	granularity;
> +	__u32		flags;
> +#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
> +#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
> +	__u8		size;
> +	__u64		nr_pages;
> +	__u32		pasid;
> +	__u64		addr;
> +};
>  #endif /* _UAPI_IOMMU_H */

Since the ioctl will be used to combine invalidations (invalidate both
ATC and TLB with a single call), we need an additional ASID field for
the SMMU - ATC is invalidated by PASID, TLB by ASID. I used to call it
"tag", but I'm leaning towards "arch_id" now
(http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=40fdef74816dd8d8d113100b9e0162fab4cec28d)

Thanks,
Jean

Re: [RFC 09/13] iommu/smmuv3: Get prepared for nested stage support

[Jean-Philippe Brucker]

On 23/08/18 13:17, Eric Auger wrote:
>  	if (ste->s1_cfg) {
> -		BUG_ON(ste_live);

Scary! :) The current code assumes that it can make modifications to the
STE in any order and enable translation after a sync. So far I haven't
been able to find anything that violates this rule in the spec: "If
software modifies the structure while it is valid, it must not allow the
structure to enter an invalid intermediate state." So maybe it's fine,
though to be safe I would have started with disabling the STE
(http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=936e49f923e101c061269eadd5fa43fef819d2e9)

Thanks,
Jean

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Auger Eric]

Hi Jean-Philippe,

On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
> Hi Eric,
> 
> On 23/08/18 16:25, Auger Eric wrote:
>>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct device *dev,
>>> +			   struct iommu_guest_stage_config *cfg)
> 
> About the name change from iommu_bind_pasid_table: is the intent to
> reuse this API for SMMUv2, which supports nested but not PASID? Seems
> like a good idea but "iommu_bind_table" may be better since "stage" is
> only used by Arm.

At the moment I don't target SMUv2 but just SMMUv3. My focus was on
nested stage enablement without enabling the multi-CD feature (PASID),
whish is not supported by the QEMU vSMMUv3. Afterwards I realized that
basically we are pointing to a CD or PASID table and that's about the
same. I don't have a strong opinion on the name, iommu_bind_guest_table
or iommu_bind_pasid_table would be fine with me. Indeed "stage" is ARM
vocable (level for Intel?)
> 
>>> +/**
>>> + * PASID table data used to bind guest PASID table to the host IOMMU. This will
>>> + * enable guest managed first level page tables.
>>> + * @version: for future extensions and identification of the data format
>>> + * @bytes: size of this structure
>>> + * @base_ptr:	PASID table pointer
>>> + * @pasid_bits:	number of bits supported in the guest PASID table, must be less
>>> + *		or equal than the host supported PASID size.
>>> + */
>>> +struct iommu_pasid_table_config {
>>> +	__u32 version;
>>> +#define PASID_TABLE_CFG_VERSION_1 1
>>> +	__u32 bytes;
>>> +	__u64 base_ptr;
>>> +	__u8 pasid_bits;
>>> +};
>>> +
>>> +/**
>>> + * Stream Table Entry S1 info
>>> + * @disabled: the smmu is disabled
>>> + * @bypassed: stage 1 is bypassed
>>> + * @aborted: aborted
>>> + * @cdptr_dma: GPA of the Context Descriptor
>>> + * @asid: address space identified associated to the CD
>>> + */
>>> +struct iommu_smmu_s1_config {
>>> +	__u8 disabled;
>>> +	__u8 bypassed;
>>> +	__u8 aborted;
>>> +	__u64 cdptr_dma;
>>> +	__u16 asid;
>> This asid field is a leftover. asid is part of the CD (owned by the
>> guest) and cannot be conveyed through this API. What is relevant is to
>> pass is the guest asid_bits (vSMMU IDR1 info), to be compared with the
>> host one. So we end up having something similar to the base_ptr and
>> pasid_bits of iommu_pasid_table_config.
> 
> That part seems strange. Userspace wouldn't try arbitrary ASID sizes
> until one fits, especially since ASID size isn't the only parameter:
> there will be SSID, IOVA, IPA sizes, HTTU features and I guess any other
> SMMU_IDRx bit that corresponds to a field in the CD or STE. Doesn't
> vSMMU need to tailor its IDR registers to whatever is supported by the
> host SMMU, in order to use nested translation?
Yes I agree this is needed anyway.
> 
> Before creating the virtual ID registers, userspace will likely want to
> know more about the physical IOMMU, by querying the kernel. I wrote
> something about that interface recently:
> https://www.spinics.net/lists/iommu/msg28039.html

Ah OK I missed that part of the discussion. About the sysfs API, we
devised one in the past for reserved_regions. I thought it would be
useful for QEMU to identify them but eventually Alex pushed to create a
VFIO API instead to make things more self-contained. We would need to
double check with him.
> 
> And if you provide this interface, checking the parameters again in
> BIND_GUEST_STAGE isn't very useful, it only tells you that userspace
> read your values. Once the guest writes the CD, it could still use the
> wrong ASID size or some other invalid config. That would be caught by
> the SMMU walker and cause a C_BAD_CD error report.

Yes actually if there is some mismatch we might get BAD_STE/BAD_CD
events. This might be another way to handle things. I did not integrate
the fault API yet. This exercise is need to understand how we can catch
things at QEMU level.
> 
>> Otherwise, the other fields (disable, bypassed, aborted) can be packed
>> as masks in a _u8 flag.
> 
> What's the difference between aborted and disabled? I'm also not sure we
> should give such fine tweaks to userspace, since the STE code is already
> pretty complicated and has to deal with several state transitions.
> Existing behavior (1) (2) (5) probably needs to be preserved:
> 
> (1) Initially no container is set, s1-translate s2-bypass with the
> default domain (non-VFIO)
> (2) A VFIO_TYPE1_NESTING_IOMMU container is set, s1-bypass s2-translate
> (3) BIND_GUEST_STAGE, s1-translate s2-translate
> (4) UNBIND_GUEST_STAGE, s1-bypass s2-translate
> (5) Remove container, s1-translate s2-bypass with the default domain
> 
> That said, when instantiating a vSMMU, QEMU may want to switch from (2)
> to an intermediate "s1-abort s2-translate" state. At the moment with
> virtio-iommu I only create the stage-2 mappings at (3). This ensures
> that transactions abort until the guest configures the vIOMMU and it
> keeps the host driver simple, but it has the downside of pinning guest
> memory lazily (necessary with virtio-iommu since the guest falls back to
> the map/unmap interface, which doesn't pin all memory upfront, if it
> can't support nested).

For now, I have not made any assumptions here and just made sure I was
passing the S1 Config part. I would be tempted to think that only bypass
is requested and abort/disabled could be implemented by the user by
tearing the binding down, as suggested by Jacob.
> 
>> Anyway this API still is at the stage of proof of concept. At least it
>> shows the similarities between that use case and the PASID one and
>> should encourage to discuss about the relevance to have a unified API to
>> pass the guest stage info.
> Other required fields in the BIND_GUEST_STAGE ioctl are at least s1dss,
> s1cdmax and s1fmt. It's not for sanity-check, they describe the guest
> configuration. The guest selects a PASID table format (1-level, 2-level
> 4k/64k) which is written into STE.s1fmt. It also selects the behavior of
> requests-without-pasid, which is written into STE.s1dss. s1cdmax
> corresponds to pasid_bits.
Agreed. in vSMMU I only support S1CDMax =0 so I did not get into those
troubles.

Do we agree here we can get rid of the struct device * parameter?

Thanks

Eric

> 
> Thanks,
> Jean
> 

Re: [RFC 02/13] iommu: Introduce tlb_invalidate API

[Auger Eric]

Hi Jean-Philippe,

On 08/31/2018 03:17 PM, Jean-Philippe Brucker wrote:
> On 23/08/18 13:17, Eric Auger wrote:
>> +/**
>> + * Translation cache invalidation information, contains generic IOMMU
>> + * data which can be parsed based on model ID by model specific drivers.
>> + * Since the invalidation of second level page tables are included in the
>> + * unmap operation, this info is only applicable to the first level
>> + * translation caches, i.e. DMA request with PASID.
>> + *
>> + * @granularity:	requested invalidation granularity, type dependent
>> + * @size:		2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.
>> + * @nr_pages:		number of pages to invalidate
>> + * @pasid:		processor address space ID value per PCI spec.
>> + * @addr:		page address to be invalidated
>> + * @flags		IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
>> + *			IOMMU_INVALIDATE_GLOBAL_PAGE: global pages
>> + *
>> + */
>> +struct iommu_tlb_invalidate_info {
>> +	struct iommu_tlb_invalidate_hdr	hdr;
>> +	enum iommu_inv_granularity	granularity;
>> +	__u32		flags;
>> +#define IOMMU_INVALIDATE_ADDR_LEAF	(1 << 0)
>> +#define IOMMU_INVALIDATE_GLOBAL_PAGE	(1 << 1)
>> +	__u8		size;
>> +	__u64		nr_pages;
>> +	__u32		pasid;
>> +	__u64		addr;
>> +};
>>  #endif /* _UAPI_IOMMU_H */
> 
> Since the ioctl will be used to combine invalidations (invalidate both
> ATC and TLB with a single call), we need an additional ASID field for
> the SMMU - ATC is invalidated by PASID, TLB by ASID. I used to call it
> "tag", but I'm leaning towards "arch_id" now
> (http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=40fdef74816dd8d8d113100b9e0162fab4cec28d)

I aknowledge I am not crystal clear about that. for a given iommu_domain
don't you have a single asid. Can't you retrieve the asid from the
iommu_domain/arm_smmu_domain/arm_smmu_s1_cfg/arm_smmu_ctx_desc.asid?
Here again I am confused bout the dual iommu_domain/struct device
parameters.

I have another trouble while doing the QEMU integration.
When the guests does an NH_ALL, this propagates an invalidation on the
whole IPA range and we must discriminate that from regular NH_VA calls.
How would you encode the NH_ALL with this API?

Besides I discover you work on virtio-iommu/stage2 enablement ;-)

Thanks

Eric
> 
> Thanks,
> Jean
> 

Re: [RFC 09/13] iommu/smmuv3: Get prepared for nested stage support

[Auger Eric]

Hi Jean-Philippe,

On 08/31/2018 03:20 PM, Jean-Philippe Brucker wrote:
> On 23/08/18 13:17, Eric Auger wrote:
>>  	if (ste->s1_cfg) {
>> -		BUG_ON(ste_live);
> 
> Scary! :) The current code assumes that it can make modifications to the
> STE in any order and enable translation after a sync. So far I haven't
> been able to find anything that violates this rule in the spec: "If
> software modifies the structure while it is valid, it must not allow the
> structure to enter an invalid intermediate state." So maybe it's fine,
> though to be safe I would have started with disabling the STE
> (http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=936e49f923e101c061269eadd5fa43fef819d2e9)

Yep this works with my setup but I was waiting for such kind of comments
to turn this prototype into something more "production level" ;-) Did
you send anything upstream, related to this branch?

Thanks

Eric
> 
> Thanks,
> Jean
> 

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Jean-Philippe Brucker]

On 31/08/2018 14:52, Auger Eric wrote:
> Do we agree here we can get rid of the struct device * parameter?

That's fine by me, in my opinion the bind operation should only be on
the domain, like map/unmap. For the invalidation however, I think we
need to keep the device as an optional parameter, because the guest may
want to invalidate the ATC (DTLB) of a single device.

Thanks,
Jean

Re: [RFC 02/13] iommu: Introduce tlb_invalidate API

[Jean-Philippe Brucker]

On 31/08/2018 15:07, Auger Eric wrote:
>> Since the ioctl will be used to combine invalidations (invalidate both
>> ATC and TLB with a single call), we need an additional ASID field for
>> the SMMU - ATC is invalidated by PASID, TLB by ASID. I used to call it
>> "tag", but I'm leaning towards "arch_id" now
>> (http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=40fdef74816dd8d8d113100b9e0162fab4cec28d)
> 
> I aknowledge I am not crystal clear about that. for a given iommu_domain
> don't you have a single asid. Can't you retrieve the asid from the
> iommu_domain/arm_smmu_domain/arm_smmu_s1_cfg/arm_smmu_ctx_desc.asid?
> Here again I am confused bout the dual iommu_domain/struct device
> parameters.

In nested mode, ASIDs are allocated by the guest and written into the CD
table. Even if there is a single CD it will still be private to the
guest. When receiving the invalidation, the host could walk the CD
tables to retrieve the ASID, but it's not guaranteed to be here anymore:
the guest could well clear a CD before sending the invalidate command.

> I have another trouble while doing the QEMU integration.
> When the guests does an NH_ALL, this propagates an invalidation on the
> whole IPA range and we must discriminate that from regular NH_VA calls.
> How would you encode the NH_ALL with this API?

I think that translates to an invalidate-all for the domain:

struct tlb_iommu_invalidate_info info = {
	.hdr.type = IOMMU_INV_TYPE_TLB,
	.granularity = IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
};

Reading the spec again, I though the API was missing a way to encode
TLBI_NH_VAA, invalidate a range for all ASIDs. Although it feels
contrived, we could represent it with the following:

struct tlb_iommu_invalidate_info info = {
	.hdr.type = IOMMU_INV_TYPE_TLB,
	.granularity = IOMMU_INV_GRANU_PAGE_PASID,
	.flags = IOMMU_INVALIDATE_GLOBAL_PAGE,
	.addr = ...
};

Thanks,
Jean

Re: [RFC 09/13] iommu/smmuv3: Get prepared for nested stage support

[Jean-Philippe Brucker]

On 31/08/2018 15:11, Auger Eric wrote:
> Yep this works with my setup but I was waiting for such kind of comments
> to turn this prototype into something more "production level" ;-) Did
> you send anything upstream, related to this branch?

No I didn't have time to clean it up yet, and don't know when I'll be
able to send it out. With host SVA and virtio-iommu, I have enough
patches in flight to keep me busy for a while :) But you can obviously
pick patches from that branch if they help.

Thanks,
Jean

Re: [RFC 02/13] iommu: Introduce tlb_invalidate API

[Auger Eric]

Hi Jean-Philippe,
On 09/03/2018 02:28 PM, Jean-Philippe Brucker wrote:
> On 31/08/2018 15:07, Auger Eric wrote:
>>> Since the ioctl will be used to combine invalidations (invalidate both
>>> ATC and TLB with a single call), we need an additional ASID field for
>>> the SMMU - ATC is invalidated by PASID, TLB by ASID. I used to call it
>>> "tag", but I'm leaning towards "arch_id" now
>>> (http://www.linux-arm.org/git?p=linux-jpb.git;a=commitdiff;h=40fdef74816dd8d8d113100b9e0162fab4cec28d)
>>
>> I aknowledge I am not crystal clear about that. for a given iommu_domain
>> don't you have a single asid. Can't you retrieve the asid from the
>> iommu_domain/arm_smmu_domain/arm_smmu_s1_cfg/arm_smmu_ctx_desc.asid?
>> Here again I am confused bout the dual iommu_domain/struct device
>> parameters.
> 
> In nested mode, ASIDs are allocated by the guest and written into the CD
> table. Even if there is a single CD it will still be private to the
> guest. When receiving the invalidation, the host could walk the CD
> tables to retrieve the ASID, but it's not guaranteed to be here anymore:
> the guest could well clear a CD before sending the invalidate command.

That's fully correct. I messed up at the beginning with this asid and
that's perfectly true the asid needs to be passed. I will respin
accordingly.

> 
>> I have another trouble while doing the QEMU integration.
>> When the guests does an NH_ALL, this propagates an invalidation on the
>> whole IPA range and we must discriminate that from regular NH_VA calls.
>> How would you encode the NH_ALL with this API?
> 
> I think that translates to an invalidate-all for the domain:
> 
> struct tlb_iommu_invalidate_info info = {
> 	.hdr.type = IOMMU_INV_TYPE_TLB,
> 	.granularity = IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
OK
> };
> 
> Reading the spec again, I though the API was missing a way to encode
> TLBI_NH_VAA, invalidate a range for all ASIDs. Although it feels
> contrived, we could represent it with the following:
> 
> struct tlb_iommu_invalidate_info info = {
> 	.hdr.type = IOMMU_INV_TYPE_TLB,
> 	.granularity = IOMMU_INV_GRANU_PAGE_PASID,
> 	.flags = IOMMU_INVALIDATE_GLOBAL_PAGE,
> 	.addr = ...
OK

Also what about CMD_CFI_CD(_ALL) propagation. Is it an
IOMMU_INV_TYPE_PASID invalidation?

Thanks

Eric
> };
> 
> Thanks,
> Jean
> 

Re: [RFC 09/13] iommu/smmuv3: Get prepared for nested stage support

[Auger Eric]

Hi Jean-Philippe,

On 09/03/2018 02:29 PM, Jean-Philippe Brucker wrote:
> On 31/08/2018 15:11, Auger Eric wrote:
>> Yep this works with my setup but I was waiting for such kind of comments
>> to turn this prototype into something more "production level" ;-) Did
>> you send anything upstream, related to this branch?
> 
> No I didn't have time to clean it up yet, and don't know when I'll be
> able to send it out. With host SVA and virtio-iommu, I have enough
> patches in flight to keep me busy for a while :) But you can obviously
> pick patches from that branch if they help.

I understand ;-) I will update the uapi according to our last
discussions and I will look at your patches, especially smmu-v3 changes,
adding your Sob when relevant.

Thanks

Eric
> 
> Thanks,
> Jean
> 

Re: [RFC 02/13] iommu: Introduce tlb_invalidate API

[Jean-Philippe Brucker]

On 03/09/2018 13:41, Auger Eric wrote:
> Also what about CMD_CFI_CD(_ALL) propagation. Is it an
> IOMMU_INV_TYPE_PASID invalidation?

Yes, INV_TYPE_PASID corresponds to the config invalidation. CMD_CFGI_CD
is granule INV_GRANU_PASID_SEL and CMD_CFGI_CD_ALL is granule
INV_GRANU_ALL_PASID

Thanks,
Jean

RE: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Tian, Kevin]

> From: Auger Eric
> Sent: Friday, August 31, 2018 9:52 PM
> 
> Hi Jean-Philippe,
> 
> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
> > Hi Eric,
> >
> > On 23/08/18 16:25, Auger Eric wrote:
> >>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct
> device *dev,
> >>> +			   struct iommu_guest_stage_config *cfg)
> >
> > About the name change from iommu_bind_pasid_table: is the intent to
> > reuse this API for SMMUv2, which supports nested but not PASID? Seems
> > like a good idea but "iommu_bind_table" may be better since "stage" is
> > only used by Arm.
> 
> At the moment I don't target SMUv2 but just SMMUv3. My focus was on
> nested stage enablement without enabling the multi-CD feature (PASID),
> whish is not supported by the QEMU vSMMUv3. Afterwards I realized that
> basically we are pointing to a CD or PASID table and that's about the
> same. I don't have a strong opinion on the name, iommu_bind_guest_table
> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is ARM
> vocable (level for Intel?)

Intel uses first level/second level. 

iommu_bind_table is a bit confusing. what should people take table as?
there is PASID table. there is also page table linked in each stage/level. and
maybe other tables in vendor-specific definition.

to me iommu_bind_pasid_table is still clearer. anyway in other places
we've used pasid explicitly in vfio/iommu APIs, then it should be general
enough to represent various implementations.

Thanks
Kevin

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Auger Eric]

Hi Kevin,
On 09/04/2018 09:57 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Friday, August 31, 2018 9:52 PM
>>
>> Hi Jean-Philippe,
>>
>> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
>>> Hi Eric,
>>>
>>> On 23/08/18 16:25, Auger Eric wrote:
>>>>> +int iommu_bind_guest_stage(struct iommu_domain *domain, struct
>> device *dev,
>>>>> +			   struct iommu_guest_stage_config *cfg)
>>>
>>> About the name change from iommu_bind_pasid_table: is the intent to
>>> reuse this API for SMMUv2, which supports nested but not PASID? Seems
>>> like a good idea but "iommu_bind_table" may be better since "stage" is
>>> only used by Arm.
>>
>> At the moment I don't target SMUv2 but just SMMUv3. My focus was on
>> nested stage enablement without enabling the multi-CD feature (PASID),
>> whish is not supported by the QEMU vSMMUv3. Afterwards I realized that
>> basically we are pointing to a CD or PASID table and that's about the
>> same. I don't have a strong opinion on the name, iommu_bind_guest_table
>> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is ARM
>> vocable (level for Intel?)
> 
> Intel uses first level/second level. 
> 
> iommu_bind_table is a bit confusing. what should people take table as?
> there is PASID table. there is also page table linked in each stage/level. and
> maybe other tables in vendor-specific definition.
> 
> to me iommu_bind_pasid_table is still clearer. anyway in other places
> we've used pasid explicitly in vfio/iommu APIs, then it should be general
> enough to represent various implementations.

Fine for me.

However I I would suggest to rename the original iommu_sva_invalidate
into something that is SVA unrelated. iommu_tlb_invalidate is not OK as
this API also is used to invalidate context caches - which are not
iotlbs -. What about iommu_cache_invalidate?

At least we must clarify that this API can be used for something else
than SVA enablement.

Thanks

Eric
> 
> Thanks
> Kevin
> 
> 
> 

RE: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Tian, Kevin]

> From: Auger Eric
> Sent: Tuesday, September 4, 2018 4:11 PM
> 
> Hi Kevin,
> On 09/04/2018 09:57 AM, Tian, Kevin wrote:
> >> From: Auger Eric
> >> Sent: Friday, August 31, 2018 9:52 PM
> >>
> >> Hi Jean-Philippe,
> >>
> >> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
> >>> Hi Eric,
> >>>
> >>> On 23/08/18 16:25, Auger Eric wrote:
> >>>>> +int iommu_bind_guest_stage(struct iommu_domain *domain,
> struct
> >> device *dev,
> >>>>> +			   struct iommu_guest_stage_config *cfg)
> >>>
> >>> About the name change from iommu_bind_pasid_table: is the intent to
> >>> reuse this API for SMMUv2, which supports nested but not PASID?
> Seems
> >>> like a good idea but "iommu_bind_table" may be better since "stage" is
> >>> only used by Arm.
> >>
> >> At the moment I don't target SMUv2 but just SMMUv3. My focus was on
> >> nested stage enablement without enabling the multi-CD feature (PASID),
> >> whish is not supported by the QEMU vSMMUv3. Afterwards I realized
> that
> >> basically we are pointing to a CD or PASID table and that's about the
> >> same. I don't have a strong opinion on the name,
> iommu_bind_guest_table
> >> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is
> ARM
> >> vocable (level for Intel?)
> >
> > Intel uses first level/second level.
> >
> > iommu_bind_table is a bit confusing. what should people take table as?
> > there is PASID table. there is also page table linked in each stage/level.
> and
> > maybe other tables in vendor-specific definition.
> >
> > to me iommu_bind_pasid_table is still clearer. anyway in other places
> > we've used pasid explicitly in vfio/iommu APIs, then it should be general
> > enough to represent various implementations.
> 
> Fine for me.
> 
> However I I would suggest to rename the original iommu_sva_invalidate
> into something that is SVA unrelated. iommu_tlb_invalidate is not OK as
> this API also is used to invalidate context caches - which are not
> iotlbs -. What about iommu_cache_invalidate?
> 
> At least we must clarify that this API can be used for something else
> than SVA enablement.
> 

Agree. using SVA is limiting.

I also agree that iommu_cache_invalidate is better, though I don't think
you want to pass guest context cache invalidation to host. that information
is fully under host control. :-)

Thanks
Kevin

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Auger Eric]

Hi Kevin,

On 09/04/2018 10:34 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Tuesday, September 4, 2018 4:11 PM
>>
>> Hi Kevin,
>> On 09/04/2018 09:57 AM, Tian, Kevin wrote:
>>>> From: Auger Eric
>>>> Sent: Friday, August 31, 2018 9:52 PM
>>>>
>>>> Hi Jean-Philippe,
>>>>
>>>> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
>>>>> Hi Eric,
>>>>>
>>>>> On 23/08/18 16:25, Auger Eric wrote:
>>>>>>> +int iommu_bind_guest_stage(struct iommu_domain *domain,
>> struct
>>>> device *dev,
>>>>>>> +			   struct iommu_guest_stage_config *cfg)
>>>>>
>>>>> About the name change from iommu_bind_pasid_table: is the intent to
>>>>> reuse this API for SMMUv2, which supports nested but not PASID?
>> Seems
>>>>> like a good idea but "iommu_bind_table" may be better since "stage" is
>>>>> only used by Arm.
>>>>
>>>> At the moment I don't target SMUv2 but just SMMUv3. My focus was on
>>>> nested stage enablement without enabling the multi-CD feature (PASID),
>>>> whish is not supported by the QEMU vSMMUv3. Afterwards I realized
>> that
>>>> basically we are pointing to a CD or PASID table and that's about the
>>>> same. I don't have a strong opinion on the name,
>> iommu_bind_guest_table
>>>> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is
>> ARM
>>>> vocable (level for Intel?)
>>>
>>> Intel uses first level/second level.
>>>
>>> iommu_bind_table is a bit confusing. what should people take table as?
>>> there is PASID table. there is also page table linked in each stage/level.
>> and
>>> maybe other tables in vendor-specific definition.
>>>
>>> to me iommu_bind_pasid_table is still clearer. anyway in other places
>>> we've used pasid explicitly in vfio/iommu APIs, then it should be general
>>> enough to represent various implementations.
>>
>> Fine for me.
>>
>> However I I would suggest to rename the original iommu_sva_invalidate
>> into something that is SVA unrelated. iommu_tlb_invalidate is not OK as
>> this API also is used to invalidate context caches - which are not
>> iotlbs -. What about iommu_cache_invalidate?
>>
>> At least we must clarify that this API can be used for something else
>> than SVA enablement.
>>
> 
> Agree. using SVA is limiting.
> 
> I also agree that iommu_cache_invalidate is better, though I don't think
> you want to pass guest context cache invalidation to host. that information
> is fully under host control. :-)

I think the confusion comes from the different terminology used in VTD
and ARM SMMU spec.

Your PASID table ~ ARM SMMU Context Descriptor (CD) table
Your Root Entry/Context Entry ~ ARM SMMU Stream Table Entry (STE)

So I meant guesr invalidates its Context Descriptor cache. He "owns"
those. Host owns the STE.

Thanks

Eric

> 
> Thanks
> Kevin
> 

RE: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Tian, Kevin]

> From: Auger Eric
> Sent: Tuesday, September 4, 2018 4:41 PM
> 
> Hi Kevin,
> 
> On 09/04/2018 10:34 AM, Tian, Kevin wrote:
> >> From: Auger Eric
> >> Sent: Tuesday, September 4, 2018 4:11 PM
> >>
> >> Hi Kevin,
> >> On 09/04/2018 09:57 AM, Tian, Kevin wrote:
> >>>> From: Auger Eric
> >>>> Sent: Friday, August 31, 2018 9:52 PM
> >>>>
> >>>> Hi Jean-Philippe,
> >>>>
> >>>> On 08/31/2018 03:11 PM, Jean-Philippe Brucker wrote:
> >>>>> Hi Eric,
> >>>>>
> >>>>> On 23/08/18 16:25, Auger Eric wrote:
> >>>>>>> +int iommu_bind_guest_stage(struct iommu_domain *domain,
> >> struct
> >>>> device *dev,
> >>>>>>> +			   struct iommu_guest_stage_config *cfg)
> >>>>>
> >>>>> About the name change from iommu_bind_pasid_table: is the intent
> to
> >>>>> reuse this API for SMMUv2, which supports nested but not PASID?
> >> Seems
> >>>>> like a good idea but "iommu_bind_table" may be better since "stage"
> is
> >>>>> only used by Arm.
> >>>>
> >>>> At the moment I don't target SMUv2 but just SMMUv3. My focus was
> on
> >>>> nested stage enablement without enabling the multi-CD feature
> (PASID),
> >>>> whish is not supported by the QEMU vSMMUv3. Afterwards I realized
> >> that
> >>>> basically we are pointing to a CD or PASID table and that's about the
> >>>> same. I don't have a strong opinion on the name,
> >> iommu_bind_guest_table
> >>>> or iommu_bind_pasid_table would be fine with me. Indeed "stage" is
> >> ARM
> >>>> vocable (level for Intel?)
> >>>
> >>> Intel uses first level/second level.
> >>>
> >>> iommu_bind_table is a bit confusing. what should people take table as?
> >>> there is PASID table. there is also page table linked in each stage/level.
> >> and
> >>> maybe other tables in vendor-specific definition.
> >>>
> >>> to me iommu_bind_pasid_table is still clearer. anyway in other places
> >>> we've used pasid explicitly in vfio/iommu APIs, then it should be
> general
> >>> enough to represent various implementations.
> >>
> >> Fine for me.
> >>
> >> However I I would suggest to rename the original iommu_sva_invalidate
> >> into something that is SVA unrelated. iommu_tlb_invalidate is not OK as
> >> this API also is used to invalidate context caches - which are not
> >> iotlbs -. What about iommu_cache_invalidate?
> >>
> >> At least we must clarify that this API can be used for something else
> >> than SVA enablement.
> >>
> >
> > Agree. using SVA is limiting.
> >
> > I also agree that iommu_cache_invalidate is better, though I don't think
> > you want to pass guest context cache invalidation to host. that
> information
> > is fully under host control. :-)
> 
> I think the confusion comes from the different terminology used in VTD
> and ARM SMMU spec.
> 
> Your PASID table ~ ARM SMMU Context Descriptor (CD) table
> Your Root Entry/Context Entry ~ ARM SMMU Stream Table Entry (STE)
> 
> So I meant guesr invalidates its Context Descriptor cache. He "owns"
> those. Host owns the STE.
> 

yes, then it makes sense.

Thanks
Kevin

Re: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Jean-Philippe Brucker]

On 04/09/2018 09:41, Auger Eric wrote:
> I think the confusion comes from the different terminology used in VTD
> and ARM SMMU spec.
> 
> Your PASID table ~ ARM SMMU Context Descriptor (CD) table
> Your Root Entry/Context Entry ~ ARM SMMU Stream Table Entry (STE)

In past discussions we used "PASID table (entry)" and "device context"
respectively. For clarity we should probably stick to those names unless
we discuss arch-specific patches

Thanks,
Jean

RE: [RFC 01/13] iommu: Introduce bind_guest_stage API

[Tian, Kevin]

> From: Jean-Philippe Brucker [mailto:jean-philippe.brucker@arm.com]
> Sent: Tuesday, September 4, 2018 5:53 PM
> 
> On 04/09/2018 09:41, Auger Eric wrote:
> > I think the confusion comes from the different terminology used in VTD
> > and ARM SMMU spec.
> >
> > Your PASID table ~ ARM SMMU Context Descriptor (CD) table
> > Your Root Entry/Context Entry ~ ARM SMMU Stream Table Entry (STE)
> 
> In past discussions we used "PASID table (entry)" and "device context"
> respectively. For clarity we should probably stick to those names unless
> we discuss arch-specific patches
> 

yes, that is a good abstraction. let's try to use them in related discussions
and patch series.

Thanks
Kevin