All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-04  1:41 ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu

This patch set improves the PASID id and PASID table management
for Intel IOMMU driver.

PATCH 1~3 replace per IOMMU idr name space with a global one.
Current per IOMMU idr doesn't work in some cases where one
application (associated with a PASID) might talk to two physical
devices simultaneously while the two devices could reside behind
two different IOMMU units.

PATCH 4~9 implement per domain PASID table. Current per IOMMU
PASID table implementation is insecure in the cases where
multiple devices under one single IOMMU unit support PASID
feature. With per domain PASID table, we can achieve finer
protection and isolation granularity.

Best regards,
Lu Baolu

Change log:
v1->v2:
  - Patches have been reviewed by "Liu Yi L <yi.l.liu@intel.com>".
  - An error case handling was added in PATCH 6/9.
  - Some commit messages are refined to be more accurate.

Lu Baolu (9):
  iommu/vt-d: Global PASID name space
  iommu/vt-d: Decouple idr bond pointer from svm
  iommu/vt-d: Use global PASID for SVM usage
  iommu/vt-d: Move device_domain_info to header
  iommu/vt-d: Per domain pasid table interfaces
  iommu/vt-d: Allocate and free pasid table
  iommu/vt-d: Calculate PTS value
  iommu/vt-d: Use per-domain pasid table
  iommu/vt-d: Clean up PASID talbe management for SVM

 drivers/iommu/Makefile      |   2 +-
 drivers/iommu/intel-iommu.c | 128 ++++++++++++++++-------------------------
 drivers/iommu/intel-pasid.c | 135 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h |  34 +++++++++++
 drivers/iommu/intel-svm.c   |  98 ++++++++++++++++----------------
 include/linux/intel-iommu.h |  90 +++++++++++++++++++++++++++--
 6 files changed, 352 insertions(+), 135 deletions(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-04  1:41 ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

This patch set improves the PASID id and PASID table management
for Intel IOMMU driver.

PATCH 1~3 replace per IOMMU idr name space with a global one.
Current per IOMMU idr doesn't work in some cases where one
application (associated with a PASID) might talk to two physical
devices simultaneously while the two devices could reside behind
two different IOMMU units.

PATCH 4~9 implement per domain PASID table. Current per IOMMU
PASID table implementation is insecure in the cases where
multiple devices under one single IOMMU unit support PASID
feature. With per domain PASID table, we can achieve finer
protection and isolation granularity.

Best regards,
Lu Baolu

Change log:
v1->v2:
  - Patches have been reviewed by "Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>".
  - An error case handling was added in PATCH 6/9.
  - Some commit messages are refined to be more accurate.

Lu Baolu (9):
  iommu/vt-d: Global PASID name space
  iommu/vt-d: Decouple idr bond pointer from svm
  iommu/vt-d: Use global PASID for SVM usage
  iommu/vt-d: Move device_domain_info to header
  iommu/vt-d: Per domain pasid table interfaces
  iommu/vt-d: Allocate and free pasid table
  iommu/vt-d: Calculate PTS value
  iommu/vt-d: Use per-domain pasid table
  iommu/vt-d: Clean up PASID talbe management for SVM

 drivers/iommu/Makefile      |   2 +-
 drivers/iommu/intel-iommu.c | 128 ++++++++++++++++-------------------------
 drivers/iommu/intel-pasid.c | 135 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h |  34 +++++++++++
 drivers/iommu/intel-svm.c   |  98 ++++++++++++++++----------------
 include/linux/intel-iommu.h |  90 +++++++++++++++++++++++++++--
 6 files changed, 352 insertions(+), 135 deletions(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v2 1/9] iommu/vt-d: Global PASID name space
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This adds the system wide PASID name space for the PASID
allocation. Currently we are using per IOMMU PASID name
spaces which are not suitable for some use cases. For an
example, one application (associated with a PASID) might
talk to two physical devices simultaneously while the two
devices could reside behind two different IOMMU units.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/Makefile      |  2 +-
 drivers/iommu/intel-iommu.c | 13 ++++++++++
 drivers/iommu/intel-pasid.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h | 30 +++++++++++++++++++++++
 4 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb6958..0a190b4 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
 obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..98c5ae9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -53,6 +53,7 @@
 #include <asm/iommu.h>
 
 #include "irq_remapping.h"
+#include "intel-pasid.h"
 
 #define ROOT_SIZE		VTD_PAGE_SIZE
 #define CONTEXT_SIZE		VTD_PAGE_SIZE
@@ -3265,6 +3266,18 @@ static int __init init_dmars(void)
 	}
 
 	for_each_active_iommu(iommu, drhd) {
+		/*
+		 * Find the max pasid size of all IOMMU's in the system.
+		 * we need to ensure the system pasid table is no bigger
+		 * than the smallest supported.
+		 */
+		if (pasid_enabled(iommu)) {
+			u32 temp = 2 << ecap_pss(iommu->ecap);
+
+			intel_pasid_max_id = min_t(u32, temp,
+						   intel_pasid_max_id);
+		}
+
 		g_iommus[iommu->seq_id] = iommu;
 
 		intel_iommu_init_qi(iommu);
diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
new file mode 100644
index 0000000..0690f39
--- /dev/null
+++ b/drivers/iommu/intel-pasid.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pasid.c - PASID idr, table and entry manipulation
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt
+
+#include <linux/dmar.h>
+#include <linux/intel-iommu.h>
+#include <linux/iommu.h>
+#include <linux/memory.h>
+#include <linux/spinlock.h>
+
+#include "intel-pasid.h"
+
+/*
+ * Intel IOMMU global PASID pool:
+ */
+static DEFINE_SPINLOCK(pasid_lock);
+u32 intel_pasid_max_id = PASID_MAX;
+static DEFINE_IDR(pasid_idr);
+
+int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
+{
+	int ret, min, max;
+
+	min = max_t(int, start, PASID_MIN);
+	max = min_t(int, end, intel_pasid_max_id);
+
+	WARN_ON(in_interrupt());
+	idr_preload(gfp);
+	spin_lock(&pasid_lock);
+	ret = idr_alloc(&pasid_idr, ptr, min, max, GFP_ATOMIC);
+	spin_unlock(&pasid_lock);
+	idr_preload_end();
+
+	return ret;
+}
+
+void intel_pasid_free_id(int pasid)
+{
+	spin_lock(&pasid_lock);
+	idr_remove(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+}
+
+void *intel_pasid_lookup_id(int pasid)
+{
+	void *p;
+
+	spin_lock(&pasid_lock);
+	p = idr_find(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+
+	return p;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
new file mode 100644
index 0000000..0c36af0
--- /dev/null
+++ b/drivers/iommu/intel-pasid.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * intel-pasid.h - PASID idr, table and entry header
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu@linux.intel.com>
+ */
+
+#ifndef __INTEL_PASID_H
+#define __INTEL_PASID_H
+
+/*
+ * Eventually I'm promised we will get a multi-level PASID table
+ * and it won't have to be physically contiguous. Until then,
+ * limit the size because 8MiB contiguous allocations can be hard
+ * to come by. The limit of 0x20000, which is 1MiB for each of
+ * the PASID and PASID-state tables, is somewhat arbitrary.
+ *
+ * PASID 0 is reserved in caching mode (virtualised IOMMU).
+ */
+#define PASID_MIN			0x1
+#define PASID_MAX			0x20000
+
+extern u32 intel_pasid_max_id;
+int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
+void intel_pasid_free_id(int pasid);
+void *intel_pasid_lookup_id(int pasid);
+
+#endif /* __INTEL_PASID_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 1/9] iommu/vt-d: Global PASID name space
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

This adds the system wide PASID name space for the PASID
allocation. Currently we are using per IOMMU PASID name
spaces which are not suitable for some use cases. For an
example, one application (associated with a PASID) might
talk to two physical devices simultaneously while the two
devices could reside behind two different IOMMU units.

Cc: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Suggested-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/Makefile      |  2 +-
 drivers/iommu/intel-iommu.c | 13 ++++++++++
 drivers/iommu/intel-pasid.c | 60 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h | 30 +++++++++++++++++++++++
 4 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb6958..0a190b4 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
 obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..98c5ae9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -53,6 +53,7 @@
 #include <asm/iommu.h>
 
 #include "irq_remapping.h"
+#include "intel-pasid.h"
 
 #define ROOT_SIZE		VTD_PAGE_SIZE
 #define CONTEXT_SIZE		VTD_PAGE_SIZE
@@ -3265,6 +3266,18 @@ static int __init init_dmars(void)
 	}
 
 	for_each_active_iommu(iommu, drhd) {
+		/*
+		 * Find the max pasid size of all IOMMU's in the system.
+		 * we need to ensure the system pasid table is no bigger
+		 * than the smallest supported.
+		 */
+		if (pasid_enabled(iommu)) {
+			u32 temp = 2 << ecap_pss(iommu->ecap);
+
+			intel_pasid_max_id = min_t(u32, temp,
+						   intel_pasid_max_id);
+		}
+
 		g_iommus[iommu->seq_id] = iommu;
 
 		intel_iommu_init_qi(iommu);
diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
new file mode 100644
index 0000000..0690f39
--- /dev/null
+++ b/drivers/iommu/intel-pasid.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pasid.c - PASID idr, table and entry manipulation
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
+ */
+
+#define pr_fmt(fmt)	"DMAR: " fmt
+
+#include <linux/dmar.h>
+#include <linux/intel-iommu.h>
+#include <linux/iommu.h>
+#include <linux/memory.h>
+#include <linux/spinlock.h>
+
+#include "intel-pasid.h"
+
+/*
+ * Intel IOMMU global PASID pool:
+ */
+static DEFINE_SPINLOCK(pasid_lock);
+u32 intel_pasid_max_id = PASID_MAX;
+static DEFINE_IDR(pasid_idr);
+
+int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
+{
+	int ret, min, max;
+
+	min = max_t(int, start, PASID_MIN);
+	max = min_t(int, end, intel_pasid_max_id);
+
+	WARN_ON(in_interrupt());
+	idr_preload(gfp);
+	spin_lock(&pasid_lock);
+	ret = idr_alloc(&pasid_idr, ptr, min, max, GFP_ATOMIC);
+	spin_unlock(&pasid_lock);
+	idr_preload_end();
+
+	return ret;
+}
+
+void intel_pasid_free_id(int pasid)
+{
+	spin_lock(&pasid_lock);
+	idr_remove(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+}
+
+void *intel_pasid_lookup_id(int pasid)
+{
+	void *p;
+
+	spin_lock(&pasid_lock);
+	p = idr_find(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+
+	return p;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
new file mode 100644
index 0000000..0c36af0
--- /dev/null
+++ b/drivers/iommu/intel-pasid.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * intel-pasid.h - PASID idr, table and entry header
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
+ */
+
+#ifndef __INTEL_PASID_H
+#define __INTEL_PASID_H
+
+/*
+ * Eventually I'm promised we will get a multi-level PASID table
+ * and it won't have to be physically contiguous. Until then,
+ * limit the size because 8MiB contiguous allocations can be hard
+ * to come by. The limit of 0x20000, which is 1MiB for each of
+ * the PASID and PASID-state tables, is somewhat arbitrary.
+ *
+ * PASID 0 is reserved in caching mode (virtualised IOMMU).
+ */
+#define PASID_MIN			0x1
+#define PASID_MAX			0x20000
+
+extern u32 intel_pasid_max_id;
+int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
+void intel_pasid_free_id(int pasid);
+void *intel_pasid_lookup_id(int pasid);
+
+#endif /* __INTEL_PASID_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 2/9] iommu/vt-d: Decouple idr bond pointer from svm
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

As we move the PASID idr out of SVM code and make it serving
as a global PASID name space, the consumer can specify a ptr
to bind it with a PASID. We shouldn't assume that each PASID
will be bond with a ptr of struct intel_svm anymore.

This patch cleans up a idr_for_each_entry() usage in the SVM
code. It's required to replace the SVM-specific idr with the
global PASID idr.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-svm.c   | 14 ++++++++++----
 include/linux/intel-iommu.h |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e8cd984..983af0c 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -298,6 +298,7 @@ static const struct mmu_notifier_ops intel_mmuops = {
 };
 
 static DEFINE_MUTEX(pasid_mutex);
+static LIST_HEAD(global_svm_list);
 
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
 {
@@ -329,13 +330,13 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 
 	mutex_lock(&pasid_mutex);
 	if (pasid && !(flags & SVM_FLAG_PRIVATE_PASID)) {
-		int i;
+		struct intel_svm *t;
 
-		idr_for_each_entry(&iommu->pasid_idr, svm, i) {
-			if (svm->mm != mm ||
-			    (svm->flags & SVM_FLAG_PRIVATE_PASID))
+		list_for_each_entry(t, &global_svm_list, list) {
+			if (t->mm != mm || (t->flags & SVM_FLAG_PRIVATE_PASID))
 				continue;
 
+			svm = t;
 			if (svm->pasid >= pasid_max) {
 				dev_warn(dev,
 					 "Limited PASID width. Cannot use existing PASID %d\n",
@@ -404,6 +405,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		svm->mm = mm;
 		svm->flags = flags;
 		INIT_LIST_HEAD_RCU(&svm->devs);
+		INIT_LIST_HEAD(&svm->list);
 		ret = -ENOMEM;
 		if (mm) {
 			ret = mmu_notifier_register(&svm->notifier, mm);
@@ -430,6 +432,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		 */
 		if (cap_caching_mode(iommu->cap))
 			intel_flush_pasid_dev(svm, sdev, svm->pasid);
+
+		list_add_tail(&svm->list, &global_svm_list);
 	}
 	list_add_rcu(&sdev->list, &svm->devs);
 
@@ -485,6 +489,8 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
 
+					list_del(&svm->list);
+
 					/* We mandate that no page faults may be outstanding
 					 * for the PASID when intel_svm_unbind_mm() is called.
 					 * If that is not obeyed, subtle errors will happen.
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ef169d6..795717e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -486,6 +486,7 @@ struct intel_svm {
 	int flags;
 	int pasid;
 	struct list_head devs;
+	struct list_head list;
 };
 
 extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 2/9] iommu/vt-d: Decouple idr bond pointer from svm
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

As we move the PASID idr out of SVM code and make it serving
as a global PASID name space, the consumer can specify a ptr
to bind it with a PASID. We shouldn't assume that each PASID
will be bond with a ptr of struct intel_svm anymore.

This patch cleans up a idr_for_each_entry() usage in the SVM
code. It's required to replace the SVM-specific idr with the
global PASID idr.

Cc: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/intel-svm.c   | 14 ++++++++++----
 include/linux/intel-iommu.h |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e8cd984..983af0c 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -298,6 +298,7 @@ static const struct mmu_notifier_ops intel_mmuops = {
 };
 
 static DEFINE_MUTEX(pasid_mutex);
+static LIST_HEAD(global_svm_list);
 
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
 {
@@ -329,13 +330,13 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 
 	mutex_lock(&pasid_mutex);
 	if (pasid && !(flags & SVM_FLAG_PRIVATE_PASID)) {
-		int i;
+		struct intel_svm *t;
 
-		idr_for_each_entry(&iommu->pasid_idr, svm, i) {
-			if (svm->mm != mm ||
-			    (svm->flags & SVM_FLAG_PRIVATE_PASID))
+		list_for_each_entry(t, &global_svm_list, list) {
+			if (t->mm != mm || (t->flags & SVM_FLAG_PRIVATE_PASID))
 				continue;
 
+			svm = t;
 			if (svm->pasid >= pasid_max) {
 				dev_warn(dev,
 					 "Limited PASID width. Cannot use existing PASID %d\n",
@@ -404,6 +405,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		svm->mm = mm;
 		svm->flags = flags;
 		INIT_LIST_HEAD_RCU(&svm->devs);
+		INIT_LIST_HEAD(&svm->list);
 		ret = -ENOMEM;
 		if (mm) {
 			ret = mmu_notifier_register(&svm->notifier, mm);
@@ -430,6 +432,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		 */
 		if (cap_caching_mode(iommu->cap))
 			intel_flush_pasid_dev(svm, sdev, svm->pasid);
+
+		list_add_tail(&svm->list, &global_svm_list);
 	}
 	list_add_rcu(&sdev->list, &svm->devs);
 
@@ -485,6 +489,8 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
 
+					list_del(&svm->list);
+
 					/* We mandate that no page faults may be outstanding
 					 * for the PASID when intel_svm_unbind_mm() is called.
 					 * If that is not obeyed, subtle errors will happen.
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ef169d6..795717e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -486,6 +486,7 @@ struct intel_svm {
 	int flags;
 	int pasid;
 	struct list_head devs;
+	struct list_head list;
 };
 
 extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 3/9] iommu/vt-d: Use global PASID for SVM usage
  2018-05-04  1:41 ` Lu Baolu
                   ` (2 preceding siblings ...)
  (?)
@ 2018-05-04  1:41 ` Lu Baolu
  -1 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This patch switches PASID management for SVM from per iommu
idr to the global idr.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-svm.c   | 22 +++++++++++-----------
 include/linux/intel-iommu.h |  1 -
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 983af0c..24d0ea1 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -26,6 +26,8 @@
 #include <linux/interrupt.h>
 #include <asm/page.h>
 
+#include "intel-pasid.h"
+
 #define PASID_ENTRY_P		BIT_ULL(0)
 #define PASID_ENTRY_FLPM_5LP	BIT_ULL(9)
 #define PASID_ENTRY_SRE		BIT_ULL(11)
@@ -85,8 +87,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 				iommu->name);
 	}
 
-	idr_init(&iommu->pasid_idr);
-
 	return 0;
 }
 
@@ -102,7 +102,7 @@ int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
 		free_pages((unsigned long)iommu->pasid_state_table, order);
 		iommu->pasid_state_table = NULL;
 	}
-	idr_destroy(&iommu->pasid_idr);
+
 	return 0;
 }
 
@@ -392,9 +392,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 			pasid_max = iommu->pasid_max;
 
 		/* Do not use PASID 0 in caching mode (virtualised IOMMU) */
-		ret = idr_alloc(&iommu->pasid_idr, svm,
-				!!cap_caching_mode(iommu->cap),
-				pasid_max - 1, GFP_KERNEL);
+		ret = intel_pasid_alloc_id(svm,
+					   !!cap_caching_mode(iommu->cap),
+					   pasid_max - 1, GFP_KERNEL);
 		if (ret < 0) {
 			kfree(svm);
 			kfree(sdev);
@@ -410,7 +410,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		if (mm) {
 			ret = mmu_notifier_register(&svm->notifier, mm);
 			if (ret) {
-				idr_remove(&svm->iommu->pasid_idr, svm->pasid);
+				intel_pasid_free_id(svm->pasid);
 				kfree(svm);
 				kfree(sdev);
 				goto out;
@@ -460,7 +460,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 	if (!iommu || !iommu->pasid_table)
 		goto out;
 
-	svm = idr_find(&iommu->pasid_idr, pasid);
+	svm = intel_pasid_lookup_id(pasid);
 	if (!svm)
 		goto out;
 
@@ -485,7 +485,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 					svm->iommu->pasid_table[svm->pasid].val = 0;
 					wmb();
 
-					idr_remove(&svm->iommu->pasid_idr, svm->pasid);
+					intel_pasid_free_id(svm->pasid);
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
 
@@ -520,7 +520,7 @@ int intel_svm_is_pasid_valid(struct device *dev, int pasid)
 	if (!iommu || !iommu->pasid_table)
 		goto out;
 
-	svm = idr_find(&iommu->pasid_idr, pasid);
+	svm = intel_pasid_lookup_id(pasid);
 	if (!svm)
 		goto out;
 
@@ -618,7 +618,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 
 		if (!svm || svm->pasid != req->pasid) {
 			rcu_read_lock();
-			svm = idr_find(&iommu->pasid_idr, req->pasid);
+			svm = intel_pasid_lookup_id(req->pasid);
 			/* It *can't* go away, because the driver is not permitted
 			 * to unbind the mm while any page faults are outstanding.
 			 * So we only need RCU to protect the internal idr code. */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 795717e..6b5ef6c 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -418,7 +418,6 @@ struct intel_iommu {
 	struct pasid_state_entry *pasid_state_table;
 	struct page_req_dsc *prq;
 	unsigned char prq_name[16];    /* Name for PRQ interrupt */
-	struct idr pasid_idr;
 	u32 pasid_max;
 #endif
 	struct q_inval  *qi;            /* Queued invalidation info */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 4/9] iommu/vt-d: Move device_domain_info to header
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This allows the per device iommu data to be accessed from other
files.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-iommu.c | 62 +++--------------------------------------
 include/linux/intel-iommu.h | 68 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 58 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 98c5ae9..caa0b5c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -381,60 +381,6 @@ static int hw_pass_through = 1;
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])
 
-struct dmar_domain {
-	int	nid;			/* node id */
-
-	unsigned	iommu_refcnt[DMAR_UNITS_SUPPORTED];
-					/* Refcount of devices per iommu */
-
-
-	u16		iommu_did[DMAR_UNITS_SUPPORTED];
-					/* Domain ids per IOMMU. Use u16 since
-					 * domain ids are 16 bit wide according
-					 * to VT-d spec, section 9.3 */
-
-	bool has_iotlb_device;
-	struct list_head devices;	/* all devices' list */
-	struct iova_domain iovad;	/* iova's that belong to this domain */
-
-	struct dma_pte	*pgd;		/* virtual address */
-	int		gaw;		/* max guest address width */
-
-	/* adjusted guest address width, 0 is level 2 30-bit */
-	int		agaw;
-
-	int		flags;		/* flags to find out type of domain */
-
-	int		iommu_coherency;/* indicate coherency of iommu access */
-	int		iommu_snooping; /* indicate snooping control feature*/
-	int		iommu_count;	/* reference count of iommu */
-	int		iommu_superpage;/* Level of superpages supported:
-					   0 == 4KiB (no superpages), 1 == 2MiB,
-					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
-	u64		max_addr;	/* maximum mapped address */
-
-	struct iommu_domain domain;	/* generic domain data structure for
-					   iommu core */
-};
-
-/* PCI domain-device relationship */
-struct device_domain_info {
-	struct list_head link;	/* link to domain siblings */
-	struct list_head global; /* link to global list */
-	u8 bus;			/* PCI bus number */
-	u8 devfn;		/* PCI devfn number */
-	u8 pasid_supported:3;
-	u8 pasid_enabled:1;
-	u8 pri_supported:1;
-	u8 pri_enabled:1;
-	u8 ats_supported:1;
-	u8 ats_enabled:1;
-	u8 ats_qdep;
-	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
-	struct intel_iommu *iommu; /* IOMMU used by this device */
-	struct dmar_domain *domain; /* pointer to domain */
-};
-
 struct dmar_rmrr_unit {
 	struct list_head list;		/* list of rmrr units	*/
 	struct acpi_dmar_header *hdr;	/* ACPI header		*/
@@ -631,7 +577,7 @@ static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
 		domains[did & 0xff] = domain;
 }
 
-static inline void *alloc_pgtable_page(int node)
+void *alloc_pgtable_page(int node)
 {
 	struct page *page;
 	void *vaddr = NULL;
@@ -642,7 +588,7 @@ static inline void *alloc_pgtable_page(int node)
 	return vaddr;
 }
 
-static inline void free_pgtable_page(void *vaddr)
+void free_pgtable_page(void *vaddr)
 {
 	free_page((unsigned long)vaddr);
 }
@@ -725,7 +671,7 @@ int iommu_calculate_agaw(struct intel_iommu *iommu)
 }
 
 /* This functionin only returns single iommu in a domain */
-static struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
+struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 {
 	int iommu_id;
 
@@ -3500,7 +3446,7 @@ static unsigned long intel_alloc_iova(struct device *dev,
 	return iova_pfn;
 }
 
-static struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
+struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
 {
 	struct dmar_domain *domain, *tmp;
 	struct dmar_rmrr_unit *rmrr;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 6b5ef6c..a4463f0 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -31,6 +31,7 @@
 #include <linux/list.h>
 #include <linux/iommu.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/dmar.h>
 
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
@@ -385,6 +386,50 @@ struct pasid_entry;
 struct pasid_state_entry;
 struct page_req_dsc;
 
+struct dmar_domain {
+	int	nid;			/* node id */
+
+	unsigned int	iommu_refcnt[DMAR_UNITS_SUPPORTED];
+					/* Refcount of devices per iommu */
+
+	u16		iommu_did[DMAR_UNITS_SUPPORTED];
+					/*
+					 * Domain ids per IOMMU. Use u16 since
+					 * domain ids are 16 bit wide according
+					 * to VT-d spec, section 9.3
+					 */
+
+	bool has_iotlb_device;
+	struct list_head devices;	/* all devices' list */
+	struct iova_domain iovad;	/* iova's that belong to this domain */
+
+	struct dma_pte	*pgd;		/* virtual address */
+	int		gaw;		/* max guest address width */
+
+	int		agaw;		/*
+					 * adjusted guest address width,
+					 * 0 is level 2 30-bit
+					 */
+
+	int		flags;		/* flags to find out type of domain */
+
+	int		iommu_coherency;/* indicate coherency of iommu access */
+	int		iommu_snooping; /* indicate snooping control feature*/
+	int		iommu_count;	/* reference count of iommu */
+	int		iommu_superpage;/*
+					 * Level of superpages supported:
+					 *  0 == 4KiB (no superpages),
+					 *  1 == 2MiB, 2 == 1GiB,
+					 *  3 == 512GiB, 4 == 1TiB
+					 */
+	u64		max_addr;	/* maximum mapped address */
+
+	struct iommu_domain domain;	/*
+					 * generic domain data structure for
+					 * iommu core
+					 */
+};
+
 struct intel_iommu {
 	void __iomem	*reg; /* Pointer to hardware regs, virtual addr */
 	u64 		reg_phys; /* physical address of hw register set */
@@ -433,6 +478,24 @@ struct intel_iommu {
 	u32		flags;      /* Software defined flags */
 };
 
+/* PCI domain-device relationship */
+struct device_domain_info {
+	struct list_head link;	/* link to domain siblings */
+	struct list_head global; /* link to global list */
+	u8 bus;			/* PCI bus number */
+	u8 devfn;		/* PCI devfn number */
+	u8 pasid_supported:3;
+	u8 pasid_enabled:1;
+	u8 pri_supported:1;
+	u8 pri_enabled:1;
+	u8 ats_supported:1;
+	u8 ats_enabled:1;
+	u8 ats_qdep;
+	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
+	struct intel_iommu *iommu; /* IOMMU used by this device */
+	struct dmar_domain *domain; /* pointer to domain */
+};
+
 static inline void __iommu_flush_cache(
 	struct intel_iommu *iommu, void *addr, int size)
 {
@@ -459,6 +522,11 @@ extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
 
 extern int dmar_ir_support(void);
 
+struct dmar_domain *get_valid_domain_for_dev(struct device *dev);
+void *alloc_pgtable_page(int node);
+void free_pgtable_page(void *vaddr);
+struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
 extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 4/9] iommu/vt-d: Move device_domain_info to header
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

This allows the per device iommu data to be accessed from other
files.

Cc: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/intel-iommu.c | 62 +++--------------------------------------
 include/linux/intel-iommu.h | 68 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 58 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 98c5ae9..caa0b5c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -381,60 +381,6 @@ static int hw_pass_through = 1;
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])
 
-struct dmar_domain {
-	int	nid;			/* node id */
-
-	unsigned	iommu_refcnt[DMAR_UNITS_SUPPORTED];
-					/* Refcount of devices per iommu */
-
-
-	u16		iommu_did[DMAR_UNITS_SUPPORTED];
-					/* Domain ids per IOMMU. Use u16 since
-					 * domain ids are 16 bit wide according
-					 * to VT-d spec, section 9.3 */
-
-	bool has_iotlb_device;
-	struct list_head devices;	/* all devices' list */
-	struct iova_domain iovad;	/* iova's that belong to this domain */
-
-	struct dma_pte	*pgd;		/* virtual address */
-	int		gaw;		/* max guest address width */
-
-	/* adjusted guest address width, 0 is level 2 30-bit */
-	int		agaw;
-
-	int		flags;		/* flags to find out type of domain */
-
-	int		iommu_coherency;/* indicate coherency of iommu access */
-	int		iommu_snooping; /* indicate snooping control feature*/
-	int		iommu_count;	/* reference count of iommu */
-	int		iommu_superpage;/* Level of superpages supported:
-					   0 == 4KiB (no superpages), 1 == 2MiB,
-					   2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
-	u64		max_addr;	/* maximum mapped address */
-
-	struct iommu_domain domain;	/* generic domain data structure for
-					   iommu core */
-};
-
-/* PCI domain-device relationship */
-struct device_domain_info {
-	struct list_head link;	/* link to domain siblings */
-	struct list_head global; /* link to global list */
-	u8 bus;			/* PCI bus number */
-	u8 devfn;		/* PCI devfn number */
-	u8 pasid_supported:3;
-	u8 pasid_enabled:1;
-	u8 pri_supported:1;
-	u8 pri_enabled:1;
-	u8 ats_supported:1;
-	u8 ats_enabled:1;
-	u8 ats_qdep;
-	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
-	struct intel_iommu *iommu; /* IOMMU used by this device */
-	struct dmar_domain *domain; /* pointer to domain */
-};
-
 struct dmar_rmrr_unit {
 	struct list_head list;		/* list of rmrr units	*/
 	struct acpi_dmar_header *hdr;	/* ACPI header		*/
@@ -631,7 +577,7 @@ static void set_iommu_domain(struct intel_iommu *iommu, u16 did,
 		domains[did & 0xff] = domain;
 }
 
-static inline void *alloc_pgtable_page(int node)
+void *alloc_pgtable_page(int node)
 {
 	struct page *page;
 	void *vaddr = NULL;
@@ -642,7 +588,7 @@ static inline void *alloc_pgtable_page(int node)
 	return vaddr;
 }
 
-static inline void free_pgtable_page(void *vaddr)
+void free_pgtable_page(void *vaddr)
 {
 	free_page((unsigned long)vaddr);
 }
@@ -725,7 +671,7 @@ int iommu_calculate_agaw(struct intel_iommu *iommu)
 }
 
 /* This functionin only returns single iommu in a domain */
-static struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
+struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 {
 	int iommu_id;
 
@@ -3500,7 +3446,7 @@ static unsigned long intel_alloc_iova(struct device *dev,
 	return iova_pfn;
 }
 
-static struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
+struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
 {
 	struct dmar_domain *domain, *tmp;
 	struct dmar_rmrr_unit *rmrr;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 6b5ef6c..a4463f0 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -31,6 +31,7 @@
 #include <linux/list.h>
 #include <linux/iommu.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
+#include <linux/dmar.h>
 
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
@@ -385,6 +386,50 @@ struct pasid_entry;
 struct pasid_state_entry;
 struct page_req_dsc;
 
+struct dmar_domain {
+	int	nid;			/* node id */
+
+	unsigned int	iommu_refcnt[DMAR_UNITS_SUPPORTED];
+					/* Refcount of devices per iommu */
+
+	u16		iommu_did[DMAR_UNITS_SUPPORTED];
+					/*
+					 * Domain ids per IOMMU. Use u16 since
+					 * domain ids are 16 bit wide according
+					 * to VT-d spec, section 9.3
+					 */
+
+	bool has_iotlb_device;
+	struct list_head devices;	/* all devices' list */
+	struct iova_domain iovad;	/* iova's that belong to this domain */
+
+	struct dma_pte	*pgd;		/* virtual address */
+	int		gaw;		/* max guest address width */
+
+	int		agaw;		/*
+					 * adjusted guest address width,
+					 * 0 is level 2 30-bit
+					 */
+
+	int		flags;		/* flags to find out type of domain */
+
+	int		iommu_coherency;/* indicate coherency of iommu access */
+	int		iommu_snooping; /* indicate snooping control feature*/
+	int		iommu_count;	/* reference count of iommu */
+	int		iommu_superpage;/*
+					 * Level of superpages supported:
+					 *  0 == 4KiB (no superpages),
+					 *  1 == 2MiB, 2 == 1GiB,
+					 *  3 == 512GiB, 4 == 1TiB
+					 */
+	u64		max_addr;	/* maximum mapped address */
+
+	struct iommu_domain domain;	/*
+					 * generic domain data structure for
+					 * iommu core
+					 */
+};
+
 struct intel_iommu {
 	void __iomem	*reg; /* Pointer to hardware regs, virtual addr */
 	u64 		reg_phys; /* physical address of hw register set */
@@ -433,6 +478,24 @@ struct intel_iommu {
 	u32		flags;      /* Software defined flags */
 };
 
+/* PCI domain-device relationship */
+struct device_domain_info {
+	struct list_head link;	/* link to domain siblings */
+	struct list_head global; /* link to global list */
+	u8 bus;			/* PCI bus number */
+	u8 devfn;		/* PCI devfn number */
+	u8 pasid_supported:3;
+	u8 pasid_enabled:1;
+	u8 pri_supported:1;
+	u8 pri_enabled:1;
+	u8 ats_supported:1;
+	u8 ats_enabled:1;
+	u8 ats_qdep;
+	struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
+	struct intel_iommu *iommu; /* IOMMU used by this device */
+	struct dmar_domain *domain; /* pointer to domain */
+};
+
 static inline void __iommu_flush_cache(
 	struct intel_iommu *iommu, void *addr, int size)
 {
@@ -459,6 +522,11 @@ extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
 
 extern int dmar_ir_support(void);
 
+struct dmar_domain *get_valid_domain_for_dev(struct device *dev);
+void *alloc_pgtable_page(int node);
+void free_pgtable_page(void *vaddr);
+struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
 extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 5/9] iommu/vt-d: Per domain pasid table interfaces
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This patch adds the interfaces for per domain pasid table
management. Currently we allocate one pasid table for all
devices under the scope of an IOMMU. It's insecure in the
cases where multiple devices under one single IOMMU unit
support PASID feature. With per domain pasid table, we can
achieve finer protection and isolation granularity.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Suggested-by: Ashok Raj <ashok.raj@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-pasid.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h |  4 +++
 include/linux/intel-iommu.h |  5 +++
 3 files changed, 84 insertions(+)

diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
index 0690f39..b8691a6 100644
--- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -13,6 +13,7 @@
 #include <linux/intel-iommu.h>
 #include <linux/iommu.h>
 #include <linux/memory.h>
+#include <linux/pci.h>
 #include <linux/spinlock.h>
 
 #include "intel-pasid.h"
@@ -58,3 +59,77 @@ void *intel_pasid_lookup_id(int pasid)
 
 	return p;
 }
+
+/*
+ * Interfaces for per domain pasid table management:
+ */
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+			    size_t entry_count)
+{
+	struct device_domain_info *info;
+	struct dmar_domain *domain;
+	struct page *pages;
+	int order;
+
+	info = dev->archdata.iommu;
+	if (WARN_ON(!info || !dev_is_pci(dev) ||
+		    !info->pasid_supported ||
+		    !info->domain))
+		return -EINVAL;
+
+	domain = info->domain;
+
+	if (entry_count > intel_pasid_max_id)
+		entry_count = intel_pasid_max_id;
+
+	order = get_order(entry_size * entry_count);
+	pages = alloc_pages_node(domain->nid, GFP_KERNEL | __GFP_ZERO, order);
+	if (!pages)
+		return -ENOMEM;
+
+	spin_lock(&pasid_lock);
+	if (domain->pasid_table) {
+		__free_pages(pages, order);
+	} else {
+		domain->pasid_table	= page_address(pages);
+		domain->order		= order;
+		domain->max_pasid	= entry_count;
+	}
+	domain->pasid_users++;
+	spin_unlock(&pasid_lock);
+
+	return 0;
+}
+
+void intel_pasid_free_table(struct device *dev)
+{
+	struct dmar_domain *domain;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain || !dev_is_pci(dev))
+		return;
+
+	spin_lock(&pasid_lock);
+	if (domain->pasid_table) {
+		domain->pasid_users--;
+		if (!domain->pasid_users) {
+			free_pages((unsigned long)domain->pasid_table,
+				   domain->order);
+			domain->pasid_table	= NULL;
+			domain->order		= 0;
+			domain->max_pasid	= 0;
+		}
+	}
+	spin_unlock(&pasid_lock);
+}
+
+void *intel_pasid_get_table(struct device *dev)
+{
+	struct dmar_domain *domain;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return NULL;
+
+	return domain->pasid_table;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
index 0c36af0..a90c60b 100644
--- a/drivers/iommu/intel-pasid.h
+++ b/drivers/iommu/intel-pasid.h
@@ -26,5 +26,9 @@ extern u32 intel_pasid_max_id;
 int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
 void intel_pasid_free_id(int pasid);
 void *intel_pasid_lookup_id(int pasid);
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+			    size_t entry_count);
+void intel_pasid_free_table(struct device *dev);
+void *intel_pasid_get_table(struct device *dev);
 
 #endif /* __INTEL_PASID_H */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index a4463f0..bee7a3f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -424,6 +424,11 @@ struct dmar_domain {
 					 */
 	u64		max_addr;	/* maximum mapped address */
 
+	void		*pasid_table;	/* pointer of pasid table */
+	unsigned int	pasid_users;	/* User number of pasid table */
+	int		order;		/* the page order of tables */
+	int		max_pasid;	/* max pasid */
+
 	struct iommu_domain domain;	/*
 					 * generic domain data structure for
 					 * iommu core
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 5/9] iommu/vt-d: Per domain pasid table interfaces
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

This patch adds the interfaces for per domain pasid table
management. Currently we allocate one pasid table for all
devices under the scope of an IOMMU. It's insecure in the
cases where multiple devices under one single IOMMU unit
support PASID feature. With per domain pasid table, we can
achieve finer protection and isolation granularity.

Cc: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Suggested-by: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/intel-pasid.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h |  4 +++
 include/linux/intel-iommu.h |  5 +++
 3 files changed, 84 insertions(+)

diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
index 0690f39..b8691a6 100644
--- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -13,6 +13,7 @@
 #include <linux/intel-iommu.h>
 #include <linux/iommu.h>
 #include <linux/memory.h>
+#include <linux/pci.h>
 #include <linux/spinlock.h>
 
 #include "intel-pasid.h"
@@ -58,3 +59,77 @@ void *intel_pasid_lookup_id(int pasid)
 
 	return p;
 }
+
+/*
+ * Interfaces for per domain pasid table management:
+ */
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+			    size_t entry_count)
+{
+	struct device_domain_info *info;
+	struct dmar_domain *domain;
+	struct page *pages;
+	int order;
+
+	info = dev->archdata.iommu;
+	if (WARN_ON(!info || !dev_is_pci(dev) ||
+		    !info->pasid_supported ||
+		    !info->domain))
+		return -EINVAL;
+
+	domain = info->domain;
+
+	if (entry_count > intel_pasid_max_id)
+		entry_count = intel_pasid_max_id;
+
+	order = get_order(entry_size * entry_count);
+	pages = alloc_pages_node(domain->nid, GFP_KERNEL | __GFP_ZERO, order);
+	if (!pages)
+		return -ENOMEM;
+
+	spin_lock(&pasid_lock);
+	if (domain->pasid_table) {
+		__free_pages(pages, order);
+	} else {
+		domain->pasid_table	= page_address(pages);
+		domain->order		= order;
+		domain->max_pasid	= entry_count;
+	}
+	domain->pasid_users++;
+	spin_unlock(&pasid_lock);
+
+	return 0;
+}
+
+void intel_pasid_free_table(struct device *dev)
+{
+	struct dmar_domain *domain;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain || !dev_is_pci(dev))
+		return;
+
+	spin_lock(&pasid_lock);
+	if (domain->pasid_table) {
+		domain->pasid_users--;
+		if (!domain->pasid_users) {
+			free_pages((unsigned long)domain->pasid_table,
+				   domain->order);
+			domain->pasid_table	= NULL;
+			domain->order		= 0;
+			domain->max_pasid	= 0;
+		}
+	}
+	spin_unlock(&pasid_lock);
+}
+
+void *intel_pasid_get_table(struct device *dev)
+{
+	struct dmar_domain *domain;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return NULL;
+
+	return domain->pasid_table;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
index 0c36af0..a90c60b 100644
--- a/drivers/iommu/intel-pasid.h
+++ b/drivers/iommu/intel-pasid.h
@@ -26,5 +26,9 @@ extern u32 intel_pasid_max_id;
 int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
 void intel_pasid_free_id(int pasid);
 void *intel_pasid_lookup_id(int pasid);
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+			    size_t entry_count);
+void intel_pasid_free_table(struct device *dev);
+void *intel_pasid_get_table(struct device *dev);
 
 #endif /* __INTEL_PASID_H */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index a4463f0..bee7a3f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -424,6 +424,11 @@ struct dmar_domain {
 					 */
 	u64		max_addr;	/* maximum mapped address */
 
+	void		*pasid_table;	/* pointer of pasid table */
+	unsigned int	pasid_users;	/* User number of pasid table */
+	int		order;		/* the page order of tables */
+	int		max_pasid;	/* max pasid */
+
 	struct iommu_domain domain;	/*
 					 * generic domain data structure for
 					 * iommu core
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 6/9] iommu/vt-d: Allocate and free pasid table
  2018-05-04  1:41 ` Lu Baolu
                   ` (5 preceding siblings ...)
  (?)
@ 2018-05-04  1:41 ` Lu Baolu
  -1 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This patch allocates PASID table for a domain at the time when
it is being created (if any devices using this domain supports
PASID feature), and free it when the domain is freed.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-iommu.c | 19 +++++++++++++++++++
 drivers/iommu/intel-svm.c   |  8 --------
 include/linux/intel-iommu.h | 10 ++++++++--
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index caa0b5c..f86302d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2460,6 +2460,24 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 		dev->archdata.iommu = info;
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
+	if (dev && dev_is_pci(dev) && info->pasid_supported) {
+		if (pasid_enabled(iommu)) {
+			size_t size, count;
+
+			size = sizeof(struct pasid_entry);
+			count = min_t(int,
+				      pci_max_pasids(to_pci_dev(dev)),
+				      intel_pasid_max_id);
+			ret = intel_pasid_alloc_table(dev, size, count);
+			if (ret) {
+				pr_err("PASID table allocation for %s failed\n",
+				       dev_name(dev));
+				dmar_remove_one_dev_info(domain, dev);
+				return NULL;
+			}
+		}
+	}
+
 	if (dev && domain_context_mapping(domain, dev)) {
 		pr_err("Domain context map for %s failed\n", dev_name(dev));
 		dmar_remove_one_dev_info(domain, dev);
@@ -4826,6 +4844,7 @@ static void dmar_remove_one_dev_info(struct dmar_domain *domain,
 	unsigned long flags;
 
 	spin_lock_irqsave(&device_domain_lock, flags);
+	intel_pasid_free_table(dev);
 	info = dev->archdata.iommu;
 	__dmar_remove_one_dev_info(info);
 	spin_unlock_irqrestore(&device_domain_lock, flags);
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 24d0ea1..3abc94f 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -34,14 +34,6 @@
 
 static irqreturn_t prq_event_thread(int irq, void *d);
 
-struct pasid_entry {
-	u64 val;
-};
-
-struct pasid_state_entry {
-	u64 val;
-};
-
 int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 {
 	struct page *pages;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index bee7a3f..08e5811 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -382,8 +382,14 @@ enum {
 #define VTD_FLAG_TRANS_PRE_ENABLED	(1 << 0)
 #define VTD_FLAG_IRQ_REMAP_PRE_ENABLED	(1 << 1)
 
-struct pasid_entry;
-struct pasid_state_entry;
+struct pasid_entry {
+	u64 val;
+};
+
+struct pasid_state_entry {
+	u64 val;
+};
+
 struct page_req_dsc;
 
 struct dmar_domain {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 7/9] iommu/vt-d: Calculate PTS value
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

Calculate PTS (PASID Table Size) value for the extended
context entry from the real size of the PASID table for
a domain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-iommu.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f86302d..5602ccd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5152,22 +5152,16 @@ static void intel_iommu_put_resv_regions(struct device *dev,
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 #define MAX_NR_PASID_BITS (20)
-static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
+static inline unsigned long intel_iommu_get_pts(struct dmar_domain *domain)
 {
-	/*
-	 * Convert ecap_pss to extend context entry pts encoding, also
-	 * respect the soft pasid_max value set by the iommu.
-	 * - number of PASID bits = ecap_pss + 1
-	 * - number of PASID table entries = 2^(pts + 5)
-	 * Therefore, pts = ecap_pss - 4
-	 * e.g. KBL ecap_pss = 0x13, PASID has 20 bits, pts = 15
-	 */
-	if (ecap_pss(iommu->ecap) < 5)
+	int pts;
+
+	pts = find_first_bit((unsigned long *)&domain->max_pasid,
+			     MAX_NR_PASID_BITS);
+	if (pts < 5)
 		return 0;
 
-	/* pasid_max is encoded as actual number of entries not the bits */
-	return find_first_bit((unsigned long *)&iommu->pasid_max,
-			MAX_NR_PASID_BITS) - 5;
+	return pts - 5;
 }
 
 int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev)
@@ -5204,7 +5198,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 		if (iommu->pasid_state_table)
 			context[1].hi = (u64)virt_to_phys(iommu->pasid_state_table);
 		context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
-			intel_iommu_get_pts(iommu);
+			intel_iommu_get_pts(domain);
 
 		wmb();
 		/* CONTEXT_TT_MULTI_LEVEL and CONTEXT_TT_DEV_IOTLB are both
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 7/9] iommu/vt-d: Calculate PTS value
@ 2018-05-04  1:41   ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w

Calculate PTS (PASID Table Size) value for the extended
context entry from the real size of the PASID table for
a domain.

Cc: Ashok Raj <ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: Kevin Tian <kevin.tian-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Cc: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Liu Yi L <yi.l.liu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iommu/intel-iommu.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f86302d..5602ccd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5152,22 +5152,16 @@ static void intel_iommu_put_resv_regions(struct device *dev,
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 #define MAX_NR_PASID_BITS (20)
-static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
+static inline unsigned long intel_iommu_get_pts(struct dmar_domain *domain)
 {
-	/*
-	 * Convert ecap_pss to extend context entry pts encoding, also
-	 * respect the soft pasid_max value set by the iommu.
-	 * - number of PASID bits = ecap_pss + 1
-	 * - number of PASID table entries = 2^(pts + 5)
-	 * Therefore, pts = ecap_pss - 4
-	 * e.g. KBL ecap_pss = 0x13, PASID has 20 bits, pts = 15
-	 */
-	if (ecap_pss(iommu->ecap) < 5)
+	int pts;
+
+	pts = find_first_bit((unsigned long *)&domain->max_pasid,
+			     MAX_NR_PASID_BITS);
+	if (pts < 5)
 		return 0;
 
-	/* pasid_max is encoded as actual number of entries not the bits */
-	return find_first_bit((unsigned long *)&iommu->pasid_max,
-			MAX_NR_PASID_BITS) - 5;
+	return pts - 5;
 }
 
 int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sdev)
@@ -5204,7 +5198,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 		if (iommu->pasid_state_table)
 			context[1].hi = (u64)virt_to_phys(iommu->pasid_state_table);
 		context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
-			intel_iommu_get_pts(iommu);
+			intel_iommu_get_pts(domain);
 
 		wmb();
 		/* CONTEXT_TT_MULTI_LEVEL and CONTEXT_TT_DEV_IOTLB are both
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 8/9] iommu/vt-d: Use per-domain pasid table
  2018-05-04  1:41 ` Lu Baolu
                   ` (7 preceding siblings ...)
  (?)
@ 2018-05-04  1:41 ` Lu Baolu
  -1 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

This patch replaces current per iommu pasid table with
the new added per domain pasid table. Each svm-capable
PCI device will be configured with a pasid table which
shares with other svm-capable devices within its iommu
domain.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-iommu.c |  6 +++---
 drivers/iommu/intel-svm.c   | 37 +++++++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5602ccd..fa6052a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5197,7 +5197,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev *sd
 	if (!(ctx_lo & CONTEXT_PASIDE)) {
 		if (iommu->pasid_state_table)
 			context[1].hi = (u64)virt_to_phys(iommu->pasid_state_table);
-		context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
+		context[1].lo = (u64)virt_to_phys(domain->pasid_table) |
 			intel_iommu_get_pts(domain);
 
 		wmb();
@@ -5265,8 +5265,8 @@ struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 		return NULL;
 	}
 
-	if (!iommu->pasid_table) {
-		dev_err(dev, "PASID not enabled on IOMMU; cannot enable SVM\n");
+	if (!intel_pasid_get_table(dev)) {
+		dev_err(dev, "No PASID table for device; cannot enable SVM\n");
 		return NULL;
 	}
 
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 3abc94f..3b14819 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -256,6 +256,7 @@ static void intel_flush_pasid_dev(struct intel_svm *svm, struct intel_svm_dev *s
 static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 {
 	struct intel_svm *svm = container_of(mn, struct intel_svm, notifier);
+	struct pasid_entry *pasid_table;
 	struct intel_svm_dev *sdev;
 
 	/* This might end up being called from exit_mmap(), *before* the page
@@ -270,11 +271,16 @@ static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 	 * page) so that we end up taking a fault that the hardware really
 	 * *has* to handle gracefully without affecting other processes.
 	 */
-	svm->iommu->pasid_table[svm->pasid].val = 0;
-	wmb();
-
 	rcu_read_lock();
 	list_for_each_entry_rcu(sdev, &svm->devs, list) {
+		pasid_table = intel_pasid_get_table(sdev->dev);
+		if (!pasid_table)
+			continue;
+
+		pasid_table[svm->pasid].val = 0;
+		/* Make sure the entry update is visible before translation. */
+		wmb();
+
 		intel_flush_pasid_dev(svm, sdev, svm->pasid);
 		intel_flush_svm_range_dev(svm, sdev, 0, -1, 0, !svm->mm);
 	}
@@ -295,6 +301,7 @@ static LIST_HEAD(global_svm_list);
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_ops *ops)
 {
 	struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
+	struct pasid_entry *pasid_table;
 	struct intel_svm_dev *sdev;
 	struct intel_svm *svm = NULL;
 	struct mm_struct *mm = NULL;
@@ -302,7 +309,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 	int pasid_max;
 	int ret;
 
-	if (WARN_ON(!iommu || !iommu->pasid_table))
+	pasid_table = intel_pasid_get_table(dev);
+	if (WARN_ON(!iommu || !pasid_table))
 		return -EINVAL;
 
 	if (dev_is_pci(dev)) {
@@ -380,8 +388,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		}
 		svm->iommu = iommu;
 
-		if (pasid_max > iommu->pasid_max)
-			pasid_max = iommu->pasid_max;
+		if (pasid_max > intel_pasid_max_id)
+			pasid_max = intel_pasid_max_id;
 
 		/* Do not use PASID 0 in caching mode (virtualised IOMMU) */
 		ret = intel_pasid_alloc_id(svm,
@@ -414,7 +422,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		if (cpu_feature_enabled(X86_FEATURE_LA57))
 			pasid_entry_val |= PASID_ENTRY_FLPM_5LP;
 
-		iommu->pasid_table[svm->pasid].val = pasid_entry_val;
+		pasid_table[svm->pasid].val = pasid_entry_val;
 
 		wmb();
 
@@ -442,6 +450,7 @@ EXPORT_SYMBOL_GPL(intel_svm_bind_mm);
 
 int intel_svm_unbind_mm(struct device *dev, int pasid)
 {
+	struct pasid_entry *pasid_table;
 	struct intel_svm_dev *sdev;
 	struct intel_iommu *iommu;
 	struct intel_svm *svm;
@@ -449,7 +458,8 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 
 	mutex_lock(&pasid_mutex);
 	iommu = intel_svm_device_to_iommu(dev);
-	if (!iommu || !iommu->pasid_table)
+	pasid_table = intel_pasid_get_table(dev);
+	if (!iommu || !pasid_table)
 		goto out;
 
 	svm = intel_pasid_lookup_id(pasid);
@@ -472,11 +482,14 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 				intel_flush_pasid_dev(svm, sdev, svm->pasid);
 				intel_flush_svm_range_dev(svm, sdev, 0, -1, 0, !svm->mm);
 				kfree_rcu(sdev, rcu);
+				pasid_table[svm->pasid].val = 0;
+				/*
+				 * Make sure the entry update is visible
+				 * before translation.
+				 */
+				wmb();
 
 				if (list_empty(&svm->devs)) {
-					svm->iommu->pasid_table[svm->pasid].val = 0;
-					wmb();
-
 					intel_pasid_free_id(svm->pasid);
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
@@ -509,7 +522,7 @@ int intel_svm_is_pasid_valid(struct device *dev, int pasid)
 
 	mutex_lock(&pasid_mutex);
 	iommu = intel_svm_device_to_iommu(dev);
-	if (!iommu || !iommu->pasid_table)
+	if (!iommu)
 		goto out;
 
 	svm = intel_pasid_lookup_id(pasid);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v2 9/9] iommu/vt-d: Clean up PASID talbe management for SVM
  2018-05-04  1:41 ` Lu Baolu
                   ` (8 preceding siblings ...)
  (?)
@ 2018-05-04  1:41 ` Lu Baolu
  -1 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-04  1:41 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse
  Cc: ashok.raj, sanjay.k.kumar, jacob.jun.pan, kevin.tian, yi.l.liu,
	yi.y.sun, iommu, linux-kernel, Lu Baolu, Jacob Pan

The previous per iommu pasid table alloc/free interfaces
are no longer used. Clean up the driver by removing it.

Cc: Ashok Raj <ashok.raj@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Liu Yi L <yi.l.liu@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Liu Yi L <yi.l.liu@intel.com>
---
 drivers/iommu/intel-iommu.c |  6 +++---
 drivers/iommu/intel-svm.c   | 17 ++---------------
 include/linux/intel-iommu.h |  5 ++---
 3 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index fa6052a..010871d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1736,7 +1736,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
 	if (pasid_enabled(iommu)) {
 		if (ecap_prs(iommu->ecap))
 			intel_svm_finish_prq(iommu);
-		intel_svm_free_pasid_tables(iommu);
+		intel_svm_exit(iommu);
 	}
 #endif
 }
@@ -3297,7 +3297,7 @@ static int __init init_dmars(void)
 			hw_pass_through = 0;
 #ifdef CONFIG_INTEL_IOMMU_SVM
 		if (pasid_enabled(iommu))
-			intel_svm_alloc_pasid_tables(iommu);
+			intel_svm_init(iommu);
 #endif
 	}
 
@@ -4274,7 +4274,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 	if (pasid_enabled(iommu))
-		intel_svm_alloc_pasid_tables(iommu);
+		intel_svm_init(iommu);
 #endif
 
 	if (dmaru->ignored) {
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 3b14819..38cae65 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -34,7 +34,7 @@
 
 static irqreturn_t prq_event_thread(int irq, void *d);
 
-int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_init(struct intel_iommu *iommu)
 {
 	struct page *pages;
 	int order;
@@ -59,15 +59,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 		iommu->pasid_max = 0x20000;
 
 	order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
-	pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
-	if (!pages) {
-		pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
-			iommu->name);
-		return -ENOMEM;
-	}
-	iommu->pasid_table = page_address(pages);
-	pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
-
 	if (ecap_dis(iommu->ecap)) {
 		/* Just making it explicit... */
 		BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct pasid_state_entry));
@@ -82,14 +73,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 	return 0;
 }
 
-int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_exit(struct intel_iommu *iommu)
 {
 	int order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
 
-	if (iommu->pasid_table) {
-		free_pages((unsigned long)iommu->pasid_table, order);
-		iommu->pasid_table = NULL;
-	}
 	if (iommu->pasid_state_table) {
 		free_pages((unsigned long)iommu->pasid_state_table, order);
 		iommu->pasid_state_table = NULL;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 08e5811..44c7613 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -470,7 +470,6 @@ struct intel_iommu {
 	 * devices away to userspace processes (e.g. for DPDK) and don't
 	 * want to trust that userspace will use *only* the PASID it was
 	 * told to. But while it's all driver-arbitrated, we're fine. */
-	struct pasid_entry *pasid_table;
 	struct pasid_state_entry *pasid_state_table;
 	struct page_req_dsc *prq;
 	unsigned char prq_name[16];    /* Name for PRQ interrupt */
@@ -539,8 +538,8 @@ void free_pgtable_page(void *vaddr);
 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
-extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
-extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
+int intel_svm_init(struct intel_iommu *iommu);
+int intel_svm_exit(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
 extern int intel_svm_finish_prq(struct intel_iommu *iommu);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-15 14:11   ` Joerg Roedel
  0 siblings, 0 replies; 26+ messages in thread
From: Joerg Roedel @ 2018-05-15 14:11 UTC (permalink / raw)
  To: Lu Baolu
  Cc: David Woodhouse, ashok.raj, sanjay.k.kumar, jacob.jun.pan,
	kevin.tian, yi.l.liu, yi.y.sun, iommu, linux-kernel

On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
> PATCH 4~9 implement per domain PASID table. Current per IOMMU
> PASID table implementation is insecure in the cases where
> multiple devices under one single IOMMU unit support PASID
> feature. With per domain PASID table, we can achieve finer
> protection and isolation granularity.


Hold on, we hat discussions in the past about doing a system-wide pasid
space, so that every mm_struct with devices attached gets the same pasid
across all devices it is talking to. Reason was that some devices (will)
require this to work correctly. This goes into the opposite direction,
so I am a bit confused here. Please explain, is this not longer
necessary?


Regards,

	Joerg

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-15 14:11   ` Joerg Roedel
  0 siblings, 0 replies; 26+ messages in thread
From: Joerg Roedel @ 2018-05-15 14:11 UTC (permalink / raw)
  To: Lu Baolu
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w, David Woodhouse

On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
> PATCH 4~9 implement per domain PASID table. Current per IOMMU
> PASID table implementation is insecure in the cases where
> multiple devices under one single IOMMU unit support PASID
> feature. With per domain PASID table, we can achieve finer
> protection and isolation granularity.


Hold on, we hat discussions in the past about doing a system-wide pasid
space, so that every mm_struct with devices attached gets the same pasid
across all devices it is talking to. Reason was that some devices (will)
require this to work correctly. This goes into the opposite direction,
so I am a bit confused here. Please explain, is this not longer
necessary?


Regards,

	Joerg

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-16  8:01     ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-16  8:01 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: David Woodhouse, ashok.raj, sanjay.k.kumar, jacob.jun.pan,
	kevin.tian, yi.l.liu, yi.y.sun, iommu, linux-kernel

Hi Joerg,

Thank you for looking at my patches.

On 05/15/2018 10:11 PM, Joerg Roedel wrote:
> On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
>> PATCH 4~9 implement per domain PASID table. Current per IOMMU
>> PASID table implementation is insecure in the cases where
>> multiple devices under one single IOMMU unit support PASID
>> feature. With per domain PASID table, we can achieve finer
>> protection and isolation granularity.
>
> Hold on, we hat discussions in the past about doing a system-wide pasid
> space, so that every mm_struct with devices attached gets the same pasid
> across all devices it is talking to. Reason was that some devices (will)
> require this to work correctly. This goes into the opposite direction,
> so I am a bit confused here. Please explain, is this not longer
> necessary?

You are right. System-wide pasid space is necessary, hence PATCH
1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
it's designed to address another potential issue.

With system-wide pasid space, we can use a system-wide pasid table,
or just keep what we have now(per iommu unit pasid table). Both
system-wide and per iommu unitpasid table mean that two devices
might share a single pasid table. That will result in an issue.

For an example, device A is assigned to access the memory space of
process A, and device B is assigned to access the memory space of
process B. The dma remapping infrastructure looks like:

                                                     .------------------.
                             .----------------.      |                  |
                             |                |      |                  |
                             .----------------.      | Paging structure |
                             |     PASID X    |--|   |  for Process A   |
                             .----------------.  |   |                  |
                             |                |  --->'------------------'
     .----------------.      .----------------.
     |                |      |     PASID Y    |--|
     .----------------.      .----------------.  |
     | Dev_A context  |---|  |                |  |   .------------------.
     '----------------'   |  .----------------.  |   |                  |
     |                |   |  |                |  |   |                  |
     '----------------'   |  .----------------.  |   | Paging structure |
     | Dev_B context  |   -->|                |  |   |  for Process B   |
     '----------------'----->'----------------'  |   |                  |
     |                |         system-wide      v-->'------------------'
     .----------------.         pasid table    
     |                |                        
     '----------------'
        Intel iommu
       context table


Since dev_A and dev_B share a pasid table, the side effect is that a flawed
dev_A might access the memory space of process B (with pasid y). Vice versa,
a flawed dev_B might access memory space of process A (with pasid x).

What PATCH 4~9 do is to remove such possibility by assigning a pasid table
for each pci device. Hence, the remapping infrastructure looks like:


                                                    .------------------.
                                                    |                  |
                            .----------------.      |                  |
                            |                |      | Paging structure |
                            .----------------.      |  for Process A   |
                            |     PASID X    |      |                  |
                            .----------------.----->'------------------'
                            |                |
                            .----------------.
                            |                |
                            .----------------.
                            |                |
                            .----------------.
   .----------------.       |                |
   |                |       .----------------.
   .----------------.       |                |
   | Dev_A context  |------>'----------------'
   '----------------'          pasid table 
   |                |          for Dev_A   
   '----------------'                      
   | Dev_B context  |-->
   '----------------'  |    .----------------.
   |                |  |    |                |      .------------------.
   .----------------.  |    .----------------.      |                  |
   |                |  |    |                |      |                  |
   '----------------'  |    .----------------.      | Paging structure |
       Intel iommu     |    |                |      |  for Process B   |
      context table    |    .----------------.      |                  |
                       |    |     PASID Y    |----->'------------------'
                       |    .----------------.
                       |    |                |
                       |    .----------------.
                       |    |                |
                       |    .----------------.
                       v--->|                |
                            '----------------'
                               pasid table 
                               for Dev_B   
                                           

With this, dev_A has no means to access memory of process B and vice versa.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-16  8:01     ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-16  8:01 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: ashok.raj-ral2JQCrhuEAvxtiuMwx3w,
	sanjay.k.kumar-ral2JQCrhuEAvxtiuMwx3w,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	yi.y.sun-ral2JQCrhuEAvxtiuMwx3w,
	jacob.jun.pan-ral2JQCrhuEAvxtiuMwx3w, David Woodhouse

Hi Joerg,

Thank you for looking at my patches.

On 05/15/2018 10:11 PM, Joerg Roedel wrote:
> On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
>> PATCH 4~9 implement per domain PASID table. Current per IOMMU
>> PASID table implementation is insecure in the cases where
>> multiple devices under one single IOMMU unit support PASID
>> feature. With per domain PASID table, we can achieve finer
>> protection and isolation granularity.
>
> Hold on, we hat discussions in the past about doing a system-wide pasid
> space, so that every mm_struct with devices attached gets the same pasid
> across all devices it is talking to. Reason was that some devices (will)
> require this to work correctly. This goes into the opposite direction,
> so I am a bit confused here. Please explain, is this not longer
> necessary?

You are right. System-wide pasid space is necessary, hence PATCH
1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
it's designed to address another potential issue.

With system-wide pasid space, we can use a system-wide pasid table,
or just keep what we have now(per iommu unit pasid table). Both
system-wide and per iommu unitpasid table mean that two devices
might share a single pasid table. That will result in an issue.

For an example, device A is assigned to access the memory space of
process A, and device B is assigned to access the memory space of
process B. The dma remapping infrastructure looks like:

                                                     .------------------.
                             .----------------.      |                  |
                             |                |      |                  |
                             .----------------.      | Paging structure |
                             |     PASID X    |--|   |  for Process A   |
                             .----------------.  |   |                  |
                             |                |  --->'------------------'
     .----------------.      .----------------.
     |                |      |     PASID Y    |--|
     .----------------.      .----------------.  |
     | Dev_A context  |---|  |                |  |   .------------------.
     '----------------'   |  .----------------.  |   |                  |
     |                |   |  |                |  |   |                  |
     '----------------'   |  .----------------.  |   | Paging structure |
     | Dev_B context  |   -->|                |  |   |  for Process B   |
     '----------------'----->'----------------'  |   |                  |
     |                |         system-wide      v-->'------------------'
     .----------------.         pasid table    
     |                |                        
     '----------------'
        Intel iommu
       context table


Since dev_A and dev_B share a pasid table, the side effect is that a flawed
dev_A might access the memory space of process B (with pasid y). Vice versa,
a flawed dev_B might access memory space of process A (with pasid x).

What PATCH 4~9 do is to remove such possibility by assigning a pasid table
for each pci device. Hence, the remapping infrastructure looks like:


                                                    .------------------.
                                                    |                  |
                            .----------------.      |                  |
                            |                |      | Paging structure |
                            .----------------.      |  for Process A   |
                            |     PASID X    |      |                  |
                            .----------------.----->'------------------'
                            |                |
                            .----------------.
                            |                |
                            .----------------.
                            |                |
                            .----------------.
   .----------------.       |                |
   |                |       .----------------.
   .----------------.       |                |
   | Dev_A context  |------>'----------------'
   '----------------'          pasid table 
   |                |          for Dev_A   
   '----------------'                      
   | Dev_B context  |-->
   '----------------'  |    .----------------.
   |                |  |    |                |      .------------------.
   .----------------.  |    .----------------.      |                  |
   |                |  |    |                |      |                  |
   '----------------'  |    .----------------.      | Paging structure |
       Intel iommu     |    |                |      |  for Process B   |
      context table    |    .----------------.      |                  |
                       |    |     PASID Y    |----->'------------------'
                       |    .----------------.
                       |    |                |
                       |    .----------------.
                       |    |                |
                       |    .----------------.
                       v--->|                |
                            '----------------'
                               pasid table 
                               for Dev_B   
                                           

With this, dev_A has no means to access memory of process B and vice versa.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-16  8:56       ` Tian, Kevin
  0 siblings, 0 replies; 26+ messages in thread
From: Tian, Kevin @ 2018-05-16  8:56 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel
  Cc: David Woodhouse, Raj, Ashok, Kumar, Sanjay K, Pan, Jacob jun,
	Liu, Yi L, Sun, Yi Y, iommu, linux-kernel

> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
> Sent: Wednesday, May 16, 2018 4:01 PM
> 
> Hi Joerg,
> 
> Thank you for looking at my patches.
> 
> On 05/15/2018 10:11 PM, Joerg Roedel wrote:
> > On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
> >> PATCH 4~9 implement per domain PASID table. Current per IOMMU
> >> PASID table implementation is insecure in the cases where
> >> multiple devices under one single IOMMU unit support PASID
> >> feature. With per domain PASID table, we can achieve finer
> >> protection and isolation granularity.
> >
> > Hold on, we hat discussions in the past about doing a system-wide pasid
> > space, so that every mm_struct with devices attached gets the same pasid
> > across all devices it is talking to. Reason was that some devices (will)
> > require this to work correctly. This goes into the opposite direction,
> > so I am a bit confused here. Please explain, is this not longer
> > necessary?
> 
> You are right. System-wide pasid space is necessary, hence PATCH
> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
> it's designed to address another potential issue.

one thing you may want to highlight is, even with PATCH 4-9 it's
still doing system-wide PASID space allocation. Just PASID table
itself is kept per-device for isolation purpose as you described
below, i.e. each device can access only those PASIDs which are
allocated to itself while the allocation happens system-wide...

> 
> With system-wide pasid space, we can use a system-wide pasid table,
> or just keep what we have now(per iommu unit pasid table). Both
> system-wide and per iommu unitpasid table mean that two devices
> might share a single pasid table. That will result in an issue.
> 
> For an example, device A is assigned to access the memory space of
> process A, and device B is assigned to access the memory space of
> process B. The dma remapping infrastructure looks like:
> 
>                                                      .------------------.
>                              .----------------.      |                  |
>                              |                |      |                  |
>                              .----------------.      | Paging structure |
>                              |     PASID X    |--|   |  for Process A   |
>                              .----------------.  |   |                  |
>                              |                |  --->'------------------'
>      .----------------.      .----------------.
>      |                |      |     PASID Y    |--|
>      .----------------.      .----------------.  |
>      | Dev_A context  |---|  |                |  |   .------------------.
>      '----------------'   |  .----------------.  |   |                  |
>      |                |   |  |                |  |   |                  |
>      '----------------'   |  .----------------.  |   | Paging structure |
>      | Dev_B context  |   -->|                |  |   |  for Process B   |
>      '----------------'----->'----------------'  |   |                  |
>      |                |         system-wide      v-->'------------------'
>      .----------------.         pasid table
>      |                |
>      '----------------'
>         Intel iommu
>        context table
> 
> 
> Since dev_A and dev_B share a pasid table, the side effect is that a flawed
> dev_A might access the memory space of process B (with pasid y). Vice
> versa,
> a flawed dev_B might access memory space of process A (with pasid x).
> 
> What PATCH 4~9 do is to remove such possibility by assigning a pasid table
> for each pci device. Hence, the remapping infrastructure looks like:
> 
> 
>                                                     .------------------.
>                                                     |                  |
>                             .----------------.      |                  |
>                             |                |      | Paging structure |
>                             .----------------.      |  for Process A   |
>                             |     PASID X    |      |                  |
>                             .----------------.----->'------------------'
>                             |                |
>                             .----------------.
>                             |                |
>                             .----------------.
>                             |                |
>                             .----------------.
>    .----------------.       |                |
>    |                |       .----------------.
>    .----------------.       |                |
>    | Dev_A context  |------>'----------------'
>    '----------------'          pasid table
>    |                |          for Dev_A
>    '----------------'
>    | Dev_B context  |-->
>    '----------------'  |    .----------------.
>    |                |  |    |                |      .------------------.
>    .----------------.  |    .----------------.      |                  |
>    |                |  |    |                |      |                  |
>    '----------------'  |    .----------------.      | Paging structure |
>        Intel iommu     |    |                |      |  for Process B   |
>       context table    |    .----------------.      |                  |
>                        |    |     PASID Y    |----->'------------------'
>                        |    .----------------.
>                        |    |                |
>                        |    .----------------.
>                        |    |                |
>                        |    .----------------.
>                        v--->|                |
>                             '----------------'
>                                pasid table
>                                for Dev_B
> 
> 
> With this, dev_A has no means to access memory of process B and vice
> versa.
> 
> Best regards,
> Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* RE: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-16  8:56       ` Tian, Kevin
  0 siblings, 0 replies; 26+ messages in thread
From: Tian, Kevin @ 2018-05-16  8:56 UTC (permalink / raw)
  To: Lu Baolu, Joerg Roedel
  Cc: Raj, Ashok, Kumar, Sanjay K,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sun, Yi Y, Pan, Jacob jun,
	David Woodhouse

> From: Lu Baolu [mailto:baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org]
> Sent: Wednesday, May 16, 2018 4:01 PM
> 
> Hi Joerg,
> 
> Thank you for looking at my patches.
> 
> On 05/15/2018 10:11 PM, Joerg Roedel wrote:
> > On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
> >> PATCH 4~9 implement per domain PASID table. Current per IOMMU
> >> PASID table implementation is insecure in the cases where
> >> multiple devices under one single IOMMU unit support PASID
> >> feature. With per domain PASID table, we can achieve finer
> >> protection and isolation granularity.
> >
> > Hold on, we hat discussions in the past about doing a system-wide pasid
> > space, so that every mm_struct with devices attached gets the same pasid
> > across all devices it is talking to. Reason was that some devices (will)
> > require this to work correctly. This goes into the opposite direction,
> > so I am a bit confused here. Please explain, is this not longer
> > necessary?
> 
> You are right. System-wide pasid space is necessary, hence PATCH
> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
> it's designed to address another potential issue.

one thing you may want to highlight is, even with PATCH 4-9 it's
still doing system-wide PASID space allocation. Just PASID table
itself is kept per-device for isolation purpose as you described
below, i.e. each device can access only those PASIDs which are
allocated to itself while the allocation happens system-wide...

> 
> With system-wide pasid space, we can use a system-wide pasid table,
> or just keep what we have now(per iommu unit pasid table). Both
> system-wide and per iommu unitpasid table mean that two devices
> might share a single pasid table. That will result in an issue.
> 
> For an example, device A is assigned to access the memory space of
> process A, and device B is assigned to access the memory space of
> process B. The dma remapping infrastructure looks like:
> 
>                                                      .------------------.
>                              .----------------.      |                  |
>                              |                |      |                  |
>                              .----------------.      | Paging structure |
>                              |     PASID X    |--|   |  for Process A   |
>                              .----------------.  |   |                  |
>                              |                |  --->'------------------'
>      .----------------.      .----------------.
>      |                |      |     PASID Y    |--|
>      .----------------.      .----------------.  |
>      | Dev_A context  |---|  |                |  |   .------------------.
>      '----------------'   |  .----------------.  |   |                  |
>      |                |   |  |                |  |   |                  |
>      '----------------'   |  .----------------.  |   | Paging structure |
>      | Dev_B context  |   -->|                |  |   |  for Process B   |
>      '----------------'----->'----------------'  |   |                  |
>      |                |         system-wide      v-->'------------------'
>      .----------------.         pasid table
>      |                |
>      '----------------'
>         Intel iommu
>        context table
> 
> 
> Since dev_A and dev_B share a pasid table, the side effect is that a flawed
> dev_A might access the memory space of process B (with pasid y). Vice
> versa,
> a flawed dev_B might access memory space of process A (with pasid x).
> 
> What PATCH 4~9 do is to remove such possibility by assigning a pasid table
> for each pci device. Hence, the remapping infrastructure looks like:
> 
> 
>                                                     .------------------.
>                                                     |                  |
>                             .----------------.      |                  |
>                             |                |      | Paging structure |
>                             .----------------.      |  for Process A   |
>                             |     PASID X    |      |                  |
>                             .----------------.----->'------------------'
>                             |                |
>                             .----------------.
>                             |                |
>                             .----------------.
>                             |                |
>                             .----------------.
>    .----------------.       |                |
>    |                |       .----------------.
>    .----------------.       |                |
>    | Dev_A context  |------>'----------------'
>    '----------------'          pasid table
>    |                |          for Dev_A
>    '----------------'
>    | Dev_B context  |-->
>    '----------------'  |    .----------------.
>    |                |  |    |                |      .------------------.
>    .----------------.  |    .----------------.      |                  |
>    |                |  |    |                |      |                  |
>    '----------------'  |    .----------------.      | Paging structure |
>        Intel iommu     |    |                |      |  for Process B   |
>       context table    |    .----------------.      |                  |
>                        |    |     PASID Y    |----->'------------------'
>                        |    .----------------.
>                        |    |                |
>                        |    .----------------.
>                        |    |                |
>                        |    .----------------.
>                        v--->|                |
>                             '----------------'
>                                pasid table
>                                for Dev_B
> 
> 
> With this, dev_A has no means to access memory of process B and vice
> versa.
> 
> Best regards,
> Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-17  1:13         ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-17  1:13 UTC (permalink / raw)
  To: Tian, Kevin, Joerg Roedel
  Cc: David Woodhouse, Raj, Ashok, Kumar, Sanjay K, Pan, Jacob jun,
	Liu, Yi L, Sun, Yi Y, iommu, linux-kernel

Hi,

On 05/16/2018 04:56 PM, Tian, Kevin wrote:
>> From: Lu Baolu [mailto:baolu.lu@linux.intel.com]
>> Sent: Wednesday, May 16, 2018 4:01 PM
>>
>> Hi Joerg,
>>
>> Thank you for looking at my patches.
>>
>> On 05/15/2018 10:11 PM, Joerg Roedel wrote:
>>> On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
>>>> PATCH 4~9 implement per domain PASID table. Current per IOMMU
>>>> PASID table implementation is insecure in the cases where
>>>> multiple devices under one single IOMMU unit support PASID
>>>> feature. With per domain PASID table, we can achieve finer
>>>> protection and isolation granularity.
>>> Hold on, we hat discussions in the past about doing a system-wide pasid
>>> space, so that every mm_struct with devices attached gets the same pasid
>>> across all devices it is talking to. Reason was that some devices (will)
>>> require this to work correctly. This goes into the opposite direction,
>>> so I am a bit confused here. Please explain, is this not longer
>>> necessary?
>> You are right. System-wide pasid space is necessary, hence PATCH
>> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
>> it's designed to address another potential issue.
> one thing you may want to highlight is, even with PATCH 4-9 it's
> still doing system-wide PASID space allocation. Just PASID table
> itself is kept per-device for isolation purpose as you described
> below, i.e. each device can access only those PASIDs which are
> allocated to itself while the allocation happens system-wide...

Yes, exactly.

Best regards,
Lu Baolu

>
>> With system-wide pasid space, we can use a system-wide pasid table,
>> or just keep what we have now(per iommu unit pasid table). Both
>> system-wide and per iommu unitpasid table mean that two devices
>> might share a single pasid table. That will result in an issue.
>>
>> For an example, device A is assigned to access the memory space of
>> process A, and device B is assigned to access the memory space of
>> process B. The dma remapping infrastructure looks like:
>>
>>                                                      .------------------.
>>                              .----------------.      |                  |
>>                              |                |      |                  |
>>                              .----------------.      | Paging structure |
>>                              |     PASID X    |--|   |  for Process A   |
>>                              .----------------.  |   |                  |
>>                              |                |  --->'------------------'
>>      .----------------.      .----------------.
>>      |                |      |     PASID Y    |--|
>>      .----------------.      .----------------.  |
>>      | Dev_A context  |---|  |                |  |   .------------------.
>>      '----------------'   |  .----------------.  |   |                  |
>>      |                |   |  |                |  |   |                  |
>>      '----------------'   |  .----------------.  |   | Paging structure |
>>      | Dev_B context  |   -->|                |  |   |  for Process B   |
>>      '----------------'----->'----------------'  |   |                  |
>>      |                |         system-wide      v-->'------------------'
>>      .----------------.         pasid table
>>      |                |
>>      '----------------'
>>         Intel iommu
>>        context table
>>
>>
>> Since dev_A and dev_B share a pasid table, the side effect is that a flawed
>> dev_A might access the memory space of process B (with pasid y). Vice
>> versa,
>> a flawed dev_B might access memory space of process A (with pasid x).
>>
>> What PATCH 4~9 do is to remove such possibility by assigning a pasid table
>> for each pci device. Hence, the remapping infrastructure looks like:
>>
>>
>>                                                     .------------------.
>>                                                     |                  |
>>                             .----------------.      |                  |
>>                             |                |      | Paging structure |
>>                             .----------------.      |  for Process A   |
>>                             |     PASID X    |      |                  |
>>                             .----------------.----->'------------------'
>>                             |                |
>>                             .----------------.
>>                             |                |
>>                             .----------------.
>>                             |                |
>>                             .----------------.
>>    .----------------.       |                |
>>    |                |       .----------------.
>>    .----------------.       |                |
>>    | Dev_A context  |------>'----------------'
>>    '----------------'          pasid table
>>    |                |          for Dev_A
>>    '----------------'
>>    | Dev_B context  |-->
>>    '----------------'  |    .----------------.
>>    |                |  |    |                |      .------------------.
>>    .----------------.  |    .----------------.      |                  |
>>    |                |  |    |                |      |                  |
>>    '----------------'  |    .----------------.      | Paging structure |
>>        Intel iommu     |    |                |      |  for Process B   |
>>       context table    |    .----------------.      |                  |
>>                        |    |     PASID Y    |----->'------------------'
>>                        |    .----------------.
>>                        |    |                |
>>                        |    .----------------.
>>                        |    |                |
>>                        |    .----------------.
>>                        v--->|                |
>>                             '----------------'
>>                                pasid table
>>                                for Dev_B
>>
>>
>> With this, dev_A has no means to access memory of process B and vice
>> versa.
>>
>> Best regards,
>> Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
@ 2018-05-17  1:13         ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-17  1:13 UTC (permalink / raw)
  To: Tian, Kevin, Joerg Roedel
  Cc: Raj, Ashok, Kumar, Sanjay K,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sun, Yi Y, Pan, Jacob jun,
	David Woodhouse

Hi,

On 05/16/2018 04:56 PM, Tian, Kevin wrote:
>> From: Lu Baolu [mailto:baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org]
>> Sent: Wednesday, May 16, 2018 4:01 PM
>>
>> Hi Joerg,
>>
>> Thank you for looking at my patches.
>>
>> On 05/15/2018 10:11 PM, Joerg Roedel wrote:
>>> On Fri, May 04, 2018 at 09:41:15AM +0800, Lu Baolu wrote:
>>>> PATCH 4~9 implement per domain PASID table. Current per IOMMU
>>>> PASID table implementation is insecure in the cases where
>>>> multiple devices under one single IOMMU unit support PASID
>>>> feature. With per domain PASID table, we can achieve finer
>>>> protection and isolation granularity.
>>> Hold on, we hat discussions in the past about doing a system-wide pasid
>>> space, so that every mm_struct with devices attached gets the same pasid
>>> across all devices it is talking to. Reason was that some devices (will)
>>> require this to work correctly. This goes into the opposite direction,
>>> so I am a bit confused here. Please explain, is this not longer
>>> necessary?
>> You are right. System-wide pasid space is necessary, hence PATCH
>> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
>> it's designed to address another potential issue.
> one thing you may want to highlight is, even with PATCH 4-9 it's
> still doing system-wide PASID space allocation. Just PASID table
> itself is kept per-device for isolation purpose as you described
> below, i.e. each device can access only those PASIDs which are
> allocated to itself while the allocation happens system-wide...

Yes, exactly.

Best regards,
Lu Baolu

>
>> With system-wide pasid space, we can use a system-wide pasid table,
>> or just keep what we have now(per iommu unit pasid table). Both
>> system-wide and per iommu unitpasid table mean that two devices
>> might share a single pasid table. That will result in an issue.
>>
>> For an example, device A is assigned to access the memory space of
>> process A, and device B is assigned to access the memory space of
>> process B. The dma remapping infrastructure looks like:
>>
>>                                                      .------------------.
>>                              .----------------.      |                  |
>>                              |                |      |                  |
>>                              .----------------.      | Paging structure |
>>                              |     PASID X    |--|   |  for Process A   |
>>                              .----------------.  |   |                  |
>>                              |                |  --->'------------------'
>>      .----------------.      .----------------.
>>      |                |      |     PASID Y    |--|
>>      .----------------.      .----------------.  |
>>      | Dev_A context  |---|  |                |  |   .------------------.
>>      '----------------'   |  .----------------.  |   |                  |
>>      |                |   |  |                |  |   |                  |
>>      '----------------'   |  .----------------.  |   | Paging structure |
>>      | Dev_B context  |   -->|                |  |   |  for Process B   |
>>      '----------------'----->'----------------'  |   |                  |
>>      |                |         system-wide      v-->'------------------'
>>      .----------------.         pasid table
>>      |                |
>>      '----------------'
>>         Intel iommu
>>        context table
>>
>>
>> Since dev_A and dev_B share a pasid table, the side effect is that a flawed
>> dev_A might access the memory space of process B (with pasid y). Vice
>> versa,
>> a flawed dev_B might access memory space of process A (with pasid x).
>>
>> What PATCH 4~9 do is to remove such possibility by assigning a pasid table
>> for each pci device. Hence, the remapping infrastructure looks like:
>>
>>
>>                                                     .------------------.
>>                                                     |                  |
>>                             .----------------.      |                  |
>>                             |                |      | Paging structure |
>>                             .----------------.      |  for Process A   |
>>                             |     PASID X    |      |                  |
>>                             .----------------.----->'------------------'
>>                             |                |
>>                             .----------------.
>>                             |                |
>>                             .----------------.
>>                             |                |
>>                             .----------------.
>>    .----------------.       |                |
>>    |                |       .----------------.
>>    .----------------.       |                |
>>    | Dev_A context  |------>'----------------'
>>    '----------------'          pasid table
>>    |                |          for Dev_A
>>    '----------------'
>>    | Dev_B context  |-->
>>    '----------------'  |    .----------------.
>>    |                |  |    |                |      .------------------.
>>    .----------------.  |    .----------------.      |                  |
>>    |                |  |    |                |      |                  |
>>    '----------------'  |    .----------------.      | Paging structure |
>>        Intel iommu     |    |                |      |  for Process B   |
>>       context table    |    .----------------.      |                  |
>>                        |    |     PASID Y    |----->'------------------'
>>                        |    .----------------.
>>                        |    |                |
>>                        |    .----------------.
>>                        |    |                |
>>                        |    .----------------.
>>                        v--->|                |
>>                             '----------------'
>>                                pasid table
>>                                for Dev_B
>>
>>
>> With this, dev_A has no means to access memory of process B and vice
>> versa.
>>
>> Best regards,
>> Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
  2018-05-16  8:01     ` Lu Baolu
  (?)
  (?)
@ 2018-05-29 11:56     ` Joerg Roedel
  2018-05-30  0:56       ` Lu Baolu
  -1 siblings, 1 reply; 26+ messages in thread
From: Joerg Roedel @ 2018-05-29 11:56 UTC (permalink / raw)
  To: Lu Baolu
  Cc: David Woodhouse, ashok.raj, sanjay.k.kumar, jacob.jun.pan,
	kevin.tian, yi.l.liu, yi.y.sun, iommu, linux-kernel

On Wed, May 16, 2018 at 04:01:00PM +0800, Lu Baolu wrote:
> You are right. System-wide pasid space is necessary, hence PATCH
> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
> it's designed to address another potential issue.

Okay, thanks for the clarification. Please send me a rebased version of
this after the merge window I will review again.


Thanks,

	Joerg

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management
  2018-05-29 11:56     ` Joerg Roedel
@ 2018-05-30  0:56       ` Lu Baolu
  0 siblings, 0 replies; 26+ messages in thread
From: Lu Baolu @ 2018-05-30  0:56 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: David Woodhouse, ashok.raj, sanjay.k.kumar, jacob.jun.pan,
	kevin.tian, yi.l.liu, yi.y.sun, iommu, linux-kernel

Hi,

On 05/29/2018 07:56 PM, Joerg Roedel wrote:
> On Wed, May 16, 2018 at 04:01:00PM +0800, Lu Baolu wrote:
>> You are right. System-wide pasid space is necessary, hence PATCH
>> 1~3 implement it. But PATCH 4~9 don't go into the opposite direction,
>> it's designed to address another potential issue.
> Okay, thanks for the clarification. Please send me a rebased version of
> this after the merge window I will review again.

Sure.

Best regards,
Lu Baolu

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-05-30  0:56 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04  1:41 [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management Lu Baolu
2018-05-04  1:41 ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 1/9] iommu/vt-d: Global PASID name space Lu Baolu
2018-05-04  1:41   ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 2/9] iommu/vt-d: Decouple idr bond pointer from svm Lu Baolu
2018-05-04  1:41   ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 3/9] iommu/vt-d: Use global PASID for SVM usage Lu Baolu
2018-05-04  1:41 ` [PATCH v2 4/9] iommu/vt-d: Move device_domain_info to header Lu Baolu
2018-05-04  1:41   ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 5/9] iommu/vt-d: Per domain pasid table interfaces Lu Baolu
2018-05-04  1:41   ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 6/9] iommu/vt-d: Allocate and free pasid table Lu Baolu
2018-05-04  1:41 ` [PATCH v2 7/9] iommu/vt-d: Calculate PTS value Lu Baolu
2018-05-04  1:41   ` Lu Baolu
2018-05-04  1:41 ` [PATCH v2 8/9] iommu/vt-d: Use per-domain pasid table Lu Baolu
2018-05-04  1:41 ` [PATCH v2 9/9] iommu/vt-d: Clean up PASID talbe management for SVM Lu Baolu
2018-05-15 14:11 ` [PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management Joerg Roedel
2018-05-15 14:11   ` Joerg Roedel
2018-05-16  8:01   ` Lu Baolu
2018-05-16  8:01     ` Lu Baolu
2018-05-16  8:56     ` Tian, Kevin
2018-05-16  8:56       ` Tian, Kevin
2018-05-17  1:13       ` Lu Baolu
2018-05-17  1:13         ` Lu Baolu
2018-05-29 11:56     ` Joerg Roedel
2018-05-30  0:56       ` Lu Baolu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.