[PATCH 0/3] iommu/vt-d: Global PASID name space

[PATCH 0/3] iommu/vt-d: Global PASID name space

[Lu Baolu]

Hi,

This patch series is trying to change the scope of PASID management
used in Intel IOMMU driver from per IOMMU to driver global. This is
required for some cases where current per-IOMMU PASID name space
doesn't work. For an example, one application (associated with one
PASID) might talk to two physical devices simultaneously where two
devices could reside behind two different IOMMU units.

Best regards,
Lu Baolu

Lu Baolu (3):
  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

 drivers/iommu/intel-iommu.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-svm.c   | 34 +++++++++++++-----------
 include/linux/intel-iommu.h |  7 ++++-
 3 files changed, 89 insertions(+), 16 deletions(-)

-- 
2.7.4

[PATCH 1/3] iommu/vt-d: Global PASID name space

[Lu Baolu]

This adds the algorithm to maintain a system wide PASID name space
for the PASID allocation. Previously we maintained a per IOMMU unit
PASID name space which is not suitable for some use cases. For an
example, one application (associated with one PASID) might talk to
two physical devices simultaneously where 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>
---
 drivers/iommu/intel-iommu.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h |  5 ++++
 2 files changed, 69 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 582fd01..2a81936 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -518,6 +518,58 @@ static int iommu_identity_mapping;
 int intel_iommu_gfx_mapped;
 EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
 
+/*
+ * Intel IOMMU global PASID pool:
+ *
+ * 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
+static DEFINE_SPINLOCK(pasid_lock);
+u32 intel_iommu_pasid_max = PASID_MAX;
+static DEFINE_IDR(pasid_idr);
+
+int intel_iommu_alloc_pasid(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_iommu_pasid_max);
+
+	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_iommu_free_pasid(int pasid)
+{
+	spin_lock(&pasid_lock);
+	idr_remove(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+}
+
+void *intel_iommu_lookup_pasid(int pasid)
+{
+	void *p;
+
+	spin_lock(&pasid_lock);
+	p = idr_find(&pasid_idr, pasid);
+	spin_unlock(&pasid_lock);
+
+	return p;
+}
+
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
 static DEFINE_SPINLOCK(device_domain_lock);
 static LIST_HEAD(device_domain_list);
@@ -3263,6 +3315,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_iommu_pasid_max = min_t(u32, temp,
+						      intel_iommu_pasid_max);
+		}
+
 		g_iommus[iommu->seq_id] = iommu;
 
 		intel_iommu_init_qi(iommu);
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 8dad3dd..bf83073 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -494,4 +494,9 @@ extern struct intel_iommu *intel_svm_device_to_iommu(struct device *dev);
 
 extern const struct attribute_group *intel_iommu_groups[];
 
+extern u32 intel_iommu_pasid_max;
+int intel_iommu_alloc_pasid(void *ptr, int start, int end, gfp_t gfp);
+void intel_iommu_free_pasid(int pasid);
+void *intel_iommu_lookup_pasid(int pasid);
+
 #endif
-- 
2.7.4

[PATCH 2/3] iommu/vt-d: Decouple idr bond pointer from svm

[Lu Baolu]

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>
---
 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 99bc9bd..f3b7394 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",
@@ -403,6 +404,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);
@@ -433,6 +435,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		 * anyway. Surely that can be left entirely to the guest? */
 		if (cap_caching_mode(iommu->cap))
 			intel_flush_pasid_dev(svm, sdev, 0);
+
+		list_add_tail(&svm->list, &global_svm_list);
 	}
 	list_add_rcu(&sdev->list, &svm->devs);
 
@@ -488,6 +492,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 bf83073..a2013dd 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

[PATCH 3/3] iommu/vt-d: Use global PASID for SVM usage

[Lu Baolu]

This patch switches PASID management for SVM from SVM specific
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>
---
 drivers/iommu/intel-svm.c   | 20 +++++++++-----------
 include/linux/intel-iommu.h |  1 -
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index f3b7394..1c45f75 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -85,8 +85,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
 				iommu->name);
 	}
 
-	idr_init(&iommu->pasid_idr);
-
 	return 0;
 }
 
@@ -102,7 +100,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 +390,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_iommu_alloc_pasid(svm,
+					      !!cap_caching_mode(iommu->cap),
+					      pasid_max - 1, GFP_KERNEL);
 		if (ret < 0) {
 			kfree(svm);
 			goto out;
@@ -409,7 +407,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_iommu_free_pasid(svm->pasid);
 				kfree(svm);
 				kfree(sdev);
 				goto out;
@@ -463,7 +461,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_iommu_lookup_pasid(pasid);
 	if (!svm)
 		goto out;
 
@@ -488,7 +486,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_iommu_free_pasid(svm->pasid);
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
 
@@ -523,7 +521,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_iommu_lookup_pasid(pasid);
 	if (!svm)
 		goto out;
 
@@ -621,7 +619,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_iommu_lookup_pasid(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 a2013dd..e50bef8 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

Re: [PATCH 0/3] iommu/vt-d: Global PASID name space

[Lu Baolu]

Hi Joerg,

A gentle ping. :-)

Any comments on this patch set?

Best regards,
Lu Baolu

On 03/15/2018 10:38 AM, Lu Baolu wrote:
> Hi,
>
> This patch series is trying to change the scope of PASID management
> used in Intel IOMMU driver from per IOMMU to driver global. This is
> required for some cases where current per-IOMMU PASID name space
> doesn't work. For an example, one application (associated with one
> PASID) might talk to two physical devices simultaneously where two
> devices could reside behind two different IOMMU units.
>
> Best regards,
> Lu Baolu
>
> Lu Baolu (3):
>   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
>
>  drivers/iommu/intel-iommu.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/intel-svm.c   | 34 +++++++++++++-----------
>  include/linux/intel-iommu.h |  7 ++++-
>  3 files changed, 89 insertions(+), 16 deletions(-)
>

Re: [PATCH 0/3] iommu/vt-d: Global PASID name space

[Joerg Roedel]

On Mon, Mar 26, 2018 at 08:38:21AM +0800, Lu Baolu wrote:
> Hi Joerg,
> 
> A gentle ping. :-)
> 
> Any comments on this patch set?

Yes, it doesn't apply on-top of my x86/vt-d branch. Please rebase it
after the merge-window to rc1 and re-send.


Thanks,

	Joerg

Re: [PATCH 0/3] iommu/vt-d: Global PASID name space

[Lu Baolu]

Hi,

On 03/29/2018 04:52 PM, Joerg Roedel wrote:
> On Mon, Mar 26, 2018 at 08:38:21AM +0800, Lu Baolu wrote:
>> Hi Joerg,
>>
>> A gentle ping. :-)
>>
>> Any comments on this patch set?
> Yes, it doesn't apply on-top of my x86/vt-d branch. Please rebase it
> after the merge-window to rc1 and re-send.

Sure.

Best regards,
Lu Baolu