Skip to contentSkip to conversation

iommu: APIs for paravirtual PASID allocation

RFC PATCH In review Lu Baolu 7 participants 13 messages Nov 12, 2018 – Feb 19, 2019 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

Lu Baolu

author
#0 lore raw

cover iommu: APIs for paravirtual PASID allocation

This adds an uniformed API set for global PASIDs used by IOMMU and device drivers which depend on IOMMU. It works for drivers running on bare metal, full virtualized environments and para- virtualized environment.

When PASID requests come from components running on the bare metal hardware, the requests will be routed to the system wide I/O ASID allocator.

            ^   ^  iommu_pasid_init() 
            |   |  iommu_pasid_exit() 
            |   |  iommu_pasid_alloc()
            |   |  iommu_pasid_free() 

.-------------------------------.
| IOMMU PASID APIs |
'-------------------------------'
| ^
v |
.------------. .---------------.
| I/O ASID | | |
| allocator | | IOMMU drivers |
| (drivers/ | | or |
| base/ | | virtio IOMMU |
| ioasid.c) | | |
'------------' '---------------'

When PASID requests come from components running in full-virtualized or para-virtualized environments, they will be routed to the vendor- specific or virtio IOMMU driver, where the requests will be intercepted and routed to the Host Linux via VFIO interfaces.

    ......................................
    .            Guest Linux             .
    ......................................
    .                                    .
    . .----------------------------.     .
    . |       IOMMU PASID APIs     |     .
    . '----------------------------'     .
    .                           |        .
    .                           v        .
    .  .----------. .---------------.    ..---------.
    .  | I/O ASID | | IOMMU drivers |    .|  QEMU   |
    .  '----------' | /virtio IOMMU |---->'---------'
    .               '---------------'    .| virtio  |
    ......................................'---------'
                                               |
                                               |
    ...........................................|....
    .                  Host Linux              |   .
    ...........................................|....
    .                      |-------------------'   .
    .                      v                       .
    .         .----------------------------.       .
    .         |       IOMMU PASID APIs     |       .
    .         '----------------------------'       .
    .               |                              .
    .               v       .---------------.      .
    .         .----------.  | IOMMU drivers |      .
    .         | I/O ASID |  | /virtio IOMMU |      .
    .         '----------'  '---------------'      .
    ................................................

Below APIs are introduced:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

It also adds below helpers to lookup or iterate PASID items.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

This patch set depends on the I/O APSID allocator posted here
[1] for discussion.

[1] https://www.spinics.net/lists/iommu/msg30639.html

Best regards,
Lu Baolu

Lu Baolu (5):
iommu: Add APIs for IOMMU PASID management
iommu/vt-d: Initialize a PASID consumer
iommu/vt-d: Enlightened PASID allocation
iommu/vt-d: Allocate and free a pasid
iommu/vt-d: Use global pasid allocator

 drivers/iommu/Kconfig       |  1 +
 drivers/iommu/intel-iommu.c | 83 +++++++++++++++++++++++++++++++---
 drivers/iommu/intel-pasid.c | 88 +++++++++++++++++++++++++-----------
 drivers/iommu/intel-pasid.h | 16 +++++--
 drivers/iommu/intel-svm.c   | 24 +++++-----
 drivers/iommu/iommu.c       | 89 +++++++++++++++++++++++++++++++++++++
 include/linux/intel-iommu.h |  5 +++
 include/linux/iommu.h       | 73 ++++++++++++++++++++++++++++++
 8 files changed, 332 insertions(+), 47 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

RFC PATCH 1/5 iommu: Add APIs for IOMMU PASID management

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Below APIs are added:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

This also adds below helpers to lookup or iterate PASID items associated with a consumer.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <[email protected]>
Cc Kevin Tian <[email protected]>
Cc Jean-Philippe Brucker <[email protected]>
---
 drivers/iommu/Kconfig |  1 +
 drivers/iommu/iommu.c | 89 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h | 73 +++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+)
drivers/iommu/Kconfig +1 -0
index d9a25715650e..39f2bb76c7b8 100644
@@ -1,6 +1,7 @@
 # IOMMU_API always gets selected by whoever wants it.
 config IOMMU_API
 	bool
+	select IOASID
 
 menuconfig IOMMU_SUPPORT
 	bool "IOMMU Hardware Support"
drivers/iommu/iommu.c +89 -0

92 lines not on this page · raw message

include/linux/iommu.h +73 -0

115 lines not on this page · raw message

Signature · 1 line
2.17.1
author
#8 lore raw

RFC PATCH 2/5 iommu/vt-d: Initialize a PASID consumer

In reply to Lu Baolu

This adds the Intel vt-d specific ops to initialize a PASID consumer.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Kevin Tian <[email protected]>
---
 drivers/iommu/intel-iommu.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
drivers/iommu/intel-iommu.c +30 -0
index 49a278a699b0..769b7059d52f 100644
@@ -5597,6 +5597,35 @@ static int intel_iommu_domain_get_attr(struct iommu_domain *domain,
 	return ret;
 }
 
+static int intel_iommu_pasid_init(struct iommu_pasid *pasid)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu = NULL, *temp;
+
+	/*
+	 *  Iterate the iommu units and get a scalable mode capable one
+	 *  for virtual command usages. Return failure if scalable mode
+	 *  doesn't support by any unit.
+	 */
+	rcu_read_lock();
+	for_each_active_iommu(temp, drhd) {
+		if (sm_supported(temp)) {
+			iommu = temp;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	if (!iommu)
+		return -ENODEV;
+
+	pasid->max = intel_pasid_max_id;
+	pasid->min = PASID_MIN;
+	pasid->priv = iommu;
+
+	return 0;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5616,6 +5645,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.domain_get_attr	= intel_iommu_domain_get_attr,
 	.get_dev_attr		= intel_iommu_get_dev_attr,
 	.set_dev_attr		= intel_iommu_set_dev_attr,
+	.pasid_init		= intel_iommu_pasid_init,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
Signature · 1 line
2.17.1
author
#9 lore raw

RFC PATCH 3/5 iommu/vt-d: Enlightened PASID allocation

In reply to Lu Baolu

If Intel IOMMU runs in caching mode, a.k.a. virtual IOMMU, the IOMMU driver should rely on the emulation software to allocate and free PASID IDs. The Intel vt-d spec revision 3.0 defines a register set to support this. This includes a capability register, a virtual command register and a virtual response register. Refer to section 10.4.42, 10.4.43, 10.4.44 for more information.

This patch adds the enlightened PASID allocation/free interfaces via the virtual command register.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Kevin Tian <[email protected]>
---
 drivers/iommu/intel-pasid.c | 70 +++++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-pasid.h | 13 ++++++-
 include/linux/intel-iommu.h |  2 ++
 3 files changed, 84 insertions(+), 1 deletion(-)
drivers/iommu/intel-pasid.c +70 -0
index 839f83974aca..fb42f0c2493e 100644
@@ -28,6 +28,76 @@ static DEFINE_SPINLOCK(pasid_lock);
 u32 intel_pasid_max_id = PASID_MAX;
 static DEFINE_IDR(pasid_idr);
 
+int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int *pasid)
+{
+	u64 res;
+	u64 cap;
+	u8 err_code;
+	unsigned long flags;
+	int ret = 0;
+
+	if (!ecap_vcs(iommu->ecap)) {
+		pr_warn("IOMMU: %s: Hardware doesn't support virtual command\n",
+			iommu->name);
+		return -ENODEV;
+	}
+
+	cap = dmar_readq(iommu->reg + DMAR_VCCAP_REG);
+	if (!(cap & DMA_VCS_PAS)) {
+		pr_warn("IOMMU: %s: Emulation software doesn't support PASID allocation\n",
+			iommu->name);
+		return -ENODEV;
+	}
+
+	raw_spin_lock_irqsave(&iommu->register_lock, flags);
+	dmar_writeq(iommu->reg + DMAR_VCMD_REG, VCMD_CMD_ALLOC);
+	IOMMU_WAIT_OP(iommu, DMAR_VCRSP_REG, dmar_readq,
+		      !(res & VCMD_VRSP_IP), res);
+	raw_spin_unlock_irqrestore(&iommu->register_lock, flags);
+
+	err_code = VCMD_VRSP_EC(res);
+	switch (err_code) {
+	case VCMD_VRSP_EC_SUCCESS:
+		*pasid = VCMD_VRSP_RESULE(res);
+		break;
+	case VCMD_VRSP_EC_UNAVAIL:
+		pr_info("IOMMU: %s: No PASID available\n", iommu->name);
+		ret = -ENOMEM;
+		break;
+	default:
+		ret = -ENODEV;
+		pr_warn("IOMMU: %s: Unkonwn error code %d\n",
+			iommu->name, err_code);
+	}
+
+	return ret;
+}
+
+void vcmd_free_pasid(struct intel_iommu *iommu, unsigned int pasid)
+{
+	u64 res;
+	u8 err_code;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&iommu->register_lock, flags);
+	dmar_writeq(iommu->reg + DMAR_VCMD_REG, (pasid << 8) | VCMD_CMD_FREE);
+	IOMMU_WAIT_OP(iommu, DMAR_VCRSP_REG, dmar_readq,
+		      !(res & VCMD_VRSP_IP), res);
+	raw_spin_unlock_irqrestore(&iommu->register_lock, flags);
+
+	err_code = VCMD_VRSP_EC(res);
+	switch (err_code) {
+	case VCMD_VRSP_EC_SUCCESS:
+		break;
+	case VCMD_VRSP_EC_INVAL:
+		pr_info("IOMMU: %s: Invalid PASID\n", iommu->name);
+		break;
+	default:
+		pr_warn("IOMMU: %s: Unkonwn error code %d\n",
+			iommu->name, err_code);
+	}
+}
+
 int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
 {
 	int ret, min, max;
drivers/iommu/intel-pasid.h +12 -1
index 381545ff9fb7..c80787d02e2d 100644
@@ -19,6 +19,16 @@
 #define PASID_PDE_SHIFT			6
 #define MAX_NR_PASID_BITS		20
 
+/* Virtual command interface for enlightened pasid management. */
+#define VCMD_CMD_ALLOC			0x1
+#define VCMD_CMD_FREE			0x2
+#define VCMD_VRSP_IP			0x1
+#define VCMD_VRSP_EC(e)			(((e) >> 1) & 0x3)
+#define VCMD_VRSP_EC_SUCCESS		0
+#define VCMD_VRSP_EC_UNAVAIL		1
+#define VCMD_VRSP_EC_INVAL		1
+#define VCMD_VRSP_RESULE(e)		(((e) >> 8) & 0xfffff)
+
 /*
  * Domain ID reserved for pasid entries programmed for first-level
  * only and pass-through transfer modes.
@@ -69,5 +79,6 @@ int intel_pasid_setup_pass_through(struct intel_iommu *iommu,
 				   struct device *dev, int pasid);
 void intel_pasid_tear_down_entry(struct intel_iommu *iommu,
 				 struct device *dev, int pasid);
-
+int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int *pasid);
+void vcmd_free_pasid(struct intel_iommu *iommu, unsigned int pasid);
 #endif /* __INTEL_PASID_H */
include/linux/intel-iommu.h +2 -0
index b563a61a6c39..4605eef3686c 100644
@@ -173,6 +173,7 @@
 #define ecap_smpwc(e)		(((e) >> 48) & 0x1)
 #define ecap_flts(e)		(((e) >> 47) & 0x1)
 #define ecap_slts(e)		(((e) >> 46) & 0x1)
+#define ecap_vcs(e)		(((e) >> 45) & 0x1)
 #define ecap_smts(e)		(((e) >> 43) & 0x1)
 #define ecap_dit(e)		((e >> 41) & 0x1)
 #define ecap_pasid(e)		((e >> 40) & 0x1)
@@ -289,6 +290,7 @@
 
 /* PRS_REG */
 #define DMA_PRS_PPR	((u32)1)
+#define DMA_VCS_PAS	((u64)1)
 
 #define IOMMU_WAIT_OP(iommu, offset, op, cond, sts)			\
 do {									\
Signature · 1 line
2.17.1
author
#10 lore raw

RFC PATCH 4/5 iommu/vt-d: Allocate and free a pasid

In reply to Lu Baolu

This adds the Intel vt-d specific ops to allocate and free a pasid value.

---
 drivers/iommu/intel-iommu.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)
drivers/iommu/intel-iommu.c +33 -0
index 769b7059d52f..8dbd0c601dab 100644
@@ -5626,6 +5626,37 @@ static int intel_iommu_pasid_init(struct iommu_pasid *pasid)
 	return 0;
 }
 
+static int intel_iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t start,
+				   ioasid_t end, ioasid_t *ioasid)
+{
+	struct intel_iommu *iommu;
+
+	iommu = pasid->priv;
+	if (!iommu)
+		return -EINVAL;
+
+	/*
+	 * In caching mode, PASID ID should be allocated and freed
+	 * through the virtual command registers. Otherwise, rely
+	 * on the iommu global idr.
+	 */
+	if (!cap_caching_mode(iommu->cap))
+		return -EAGAIN;
+
+	return vcmd_alloc_pasid(iommu, ioasid);
+}
+
+static void intel_iommu_pasid_free(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	struct intel_iommu *iommu;
+
+	iommu = pasid->priv;
+	if (!iommu || !cap_caching_mode(iommu->cap))
+		return;
+
+	vcmd_free_pasid(iommu, ioasid);
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5646,6 +5677,8 @@ const struct iommu_ops intel_iommu_ops = {
 	.get_dev_attr		= intel_iommu_get_dev_attr,
 	.set_dev_attr		= intel_iommu_set_dev_attr,
 	.pasid_init		= intel_iommu_pasid_init,
+	.pasid_alloc		= intel_iommu_pasid_alloc,
+	.pasid_free		= intel_iommu_pasid_free,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
Signature · 1 line
2.17.1
author
#11 lore raw

RFC PATCH 5/5 iommu/vt-d: Use global pasid allocator

In reply to Lu Baolu

This uses global pasid allocator in the Intel iommu driver.

---
 drivers/iommu/intel-iommu.c | 20 +++++++++++++++-----
 drivers/iommu/intel-pasid.c | 36 ------------------------------------
 drivers/iommu/intel-pasid.h |  3 ---
 drivers/iommu/intel-svm.c   | 24 +++++++++++++-----------
 include/linux/intel-iommu.h |  3 +++
 5 files changed, 31 insertions(+), 55 deletions(-)
drivers/iommu/intel-iommu.c +15 -5
index 8dbd0c601dab..e15b81afcbbc 100644
@@ -1698,7 +1698,11 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
 		if (ecap_prs(iommu->ecap))
 			intel_svm_finish_prq(iommu);
 	}
+
+	iommu_pasid_exit(iommu->svm_pasid);
 #endif
+
+	iommu_pasid_exit(iommu->aux_pasid);
 }
 
 static struct dmar_domain *alloc_domain(int flags)
@@ -5050,7 +5054,7 @@ static void auxiliary_unlink_device(struct dmar_domain *domain,
 	domain->auxd_refcnt--;
 
 	if (!domain->auxd_refcnt && domain->default_pasid > 0)
-		intel_pasid_free_id(domain->default_pasid);
+		iommu_pasid_free(info->iommu->aux_pasid, domain->default_pasid);
 }
 
 static int domain_add_dev_auxd(struct dmar_domain *domain,
@@ -5067,9 +5071,10 @@ static int domain_add_dev_auxd(struct dmar_domain *domain,
 
 	spin_lock_irqsave(&device_domain_lock, flags);
 	if (domain->default_pasid <= 0) {
-		domain->default_pasid = intel_pasid_alloc_id(domain, PASID_MIN,
-				pci_max_pasids(to_pci_dev(dev)), GFP_ATOMIC);
-		if (domain->default_pasid < 0) {
+		ret = iommu_pasid_alloc(iommu->aux_pasid, PASID_MIN,
+					pci_max_pasids(to_pci_dev(dev)),
+					domain, &domain->default_pasid);
+		if (ret) {
 			pr_err("Can't allocate default pasid\n");
 			ret = -ENODEV;
 			goto pasid_failed;
@@ -5099,7 +5104,7 @@ static int domain_add_dev_auxd(struct dmar_domain *domain,
 attach_failed:
 	spin_unlock(&iommu->lock);
 	if (!domain->auxd_refcnt && domain->default_pasid > 0)
-		intel_pasid_free_id(domain->default_pasid);
+		iommu_pasid_free(iommu->aux_pasid, domain->default_pasid);
 pasid_failed:
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
@@ -5525,6 +5530,7 @@ static int intel_iommu_enable_auxd(struct device *dev)
 {
 	struct device_domain_info *info;
 	struct dmar_domain *domain;
+	struct intel_iommu *iommu;
 	unsigned long flags;
 
 	if (!scalable_mode_support())
@@ -5539,6 +5545,10 @@ static int intel_iommu_enable_auxd(struct device *dev)
 	info->auxd_enabled = 1;
 	spin_unlock_irqrestore(&device_domain_lock, flags);
 
+	iommu = info->iommu;
+	if (!iommu->aux_pasid)
+		iommu->aux_pasid = iommu_pasid_init(&pci_bus_type);
+
 	return 0;
 }
 
drivers/iommu/intel-pasid.c +0 -36
index fb42f0c2493e..68eaef3bd4fd 100644
@@ -26,7 +26,6 @@
  */
 static DEFINE_SPINLOCK(pasid_lock);
 u32 intel_pasid_max_id = PASID_MAX;
-static DEFINE_IDR(pasid_idr);
 
 int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int *pasid)
 {
@@ -98,41 +97,6 @@ void vcmd_free_pasid(struct intel_iommu *iommu, unsigned int pasid)
 	}
 }
 
-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;
-}
-
 /*
  * Per device pasid table management:
  */
drivers/iommu/intel-pasid.h +0 -3
index c80787d02e2d..029f82ce9f4a 100644
@@ -60,9 +60,6 @@ struct pasid_table {
 };
 
 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);
 void intel_pasid_free_table(struct device *dev);
 struct pasid_table *intel_pasid_get_table(struct device *dev);
drivers/iommu/intel-svm.c +13 -11
index 27cf1963207d..bb57a024eae7 100644
@@ -261,6 +261,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		BUG_ON(!mm);
 	}
 
+	if (!iommu->svm_pasid)
+		iommu->svm_pasid = iommu_pasid_init(&pci_bus_type);
+
 	mutex_lock(&pasid_mutex);
 	if (pasid && !(flags & SVM_FLAG_PRIVATE_PASID)) {
 		struct intel_svm *t;
@@ -325,15 +328,14 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 			pasid_max = intel_pasid_max_id;
 
 		/* Do not use PASID 0 in caching mode (virtualised IOMMU) */
-		ret = intel_pasid_alloc_id(svm,
-					   !!cap_caching_mode(iommu->cap),
-					   pasid_max - 1, GFP_KERNEL);
-		if (ret < 0) {
+		ret = iommu_pasid_alloc(iommu->svm_pasid,
+					!!cap_caching_mode(iommu->cap),
+					pasid_max - 1, svm, &svm->pasid);
+		if (ret) {
 			kfree(svm);
 			kfree(sdev);
 			goto out;
 		}
-		svm->pasid = ret;
 		svm->notifier.ops = &intel_mmuops;
 		svm->mm = mm;
 		svm->flags = flags;
@@ -343,7 +345,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) {
-				intel_pasid_free_id(svm->pasid);
+				iommu_pasid_free(iommu->svm_pasid, svm->pasid);
 				kfree(svm);
 				kfree(sdev);
 				goto out;
@@ -356,7 +358,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		if (ret) {
 			if (mm)
 				mmu_notifier_unregister(&svm->notifier, mm);
-			intel_pasid_free_id(svm->pasid);
+			iommu_pasid_free(iommu->svm_pasid, svm->pasid);
 			kfree(svm);
 			kfree(sdev);
 			goto out;
@@ -389,7 +391,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 	if (!iommu)
 		goto out;
 
-	svm = intel_pasid_lookup_id(pasid);
+	svm = iommu_pasid_find(iommu->svm_pasid, pasid);
 	if (!svm)
 		goto out;
 
@@ -411,7 +413,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
 				kfree_rcu(sdev, rcu);
 
 				if (list_empty(&svm->devs)) {
-					intel_pasid_free_id(svm->pasid);
+					iommu_pasid_free(iommu->svm_pasid, svm->pasid);
 					if (svm->mm)
 						mmu_notifier_unregister(&svm->notifier, svm->mm);
 
@@ -446,7 +448,7 @@ int intel_svm_is_pasid_valid(struct device *dev, int pasid)
 	if (!iommu)
 		goto out;
 
-	svm = intel_pasid_lookup_id(pasid);
+	svm = iommu_pasid_find(iommu->svm_pasid, pasid);
 	if (!svm)
 		goto out;
 
@@ -545,7 +547,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 
 		if (!svm || svm->pasid != req->pasid) {
 			rcu_read_lock();
-			svm = intel_pasid_lookup_id(req->pasid);
+			svm = iommu_pasid_find(iommu->svm_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. */
include/linux/intel-iommu.h +3 -0
index 4605eef3686c..d79f99388293 100644
@@ -552,6 +552,7 @@ struct intel_iommu {
 #ifdef CONFIG_INTEL_IOMMU_SVM
 	struct page_req_dsc *prq;
 	unsigned char prq_name[16];    /* Name for PRQ interrupt */
+	struct iommu_pasid *svm_pasid;	/* SVM PASID consumer */
 #endif
 	struct q_inval  *qi;            /* Queued invalidation info */
 	u32 *iommu_state; /* Store iommu states between suspend and resume.*/
@@ -564,6 +565,8 @@ struct intel_iommu {
 	struct iommu_device iommu;  /* IOMMU core code handle */
 	int		node;
 	u32		flags;      /* Software defined flags */
+
+	struct iommu_pasid *aux_pasid; /* Aux-domain pasid consumer */
 };
 
 /* PCI domain-device relationship */
Signature · 1 line
2.17.1
#12 lore raw
In reply to Lu Baolu
Quoted 5 lines · Lu Baolu

This adds an uniformed API set for global PASIDs used by IOMMU and device drivers which depend on IOMMU. It works for drivers running on bare metal, full virtualized environments and para- virtualized environment.

Are there also some QEMU patches for this in RFC state?

#13 lore raw
Quoted 8 lines · Konrad Rzeszutek Wilk
Quoted 5 lines · Lu Baolu

This adds an uniformed API set for global PASIDs used by IOMMU and device drivers which depend on IOMMU. It works for drivers running on bare metal, full virtualized environments and para- virtualized environment.

Are there also some QEMU patches for this in RFC state?

Yes, we already have draft codes and are working on refining them. Will send patches out for your review. Thanks!

#2 lore raw

RE: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

In reply to Lu Baolu
Quoted 66 lines

From: Lu Baolu [mailto:baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]]
Sent: Sunday, November 11, 2018 10:45 PM
Subject: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Below APIs are added:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

This also adds below helpers to lookup or iterate PASID items associated with a consumer.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Kevin Tian <[email protected]>
Cc Jean-Philippe Brucker <[email protected]>
---
 drivers/iommu/Kconfig |  1 +
 drivers/iommu/iommu.c | 89 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h | 73 +++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+)
drivers/iommu/Kconfig index renamed from drivers/iommu/Kconfig +0 -0

d9a25715650e..39f2bb76c7b8 100644

drivers/iommu/Kconfig +1 -0
@@ -1,6 +1,7 @@
 # IOMMU_API always gets selected by whoever wants it.
 config IOMMU_API
 	bool
+	select IOASID
 
 menuconfig IOMMU_SUPPORT
 	bool "IOMMU Hardware Support"
drivers/iommu/iommu.c index renamed from drivers/iommu/iommu.c +0 -0

0b7c96d1425e..570b244897bb 100644

drivers/iommu/iommu.c void iommu_detach_device_aux(struct iommu_domain

Quoted 10 lines
drivers/iommu/iommu.c +6 -0
@@ -2082,3 +2082,92 @@ void iommu_detach_device_aux(struct iommu_domain
 *domain, struct device *dev)
 	}
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device_aux);
+
+/*
+ * APIs for PASID used by IOMMU and the device drivers which depend
+ * on IOMMU.
+ */
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus) {

I'm thinking about if using struct iommu_domain here is better than struct bus_type. The major purpose is to pass iommu_ops in it and route into iommu-sublayer. iommu_domain may be better since some modules like vfio_iommu_type1 would use iommu_domain more than bus type.

Thanks,
Yi Liu

author
#3 lore raw

Re: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

Hi,

Quoted 74 lines · Yi L
Quoted 74 lines

From: Lu Baolu [mailto:baolu.lu-VuQAYsv1563Yd54FQh9/[email protected]]
Sent: Sunday, November 11, 2018 10:45 PM
Subject: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Below APIs are added:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

This also adds below helpers to lookup or iterate PASID items associated with a consumer.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <jacob.jun.pan-VuQAYsv1563Yd54FQh9/[email protected]>
Cc Kevin Tian <[email protected]>
Cc Jean-Philippe Brucker <[email protected]>
---
  drivers/iommu/Kconfig |  1 +
  drivers/iommu/iommu.c | 89 +++++++++++++++++++++++++++++++++++++++++++
  include/linux/iommu.h | 73 +++++++++++++++++++++++++++++++++++
  3 files changed, 163 insertions(+)
drivers/iommu/Kconfig index renamed from drivers/iommu/Kconfig +0 -0

d9a25715650e..39f2bb76c7b8 100644

drivers/iommu/Kconfig +1 -0
@@ -1,6 +1,7 @@
  # IOMMU_API always gets selected by whoever wants it.
  config IOMMU_API
  	bool
+	select IOASID
 
  menuconfig IOMMU_SUPPORT
  	bool "IOMMU Hardware Support"
drivers/iommu/iommu.c index renamed from drivers/iommu/iommu.c +0 -0

0b7c96d1425e..570b244897bb 100644

drivers/iommu/iommu.c +4 -0
@@ -2082,3 +2082,92 @@ void iommu_detach_device_aux(struct iommu_domain
 *domain, struct device *dev)
  	}
  }
  EXPORT_SYMBOL_GPL(iommu_detach_device_aux);
+
+/*
+ * APIs for PASID used by IOMMU and the device drivers which depend
+ * on IOMMU.
Quoted 8 lines
Quoted 2 lines

+ */
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus) {

I'm thinking about if using struct iommu_domain here is better than struct bus_type. The major purpose is to pass iommu_ops in it and route into iommu-sublayer. iommu_domain may be better since some modules like vfio_iommu_type1 would use iommu_domain more than bus type.

But drivers might call this during initialization when it doesn't has any domain yet.

Best regards,
Lu Baolu

#4 lore raw

Re: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

In reply to Lu Baolu
Quoted 5 lines · On Mon, 12 Nov 2018 14:44:57 +0800 Lu Baolu <[email protected]> wrote:

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Trying to integrate this with VT-d SVM code had some thoughts. First of all, it seems to me the problem we are trying to solve here is to allow custom PASID/IOASID allocator.
IOASID, as in driver base, is a common infrastructure of its own right. So it is OK to let device drivers such as VT-d driver directly communicate with IOASID w/o going through common IOMMU layer. Therefore I see little value of adding this wrapper to ioasid code, instead I feel it might be less work to enhance ioasid with the following:

1. allow ioasid code to register a system wide custom asid allocator, which takes precedence over the idr allocator
e.g.
typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data); void ioasid_set_allocator(ioasid_alloc_fn_t fn)
{}
We can still use idr for tracking ioasid and private data lookup, since the ioasid idr is exclusively owned by ioasid_alloc, we can rest and be sure there is no conflict with other callers. See code comments below.

2. support setting asid private data _after_ asid is allocated. The use case is that PASID allocation and mm bind can be split into separate stages. Private data (mm related) are not available at the time of allocation, only bind time.
Since IDR needs the data pointer at allocation time, we can still create an empty ioasid_data for ioasid tracking at alloc time. i.e. struct ioasid_data {
void *ptr; /* private data */
};

3. allow NULL ioasid_set
I also had a hard time understanding the use of ioasid_set, since there is already a private data associated with each ioasid, is it not enough to group ioasid based on private data?

So the usage scenarios can be.
1. during boot, vt-d driver register a custom ioasid allocator (vcmd) if it detects its running as a guest.

2. Running as a guest, all pasid allocation via ioasid_alloc() will go to this custom allocators and tracked

3. for native case, there is no custom allocator, ioasid just use IDR alloc

4. for native bind mm, pasid allocation already has private data filled in when calling ioasid_alloc

5. for guest bind, pasid is allocated with empty private data (called by VFIO layer), but private data is filled in later by bind guest pasid inside the vt-d driver.

So my thinking is that we can avoid having another layer of APIs as below and keep everything within ioasid. Also allows private data to be managed at lower level as compared to VFIO.

Thanks,

Jacob

Quoted 121 lines

Below APIs are added:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

This also adds below helpers to lookup or iterate PASID items associated with a consumer.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <[email protected]>
Cc Kevin Tian <[email protected]>
Cc Jean-Philippe Brucker <[email protected]>
---
 drivers/iommu/Kconfig |  1 +
 drivers/iommu/iommu.c | 89

+++++++++++++++++++++++++++++++++++++++++++ include/linux/iommu.h | 73 +++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+)

drivers/iommu/Kconfig +1 -0
index d9a25715650e..39f2bb76c7b8 100644
@@ -1,6 +1,7 @@
 # IOMMU_API always gets selected by whoever wants it.
 config IOMMU_API
 	bool
+	select IOASID
 
 menuconfig IOMMU_SUPPORT
 	bool "IOMMU Hardware Support"
drivers/iommu/iommu.c +26 -0
index 0b7c96d1425e..570b244897bb 100644
@@ -2082,3 +2082,92 @@ void iommu_detach_device_aux(struct
 iommu_domain *domain, struct device *dev) }
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device_aux);
+
+/*
+ * APIs for PASID used by IOMMU and the device drivers which depend
+ * on IOMMU.
+ */
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus)
+{
+	struct iommu_pasid *pasid;
+	int ret;
+
+	if (!bus || !bus->iommu_ops)
+		return NULL;
+
+	pasid = kzalloc(sizeof(*pasid), GFP_KERNEL);
+	if (!pasid)
+		return NULL;
+
+	pasid->ops = bus->iommu_ops;
+	/*
+	 * The default range of an IOMMU PASID is from 0 to the full
+	 * 20bit integer.
+	 */
+	pasid->min = 0;
+	pasid->max = 0x100000;
+	/*
+	 * Give vendor specific iommu drivers a chance to set the

pasid

drivers/iommu/iommu.c +31 -0
+	 * limits imposed by the iommu hardware.
+	 */
+	if (bus->iommu_ops->pasid_init) {
+		ret = bus->iommu_ops->pasid_init(pasid);
+		if (ret) {
+			kfree(pasid);
+			return NULL;
+		}
+	}
+
+	return pasid;
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_init);
+
+void iommu_pasid_exit(struct iommu_pasid *pasid)
+{
+	kfree(pasid);
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_exit);
+
+int iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		      ioasid_t max, void *private, ioasid_t *ioasid)
+{
+	ioasid_t start, end, hw, val;
+	int ret = -EAGAIN;
+
+	start = max_t(int, min, pasid->min);
+	end = min_t(int, max, pasid->max);
+
+	if (pasid->ops->pasid_alloc)
+		ret = pasid->ops->pasid_alloc(pasid, start, end,

&hw); +

drivers/iommu/iommu.c void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev) · line 2146

Quoted 4 lines
drivers/iommu/iommu.c +4 -0
+	if (ret == -EAGAIN)
+                val = ioasid_alloc(&pasid->set, start, end, private);
+	else if (ret == 0)
+		val = ioasid_alloc(&pasid->set, hw, hw + 1, private);

this may fail due to conflict with other callers of ioasic_alloc(), but we should really respect the iommu ops allocator. If we move the pasid_alloc op into ioasid code, then we don't need to worry about the conflict.

Quoted 142 lines
drivers/iommu/iommu.c +27 -0
+	else
+		goto hw_ret;
+
+	if (val == INVALID_IOASID)
+		goto ioasid_ret;
+
+	*ioasid = val;
+
+        return 0;
+
+ioasid_ret:
+	if (pasid->ops->pasid_free)
+		pasid->ops->pasid_free(pasid, hw);
+
+hw_ret:
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_alloc);
+
+void iommu_pasid_free(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	if (pasid->ops->pasid_free)
+		pasid->ops->pasid_free(pasid, ioasid);
+
+	ioasid_free(ioasid);
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_free);
include/linux/iommu.h +10 -0
index 9bf1b3f2457a..4f5202c8170b 100644
@@ -20,6 +20,7 @@
 #define __LINUX_IOMMU_H
 
 #include <linux/scatterlist.h>
+#include <linux/ioasid.h>
 #include <linux/device.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -48,6 +49,7 @@ struct bus_type;
 struct device;
 struct iommu_domain;
 struct notifier_block;
+struct iommu_pasid;
 
 /* iommu fault flags */
 #define IOMMU_FAULT_READ	0x0
@@ -194,6 +196,9 @@ enum iommu_dev_attr {
  * @of_xlate: add OF master IDs to iommu grouping
  * @get_dev_attr: get per device IOMMU attributions
  * @set_dev_attr: set per device IOMMU attributions
+ * @pasid_init: initialize a pasid consumer
+ * @pasid_alloc: allocate a pasid from low level driver
+ * @pasid_free: free a pasid to low level driver
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -246,6 +251,12 @@ struct iommu_ops {
 	int (*attach_dev_aux)(struct iommu_domain *domain, struct
 device *dev); void (*detach_dev_aux)(struct iommu_domain *domain,
 struct device *dev); 
+	/* IOMMU pasid callbacks */
+	int (*pasid_init)(struct iommu_pasid *pasid);
+	int (*pasid_alloc)(struct iommu_pasid *pasid, ioasid_t start,
+			   ioasid_t end, ioasid_t *ioasid);
+	void (*pasid_free)(struct iommu_pasid *pasid, ioasid_t

ioasid); +

include/linux/iommu.h +0 -0
 	unsigned long pgsize_bitmap;
 };
@@ -428,12 +439,41 @@ extern int iommu_attach_device_aux(struct

iommu_domain *domain, extern void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev);

include/linux/iommu.h +20 -0
+/*
+ * Per IOMMU PASID consumer data.
+ */
+struct iommu_pasid {
+	ioasid_t		max;
+	ioasid_t		min;
+	struct ioasid_set	set;
+	const struct iommu_ops	*ops;
+
+	/* vendor specific iommu private data */
+	void			*priv;
+};
+
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus);
+void iommu_pasid_exit(struct iommu_pasid *pasid);
+int iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		      ioasid_t max, void *private, ioasid_t *ioasid);
+void iommu_pasid_free(struct iommu_pasid *pasid, ioasid_t iosid);
+static inline int
+iommu_pasid_for_each(struct iommu_pasid *pasid, ioasid_iter_t func,

void *data) +{

include/linux/iommu.h +32 -0
+	return ioasid_for_each(&pasid->set, func, data);
+}
+static inline void*
+iommu_pasid_find(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	return ioasid_find(&pasid->set, ioasid);
+}
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
 struct iommu_group {};
 struct iommu_fwspec {};
 struct iommu_device {};
+struct iommu_pasid {};
 
 static inline bool iommu_present(struct bus_type *bus)
 {
@@ -734,6 +774,39 @@ static inline void
 iommu_detach_device_aux(struct iommu_domain *domain, struct device
 *dev) {
 }
+
+static inline struct iommu_pasid *
+iommu_pasid_init(struct bus_type *bus)
+{
+	return NULL;
+}
+
+static inline void iommu_pasid_exit(struct iommu_pasid *pasid)
+{
+}
+
+static inline int
+iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		  ioasid_t max, void *private, ioasid_t *ioasid)
+{
+	return -ENODEV;
+}
+
+static inline void iommu_pasid_free(struct iommu_pasid *pasid,
 ioasid_t iosid) +{
+}
+
+static inline int
+iommu_pasid_for_each(struct iommu_pasid *pasid, ioasid_iter_t func,
 void *data) +{
+	return -ENODEV;

include/linux/iommu.h static inline void · line 812

Quoted 10 lines
include/linux/iommu.h +7 -0
+}
+
+static inline void*
+iommu_pasid_find(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	return NULL;
+}
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

[Jacob Pan]

#5 lore raw

Re: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

Hi Jacob, Lu,

Quoted 17 lines · Jacob Pan
Quoted 5 lines · On Mon, 12 Nov 2018 14:44:57 +0800 Lu Baolu <[email protected]> wrote:

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Trying to integrate this with VT-d SVM code had some thoughts. First of all, it seems to me the problem we are trying to solve here is to allow custom PASID/IOASID allocator.
IOASID, as in driver base, is a common infrastructure of its own right. So it is OK to let device drivers such as VT-d driver directly communicate with IOASID w/o going through common IOMMU layer. Therefore I see little value of adding this wrapper to ioasid code, instead I feel it might be less work to enhance ioasid with the following:

Quoted 9 lines

1. allow ioasid code to register a system wide custom asid allocator, which takes precedence over the idr allocator
e.g.
typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data); void ioasid_set_allocator(ioasid_alloc_fn_t fn)
{}
We can still use idr for tracking ioasid and private data lookup, since the ioasid idr is exclusively owned by ioasid_alloc, we can rest and be sure there is no conflict with other callers. See code comments below.

Yes, I think we need the alloc function to be registered at boot time. Because even when using a PV allocation instead of idr allocation, we do need a way to quickly retrieve PASID->mm when handling an I/O page fault. In the ioasid_alloc() function, we could do, roughly:

static ioasid_t (*ioasid_alloc_fn)(...);

ioasid_t ioasid_alloc(... min, max...)

{
	if (ioasid_alloc_fn) {
	   ioasid = ioasid_alloc_fn(min, max);

	   /* With PV allocation, only use the IDR for storage */
	   min = ioasid;
	   max = ioasid + 1;
	}

	idr_lock
	ioasid = idr_alloc(... min, max... );
	idr_unlock
}
Quoted 9 lines

2. support setting asid private data _after_ asid is allocated. The use case is that PASID allocation and mm bind can be split into separate stages. Private data (mm related) are not available at the time of allocation, only bind time.
Since IDR needs the data pointer at allocation time, we can still create an empty ioasid_data for ioasid tracking at alloc time. i.e. struct ioasid_data {
void *ptr; /* private data */
};

I think I agree as well (though finding the right ordering and locking in SVA allocation is giving me the biggest headache these days). If the PASID becomes reachable early we need to make sure that concurrent threads working on the PASID space can deal with incomplete data, but I think that might be one of the easiest alternatives.

Quoted 4 lines

3. allow NULL ioasid_set
I also had a hard time understanding the use of ioasid_set, since there is already a private data associated with each ioasid, is it not enough to group ioasid based on private data?

The idea behind ioasid_set is that different modules compete for the single IOASID space, and associate different data types to their IOASIDs.

(a) iommu-sva associates an mm_struct
(b) IOMMU driver allocates a PASIDs for each auxiliary domain, and probably associates an iommu_domain to it.
(c) device drivers (e.g. AMD KFD) want some PASIDs for their own use in the same space, and will have their own private data in there.

For (a) we also want the io_pgfault code to find the mm associated with a PASID:
iopf_handle_mm_fault()

    iommu_sva_find(pasid)
      ioasid_find(&shared_pasid_set, pasid) -> mm struct

And in this case we really need the search to be only on the shared_pasid_set, or else the IOPF code gets something that's not an mm struct.

The easy alternative is to let the IOMMU driver handle both (a) and (b), and store the same data type for both. I've been considering this a few times this week, but it doesn't solve case (c).

Thanks,
Jean

Quoted 310 lines

So the usage scenarios can be.
1. during boot, vt-d driver register a custom ioasid allocator (vcmd) if it detects its running as a guest.

2. Running as a guest, all pasid allocation via ioasid_alloc() will go to this custom allocators and tracked

3. for native case, there is no custom allocator, ioasid just use IDR alloc

4. for native bind mm, pasid allocation already has private data filled in when calling ioasid_alloc

5. for guest bind, pasid is allocated with empty private data (called by VFIO layer), but private data is filled in later by bind guest pasid inside the vt-d driver.

So my thinking is that we can avoid having another layer of APIs as below and keep everything within ioasid. Also allows private data to be managed at lower level as compared to VFIO.

Thanks,

Jacob

Quoted 121 lines

Below APIs are added:

* iommu_pasid_init(pasid)
- Initialize a PASID consumer. The vendor specific IOMMU drivers are able to set the PASID range imposed by IOMMU hardware through a callback in iommu_ops.

* iommu_pasid_exit(pasid)
- The PASID consumer stops consuming any PASID.

* iommu_pasid_alloc(pasid, min, max, private, *ioasid)
- Allocate a PASID and associate a @private data with this PASID. The PASID value is stored in @ioaisd if returning success.

* iommu_pasid_free(pasid, ioasid)
- Free a PASID to the pool so that it could be consumed by others.

This also adds below helpers to lookup or iterate PASID items associated with a consumer.

* iommu_pasid_for_each(pasid, func, data)
- Iterate PASID items of the consumer identified by @pasid, and call @func() against each item. An error returned from @func() will break the iteration.

* iommu_pasid_find(pasid, ioasid)
- Retrieve the private data associated with @ioasid.

Cc Ashok Raj <[email protected]>
Cc Jacob Pan <[email protected]>
Cc Kevin Tian <[email protected]>
Cc Jean-Philippe Brucker <[email protected]>
---
 drivers/iommu/Kconfig |  1 +
 drivers/iommu/iommu.c | 89

+++++++++++++++++++++++++++++++++++++++++++ include/linux/iommu.h | 73 +++++++++++++++++++++++++++++++++++ 3 files changed, 163 insertions(+)

drivers/iommu/Kconfig +1 -0
index d9a25715650e..39f2bb76c7b8 100644
@@ -1,6 +1,7 @@
 # IOMMU_API always gets selected by whoever wants it.
 config IOMMU_API
 	bool
+	select IOASID
 
 menuconfig IOMMU_SUPPORT
 	bool "IOMMU Hardware Support"
drivers/iommu/iommu.c +26 -0
index 0b7c96d1425e..570b244897bb 100644
@@ -2082,3 +2082,92 @@ void iommu_detach_device_aux(struct
 iommu_domain *domain, struct device *dev) }
 }
 EXPORT_SYMBOL_GPL(iommu_detach_device_aux);
+
+/*
+ * APIs for PASID used by IOMMU and the device drivers which depend
+ * on IOMMU.
+ */
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus)
+{
+	struct iommu_pasid *pasid;
+	int ret;
+
+	if (!bus || !bus->iommu_ops)
+		return NULL;
+
+	pasid = kzalloc(sizeof(*pasid), GFP_KERNEL);
+	if (!pasid)
+		return NULL;
+
+	pasid->ops = bus->iommu_ops;
+	/*
+	 * The default range of an IOMMU PASID is from 0 to the full
+	 * 20bit integer.
+	 */
+	pasid->min = 0;
+	pasid->max = 0x100000;
+	/*
+	 * Give vendor specific iommu drivers a chance to set the

pasid

drivers/iommu/iommu.c +31 -0
+	 * limits imposed by the iommu hardware.
+	 */
+	if (bus->iommu_ops->pasid_init) {
+		ret = bus->iommu_ops->pasid_init(pasid);
+		if (ret) {
+			kfree(pasid);
+			return NULL;
+		}
+	}
+
+	return pasid;
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_init);
+
+void iommu_pasid_exit(struct iommu_pasid *pasid)
+{
+	kfree(pasid);
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_exit);
+
+int iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		      ioasid_t max, void *private, ioasid_t *ioasid)
+{
+	ioasid_t start, end, hw, val;
+	int ret = -EAGAIN;
+
+	start = max_t(int, min, pasid->min);
+	end = min_t(int, max, pasid->max);
+
+	if (pasid->ops->pasid_alloc)
+		ret = pasid->ops->pasid_alloc(pasid, start, end,

&hw); +

drivers/iommu/iommu.c void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev) · line 2146

Quoted 4 lines
drivers/iommu/iommu.c +4 -0
+	if (ret == -EAGAIN)
+                val = ioasid_alloc(&pasid->set, start, end, private);
+	else if (ret == 0)
+		val = ioasid_alloc(&pasid->set, hw, hw + 1, private);

this may fail due to conflict with other callers of ioasic_alloc(), but we should really respect the iommu ops allocator. If we move the pasid_alloc op into ioasid code, then we don't need to worry about the conflict.

Quoted 142 lines
drivers/iommu/iommu.c +27 -0
+	else
+		goto hw_ret;
+
+	if (val == INVALID_IOASID)
+		goto ioasid_ret;
+
+	*ioasid = val;
+
+        return 0;
+
+ioasid_ret:
+	if (pasid->ops->pasid_free)
+		pasid->ops->pasid_free(pasid, hw);
+
+hw_ret:
+	return -ENODEV;
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_alloc);
+
+void iommu_pasid_free(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	if (pasid->ops->pasid_free)
+		pasid->ops->pasid_free(pasid, ioasid);
+
+	ioasid_free(ioasid);
+}
+EXPORT_SYMBOL_GPL(iommu_pasid_free);
include/linux/iommu.h +10 -0
index 9bf1b3f2457a..4f5202c8170b 100644
@@ -20,6 +20,7 @@
 #define __LINUX_IOMMU_H
 
 #include <linux/scatterlist.h>
+#include <linux/ioasid.h>
 #include <linux/device.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -48,6 +49,7 @@ struct bus_type;
 struct device;
 struct iommu_domain;
 struct notifier_block;
+struct iommu_pasid;
 
 /* iommu fault flags */
 #define IOMMU_FAULT_READ	0x0
@@ -194,6 +196,9 @@ enum iommu_dev_attr {
  * @of_xlate: add OF master IDs to iommu grouping
  * @get_dev_attr: get per device IOMMU attributions
  * @set_dev_attr: set per device IOMMU attributions
+ * @pasid_init: initialize a pasid consumer
+ * @pasid_alloc: allocate a pasid from low level driver
+ * @pasid_free: free a pasid to low level driver
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -246,6 +251,12 @@ struct iommu_ops {
 	int (*attach_dev_aux)(struct iommu_domain *domain, struct
 device *dev); void (*detach_dev_aux)(struct iommu_domain *domain,
 struct device *dev); 
+	/* IOMMU pasid callbacks */
+	int (*pasid_init)(struct iommu_pasid *pasid);
+	int (*pasid_alloc)(struct iommu_pasid *pasid, ioasid_t start,
+			   ioasid_t end, ioasid_t *ioasid);
+	void (*pasid_free)(struct iommu_pasid *pasid, ioasid_t

ioasid); +

include/linux/iommu.h +0 -0
 	unsigned long pgsize_bitmap;
 };
@@ -428,12 +439,41 @@ extern int iommu_attach_device_aux(struct

iommu_domain *domain, extern void iommu_detach_device_aux(struct iommu_domain *domain, struct device *dev);

include/linux/iommu.h +20 -0
+/*
+ * Per IOMMU PASID consumer data.
+ */
+struct iommu_pasid {
+	ioasid_t		max;
+	ioasid_t		min;
+	struct ioasid_set	set;
+	const struct iommu_ops	*ops;
+
+	/* vendor specific iommu private data */
+	void			*priv;
+};
+
+struct iommu_pasid *iommu_pasid_init(struct bus_type *bus);
+void iommu_pasid_exit(struct iommu_pasid *pasid);
+int iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		      ioasid_t max, void *private, ioasid_t *ioasid);
+void iommu_pasid_free(struct iommu_pasid *pasid, ioasid_t iosid);
+static inline int
+iommu_pasid_for_each(struct iommu_pasid *pasid, ioasid_iter_t func,

void *data) +{

include/linux/iommu.h +32 -0
+	return ioasid_for_each(&pasid->set, func, data);
+}
+static inline void*
+iommu_pasid_find(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	return ioasid_find(&pasid->set, ioasid);
+}
 #else /* CONFIG_IOMMU_API */
 
 struct iommu_ops {};
 struct iommu_group {};
 struct iommu_fwspec {};
 struct iommu_device {};
+struct iommu_pasid {};
 
 static inline bool iommu_present(struct bus_type *bus)
 {
@@ -734,6 +774,39 @@ static inline void
 iommu_detach_device_aux(struct iommu_domain *domain, struct device
 *dev) {
 }
+
+static inline struct iommu_pasid *
+iommu_pasid_init(struct bus_type *bus)
+{
+	return NULL;
+}
+
+static inline void iommu_pasid_exit(struct iommu_pasid *pasid)
+{
+}
+
+static inline int
+iommu_pasid_alloc(struct iommu_pasid *pasid, ioasid_t min,
+		  ioasid_t max, void *private, ioasid_t *ioasid)
+{
+	return -ENODEV;
+}
+
+static inline void iommu_pasid_free(struct iommu_pasid *pasid,
 ioasid_t iosid) +{
+}
+
+static inline int
+iommu_pasid_for_each(struct iommu_pasid *pasid, ioasid_iter_t func,
 void *data) +{
+	return -ENODEV;

include/linux/iommu.h static inline void · line 812

Quoted 10 lines
include/linux/iommu.h +7 -0
+}
+
+static inline void*
+iommu_pasid_find(struct iommu_pasid *pasid, ioasid_t ioasid)
+{
+	return NULL;
+}
 #endif /* CONFIG_IOMMU_API */
 
 #ifdef CONFIG_IOMMU_DEBUGFS

[Jacob Pan]

#7 lore raw

Re: [RFC PATCH 1/5] iommu: Add APIs for IOMMU PASID management

In reply to a message not in the archive
Quoted 40 lines · On Fri, 15 Feb 2019 17:33:34 +0000 Jean-Philippe Brucker…

Hi Jacob, Lu,

Quoted 18 lines · Jacob Pan
Quoted 5 lines · On Mon, 12 Nov 2018 14:44:57 +0800 Lu Baolu…

This adds APIs for IOMMU drivers and device drivers to manage the PASIDs used for DMA transfer and translation. It bases on I/O ASID allocator for PASID namespace management and relies on vendor specific IOMMU drivers for paravirtual PASIDs.

Trying to integrate this with VT-d SVM code had some thoughts. First of all, it seems to me the problem we are trying to solve here is to allow custom PASID/IOASID allocator.
IOASID, as in driver base, is a common infrastructure of its own right. So it is OK to let device drivers such as VT-d driver directly communicate with IOASID w/o going through common IOMMU layer. Therefore I see little value of adding this wrapper to ioasid code, instead I feel it might be less work to enhance ioasid with the following:

Quoted 10 lines

1. allow ioasid code to register a system wide custom asid allocator, which takes precedence over the idr allocator e.g.
typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data); void ioasid_set_allocator(ioasid_alloc_fn_t fn) {}
We can still use idr for tracking ioasid and private data lookup, since the ioasid idr is exclusively owned by ioasid_alloc, we can rest and be sure there is no conflict with other callers. See code comments below.

Yes, I think we need the alloc function to be registered at boot time. Because even when using a PV allocation instead of idr allocation, we do need a way to quickly retrieve PASID->mm when handling an I/O page fault. In the ioasid_alloc() function, we could do, roughly:

static ioasid_t (*ioasid_alloc_fn)(...);

ioasid_t ioasid_alloc(... min, max...)

Quoted 14 lines
{
	if (ioasid_alloc_fn) {
	   ioasid = ioasid_alloc_fn(min, max);

	   /* With PV allocation, only use the IDR for storage */
	   min = ioasid;
	   max = ioasid + 1;
	}

	idr_lock
	ioasid = idr_alloc(... min, max... );
	idr_unlock
}

Completely agreed, i have made similar modifications locally and works well with VT-d. Similarly for the free function, so I combined them like this.
typedef ioasid_t (*ioasid_alloc_fn_t)(ioasid_t min, ioasid_t max, void *data);
typedef int (*ioasid_free_fn_t)(ioasid_t ioasid, void *data);

struct ioasid_allocator {
	ioasid_alloc_fn_t alloc;
	ioasid_free_fn_t free;
	void *pdata; /* used for allocator, e.g. iommu model specific
		      * data
		      */
};

The limitation is that, we only support one allocator. I assume there is only one pvIOMMU in the guest kernel?

Quoted 37 lines
Quoted 9 lines

2. support setting asid private data _after_ asid is allocated. The use case is that PASID allocation and mm bind can be split into separate stages. Private data (mm related) are not available at the time of allocation, only bind time.
Since IDR needs the data pointer at allocation time, we can still create an empty ioasid_data for ioasid tracking at alloc time. i.e. struct ioasid_data {
void *ptr; /* private data */
};

I think I agree as well (though finding the right ordering and locking in SVA allocation is giving me the biggest headache these days). If the PASID becomes reachable early we need to make sure that concurrent threads working on the PASID space can deal with incomplete data, but I think that might be one of the easiest alternatives.

Quoted 4 lines

3. allow NULL ioasid_set
I also had a hard time understanding the use of ioasid_set, since there is already a private data associated with each ioasid, is it not enough to group ioasid based on private data?

The idea behind ioasid_set is that different modules compete for the single IOASID space, and associate different data types to their IOASIDs.

(a) iommu-sva associates an mm_struct
(b) IOMMU driver allocates a PASIDs for each auxiliary domain, and probably associates an iommu_domain to it.
(c) device drivers (e.g. AMD KFD) want some PASIDs for their own use in the same space, and will have their own private data in there.

For (a) we also want the io_pgfault code to find the mm associated with a PASID:
iopf_handle_mm_fault()

    iommu_sva_find(pasid)
      ioasid_find(&shared_pasid_set, pasid) -> mm struct
Quoted 8 lines

And in this case we really need the search to be only on the shared_pasid_set, or else the IOPF code gets something that's not an mm struct.

The easy alternative is to let the IOMMU driver handle both (a) and (b), and store the same data type for both. I've been considering this a few times this week, but it doesn't solve case (c).

Thanks for explaining. If the consumer and producer/allocator of an IOASID is the same module, then they should know what private data type they are searching for. So no need for the set in this case? In case of vSVA, I do find set is useful. In this case, VFIO allocates IOASID, IOMMU driver does bind and set private data. ioasid_set would be useful for VFIO layer to enforce security check such that only ioasid allocated by the same qemu process can perform bind operation on it.

Thanks,

Jacob

Quoted 25 lines

Thanks,
Jean

Quoted 22 lines

So the usage scenarios can be.
1. during boot, vt-d driver register a custom ioasid allocator (vcmd) if it detects its running as a guest.

2. Running as a guest, all pasid allocation via ioasid_alloc() will go to this custom allocators and tracked

3. for native case, there is no custom allocator, ioasid just use IDR alloc

4. for native bind mm, pasid allocation already has private data filled in when calling ioasid_alloc

5. for guest bind, pasid is allocated with empty private data (called by VFIO layer), but private data is filled in later by bind guest pasid inside the vt-d driver.

So my thinking is that we can avoid having another layer of APIs as below and keep everything within ioasid. Also allows private data to be managed at lower level as compared to VFIO.

Quoted 11 lines
Quoted 4 lines

Thanks,

Jacob

[...]
[...]
[...]

Quoted 3 lines

[Jacob Pan]

[Jacob Pan]