Use 1st-level for DMA remapping in guest

This adds functions to manipulate first level page tables which could be used by a scalale mode capable IOMMU unit.

intel_mmmap_range(domain, addr, end, phys_addr, prot)

- Map an iova range of [addr, end) to the physical memory started at @phys_addr with the @prot permissions.

intel_mmunmap_range(domain, addr, end)
- Tear down the map of an iova range [addr, end). A page list will be returned which will be freed after iotlb flushing.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

Quoted 2 lines · Lu Baolu

This adds functions to manipulate first level page tables which could be used by a scalale mode capable IOMMU unit.

s/scalale/scalable

Quoted 3 lines

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Quoted 9 lines

- Map an iova range of [addr, end) to the physical memory started at @phys_addr with the @prot permissions.

intel_mmunmap_range(domain, addr, end)
- Tear down the map of an iova range [addr, end). A page list will be returned which will be freed after iotlb flushing.

Quoted 6 lines

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 2 lines · Lu Baolu

Quoted 2 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Quoted 4 lines

Quoted 1 line

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

Quoted 11 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 6 lines · Lu Baolu

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

Quoted 4 lines

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

what is the point of keeping 'mm' here? replace it with 'iommu'?

Quoted 16 lines · Kevin

Quoted 15 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 6 lines · Lu Baolu

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

Quoted 1 line

what is the point of keeping 'mm' here? replace it with 'iommu'?

I'm not sure of what Baolu thought, but to me "mm" makes sense itself to identify this from real IOMMU page tables (because IIUC these will be MMU page tables). We can come up with better names, but IMHO "iommu" can be a bit misleading to let people refer to the 2nd level page table.

From: Lu Baolu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:52 PM

Hi Peter and Kevin,

Quoted 17 lines · Peter Xu

Quoted 16 lines · Kevin

Quoted 15 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 6 lines · Lu Baolu

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

Quoted 7 lines

Quoted 1 line

what is the point of keeping 'mm' here? replace it with 'iommu'?

I'm not sure of what Baolu thought, but to me "mm" makes sense itself to identify this from real IOMMU page tables (because IIUC these will be MMU page tables). We can come up with better names, but IMHO "iommu" can be a bit misleading to let people refer to the 2nd level page table.

"mm" represents a CPU (first level) page table;

vs.

"io" represents an IOMMU (second level) page table.

Quoted 31 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:52 PM

Hi Peter and Kevin,

Quoted 17 lines · Peter Xu

Quoted 16 lines · Kevin

Quoted 15 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 6 lines · Lu Baolu

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

Quoted 7 lines

Quoted 1 line

what is the point of keeping 'mm' here? replace it with 'iommu'?

I'm not sure of what Baolu thought, but to me "mm" makes sense itself to identify this from real IOMMU page tables (because IIUC these will be MMU page tables). We can come up with better names, but IMHO "iommu" can be a bit misleading to let people refer to the 2nd level page table.

Quoted 6 lines

"mm" represents a CPU (first level) page table;

vs.

"io" represents an IOMMU (second level) page table.

IOMMU first level is not equivalent to CPU page table, though you can use the latter as the first level (e.g. in SVA). Especially here you are making IOVA->GPA as the first level, which is not CPU page table.

btw both levels are for "io" i.e. DMA purposes from VT-d p.o.v. They are just hierarchical structures implemented by VT-d, with slightly different format.

Quoted 31 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:52 PM

Hi Peter and Kevin,

Quoted 17 lines · Peter Xu

Quoted 16 lines · Kevin

Quoted 15 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 12:31 PM

Quoted 6 lines · Lu Baolu

Quoted 3 lines

Quoted 1 line

intel_mmmap_range(domain, addr, end, phys_addr, prot)

Maybe think of a different name..? mmmap seems a bit weird :-)

Yes. I don't like it either. I've thought about it and haven't figured out a satisfied one. Do you have any suggestions?

How about at least split the word using "_"? Like "mm_map", then apply it to all the "mmm*" prefixes. Otherwise it'll be easily misread as mmap() which is totally irrelevant to this...

Quoted 7 lines

Quoted 1 line

what is the point of keeping 'mm' here? replace it with 'iommu'?

I'm not sure of what Baolu thought, but to me "mm" makes sense itself to identify this from real IOMMU page tables (because IIUC these will be MMU page tables). We can come up with better names, but IMHO "iommu" can be a bit misleading to let people refer to the 2nd level page table.

Quoted 6 lines

"mm" represents a CPU (first level) page table;

vs.

"io" represents an IOMMU (second level) page table.

IOMMU first level is not equivalent to CPU page table, though you can use the latter as the first level (e.g. in SVA). Especially here you are making IOVA->GPA as the first level, which is not CPU page table.

btw both levels are for "io" i.e. DMA purposes from VT-d p.o.v. They are just hierarchical structures implemented by VT-d, with slightly different format. The specification doesn't limit how you use them for. In a hypothetical case, an IOMMU may implement exactly same CPU-page- table format and support page faults for both levels. Then you can even link the CPU page table to the 2nd level for sure.

Maybe we just name it from VT-d context, e.g. intel_map_first_level_range, Intel_map_second_level_range, and then register them as dmar domain callback as you replied in another mail.

Thanks
Kevin

This adds functions to manipulate first level page tables which could be used by a scalale mode capable IOMMU unit.

intel_mmmap_range(domain, addr, end, phys_addr, prot)
- Map an iova range of [addr, end) to the physical memory started at @phys_addr with the @prot permissions.

intel_mmunmap_range(domain, addr, end)
- Tear down the map of an iova range [addr, end). A page list will be returned which will be freed after iotlb flushing.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

---
 drivers/iommu/Makefile             |   2 +-
 drivers/iommu/intel-pgtable.c      | 342 +++++++++++++++++++++++++++++
 include/linux/intel-iommu.h        |  24 +-
 include/trace/events/intel_iommu.h |  60 +++++
 4 files changed, 426 insertions(+), 2 deletions(-)
 create mode 100644 drivers/iommu/intel-pgtable.c

drivers/iommu/Makefile +1 -1

index 4f405f926e73..dc550e14cc58 100644
@@ -17,7 +17,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
 obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o intel-pgtable.o
 obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += intel-iommu-debugfs.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o

drivers/iommu/intel-pgtable.c new file +0 -0

new file mode 100644
index 000000000000..8e95978cd381

drivers/iommu/intel-pgtable.c new file +11 -0

+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ */
+
+#define pr_fmt(fmt)     "DMAR: " fmt
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/io.h>

drivers/iommu/intel-pgtable.c new file +35 -0

+		domain_flush_cache(domain, nm, sizeof(nm##_t));		\
+	}								\
+	spin_unlock(&(domain)->page_table_lock);			\
+} while(0);
+
+static int
+mmmap_pte_range(struct dmar_domain *domain, pmd_t *pmd, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	pte_t *pte, *first_pte;
+	u64 pfn;
+
+	pfn = phys_addr >> PAGE_SHIFT;
+	if (unlikely(pmd_none(*pmd)))
+		pgtable_populate(domain, pmd);
+
+	first_pte = pte = pte_offset_kernel(pmd, addr);
+
+	do {
+		set_pte(pte, pfn_pte(pfn, prot));
+		pfn++;
+	} while (pte++, addr += PAGE_SIZE, addr != end);
+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	return 0;
+}
+
+static int
+mmmap_pmd_range(struct dmar_domain *domain, pud_t *pud, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	pmd_t *pmd;
+

drivers/iommu/intel-pgtable.c new file +13 -0

+	} while (pmd++, addr = next, addr != end);
+
+	return 0;
+}
+
+static int
+mmmap_pud_range(struct dmar_domain *domain, p4d_t *p4d, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	pud_t *pud;
+
+	if (unlikely(p4d_none(*p4d)))

drivers/iommu/intel-pgtable.c new file +13 -0

+	} while (pud++, addr = next, addr != end);
+
+	return 0;
+}
+
+static int
+mmmap_p4d_range(struct dmar_domain *domain, pgd_t *pgd, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	p4d_t *p4d;
+
+	if (cpu_feature_enabled(X86_FEATURE_LA57) && unlikely(pgd_none(*pgd)))

drivers/iommu/intel-pgtable.c new file +18 -0

+	} while (p4d++, addr = next, addr != end);
+
+	return 0;
+}
+
+int intel_mmmap_range(struct dmar_domain *domain, unsigned long addr,
+		      unsigned long end, phys_addr_t phys_addr, int dma_prot)
+{
+	unsigned long next;
+	pgprot_t prot;
+	pgd_t *pgd;
+
+	trace_domain_mm_map(domain, addr, end, phys_addr);
+
+	/*
+	 * There is no PAGE_KERNEL_WO for a pte entry, so let's use RW
+	 * for a pte that requires write operation.
+	 */

drivers/iommu/intel-pgtable.c new file +19 -0

+	} while (pgd++, addr = next, addr != end);
+
+	return 0;
+}
+
+/*
+ * mmunmap: Unmap an existing mapping between a range of IO vitual address
+ *          and physical addresses.
+ */
+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;

drivers/iommu/intel-pgtable.c new file +7 -0

+		struct page *pte_page;
+
+		pte = (pte_t *)pmd_page_vaddr(*pmd);
+		for (i = 0; i < PTRS_PER_PTE; i++)
+			if (!pte || !pte_none(pte[i]))
+				goto pte_out;
+

Quoted 45 lines · Lu Baolu

This adds functions to manipulate first level page tables which could be used by a scalale mode capable IOMMU unit.

intel_mmmap_range(domain, addr, end, phys_addr, prot)
- Map an iova range of [addr, end) to the physical memory started at @phys_addr with the @prot permissions.

intel_mmunmap_range(domain, addr, end)
- Tear down the map of an iova range [addr, end). A page list will be returned which will be freed after iotlb flushing.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

---
  drivers/iommu/Makefile             |   2 +-
  drivers/iommu/intel-pgtable.c      | 342 +++++++++++++++++++++++++++++
  include/linux/intel-iommu.h        |  24 +-
  include/trace/events/intel_iommu.h |  60 +++++
  4 files changed, 426 insertions(+), 2 deletions(-)

create mode 100644 drivers/iommu/intel-pgtable.c

drivers/iommu/Makefile +1 -1

index 4f405f926e73..dc550e14cc58 100644
@@ -17,7 +17,7 @@ obj-$(CONFIG_ARM_SMMU) += arm-smmu.o arm-smmu-impl.o
  obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
  obj-$(CONFIG_DMAR_TABLE) += dmar.o
  obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-trace.o intel-pgtable.o
  obj-$(CONFIG_INTEL_IOMMU_DEBUGFS) += intel-iommu-debugfs.o
  obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
  obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o

drivers/iommu/intel-pgtable.c new file +0 -0

new file mode 100644
index 000000000000..8e95978cd381
@@ -0,0 +1,342 @@

drivers/iommu/intel-pgtable.c line 3

Quoted 3 lines

drivers/iommu/intel-pgtable.c new file +3 -0

+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pgtable.c - Intel IOMMU page table manipulation library

Could this be a bit misleading? Normally I'll use "IOMMU page table" to refer to the 2nd level page table only, and I'm always understanding it as "the new IOMMU will understand MMU page table as the 1st level". At least mention "IOMMU 1st level page table"?

Quoted 11 lines

drivers/iommu/intel-pgtable.c new file +11 -0

+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ */
+
+#define pr_fmt(fmt)     "DMAR: " fmt
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/io.h>

drivers/iommu/intel-pgtable.c line 24

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+#include <linux/export.h>
+#include <linux/intel-iommu.h>
+#include <asm/cacheflush.h>
+#include <asm/pgtable.h>
+#include <asm/pgalloc.h>
+#include <trace/events/intel_iommu.h>
+
+#ifdef CONFIG_X86
+/*
+ * mmmap: Map a range of IO virtual address to physical addresses.

"... to physical addresses using MMU page table"?

Might be clearer?

drivers/iommu/intel-pgtable.c line 31

Quoted 8 lines

drivers/iommu/intel-pgtable.c new file +7 -0

+ */
+#define pgtable_populate(domain, nm)					\
+do {									\
+	void *__new = alloc_pgtable_page(domain->nid);			\
+	if (!__new)							\
+		return -ENOMEM;						\
+	smp_wmb();							\

Could I ask what's this wmb used for?

drivers/iommu/intel-pgtable.c line 32

Quoted 2 lines

drivers/iommu/intel-pgtable.c new file +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

drivers/iommu/intel-pgtable.c line 36

Quoted 5 lines

drivers/iommu/intel-pgtable.c new file +4 -0

+	if (nm ## _present(*nm)) {					\
+		free_pgtable_page(__new);				\
+	} else {							\
+		set_##nm(nm, __##nm(__pa(__new) | _PAGE_TABLE));	\

It seems to me that PV could trap calls to set_pte(). Then these could also be trapped by e.g. Xen? Are these traps needed? Is there side effect? I'm totally not familiar with this, but just ask aloud...

Quoted 40 lines

drivers/iommu/intel-pgtable.c new file +39 -0

+		domain_flush_cache(domain, nm, sizeof(nm##_t));		\
+	}								\
+	spin_unlock(&(domain)->page_table_lock);			\
+} while(0);
+
+static int
+mmmap_pte_range(struct dmar_domain *domain, pmd_t *pmd, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	pte_t *pte, *first_pte;
+	u64 pfn;
+
+	pfn = phys_addr >> PAGE_SHIFT;
+	if (unlikely(pmd_none(*pmd)))
+		pgtable_populate(domain, pmd);
+
+	first_pte = pte = pte_offset_kernel(pmd, addr);
+
+	do {
+		set_pte(pte, pfn_pte(pfn, prot));
+		pfn++;
+	} while (pte++, addr += PAGE_SIZE, addr != end);
+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	return 0;
+}
+
+static int
+mmmap_pmd_range(struct dmar_domain *domain, pud_t *pud, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	pmd_t *pmd;
+
+	if (unlikely(pud_none(*pud)))
+		pgtable_populate(domain, pud);
+	pmd = pmd_offset(pud, addr);
+

drivers/iommu/intel-pgtable.c line 81

Quoted 6 lines

drivers/iommu/intel-pgtable.c new file +6 -0

+	phys_addr -= addr;
+	do {
+		next = pmd_addr_end(addr, end);
+		if (mmmap_pte_range(domain, pmd, addr, next,
+				    phys_addr + addr, prot))
+			return -ENOMEM;

How about return the errcode of mmmap_pte_range() directly?

Quoted 14 lines

drivers/iommu/intel-pgtable.c new file +13 -0

+	} while (pmd++, addr = next, addr != end);
+
+	return 0;
+}
+
+static int
+mmmap_pud_range(struct dmar_domain *domain, p4d_t *p4d, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	pud_t *pud;
+
+	if (unlikely(p4d_none(*p4d)))

drivers/iommu/intel-pgtable.c line 104

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+		pgtable_populate(domain, p4d);
+
+	pud = pud_offset(p4d, addr);
+
+	phys_addr -= addr;
+	do {
+		next = pud_addr_end(addr, end);
+		if (mmmap_pmd_range(domain, pud, addr, next,
+				    phys_addr + addr, prot))
+			return -ENOMEM;

Same.

Quoted 13 lines

drivers/iommu/intel-pgtable.c new file +13 -0

+	} while (pud++, addr = next, addr != end);
+
+	return 0;
+}
+
+static int
+mmmap_p4d_range(struct dmar_domain *domain, pgd_t *pgd, unsigned long addr,
+		unsigned long end, phys_addr_t phys_addr, pgprot_t prot)
+{
+	unsigned long next;
+	p4d_t *p4d;
+
+	if (cpu_feature_enabled(X86_FEATURE_LA57) && unlikely(pgd_none(*pgd)))

drivers/iommu/intel-pgtable.c line 127

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+		pgtable_populate(domain, pgd);
+
+	p4d = p4d_offset(pgd, addr);
+
+	phys_addr -= addr;
+	do {
+		next = p4d_addr_end(addr, end);
+		if (mmmap_pud_range(domain, p4d, addr, next,
+				    phys_addr + addr, prot))
+			return -ENOMEM;

Same.

Quoted 18 lines

drivers/iommu/intel-pgtable.c new file +18 -0

+	} while (p4d++, addr = next, addr != end);
+
+	return 0;
+}
+
+int intel_mmmap_range(struct dmar_domain *domain, unsigned long addr,
+		      unsigned long end, phys_addr_t phys_addr, int dma_prot)
+{
+	unsigned long next;
+	pgprot_t prot;
+	pgd_t *pgd;
+
+	trace_domain_mm_map(domain, addr, end, phys_addr);
+
+	/*
+	 * There is no PAGE_KERNEL_WO for a pte entry, so let's use RW
+	 * for a pte that requires write operation.
+	 */

drivers/iommu/intel-pgtable.c line 155

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+	prot = dma_prot & DMA_PTE_WRITE ? PAGE_KERNEL : PAGE_KERNEL_RO;
+	BUG_ON(addr >= end);
+
+	phys_addr -= addr;
+	pgd = pgd_offset_pgd(domain->pgd, addr);
+	do {
+		next = pgd_addr_end(addr, end);
+		if (mmmap_p4d_range(domain, pgd, addr, next,
+				    phys_addr + addr, prot))
+			return -ENOMEM;

Same.

Quoted 24 lines

drivers/iommu/intel-pgtable.c new file +24 -0

+	} while (pgd++, addr = next, addr != end);
+
+	return 0;
+}
+
+/*
+ * mmunmap: Unmap an existing mapping between a range of IO vitual address
+ *          and physical addresses.
+ */
+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;
+	pte = pte_offset_kernel(pmd, addr);
+	first_pte = pte;
+	do {
+		set_pte(pte, __pte(0));
+	} while (pte++, addr += PAGE_SIZE, addr != end);

drivers/iommu/intel-pgtable.c line 184

Quoted 5 lines

drivers/iommu/intel-pgtable.c new file +5 -0

+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	/* Add page to free list if all entries are empty. */
+	if (reclaim) {

Shouldn't we know whether to reclaim if with (addr, end) specified as long as they cover the whole range of this PMD?

Also I noticed that this value right now is passed in from the very top of the unmap() call. I didn't really catch the point of that...

I'll have similar question to below a few places but I'll skip to comment after I understand this one.

Quoted 11 lines

drivers/iommu/intel-pgtable.c new file +11 -0

+		struct page *pte_page;
+
+		pte = (pte_t *)pmd_page_vaddr(*pmd);
+		for (i = 0; i < PTRS_PER_PTE; i++)
+			if (!pte || !pte_none(pte[i]))
+				goto pte_out;
+
+		pte_page = pmd_page(*pmd);
+		pte_page->freelist = freelist;
+		freelist = pte_page;
+		pmd_clear(pmd);

drivers/iommu/intel-pgtable.c line 201

Quoted 6 lines

drivers/iommu/intel-pgtable.c new file +6 -0

+		domain_flush_cache(domain, pmd, sizeof(pmd_t));
+	}
+
+pte_out:
+	return freelist;
+}

Regards,

Quoted 10 lines

drivers/iommu/intel-pgtable.c line 3

Quoted 4 lines

drivers/iommu/intel-pgtable.c new file +3 -0

@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pgtable.c - Intel IOMMU page table manipulation library

Could this be a bit misleading? Normally I'll use "IOMMU page table" to refer to the 2nd level page table only, and I'm always understanding it as "the new IOMMU will understand MMU page table as the 1st level". At least mention "IOMMU 1st level page table"?

This file is a place holder for all code that manipulating iommu page tables (both first level and second level). Instead of putting everything in intel_iommu.c, let's make the code more structured so that it's easier for reading and maintaining. This is the motivation of this file.

Quoted 25 lines

Quoted 12 lines

drivers/iommu/intel-pgtable.c new file +11 -0

+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ */
+
+#define pr_fmt(fmt)     "DMAR: " fmt
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/io.h>

drivers/iommu/intel-pgtable.c line 24

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+#include <linux/export.h>
+#include <linux/intel-iommu.h>
+#include <asm/cacheflush.h>
+#include <asm/pgtable.h>
+#include <asm/pgalloc.h>
+#include <trace/events/intel_iommu.h>
+
+#ifdef CONFIG_X86
+/*
+ * mmmap: Map a range of IO virtual address to physical addresses.

"... to physical addresses using MMU page table"?

Quoted 1 line

Might be clearer?

Yes.

Quoted 10 lines

drivers/iommu/intel-pgtable.c line 31

Quoted 8 lines

drivers/iommu/intel-pgtable.c new file +7 -0

+ */
+#define pgtable_populate(domain, nm)					\
+do {									\
+	void *__new = alloc_pgtable_page(domain->nid);			\
+	if (!__new)							\
+		return -ENOMEM;						\
+	smp_wmb();							\

Could I ask what's this wmb used for?

Sure. This is answered by a comment in __pte_alloc() in mm/memory.c. Let me post it here.

/*
* Ensure all pte setup (eg. pte page lock and page clearing) are
* visible before the pte is made visible to other CPUs by being
* put into page tables.
*
* The other side of the story is the pointer chasing in the page
* table walking code (when walking the page table without locking;
* ie. most of the time). Fortunately, these data accesses consist
* of a chain of data-dependent loads, meaning most CPUs (alpha
* being the notable exception) will already guarantee loads are
* seen in-order. See the alpha page table accessors for the
* smp_read_barrier_depends() barriers in page table walking code. */
smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */

Quoted 9 lines

drivers/iommu/intel-pgtable.c line 32

Quoted 3 lines

drivers/iommu/intel-pgtable.c new file +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

hence we only need
a spin lock to serialize multiple tries of populating a entry for pde. As for pte, we can assume there is only single thread which can access it at a time because different mappings will have different iova's.

Quoted 10 lines

drivers/iommu/intel-pgtable.c line 36

Quoted 6 lines

drivers/iommu/intel-pgtable.c new file +4 -0

+	if (nm ## _present(*nm)) {					\
+		free_pgtable_page(__new);				\
+	} else {							\
+		set_##nm(nm, __##nm(__pa(__new) | _PAGE_TABLE));	\

It seems to me that PV could trap calls to set_pte(). Then these could also be trapped by e.g. Xen? Are these traps needed? Is there side effect? I'm totally not familiar with this, but just ask aloud...

Good catch. But I don't think a vIOMMU could get a chance to run in a PV environment. I might miss something?

Quoted 15 lines

Quoted 15 lines

drivers/iommu/intel-pgtable.c new file +15 -0

+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;
+	pte = pte_offset_kernel(pmd, addr);
+	first_pte = pte;
+	do {
+		set_pte(pte, __pte(0));
+	} while (pte++, addr += PAGE_SIZE, addr != end);

Quoted 8 lines

drivers/iommu/intel-pgtable.c line 184

Quoted 5 lines

drivers/iommu/intel-pgtable.c new file +5 -0

+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	/* Add page to free list if all entries are empty. */
+	if (reclaim) {

Shouldn't we know whether to reclaim if with (addr, end) specified as long as they cover the whole range of this PMD?

Current policy is that we don't reclaim any pages until the whole page table will be torn down.

The gain is that we don't have to use a
spinlock when map/unmap a pmd entry anymore.

On Thu, Sep 26, 2019 at 10:35:24AM +0800, Lu Baolu wrote:

[...]

Quoted 11 lines

Quoted 10 lines

drivers/iommu/intel-pgtable.c line 3

Quoted 4 lines

drivers/iommu/intel-pgtable.c new file +3 -0

@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pgtable.c - Intel IOMMU page table manipulation library

Could this be a bit misleading? Normally I'll use "IOMMU page table" to refer to the 2nd level page table only, and I'm always understanding it as "the new IOMMU will understand MMU page table as the 1st level". At least mention "IOMMU 1st level page table"?

Quoted 5 lines

This file is a place holder for all code that manipulating iommu page tables (both first level and second level). Instead of putting everything in intel_iommu.c, let's make the code more structured so that it's easier for reading and maintaining. This is the motivation of this file.

I see.

Quoted 43 lines

Quoted 25 lines

Quoted 12 lines

drivers/iommu/intel-pgtable.c new file +11 -0

+ *
+ * Copyright (C) 2019 Intel Corporation
+ *
+ * Author: Lu Baolu <[email protected]>
+ */
+
+#define pr_fmt(fmt)     "DMAR: " fmt
+#include <linux/vmalloc.h>
+#include <linux/mm.h>
+#include <linux/sched.h>
+#include <linux/io.h>

drivers/iommu/intel-pgtable.c line 24

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+#include <linux/export.h>
+#include <linux/intel-iommu.h>
+#include <asm/cacheflush.h>
+#include <asm/pgtable.h>
+#include <asm/pgalloc.h>
+#include <trace/events/intel_iommu.h>
+
+#ifdef CONFIG_X86
+/*
+ * mmmap: Map a range of IO virtual address to physical addresses.

"... to physical addresses using MMU page table"?

Quoted 1 line

Might be clearer?

Yes.

Quoted 10 lines

drivers/iommu/intel-pgtable.c line 31

Quoted 8 lines

drivers/iommu/intel-pgtable.c new file +7 -0

+ */
+#define pgtable_populate(domain, nm)					\
+do {									\
+	void *__new = alloc_pgtable_page(domain->nid);			\
+	if (!__new)							\
+		return -ENOMEM;						\
+	smp_wmb();							\

Could I ask what's this wmb used for?

Sure. This is answered by a comment in __pte_alloc() in mm/memory.c. Let me post it here.

Quoted 14 lines

/*
* Ensure all pte setup (eg. pte page lock and page clearing) are
* visible before the pte is made visible to other CPUs by being
* put into page tables.
*
* The other side of the story is the pointer chasing in the page
* table walking code (when walking the page table without locking;
* ie. most of the time). Fortunately, these data accesses consist
* of a chain of data-dependent loads, meaning most CPUs (alpha
* being the notable exception) will already guarantee loads are
* seen in-order. See the alpha page table accessors for the
* smp_read_barrier_depends() barriers in page table walking code. */
smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */

Ok. I don't understand the rationale much behind but the comment seems to make sense... Could you help to comment above, like "please reference to comment in __pte_alloc" above the line?

Quoted 8 lines

Quoted 8 lines

drivers/iommu/intel-pgtable.c line 32

Quoted 4 lines

drivers/iommu/intel-pgtable.c new file +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Quoted 4 lines

Quoted 2 lines

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

I see. Though, could I ask why? It seems to me that the existing 2nd level page table does not keep these when unmap, and it's not even use locking at all by leveraging cmpxchg()?

Quoted 5 lines

hence we only need
a spin lock to serialize multiple tries of populating a entry for pde. As for pte, we can assume there is only single thread which can access it at a time because different mappings will have different iova's.

Ah yes sorry nm will never be pte here... so do you mean the upper layer, e.g., the iova allocator will make sure the ranges to be mapped will never collapse with each other so setting PTEs do not need lock?

Quoted 12 lines

Quoted 11 lines

drivers/iommu/intel-pgtable.c line 36

Quoted 7 lines

drivers/iommu/intel-pgtable.c new file +4 -0

+	if (nm ## _present(*nm)) {					\
+		free_pgtable_page(__new);				\
+	} else {							\
+		set_##nm(nm, __##nm(__pa(__new) | _PAGE_TABLE));	\

It seems to me that PV could trap calls to set_pte(). Then these could also be trapped by e.g. Xen? Are these traps needed? Is there side effect? I'm totally not familiar with this, but just ask aloud...

Quoted 2 lines

Good catch. But I don't think a vIOMMU could get a chance to run in a PV environment. I might miss something?

I don't know... Is there reason to not allow a Xen guest to use 1st level mapping?

While on the other side... If the PV interface will never be used, then could native_set_##nm() be used directly?

[...]

Quoted 24 lines

Quoted 23 lines

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;

drivers/iommu/intel-pgtable.c line 184

Quoted 10 lines

drivers/iommu/intel-pgtable.c new file +10 -0

+	pte = pte_offset_kernel(pmd, addr);
+	first_pte = pte;
+	do {
+		set_pte(pte, __pte(0));
+	} while (pte++, addr += PAGE_SIZE, addr != end);
+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	/* Add page to free list if all entries are empty. */
+	if (reclaim) {

Shouldn't we know whether to reclaim if with (addr, end) specified as long as they cover the whole range of this PMD?

Quoted 2 lines

Current policy is that we don't reclaim any pages until the whole page table will be torn down.

Ah OK. But I saw that you're passing in relaim==!start_addr. Shouldn't that errornously trigger if one wants to unmap the 1st page as well even if not the whole address space?

Quoted 3 lines

The gain is that we don't have to use a
spinlock when map/unmap a pmd entry anymore.

So this question should also related to above on the locking - have you thought about using the same way (IIUC) as the 2nd level page table to use cmpxchg()? AFAIU that does not need any lock?

For me it's perfectly fine to use a lock at least for initial version, I just want to know the considerations behind in case I missed anything important.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

drivers/iommu/intel-pgtable.c line 32

Quoted 1 line

drivers/iommu/intel-pgtable.c new file +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

I see. Though, could I ask why? It seems to me that the existing 2nd level page table does not keep these when unmap, and it's not even use locking at all by leveraging cmpxchg()?

I still need some time to understand how cmpxchg() solves the race issue when reclaims pages. For example.

Thread A Thread B

(quoted diff) +0 -3

-A1: check all PTE's empty		-B1: up-level PDE valid
-A2: clear the up-level PDE
-A3: reclaim the page			-B2: populate the PTEs

Both (A1,A2) and (B1,B2) should be atomic. Otherwise, race could happen.

Actually, the iova allocator always packs IOVA ranges close to the top of the address space. This results in requiring a minimal number of pages to map the allocated IOVA ranges, which makes memory onsumption by IOMMU page tables tolerable. Hence, we don't need to reclaim the pages until the whole page table is about to tear down. The real data on my test machine also improves this.

Quoted 27 lines

Quoted 26 lines

Quoted 15 lines

Quoted 15 lines

drivers/iommu/intel-pgtable.c new file +15 -0

+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;
+	pte = pte_offset_kernel(pmd, addr);
+	first_pte = pte;
+	do {
+		set_pte(pte, __pte(0));
+	} while (pte++, addr += PAGE_SIZE, addr != end);

Quoted 8 lines

drivers/iommu/intel-pgtable.c line 184

Quoted 5 lines

drivers/iommu/intel-pgtable.c new file +5 -0

+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	/* Add page to free list if all entries are empty. */
+	if (reclaim) {

Shouldn't we know whether to reclaim if with (addr, end) specified as long as they cover the whole range of this PMD?

Current policy is that we don't reclaim any pages until the whole page table will be torn down.

Quoted 3 lines

Ah OK. But I saw that you're passing in relaim==!start_addr. Shouldn't that errornously trigger if one wants to unmap the 1st page as well even if not the whole address space?

IOVA 0 is assumed to be reserved by the allocator. Otherwise, we have no means to check whether a IOVA is valid.

Hi, Baolu,

Quoted 22 lines · Lu Baolu

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

drivers/iommu/intel-pgtable.c line 32

Quoted 1 line

drivers/iommu/intel-pgtable.c new file +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

I see. Though, could I ask why? It seems to me that the existing 2nd level page table does not keep these when unmap, and it's not even use locking at all by leveraging cmpxchg()?

I still need some time to understand how cmpxchg() solves the race issue when reclaims pages. For example.

Thread A Thread B

(quoted diff) +0 -3

-A1: check all PTE's empty		-B1: up-level PDE valid
-A2: clear the up-level PDE
-A3: reclaim the page			-B2: populate the PTEs

Quoted 1 line

Both (A1,A2) and (B1,B2) should be atomic. Otherwise, race could happen.

I'm not sure of this, but IMHO it is similarly because we need to allocate the iova ranges from iova allocator first, so thread A (who's going to unmap pages) and thread B (who's going to map new pages) should never have collapsed regions if happening concurrently. I'm

referring to intel_unmap() in which we won't free the iova region before domain_unmap() completes (which should cover the whole process of A1-A3) so the same iova range to be unmapped won't be allocated to any new pages in some other thread.

There's also a hint in domain_unmap():

   /* we don't need lock here; nobody else touches the iova range */

Quoted 7 lines

Actually, the iova allocator always packs IOVA ranges close to the top of the address space. This results in requiring a minimal number of pages to map the allocated IOVA ranges, which makes memory onsumption by IOMMU page tables tolerable. Hence, we don't need to reclaim the pages until the whole page table is about to tear down. The real data on my test machine also improves this.

Do you mean you have run the code with a 1st-level-supported IOMMU hardware? IMHO any data point would be good to be in the cover letter as reference.

[...]

Quoted 27 lines

Quoted 27 lines

Quoted 26 lines

Quoted 15 lines

Quoted 15 lines

drivers/iommu/intel-pgtable.c new file +15 -0

+static struct page *
+mmunmap_pte_range(struct dmar_domain *domain, pmd_t *pmd,
+		  unsigned long addr, unsigned long end,
+		  struct page *freelist, bool reclaim)
+{
+	int i;
+	unsigned long start;
+	pte_t *pte, *first_pte;
+
+	start = addr;
+	pte = pte_offset_kernel(pmd, addr);
+	first_pte = pte;
+	do {
+		set_pte(pte, __pte(0));
+	} while (pte++, addr += PAGE_SIZE, addr != end);

Quoted 8 lines

drivers/iommu/intel-pgtable.c line 184

Quoted 5 lines

drivers/iommu/intel-pgtable.c new file +5 -0

+
+	domain_flush_cache(domain, first_pte, (void *)pte - (void *)first_pte);
+
+	/* Add page to free list if all entries are empty. */
+	if (reclaim) {

Shouldn't we know whether to reclaim if with (addr, end) specified as long as they cover the whole range of this PMD?

Current policy is that we don't reclaim any pages until the whole page table will be torn down.

Quoted 6 lines

Quoted 3 lines

Ah OK. But I saw that you're passing in relaim==!start_addr. Shouldn't that errornously trigger if one wants to unmap the 1st page as well even if not the whole address space?

IOVA 0 is assumed to be reserved by the allocator. Otherwise, we have no means to check whether a IOVA is valid.

Is this an assumption of the allocator? Could that change in the future?

IMHO that's not necessary if so, after all it's as simple as replacing (!start_addr) with (start == 0 && end == END). I see that in domain_unmap() it has a similar check when freeing pgd:

   if (start_pfn == 0 && last_pfn == DOMAIN_MAX_PFN(domain->gaw))

Thanks,

Hi Peter,

Quoted 25 lines · Peter Xu

Hi, Baolu,

Quoted 22 lines · Lu Baolu

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

I see. Though, could I ask why? It seems to me that the existing 2nd level page table does not keep these when unmap, and it's not even use locking at all by leveraging cmpxchg()?

I still need some time to understand how cmpxchg() solves the race issue when reclaims pages. For example.

Thread A Thread B

(quoted diff) +0 -3

-A1: check all PTE's empty		-B1: up-level PDE valid
-A2: clear the up-level PDE
-A3: reclaim the page			-B2: populate the PTEs

Quoted 6 lines

Quoted 1 line

Both (A1,A2) and (B1,B2) should be atomic. Otherwise, race could happen.

I'm not sure of this, but IMHO it is similarly because we need to allocate the iova ranges from iova allocator first, so thread A (who's going to unmap pages) and thread B (who's going to map new pages) should never have collapsed regions if happening concurrently. I'm

Although they don't collapse, they might share a same pmd entry. If A cleared the pmd entry and B goes ahead with populating the pte's. It will crash.

Quoted 35 lines · Lu Baolu

Hi Peter,

Quoted 25 lines · Peter Xu

Hi, Baolu,

Quoted 22 lines · Lu Baolu

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

Review

Quoted 1 line

(quoted diff) +1 -0

+	spin_lock(&(domain)->page_table_lock);				\

Is this intended to lock here instead of taking the lock during the whole page table walk? Is it safe?

Quoted 8 lines

Quoted 4 lines

Quoted 2 lines

Taking the example where nm==PTE: when we reach here how do we guarantee that the PMD page that has this PTE is still valid?

We will always keep the non-leaf pages in the table,

I see. Though, could I ask why? It seems to me that the existing 2nd level page table does not keep these when unmap, and it's not even use locking at all by leveraging cmpxchg()?

I still need some time to understand how cmpxchg() solves the race issue when reclaims pages. For example.

Thread A Thread B

(quoted diff) +0 -3

-A1: check all PTE's empty		-B1: up-level PDE valid
-A2: clear the up-level PDE
-A3: reclaim the page			-B2: populate the PTEs

Quoted 6 lines

Quoted 1 line

Both (A1,A2) and (B1,B2) should be atomic. Otherwise, race could happen.

I'm not sure of this, but IMHO it is similarly because we need to allocate the iova ranges from iova allocator first, so thread A (who's going to unmap pages) and thread B (who's going to map new pages) should never have collapsed regions if happening concurrently. I'm

Quoted 3 lines

Although they don't collapse, they might share a same pmd entry. If A cleared the pmd entry and B goes ahead with populating the pte's. It will crash.

My understanding is that if A was not owning all the pages on that PMD entry then it will never free the page that was backing that PMD entry. Please refer to the code in dma_pte_clear_level() where it has:

         /* If range covers entire pagetable, free it */
         if (start_pfn <= level_pfn &&
                 last_pfn >= level_pfn + level_size(level) - 1) {
                 ...
         } else {
                 ...
         }

Note that when going into the else block, the PMD won't be freed but only the PTEs that upon the PMD will be cleared.

In the case you mentioned above, IMHO it should go into that else block. Say, thread A must not contain the whole range of that PMD otherwise thread B won't get allocated with pages within that range covered by the same PMD.

Thanks,

From: Lu Baolu [mailto:[email protected]]
Sent: Monday, September 23, 2019 8:25 PM

If a dmar domain has DOMAIN_FLAG_FIRST_LEVEL_TRANS bit set in its flags, IOMMU will use the first level page table for translation. Hence, we need to map or unmap addresses in the first level page table.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

---
 drivers/iommu/intel-iommu.c | 94 ++++++++++++++++++++++++++++++++-
----
 1 file changed, 82 insertions(+), 12 deletions(-)

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

index 9cfe8098d993..103480016010 100644
@@ -168,6 +168,11 @@ static inline unsigned long virt_to_dma_pfn(void
 *p)
 	return page_to_dma_pfn(virt_to_page(p));
 }
 
+static inline unsigned long dma_pfn_to_addr(unsigned long pfn)
+{
+	return pfn << VTD_PAGE_SHIFT;
+}
+
 /* global iommu list, set NULL for ignored DMAR units */
 static struct intel_iommu **g_iommus;

@@ -307,6 +312,9 @@ static int hw_pass_through = 1;
  */
 #define DOMAIN_FLAG_LOSE_CHILDREN		BIT(1)
 
+/* Domain uses first level translation for DMA remapping. */
+#define DOMAIN_FLAG_FIRST_LEVEL_TRANS		BIT(2)
+
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])

@@ -552,6 +560,11 @@ static inline int domain_type_is_si(struct
 dmar_domain *domain)
 	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
 }
 
+static inline int domain_type_is_flt(struct dmar_domain *domain)
+{
+	return domain->flags & DOMAIN_FLAG_FIRST_LEVEL_TRANS;
+}
+
 static inline int domain_pfn_supported(struct dmar_domain *domain,
 				       unsigned long pfn)

{

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

@@ -1147,8 +1160,15 @@ static struct page *domain_unmap(struct
 dmar_domain *domain,
 	BUG_ON(start_pfn > last_pfn);
 
 	/* we don't need lock here; nobody else touches the iova range */
-	freelist = dma_pte_clear_level(domain, agaw_to_level(domain-
 >agaw),
-				       domain->pgd, 0, start_pfn, last_pfn,

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

 	/* free pgd */
 	if (start_pfn == 0 && last_pfn == DOMAIN_MAX_PFN(domain->gaw))

{

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

@@ -2213,9 +2233,10 @@ static inline int hardware_largepage_caps(struct
 dmar_domain *domain,
 	return level;
 }
 
-static int __domain_mapping(struct dmar_domain *domain, unsigned long
 iov_pfn,
-			    struct scatterlist *sg, unsigned long phys_pfn,
-			    unsigned long nr_pages, int prot)
+static int
+__domain_mapping_dma(struct dmar_domain *domain, unsigned long
 iov_pfn,
+		     struct scatterlist *sg, unsigned long phys_pfn,
+		     unsigned long nr_pages, int prot)

{

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

 	struct dma_pte *first_pte = NULL, *pte = NULL;
 	phys_addr_t uninitialized_var(pteval);

@@ -2223,13 +2244,6 @@ static int __domain_mapping(struct
 dmar_domain *domain, unsigned long iov_pfn,
 	unsigned int largepage_lvl = 0;
 	unsigned long lvl_pages = 0;
 
-	BUG_ON(!domain_pfn_supported(domain, iov_pfn + nr_pages - 1));
-
-	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
-		return -EINVAL;
-
-	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
-
 	if (!sg) {
 		sg_res = nr_pages;

pteval = ((phys_addr_t)phys_pfn << VTD_PAGE_SHIFT) |
prot;

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

@@ -2328,6 +2342,62 @@ static int __domain_mapping(struct
 dmar_domain *domain, unsigned long iov_pfn,
 	return 0;
 }
 
+static int
+__domain_mapping_mm(struct dmar_domain *domain, unsigned long
 iov_pfn,
+		    struct scatterlist *sg, unsigned long phys_pfn,
+		    unsigned long nr_pages, int prot)
+{
+	int ret = 0;
+
+	if (!sg)
+		return intel_mmmap_range(domain,
 dma_pfn_to_addr(iov_pfn),
+					 dma_pfn_to_addr(iov_pfn +

nr_pages),

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

+					 dma_pfn_to_addr(phys_pfn), prot);
+
+	while (nr_pages > 0) {
+		unsigned long sg_pages, phys;
+		unsigned long pgoff = sg->offset & ~PAGE_MASK;
+
+		sg_pages = aligned_nrpages(sg->offset, sg->length);
+		phys = sg_phys(sg) - pgoff;
+
+		ret = intel_mmmap_range(domain,

dma_pfn_to_addr(iov_pfn),

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

+					dma_pfn_to_addr(iov_pfn +

sg_pages),

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

+					phys, prot);
+		if (ret)
+			break;
+
+		sg->dma_address =

((dma_addr_t)dma_pfn_to_addr(iov_pfn)) + pgoff;

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

+		sg->dma_length = sg->length;
+
+		nr_pages -= sg_pages;
+		iov_pfn += sg_pages;
+		sg = sg_next(sg);
+	}
+
+	return ret;
+}
+
+static int
+__domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
+		 struct scatterlist *sg, unsigned long phys_pfn,
+		 unsigned long nr_pages, int prot)
+{
+	BUG_ON(!domain_pfn_supported(domain, iov_pfn + nr_pages - 1));
+
+	if ((prot & (DMA_PTE_READ|DMA_PTE_WRITE)) == 0)
+		return -EINVAL;
+
+	prot &= DMA_PTE_READ | DMA_PTE_WRITE | DMA_PTE_SNP;
+
+	if (domain_type_is_flt(domain))
+		return __domain_mapping_mm(domain, iov_pfn, sg,
+					   phys_pfn, nr_pages, prot);
+	else
+		return __domain_mapping_dma(domain, iov_pfn, sg,
+					    phys_pfn, nr_pages, prot);
+}
+
 static int domain_mapping(struct dmar_domain *domain, unsigned long

iov_pfn,

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

 			  struct scatterlist *sg, unsigned long phys_pfn,
 			  unsigned long nr_pages, int prot)
--

2.17.1

Quoted 66 lines

From: Lu Baolu [mailto:[email protected]]
Sent: Monday, September 23, 2019 8:25 PM

If a dmar domain has DOMAIN_FLAG_FIRST_LEVEL_TRANS bit set in its flags, IOMMU will use the first level page table for translation. Hence, we need to map or unmap addresses in the first level page table.

Cc Ashok Raj <[email protected]>

Cc Jacob Pan <[email protected]>

Cc Kevin Tian <[email protected]>

Cc Liu Yi L <[email protected]>

Cc Yi Sun <[email protected]>

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

---
  drivers/iommu/intel-iommu.c | 94 ++++++++++++++++++++++++++++++++-
----
  1 file changed, 82 insertions(+), 12 deletions(-)

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

index 9cfe8098d993..103480016010 100644
@@ -168,6 +168,11 @@ static inline unsigned long virt_to_dma_pfn(void
 *p)
  	return page_to_dma_pfn(virt_to_page(p));
  }
 
+static inline unsigned long dma_pfn_to_addr(unsigned long pfn)
+{
+	return pfn << VTD_PAGE_SHIFT;
+}
+
  /* global iommu list, set NULL for ignored DMAR units */
  static struct intel_iommu **g_iommus;

@@ -307,6 +312,9 @@ static int hw_pass_through = 1;
   */
  #define DOMAIN_FLAG_LOSE_CHILDREN		BIT(1)
 
+/* Domain uses first level translation for DMA remapping. */
+#define DOMAIN_FLAG_FIRST_LEVEL_TRANS		BIT(2)
+
  #define for_each_domain_iommu(idx, domain)			\
  	for (idx = 0; idx < g_num_of_iommus; idx++)		\
  		if (domain->iommu_refcnt[idx])

@@ -552,6 +560,11 @@ static inline int domain_type_is_si(struct
 dmar_domain *domain)
  	return domain->flags & DOMAIN_FLAG_STATIC_IDENTITY;
  }
 
+static inline int domain_type_is_flt(struct dmar_domain *domain)
+{
+	return domain->flags & DOMAIN_FLAG_FIRST_LEVEL_TRANS;
+}
+
  static inline int domain_pfn_supported(struct dmar_domain *domain,
  				       unsigned long pfn)

  {

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

@@ -1147,8 +1160,15 @@ static struct page *domain_unmap(struct
 dmar_domain *domain,
  	BUG_ON(start_pfn > last_pfn);
 
  	/* we don't need lock here; nobody else touches the iova range */
-	freelist = dma_pte_clear_level(domain, agaw_to_level(domain-
 > agaw),

drivers/iommu/intel-iommu.c static struct page *domain_unmap(struct · line 1174

Quoted 11 lines

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

-				       domain->pgd, 0, start_pfn, last_pfn,
 NULL);
+	if (domain_type_is_flt(domain))
+		freelist = intel_mmunmap_range(domain,
+					       dma_pfn_to_addr(start_pfn),
+					       dma_pfn_to_addr(last_pfn + 1));
+	else
+		freelist = dma_pte_clear_level(domain,
+					       agaw_to_level(domain->agaw),
+					       domain->pgd, 0, start_pfn,
+					       last_pfn, NULL);

what about providing an unified interface at the caller side, then having the level differentiated within the interface?

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

+/*
+ * Check and return whether first level is used by default for
+ * DMA translation.
+ */
+static bool first_level_by_default(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:50 PM

Quoted 9 lines · Lu Baolu

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

+/*
+ * Check and return whether first level is used by default for
+ * DMA translation.
+ */
+static bool first_level_by_default(void)
+{
+	struct dmar_drhd_unit *drhd;
+	struct intel_iommu *iommu;
+

drivers/iommu/intel-iommu.c static void free_dmar_iommu(struct intel_iommu *iommu) · line 1743

Quoted 10 lines

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

+	rcu_read_lock();
+	for_each_active_iommu(iommu, drhd)
+		if (!sm_supported(iommu) ||
+		    !ecap_flts(iommu->ecap) ||
+		    !cap_caching_mode(iommu->cap))
+			return false;
+	rcu_read_unlock();
+
+	return true;
+}

"If no caching mode, then we will not use 1st level."

Hmm, does the vIOMMU needs to support caching-mode if with the solution you proposed here? Caching mode is only necessary for shadowing AFAICT, and after all you're going to use full-nested, then... then I would think it's not needed. And if so, with this patch 1st level will be disabled. Sounds like a paradox...

I'm thinking what would be the big picture for this to work now: For the vIOMMU, instead of exposing the caching-mode, I'm thinking maybe we should expose it with ecap.FLTS=1 while we can keep ecap.SLTS=0 then it's natural that we can only use 1st level translation in the guest for all the domains (and I assume such an ecap value should never happen on real hardware, am I right?).

This patchset aims to move IOVA (I/O Virtual Address) translation to 1st-level page table under scalable mode. The major purpose of this effort is to make guest IOVA support more efficient.

As Intel VT-d architecture offers caching-mode, guest IOVA (GIOVA) support is now implemented in a shadow page manner. The device simulation software, like QEMU, has to figure out GIOVA->GPA mapping and writes to a shadowed page table, which will be used by pIOMMU. Each time when mappings are created or destroyed in vIOMMU, the simulation software will intervene. The change on GIOVA->GPA will be shadowed to host, and the pIOMMU will be updated via VFIO/IOMMU interfaces.

     .-----------.
     |  vIOMMU   |
     |-----------|                 .--------------------.
     |           |IOTLB flush trap |        QEMU        |
     .-----------. (map/unmap)     |--------------------|
     | GVA->GPA  |---------------->|      .----------.  |
     '-----------'                 |      | GPA->HPA |  |
     |           |                 |      '----------'  |
     '-----------'                 |                    |
                                   |                    |
                                   '--------------------'
                                                |
            <------------------------------------
            |
            v VFIO/IOMMU API
      .-----------.
      |  pIOMMU   |
      |-----------|
      |           |
      .-----------.
      | GVA->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 is more secure and software is simplified 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)      |-----------|
     | GVA->GPA  |---------------->|           |
     '-----------'                 '-----------'
     |           |                       |
     '-----------'                       |
           <------------------------------
           |      VFIO/IOMMU          
           |  cache invalidation and  
           | guest gpd bind interfaces
           v

     .-----------.
     |  pIOMMU   |
     |-----------|
     .-----------.
     | GVA->GPA  |<---First level
     '-----------'
     | GPA->HPA  |<---Scond level
     '-----------'
     '-----------'

This patch series only aims to achieve the first goal, a.k.a using first level translation for IOVA mappings in vIOMMU. I am sending it out for your comments. Any comments, suggestions and concerns are welcomed.

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

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

Lu Baolu (4):
iommu/vt-d: Move domain_flush_cache helper into header iommu/vt-d: Add first level page table interfaces iommu/vt-d: Map/unmap domain with mmmap/mmunmap iommu/vt-d: Identify domains using first level page table

 drivers/iommu/Makefile             |   2 +-
 drivers/iommu/intel-iommu.c        | 142 ++++++++++--
 drivers/iommu/intel-pgtable.c      | 342
+++++++++++++++++++++++++++++ include/linux/intel-iommu.h        |
31 ++- include/trace/events/intel_iommu.h |  60 +++++
 5 files changed, 553 insertions(+), 24 deletions(-)
 create mode 100644 drivers/iommu/intel-pgtable.c

Quoted 12 lines

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 is more secure and software is simplified as we only need to flush the pIOMMU IOTLB and possible device-IOTLB when an IOVA mapping in vIOMMU is torn down.

Quoted 14 lines

     .-----------.
     |  vIOMMU   |
     |-----------|                 .-----------.
     |           |IOTLB flush trap |   QEMU    |
     .-----------.    (unmap)      |-----------|
     | GVA->GPA  |---------------->|           |
     '-----------'                 '-----------'
     |           |                       |
     '-----------'                       |
           <------------------------------
           |      VFIO/IOMMU          
           |  cache invalidation and  
           | guest gpd bind interfaces
           v

For vSVA, the guest PGD bind interface will mark the PASID as guest PASID and will inject page request into the guest. In FL gIOVA case, I guess we are assuming there is no page fault for GIOVA. I will need to add a flag in the gpgd bind such that any PRS will be auto responded with invalid.

Also, native use of IOVA FL map is not to be supported? i.e. IOMMU API and DMA API for native usage will continue to be SL only?

Quoted 7 lines

     .-----------.
     |  pIOMMU   |
     |-----------|
     .-----------.
     | GVA->GPA  |<---First level
     '-----------'
     | GPA->HPA  |<---Scond level

Quoted 22 lines

     '-----------'
     '-----------'

This patch series only aims to achieve the first goal, a.k.a using first level translation for IOVA mappings in vIOMMU. I am sending it out for your comments. Any comments, suggestions and concerns are welcomed.

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

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

Lu Baolu (4):
iommu/vt-d: Move domain_flush_cache helper into header iommu/vt-d: Add first level page table interfaces iommu/vt-d: Map/unmap domain with mmmap/mmunmap iommu/vt-d: Identify domains using first level page table

Quoted 8 lines

 drivers/iommu/Makefile             |   2 +-
 drivers/iommu/intel-iommu.c        | 142 ++++++++++--
 drivers/iommu/intel-pgtable.c      | 342
+++++++++++++++++++++++++++++ include/linux/intel-iommu.h        |
31 ++- include/trace/events/intel_iommu.h |  60 +++++
 5 files changed, 553 insertions(+), 24 deletions(-)
 create mode 100644 drivers/iommu/intel-pgtable.c

[Jacob Pan]

Hi Jacob

Quoted 26 lines · Jacob Pan

Quoted 26 lines

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 is more secure and software is simplified 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)      |-----------|
      | GVA->GPA  |---------------->|           |
      '-----------'                 '-----------'
      |           |                       |
      '-----------'                       |
            <------------------------------
            |      VFIO/IOMMU
            |  cache invalidation and
            | guest gpd bind interfaces
            v

Quoted 5 lines

For vSVA, the guest PGD bind interface will mark the PASID as guest PASID and will inject page request into the guest. In FL gIOVA case, I guess we are assuming there is no page fault for GIOVA. I will need to add a flag in the gpgd bind such that any PRS will be auto responded with invalid.

Is there real need to enforce this? I'm not sure if there is any limitation in the spec, and if so, can the guest check that instead?

Also i believe the idea is to overcommit PASID#0 such uses. Thought we had a capability to expose this to the vIOMMU as well. Not sure if this is already documented, if not should be up in the next rev.

Quoted 10 lines

Also, native use of IOVA FL map is not to be supported? i.e. IOMMU API and DMA API for native usage will continue to be SL only?

Quoted 7 lines

      .-----------.
      |  pIOMMU   |
      |-----------|
      .-----------.
      | GVA->GPA  |<---First level
      '-----------'
      | GPA->HPA  |<---Scond level

s/Scond/Second

From: Raj, Ashok
Sent: Tuesday, September 24, 2019 4:26 AM

Hi Jacob

Quoted 6 lines · Jacob Pan

Quoted 6 lines

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

Quoted 13 lines

Quoted 13 lines

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 is more secure and software is simplified 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)      |-----------|
     | GVA->GPA  |---------------->|           |
     '-----------'                 '-----------'

Quoted 7 lines

Quoted 7 lines

     |           |                       |
     '-----------'                       |

<------------------------------
| VFIO/IOMMU
| cache invalidation and
| guest gpd bind interfaces
v

Quoted 5 lines

For vSVA, the guest PGD bind interface will mark the PASID as guest PASID and will inject page request into the guest. In FL gIOVA case, I guess we are assuming there is no page fault for GIOVA. I will need to add a flag in the gpgd bind such that any PRS will be auto responded with invalid.

Is there real need to enforce this? I'm not sure if there is any limitation in the spec, and if so, can the guest check that instead?

Also i believe the idea is to overcommit PASID#0 such uses. Thought we had a capability to expose this to the vIOMMU as well. Not sure if this is already documented, if not should be up in the next rev.

Quoted 10 lines

Also, native use of IOVA FL map is not to be supported? i.e. IOMMU API and DMA API for native usage will continue to be SL only?

Quoted 7 lines

     .-----------.
     |  pIOMMU   |
     |-----------|
     .-----------.
     | GVA->GPA  |<---First level
     '-----------'
     | GPA->HPA  |<---Scond level

s/Scond/Second

Quoted 4 lines

Quoted 4 lines

     '-----------'
     '-----------'

This patch series only aims to achieve the first goal, a.k.a using

Quoted 28 lines

Quoted 18 lines

first level translation for IOVA mappings in vIOMMU. I am sending it out for your comments. Any comments, suggestions and concerns are welcomed.

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

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

Lu Baolu (4):
iommu/vt-d: Move domain_flush_cache helper into header iommu/vt-d: Add first level page table interfaces iommu/vt-d: Map/unmap domain with mmmap/mmunmap iommu/vt-d: Identify domains using first level page table

Quoted 8 lines

 drivers/iommu/Makefile             |   2 +-
 drivers/iommu/intel-iommu.c        | 142 ++++++++++--
 drivers/iommu/intel-pgtable.c      | 342
+++++++++++++++++++++++++++++ include/linux/intel-iommu.h        |
31 ++- include/trace/events/intel_iommu.h |  60 +++++
 5 files changed, 553 insertions(+), 24 deletions(-)
 create mode 100644 drivers/iommu/intel-pgtable.c

[Jacob Pan]

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

      '-----------'
      '-----------'

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

Also earlier you mentioned the new approach (nested) is more secure than shadowing. why?

Hi Kevin,

Quoted 6 lines · Kevin

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

      '-----------'
      '-----------'

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page table and VMEXIT when map an gIOVA. This includes below 4 steps (maybe not accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:57 PM

Quoted 11 lines · Lu Baolu

Hi Kevin,

Quoted 6 lines · Kevin

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

      '-----------'
      '-----------'

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page

table

Quoted 9 lines

and VMEXIT when map an gIOVA. This includes below 4 steps (maybe not accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

Would it make sense to use 1st level even for bare-metal to replace the 2nd level?

What I'm thinking is the DPDK apps - they have MMU page table already there for the huge pages, then if they can use 1st level as the default device page table then it even does not need to map, because it can simply bind the process root page table pointer to the 1st level page root pointer of the device contexts that it uses.

Quoted 26 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:57 PM

Quoted 11 lines · Lu Baolu

Hi Kevin,

Quoted 6 lines · Kevin

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

      '-----------'
      '-----------'

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page

table

Quoted 9 lines

and VMEXIT when map an gIOVA. This includes below 4 steps (maybe not accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

Quoted 9 lines

Would it make sense to use 1st level even for bare-metal to replace the 2nd level?

What I'm thinking is the DPDK apps - they have MMU page table already there for the huge pages, then if they can use 1st level as the default device page table then it even does not need to map, because it can simply bind the process root page table pointer to the 1st level page root pointer of the device contexts that it uses.

Then you need bear with possible page faults from using CPU page table, while most devices don't support it today.

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 3:45 PM

Quoted 10 lines · Kevin

Quoted 10 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:57 PM

Quoted 6 lines · Lu Baolu

Hi Kevin,

Quoted 3 lines · Kevin

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

      '-----------'
      '-----------'

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page

table

Quoted 1 line

and VMEXIT when map an gIOVA. This includes below 4 steps (maybe

not

Quoted 19 lines

Quoted 18 lines

Quoted 8 lines

accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

Would it make sense to use 1st level even for bare-metal to replace the 2nd level?

What I'm thinking is the DPDK apps - they have MMU page table already there for the huge pages, then if they can use 1st level as the default device page table then it even does not need to map, because it can simply bind the process root page table pointer to the 1st level page root pointer of the device contexts that it uses.

Quoted 2 lines

Then you need bear with possible page faults from using CPU page table, while most devices don't support it today.

Right, I was just thinking aloud. After all neither do we have IOMMU hardware to support 1st level (or am I wrong?)... It's just that when

the 1st level is ready it should sound doable because IIUC PRI should be always with the 1st level support no matter on IOMMU side or the device side?

I'm actually not sure about whether my understanding here is correct... I thought the pasid binding previously was only for some vendor kernel drivers but not a general thing to userspace. I feel like that should be doable in the future once we've got some new syscall interface ready to deliver 1st level page table (e.g., via vfio?) then applications like DPDK seems to be able to use that too even directly via bare metal.

Quoted 41 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 3:45 PM

Quoted 10 lines · Kevin

Quoted 10 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:57 PM

Quoted 6 lines · Lu Baolu

Hi Kevin,

Quoted 3 lines · Kevin

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

      '-----------'
      '-----------'

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page

table

Quoted 1 line

and VMEXIT when map an gIOVA. This includes below 4 steps (maybe

not

Quoted 19 lines

Quoted 18 lines

Quoted 8 lines

accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

Would it make sense to use 1st level even for bare-metal to replace the 2nd level?

What I'm thinking is the DPDK apps - they have MMU page table already there for the huge pages, then if they can use 1st level as the default device page table then it even does not need to map, because it can simply bind the process root page table pointer to the 1st level page root pointer of the device contexts that it uses.

Quoted 5 lines

Quoted 2 lines

Then you need bear with possible page faults from using CPU page table, while most devices don't support it today.

Right, I was just thinking aloud. After all neither do we have IOMMU hardware to support 1st level (or am I wrong?)... It's just that when

You are right. Current VT-d supports only 2nd level.

Quoted 3 lines

the 1st level is ready it should sound doable because IIUC PRI should be always with the 1st level support no matter on IOMMU side or the device side?

No. PRI is not tied to 1st or 2nd level. Actually from device p.o.v, it's just a protocol to trigger page fault, but the device doesn't care whether the page fault is on 1st or 2nd level in the IOMMU side. The only relevant part is that a PRI request can have PASID tagged or cleared. When it's tagged with PASID, the IOMMU will locate the translation table under the given PASID (either 1st or 2nd level is fine, according to PASID entry setting). When no PASID is included, the IOMMU locates the translation from default entry (e.g. PASID#0 or any PASID contained in RID2PASID in VT-d).

Your knowledge happened to be correct in deprecated ECS mode. At that time, there is only one 2nd level per context entry which doesn't support page fault, and there is only one 1st level per PASID entry which supports page fault. Then PRI could be indirectly connected to 1st level, but this just changed with new scalable mode.

Another note is that the PRI capability only indicates that a device is capable of handling page faults, but not that a device can tolerate page fault for any of its DMA access. If the latter is fasle, using CPU page table for DPDK usage is still risky (and specific to device behavior)

Quoted 9 lines

I'm actually not sure about whether my understanding here is correct... I thought the pasid binding previously was only for some vendor kernel drivers but not a general thing to userspace. I feel like that should be doable in the future once we've got some new syscall interface ready to deliver 1st level page table (e.g., via vfio?) then applications like DPDK seems to be able to use that too even directly via bare metal.

using 1st level for userspace is different from supporting DMA page fault in userspace. The former is purely about which structure to keep the mapping. I think we may do the same thing for both bare metal and guest (using 2nd level only for GPA when nested is enabled on the IOMMU). But reusing CPU page table for userspace is more tricky. :-)

Quoted 84 lines · Kevin

Quoted 41 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 3:45 PM

Quoted 10 lines · Kevin

Quoted 10 lines

From: Peter Xu [mailto:[email protected]]
Sent: Wednesday, September 25, 2019 2:57 PM

Quoted 6 lines · Lu Baolu

Hi Kevin,

Quoted 3 lines · Kevin

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

       '-----------'
       '-----------'

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Quoted 1 line

Quoted 1 line

This patch series only aims to achieve the first goal, a.k.a using

first goal? then what are other goals? I didn't spot such information.

The overall goal is to use IOMMU nested mode to avoid shadow page

table

Quoted 1 line

and VMEXIT when map an gIOVA. This includes below 4 steps (maybe

not

Quoted 19 lines

Quoted 18 lines

Quoted 8 lines

accurate, but you could get the point.)

1) GIOVA mappings over 1st-level page table;
2) binding vIOMMU 1st level page table to the pIOMMU;
3) using pIOMMU second level for GPA->HPA translation;
4) enable nested (a.k.a. dual stage) translation in host.

This patch set aims to achieve 1).

Would it make sense to use 1st level even for bare-metal to replace the 2nd level?

What I'm thinking is the DPDK apps - they have MMU page table already there for the huge pages, then if they can use 1st level as the default device page table then it even does not need to map, because it can simply bind the process root page table pointer to the 1st level page root pointer of the device contexts that it uses.

Quoted 5 lines

Quoted 2 lines

Then you need bear with possible page faults from using CPU page table, while most devices don't support it today.

Right, I was just thinking aloud. After all neither do we have IOMMU hardware to support 1st level (or am I wrong?)... It's just that when

You are right. Current VT-d supports only 2nd level.

Quoted 3 lines

the 1st level is ready it should sound doable because IIUC PRI should be always with the 1st level support no matter on IOMMU side or the device side?

No. PRI is not tied to 1st or 2nd level. Actually from device p.o.v, it's just a protocol to trigger page fault, but the device doesn't care whether the page fault is on 1st or 2nd level in the IOMMU side. The only relevant part is that a PRI request can have PASID tagged or cleared. When it's tagged with PASID, the IOMMU will locate the translation table under the given PASID (either 1st or 2nd level is fine, according to PASID entry setting). When no PASID is included, the IOMMU locates the translation from default entry (e.g. PASID#0 or any PASID contained in RID2PASID in VT-d).

Your knowledge happened to be correct in deprecated ECS mode. At that time, there is only one 2nd level per context entry which doesn't support page fault, and there is only one 1st level per PASID entry which supports page fault. Then PRI could be indirectly connected to 1st level, but this just changed with new scalable mode.

Another note is that the PRI capability only indicates that a device is capable of handling page faults, but not that a device can tolerate page fault for any of its DMA access. If the latter is fasle, using CPU page table for DPDK usage is still risky (and specific to device behavior)

Quoted 9 lines

I'm actually not sure about whether my understanding here is correct... I thought the pasid binding previously was only for some vendor kernel drivers but not a general thing to userspace. I feel like that should be doable in the future once we've got some new syscall interface ready to deliver 1st level page table (e.g., via vfio?) then applications like DPDK seems to be able to use that too even directly via bare metal.

Quoted 6 lines

using 1st level for userspace is different from supporting DMA page fault in userspace. The former is purely about which structure to keep the mapping. I think we may do the same thing for both bare metal and guest (using 2nd level only for GPA when nested is enabled on the IOMMU). But reusing CPU page table for userspace is more tricky. :-)

Yes I should have mixed up the 1st level page table and PRI a bit, and after all my initial question should be irrelevant to this series as well so it's already a bit out of topic (sorry for that).

And, thanks for explaining these. :)

Hi Baolu,

Quoted 50 lines · On Mon, 23 Sep 2019 20:24:50 +0800 Lu Baolu <[email protected]> wrote:

This patchset aims to move IOVA (I/O Virtual Address) translation to 1st-level page table under scalable mode. The major purpose of this effort is to make guest IOVA support more efficient.

As Intel VT-d architecture offers caching-mode, guest IOVA (GIOVA) support is now implemented in a shadow page manner. The device simulation software, like QEMU, has to figure out GIOVA->GPA mapping and writes to a shadowed page table, which will be used by pIOMMU. Each time when mappings are created or destroyed in vIOMMU, the simulation software will intervene. The change on GIOVA->GPA will be shadowed to host, and the pIOMMU will be updated via VFIO/IOMMU interfaces.

      .-----------.
      |  vIOMMU   |
      |-----------|                 .--------------------.
      |           |IOTLB flush trap |        QEMU        |
      .-----------. (map/unmap)     |--------------------|
      | GVA->GPA  |---------------->|      .----------.  |
      '-----------'                 |      | GPA->HPA |  |
      |           |                 |      '----------'  |
      '-----------'                 |                    |
                                    |                    |
                                    '--------------------'
                                                 |
             <------------------------------------
             |
             v VFIO/IOMMU API
       .-----------.
       |  pIOMMU   |
       |-----------|
       |           |
       .-----------.
       | GVA->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 is more secure and software is simplified as we only need to flush the pIOMMU IOTLB and possible device-IOTLB when an IOVA mapping in vIOMMU is torn down.

Quoted 14 lines

      .-----------.
      |  vIOMMU   |
      |-----------|                 .-----------.
      |           |IOTLB flush trap |   QEMU    |
      .-----------.    (unmap)      |-----------|
      | GVA->GPA  |---------------->|           |
      '-----------'                 '-----------'
      |           |                       |
      '-----------'                       |
            <------------------------------
            |      VFIO/IOMMU
            |  cache invalidation and
            | guest gpd bind interfaces
            v

For vSVA, the guest PGD bind interface will mark the PASID as guest PASID and will inject page request into the guest. In FL gIOVA case, I guess we are assuming there is no page fault for GIOVA. I will need to add a flag in the gpgd bind such that any PRS will be auto responded with invalid.

Also, native use of IOVA FL map is not to be supported? i.e. IOMMU API and DMA API for native usage will continue to be SL only?

Quoted 29 lines

      .-----------.
      |  pIOMMU   |
      |-----------|
      .-----------.
      | GVA->GPA  |<---First level
      '-----------'
      | GPA->HPA  |<---Scond level
      '-----------'
      '-----------'

This patch series only aims to achieve the first goal, a.k.a using first level translation for IOVA mappings in vIOMMU. I am sending it out for your comments. Any comments, suggestions and concerns are welcomed.

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

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

Lu Baolu (4):
iommu/vt-d: Move domain_flush_cache helper into header iommu/vt-d: Add first level page table interfaces iommu/vt-d: Map/unmap domain with mmmap/mmunmap iommu/vt-d: Identify domains using first level page table

Quoted 8 lines

  drivers/iommu/Makefile             |   2 +-
  drivers/iommu/intel-iommu.c        | 142 ++++++++++--
  drivers/iommu/intel-pgtable.c      | 342
+++++++++++++++++++++++++++++ include/linux/intel-iommu.h        |
31 ++- include/trace/events/intel_iommu.h |  60 +++++
  5 files changed, 553 insertions(+), 24 deletions(-)

create mode 100644 drivers/iommu/intel-pgtable.c

[Jacob Pan]