iommu/vt-d: Several misc fixes

PATCH In review Lu Baolu 1 participant 7 messages Mar 20, 2021 – Apr 6, 2021

Subsystems (2)

IOMMU Intel IOMMU (VT-d)

lore

Share & tools

lore

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #0 lore raw

v2 cover iommu/vt-d: Several misc fixes

Hi Joerg,

This series includes some misc fixes for the VT-d iommu driver. Please help to review and merge.

Best regards,
baolu

Change log:
v1->v2:
- v1: https://lore.kernel.org/linux-iommu/[email protected]/
- [PATCH 2/5] iommu/vt-d: Remove WO permissions on second-level paging entries
- Refine the commit message to make the intention clear.

Lu Baolu (5):
iommu/vt-d: Report the right page fault address iommu/vt-d: Remove WO permissions on second-level paging entries iommu/vt-d: Invalidate PASID cache when root/context entry changed iommu/vt-d: Use user privilege for RID2PASID translation iommu/vt-d: Avoid unnecessary cache flush in pasid entry teardown

 drivers/iommu/intel/iommu.c | 28 ++++++++++++++++------------
 drivers/iommu/intel/pasid.c |  6 +++++-
 drivers/iommu/intel/svm.c   |  2 +-
 include/linux/intel-iommu.h |  1 +
 4 files changed, 23 insertions(+), 14 deletions(-)

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #1 lore raw

PATCH v2 1/5 iommu/vt-d: Report the right page fault address

The Address field of the Page Request Descriptor only keeps bit [63:12] of the offending address. Convert it to a full address before reporting it to device drivers.

Fixes eb8d93ea3c1d3 ("iommu/vt-d: Report page request faults for guest SVA")

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/iommu/intel/svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 54db58945c2d..677d7f6b43bb 100644
@@ -862,7 +862,7 @@ intel_svm_prq_report(struct device *dev, struct page_req_dsc *desc)
 	/* Fill in event data for device specific processing */
 	memset(&event, 0, sizeof(struct iommu_fault_event));
 	event.fault.type = IOMMU_FAULT_PAGE_REQ;
-	event.fault.prm.addr = desc->addr;
+	event.fault.prm.addr = (u64)desc->addr << VTD_PAGE_SHIFT;
 	event.fault.prm.pasid = desc->pasid;
 	event.fault.prm.grpid = desc->prg_index;
 	event.fault.prm.perm = prq_to_iommu_prot(desc);

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #2 lore raw

PATCH v2 2/5 iommu/vt-d: Remove WO permissions on second-level paging entries

In reply to Lu Baolu

When the first level page table is used for IOVA translation, it only supports Read-Only and Read-Write permissions. The Write-Only permission is not supported as the PRESENT bit (implying Read permission) should always set. When using second level, we still give separate permissions that allows WriteOnly which seems inconsistent and awkward. We want to have consistent behavior. After moving to 1st level, we don't want things to work sometimes, and break if we use 2nd level for the same mappings. Hence remove this configuration.

Suggested-by Ashok Raj <[email protected]>

Fixes b802d070a52a1 ("iommu/vt-d: Use iova over first level")

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/iommu/intel/iommu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

index 167219ea8d70..132cbf9f214f 100644
@@ -2304,8 +2304,9 @@ __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 		return -EINVAL;
 
 	attr = prot & (DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP);
+	attr |= DMA_FL_PTE_PRESENT;
 	if (domain_use_first_level(domain)) {
-		attr |= DMA_FL_PTE_PRESENT | DMA_FL_PTE_XD | DMA_FL_PTE_US;
+		attr |= DMA_FL_PTE_XD | DMA_FL_PTE_US;
 
 		if (domain->domain.type == IOMMU_DOMAIN_DMA) {
 			attr |= DMA_FL_PTE_ACCESS;

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #3 lore raw

PATCH v2 3/5 iommu/vt-d: Invalidate PASID cache when root/context entry changed

In reply to Lu Baolu

When the Intel IOMMU is operating in the scalable mode, some information from the root and context table may be used to tag entries in the PASID cache. Software should invalidate the PASID-cache when changing root or context table entries.

Suggested-by Ashok Raj <[email protected]>

Fixes 7373a8cc38197 ("iommu/vt-d: Setup context and enable RID2PASID support")

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/iommu/intel/iommu.c | 18 +++++++++---------
 include/linux/intel-iommu.h |  1 +
 2 files changed, 10 insertions(+), 9 deletions(-)

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

index 132cbf9f214f..868f195f55ff 100644
@@ -1339,6 +1339,11 @@ static void iommu_set_root_entry(struct intel_iommu *iommu)
 		      readl, (sts & DMA_GSTS_RTPS), sts);
 
 	raw_spin_unlock_irqrestore(&iommu->register_lock, flag);
+
+	iommu->flush.flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL);
+	if (sm_supported(iommu))
+		qi_flush_pasid_cache(iommu, 0, QI_PC_GLOBAL, 0);
+	iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 }
 
 void iommu_flush_write_buffer(struct intel_iommu *iommu)

@@ -2422,6 +2427,10 @@ static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8 devfn
 				   (((u16)bus) << 8) | devfn,
 				   DMA_CCMD_MASK_NOBIT,
 				   DMA_CCMD_DEVICE_INVL);
+
+	if (sm_supported(iommu))
+		qi_flush_pasid_cache(iommu, did_old, QI_PC_ALL_PASIDS, 0);
+
 	iommu->flush.flush_iotlb(iommu,
 				 did_old,
 				 0,

@@ -3267,8 +3276,6 @@ static int __init init_dmars(void)
 		register_pasid_allocator(iommu);
 #endif
 		iommu_set_root_entry(iommu);
-		iommu->flush.flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL);
-		iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 	}
 
 #ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA

@@ -3458,12 +3465,7 @@ static int init_iommu_hw(void)
 		}
 
 		iommu_flush_write_buffer(iommu);
-
 		iommu_set_root_entry(iommu);
-
-		iommu->flush.flush_context(iommu, 0, 0, 0,
-					   DMA_CCMD_GLOBAL_INVL);
-		iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 		iommu_enable_translation(iommu);
 		iommu_disable_protect_mem_regions(iommu);
 	}

@@ -3846,8 +3848,6 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
 		goto disable_iommu;
 
 	iommu_set_root_entry(iommu);
-	iommu->flush.flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL);
-	iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
 	iommu_enable_translation(iommu);
 
 	iommu_disable_protect_mem_regions(iommu);

include/linux/intel-iommu.h +1 -0

index 1732298ce888..76f974da8ca4 100644
@@ -378,6 +378,7 @@ enum {
 /* PASID cache invalidation granu */
 #define QI_PC_ALL_PASIDS	0
 #define QI_PC_PASID_SEL		1
+#define QI_PC_GLOBAL		3
 
 #define QI_EIOTLB_ADDR(addr)	((u64)(addr) & VTD_PAGE_MASK)
 #define QI_EIOTLB_IH(ih)	(((u64)ih) << 6)

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #4 lore raw

PATCH v2 4/5 iommu/vt-d: Use user privilege for RID2PASID translation

In reply to Lu Baolu

When first-level page tables are used for IOVA translation, we use user privilege by setting U/S bit in the page table entry. This is to make it consistent with the second level translation, where the U/S enforcement is not available. Clear the SRE (Supervisor Request Enable) field in the pasid table entry of RID2PASID so that requests requesting the supervisor privilege are blocked and treated as DMA remapping faults.

Suggested-by Jacob Pan <[email protected]>

Fixes b802d070a52a1 ("iommu/vt-d: Use iova over first level")

Signed-off-by Lu Baolu <[email protected]>

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

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

index 868f195f55ff..7354f9ce47d8 100644
@@ -2494,9 +2494,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 				    struct device *dev,
 				    u32 pasid)
 {
-	int flags = PASID_FLAG_SUPERVISOR_MODE;
 	struct dma_pte *pgd = domain->pgd;
 	int agaw, level;
+	int flags = 0;
 
 	/*
 	 * Skip top levels of page tables for iommu which has

@@ -2512,7 +2512,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
 	if (level != 4 && level != 5)
 		return -EINVAL;
 
-	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
+	if (pasid != PASID_RID2PASID)
+		flags |= PASID_FLAG_SUPERVISOR_MODE;
+	if (level == 5)
+		flags |= PASID_FLAG_FL5LP;
 
 	return intel_pasid_setup_first_level(iommu, dev, (pgd_t *)pgd, pasid,
 					     domain->iommu_did[iommu->seq_id],

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

index 0bf7e0a76890..dd69df5a188a 100644
@@ -673,7 +673,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
 	 * Since it is a second level only translation setup, we should
 	 * set SRE bit as well (addresses are expected to be GPAs).
 	 */
-	pasid_set_sre(pte);
+	if (pasid != PASID_RID2PASID)
+		pasid_set_sre(pte);
 	pasid_set_present(pte);
 	pasid_flush_caches(iommu, pte, pasid, did);

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Lu Baolu

author maintainer

2021-04-06 01:07 UTC #5 lore raw

Re: [PATCH v2 4/5] iommu/vt-d: Use user privilege for RID2PASID translation

Quoted 10 lines · Lu Baolu

When first-level page tables are used for IOVA translation, we use user privilege by setting U/S bit in the page table entry. This is to make it consistent with the second level translation, where the U/S enforcement is not available. Clear the SRE (Supervisor Request Enable) field in the pasid table entry of RID2PASID so that requests requesting the supervisor privilege are blocked and treated as DMA remapping faults.
Suggested-by Jacob Pan <[email protected]>
Fixes b802d070a52a1 ("iommu/vt-d: Use iova over first level")
Signed-off-by Lu Baolu <[email protected]>

We found some devices still require SRE to be set during internal tests. I will drop this patch from my queue for v5.13 for now.

Best regards,
baolu

Quoted 47 lines

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

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

index 868f195f55ff..7354f9ce47d8 100644
@@ -2494,9 +2494,9 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
  				    struct device *dev,
  				    u32 pasid)
  {
-	int flags = PASID_FLAG_SUPERVISOR_MODE;
  	struct dma_pte *pgd = domain->pgd;
  	int agaw, level;
+	int flags = 0;
 
  	/*
  	 * Skip top levels of page tables for iommu which has

@@ -2512,7 +2512,10 @@ static int domain_setup_first_level(struct intel_iommu *iommu,
  	if (level != 4 && level != 5)
  		return -EINVAL;
 
-	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
+	if (pasid != PASID_RID2PASID)
+		flags |= PASID_FLAG_SUPERVISOR_MODE;
+	if (level == 5)
+		flags |= PASID_FLAG_FL5LP;
 
  	return intel_pasid_setup_first_level(iommu, dev, (pgd_t *)pgd, pasid,
  					     domain->iommu_did[iommu->seq_id],

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

index 0bf7e0a76890..dd69df5a188a 100644
@@ -673,7 +673,8 @@ int intel_pasid_setup_second_level(struct intel_iommu *iommu,
  	 * Since it is a second level only translation setup, we should
  	 * set SRE bit as well (addresses are expected to be GPAs).
  	 */
-	pasid_set_sre(pte);
+	if (pasid != PASID_RID2PASID)
+		pasid_set_sre(pte);
  	pasid_set_present(pte);
  	pasid_flush_caches(iommu, pte, pasid, did);

Lu Baolu

author maintainer

2021-03-20 03:03 UTC #6 lore raw

PATCH v2 5/5 iommu/vt-d: Avoid unnecessary cache flush in pasid entry teardown

In reply to Lu Baolu

When a present pasid entry is disassembled, all kinds of pasid related caches need to be flushed. But when a pasid entry is not being used (PRESENT bit not set), we don't need to do this. Check the PRESENT bit in intel_pasid_tear_down_entry() and avoid flushing caches if it's not set.

Signed-off-by Lu Baolu <[email protected]>

---
 drivers/iommu/intel/pasid.c | 3 +++
 1 file changed, 3 insertions(+)

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

index dd69df5a188a..7a73385edcc0 100644
@@ -502,6 +502,9 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
 	if (WARN_ON(!pte))
 		return;
 
+	if (!(pte->val[0] & PASID_PTE_PRESENT))
+		return;
+
 	did = pasid_get_domain_id(pte);
 	intel_pasid_clear_entry(dev, pasid, fault_ignore);

Signature · 6 lines

2.25.1

_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu