Use 1st-level for IOVA translation

PATCH Superseded Lu Baolu 2 participants 22 messages Dec 11, 2019 – Dec 17, 2019

Subsystems (2)

KVM IOMMU

lore

Share & tools

lore

Lu Baolu

author

2019-12-11 02:13 UTC #0 lore raw

v3 cover Use 1st-level for IOVA translation

Intel VT-d in scalable mode supports two types of page tables for DMA translation: the first level page table and the second level page table. The first level page table uses the same format as the CPU page table, while the second level page table keeps compatible with previous formats. The software is able to choose any one of them for DMA remapping according to the use case.

This patchset aims to move IOVA (I/O Virtual Address) translation to 1st-level page table in scalable mode. This will simplify vIOMMU (IOMMU simulated by VM hypervisor) design by using the two-stage translation, a.k.a. nested mode translation.

As Intel VT-d architecture offers caching mode, guest IOVA (GIOVA) support is currently implemented in a shadow page manner. The device simulation software, like QEMU, has to figure out GIOVA->GPA mappings and write them to a shadowed page table, which will be used by the physical IOMMU. Each time when mappings are created or destroyed in vIOMMU, the simulation software has to intervene. Hence, the changes on GIOVA->GPA could be shadowed to host.

     .-----------.
     |  vIOMMU   |
     |-----------|                 .--------------------.
     |           |IOTLB flush trap |        QEMU        |
     .-----------. (map/unmap)     |--------------------|
     |GIOVA->GPA |---------------->|    .------------.  |
     '-----------'                 |    | GIOVA->HPA |  |
     |           |                 |    '------------'  |
     '-----------'                 |                    |
                                   |                    |
                                   '--------------------'
                                                |
            <------------------------------------
            |
            v VFIO/IOMMU API
      .-----------.
      |  pIOMMU   |
      |-----------|
      |           |
      .-----------.
      |GIOVA->HPA |
      '-----------'
      |           |
      '-----------'

In VT-d 3.0, scalable mode is introduced, which offers two-level translation page tables and nested translation mode. Regards to GIOVA support, it can be simplified by 1) moving the GIOVA support over 1st-level page table to store GIOVA->GPA mapping in vIOMMU,
2) binding vIOMMU 1st level page table to the pIOMMU, 3) using pIOMMU second level for GPA->HPA translation, and 4) enable nested (a.k.a. dual-stage) translation in host. Compared with current shadow GIOVA support, the new approach makes the vIOMMU design simpler and more efficient as we only need to flush the pIOMMU IOTLB and possible device-IOTLB when an IOVA mapping in vIOMMU is torn down.

     .-----------.
     |  vIOMMU   |
     |-----------|                 .-----------.
     |           |IOTLB flush trap |   QEMU    |
     .-----------.    (unmap)      |-----------|
     |GIOVA->GPA |---------------->|           |
     '-----------'                 '-----------'
     |           |                       |
     '-----------'                       |
           <------------------------------
           |      VFIO/IOMMU          
           |  cache invalidation and  
           | guest gpd bind interfaces
           v
     .-----------.
     |  pIOMMU   |
     |-----------|
     .-----------.
     |GIOVA->GPA |<---First level
     '-----------'
     | GPA->HPA  |<---Scond level
     '-----------'
     '-----------'

This patch applies the first level page table for IOVA translation unless the DOMAIN_ATTR_NESTING domain attribution has been set. Setting of this attribution means the second level will be used to map gPA (guest physical address) to hPA (host physical address), and the mappings between gVA (guest virtual address) and gPA will be maintained by the guest with the page table address binding to host's first level.

Based-on-idea-by Ashok Raj <[email protected]>

Based-on-idea-by Kevin Tian <[email protected]>

Based-on-idea-by Liu Yi L <[email protected]>

Based-on-idea-by Jacob Pan <[email protected]>

Based-on-idea-by Sanjay Kumar <[email protected]>

Based-on-idea-by Lu Baolu <[email protected]>

Change log:

v2->v3:
- The previous version was posted here
https://lkml.org/lkml/2019/11/27/1831
- Accept Jacob's suggestion on merging two page tables.

v1->v2
- The first series was posted here
https://lkml.org/lkml/2019/9/23/297
- Use per domain page table ops to handle different page tables.
- Use first level for DMA remapping by default on both bare metal and vm guest.
- Code refine according to code review comments for v1.

Lu Baolu (6):
iommu/vt-d: Identify domains using first level page table iommu/vt-d: Add set domain DOMAIN_ATTR_NESTING attr iommu/vt-d: Add PASID_FLAG_FL5LP for first-level pasid setup iommu/vt-d: Setup pasid entries for iova over first level iommu/vt-d: Flush PASID-based iotlb for iova over first level iommu/vt-d: Use iova over first level

 drivers/iommu/dmar.c        |  41 ++++++++
 drivers/iommu/intel-iommu.c | 185 ++++++++++++++++++++++++++++++++----
 drivers/iommu/intel-pasid.c |   7 +-
 drivers/iommu/intel-pasid.h |   6 ++
 drivers/iommu/intel-svm.c   |   8 +-
 include/linux/intel-iommu.h |  12 ++-
 6 files changed, 231 insertions(+), 28 deletions(-)

Signature · 1 line

2.17.1

Lu Baolu

author

2019-12-11 02:13 UTC #1 lore raw

PATCH v3 1/6 iommu/vt-d: Identify domains using first level page table

This checks whether a domain should use the first level page table for map/unmap and marks it in the domain structure.

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

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

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

index 82b9687e82d3..c93fe716e6b0 100644
@@ -307,6 +307,14 @@ static int hw_pass_through = 1;
  */
 #define DOMAIN_FLAG_LOSE_CHILDREN		BIT(1)
 
+/*
+ * When VT-d works in the scalable mode, it allows DMA translation to
+ * happen through either first level or second level page table. This
+ * bit marks that the DMA translation for the domain goes through the
+ * first level page table, otherwise, it goes through the second level.
+ */
+#define DOMAIN_FLAG_USE_FIRST_LEVEL		BIT(2)
+
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])

@@ -1714,6 +1722,35 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
 #endif
 }
 
+/*
+ * Check and return whether first level is used by default for
+ * DMA translation. Currently, we make it off by setting
+ * first_level_support = 0, and will change it to -1 after all
+ * map/unmap paths support first level page table.
+ */
+static bool first_level_by_default(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	static int first_level_support = 0;
+
+	if (likely(first_level_support != -1))
+		return first_level_support;
+
+	first_level_support = 1;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!sm_supported(iommu) || !ecap_flts(iommu->ecap)) {
+			first_level_support = 0;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return first_level_support;
+}
+
 static struct dmar_domain *alloc_domain(int flags)
 {
 	struct dmar_domain *domain;

@@ -1725,6 +1762,8 @@ static struct dmar_domain *alloc_domain(int flags)
 	memset(domain, 0, sizeof(*domain));
 	domain->nid = NUMA_NO_NODE;
 	domain->flags = flags;
+	if (first_level_by_default())
+		domain->flags |= DOMAIN_FLAG_USE_FIRST_LEVEL;
 	domain->has_iotlb_device = false;
 	INIT_LIST_HEAD(&domain->devices);

Signature · 1 line

2.17.1

Lu Baolu

author

2019-12-11 02:13 UTC #2 lore raw

PATCH v3 2/6 iommu/vt-d: Add set domain DOMAIN_ATTR_NESTING attr

In reply to Lu Baolu

This adds the Intel VT-d specific callback of setting DOMAIN_ATTR_NESTING domain attribution. It is necessary to let the VT-d driver know that the domain represents a virtual machine which requires the IOMMU hardware to support nested translation mode. Return success if the IOMMU hardware suports nested mode, otherwise failure.

Signed-off-by Yi Sun <[email protected]>

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

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

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

index c93fe716e6b0..2b5a47584baf 100644
@@ -315,6 +315,12 @@ static int hw_pass_through = 1;
  */
 #define DOMAIN_FLAG_USE_FIRST_LEVEL		BIT(2)
 
+/*
+ * Domain represents a virtual machine which demands iommu nested
+ * translation mode support.
+ */
+#define DOMAIN_FLAG_NESTING_MODE		BIT(3)
+
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])

@@ -5635,6 +5641,24 @@ static inline bool iommu_pasid_support(void)
 	return ret;
 }
 
+static inline bool nested_mode_support(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+	bool ret = true;
+
+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd) {
+		if (!sm_supported(iommu) || !ecap_nest(iommu->ecap)) {
+			ret = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static bool intel_iommu_capable(enum iommu_cap cap)
 {
 	if (cap == IOMMU_CAP_CACHE_COHERENCY)

@@ -6013,10 +6037,42 @@ static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain,
 	return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO;
 }
 
+static int
+intel_iommu_domain_set_attr(struct iommu_domain *domain,
+			    enum iommu_attr attr, void *data)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	unsigned long flags;
+	int ret = 0;
+
+	if (domain->type != IOMMU_DOMAIN_UNMANAGED)
+		return -EINVAL;
+
+	switch (attr) {
+	case DOMAIN_ATTR_NESTING:
+		spin_lock_irqsave(&device_domain_lock, flags);
+		if (nested_mode_support() &&
+		    list_empty(&dmar_domain->devices)) {
+			dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE;
+			dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL;
+		} else {
+			ret = -ENODEV;
+		}
+		spin_unlock_irqrestore(&device_domain_lock, flags);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
 	.domain_free		= intel_iommu_domain_free,
+	.domain_set_attr	= intel_iommu_domain_set_attr,
 	.attach_dev		= intel_iommu_attach_device,
 	.detach_dev		= intel_iommu_detach_device,
 	.aux_attach_dev		= intel_iommu_aux_attach_device,

Signature · 6 lines

2.17.1

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

Lu Baolu

author

2019-12-11 02:13 UTC #3 lore raw

PATCH v3 3/6 iommu/vt-d: Add PASID_FLAG_FL5LP for first-level pasid setup

In reply to Lu Baolu

Current intel_pasid_setup_first_level() use 5-level paging for first level translation if CPUs use 5-level paging mode too. This makes sense for SVA usages since the page table is shared between CPUs and IOMMUs. But it makes no sense if we only want to use first level for IOVA translation. Add PASID_FLAG_FL5LP bit in the flags which indicates whether the 5-level paging mode should be used.

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

---
 drivers/iommu/intel-pasid.c | 7 ++-----
 drivers/iommu/intel-pasid.h | 6 ++++++
 drivers/iommu/intel-svm.c   | 8 ++++++--
 3 files changed, 14 insertions(+), 7 deletions(-)

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

index 3cb569e76642..22b30f10b396 100644
@@ -477,18 +477,15 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu,
 		pasid_set_sre(pte);
 	}
 
-#ifdef CONFIG_X86
-	/* Both CPU and IOMMU paging mode need to match */
-	if (cpu_feature_enabled(X86_FEATURE_LA57)) {
+	if (flags & PASID_FLAG_FL5LP) {
 		if (cap_5lp_support(iommu->cap)) {
 			pasid_set_flpm(pte, 1);
 		} else {
-			pr_err("VT-d has no 5-level paging support for CPU\n");
+			pr_err("No 5-level paging support for first-level\n");
 			pasid_clear_entry(pte);
 			return -EINVAL;
 		}
 	}
-#endif /* CONFIG_X86 */
 
 	pasid_set_domain_id(pte, did);
 	pasid_set_address_width(pte, iommu->agaw);

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

index fc8cd8f17de1..92de6df24ccb 100644
@@ -37,6 +37,12 @@
  */
 #define PASID_FLAG_SUPERVISOR_MODE	BIT(0)
 
+/*
+ * The PASID_FLAG_FL5LP flag Indicates using 5-level paging for first-
+ * level translation, otherwise, 4-level paging will be used.
+ */
+#define PASID_FLAG_FL5LP		BIT(1)
+
 struct pasid_dir_entry {
 	u64 val;
 };

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

index 04023033b79f..d7f2a5358900 100644
@@ -364,7 +364,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		ret = intel_pasid_setup_first_level(iommu, dev,
 				mm ? mm->pgd : init_mm.pgd,
 				svm->pasid, FLPT_DEFAULT_DID,
-				mm ? 0 : PASID_FLAG_SUPERVISOR_MODE);
+				(mm ? 0 : PASID_FLAG_SUPERVISOR_MODE) |
+				(cpu_feature_enabled(X86_FEATURE_LA57) ?
+				 PASID_FLAG_FL5LP : 0));
 		spin_unlock(&iommu->lock);
 		if (ret) {
 			if (mm)

@@ -385,7 +387,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 		ret = intel_pasid_setup_first_level(iommu, dev,
 						mm ? mm->pgd : init_mm.pgd,
 						svm->pasid, FLPT_DEFAULT_DID,
-						mm ? 0 : PASID_FLAG_SUPERVISOR_MODE);
+						(mm ? 0 : PASID_FLAG_SUPERVISOR_MODE) |
+						(cpu_feature_enabled(X86_FEATURE_LA57) ?
+						PASID_FLAG_FL5LP : 0));
 		spin_unlock(&iommu->lock);
 		if (ret) {
 			kfree(sdev);

Signature · 6 lines

2.17.1

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

Lu Baolu

author

2019-12-11 02:13 UTC #4 lore raw

PATCH v3 4/6 iommu/vt-d: Setup pasid entries for iova over first level

In reply to Lu Baolu

Intel VT-d in scalable mode supports two types of page tables for IOVA translation: first level and second level. The IOMMU driver can choose one from both for IOVA translation according to the use case. This sets up the pasid entry if a domain is selected to use the first-level page table for iova translation.

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

---
 drivers/iommu/intel-iommu.c | 48 +++++++++++++++++++++++++++++++++++--
 include/linux/intel-iommu.h | 10 ++++----
 2 files changed, 52 insertions(+), 6 deletions(-)

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

index 2b5a47584baf..83a7abf0c4f0 100644
@@ -571,6 +571,11 @@ static inline int domain_type_is_si(struct dmar_domain *domain)
 	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
 }
 
+static inline bool domain_use_first_level(struct dmar_domain *domain)
+{
+	return domain->flags & DOMAIN_FLAG_USE_FIRST_LEVEL;
+}
+
 static inline int domain_pfn_supported(struct dmar_domain *domain,
 				       unsigned long pfn)
 {

@@ -2288,6 +2293,8 @@ static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 		return -EINVAL;
 
 	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
+	if (domain_use_first_level(domain))
+		prot |= DMA_FL_PTE_PRESENT;
 
 	if (!sg) {
 		sg_res = nr_pages;

@@ -2515,6 +2522,36 @@ dmar_search_domain_by_dev_info(int segment, int bus, int devfn)
 	return NULL;
 }
 
+static int domain_setup_first_level(struct intel_iommu *iommu,
+				    struct dmar_domain *domain,
+				    struct device *dev,
+				    int pasid)
+{
+	int flags = PASID_FLAG_SUPERVISOR_MODE;
+	struct dma_pte *pgd = domain->pgd;
+	int agaw, level;
+
+	/*
+	 * Skip top levels of page tables for iommu which has
+	 * less agaw than default. Unnecessary for PT mode.
+	 */
+	for (agaw = domain->agaw; agaw > iommu->agaw; agaw--) {
+		pgd = phys_to_virt(dma_pte_addr(pgd));
+		if (!dma_pte_present(pgd))
+			return -ENOMEM;
+	}
+
+	level = agaw_to_level(agaw);
+	if (level != 4 && level != 5)
+		return -EINVAL;
+
+	flags |= (level == 5) ? PASID_FLAG_FL5LP : 0;
+
+	return intel_pasid_setup_first_level(iommu, dev, (pgd_t *)pgd, pasid,
+					     domain->iommu_did[iommu->seq_id],
+					     flags);
+}
+
 static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 						    int bus, int devfn,
 						    struct device *dev,

@@ -2614,6 +2651,9 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 		if (hw_pass_through && domain_type_is_si(domain))
 			ret = intel_pasid_setup_pass_through(iommu, domain,
 					dev, PASID_RID2PASID);
+		else if (domain_use_first_level(domain))
+			ret = domain_setup_first_level(iommu, domain, dev,
+					PASID_RID2PASID);
 		else
 			ret = intel_pasid_setup_second_level(iommu, domain,
 					dev, PASID_RID2PASID);

@@ -5369,8 +5409,12 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 		goto attach_failed;
 
 	/* Setup the PASID entry for mediated devices: */
-	ret = intel_pasid_setup_second_level(iommu, domain, dev,
-					     domain->default_pasid);
+	if (domain_use_first_level(domain))
+		ret = domain_setup_first_level(iommu, domain, dev,
+					       domain->default_pasid);
+	else
+		ret = intel_pasid_setup_second_level(iommu, domain, dev,
+						     domain->default_pasid);
 	if (ret)
 		goto table_failed;
 	spin_unlock(&iommu->lock);

include/linux/intel-iommu.h +6 -4

index aaece25c055f..66b525bad434 100644
@@ -34,10 +34,12 @@
 #define VTD_STRIDE_SHIFT        (9)
 #define VTD_STRIDE_MASK         (((u64)-1) << VTD_STRIDE_SHIFT)
 
-#define DMA_PTE_READ (1)
-#define DMA_PTE_WRITE (2)
-#define DMA_PTE_LARGE_PAGE (1 << 7)
-#define DMA_PTE_SNP (1 << 11)
+#define DMA_PTE_READ		(1)
+#define DMA_PTE_WRITE		(2)
+#define DMA_PTE_LARGE_PAGE	(1 << 7)
+#define DMA_PTE_SNP		(1 << 11)
+
+#define DMA_FL_PTE_PRESENT	(1)
 
 #define CONTEXT_TT_MULTI_LEVEL	0
 #define CONTEXT_TT_DEV_IOTLB	1

Signature · 1 line

2.17.1

Liu, Yi L

2019-12-13 09:23 UTC #5 lore raw

RE: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level

Hi Allen,

Quoted 31 lines

From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 11, 2019 10:12 AM
Subject: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Intel VT-d in scalable mode supports two types of page tables for IOVA translation:
first level and second level. The IOMMU driver can choose one from both for IOVA translation according to the use case. This sets up the pasid entry if a domain is selected to use the first-level page table for iova translation.
Signed-off-by Lu Baolu <[email protected]>
---
 drivers/iommu/intel-iommu.c | 48 +++++++++++++++++++++++++++++++++++--
 include/linux/intel-iommu.h | 10 ++++----
 2 files changed, 52 insertions(+), 6 deletions(-)
drivers/iommu/intel-iommu.c index renamed from drivers/iommu/intel-iommu.c +0 -0
2b5a47584baf..83a7abf0c4f0 100644
drivers/iommu/intel-iommu.c +3 -0
@@ -571,6 +571,11 @@ static inline int domain_type_is_si(struct dmar_domain
 *domain)
 	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;  }
 
+static inline bool domain_use_first_level(struct dmar_domain *domain) {
+	return domain->flags & DOMAIN_FLAG_USE_FIRST_LEVEL; }
+
 static inline int domain_pfn_supported(struct dmar_domain *domain,
 				       unsigned long pfn)
 {

drivers/iommu/intel-iommu.c static int __domain_mapping(struct dmar_domain · line 2298

Quoted 6 lines

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

@@ -2288,6 +2293,8 @@ static int __domain_mapping(struct dmar_domain
 *domain, unsigned long iov_pfn,
 		return -EINVAL;
 
 	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
+	if (domain_use_first_level(domain))
+		prot |= DMA_FL_PTE_PRESENT;

For DMA_PTE_SNP bit, I think there needs some work. The bit 11 of prot should be cleared when FLPT is used for IOVA.

Also, we need to set bit 63 "XD" properly. e.g. If bit 11 of prot is set, it means snoop required, then "XD" bit is "0". If bit 11 of prot is "0", it means this domain is not snooping, so you may want to set "XD" bit as "1". With such enhancement, I think IOVA over FLPT would have as less difference with IOVA over SLPT.

Regards,
Yi Liu

Lu Baolu

author

2019-12-14 03:04 UTC #6 lore raw

Re: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level

Hi Liu Yi,

Thanks for reviewing my patch.

Quoted 34 lines · Yi L

Quoted 34 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 11, 2019 10:12 AM
Subject: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Intel VT-d in scalable mode supports two types of page tables for IOVA translation:
first level and second level. The IOMMU driver can choose one from both for IOVA translation according to the use case. This sets up the pasid entry if a domain is selected to use the first-level page table for iova translation.
Signed-off-by Lu Baolu <[email protected]>
---
  drivers/iommu/intel-iommu.c | 48 +++++++++++++++++++++++++++++++++++--
  include/linux/intel-iommu.h | 10 ++++----
  2 files changed, 52 insertions(+), 6 deletions(-)
drivers/iommu/intel-iommu.c index renamed from drivers/iommu/intel-iommu.c +0 -0
2b5a47584baf..83a7abf0c4f0 100644
drivers/iommu/intel-iommu.c +3 -0
@@ -571,6 +571,11 @@ static inline int domain_type_is_si(struct dmar_domain
 *domain)
  	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;  }
 
+static inline bool domain_use_first_level(struct dmar_domain *domain) {
+	return domain->flags & DOMAIN_FLAG_USE_FIRST_LEVEL; }
+
  static inline int domain_pfn_supported(struct dmar_domain *domain,
  				       unsigned long pfn)
  {
@@ -2288,6 +2293,8 @@ static int __domain_mapping(struct dmar_domain
 *domain, unsigned long iov_pfn,
  		return -EINVAL;
 

Quoted 6 lines

drivers/iommu/intel-iommu.c static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn, · line 2297
Quoted 3 lines
drivers/iommu/intel-iommu.c +2 -0
  	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
+	if (domain_use_first_level(domain))
+		prot |= DMA_FL_PTE_PRESENT;
For DMA_PTE_SNP bit, I think there needs some work. The bit 11 of prot should be cleared when FLPT is used for IOVA.

SNP (bit 11) is only for second level. This bit is ignored for first level page table walk. We should clear this bit for first level anyway.

Quoted 6 lines

Also, we need to set bit 63 "XD" properly. e.g. If bit 11 of prot is set, it means snoop required, then "XD" bit is "0". If bit 11 of prot is "0", it means this domain is not snooping, so you may want to set "XD" bit as "1". With such enhancement, I think IOVA over FLPT would have as less difference with IOVA over SLPT.

XD (bit 63) is only for the first level, and SNP (bit 11) is only for second level, right? I think we need to always set XD bit for IOVA over FL case. thoughts?

Best regards,
baolu

Liu, Yi L

2019-12-15 09:37 UTC #7 lore raw

RE: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level

Hi Baolu,

Quoted 58 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Saturday, December 14, 2019 11:04 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Hi Liu Yi,
Thanks for reviewing my patch.
Quoted 7 lines · Yi L
Quoted 7 lines
From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 11, 2019 10:12 AM
Subject: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Intel VT-d in scalable mode supports two types of page tables for IOVA
translation:
Quoted 3 lines
Quoted 3 lines
first level and second level. The IOMMU driver can choose one from both for IOVA translation according to the use case. This sets up the pasid entry if a domain is selected to use the first-level page table for iova
translation.
Quoted 27 lines
Quoted 27 lines
Signed-off-by Lu Baolu <[email protected]>
---
  drivers/iommu/intel-iommu.c | 48 +++++++++++++++++++++++++++++++++++--
  include/linux/intel-iommu.h | 10 ++++----
  2 files changed, 52 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/iommu/intel-iommu.c index
2b5a47584baf..83a7abf0c4f0 100644
drivers/iommu/intel-iommu.c +0 -0
@@ -571,6 +571,11 @@ static inline int domain_type_is_si(struct
dmar_domain
*domain)
return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY; }
drivers/iommu/intel-iommu.c +3 -0
+static inline bool domain_use_first_level(struct dmar_domain *domain) {
+	return domain->flags & DOMAIN_FLAG_USE_FIRST_LEVEL; }
+
  static inline int domain_pfn_supported(struct dmar_domain *domain,
  				       unsigned long pfn)
  {
@@ -2288,6 +2293,8 @@ static int __domain_mapping(struct dmar_domain
 *domain, unsigned long iov_pfn,
  		return -EINVAL;
 
Quoted 6 lines
drivers/iommu/intel-iommu.c static int __domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn, · line 2297
Quoted 3 lines
drivers/iommu/intel-iommu.c +2 -0
  	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
+	if (domain_use_first_level(domain))
+		prot |= DMA_FL_PTE_PRESENT;
For DMA_PTE_SNP bit, I think there needs some work. The bit 11 of prot should be cleared when FLPT is used for IOVA.

Quoted 2 lines

SNP (bit 11) is only for second level. This bit is ignored for first level page table walk. We should clear this bit for first level anyway.

I think this is what I meant above? This patch somehow misses the operation to clear the bit 11.

Quoted 9 lines

Quoted 6 lines
Also, we need to set bit 63 "XD" properly. e.g. If bit 11 of prot is set, it means snoop required, then "XD" bit is "0". If bit 11 of prot is "0", it means this domain is not snooping, so you may want to set "XD" bit as "1". With such enhancement, I think IOVA over FLPT would have as less difference with IOVA over SLPT.
XD (bit 63) is only for the first level, and SNP (bit 11) is only for second level, right? I think we need to always set XD bit for IOVA over FL case. thoughts?

Oops, I made a mistake here. Please forget SNP bit, there is no way to control SNP with first level page table.:-)

Actually, it is execute (bit 1) of second level page table which I wanted to say. If software sets R/W/X permission to an IOVA, with IOVA over second level page table, it will set bit 1. However, if IOVA is over first level page table, it may need to clear XD bit. This is what I want to say here. If IOVA doesn’t allow execute permission, it's ok to always set XD bit for IOVA over FL case. But I would like to do it just as what we did for R/W permission. R/W permission relies on the permission configured by the page map caller. right?

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

Lu Baolu

author

2019-12-17 02:04 UTC #8 lore raw

Re: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level

Hi Yi,

Quoted 5 lines · Yi L

Quoted 2 lines
XD (bit 63) is only for the first level, and SNP (bit 11) is only for second level, right? I think we need to always set XD bit for IOVA over FL case. thoughts?
Oops, I made a mistake here. Please forget SNP bit, there is no way to control SNP with first level page table.:-)

Quoted 7 lines

Actually, it is execute (bit 1) of second level page table which I wanted to say. If software sets R/W/X permission to an IOVA, with IOVA over second level page table, it will set bit 1. However, if IOVA is over first level page table, it may need to clear XD bit. This is what I want to say here. If IOVA doesn’t allow execute permission, it's ok to always set XD bit for IOVA over FL case. But I would like to do it just as what we did for R/W permission. R/W permission relies on the permission configured by the page map caller. right?

Got your point.

Current driver always cleard X (bit 2) in the second level page table. So we will always set XD bit (bit 63) in the first level page table. If we decide to use the X permission, we need a separated patch, right?

Best regards,
baolu

Liu, Yi L

2019-12-17 02:33 UTC #9 lore raw

RE: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level

Quoted 23 lines

From: Lu Baolu < [email protected] >
Sent: Tuesday, December 17, 2019 10:04 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 4/6] iommu/vt-d: Setup pasid entries for iova over first level
Hi Yi,
Quoted 2 lines · Yi L
Quoted 2 lines
XD (bit 63) is only for the first level, and SNP (bit 11) is only for second level, right? I think we need to always set XD bit for IOVA over FL case.
thoughts?
Quoted 10 lines
Oops, I made a mistake here. Please forget SNP bit, there is no way to control SNP with first level page table.:-)
Actually, it is execute (bit 1) of second level page table which I wanted to say. If software sets R/W/X permission to an IOVA, with IOVA over second level page table, it will set bit 1. However, if IOVA is over first level page table, it may need to clear XD bit. This is what I want to say here. If IOVA doesn’t allow execute permission, it's ok to always set XD bit for IOVA over FL case. But I would like to do it just as what we did for R/W permission. R/W permission relies on the permission

Quoted 6 lines

configured by the page map caller. right?
Got your point.
Current driver always cleard X (bit 2) in the second level page table. So we will always set XD bit (bit 63) in the first level page table.

yes, I also noticed X (bit 2) is not used in intel-iommu driver. So I know why you set XD for IOVA over FL case. But it's a little bit weird to hard code it. That's why I suggested to relay page map caller's permission input.

Quoted 1 line

If we decide to use the X permission, we need a separated patch, right?

sure, it would be a separate patch since current code doesn’t apply X permission.

Regards,
Yi Liu

Lu Baolu

author

2019-12-11 02:13 UTC #10 lore raw

PATCH v3 5/6 iommu/vt-d: Flush PASID-based iotlb for iova over first level

In reply to Lu Baolu

When software has changed first-level tables, it should invalidate the affected IOTLB and the paging-structure-caches using the PASID- based-IOTLB Invalidate Descriptor defined in spec 6.5.2.4.

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

---
 drivers/iommu/dmar.c        | 41 ++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-iommu.c | 44 ++++++++++++++++++++++++-------------
 include/linux/intel-iommu.h |  2 ++
 3 files changed, 72 insertions(+), 15 deletions(-)

drivers/iommu/dmar.c +41 -0

index 3acfa6a25fa2..fb30d5053664 100644
@@ -1371,6 +1371,47 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
 	qi_submit_sync(&desc, iommu);
 }
 
+/* PASID-based IOTLB invalidation */
+void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
+		     unsigned long npages, bool ih)
+{
+	struct qi_desc desc = {.qw2 = 0, .qw3 = 0};
+
+	/*
+	 * npages == -1 means a PASID-selective invalidation, otherwise,
+	 * a positive value for Page-selective-within-PASID invalidation.
+	 * 0 is not a valid input.
+	 */
+	if (WARN_ON(!npages)) {
+		pr_err("Invalid input npages = %ld\n", npages);
+		return;
+	}
+
+	if (npages == -1) {
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = 0;
+	} else {
+		int mask = ilog2(__roundup_pow_of_two(npages));
+		unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
+
+		if (WARN_ON_ONCE(!ALIGN(addr, align)))
+			addr &= ~(align - 1);
+
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = QI_EIOTLB_ADDR(addr) |
+				QI_EIOTLB_IH(ih) |
+				QI_EIOTLB_AM(mask);
+	}
+
+	qi_submit_sync(&desc, iommu);
+}
+
 /*
  * Disable Queued Invalidation interface.
  */

drivers/iommu/intel-iommu.c +29 ~~-15~~

62 lines not on this page · raw message

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

index 66b525bad434..b693540e4b4f 100644
@@ -648,6 +648,8 @@ extern void qi_flush_iotlb(struct intel_iommu *iommu, u16 did, u64 addr,
 			  unsigned int size_order, u64 type);
 extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 pfsid,
 			u16 qdep, u64 addr, unsigned mask);
+void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
+		     unsigned long npages, bool ih);
 extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
 
 extern int dmar_ir_support(void);

Signature · 6 lines

2.17.1

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

Liu, Yi L

2019-12-13 20:36 UTC #11 lore raw

RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi Allen,

Quoted 86 lines

From: [email protected] [mailto:[email protected]] On Behalf Of Lu Baolu
Sent: Wednesday, December 11, 2019 10:12 AM
To: Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>;
Subject: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

When software has changed first-level tables, it should invalidate the affected IOTLB and the paging-structure-caches using the PASID- based-IOTLB Invalidate Descriptor defined in spec 6.5.2.4.

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

---
 drivers/iommu/dmar.c        | 41 ++++++++++++++++++++++++++++++++++
 drivers/iommu/intel-iommu.c | 44 ++++++++++++++++++++++++-------------
 include/linux/intel-iommu.h |  2 ++
 3 files changed, 72 insertions(+), 15 deletions(-)

drivers/iommu/dmar.c +41 -0

index 3acfa6a25fa2..fb30d5053664 100644
@@ -1371,6 +1371,47 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16
 sid, u16 pfsid,
 	qi_submit_sync(&desc, iommu);
 }
 
+/* PASID-based IOTLB invalidation */
+void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
+		     unsigned long npages, bool ih)
+{
+	struct qi_desc desc = {.qw2 = 0, .qw3 = 0};
+
+	/*
+	 * npages == -1 means a PASID-selective invalidation, otherwise,
+	 * a positive value for Page-selective-within-PASID invalidation.
+	 * 0 is not a valid input.
+	 */
+	if (WARN_ON(!npages)) {
+		pr_err("Invalid input npages = %ld\n", npages);
+		return;
+	}
+
+	if (npages == -1) {
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = 0;
+	} else {
+		int mask = ilog2(__roundup_pow_of_two(npages));
+		unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
+
+		if (WARN_ON_ONCE(!ALIGN(addr, align)))
+			addr &= ~(align - 1);
+
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = QI_EIOTLB_ADDR(addr) |
+				QI_EIOTLB_IH(ih) |
+				QI_EIOTLB_AM(mask);
+	}
+
+	qi_submit_sync(&desc, iommu);
+}
+
 /*
  * Disable Queued Invalidation interface.

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

index 83a7abf0c4f0..e47f5fe37b59 100644
@@ -1520,18 +1520,24 @@ static void iommu_flush_iotlb_psi(struct intel_iommu
 *iommu,
 
 	if (ih)
 		ih = 1 << 6;
-	/*
-	 * Fallback to domain selective flush if no PSI support or the size is
-	 * too big.
-	 * PSI requires page size to be 2 ^ x, and the base address is naturally
-	 * aligned to the size
-	 */
-	if (!cap_pgsel_inv(iommu->cap) || mask > cap_max_amask_val(iommu-

drivers/iommu/intel-iommu.c static void iommu_flush_iotlb_psi(struct intel_iommu · line 1528

Quoted 10 lines

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

 >cap))
-		iommu->flush.flush_iotlb(iommu, did, 0, 0,
-						DMA_TLB_DSI_FLUSH);
-	else
-		iommu->flush.flush_iotlb(iommu, did, addr | ih, mask,
-						DMA_TLB_PSI_FLUSH);
+
+	if (domain_use_first_level(domain)) {
+		qi_flush_piotlb(iommu, did, domain->default_pasid,
+				addr, pages, ih);

I'm not sure if my understanding is correct. But let me tell a story. Assuming we assign a mdev and a PF/VF to a single VM, then there will be p_iotlb tagged with PASID_RID2PASID and p_iotlb tagged with default_pasid. We may want to flush both... If this operation is invoked per-device, then need to pass in a hint to indicate whether to use PASID_RID2PASID or default_pasid, or you may just issue two flush with the two PASID values. Thoughts?

Regards,
Yi Liu

Lu Baolu

author

2019-12-14 03:25 UTC #12 lore raw

Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi Liu Yi,

Quoted 87 lines · Yi L

Quoted 87 lines

When software has changed first-level tables, it should invalidate the affected IOTLB and the paging-structure-caches using the PASID- based-IOTLB Invalidate Descriptor defined in spec 6.5.2.4.

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

---
  drivers/iommu/dmar.c        | 41 ++++++++++++++++++++++++++++++++++
  drivers/iommu/intel-iommu.c | 44 ++++++++++++++++++++++++-------------
  include/linux/intel-iommu.h |  2 ++
  3 files changed, 72 insertions(+), 15 deletions(-)

drivers/iommu/dmar.c +41 -0

index 3acfa6a25fa2..fb30d5053664 100644
@@ -1371,6 +1371,47 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16
 sid, u16 pfsid,
  	qi_submit_sync(&desc, iommu);
  }
 
+/* PASID-based IOTLB invalidation */
+void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
+		     unsigned long npages, bool ih)
+{
+	struct qi_desc desc = {.qw2 = 0, .qw3 = 0};
+
+	/*
+	 * npages == -1 means a PASID-selective invalidation, otherwise,
+	 * a positive value for Page-selective-within-PASID invalidation.
+	 * 0 is not a valid input.
+	 */
+	if (WARN_ON(!npages)) {
+		pr_err("Invalid input npages = %ld\n", npages);
+		return;
+	}
+
+	if (npages == -1) {
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = 0;
+	} else {
+		int mask = ilog2(__roundup_pow_of_two(npages));
+		unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
+
+		if (WARN_ON_ONCE(!ALIGN(addr, align)))
+			addr &= ~(align - 1);
+
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = QI_EIOTLB_ADDR(addr) |
+				QI_EIOTLB_IH(ih) |
+				QI_EIOTLB_AM(mask);
+	}
+
+	qi_submit_sync(&desc, iommu);
+}
+
  /*
   * Disable Queued Invalidation interface.

*/

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

index 83a7abf0c4f0..e47f5fe37b59 100644
@@ -1520,18 +1520,24 @@ static void iommu_flush_iotlb_psi(struct intel_iommu
 *iommu,
 
  	if (ih)
  		ih = 1 << 6;
-	/*
-	 * Fallback to domain selective flush if no PSI support or the size is
-	 * too big.
-	 * PSI requires page size to be 2 ^ x, and the base address is naturally
-	 * aligned to the size
-	 */
-	if (!cap_pgsel_inv(iommu->cap) || mask > cap_max_amask_val(iommu-
 > cap))

Quoted 14 lines

drivers/iommu/intel-iommu.c static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, · line 1526
Quoted 9 lines
drivers/iommu/intel-iommu.c +4 -5
-		iommu->flush.flush_iotlb(iommu, did, 0, 0,
-						DMA_TLB_DSI_FLUSH);
-	else
-		iommu->flush.flush_iotlb(iommu, did, addr | ih, mask,
-						DMA_TLB_PSI_FLUSH);
+
+	if (domain_use_first_level(domain)) {
+		qi_flush_piotlb(iommu, did, domain->default_pasid,
+				addr, pages, ih);
I'm not sure if my understanding is correct. But let me tell a story. Assuming we assign a mdev and a PF/VF to a single VM, then there will be p_iotlb tagged with PASID_RID2PASID and p_iotlb tagged with default_pasid. We may want to flush both... If this operation is

I assume that SRIOV and SIOV are exclusive. You can't enable both SRIOV and SIOV on a single device. So the mdev and PF/VF are from different devices, right?

Or, in SRIOV case, you can wrap a PF or VF as a mediated device. But this mdev still be backed with a pasid of RID2PASID.

Quoted 3 lines

invoked per-device, then need to pass in a hint to indicate whether to use PASID_RID2PASID or default_pasid, or you may just issue two flush with the two PASID values. Thoughts?

This is per-domain and each domain has specific domain id and default pasid (assume default domain is 0 in RID2PASID case).

Best regards,
baolu

Liu, Yi L

2019-12-15 09:23 UTC #13 lore raw

RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi Baolu,

Please check replies below:

Quoted 118 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Saturday, December 14, 2019 11:24 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Hi Liu Yi,
Quoted 1 line · Yi L
Quoted 1 line
From: [email protected] [mailto:[email protected]] On
Behalf
Quoted 3 lines
Quoted 3 lines
Of Lu Baolu
Sent: Wednesday, December 11, 2019 10:12 AM
To: Joerg Roedel <[email protected]>; David Woodhouse
<[email protected]>;
Quoted 1 line
Quoted 1 line
Subject: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first
level
Quoted 12 lines
Quoted 12 lines
When software has changed first-level tables, it should invalidate the affected IOTLB and the paging-structure-caches using the PASID- based-IOTLB Invalidate Descriptor defined in spec 6.5.2.4.
Signed-off-by Lu Baolu <[email protected]>
---
  drivers/iommu/dmar.c        | 41 ++++++++++++++++++++++++++++++++++
  drivers/iommu/intel-iommu.c | 44 ++++++++++++++++++++++++-------------
  include/linux/intel-iommu.h |  2 ++
  3 files changed, 72 insertions(+), 15 deletions(-)
Quoted 5 lines
drivers/iommu/dmar.c void qi_flush_dev_iotlb(struct intel_iommu *iommu,
Quoted 5 lines
drivers/iommu/dmar.c +0 -0
index 3acfa6a25fa2..fb30d5053664 100644
@@ -1371,6 +1371,47 @@ void qi_flush_dev_iotlb(struct intel_iommu *iommu,
u16
Quoted 45 lines
Quoted 45 lines
drivers/iommu/dmar.c +41 -0
 sid, u16 pfsid,
  	qi_submit_sync(&desc, iommu);
  }
 
+/* PASID-based IOTLB invalidation */
+void qi_flush_piotlb(struct intel_iommu *iommu, u16 did, u32 pasid, u64 addr,
+		     unsigned long npages, bool ih)
+{
+	struct qi_desc desc = {.qw2 = 0, .qw3 = 0};
+
+	/*
+	 * npages == -1 means a PASID-selective invalidation, otherwise,
+	 * a positive value for Page-selective-within-PASID invalidation.
+	 * 0 is not a valid input.
+	 */
+	if (WARN_ON(!npages)) {
+		pr_err("Invalid input npages = %ld\n", npages);
+		return;
+	}
+
+	if (npages == -1) {
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_NONG_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = 0;
+	} else {
+		int mask = ilog2(__roundup_pow_of_two(npages));
+		unsigned long align = (1ULL << (VTD_PAGE_SHIFT + mask));
+
+		if (WARN_ON_ONCE(!ALIGN(addr, align)))
+			addr &= ~(align - 1);
+
+		desc.qw0 = QI_EIOTLB_PASID(pasid) |
+				QI_EIOTLB_DID(did) |
+				QI_EIOTLB_GRAN(QI_GRAN_PSI_PASID) |
+				QI_EIOTLB_TYPE;
+		desc.qw1 = QI_EIOTLB_ADDR(addr) |
+				QI_EIOTLB_IH(ih) |
+				QI_EIOTLB_AM(mask);
+	}
+
+	qi_submit_sync(&desc, iommu);
+}
+
Quoted 8 lines
drivers/iommu/intel-iommu.c static void iommu_flush_iotlb_psi(struct
Quoted 8 lines
drivers/iommu/dmar.c +0 -0
  /*
   * Disable Queued Invalidation interface.
   */
drivers/iommu/intel-iommu.c +0 -0
index 83a7abf0c4f0..e47f5fe37b59 100644
@@ -1520,18 +1520,24 @@ static void iommu_flush_iotlb_psi(struct
intel_iommu
Quoted 12 lines
Quoted 12 lines
drivers/iommu/intel-iommu.c +0 -7
 *iommu,
 
  	if (ih)
  		ih = 1 << 6;
-	/*
-	 * Fallback to domain selective flush if no PSI support or the size is
-	 * too big.
-	 * PSI requires page size to be 2 ^ x, and the base address is naturally
-	 * aligned to the size
-	 */
-	if (!cap_pgsel_inv(iommu->cap) || mask > cap_max_amask_val(iommu-
 > cap))
Quoted 14 lines
drivers/iommu/intel-iommu.c static void iommu_flush_iotlb_psi(struct intel_iommu *iommu, · line 1526
Quoted 9 lines
drivers/iommu/intel-iommu.c +4 -5
-		iommu->flush.flush_iotlb(iommu, did, 0, 0,
-						DMA_TLB_DSI_FLUSH);
-	else
-		iommu->flush.flush_iotlb(iommu, did, addr | ih, mask,
-						DMA_TLB_PSI_FLUSH);
+
+	if (domain_use_first_level(domain)) {
+		qi_flush_piotlb(iommu, did, domain->default_pasid,
+				addr, pages, ih);
I'm not sure if my understanding is correct. But let me tell a story. Assuming we assign a mdev and a PF/VF to a single VM, then there will be p_iotlb tagged with PASID_RID2PASID and p_iotlb tagged with default_pasid. We may want to flush both... If this operation is

Quoted 2 lines

I assume that SRIOV and SIOV are exclusive. You can't enable both SRIOV and SIOV on a single device.

yes, but I'm not talking use them on a single device...

Quoted 2 lines

So the mdev and PF/VF are from different
devices, right?

yes, the case I mentioned above is: a mdev from a device (say devA), and another device (say devB). Create mdev on devA and assign it to a VM together with devB.

Quoted 3 lines

Or, in SRIOV case, you can wrap a PF or VF as a mediated device. But this mdev still be backed with a pasid of RID2PASID.

My comment has no business with wrapping PF/VR as mdev...

Quoted 7 lines

Quoted 4 lines
invoked per-device, then need to pass in a hint to indicate whether to use PASID_RID2PASID or default_pasid, or you may just issue two flush with the two PASID values. Thoughts?
This is per-domain and each domain has specific domain id and default pasid (assume default domain is 0 in RID2PASID case).

Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right? If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?
Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.

Regards,
Yi Liu

Lu Baolu

author

2019-12-17 01:20 UTC #14 lore raw

Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi Yi,

Quoted 2 lines · Yi L

Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right?

No exactly. Each domain has a specific default pasid, no matter normal domain (RID based) or aux-domain (PASID based). The difference is for a normal domain RID2PASID value is used, for an aux-domain the pasid is allocated from a global pool.

The same concept used in VT-d 3.x scalable mode. For RID based DMA translation RID2PASID value is used when walking the tables; For PASID based DMA translation a real pasid in the transaction is used.

Quoted 6 lines

If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?

A domain's counterpart is IOMMU group. So we say attach/detach domain to/from devices in a group. We don't allow devices with different default pasid sitting in a same group, right?

Quoted 3 lines

Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as

No. We will have two IOMMU groups and two domains. Correct me if my understanding is not right.

Best regards,
baolu

Quoted 8 lines

normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.
Regards,
Yi Liu

Lu Baolu

author

2019-12-17 01:38 UTC #15 lore raw

Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi again,

Quoted 26 lines · Lu Baolu

Hi Yi,
Quoted 2 lines · Yi L
Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right?
No exactly. Each domain has a specific default pasid, no matter normal domain (RID based) or aux-domain (PASID based). The difference is for a normal domain RID2PASID value is used, for an aux-domain the pasid is allocated from a global pool.
The same concept used in VT-d 3.x scalable mode. For RID based DMA translation RID2PASID value is used when walking the tables; For PASID based DMA translation a real pasid in the transaction is used.
Quoted 6 lines
If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?
A domain's counterpart is IOMMU group. So we say attach/detach domain to/from devices in a group. We don't allow devices with different default pasid sitting in a same group, right?

Quoted 6 lines

Quoted 3 lines
Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as
No. We will have two IOMMU groups and two domains. Correct me if my understanding is not right.

Reconsidered this. Unfortunately, my understanding is not right. :-(

A single domain could be attached to multiple IOMMU groups. So it comes to the issue you concerned. Do I understand it right?

Quoted 6 lines

Quoted 6 lines
normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.

You are right. I will change it accordingly. The logic should look like:

if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()

Does this work for you? Thanks for catching this!

Best regards,
baolu

Lu Baolu

author

2019-12-17 01:40 UTC #16 lore raw

Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi,

Quoted 8 lines · Lu Baolu

You are right. I will change it accordingly. The logic should look like:
if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()

Both! so no "else" here.

Best regards,
baolu

Quoted 1 line

Does this work for you? Thanks for catching this!

Liu, Yi L

2019-12-17 02:44 UTC #17 lore raw

RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Quoted 14 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Tuesday, December 17, 2019 9:39 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Hi,
Quoted 3 lines · Lu Baolu
You are right. I will change it accordingly. The logic should look like:

Quoted 7 lines

Quoted 5 lines
if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()
Both! so no "else" here.

aha, if we want to flush more precisely, we may check whether domain->default_pasid is allocated. If not, it means no mdev is involved, then we may save a p_iotlb_inv_dsc submission and also save VT-d hardware circles from doing useless flush. This is just optimization, it's up to you to pick it or not in this patchset. I'm fine with flush "both" since it guarantees correctness.

Regards,
Yi Liu

Liu, Yi L

2019-12-17 02:26 UTC #18 lore raw

RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

In reply to Lu Baolu

Quoted 41 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Tuesday, December 17, 2019 9:37 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Hi again,
Quoted 30 lines · Lu Baolu
Hi Yi,
Quoted 2 lines · Yi L
Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right?
No exactly. Each domain has a specific default pasid, no matter normal domain (RID based) or aux-domain (PASID based). The difference is for a normal domain RID2PASID value is used, for an aux-domain the pasid is allocated from a global pool.
The same concept used in VT-d 3.x scalable mode. For RID based DMA translation RID2PASID value is used when walking the tables; For PASID based DMA translation a real pasid in the transaction is used.
Quoted 6 lines
If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?
A domain's counterpart is IOMMU group. So we say attach/detach domain to/from devices in a group. We don't allow devices with different default pasid sitting in a same group, right?
Quoted 3 lines
Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as

Quoted 7 lines

Quoted 2 lines
No. We will have two IOMMU groups and two domains. Correct me if my understanding is not right.
Reconsidered this. Unfortunately, my understanding is not right. :-(
A single domain could be attached to multiple IOMMU groups. So it comes to the issue you concerned. Do I understand it right?

yes. Device within the same group has no such issue since such devices are not able to enabled aux-domain. Now our understanding are aligned. :-)

Quoted 10 lines

Quoted 6 lines
Quoted 6 lines
normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.
You are right. I will change it accordingly. The logic should look like:

Quoted 6 lines

if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()
Does this work for you? Thanks for catching this!

If no else, it would work for scalable mode. ^_^ I noticed you've already corrected by yourself in another reply. :-) Look forward to your next version.

Regards,
Yi Liu

Liu, Yi L

2019-12-17 02:36 UTC #19 lore raw

RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Quoted 76 lines

From: Liu, Yi L <[email protected]>
Sent: Tuesday, December 17, 2019 10:26 AM
To: Lu Baolu <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Quoted 41 lines
From: Lu Baolu [mailto:[email protected]]
Sent: Tuesday, December 17, 2019 9:37 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Hi again,
Quoted 30 lines · Lu Baolu
Hi Yi,
Quoted 2 lines · Yi L
Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right?
No exactly. Each domain has a specific default pasid, no matter normal domain (RID based) or aux-domain (PASID based). The difference is for a normal domain RID2PASID value is used, for an aux-domain the pasid is allocated from a global pool.
The same concept used in VT-d 3.x scalable mode. For RID based DMA translation RID2PASID value is used when walking the tables; For PASID based DMA translation a real pasid in the transaction is used.
Quoted 6 lines
If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?
A domain's counterpart is IOMMU group. So we say attach/detach domain to/from devices in a group. We don't allow devices with different default pasid sitting in a same group, right?
Quoted 3 lines
Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as
Quoted 7 lines
Quoted 2 lines
No. We will have two IOMMU groups and two domains. Correct me if my understanding is not right.
Reconsidered this. Unfortunately, my understanding is not right. :-(
A single domain could be attached to multiple IOMMU groups. So it comes to the issue you concerned. Do I understand it right?
yes. Device within the same group has no such issue since such devices are not able to enabled aux-domain. Now our understanding are aligned. :-)
Quoted 15 lines
Quoted 6 lines
Quoted 6 lines
normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.
You are right. I will change it accordingly. The logic should look like:
if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()

Quoted 5 lines

Quoted 1 line
Does this work for you? Thanks for catching this!
If no else, it would work for scalable mode. ^_^ I noticed you've already corrected by yourself in another reply. :-) Look forward to your next version.

BTW. The discussion in this thread may apply to other cache flush in your series. Please have a check. At least, there are two places which need to be updated in this single patch.

Regards,
Yi Liu

Lu Baolu

author

2019-12-17 04:13 UTC #20 lore raw

Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level

Hi,

Quoted 69 lines · Yi L

Quoted 69 lines
From: Liu, Yi L <[email protected]>
Sent: Tuesday, December 17, 2019 10:26 AM
To: Lu Baolu <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: RE: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Quoted 38 lines
From: Lu Baolu [mailto:[email protected]]
Sent: Tuesday, December 17, 2019 9:37 AM
To: Liu, Yi L <[email protected]>; Joerg Roedel <[email protected]>; David Woodhouse <[email protected]>; Alex Williamson <[email protected]>
Subject: Re: [PATCH v3 5/6] iommu/vt-d: Flush PASID-based iotlb for iova over first level
Hi again,
Quoted 27 lines · Lu Baolu
Hi Yi,
Quoted 2 lines · Yi L
Ok, let me explain more... default pasid is meaningful only when the domain has been attached to a device as an aux-domain. right?
No exactly. Each domain has a specific default pasid, no matter normal domain (RID based) or aux-domain (PASID based). The difference is for a normal domain RID2PASID value is used, for an aux-domain the pasid is allocated from a global pool.
The same concept used in VT-d 3.x scalable mode. For RID based DMA translation RID2PASID value is used when walking the tables; For PASID based DMA translation a real pasid in the transaction is used.
Quoted 6 lines
If a domain only has one device, and it is attached to this device as normal domain (normal domain means non aux-domain here). Then you should flush cache with domain-id and RID2PASID value. If a domain has one device, and it is attached to this device as aux-domain. Then you may want to flush cache with domain-id and default pasid. right?
A domain's counterpart is IOMMU group. So we say attach/detach domain to/from devices in a group. We don't allow devices with different default pasid sitting in a same group, right?
Quoted 3 lines
Then let's come to the case I mentioned in previous email. a mdev and another device assigned to a single VM. In host, you will have a domain which has two devices, one device(deva) is attached as
Quoted 6 lines
Quoted 2 lines
No. We will have two IOMMU groups and two domains. Correct me if my understanding is not right.
Reconsidered this. Unfortunately, my understanding is not right. :-(
A single domain could be attached to multiple IOMMU groups. So it comes to the issue you concerned. Do I understand it right?
yes. Device within the same group has no such issue since such devices are not able to enabled aux-domain. Now our understanding are aligned. :-)
Quoted 13 lines
Quoted 5 lines
Quoted 5 lines
normal domain, another one (devB) is attached as aux-domain. Then which pasid should be used when the mapping in IOVA page table is modified? RID2PASID or default pasid? I think both should be used since the domain means differently to the two devices. If you just use default pasid, then deva may still be able to use stale caches.
You are right. I will change it accordingly. The logic should look like:
if (domain attached to physical device)
flush_piotlb_with_RID2PASID()
else if (domain_attached_to_mdev_device)
flush_piotlb_with_default_pasid()

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Does this work for you? Thanks for catching this!
If no else, it would work for scalable mode. ^_^ I noticed you've already corrected by yourself in another reply. :-) Look forward to your next version.
BTW. The discussion in this thread may apply to other cache flush in your series. Please have a check. At least, there are two places which need to be updated in this single patch.

Sure. I will.

Best regards,

baolu

Quoted 3 lines

Regards,
Yi Liu

Lu Baolu

author

2019-12-11 02:13 UTC #21 lore raw

PATCH v3 6/6 iommu/vt-d: Use iova over first level

In reply to Lu Baolu

After we make all map/unmap paths support first level page table. Let's turn it on if hardware supports scalable mode.

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

---
 drivers/iommu/intel-iommu.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

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

index e47f5fe37b59..9228f121a040 100644
@@ -1749,15 +1749,13 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
 
 /*
  * Check and return whether first level is used by default for
- * DMA translation. Currently, we make it off by setting
- * first_level_support = 0, and will change it to -1 after all
- * map/unmap paths support first level page table.
+ * DMA translation.
  */
 static bool first_level_by_default(void)
 {
 	struct dmar_drhd_unit *drhd;
 	struct intel_iommu *iommu;
-	static int first_level_support = 0;
+	static int first_level_support = -1;
 
 	if (likely(first_level_support != -1))
 		return first_level_support;

Signature · 6 lines

2.17.1

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