Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
On Sat, Dec 11, 2021 at 08:39:12AM +, Tian, Kevin wrote: > Uniqueness is not the main argument of using global PASIDs for > SWQ, since it can be defined either in per-RID or in global PASID > space. No SVA architecture can allow two processes to use the > same PASID to submit work unless they share mm! > > IMO the real reason is that SWQ for user SVA must be accessed > via ENQCMD instruction which fetches the PASID from a CPU MSR This really should have been inside a comment in the struct mm "pasid is the value used by x86 ENQCMD" (and if we phrase it that way I wonder why it is in a struct mm not some process or task related struct, since it has nothing to do with page tables) And, IMHO, the IOMMU part of the code should avoid using this field. IOMMU should be able to create arbitarily many "SVA" iommu_domains for use by PASID even if they can't be used with ENQCMD. Such is proper layering. Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
> From: Jacob Pan > Sent: Saturday, December 11, 2021 2:06 AM > > Hi Jason, > > On Fri, 10 Dec 2021 08:31:09 -0400, Jason Gunthorpe > wrote: > > > On Fri, Dec 10, 2021 at 09:06:24AM +, Jean-Philippe Brucker wrote: > > > On Thu, Dec 09, 2021 at 10:14:04AM -0800, Jacob Pan wrote: > > > > > This looks like we're just one step away from device drivers needing > > > > > multiple PASIDs for kernel DMA so I'm trying to figure out how to > > > > > evolve the API towards that. It's probably as simple as keeping a > > > > > kernel IOASID set at first, but then we'll probably want to > > > > > optimize by having multiple overlapping sets for each device driver > > > > > (all separate from the SVA set). > > > > Sounds reasonable to start with a kernel set for in-kernel DMA once > > > > we need multiple ones. But I am not sure what *overlapping* sets > mean > > > > here, could you explain? > > > > > > Given that each device uses a separate PASID table, we could allocate > > > the same set of PASID values for different device drivers. We just need > > > to make sure that those values are different from PASIDs allocated for > > > user SVA. > > > > Why does user SVA need global values anyhow? > > > Currently, we have mm.pasid for user SVA. mm is global. We could have per > device PASID for dedicated devices (not shared across mm's), but that would > make things a lot more complex. I am thinking multiple PASIDs per mm is > needed, right? > > For VT-d, the shared workqueue (SWQ) requires global PASIDs in that we > cannot have two processes use the same PASID to submit work on a > workqueue > shared by the two processes. Each process's PASID must be unique to the > SWQ's PASID table. > Uniqueness is not the main argument of using global PASIDs for SWQ, since it can be defined either in per-RID or in global PASID space. No SVA architecture can allow two processes to use the same PASID to submit work unless they share mm! IMO the real reason is that SWQ for user SVA must be accessed via ENQCMD instruction which fetches the PASID from a CPU MSR (managed as part of XSAVE state). This assumes a single PASID representing the address space for the user task regardless how many devices are attached to this task. Otherwise the kernel doesn't know when to set which PASID if multiple PASIDs per mm. Then this single PASID has to be global cross all attached devices to ensure uniqueness. Of course SWQ is only one configuration for user SVA. In theory multiple per-device PASIDs are still feasible for non-SWQ cases. But given mm is global and the SWQ restriction, it's not worthwhile of adding that complexity... Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
Hi Jason, On Fri, 10 Dec 2021 08:31:09 -0400, Jason Gunthorpe wrote: > On Fri, Dec 10, 2021 at 09:06:24AM +, Jean-Philippe Brucker wrote: > > On Thu, Dec 09, 2021 at 10:14:04AM -0800, Jacob Pan wrote: > > > > This looks like we're just one step away from device drivers needing > > > > multiple PASIDs for kernel DMA so I'm trying to figure out how to > > > > evolve the API towards that. It's probably as simple as keeping a > > > > kernel IOASID set at first, but then we'll probably want to > > > > optimize by having multiple overlapping sets for each device driver > > > > (all separate from the SVA set). > > > Sounds reasonable to start with a kernel set for in-kernel DMA once > > > we need multiple ones. But I am not sure what *overlapping* sets mean > > > here, could you explain? > > > > Given that each device uses a separate PASID table, we could allocate > > the same set of PASID values for different device drivers. We just need > > to make sure that those values are different from PASIDs allocated for > > user SVA. > > Why does user SVA need global values anyhow? > Currently, we have mm.pasid for user SVA. mm is global. We could have per device PASID for dedicated devices (not shared across mm's), but that would make things a lot more complex. I am thinking multiple PASIDs per mm is needed, right? For VT-d, the shared workqueue (SWQ) requires global PASIDs in that we cannot have two processes use the same PASID to submit work on a workqueue shared by the two processes. Each process's PASID must be unique to the SWQ's PASID table. > Jason Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
On Fri, Dec 10, 2021 at 09:06:24AM +, Jean-Philippe Brucker wrote: > On Thu, Dec 09, 2021 at 10:14:04AM -0800, Jacob Pan wrote: > > > This looks like we're just one step away from device drivers needing > > > multiple PASIDs for kernel DMA so I'm trying to figure out how to evolve > > > the API towards that. It's probably as simple as keeping a kernel IOASID > > > set at first, but then we'll probably want to optimize by having multiple > > > overlapping sets for each device driver (all separate from the SVA set). > > Sounds reasonable to start with a kernel set for in-kernel DMA once we need > > multiple ones. But I am not sure what *overlapping* sets mean here, could > > you explain? > > Given that each device uses a separate PASID table, we could allocate the > same set of PASID values for different device drivers. We just need to > make sure that those values are different from PASIDs allocated for user > SVA. Why does user SVA need global values anyhow? Jason ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
On Thu, Dec 09, 2021 at 10:14:04AM -0800, Jacob Pan wrote: > > This looks like we're just one step away from device drivers needing > > multiple PASIDs for kernel DMA so I'm trying to figure out how to evolve > > the API towards that. It's probably as simple as keeping a kernel IOASID > > set at first, but then we'll probably want to optimize by having multiple > > overlapping sets for each device driver (all separate from the SVA set). > Sounds reasonable to start with a kernel set for in-kernel DMA once we need > multiple ones. But I am not sure what *overlapping* sets mean here, could > you explain? Given that each device uses a separate PASID table, we could allocate the same set of PASID values for different device drivers. We just need to make sure that those values are different from PASIDs allocated for user SVA. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
Hi Jean-Philippe, On Thu, 9 Dec 2021 11:03:23 +, Jean-Philippe Brucker wrote: > Hi Jacob, > > On Tue, Dec 07, 2021 at 05:47:11AM -0800, Jacob Pan wrote: > > In-kernel DMA is managed by DMA mapping APIs, which supports per device > > addressing mode for legacy DMA requests. With the introduction of > > Process Address Space ID (PASID), device DMA can now target at a finer > > granularity per PASID + Requester ID (RID). > > > > However, for in-kernel DMA there is no need to differentiate between > > legacy DMA and DMA with PASID in terms of mapping. DMA address mapping > > for RID+PASID can be made identical to the RID. The benefit for the > > drivers is the continuation of DMA mapping APIs without change. > > > > This patch reserves a special IOASID for devices that perform in-kernel > > DMA requests with PASID. This global IOASID is excluded from the > > IOASID allocator. The analogous case is PASID #0, a special PASID > > reserved for DMA requests without PASID (legacy). We could have > > different kernel PASIDs for individual devices, but for simplicity > > reasons, a globally reserved one will fit the bill. > > > > Signed-off-by: Jacob Pan > > --- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- > > drivers/iommu/intel/iommu.c | 4 ++-- > > drivers/iommu/intel/pasid.h | 3 +-- > > drivers/iommu/intel/svm.c | 2 +- > > drivers/iommu/ioasid.c | 2 ++ > > include/linux/ioasid.h | 4 > > 6 files changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index > > ee66d1f4cb81..ac79a37ffe06 100644 --- > > a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +++ > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c @@ -329,7 +329,7 @@ > > __arm_smmu_sva_bind(struct device *dev, struct mm_struct *mm) return > > ERR_PTR(-ENOMEM); > > /* Allocate a PASID for this mm if necessary */ > > - ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) - > > 1); > > + ret = iommu_sva_alloc_pasid(mm, IOASID_ALLOC_BASE, (1U << > > master->ssid_bits) - 1); > > I'd rather keep hardware limits as parameters here. PASID#0 is reserved by > the SMMUv3 hardware so we have to pass at least 1 here, but VT-d could > change RID_PASID and pass 0. On the other hand IOASID_DMA_PASID depends on > device drivers needs and is not needed on all systems, so I think could > stay within the ioasid allocator. Could VT-d do an > ioasid_alloc()/ioasid_get() to reserve this global PASID, storing it > under the device_domain_lock? > Yes, this works. We can delegate DMA PASID allocation to vendor drivers. My proposal here is driven by simplicity. > This looks like we're just one step away from device drivers needing > multiple PASIDs for kernel DMA so I'm trying to figure out how to evolve > the API towards that. It's probably as simple as keeping a kernel IOASID > set at first, but then we'll probably want to optimize by having multiple > overlapping sets for each device driver (all separate from the SVA set). Sounds reasonable to start with a kernel set for in-kernel DMA once we need multiple ones. But I am not sure what *overlapping* sets mean here, could you explain? > Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
Hi Jacob, On Tue, Dec 07, 2021 at 05:47:11AM -0800, Jacob Pan wrote: > In-kernel DMA is managed by DMA mapping APIs, which supports per device > addressing mode for legacy DMA requests. With the introduction of > Process Address Space ID (PASID), device DMA can now target at a finer > granularity per PASID + Requester ID (RID). > > However, for in-kernel DMA there is no need to differentiate between > legacy DMA and DMA with PASID in terms of mapping. DMA address mapping > for RID+PASID can be made identical to the RID. The benefit for the > drivers is the continuation of DMA mapping APIs without change. > > This patch reserves a special IOASID for devices that perform in-kernel > DMA requests with PASID. This global IOASID is excluded from the > IOASID allocator. The analogous case is PASID #0, a special PASID > reserved for DMA requests without PASID (legacy). We could have different > kernel PASIDs for individual devices, but for simplicity reasons, a > globally reserved one will fit the bill. > > Signed-off-by: Jacob Pan > --- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +- > drivers/iommu/intel/iommu.c | 4 ++-- > drivers/iommu/intel/pasid.h | 3 +-- > drivers/iommu/intel/svm.c | 2 +- > drivers/iommu/ioasid.c | 2 ++ > include/linux/ioasid.h | 4 > 6 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > index ee66d1f4cb81..ac79a37ffe06 100644 > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c > @@ -329,7 +329,7 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct > *mm) > return ERR_PTR(-ENOMEM); > > /* Allocate a PASID for this mm if necessary */ > - ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) - 1); > + ret = iommu_sva_alloc_pasid(mm, IOASID_ALLOC_BASE, (1U << > master->ssid_bits) - 1); I'd rather keep hardware limits as parameters here. PASID#0 is reserved by the SMMUv3 hardware so we have to pass at least 1 here, but VT-d could change RID_PASID and pass 0. On the other hand IOASID_DMA_PASID depends on device drivers needs and is not needed on all systems, so I think could stay within the ioasid allocator. Could VT-d do an ioasid_alloc()/ioasid_get() to reserve this global PASID, storing it under the device_domain_lock? This looks like we're just one step away from device drivers needing multiple PASIDs for kernel DMA so I'm trying to figure out how to evolve the API towards that. It's probably as simple as keeping a kernel IOASID set at first, but then we'll probably want to optimize by having multiple overlapping sets for each device driver (all separate from the SVA set). Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/4] ioasid: Reserve a global PASID for in-kernel DMA
In-kernel DMA is managed by DMA mapping APIs, which supports per device
addressing mode for legacy DMA requests. With the introduction of
Process Address Space ID (PASID), device DMA can now target at a finer
granularity per PASID + Requester ID (RID).
However, for in-kernel DMA there is no need to differentiate between
legacy DMA and DMA with PASID in terms of mapping. DMA address mapping
for RID+PASID can be made identical to the RID. The benefit for the
drivers is the continuation of DMA mapping APIs without change.
This patch reserves a special IOASID for devices that perform in-kernel
DMA requests with PASID. This global IOASID is excluded from the
IOASID allocator. The analogous case is PASID #0, a special PASID
reserved for DMA requests without PASID (legacy). We could have different
kernel PASIDs for individual devices, but for simplicity reasons, a
globally reserved one will fit the bill.
Signed-off-by: Jacob Pan
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 2 +-
drivers/iommu/intel/iommu.c | 4 ++--
drivers/iommu/intel/pasid.h | 3 +--
drivers/iommu/intel/svm.c | 2 +-
drivers/iommu/ioasid.c | 2 ++
include/linux/ioasid.h | 4
6 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index ee66d1f4cb81..ac79a37ffe06 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -329,7 +329,7 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct
*mm)
return ERR_PTR(-ENOMEM);
/* Allocate a PASID for this mm if necessary */
- ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) - 1);
+ ret = iommu_sva_alloc_pasid(mm, IOASID_ALLOC_BASE, (1U <<
master->ssid_bits) - 1);
if (ret)
goto err_free_bond;
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 6afb4d4e09ef..60253bc436bb 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3253,7 +3253,7 @@ static ioasid_t intel_vcmd_ioasid_alloc(ioasid_t min,
ioasid_t max, void *data)
* PASID range. Host can partition guest PASID range based on
* policies but it is out of guest's control.
*/
- if (min < PASID_MIN || max > intel_pasid_max_id)
+ if (min < IOASID_ALLOC_BASE || max > intel_pasid_max_id)
return INVALID_IOASID;
if (vcmd_alloc_pasid(iommu, ))
@@ -4824,7 +4824,7 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
u32 pasid;
/* No private data needed for the default pasid */
- pasid = ioasid_alloc(NULL, PASID_MIN,
+ pasid = ioasid_alloc(NULL, IOASID_ALLOC_BASE,
pci_max_pasids(to_pci_dev(dev)) - 1,
NULL);
if (pasid == INVALID_IOASID) {
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index d5552e2c160d..c3a714535c03 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -10,8 +10,7 @@
#ifndef __INTEL_PASID_H
#define __INTEL_PASID_H
-#define PASID_RID2PASID0x0
-#define PASID_MIN 0x1
+#define PASID_RID2PASIDIOASID_DMA_NO_PASID
#define PASID_MAX 0x10
#define PASID_PTE_MASK 0x3F
#define PASID_PTE_PRESENT 1
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 5b5d69b04fcc..95dcaf78c22c 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -511,7 +511,7 @@ static int intel_svm_alloc_pasid(struct device *dev, struct
mm_struct *mm,
ioasid_t max_pasid = dev_is_pci(dev) ?
pci_max_pasids(to_pci_dev(dev)) : intel_pasid_max_id;
- return iommu_sva_alloc_pasid(mm, PASID_MIN, max_pasid - 1);
+ return iommu_sva_alloc_pasid(mm, IOASID_ALLOC_BASE, max_pasid - 1);
}
static void intel_svm_free_pasid(struct mm_struct *mm)
diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
index 50ee27bbd04e..89c6132bf1ec 100644
--- a/drivers/iommu/ioasid.c
+++ b/drivers/iommu/ioasid.c
@@ -317,6 +317,8 @@ ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min,
ioasid_t max,
data->private = private;
refcount_set(>refs, 1);
+ if (min < IOASID_ALLOC_BASE)
+ min = IOASID_ALLOC_BASE;
/*
* Custom allocator needs allocator data to perform platform specific
* operations.
diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
index e9dacd4b9f6b..4d435cbd48b8 100644
--- a/include/linux/ioasid.h
+++ b/include/linux/ioasid.h
@@ -6,6 +6,10 @@
#include
#define INVALID_IOASID ((ioasid_t)-1)
+#define IOASID_DMA_NO_PASID0
