Re-enable IDXD kernel workqueue under DMA API

PCIe Process address space ID (PASID) is used to tag DMA traffic, it provides finer grained isolation than requester ID (RID).

For each device/RID, 0 is a special PASID for the normal DMA (no PASID). This is universal across all architectures that supports PASID, therefore warranted to be reserved globally and declared in the common header. Consequently, we can avoid the conflict between different PASID use cases in the generic code. e.g. SVA and DMA API with PASIDs.

This paved away for device drivers to choose global PASID policy while continue doing normal DMA.

Noting that VT-d could support none-zero RID/NO_PASID, but currently not used.

Reviewed-by Lu Baolu <[email protected]>

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

---
v7:
- renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more generic
v6:
- let SMMU code use the common RID_PASID macro

---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 10 ++++----
 drivers/iommu/intel/iommu.c                   | 24 +++++++++----------
 drivers/iommu/intel/pasid.c                   |  2 +-
 drivers/iommu/intel/pasid.h                   |  1 -
 include/linux/iommu.h                         |  1 +
 6 files changed, 20 insertions(+), 20 deletions(-)

(quoted diff) +0 -0

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index a5a63b1c947e..5e6b39881c04 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 @@ -80,7 +80,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
* be some overlap between use of both ASIDs, until we
invalidate the

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

 	 * TLB.
 	 */
-	arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+	arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID, cd);
 
 	/* Invalidate TLB entries previously associated with that

context */ arm_smmu_tlb_inv_asid(smmu, asid);

(quoted diff) +0 -0

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3fd83fb75722..6d64c8fc923f 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1053,7 +1053,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid, /*

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -1

 	 * This function handles the following cases:
 	 *
-	 * (1) Install primary CD, for normal DMA traffic (SSID = 0).
+	 * (1) Install primary CD, for normal DMA traffic (SSID =

IOMMU_NO_PASID = 0).

(quoted diff) +0 -0

 	 * (2) Install a secondary CD, for SID+SSID traffic.
 	 * (3) Update ASID of a CD. Atomically write the first 64 bits

of the

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2 -2

 	 *     CD, then invalidate the old entry and mappings.

@@ -1869,7 +1869,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
 		arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
 	}
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
 }
 
 static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,

@@ -1957,7 +1957,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned
 long iova, size_t size,
 	 * Unfortunately, this can't be leaf-only since we may have
 	 * zapped an entire table.
 	 */
-	arm_smmu_atc_inv_domain(smmu_domain, 0, iova, size);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, iova, size);
 }
 

void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int
asid, @@ -2131,7 +2131,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -1

 	 * the master has been added to the devices list for this domain.
 	 * This isn't an issue because the STE hasn't been installed yet.
 	 */
-	ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
+	ret = arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID,

&cfg->cd); if (ret)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -1

 		goto out_free_cd_tables;

@@ -2317,7 +2317,7 @@ static void arm_smmu_enable_ats(struct
 arm_smmu_master *master) pdev = to_pci_dev(master->dev);
 
 	atomic_inc(&smmu_domain->nr_ats_masters);
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
 	if (pci_enable_ats(pdev, stu))
 		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n",
 stu); }

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

index b871a6afd803..4eba9973f537 100644
@@ -877,7 +877,7 @@ void dmar_fault_dump_ptes(struct intel_iommu *iommu,
 u16 source_id, }
 	/* For request-without-pasid, get the pasid from context entry */
 	if (intel_iommu_sm && pasid == IOMMU_PASID_INVALID)
-		pasid = PASID_RID2PASID;
+		pasid = IOMMU_NO_PASID;
 
 	dir_index = pasid >> PASID_PDE_SHIFT;
 	pde = &dir[dir_index];

@@ -1457,7 +1457,7 @@ static void __iommu_flush_dev_iotlb(struct
 device_domain_info *info, qdep = info->ats_qdep;
 	qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
 			   qdep, addr, mask);
-	quirk_extra_dev_tlb_flush(info, addr, mask, PASID_RID2PASID,
 qdep);
+	quirk_extra_dev_tlb_flush(info, addr, mask, IOMMU_NO_PASID,
 qdep); }
 

static void iommu_flush_dev_iotlb(struct dmar_domain *domain,

drivers/iommu/intel/iommu.c +3 -5

@@ -1492,7 +1492,7 @@ static void iommu_flush_iotlb_psi(struct
 intel_iommu *iommu, ih = 1 << 6;
 
 	if (domain->use_first_level) {
-		qi_flush_piotlb(iommu, did, PASID_RID2PASID, addr,
 pages, ih);
+		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr, pages,
 ih); } else {
 		unsigned long bitmask = aligned_pages - 1;

@@ -1562,7 +1562,7 @@ static void intel_flush_iotlb_all(struct
 iommu_domain *domain) u16 did = domain_id_iommu(dmar_domain, iommu);
 
 		if (dmar_domain->use_first_level)
-			qi_flush_piotlb(iommu, did, PASID_RID2PASID, 0,
-1, 0);
+			qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, 0,
-1, 0); else
 			iommu->flush.flush_iotlb(iommu, did, 0, 0,
 						 DMA_TLB_DSI_FLUSH);

@@ -1952,7 +1952,7 @@ static int domain_context_mapping_one(struct
 dmar_domain *domain, context_pdts(pds);
 
 		/* Setup the RID_PASID field: */
-		context_set_sm_rid2pasid(context, PASID_RID2PASID);
+		context_set_sm_rid2pasid(context, IOMMU_NO_PASID);
 
 		/*
 		 * Setup the Device-TLB enable bit and Page request

@@ -2432,13 +2432,13 @@ static int dmar_domain_attach_device(struct

dmar_domain *domain, /* Setup the PASID entry for requests without PASID:
*/ if (hw_pass_through && domain_type_is_si(domain))
ret = intel_pasid_setup_pass_through(iommu,
domain,

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

-					dev, PASID_RID2PASID);
+					dev, IOMMU_NO_PASID);
 		else if (domain->use_first_level)
 			ret = domain_setup_first_level(iommu, domain,

dev,

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

-					PASID_RID2PASID);
+					IOMMU_NO_PASID);
 		else
 			ret = intel_pasid_setup_second_level(iommu,

domain,

drivers/iommu/intel/iommu.c +3 -3

-					dev, PASID_RID2PASID);
+					dev, IOMMU_NO_PASID);
 		if (ret) {
 			dev_err(dev, "Setup RID2PASID failed\n");
 			device_block_translation(dev);

@@ -3975,7 +3975,7 @@ static void dmar_remove_one_dev_info(struct device
 *dev) if (!dev_is_real_dma_subdevice(info->dev)) {
 		if (dev_is_pci(info->dev) && sm_supported(iommu))
 			intel_pasid_tear_down_entry(iommu, info->dev,
-					PASID_RID2PASID, false);
+					IOMMU_NO_PASID, false);
 
 		iommu_disable_pci_caps(info);
 		domain_context_clear(info);

@@ -4004,7 +4004,7 @@ static void device_block_translation(struct device
 *dev) if (!dev_is_real_dma_subdevice(dev)) {
 		if (sm_supported(iommu))
 			intel_pasid_tear_down_entry(iommu, dev,
-						    PASID_RID2PASID,
 false);
+						    IOMMU_NO_PASID,

false); else

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

 			domain_context_clear(info);
 	}

@@ -4339,7 +4339,7 @@ static void domain_set_force_snooping(struct
 dmar_domain *domain) 
 	list_for_each_entry(info, &domain->devices, link)
 		intel_pasid_setup_page_snoop_control(info->iommu,
 info->dev,
-						     PASID_RID2PASID);
+						     IOMMU_NO_PASID);
 }
 

static bool intel_iommu_enforce_cache_coherency(struct iommu_domain
*domain) @@ -4994,7 +4994,7 @@ void quirk_extra_dev_tlb_flush(struct device_domain_info *info, return;

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

 	sid = PCI_DEVID(info->bus, info->devfn);
-	if (pasid == PASID_RID2PASID) {
+	if (pasid == IOMMU_NO_PASID) {
 		qi_flush_dev_iotlb(info->iommu, sid, info->pfsid,
 				   qdep, address, mask);
 	} else {

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

index c5d479770e12..23dca3bc319d 100644
@@ -438,7 +438,7 @@ devtlb_invalidation_with_pasid(struct intel_iommu
 *iommu,
 	 * SVA usage, device could do DMA with multiple PASIDs. It is

more

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

 	 * efficient to flush devTLB specific to the PASID.
 	 */
-	if (pasid == PASID_RID2PASID)
+	if (pasid == IOMMU_NO_PASID)
 		qi_flush_dev_iotlb(iommu, sid, pfsid, qdep, 0, 64 -

VTD_PAGE_SHIFT); else
qi_flush_dev_iotlb_pasid(iommu, sid, pfsid, pasid, qdep,
0, 64 - VTD_PAGE_SHIFT); diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h index d6b7d21244b1..027d30afaba6 100644

drivers/iommu/intel/pasid.h +0 -1

@@ -10,7 +10,6 @@
 #ifndef __INTEL_PASID_H
 #define __INTEL_PASID_H
 
-#define PASID_RID2PASID			0x0
 #define PASID_MIN			0x1
 #define PASID_MAX			0x100000
 #define PASID_PTE_MASK			0x3F

include/linux/iommu.h +0 -0

index e8c9a7da1060..c714d659d114 100644

PCIe Process address space ID (PASID) is used to tag DMA traffic, it provides finer grained isolation than requester ID (RID).

For each device/RID, 0 is a special PASID for the normal DMA (no PASID). This is universal across all architectures that supports PASID, therefore warranted to be reserved globally and declared in the common header. Consequently, we can avoid the conflict between different PASID use cases in the generic code. e.g. SVA and DMA API with PASIDs.

This paved away for device drivers to choose global PASID policy while continue doing normal DMA.

Noting that VT-d could support none-zero RID/NO_PASID, but currently not used.

Reviewed-by Lu Baolu <[email protected]>

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

---
v7:
- renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more generic
v6:
- let SMMU code use the common RID_PASID macro

---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 10 ++++----
 drivers/iommu/intel/iommu.c                   | 24 +++++++++----------
 drivers/iommu/intel/pasid.c                   |  2 +-
 drivers/iommu/intel/pasid.h                   |  1 -
 include/linux/iommu.h                         |  1 +
 6 files changed, 20 insertions(+), 20 deletions(-)

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

index a5a63b1c947e..5e6b39881c04 100644
@@ -80,7 +80,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
 	 * be some overlap between use of both ASIDs, until we invalidate the
 	 * TLB.
 	 */
-	arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+	arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID, cd);
 
 	/* Invalidate TLB entries previously associated with that context */
 	arm_smmu_tlb_inv_asid(smmu, asid);

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +4 -4

index 3fd83fb75722..6d64c8fc923f 100644
@@ -1053,7 +1053,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
 	/*
 	 * This function handles the following cases:
 	 *
-	 * (1) Install primary CD, for normal DMA traffic (SSID = 0).
+	 * (1) Install primary CD, for normal DMA traffic (SSID = IOMMU_NO_PASID = 0).
 	 * (2) Install a secondary CD, for SID+SSID traffic.
 	 * (3) Update ASID of a CD. Atomically write the first 64 bits of the
 	 *     CD, then invalidate the old entry and mappings.

@@ -1869,7 +1869,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
 		arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
 	}
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
 }
 
 static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,

@@ -1957,7 +1957,7 @@ static void arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
 	 * Unfortunately, this can't be leaf-only since we may have
 	 * zapped an entire table.
 	 */
-	arm_smmu_atc_inv_domain(smmu_domain, 0, iova, size);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, iova, size);
 }
 
 void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int asid,

@@ -2131,7 +2131,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
 	 * the master has been added to the devices list for this domain.
 	 * This isn't an issue because the STE hasn't been installed yet.
 	 */
-	ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
+	ret = arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID, &cfg->cd);
 	if (ret)
 		goto out_free_cd_tables;
 

Hi Jacob,

Quoted 93 lines · Jacob Pan

PCIe Process address space ID (PASID) is used to tag DMA traffic, it provides finer grained isolation than requester ID (RID).

For each device/RID, 0 is a special PASID for the normal DMA (no PASID). This is universal across all architectures that supports PASID, therefore warranted to be reserved globally and declared in the common header. Consequently, we can avoid the conflict between different PASID use cases in the generic code. e.g. SVA and DMA API with PASIDs.

This paved away for device drivers to choose global PASID policy while continue doing normal DMA.

Noting that VT-d could support none-zero RID/NO_PASID, but currently not used.

Reviewed-by Lu Baolu <[email protected]>

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

---
v7:
- renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more generic
v6:
- let SMMU code use the common RID_PASID macro

---
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 10 ++++----
 drivers/iommu/intel/iommu.c                   | 24 +++++++++----------
 drivers/iommu/intel/pasid.c                   |  2 +-
 drivers/iommu/intel/pasid.h                   |  1 -
 include/linux/iommu.h                         |  1 +
 6 files changed, 20 insertions(+), 20 deletions(-)

(quoted diff) +0 -0

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c index a5a63b1c947e..5e6b39881c04 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 @@ -80,7 +80,7 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
* be some overlap between use of both ASIDs, until we
invalidate the

(quoted diff) +1 -1

 	 * TLB.
 	 */
-	arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
+	arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID, cd);
 
 	/* Invalidate TLB entries previously associated with that

context */ arm_smmu_tlb_inv_asid(smmu, asid);

(quoted diff) +0 -0

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 3fd83fb75722..6d64c8fc923f 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1053,7 +1053,7 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid, /*

(quoted diff) +1 -1

 	 * This function handles the following cases:
 	 *
-	 * (1) Install primary CD, for normal DMA traffic (SSID = 0).
+	 * (1) Install primary CD, for normal DMA traffic (SSID =

IOMMU_NO_PASID = 0).

(quoted diff) +0 -0

 	 * (2) Install a secondary CD, for SID+SSID traffic.
 	 * (3) Update ASID of a CD. Atomically write the first 64 bits

of the

(quoted diff) +2 -2

 	 *     CD, then invalidate the old entry and mappings.

@@ -1869,7 +1869,7 @@ static void arm_smmu_tlb_inv_context(void *cookie)
 		cmd.tlbi.vmid	= smmu_domain->s2_cfg.vmid;
 		arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
 	}
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
 }
 
 static void __arm_smmu_tlb_inv_range(struct arm_smmu_cmdq_ent *cmd,

@@ -1957,7 +1957,7 @@ static void
 arm_smmu_tlb_inv_range_domain(unsigned long iova, size_t size,
 	 * Unfortunately, this can't be leaf-only since we may have
 	 * zapped an entire table.
 	 */
-	arm_smmu_atc_inv_domain(smmu_domain, 0, iova, size);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, iova,
 size); }
 

void arm_smmu_tlb_inv_range_asid(unsigned long iova, size_t size, int
asid, @@ -2131,7 +2131,7 @@ static int arm_smmu_domain_finalise_s1(struct arm_smmu_domain *smmu_domain,
* the master has been added to the devices list for this
domain.
* This isn't an issue because the STE hasn't been installed
yet. */

(quoted diff) +1 -1

-	ret = arm_smmu_write_ctx_desc(smmu_domain, 0, &cfg->cd);
+	ret = arm_smmu_write_ctx_desc(smmu_domain, IOMMU_NO_PASID,

&cfg->cd); if (ret)

(quoted diff) +0 -0

 		goto out_free_cd_tables;

Review static void arm_smmu_enable_ats(struct · line 2323

Quoted 8 lines

(quoted diff) +1 -1

@@ -2317,7 +2317,7 @@ static void arm_smmu_enable_ats(struct
 arm_smmu_master *master) pdev = to_pci_dev(master->dev);
 
 	atomic_inc(&smmu_domain->nr_ats_masters);
-	arm_smmu_atc_inv_domain(smmu_domain, 0, 0, 0);
+	arm_smmu_atc_inv_domain(smmu_domain, IOMMU_NO_PASID, 0, 0);
 	if (pci_enable_ats(pdev, stu))
 		dev_err(master->dev, "Failed to enable ATS (STU
 %zu)\n", stu); }  

Thanks, I think this makes the code clearer. For consistency you could also add these:

---

(quoted diff) +0 -0

b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c index 6d64c8fc923fc..c977106357961 100644 --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -1601,7 +1601,7 @@ static void arm_smmu_handle_ppr(struct arm_smmu_device *smmu, u64 *evt)

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +1 -1

 	sid = FIELD_GET(PRIQ_0_SID, evt[0]);
 	ssv = FIELD_GET(PRIQ_0_SSID_V, evt[0]);
-	ssid = ssv ? FIELD_GET(PRIQ_0_SSID, evt[0]) : 0;
+	ssid = ssv ? FIELD_GET(PRIQ_0_SSID, evt[0]) : IOMMU_NO_PASID;
 	last = FIELD_GET(PRIQ_0_PRG_LAST, evt[0]);
 	grpid = FIELD_GET(PRIQ_1_PRG_IDX, evt[1]);

@@ -1742,7 +1742,7 @@ arm_smmu_atc_inv_to_cmd(int ssid, unsigned long

iova, size_t size, */

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +2 -3

 	*cmd = (struct arm_smmu_cmdq_ent) {
 		.opcode			= CMDQ_OP_ATC_INV,
-		.substream_valid	= !!ssid,
+		.substream_valid	= (ssid != IOMMU_NO_PASID),
 		.atc.ssid		= ssid,
 	};

@@ -1789,7 +1789,7 @@ static int arm_smmu_atc_inv_master(struct
 arm_smmu_master *master) struct arm_smmu_cmdq_ent cmd;
 	struct arm_smmu_cmdq_batch cmds;
 
-	arm_smmu_atc_inv_to_cmd(0, 0, 0, &cmd);
+	arm_smmu_atc_inv_to_cmd(IOMMU_NO_PASID, 0, 0, &cmd);
 
 	cmds.num = 0;
 	for (i = 0; i < master->num_streams; i++) {
---

With that, for SMMU:

Reviewed-by Jean-Philippe Brucker <[email protected]>

From: Jacob Pan <[email protected]>
Sent: Wednesday, May 24, 2023 1:35 AM

/* Allocate a PASID for the mm within range (inclusive) */
-static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)

drivers/iommu/iommu-sva.c +2 -3

+static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev)
 {
+	ioasid_t pasid;
 	int ret = 0;
 
-	if (min == IOMMU_PASID_INVALID ||
-	    max == IOMMU_PASID_INVALID ||
-	    min == 0 || max < min)

drivers/iommu/iommu.c +7 -0

+ioasid_t iommu_alloc_global_pasid_dev(struct device *dev)
+{
+	int ret;
+	ioasid_t max;
+
+	max = dev->iommu->max_pasids;
+	ret = ida_alloc_range(&iommu_global_pasid_ida,

IOMMU_FIRST_GLOBAL_PASID, max, GFP_KERNEL);

Quoted 19 lines

From: Jacob Pan <[email protected]>
Sent: Wednesday, May 24, 2023 1:35 AM

/* Allocate a PASID for the mm within range (inclusive) */
-static int iommu_sva_alloc_pasid(struct mm_struct *mm, ioasid_t min, ioasid_t max)
+static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev) {

drivers/iommu/iommu-sva.c +1 -5

+	ioasid_t pasid;
 	int ret = 0;
 
-	if (min == IOMMU_PASID_INVALID ||
-	    max == IOMMU_PASID_INVALID ||
-	    min == 0 || max < min)
-		return -EINVAL;
-
 	if (!arch_pgtable_dma_compat(mm))
 		return -EBUSY;

drivers/iommu/iommu-sva.c line 26

Quoted 5 lines

drivers/iommu/iommu-sva.c +1 -1

 	mutex_lock(&iommu_sva_lock);
 	/* Is a PASID already associated with this mm? */
 	if (mm_valid_pasid(mm)) {
-		if (mm->pasid < min || mm->pasid > max)
+		if (mm->pasid > dev->iommu->max_pasids)  

">" should be ">="

Quoted 8 lines

drivers/iommu/iommu.c +7 -0

+ioasid_t iommu_alloc_global_pasid_dev(struct device *dev)
+{
+	int ret;
+	ioasid_t max;
+
+	max = dev->iommu->max_pasids;
+	ret = ida_alloc_range(&iommu_global_pasid_ida,

IOMMU_FIRST_GLOBAL_PASID, max, GFP_KERNEL);

max is inclusive. Here should minus one.

otherwise looks good to me,

Reviewed-by Kevin Tian <[email protected]>

From: Jacob Pan <[email protected]>
Sent: Wednesday, May 24, 2023 1:35 AM

(quoted diff) +0 -0

@@ -1472,6 +1482,37 @@ static void iommu_flush_dev_iotlb(struct
 dmar_domain *domain,
 	spin_lock_irqsave(&domain->lock, flags);
 	list_for_each_entry(info, &domain->devices, link)
 		__iommu_flush_dev_iotlb(info, addr, mask);

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

+static void domain_flush_pasid_iotlb(struct intel_iommu *iommu,
+				     struct dmar_domain *domain, u64 addr,
+				     unsigned long npages, bool ih)
+{
+	u16 did = domain_id_iommu(domain, iommu);
+	struct dev_pasid_info *dev_pasid;
+	unsigned long flags;
+
+	spin_lock_irqsave(&domain->lock, flags);
+	list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain)
+		qi_flush_piotlb(iommu, did, dev_pasid->pasid, addr, npages,
 ih);
+
+	if (!list_empty(&domain->devices))
+		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr,

npages, ih);

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

 	spin_unlock_irqrestore(&domain->lock, flags);
 }

@@ -1492,7 +1533,7 @@ static void iommu_flush_iotlb_psi(struct
 intel_iommu *iommu,
 		ih = 1 << 6;
 
 	if (domain->use_first_level) {
-		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr, pages,
 ih);
+		domain_flush_pasid_iotlb(iommu, domain, addr, pages, ih);
 	} else {

unsigned long bitmask = aligned_pages - 1;

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

@@ -4720,25 +4762,99 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain,
 static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t
 pasid)
 {
 	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct dev_pasid_info *curr, *dev_pasid = NULL;
+	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
+	unsigned long flags;
 
-	/* Domain type specific cleanup: */
 	domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
-	if (domain) {
-		switch (domain->type) {
-		case IOMMU_DOMAIN_SVA:
-			intel_svm_remove_dev_pasid(dev, pasid);
-			break;
-		default:
-			/* should never reach here */
-			WARN_ON(1);
+	if (!domain)
+		goto out_tear_down;

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

+static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+				     struct device *dev, ioasid_t pasid)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu = info->iommu;
+	struct dev_pasid_info *dev_pasid;
+	unsigned long flags;
+	int ret;
+
+	if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+		return -EOPNOTSUPP;
+

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

+
+	ret = domain_attach_iommu(dmar_domain, iommu);
+	if (ret)
+		goto out_free;

Quoted 12 lines

From: Jacob Pan <[email protected]>
Sent: Wednesday, May 24, 2023 1:35 AM

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

@@ -1472,6 +1482,37 @@ static void iommu_flush_dev_iotlb(struct
 dmar_domain *domain,
  	spin_lock_irqsave(&domain->lock, flags);
  	list_for_each_entry(info, &domain->devices, link)
  		__iommu_flush_dev_iotlb(info, addr, mask);
+
+	list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain)
 {
+		info = dev_iommu_priv_get(dev_pasid->dev);

drivers/iommu/intel/iommu.c static void iommu_flush_dev_iotlb(struct dmar_domain *domain, · line 1493

Quoted 6 lines

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

+		qi_flush_dev_iotlb_pasid(info->iommu,
+					 PCI_DEVID(info->bus, info->devfn),
+					 info->pfsid, dev_pasid->pasid,
+					 info->ats_qdep, addr,
+					 mask);
+	}

Check info->ats_enabled instead of doing it blindly.

Quoted 13 lines

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

+static void domain_flush_pasid_iotlb(struct intel_iommu *iommu,
+				     struct dmar_domain *domain, u64 addr,
+				     unsigned long npages, bool ih)
+{
+	u16 did = domain_id_iommu(domain, iommu);
+	struct dev_pasid_info *dev_pasid;
+	unsigned long flags;
+
+	spin_lock_irqsave(&domain->lock, flags);
+	list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain)
+		qi_flush_piotlb(iommu, did, dev_pasid->pasid, addr, npages,
 ih);

Quoted 4 lines

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

+
+	if (!list_empty(&domain->devices))
+		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr,

npages, ih);

Old code doesn't have this empty list check. I'm not sure whether any corner case might exist but if you do plan to add it it's better to put it in a separate patch to allow bisect.

Quoted 8 lines

  	spin_unlock_irqrestore(&domain->lock, flags);
  }

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

@@ -1492,7 +1533,7 @@ static void iommu_flush_iotlb_psi(struct
 intel_iommu *iommu,
  		ih = 1 << 6;
 

Quoted 7 lines

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

  	if (domain->use_first_level) {
-		qi_flush_piotlb(iommu, did, IOMMU_NO_PASID, addr, pages,
 ih);
+		domain_flush_pasid_iotlb(iommu, domain, addr, pages, ih);
  	} else {

  		unsigned long bitmask = aligned_pages - 1;

Why cannot this pasid be used with a second level config?

Quoted 24 lines

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

@@ -4720,25 +4762,99 @@ static void intel_iommu_iotlb_sync_map(struct
 iommu_domain *domain,
  static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t
 pasid)
  {
  	struct intel_iommu *iommu = device_to_iommu(dev, NULL, NULL);
+	struct dev_pasid_info *curr, *dev_pasid = NULL;
+	struct dmar_domain *dmar_domain;
  	struct iommu_domain *domain;
+	unsigned long flags;
 
-	/* Domain type specific cleanup: */
  	domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0);
-	if (domain) {
-		switch (domain->type) {
-		case IOMMU_DOMAIN_SVA:
-			intel_svm_remove_dev_pasid(dev, pasid);
-			break;
-		default:
-			/* should never reach here */
-			WARN_ON(1);
+	if (!domain)
+		goto out_tear_down;

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

Quoted 10 lines

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

+
+	/*
+	 * The SVA implementation needs to stop mm notification, drain the
+	 * pending page fault requests before tearing down the pasid entry.
+	 * The VT-d spec (section 6.2.3.1) also recommends that software
+	 * could use a reserved domain id for all first-only and pass-through
+	 * translations. Hence there's no need to call
 domain_detach_iommu()
+	 * in the sva domain case.
+	 */

It's probably clearer to say:

/*
* SVA domain requires special treatment before tearing down the pasid
* entry:
* 1) pasid is stored in mm instead of in dev_pasid;
* 2) all SVA domains share a reserved domain id per recommendation
* from VT-d spec (section 6.2.3.1) so domain_detach_iommu() is
* not required;
* 3) additional cleanup is required e.g. stopping mm notification,
* draining the pending page fault requests, etc.
* Better handle it in a separate helper.
*/

Quoted 18 lines

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

+static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+				     struct device *dev, ioasid_t pasid)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu = info->iommu;
+	struct dev_pasid_info *dev_pasid;
+	unsigned long flags;
+	int ret;
+
+	if (!pasid_supported(iommu) || dev_is_real_dma_subdevice(dev))
+		return -EOPNOTSUPP;
+
+	if (context_copied(iommu, info->bus, info->devfn))
+		return -EBUSY;
+
+	ret = prepare_domain_attach_device(domain, dev);

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain, · line 4826

Quoted 6 lines

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

+	if (ret)
+		return ret;
+
+	dev_pasid = kzalloc(sizeof(*dev_pasid), GFP_KERNEL);
+	if (!dev_pasid)
+		return -ENOMEM;

should it check whether this pasid has been attached?

Quoted 10 lines

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

+
+	ret = domain_attach_iommu(dmar_domain, iommu);
+	if (ret)
+		goto out_free;
+
+	if (domain_type_is_si(dmar_domain))
+		ret = intel_pasid_setup_pass_through(iommu, dmar_domain,
+						     dev, pasid);
+	else if (dmar_domain->use_first_level)

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain, · line 4840

Quoted 5 lines

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

+		ret = domain_setup_first_level(iommu, dmar_domain,
+					       dev, pasid);
+	else
+		ret = intel_pasid_setup_second_level(iommu, dmar_domain,
+						     dev, pasid);

Here you allow attaching pasid to a domain using second-level but all prior changes are only for first-level.

From: Baolu Lu <[email protected]>
Sent: Friday, May 26, 2023 10:43 AM

Quoted 10 lines

Quoted 10 lines

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

+
+	ret = domain_attach_iommu(dmar_domain, iommu);
+	if (ret)
+		goto out_free;
+
+	if (domain_type_is_si(dmar_domain))
+		ret = intel_pasid_setup_pass_through(iommu, dmar_domain,
+						     dev, pasid);
+	else if (dmar_domain->use_first_level)

Quoted 8 lines

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain, · line 4840

Quoted 5 lines

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

+		ret = domain_setup_first_level(iommu, dmar_domain,
+					       dev, pasid);
+	else
+		ret = intel_pasid_setup_second_level(iommu, dmar_domain,
+						     dev, pasid);

Here you allow attaching pasid to a domain using second-level but all prior changes are only for first-level.

As explained, prior changes are for pasid-base iotlb invalidation for first level page table change. Or perhaps I didn't get you correctly?

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain, · line 4781

Quoted 9 lines

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

+	/*
+	 * The SVA implementation needs to stop mm notification, drain the
+	 * pending page fault requests before tearing down the pasid entry.
+	 * The VT-d spec (section 6.2.3.1) also recommends that software
+	 * could use a reserved domain id for all first-only and pass-through
+	 * translations. Hence there's no need to call

domain_detach_iommu()

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

+	 * in the sva domain case.
+	 */

It's probably clearer to say:

/*
* SVA domain requires special treatment before tearing down the pasid
* entry:
* 1) pasid is stored in mm instead of in dev_pasid;

* 2) all SVA domains share a reserved domain id per recommendation
* from VT-d spec (section 6.2.3.1) so domain_detach_iommu() is
* not required;

Quoted 11 lines

drivers/iommu/intel/iommu.c static void intel_iommu_iotlb_sync_map(struct iommu_domain *domain, · line 4781

Quoted 9 lines

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

+	/*
+	 * The SVA implementation needs to stop mm notification, drain the
+	 * pending page fault requests before tearing down the pasid entry.
+	 * The VT-d spec (section 6.2.3.1) also recommends that software
+	 * could use a reserved domain id for all first-only and pass-through
+	 * translations. Hence there's no need to call

domain_detach_iommu()

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

+	 * in the sva domain case.
+	 */

It's probably clearer to say:

Is this what is going on ??

Quoted 4 lines

/*
* SVA domain requires special treatment before tearing down the pasid
* entry:
* 1) pasid is stored in mm instead of in dev_pasid;

Why? The mm pasid should not be used by any driver code, the PASID the SVA is hooked to does NOT have to be the mm PASID.

Quoted 4 lines

* 2) all SVA domains share a reserved domain id per recommendation
* from VT-d spec (section 6.2.3.1) so domain_detach_iommu() is
* not required;

The DID should be managed and allocated for the S2 pointer and the flushing logic should work genericly by tracking the S2's being used and flushing their DIDs when all the S2s fall out of use. The special identity S2 just gets a static DID that never falls out of use.

From: Jason Gunthorpe <[email protected]>
Sent: Wednesday, May 31, 2023 1:09 AM

Quoted 3 lines

* 2) all SVA domains share a reserved domain id per recommendation
* from VT-d spec (section 6.2.3.1) so domain_detach_iommu() is
* not required;

The DID should be managed and allocated for the S2 pointer and the flushing logic should work genericly by tracking the S2's being used and flushing their DIDs when all the S2s fall out of use. The special identity S2 just gets a static DID that never falls out of use.

Kernel workqueues were disabled due to flawed use of kernel VA and SVA API. Now that we have the support for attaching PASID to the device's default domain and the ability to reserve global PASIDs from SVA APIs, we can re-enable the kernel work queues and use them under DMA API.

We also use non-privileged access for in-kernel DMA to be consistent with the IOMMU settings. Consequently, interrupt for user privilege is enabled for work completion IRQs.

Hi Joerg and all,

IDXD kernel work queues were disabled due to the flawed use of kernel VA and SVA API.
Link:
https://lore.kernel.org/linux-iommu/[email protected]/

The solution is to enable it under DMA API where IDXD shared workqueue users can use ENQCMDS to submit work on buffers mapped by DMA API.

This patchset adds support for attaching PASID to the device's default domain and the ability to allocate global PASIDs from IOMMU APIs. IDXD driver can then re-enable the kernel work queues and use them under DMA API.

This depends on the IOASID removal series. (merged)
https://lore.kernel.org/all/[email protected]/

Thanks,

Jacob

---
Changelog:
v7:
- renamed IOMMU_DEF_RID_PASID to be IOMMU_NO_PASID to be more
generic (Jean)
- simplify range checking for sva PASID (Baolu)
v6:
- use a simplified version of vt-d driver change for
set_device_pasid from Baolu.
- check and rename global PASID allocation base
v5:
- exclude two patches related to supervisor mode, taken by VT-d maintainer Baolu.
- move PASID range check into allocation API so that device
drivers only need to pass in struct device*. (Kevin)
- factor out helper functions in device-domain attach (Baolu)
- make explicit use of RID_PASID across architectures
v4:
- move dummy functions outside ifdef CONFIG_IOMMU_SVA (Baolu)
- dropped domain type check while disabling idxd system PASID
(Baolu)

v3:
- moved global PASID allocation API from SVA to IOMMU (Kevin)
- remove #ifdef around global PASID reservation during boot
(Baolu)
- remove restriction on PASID 0 allocation (Baolu)
- fix a bug in sysfs domain change when attaching devices
- clear idxd user interrupt enable bit after disabling device(
Fenghua) v2:
- refactored device PASID attach domain ops based on Baolu's
early patch
- addressed TLB flush gap
- explicitly reserve RID_PASID from SVA PASID number space
- get dma domain directly, avoid checking domain types

Jacob Pan (3):
iommu: Generalize PASID 0 for normal DMA w/o PASID
iommu: Move global PASID allocation from SVA to core dmaengine/idxd: Re-enable kernel workqueue under DMA API

Lu Baolu (1):
iommu/vt-d: Add set_dev_pasid callback for dma domain

 drivers/dma/idxd/device.c                     |  30 +---
 drivers/dma/idxd/dma.c                        |   5 +-
 drivers/dma/idxd/init.c                       |  60 ++++++-
 drivers/dma/idxd/sysfs.c                      |   7 -
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   2 +-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  10 +-
 drivers/iommu/intel/iommu.c                   | 159 +++++++++++++++---
 drivers/iommu/intel/iommu.h                   |   7 +
 drivers/iommu/intel/pasid.c                   |   2 +-
 drivers/iommu/intel/pasid.h                   |   1 -
 drivers/iommu/iommu-sva.c                     |  28 ++-
 drivers/iommu/iommu.c                         |  24 +++
 include/linux/iommu.h                         |  11 ++
 13 files changed, 261 insertions(+), 85 deletions(-)

Hi Jason,

Do you have any comments on this set? this is a follow-up of the IOASID removal series.

Quoted 4 lines · Jacob Pan

Hi Jason,

Do you have any comments on this set? this is a follow-up of the IOASID removal series.

It looks OK in general, I see it still needs another spin before the driver part is OK?