Enable PASID for DMA API users

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 <[email protected]>

---
 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(-)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c +0 -0

index ee66d1f4cb81..ac79a37ffe06 100644

Hi Jacob,

Quoted 38 lines · Jacob Pan

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 <[email protected]>

---
 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(-)

(quoted diff) +0 -0

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);

(quoted diff) +0 -1

 	/* Allocate a PASID for this mm if necessary */
-	ret = iommu_sva_alloc_pasid(mm, 1, (1U << master->ssid_bits) -

1);

(quoted diff) +1 -0

+	ret = iommu_sva_alloc_pasid(mm, IOASID_ALLOC_BASE, (1U <<

Quoted 1 line

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).

Quoted 5 lines

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?

Quoted 5 lines · Jacob Pan

Quoted 5 lines

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).

Quoted 3 lines

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.

Quoted 11 lines · Jean-Philippe Brucker

Quoted 9 lines · Jacob Pan

Quoted 6 lines

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?

Quoted 4 lines

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

From: Jacob Pan <[email protected]>
Sent: Saturday, December 11, 2021 2:06 AM

Hi Jason,

Quoted 10 lines · Jason Gunthorpe

Quoted 9 lines · Jean-Philippe Brucker

Quoted 8 lines · Jacob Pan

Quoted 6 lines

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

Quoted 9 lines

Quoted 6 lines

Quoted 1 line

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

DMA mapping API is the de facto standard for in-kernel DMA. It operates on a per device/RID basis which is not PASID-aware.

Some modern devices such as Intel Data Streaming Accelerator, PASID is required for certain work submissions. To allow such devices use DMA mapping API, we need the following functionalities:
1. Provide device a way to retrieve a kernel PASID for work submission
2. Enable the kernel PASID on the IOMMU
3. Establish address space for the kernel PASID that matches the default domain. Let it be IOVA or physical address in case of pass-through.

This patch introduces a driver facing API that enables DMA API PASID usage. Once enabled, device drivers can continue to use DMA APIs as is. There is no difference in dma_handle between without PASID and with PASID.

Hi Jacob,

Quoted 11 lines · Jacob Pan

DMA mapping API is the de facto standard for in-kernel DMA. It operates on a per device/RID basis which is not PASID-aware.

Some modern devices such as Intel Data Streaming Accelerator, PASID is required for certain work submissions. To allow such devices use DMA mapping API, we need the following functionalities:
1. Provide device a way to retrieve a kernel PASID for work submission
2. Enable the kernel PASID on the IOMMU
3. Establish address space for the kernel PASID that matches the default domain. Let it be IOVA or physical address in case of pass-through.

Quoted 4 lines

This patch introduces a driver facing API that enables DMA API PASID usage. Once enabled, device drivers can continue to use DMA APIs as is. There is no difference in dma_handle between without PASID and with PASID.

Can a device issue DMA requests with PASID even there's no system IOMMU or the system IOMMU is disabled?

Best regards,
baolu

From: Jacob Pan <[email protected]>
Sent: Thursday, December 9, 2021 2:50 AM

Quoted 1 line

Can a device issue DMA requests with PASID even there's no system

IOMMU

Quoted 2 lines

or the system IOMMU is disabled?

Good point.
If IOMMU is not enabled, device cannot issue DMA requests with PASID. This API will not be available. Forgot to add dummy functions to the header.

Quoted 11 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, December 9, 2021 2:50 AM

Quoted 1 line

Can a device issue DMA requests with PASID even there's no system

IOMMU

Quoted 2 lines

or the system IOMMU is disabled?

Good point.
If IOMMU is not enabled, device cannot issue DMA requests with PASID. This API will not be available. Forgot to add dummy functions to the header.

PASID is a PCI thing, not defined by IOMMU.

I think the key is physically if IOMMU is disabled, how will root complex handle a PCI memory request including a PASID TLP prefix? Does it block such request due to no IOMMU to consume PASID or simply ignore PASID and continue routing the request to the memory controller?

If block, then having an iommu interface makes sense.

If ignore, possibly a DMA API call makes more sense instead, implying that this extension can be used even when iommu is disabled.

I think that is what Baolu wants to point out.

Quoted 19 lines · Kevin

Quoted 11 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, December 9, 2021 2:50 AM

Quoted 1 line

Can a device issue DMA requests with PASID even there's no system

IOMMU

Quoted 2 lines

or the system IOMMU is disabled?

Good point.
If IOMMU is not enabled, device cannot issue DMA requests with PASID. This API will not be available. Forgot to add dummy functions to the header.

PASID is a PCI thing, not defined by IOMMU.

I think the key is physically if IOMMU is disabled, how will root complex handle a PCI memory request including a PASID TLP prefix? Does it block such request due to no IOMMU to consume PASID or simply ignore PASID and continue routing the request to the memory controller?

Quoted 6 lines

If block, then having an iommu interface makes sense.

If ignore, possibly a DMA API call makes more sense instead, implying that this extension can be used even when iommu is disabled.

I think that is what Baolu wants to point out.

Yes, exactly. Imagining in the VM guest environment, do we require a vIOMMU for this functionality? vIOMMU is not performance friendly if we put aside the security considerations.

Best regards,
baolu

Hi Lu,

Quoted 14 lines · Lu Baolu

Quoted 13 lines · Kevin

Quoted 11 lines

From: Jacob Pan <[email protected]>
Sent: Thursday, December 9, 2021 2:50 AM

Quoted 1 line

Can a device issue DMA requests with PASID even there's no system

IOMMU

Quoted 2 lines

or the system IOMMU is disabled?

Good point.
If IOMMU is not enabled, device cannot issue DMA requests with PASID. This API will not be available. Forgot to add dummy functions to the header.

PASID is a PCI thing, not defined by IOMMU.

Quoted 6 lines

Quoted 6 lines

I think the key is physically if IOMMU is disabled, how will root complex handle a PCI memory request including a PASID TLP prefix? Does it block such request due to no IOMMU to consume PASID or simply ignore PASID and continue routing the request to the memory controller?

Quoted 7 lines

Quoted 6 lines

If block, then having an iommu interface makes sense.

If ignore, possibly a DMA API call makes more sense instead, implying that this extension can be used even when iommu is disabled.

I think that is what Baolu wants to point out.

Thanks for clarifying, very good point.
Looking at the PCIe spec. I don't see specific rules for RC to ignore or block PASID TLP if not enabled.
"- A Root Complex that supports PASID TLP Prefixes must have a device specific mechanism for enabling them. By default usage of PASID TLP Prefixes is disabled
- Root Complexes may optionally support TLPs with PASID TLP Prefixes. The
mechanism used to detect whether a Root Complex supports the PASID TLP Prefix is implementation specific

"
For all practical purposes, why would someone sets up PASID for DMA just to be ignored? An IOMMU interface makes sense to me.

Quoted 4 lines

Yes, exactly. Imagining in the VM guest environment, do we require a vIOMMU for this functionality? vIOMMU is not performance friendly if we put aside the security considerations.

The primary use case for accelerators to use in-kernel DMA will be in pass-through mode. vIOMMU should be able to do PT with good performance, right? no nesting, IO page faults.

Quoted 4 lines

Prefixes is disabled
- Root Complexes may optionally support TLPs with PASID TLP Prefixes.
The mechanism used to detect whether a Root Complex supports the PASID TLP Prefix is implementation specific

Isn't implementation specific mechanism is IOMMU?

Quoted 8 lines

"
For all practical purposes, why would someone sets up PASID for DMA just to be ignored? An IOMMU interface makes sense to me.

Quoted 4 lines

Yes, exactly. Imagining in the VM guest environment, do we require a vIOMMU for this functionality? vIOMMU is not performance friendly if we put aside the security considerations.

Quoted 3 lines

The primary use case for accelerators to use in-kernel DMA will be in pass-through mode. vIOMMU should be able to do PT with good performance, right? no nesting, IO page faults.

But from an enabling perspective when PASID is in use we have to mandate either the presence of an IOMMU, or some hypercall that will do the required plumbing for PASID isn't it?

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan <[email protected]>

 drivers/iommu/intel/iommu.c | 105 +++++++++++++++++++++++++++++++++++-
 drivers/iommu/intel/pasid.c |   7 +++
 include/linux/intel-iommu.h |   3 +-
 3 files changed, 113 insertions(+), 2 deletions(-)

drivers/iommu/intel/iommu.c +9 -1

index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct intel_iommu *iommu,
 	if (domain->default_pasid)
 		qi_flush_piotlb(iommu, did, domain->default_pasid,
 				addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we could have SL
+		 * for PASID in the future such as vIOMMU PT. this doesn't get hit.
+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
 	if (!list_empty(&domain->devices))
 		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr, npages, ih);
 }

Quoted 44 lines · Jacob Pan

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan <[email protected]>

 drivers/iommu/intel/iommu.c | 105 +++++++++++++++++++++++++++++++++++-
 drivers/iommu/intel/pasid.c |   7 +++
 include/linux/intel-iommu.h |   3 +-
 3 files changed, 113 insertions(+), 2 deletions(-)

drivers/iommu/intel/iommu.c +4 -1

index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
 intel_iommu *iommu, if (domain->default_pasid)
 		qi_flush_piotlb(iommu, did, domain->default_pasid,
 				addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we

could have SL

drivers/iommu/intel/iommu.c +1 -0

+		 * for PASID in the future such as vIOMMU PT. this

doesn't get hit.

drivers/iommu/intel/iommu.c +4 -0

+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
 	if (!list_empty(&domain->devices))
 		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,

npages, ih); }

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5706

Quoted 5 lines

drivers/iommu/intel/iommu.c +2 -0

@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain, }
 }
 
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{  

This seems like completely the wrong kind of op.

At the level of the iommu driver things should be iommu_domain centric

The op should be

int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid)

Where 'dev' purpose is to provide the RID

The iommu_domain passed in should be the 'default domain' ie the table used for on-demand mapping, or the passthrough page table.

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5708

Quoted 2 lines

drivers/iommu/intel/iommu.c +2 -0

+	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct device_domain_info *info;  

I don't even want to know why an iommu driver is tracking its own per-device state. That seems like completely wrong layering.

Hi Jason,

Quoted 57 lines · Jason Gunthorpe

Quoted 44 lines · Jacob Pan

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan <[email protected]>

  drivers/iommu/intel/iommu.c | 105 +++++++++++++++++++++++++++++++++++-
  drivers/iommu/intel/pasid.c |   7 +++
  include/linux/intel-iommu.h |   3 +-
  3 files changed, 113 insertions(+), 2 deletions(-)

drivers/iommu/intel/iommu.c +4 -1

index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
 intel_iommu *iommu, if (domain->default_pasid)
  		qi_flush_piotlb(iommu, did, domain->default_pasid,
  				addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we

could have SL

drivers/iommu/intel/iommu.c +1 -0

+		 * for PASID in the future such as vIOMMU PT. this

doesn't get hit.

drivers/iommu/intel/iommu.c +4 -0

+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
  	if (!list_empty(&domain->devices))
  		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,

npages, ih); }

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5706

Quoted 5 lines

drivers/iommu/intel/iommu.c +2 -0

@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain, }
  }
 
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{

This seems like completely the wrong kind of op.

At the level of the iommu driver things should be iommu_domain centric

The op should be

Quoted 8 lines

int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid)

Where 'dev' purpose is to provide the RID

The iommu_domain passed in should be the 'default domain' ie the table used for on-demand mapping, or the passthrough page table.

Makes sense. DMA API is device centric, iommu API is domain centric. It should be the common IOMMU code to get the default domain then pass to vendor drivers. Then we can enforce default domain behavior across all vendor drivers.
i.e.

	dom = iommu_get_dma_domain(dev);
	attach_dev_pasid(dom, dev, pasid);

Quoted 6 lines

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5708

Quoted 2 lines

drivers/iommu/intel/iommu.c +2 -0

+	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct device_domain_info *info;

I don't even want to know why an iommu driver is tracking its own per-device state. That seems like completely wrong layering.

This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level, devTLB is per device.

For multi-device groups, this is a need to track how many devices are using the kernel DMA PASID.

Are you suggesting we add the tracking info in the generic layer? i.e. iommu_group.

We could also have a generic device domain info to replace what is in VT-d and FSL IOMMU driver, etc.

Quoted 87 lines · Jacob Pan

Hi Jason,

Quoted 58 lines · Jason Gunthorpe

Quoted 45 lines · Jacob Pan

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan <[email protected]>

  drivers/iommu/intel/iommu.c | 105
+++++++++++++++++++++++++++++++++++- drivers/iommu/intel/pasid.c |
7 +++ include/linux/intel-iommu.h |   3 +-
  3 files changed, 113 insertions(+), 2 deletions(-)

drivers/iommu/intel/iommu.c +4 -1

index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
 intel_iommu *iommu, if (domain->default_pasid)
  		qi_flush_piotlb(iommu, did, domain->default_pasid,
  				addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we

could have SL

drivers/iommu/intel/iommu.c +1 -0

+		 * for PASID in the future such as vIOMMU PT. this

doesn't get hit.

drivers/iommu/intel/iommu.c +4 -0

+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
  	if (!list_empty(&domain->devices))
  		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,

npages, ih); }

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5706

Quoted 5 lines

drivers/iommu/intel/iommu.c +2 -0

@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain, }
  }
 
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{  

This seems like completely the wrong kind of op.

At the level of the iommu driver things should be iommu_domain centric

The op should be

Quoted 8 lines

int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid)

Where 'dev' purpose is to provide the RID

The iommu_domain passed in should be the 'default domain' ie the table used for on-demand mapping, or the passthrough page table.

Makes sense. DMA API is device centric, iommu API is domain centric. It should be the common IOMMU code to get the default domain then pass to vendor drivers. Then we can enforce default domain behavior across all vendor drivers.
i.e.

	dom = iommu_get_dma_domain(dev);
	attach_dev_pasid(dom, dev, pasid);

Quoted 6 lines

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5708

Quoted 2 lines

drivers/iommu/intel/iommu.c +2 -0

+	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct device_domain_info *info;  

I don't even want to know why an iommu driver is tracking its own per-device state. That seems like completely wrong layering.

This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level, devTLB is per device.

Quoted 8 lines

For multi-device groups, this is a need to track how many devices are using the kernel DMA PASID.

Are you suggesting we add the tracking info in the generic layer? i.e. iommu_group.

We could also have a generic device domain info to replace what is in VT-d and FSL IOMMU driver, etc.

The store place of per-device iommu driver private data has already been standardized. The iommu core provides below interfaces for this purpose:

void dev_iommu_priv_set(struct device *dev, void *priv); void *dev_iommu_priv_get(struct device *dev);

If we have anything generic among different vendor iommu drivers, perhaps we could move them into dev->iommu.

Best regards,
baolu

For DMA PASID storage, can we store it in the iommu_domain instead of iommu_group?

Quoted 94 lines · Lu Baolu

Quoted 85 lines · Jacob Pan

Hi Jason,

Quoted 57 lines · Jason Gunthorpe

Quoted 45 lines · Jacob Pan

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan<[email protected]>

   drivers/iommu/intel/iommu.c | 105
+++++++++++++++++++++++++++++++++++- drivers/iommu/intel/pasid.c |
7 +++ include/linux/intel-iommu.h |   3 +-
   3 files changed, 113 insertions(+), 2 deletions(-)

drivers/iommu/intel/iommu.c +4 -1

index 60253bc436bb..a2ef6b9e4bfc 100644
+++ b/drivers/iommu/intel/iommu.c

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
 intel_iommu *iommu, if (domain->default_pasid)
   		qi_flush_piotlb(iommu, did, domain->default_pasid,
   				addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we

could have SL

drivers/iommu/intel/iommu.c +1 -0

+		 * for PASID in the future such as vIOMMU PT. this

doesn't get hit.

drivers/iommu/intel/iommu.c +4 -0

+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
   	if (!list_empty(&domain->devices))
   		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,

npages, ih); }

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5706

Quoted 5 lines

drivers/iommu/intel/iommu.c +2 -0

@@ -5695,6 +5702,100 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain, }
   }
 
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{

This seems like completely the wrong kind of op.

At the level of the iommu driver things should be iommu_domain centric

The op should be

Quoted 8 lines

int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid)

Where 'dev' purpose is to provide the RID

The iommu_domain passed in should be the 'default domain' ie the table used for on-demand mapping, or the passthrough page table.

Makes sense. DMA API is device centric, iommu API is domain centric. It should be the common IOMMU code to get the default domain then pass to vendor drivers. Then we can enforce default domain behavior across all vendor drivers.
i.e.

	dom = iommu_get_dma_domain(dev);
	attach_dev_pasid(dom, dev, pasid);

Quoted 5 lines

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct · line 5708

Quoted 2 lines

drivers/iommu/intel/iommu.c +2 -0

+	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct device_domain_info *info;

I don't even want to know why an iommu driver is tracking its own per-device state. That seems like completely wrong layering.

This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level, devTLB is per device.

Quoted 8 lines

For multi-device groups, this is a need to track how many devices are using the kernel DMA PASID.

Are you suggesting we add the tracking info in the generic layer? i.e. iommu_group.

We could also have a generic device domain info to replace what is in VT-d and FSL IOMMU driver, etc.

Quoted 9 lines

The store place of per-device iommu driver private data has already been standardized. The iommu core provides below interfaces for this purpose:

void dev_iommu_priv_set(struct device *dev, void *priv); void *dev_iommu_priv_get(struct device *dev);

If we have anything generic among different vendor iommu drivers, perhaps we could move them into dev->iommu.

Yes, good suggestion. DMA PASID should be a generic feature, not suitable for the opaque private date. Can we agree on adding the following flag for devTLB invalidation?

(quoted diff) +1 -0

@@ -379,6 +379,7 @@ struct dev_iommu {
         struct iommu_fwspec             *fwspec;
         struct iommu_device             *iommu_dev;
         void                            *priv;
+       u32 pasid_dma_enabled           : 1;
  };
 

For DMA PASID storage, can we store it in the iommu_domain instead of iommu_group? In the end, this PASID is only used for the default domain. It will be easier to refcount how many attached devices are using the PASID. Destroy the PASID when no devices in the group are using PASID DMA. IOTLB flush is per domain also.

Quoted 120 lines · Jacob Pan

Quoted 100 lines · Lu Baolu

Quoted 87 lines · Jacob Pan

Hi Jason,

Quoted 58 lines · Jason Gunthorpe

Quoted 45 lines · Jacob Pan

Between DMA requests with and without PASID (legacy), DMA mapping APIs are used indiscriminately on a device. Therefore, we should always match the addressing mode of the legacy DMA when enabling kernel PASID.

This patch adds support for VT-d driver where the kernel PASID is programmed to match RIDPASID. i.e. if the device is in pass-through, the kernel PASID is also in pass-through; if the device is in IOVA mode, the kernel PASID will also be using the same IOVA space.

There is additional handling for IOTLB and device TLB flush w.r.t. the kernel PASID. On VT-d, PASID-selective IOTLB flush is also on a per-domain basis; whereas device TLB flush is per device. Note that IOTLBs are used even when devices are in pass-through mode. ATS is enabled device-wide, but the device drivers can choose to manage ATS at per PASID level whenever control is available.

Signed-off-by Jacob Pan<[email protected]>

   drivers/iommu/intel/iommu.c | 105
+++++++++++++++++++++++++++++++++++- drivers/iommu/intel/pasid.c |
7 +++ include/linux/intel-iommu.h |   3 +-
   3 files changed, 113 insertions(+), 2 deletions(-)

(quoted diff) +0 -0

b/drivers/iommu/intel/iommu.c index 60253bc436bb..a2ef6b9e4bfc 100644 +++ b/drivers/iommu/intel/iommu.c

(quoted diff) +3 -1

@@ -1743,7 +1743,14 @@ static void domain_flush_piotlb(struct
 intel_iommu *iommu, if (domain->default_pasid)
   		qi_flush_piotlb(iommu, did,
 domain->default_pasid, addr, npages, ih);
-
+	if (domain->kernel_pasid && !domain_type_is_si(domain)) {
+		/*
+		 * REVISIT: we only do PASID IOTLB inval for FL, we

could have SL

(quoted diff) +1 -0

+		 * for PASID in the future such as vIOMMU PT. this

doesn't get hit.

drivers/iommu/intel/iommu.c +4 -0

+		 */
+		qi_flush_piotlb(iommu, did, domain->kernel_pasid,
+				addr, npages, ih);
+	}
   	if (!list_empty(&domain->devices))
   		qi_flush_piotlb(iommu, did, PASID_RID2PASID,

addr, npages, ih); }

drivers/iommu/intel/iommu.c static void · line 5706

Quoted 5 lines

drivers/iommu/intel/iommu.c +2 -0

@@ -5695,6 +5702,100 @@ static void
 intel_iommu_iotlb_sync_map(struct iommu_domain *domain, }
   }
 
+static int intel_enable_pasid_dma(struct device *dev, u32 pasid)
+{  

This seems like completely the wrong kind of op.

At the level of the iommu driver things should be iommu_domain centric

The op should be

Quoted 8 lines

int attach_dev_pasid(struct iommu_domain *domain, struct device *dev, ioasid_t pasid)

Where 'dev' purpose is to provide the RID

The iommu_domain passed in should be the 'default domain' ie the table used for on-demand mapping, or the passthrough page table.

Makes sense. DMA API is device centric, iommu API is domain centric. It should be the common IOMMU code to get the default domain then pass to vendor drivers. Then we can enforce default domain behavior across all vendor drivers.
i.e.

	dom = iommu_get_dma_domain(dev);
	attach_dev_pasid(dom, dev, pasid);

Quoted 6 lines

Review static void · line 5709

Quoted 3 lines

(quoted diff) +2 -0

+	struct intel_iommu *iommu = device_to_iommu(dev, NULL,
 NULL);
+	struct device_domain_info *info;  

I don't even want to know why an iommu driver is tracking its own per-device state. That seems like completely wrong layering.

This is for IOTLB and deTLB flush. IOTLB is flushed at per domain level, devTLB is per device.

Quoted 8 lines

For multi-device groups, this is a need to track how many devices are using the kernel DMA PASID.

Are you suggesting we add the tracking info in the generic layer? i.e. iommu_group.

We could also have a generic device domain info to replace what is in VT-d and FSL IOMMU driver, etc.

The store place of per-device iommu driver private data has already been standardized. The iommu core provides below interfaces for this purpose:

Quoted 6 lines

void dev_iommu_priv_set(struct device *dev, void *priv); void *dev_iommu_priv_get(struct device *dev);

If we have anything generic among different vendor iommu drivers, perhaps we could move them into dev->iommu.

Yes, good suggestion. DMA PASID should be a generic feature, not suitable for the opaque private date. Can we agree on adding the following flag for devTLB invalidation?

(quoted diff) +1 -0

@@ -379,6 +379,7 @@ struct dev_iommu {
         struct iommu_fwspec             *fwspec;
         struct iommu_device             *iommu_dev;
         void                            *priv;
+       u32 pasid_dma_enabled           : 1;
  };
 

Quoted 5 lines

For DMA PASID storage, can we store it in the iommu_domain instead of iommu_group? In the end, this PASID is only used for the default domain. It will be easier to refcount how many attached devices are using the PASID. Destroy the PASID when no devices in the group are using PASID DMA. IOTLB flush is per domain also.

Tying pasid to an iommu_domain is not a good idea. An iommu_domain represents an I/O address translation table. It could be attached to a device or a PASID on the device.

Perhaps the dev_iommu is a reasonable place for this.

(quoted diff) +2 -0

@@ -390,6 +390,8 @@ struct dev_iommu {
         struct iommu_fwspec             *fwspec;
         struct iommu_device             *iommu_dev;
         void                            *priv;
+       unsigned int                    pasid_bits;
+       u32                             kernel_dma_pasid;
  };
 

@pasid_bits is a static attribute of a device which supports PASID feature. It reads the PASID bitwidth that the device could support. The vendor iommu driver could set this when the PASID feature is about to be enabled. Normally, it's the MIN of device and iommu capabilities.

@kernel_dma_pasid is the PASID value used for kernel DMA if it's enabled. It reads INVALID_IOASID if kernel DMA with PASID is not enabled.

Best regards,
baolu

Quoted 3 lines

Tying pasid to an iommu_domain is not a good idea. An iommu_domain represents an I/O address translation table. It could be attached to a device or a PASID on the device.

I don;t think we can avoid storing PASID at domain level or the group's default domain. IOTLB flush is per domain. Default domain of DMA type is already tying to PASID0, right?

Quoted 4 lines · Jacob Pan

Quoted 3 lines

Tying pasid to an iommu_domain is not a good idea. An iommu_domain represents an I/O address translation table. It could be attached to a device or a PASID on the device.

Quoted 3 lines

I don;t think we can avoid storing PASID at domain level or the group's default domain. IOTLB flush is per domain. Default domain of DMA type is already tying to PASID0, right?

No, it is just wrong.

If the HW requires a list of everything that is connected to the iommu_domain then it's private iommu_domain should have that list.

But it is a *list* not a single PASID.

If one device has 10 PASID's pointing to this domain you must flush them all if that is what the HW requires.

Jason

Quoted 3 lines

If one device has 10 PASID's pointing to this domain you must flush them all if that is what the HW requires.

Yes. My point is that other than PASID 0 is a given, we must track the 10 PASIDs to avoid wasted flush. It also depend on how TLBs are tagged and flush granularity available. But at the API level, should we support all the cases?

In-kernel DMA should be managed by DMA mapping API. The existing kernel PASID support is based on the SVA machinery in SVA lib that is intended for user process SVA. The binding between a kernel PASID and kernel mapping has many flaws. See discussions in the link below.

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Link https://lore.kernel.org/linux-iommu/[email protected]/ open here

Signed-off-by Jacob Pan <[email protected]>

---
  .../admin-guide/kernel-parameters.txt         |  6 --
  drivers/dma/Kconfig                           | 10 ----
  drivers/dma/idxd/idxd.h                       |  1 -
  drivers/dma/idxd/init.c                       | 59 ++++++-------------
  drivers/dma/idxd/sysfs.c                      |  7 ---
  5 files changed, 19 insertions(+), 64 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +0 -6

index 9725c546a0d4..fe73d02c62f3 100644
@@ -1751,12 +1751,6 @@
  			In such case C2/C3 won't be used again.
  			idle=nomwait: Disable mwait for CPU C-states
 
-	idxd.sva=	[HW]
-			Format: <bool>
-			Allow force disabling of Shared Virtual Memory (SVA)
-			support for the idxd driver. By default it is set to
-			true (1).
-
  	idxd.tc_override= [HW]
  			Format: <bool>
  			Allow override of default traffic class configuration

drivers/dma/Kconfig +0 -10

index 6bcdb4e6a0d1..3b28bd720e7d 100644
@@ -313,16 +313,6 @@ config INTEL_IDXD_COMPAT
 
  	  If unsure, say N.
 
-# Config symbol that collects all the dependencies that's necessary to
-# support shared virtual memory for the devices supported by idxd.
-config INTEL_IDXD_SVM
-	bool "Accelerator Shared Virtual Memory Support"
-	depends on INTEL_IDXD
-	depends on INTEL_IOMMU_SVM
-	depends on PCI_PRI
-	depends on PCI_PASID
-	depends on PCI_IOV
-
  config INTEL_IDXD_PERFMON
  	bool "Intel Data Accelerators performance monitor support"
  	depends on INTEL_IDXD

drivers/dma/idxd/idxd.h +0 -1

index 0cf8d3145870..3155e3a2d3ae 100644
@@ -262,7 +262,6 @@ struct idxd_device {
  	struct idxd_wq **wqs;
  	struct idxd_engine **engines;
 
-	struct iommu_sva *sva;
  	unsigned int pasid;
 
  	int num_groups;

drivers/dma/idxd/init.c +19 -40

index 7bf03f371ce1..44633f8113e2 100644
@@ -16,6 +16,7 @@
  #include <linux/idr.h>
  #include <linux/intel-svm.h>
  #include <linux/iommu.h>
+#include <linux/dma-iommu.h>
  #include <uapi/linux/idxd.h>
  #include <linux/dmaengine.h>
  #include "../dmaengine.h"

@@ -28,10 +29,6 @@ MODULE_LICENSE("GPL v2");
  MODULE_AUTHOR("Intel Corporation");
  MODULE_IMPORT_NS(IDXD);
 
-static bool sva = true;
-module_param(sva, bool, 0644);
-MODULE_PARM_DESC(sva, "Toggle SVA support on/off");
-
  bool tc_override;
  module_param(tc_override, bool, 0644);
  MODULE_PARM_DESC(tc_override, "Override traffic class defaults");

@@ -530,36 +527,22 @@ static struct idxd_device *idxd_alloc(struct pci_dev *pdev, struct idxd_driver_d
 
  static int idxd_enable_system_pasid(struct idxd_device *idxd)
  {
-	int flags;
-	unsigned int pasid;
-	struct iommu_sva *sva;
-
-	flags = SVM_FLAG_SUPERVISOR_MODE;
-
-	sva = iommu_sva_bind_device(&idxd->pdev->dev, NULL, &flags);
-	if (IS_ERR(sva)) {
-		dev_warn(&idxd->pdev->dev,
-			 "iommu sva bind failed: %ld\n", PTR_ERR(sva));
-		return PTR_ERR(sva);
-	}
+	u32 pasid;
 
-	pasid = iommu_sva_get_pasid(sva);
-	if (pasid == IOMMU_PASID_INVALID) {
-		iommu_sva_unbind_device(sva);
+	pasid = iommu_enable_pasid_dma(&idxd->pdev->dev);
+	if (pasid == INVALID_IOASID) {
+		dev_err(&idxd->pdev->dev, "No kernel DMA PASID\n");
  		return -ENODEV;
  	}
-
-	idxd->sva = sva;
  	idxd->pasid = pasid;
-	dev_dbg(&idxd->pdev->dev, "system pasid: %u\n", pasid);
+
  	return 0;
  }
 
  static void idxd_disable_system_pasid(struct idxd_device *idxd)
  {
-
-	iommu_sva_unbind_device(idxd->sva);
-	idxd->sva = NULL;
+	iommu_disable_pasid_dma(&idxd->pdev->dev);
+	idxd->pasid = 0;
  }
 
  static int idxd_probe(struct idxd_device *idxd)

@@ -575,21 +558,17 @@ static int idxd_probe(struct idxd_device *idxd)
 
  	dev_dbg(dev, "IDXD reset complete\n");
 
-	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
-		rc = iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA);
-		if (rc == 0) {
-			rc = idxd_enable_system_pasid(idxd);
-			if (rc < 0) {
-				iommu_dev_disable_feature(dev, IOMMU_DEV_FEAT_SVA);
-				dev_warn(dev, "Failed to enable PASID. No SVA support: %d\n", rc);
-			} else {
-				set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
-			}
-		} else {
-			dev_warn(dev, "Unable to turn on SVA feature.\n");
-		}
-	} else if (!sva) {
-		dev_warn(dev, "User forced SVA off via module param.\n");
+	/*
+	 * Try to enable both in-kernel and user DMA request with PASID.
+	 * PASID is supported unless both user and kernel PASID are
+	 * supported. Do not fail probe here in that idxd can still be
+	 * used w/o PASID or IOMMU.
+	 */
+	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||
+		idxd_enable_system_pasid(idxd)) {
+		dev_warn(dev, "Failed to enable PASID\n");
+	} else {
+		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
  	}
 
  	idxd_read_caps(idxd);

drivers/dma/idxd/sysfs.c +0 -7

index a9025be940db..35737299c355 100644
@@ -776,13 +776,6 @@ static ssize_t wq_name_store(struct device *dev,
  	if (strlen(buf) > WQ_NAME_SIZE || strlen(buf) == 0)
  		return -EINVAL;
 
-	/*
-	 * This is temporarily placed here until we have SVM support for
-	 * dmaengine.
-	 */
-	if (wq->type == IDXD_WQT_KERNEL && device_pasid_enabled(wq->idxd))
-		return -EOPNOTSUPP;
-
  	memset(wq->name, 0, WQ_NAME_SIZE + 1);
  	strncpy(wq->name, buf, WQ_NAME_SIZE);
  	strreplace(wq->name, '\n', '\0');

Quoted 11 lines · Jacob Pan

In-kernel DMA should be managed by DMA mapping API. The existing kernel PASID support is based on the SVA machinery in SVA lib that is intended for user process SVA. The binding between a kernel PASID and kernel mapping has many flaws. See discussions in the link below.

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Quoted 2 lines

Link https://lore.kernel.org/linux-iommu/[email protected]/ open here

Signed-off-by Jacob Pan <[email protected]>

Acked-by Dave Jiang <[email protected]>

Also cc Vinod and dmaengine@vger

Pls resend collecting acks. I dont have this in my queue

In-kernel DMA should be managed by DMA mapping API. The existing kernel PASID support is based on the SVA machinery in SVA lib that is intended for user process SVA. The binding between a kernel PASID and kernel mapping has many flaws. See discussions in the link below.

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Quoted 10 lines · Jacob Pan

In-kernel DMA should be managed by DMA mapping API. The existing kernel PASID support is based on the SVA machinery in SVA lib that is intended for user process SVA. The binding between a kernel PASID and kernel mapping has many flaws. See discussions in the link below.

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

drivers/dma/idxd/init.c static int idxd_probe(struct idxd_device *idxd) · line 572

Quoted 12 lines

drivers/dma/idxd/init.c +11 -0

+	/*
+	 * Try to enable both in-kernel and user DMA request with PASID.
+	 * PASID is supported unless both user and kernel PASID are
+	 * supported. Do not fail probe here in that idxd can still be
+	 * used w/o PASID or IOMMU.
+	 */
+	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||
+		idxd_enable_system_pasid(idxd)) {
+		dev_warn(dev, "Failed to enable PASID\n");
+	} else {
+		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
  	}

Huh? How can the driver keep going if PASID isn't supported? I thought the whole point of this was because the device cannot do DMA without PASID at all?

Jason

Quoted 16 lines · Jason Gunthorpe

Quoted 10 lines · Jacob Pan

In-kernel DMA should be managed by DMA mapping API. The existing kernel PASID support is based on the SVA machinery in SVA lib that is intended for user process SVA. The binding between a kernel PASID and kernel mapping has many flaws. See discussions in the link below.

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

Quoted 15 lines

drivers/dma/idxd/init.c static int idxd_probe(struct idxd_device *idxd) · line 572

Quoted 12 lines

drivers/dma/idxd/init.c +11 -0

+	/*
+	 * Try to enable both in-kernel and user DMA request with PASID.
+	 * PASID is supported unless both user and kernel PASID are
+	 * supported. Do not fail probe here in that idxd can still be
+	 * used w/o PASID or IOMMU.
+	 */
+	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||
+		idxd_enable_system_pasid(idxd)) {
+		dev_warn(dev, "Failed to enable PASID\n");
+	} else {
+		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
  	}

Huh? How can the driver keep going if PASID isn't supported? I thought the whole point of this was because the device cannot do DMA without PASID at all?

There are 2 types of WQ supported with the DSA devices. A dedicated WQ type and a shared WQ type. The dedicated WQ type can support DMA with and without PASID. The shared wq type must have a PASID to operate. The driver can support dedicated WQ only without PASID usage when there is no PASID support.

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, December 9, 2021 1:51 AM

Quoted 16 lines

Quoted 7 lines

Quoted 7 lines

drivers/dma/idxd/init.c +7 -0

+	/*
+	 * Try to enable both in-kernel and user DMA request with PASID.
+	 * PASID is supported unless both user and kernel PASID are
+	 * supported. Do not fail probe here in that idxd can still be
+	 * used w/o PASID or IOMMU.
+	 */
+	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||

Quoted 8 lines

drivers/dma/idxd/init.c static int idxd_probe(struct idxd_device *idxd) · line 572

Quoted 5 lines

drivers/dma/idxd/init.c +4 -0

+		idxd_enable_system_pasid(idxd)) {
+		dev_warn(dev, "Failed to enable PASID\n");
+	} else {
+		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
  	}

Huh? How can the driver keep going if PASID isn't supported? I thought the whole point of this was because the device cannot do DMA without PASID at all?

Quoted 1 line

There are 2 types of WQ supported with the DSA devices. A dedicated WQ

type

Quoted 1 line

and a shared WQ type. The dedicated WQ type can support DMA with and

without

Quoted 3 lines

PASID. The shared wq type must have a PASID to operate. The driver can support dedicated WQ only without PASID usage when there is no PASID support.

Can you add to the cover letter why does the kernel require to use the shared WQ?

Jason

Quoted 27 lines

From: Jason Gunthorpe <[email protected]>
Sent: Thursday, December 9, 2021 1:51 AM

Quoted 19 lines

Quoted 10 lines

Quoted 10 lines

(quoted diff) +2 -0

+	/*
+	 * Try to enable both in-kernel and user DMA request

with PASID.

(quoted diff) +1 -0

+	 * PASID is supported unless both user and kernel PASID

are

(quoted diff) +1 -0

+	 * supported. Do not fail probe here in that idxd can

still be

drivers/dma/idxd/init.c +3 -0

+	 * used w/o PASID or IOMMU.
+	 */
+	if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA) ||

Quoted 8 lines

drivers/dma/idxd/init.c static int idxd_probe(struct idxd_device *idxd) · line 572

Quoted 5 lines

drivers/dma/idxd/init.c +4 -0

+		idxd_enable_system_pasid(idxd)) {
+		dev_warn(dev, "Failed to enable PASID\n");
+	} else {
+		set_bit(IDXD_FLAG_PASID_ENABLED, &idxd->flags);
  	}  

Huh? How can the driver keep going if PASID isn't supported? I thought the whole point of this was because the device cannot do DMA without PASID at all?

Quoted 2 lines

There are 2 types of WQ supported with the DSA devices. A dedicated WQ

type

Quoted 1 line

and a shared WQ type. The dedicated WQ type can support DMA with and

without

Quoted 8 lines

Quoted 3 lines

PASID. The shared wq type must have a PASID to operate. The driver can support dedicated WQ only without PASID usage when there is no PASID support.

Can you add to the cover letter why does the kernel require to use the shared WQ?

Jason

Two reasons:

On native the shared WQ is useful when the kernel wants to offload some memory operations (e.g. page-zeroing) to DSA. When #CPUs are more than #WQs, this allows per-cpu lock-less submissions using ENQCMD(PASID, payload) instruction.

In guest the virtual DSA HW may only contain a WQ in shared mode (unchangeable by the guest) when the host admin wants to share the limited WQ resource among many VMs. Then there is no choice in guest regardless whether it's for user or kernel controlled DMA.

Quoted 5 lines

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

Hi Jason,

Quoted 8 lines · Jason Gunthorpe

Quoted 5 lines

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

Quoted 2 lines

You can't really say a driver is using the DMA API without actually calling the DMA API..

The IDXD driver is not aware of addressing mode, it is up to the user of dmaengine API to prepare the buffer mappings. Here we only set up the PASID such that it can be picked up during DMA work submission. I tested with /drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also tested with other pieces under development.

Quoted 14 lines · Jacob Pan

Hi Jason,

Quoted 10 lines · Jason Gunthorpe

Quoted 5 lines

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

Quoted 5 lines

The IDXD driver is not aware of addressing mode, it is up to the user of dmaengine API to prepare the buffer mappings. Here we only set up the PASID such that it can be picked up during DMA work submission. I tested with /drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also tested with other pieces under development.

Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't those need the DMA API?

I'm still very confused how this can radically change from using kSVA to DMA API and NOT introduce some more changes than this. They are not

the same thing, they do not use the same IOVA's. Did you test this with bypass mode off?

Jason

Hi Jason,

Quoted 21 lines · Jason Gunthorpe

Quoted 19 lines · Jacob Pan

Hi Jason,

Quoted 10 lines · Jason Gunthorpe

Quoted 5 lines

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

The IDXD driver is not aware of addressing mode, it is up to the user of dmaengine API to prepare the buffer mappings. Here we only set up the PASID such that it can be picked up during DMA work submission. I tested with /drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also tested with other pieces under development.

Quoted 3 lines

Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't those need the DMA API?

Do you mean wq completion record address? It is already using DMA API.

	wq->compls = dma_alloc_coherent(dev, wq->compls_size,
&wq->compls_addr, GFP_KERNEL);
	desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

Quoted 2 lines

the same thing, they do not use the same IOVA's. Did you test this with bypass mode off?

Yes with dmatest. IOVA is the default, I separated out the SATC patch which will put internal accelerators in bypass mode. It can also be verified by iommu debugfs dump of DMA PASID (2) and PASID 0 (RIDPASID) are pointing to he same default domain. e.g

Quoted 23 lines · Jacob Pan

Hi Jason,

Quoted 19 lines · Jason Gunthorpe

Quoted 18 lines · Jacob Pan

Hi Jason,

Quoted 9 lines · Jason Gunthorpe

Quoted 5 lines

This patch utilizes iommu_enable_pasid_dma() to enable DSA to perform DMA requests with PASID under the same mapping managed by DMA mapping API. In addition, SVA-related bits for kernel DMA are removed. As a result, DSA users shall use DMA mapping API to obtain DMA handles instead of using kernel virtual addresses.

Er, shouldn't this be adding dma_map/etc type calls?

You can't really say a driver is using the DMA API without actually calling the DMA API..

The IDXD driver is not aware of addressing mode, it is up to the user of dmaengine API to prepare the buffer mappings. Here we only set up the PASID such that it can be picked up during DMA work submission. I tested with /drivers/dma/dmatest.c which does dma_map_page(), map_single etc. also tested with other pieces under development.

Quoted 7 lines

Quoted 3 lines

Ignoring the work, doesn't IDXD prepare the DMA queues itself, don't those need the DMA API?

Do you mean wq completion record address? It is already using DMA API.

	wq->compls = dma_alloc_coherent(dev, wq->compls_size,
&wq->compls_addr, GFP_KERNEL);
	desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

I would have expected something on the queue submission side too?

Quoted 7 lines

Quoted 3 lines

the same thing, they do not use the same IOVA's. Did you test this with bypass mode off?

Yes with dmatest. IOVA is the default, I separated out the SATC patch which will put internal accelerators in bypass mode. It can also be verified by iommu debugfs dump of DMA PASID (2) and PASID 0 (RIDPASID) are pointing to he same default domain. e.g

Well, OK then..

Jason

From: Jiang, Dave <[email protected]>
Sent: Thursday, December 9, 2021 8:12 AM

Quoted 6 lines

Quoted 5 lines

Quoted 1 line

Do you mean wq completion record address? It is already using DMA API.

	wq->compls = dma_alloc_coherent(dev, wq->compls_size,
&wq->compls_addr, GFP_KERNEL);
	desc->compl_dma = wq->compls_addr + idxd->data->compl_size * i;

I would have expected something on the queue submission side too?

DSA is different than typical DMA devices in the past. Instead of a software descriptor ring where the device DMA to fetch the descriptors after the software ringing a doorbell or writing a head index, the descriptors are submitted directly to the device via a CPU instruction (i.e. MOVDIR64B or ENQCMD(S)). The CPU takes the KVA of the 64B descriptor and writes to the device atomically. No DMA mapping is necessary in this case.

Modern accelerators such as Intel's Data Streaming Accelerator (DSA) can perform DMA requests with PASID, which is a finer granularity than the device's requester ID(RID). In fact, work submissions on DSA shared work queues require PASID.

DMA mapping API is the de facto standard for in-kernel DMA. However, it operates on a per device/RID basis which is not PASID-aware.

This patch introduces the following driver facing API that enables DMA API PASID usage: ioasid_t iommu_enable_pasid_dma(struct device *dev);

A PASID field is added to struct device for the purposes of storing kernel DMA PASID and flushing device IOTLBs. A separate use case in interrupt

message store (IMS) also hinted adding a PASID field to struct device.
https://lore.kernel.org/all/[email protected]/
IMS virtualization and DMA API does not overlap.

Once enabled, device drivers can continue to use DMA APIs as-is. There is no difference in terms of mapping in dma_handle between without PASID and with PASID. The DMA mapping performed by IOMMU will be identical for both requests with and without PASID (legacy), let it be IOVA or PA in case of pass-through.

Quoted 4 lines · Jacob Pan

Modern accelerators such as Intel's Data Streaming Accelerator (DSA) can perform DMA requests with PASID, which is a finer granularity than the device's requester ID(RID). In fact, work submissions on DSA shared work queues require PASID.

Lets use plain langauge please:

DSA HW cannot do DMA from its RID, so always requires a PASID, even for kernel controlled DMA.

To allow it to use the DMA API we must associate a PASID with the iommu_domain that the DMA API is already using for the device's RID.

This way DMA tagged with the PASID will be treated exactly the same as DMA originating from the RID.

Quoted 5 lines

DMA mapping API is the de facto standard for in-kernel DMA. However, it operates on a per device/RID basis which is not PASID-aware.

This patch introduces the following driver facing API that enables DMA API PASID usage: ioasid_t iommu_enable_pasid_dma(struct device *dev);

This is the wrong API, IMHO

It should be more like

int iommu_get_dma_api_pasid(struct device *dev, ioasid_t *pasid);

void iommu_destroy_dma_api_pasid(struct device *dev);

Quoted 3 lines

A PASID field is added to struct device for the purposes of storing kernel DMA PASID and flushing device IOTLBs. A separate use case in interrupt

And this really should not be touching the struct device at all.

At worst the PASID should be stored in the iommu_group.

Quoted 3 lines

message store (IMS) also hinted adding a PASID field to struct device.
https://lore.kernel.org/all/[email protected]/
IMS virtualization and DMA API does not overlap.

This is under debate, I'm skeptical it will happen considering the new direction for this work.

Quoted 5 lines

Once enabled, device drivers can continue to use DMA APIs as-is. There is no difference in terms of mapping in dma_handle between without PASID and with PASID. The DMA mapping performed by IOMMU will be identical for both requests with and without PASID (legacy), let it be IOVA or PA in case of pass-through.

In other words all this does is connect the PASID to the normal DMA-API owned iommu_domain.