[PATCH 0/7] amd iommu smart tlb flushing

[PATCH 0/7] amd iommu smart tlb flushing

[Joerg Roedel]

Hi,

this patchset implements a smarter io-tlb flushing strategy for the AMD IOMMU
driver. The old behavior was to flush only a single 4kb entry or the whole
domain io-tlb. This patchset implements book keeping about the ranges of PTEs
changed in the page table and flushes only that ranges when the flush is
actually done.

	Joerg

Joerg Roedel (7):
      x86/amd-iommu: Add flush_info to protection domains
      x86/amd-iommu: Introduce iommu_update_domain_tlb function
      x86/amd-iommu: Move dte udpate flag into flush_info structure
      x86/amd-iommu: Move functions to get rid of forward declarations
      x86/amd-iommu: Add function to commit domain changes
      x86/amd-iommu: Introduce iommu_update_dma_ops_domain()
      x86/amd-iommu: Reimplement iommu_flush_tlb_pde using iommu_update_domain_tlb

 arch/x86/include/asm/amd_iommu_types.h |   13 ++-
 arch/x86/kernel/amd_iommu.c            |  225 ++++++++++++++++----------------
 2 files changed, 124 insertions(+), 114 deletions(-)

[PATCH 1/7] x86/amd-iommu: Add flush_info to protection domains

[Joerg Roedel]

This patch adds a new sub-struct to protection domains which
is used to keep information about what parts of the domain
needs to be flushed on the hardware side.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/amd_iommu_types.h |   11 +++++++++++
 arch/x86/kernel/amd_iommu.c            |   23 +++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index ba19ad4..30c4410 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -230,6 +230,16 @@ extern bool amd_iommu_np_cache;
 #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
 
 /*
+ * This struct holds information about the parts of a protection domain that
+ * needs to be flushed on the IOMMU hardware.
+ */
+struct flush_info {
+	bool tlb;
+	u64 start;
+	u64 end;
+};
+
+/*
  * This structure contains generic data for  IOMMU protection domains
  * independent of their use.
  */
@@ -244,6 +254,7 @@ struct protection_domain {
 	bool updated;		/* complete domain flush required */
 	unsigned dev_cnt;	/* devices assigned to this domain */
 	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
+	struct flush_info flush;
 	void *priv;		/* private data */
 
 };
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index adb0ba0..fcb85e8 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -78,6 +78,19 @@ static struct iommu_dev_data *get_dev_data(struct device *dev)
 	return dev->archdata.iommu;
 }
 
+static void update_flush_info_tlb(struct protection_domain *domain,
+				  u64 start, u64 end)
+{
+	if (!domain->flush.tlb) {
+		domain->flush.tlb   = true;
+		domain->flush.start = start;
+		domain->flush.end   = end;
+	} else {
+		domain->flush.start = min(start, domain->flush.start);
+		domain->flush.end   = max(end  , domain->flush.end);
+	}
+}
+
 /*
  * In this function the list of preallocated protection domains is traversed to
  * find the domain for a specific device
@@ -1849,6 +1862,9 @@ retry:
 
 	ADD_STATS_COUNTER(alloced_io_mem, size);
 
+	if (unlikely(amd_iommu_np_cache))
+		update_flush_info_tlb(&dma_dom->domain, start, size);
+
 	if (unlikely(dma_dom->need_flush && !amd_iommu_unmap_flush)) {
 		iommu_flush_tlb(&dma_dom->domain);
 		dma_dom->need_flush = false;
@@ -1895,6 +1911,8 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 		start += PAGE_SIZE;
 	}
 
+	update_flush_info_tlb(&dma_dom->domain, dma_addr, size);
+
 	SUB_STATS_COUNTER(alloced_io_mem, size);
 
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
@@ -2465,6 +2483,9 @@ static int amd_iommu_map_range(struct iommu_domain *dom,
 		paddr += PAGE_SIZE;
 	}
 
+	if (unlikely(amd_iommu_np_cache))
+		update_flush_info_tlb(domain, iova, iova + size);
+
 	return 0;
 }
 
@@ -2482,6 +2503,8 @@ static void amd_iommu_unmap_range(struct iommu_domain *dom,
 		iova  += PAGE_SIZE;
 	}
 
+	update_flush_info_tlb(domain, iova, iova + size);
+
 	iommu_flush_tlb_pde(domain);
 }
 
-- 
1.6.6

[PATCH 2/7] x86/amd-iommu: Introduce iommu_update_domain_tlb function

[Joerg Roedel]

This patch introduces a function which only flushes the
necessary parts of the IO/TLB for a domain using size aware
flushing commands.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   51 +++++++++++++++++++++++++++---------------
 1 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index fcb85e8..9318512 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -617,24 +617,35 @@ static void __iommu_flush_pages(struct protection_domain *domain,
 	return;
 }
 
-static void iommu_flush_pages(struct protection_domain *domain,
-			     u64 address, size_t size)
-{
-	__iommu_flush_pages(domain, address, size, 0);
-}
-
-/* Flush the whole IO/TLB for a given protection domain */
-static void iommu_flush_tlb(struct protection_domain *domain)
-{
-	__iommu_flush_pages(domain, 0, CMD_INV_IOMMU_ALL_PAGES_ADDRESS, 0);
-}
-
 /* Flush the whole IO/TLB for a given protection domain - including PDE */
 static void iommu_flush_tlb_pde(struct protection_domain *domain)
 {
 	__iommu_flush_pages(domain, 0, CMD_INV_IOMMU_ALL_PAGES_ADDRESS, 1);
 }
 
+/* Flush a range of pages in the domain */
+static void iommu_update_domain_tlb(struct protection_domain *domain, bool pde)
+{
+	u64 address, mask;
+	int i, order;
+
+	if (!domain->flush.tlb)
+		return;
+
+	order   = get_order(domain->flush.end - domain->flush.start);
+	mask    = (0x1000ULL << order) - 1;
+	address = ((domain->flush.start & ~mask) | (mask >> 1)) & ~0xfffULL;
+
+	for (i = 0; i < amd_iommus_present; ++i) {
+		if (!domain->dev_iommu[i])
+			continue;
+
+		iommu_queue_inv_iommu_pages(amd_iommus[i], address,
+					    domain->id, pde, (order != 0));
+	}
+
+	domain->flush.tlb = false;
+}
 
 /*
  * This function flushes the DTEs for all devices in domain
@@ -1865,11 +1876,11 @@ retry:
 	if (unlikely(amd_iommu_np_cache))
 		update_flush_info_tlb(&dma_dom->domain, start, size);
 
-	if (unlikely(dma_dom->need_flush && !amd_iommu_unmap_flush)) {
-		iommu_flush_tlb(&dma_dom->domain);
+	if (unlikely((dma_dom->need_flush && !amd_iommu_unmap_flush)) ||
+		      amd_iommu_np_cache) {
+		iommu_update_domain_tlb(&dma_dom->domain, false);
 		dma_dom->need_flush = false;
-	} else if (unlikely(amd_iommu_np_cache))
-		iommu_flush_pages(&dma_dom->domain, address, size);
+	}
 
 out:
 	return address;
@@ -1918,7 +1929,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
 
 	if (amd_iommu_unmap_flush || dma_dom->need_flush) {
-		iommu_flush_pages(&dma_dom->domain, dma_addr, size);
+		iommu_update_domain_tlb(&dma_dom->domain, false);
 		dma_dom->need_flush = false;
 	}
 }
@@ -2486,6 +2497,9 @@ static int amd_iommu_map_range(struct iommu_domain *dom,
 	if (unlikely(amd_iommu_np_cache))
 		update_flush_info_tlb(domain, iova, iova + size);
 
+	iommu_update_domain_tlb(domain, true);
+	iommu_flush_complete(domain);
+
 	return 0;
 }
 
@@ -2505,7 +2519,8 @@ static void amd_iommu_unmap_range(struct iommu_domain *dom,
 
 	update_flush_info_tlb(domain, iova, iova + size);
 
-	iommu_flush_tlb_pde(domain);
+	iommu_update_domain_tlb(domain, true);
+	iommu_flush_complete(domain);
 }
 
 static phys_addr_t amd_iommu_iova_to_phys(struct iommu_domain *dom,
-- 
1.6.6

[PATCH 3/7] x86/amd-iommu: Move dte udpate flag into flush_info structure

[Joerg Roedel]

This patch moves the flag to indicate a DTE flush is needed
to the new flush_info structure. It also renames the
update_domain function to iommu_update_domain_dte to match
the name with iommu_update_domain_tlb nomenclature.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/amd_iommu_types.h |    2 +-
 arch/x86/kernel/amd_iommu.c            |   22 +++++++++++-----------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
index 30c4410..529180f 100644
--- a/arch/x86/include/asm/amd_iommu_types.h
+++ b/arch/x86/include/asm/amd_iommu_types.h
@@ -234,6 +234,7 @@ extern bool amd_iommu_np_cache;
  * needs to be flushed on the IOMMU hardware.
  */
 struct flush_info {
+	bool dte;
 	bool tlb;
 	u64 start;
 	u64 end;
@@ -251,7 +252,6 @@ struct protection_domain {
 	int mode;		/* paging mode (0-6 levels) */
 	u64 *pt_root;		/* page table root pointer */
 	unsigned long flags;	/* flags to find out type of domain */
-	bool updated;		/* complete domain flush required */
 	unsigned dev_cnt;	/* devices assigned to this domain */
 	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
 	struct flush_info flush;
diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 9318512..02ab775 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -58,7 +58,7 @@ struct iommu_cmd {
 };
 
 static void reset_iommu_command_buffer(struct amd_iommu *iommu);
-static void update_domain(struct protection_domain *domain);
+static void iommu_update_domain_dte(struct protection_domain *domain);
 
 /****************************************************************************
  *
@@ -745,9 +745,9 @@ static bool increase_address_space(struct protection_domain *domain,
 
 	*pte             = PM_LEVEL_PDE(domain->mode,
 					virt_to_phys(domain->pt_root));
-	domain->pt_root  = pte;
-	domain->mode    += 1;
-	domain->updated  = true;
+	domain->pt_root    = pte;
+	domain->mode      += 1;
+	domain->flush.dte  = true;
 
 	return true;
 }
@@ -856,7 +856,7 @@ static int iommu_map_page(struct protection_domain *dom,
 
 	*pte = __pte;
 
-	update_domain(dom);
+	iommu_update_domain_dte(dom);
 
 	return 0;
 }
@@ -1072,12 +1072,12 @@ static int alloc_new_range(struct dma_ops_domain *dma_dom,
 		dma_ops_reserve_addresses(dma_dom, i << PAGE_SHIFT, 1);
 	}
 
-	update_domain(&dma_dom->domain);
+	iommu_update_domain_dte(&dma_dom->domain);
 
 	return 0;
 
 out_free:
-	update_domain(&dma_dom->domain);
+	iommu_update_domain_dte(&dma_dom->domain);
 
 	free_page((unsigned long)dma_dom->aperture[index]->bitmap);
 
@@ -1708,16 +1708,16 @@ static void update_device_table(struct protection_domain *domain)
 	}
 }
 
-static void update_domain(struct protection_domain *domain)
+static void iommu_update_domain_dte(struct protection_domain *domain)
 {
-	if (!domain->updated)
+	if (!domain->flush.dte)
 		return;
 
 	update_device_table(domain);
 	iommu_flush_domain_devices(domain);
 	iommu_flush_tlb_pde(domain);
 
-	domain->updated = false;
+	domain->flush.dte = false;
 }
 
 /*
@@ -1741,7 +1741,7 @@ static u64* dma_ops_get_pte(struct dma_ops_domain *dom,
 	} else
 		pte += PM_LEVEL_INDEX(0, address);
 
-	update_domain(&dom->domain);
+	iommu_update_domain_dte(&dom->domain);
 
 	return pte;
 }
-- 
1.6.6

[PATCH 4/7] x86/amd-iommu: Move functions to get rid of forward declarations

[Joerg Roedel]

This patch moves some functions in amd_iommu.c to get rid of
the iommu_update_domain_dte forward declaration.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   90 +++++++++++++++++++++----------------------
 1 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 02ab775..bda00f0 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -58,7 +58,6 @@ struct iommu_cmd {
 };
 
 static void reset_iommu_command_buffer(struct amd_iommu *iommu);
-static void iommu_update_domain_dte(struct protection_domain *domain);
 
 /****************************************************************************
  *
@@ -90,6 +89,38 @@ static void update_flush_info_tlb(struct protection_domain *domain,
 		domain->flush.end   = max(end  , domain->flush.end);
 	}
 }
+static void set_dte_entry(u16 devid, struct protection_domain *domain)
+{
+	u64 pte_root = virt_to_phys(domain->pt_root);
+
+	pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
+		    << DEV_ENTRY_MODE_SHIFT;
+	pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
+
+	amd_iommu_dev_table[devid].data[2] = domain->id;
+	amd_iommu_dev_table[devid].data[1] = upper_32_bits(pte_root);
+	amd_iommu_dev_table[devid].data[0] = lower_32_bits(pte_root);
+}
+
+static void clear_dte_entry(u16 devid)
+{
+	/* remove entry from the device table seen by the hardware */
+	amd_iommu_dev_table[devid].data[0] = IOMMU_PTE_P | IOMMU_PTE_TV;
+	amd_iommu_dev_table[devid].data[1] = 0;
+	amd_iommu_dev_table[devid].data[2] = 0;
+
+	amd_iommu_apply_erratum_63(devid);
+}
+
+static void update_device_table(struct protection_domain *domain)
+{
+	struct iommu_dev_data *dev_data;
+
+	list_for_each_entry(dev_data, &domain->dev_list, list) {
+		u16 devid = get_device_id(dev_data->dev);
+		set_dte_entry(devid, domain);
+	}
+}
 
 /*
  * In this function the list of preallocated protection domains is traversed to
@@ -718,6 +749,18 @@ static void reset_iommu_command_buffer(struct amd_iommu *iommu)
 	iommu->reset_in_progress = false;
 }
 
+static void iommu_update_domain_dte(struct protection_domain *domain)
+{
+	if (!domain->flush.dte)
+		return;
+
+	update_device_table(domain);
+	iommu_flush_domain_devices(domain);
+	iommu_flush_tlb_pde(domain);
+
+	domain->flush.dte = false;
+}
+
 /****************************************************************************
  *
  * The functions below are used the create the page table mappings for
@@ -1366,29 +1409,6 @@ static bool dma_ops_domain(struct protection_domain *domain)
 	return domain->flags & PD_DMA_OPS_MASK;
 }
 
-static void set_dte_entry(u16 devid, struct protection_domain *domain)
-{
-	u64 pte_root = virt_to_phys(domain->pt_root);
-
-	pte_root |= (domain->mode & DEV_ENTRY_MODE_MASK)
-		    << DEV_ENTRY_MODE_SHIFT;
-	pte_root |= IOMMU_PTE_IR | IOMMU_PTE_IW | IOMMU_PTE_P | IOMMU_PTE_TV;
-
-	amd_iommu_dev_table[devid].data[2] = domain->id;
-	amd_iommu_dev_table[devid].data[1] = upper_32_bits(pte_root);
-	amd_iommu_dev_table[devid].data[0] = lower_32_bits(pte_root);
-}
-
-static void clear_dte_entry(u16 devid)
-{
-	/* remove entry from the device table seen by the hardware */
-	amd_iommu_dev_table[devid].data[0] = IOMMU_PTE_P | IOMMU_PTE_TV;
-	amd_iommu_dev_table[devid].data[1] = 0;
-	amd_iommu_dev_table[devid].data[2] = 0;
-
-	amd_iommu_apply_erratum_63(devid);
-}
-
 static void do_attach(struct device *dev, struct protection_domain *domain)
 {
 	struct iommu_dev_data *dev_data;
@@ -1698,28 +1718,6 @@ static struct protection_domain *get_domain(struct device *dev)
 	return &dma_dom->domain;
 }
 
-static void update_device_table(struct protection_domain *domain)
-{
-	struct iommu_dev_data *dev_data;
-
-	list_for_each_entry(dev_data, &domain->dev_list, list) {
-		u16 devid = get_device_id(dev_data->dev);
-		set_dte_entry(devid, domain);
-	}
-}
-
-static void iommu_update_domain_dte(struct protection_domain *domain)
-{
-	if (!domain->flush.dte)
-		return;
-
-	update_device_table(domain);
-	iommu_flush_domain_devices(domain);
-	iommu_flush_tlb_pde(domain);
-
-	domain->flush.dte = false;
-}
-
 /*
  * This function fetches the PTE for a given address in the aperture
  */
-- 
1.6.6

[PATCH 5/7] x86/amd-iommu: Add function to commit domain changes

[Joerg Roedel]

This patch adds the function iommu_update_domain() which
makes sure that the hardware caches are in sync with the
software changes. This function is used in the iommu-api
path.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index bda00f0..4063f55 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -761,6 +761,13 @@ static void iommu_update_domain_dte(struct protection_domain *domain)
 	domain->flush.dte = false;
 }
 
+static void iommu_update_domain(struct protection_domain *domain)
+{
+	iommu_update_domain_dte(domain);
+	iommu_update_domain_tlb(domain, false);
+	iommu_flush_complete(domain);
+}
+
 /****************************************************************************
  *
  * The functions below are used the create the page table mappings for
@@ -899,8 +906,6 @@ static int iommu_map_page(struct protection_domain *dom,
 
 	*pte = __pte;
 
-	iommu_update_domain_dte(dom);
-
 	return 0;
 }
 
@@ -2495,8 +2500,7 @@ static int amd_iommu_map_range(struct iommu_domain *dom,
 	if (unlikely(amd_iommu_np_cache))
 		update_flush_info_tlb(domain, iova, iova + size);
 
-	iommu_update_domain_tlb(domain, true);
-	iommu_flush_complete(domain);
+	iommu_update_domain(domain);
 
 	return 0;
 }
@@ -2517,8 +2521,7 @@ static void amd_iommu_unmap_range(struct iommu_domain *dom,
 
 	update_flush_info_tlb(domain, iova, iova + size);
 
-	iommu_update_domain_tlb(domain, true);
-	iommu_flush_complete(domain);
+	iommu_update_domain(domain);
 }
 
 static phys_addr_t amd_iommu_iova_to_phys(struct iommu_domain *dom,
-- 
1.6.6

[PATCH 6/7] x86/amd-iommu: Introduce iommu_update_dma_ops_domain()

[Joerg Roedel]

This new functions does the same for dma_ops domains as
iommu_update_domain for other domains. For dma_ops domains
we can't use iommu_update_domain because we need to care
about lazy tlb flushing.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   45 ++++++++++++++++++++----------------------
 1 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index 4063f55..a645756 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -768,6 +768,18 @@ static void iommu_update_domain(struct protection_domain *domain)
 	iommu_flush_complete(domain);
 }
 
+static void iommu_update_dma_ops_domain(struct dma_ops_domain *dma_dom)
+{
+	iommu_update_domain_dte(&dma_dom->domain);
+
+	if (dma_dom->need_flush) {
+		iommu_update_domain_tlb(&dma_dom->domain, false);
+		dma_dom->need_flush = false;
+	}
+
+	iommu_flush_complete(&dma_dom->domain);
+}
+
 /****************************************************************************
  *
  * The functions below are used the create the page table mappings for
@@ -1120,12 +1132,9 @@ static int alloc_new_range(struct dma_ops_domain *dma_dom,
 		dma_ops_reserve_addresses(dma_dom, i << PAGE_SHIFT, 1);
 	}
 
-	iommu_update_domain_dte(&dma_dom->domain);
-
 	return 0;
 
 out_free:
-	iommu_update_domain_dte(&dma_dom->domain);
 
 	free_page((unsigned long)dma_dom->aperture[index]->bitmap);
 
@@ -1744,8 +1753,6 @@ static u64* dma_ops_get_pte(struct dma_ops_domain *dom,
 	} else
 		pte += PM_LEVEL_INDEX(0, address);
 
-	iommu_update_domain_dte(&dom->domain);
-
 	return pte;
 }
 
@@ -1876,15 +1883,10 @@ retry:
 
 	ADD_STATS_COUNTER(alloced_io_mem, size);
 
-	if (unlikely(amd_iommu_np_cache))
+	if (unlikely(amd_iommu_np_cache ||
+		     (dma_dom->need_flush && !amd_iommu_unmap_flush)))
 		update_flush_info_tlb(&dma_dom->domain, start, size);
 
-	if (unlikely((dma_dom->need_flush && !amd_iommu_unmap_flush)) ||
-		      amd_iommu_np_cache) {
-		iommu_update_domain_tlb(&dma_dom->domain, false);
-		dma_dom->need_flush = false;
-	}
-
 out:
 	return address;
 
@@ -1930,11 +1932,6 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 	SUB_STATS_COUNTER(alloced_io_mem, size);
 
 	dma_ops_free_addresses(dma_dom, dma_addr, pages);
-
-	if (amd_iommu_unmap_flush || dma_dom->need_flush) {
-		iommu_update_domain_tlb(&dma_dom->domain, false);
-		dma_dom->need_flush = false;
-	}
 }
 
 /*
@@ -1968,7 +1965,7 @@ static dma_addr_t map_page(struct device *dev, struct page *page,
 	if (addr == DMA_ERROR_CODE)
 		goto out;
 
-	iommu_flush_complete(domain);
+	iommu_update_dma_ops_domain(domain->priv);
 
 out:
 	spin_unlock_irqrestore(&domain->lock, flags);
@@ -1995,7 +1992,7 @@ static void unmap_page(struct device *dev, dma_addr_t dma_addr, size_t size,
 
 	__unmap_single(domain->priv, dma_addr, size, dir);
 
-	iommu_flush_complete(domain);
+	iommu_update_dma_ops_domain(domain->priv);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 }
@@ -2060,9 +2057,9 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 			goto unmap;
 	}
 
-	iommu_flush_complete(domain);
-
 out:
+	iommu_update_dma_ops_domain(domain->priv);
+
 	spin_unlock_irqrestore(&domain->lock, flags);
 
 	return mapped_elems;
@@ -2106,7 +2103,7 @@ static void unmap_sg(struct device *dev, struct scatterlist *sglist,
 		s->dma_address = s->dma_length = 0;
 	}
 
-	iommu_flush_complete(domain);
+	iommu_update_dma_ops_domain(domain->priv);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 }
@@ -2156,7 +2153,7 @@ static void *alloc_coherent(struct device *dev, size_t size,
 		goto out_free;
 	}
 
-	iommu_flush_complete(domain);
+	iommu_update_dma_ops_domain(domain->priv);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 
@@ -2188,7 +2185,7 @@ static void free_coherent(struct device *dev, size_t size,
 
 	__unmap_single(domain->priv, dma_addr, size, DMA_BIDIRECTIONAL);
 
-	iommu_flush_complete(domain);
+	iommu_update_dma_ops_domain(domain->priv);
 
 	spin_unlock_irqrestore(&domain->lock, flags);
 
-- 
1.6.6

[PATCH 7/7] x86/amd-iommu: Reimplement iommu_flush_tlb_pde using iommu_update_domain_tlb

[Joerg Roedel]

This patch gets rid of the __iommu_flush_pages function by
reimplementing iommu_flush_tlb_pde using the new
iommu_update_domain_tlb function.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kernel/amd_iommu.c |   51 ++++++-------------------------------------
 1 files changed, 7 insertions(+), 44 deletions(-)

diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
index a645756..a5a1c74 100644
--- a/arch/x86/kernel/amd_iommu.c
+++ b/arch/x86/kernel/amd_iommu.c
@@ -610,50 +610,6 @@ static int iommu_queue_inv_iommu_pages(struct amd_iommu *iommu,
 	return ret;
 }
 
-/*
- * TLB invalidation function which is called from the mapping functions.
- * It invalidates a single PTE if the range to flush is within a single
- * page. Otherwise it flushes the whole TLB of the IOMMU.
- */
-static void __iommu_flush_pages(struct protection_domain *domain,
-				u64 address, size_t size, int pde)
-{
-	int s = 0, i;
-	unsigned long pages = iommu_num_pages(address, size, PAGE_SIZE);
-
-	address &= PAGE_MASK;
-
-	if (pages > 1) {
-		/*
-		 * If we have to flush more than one page, flush all
-		 * TLB entries for this domain
-		 */
-		address = CMD_INV_IOMMU_ALL_PAGES_ADDRESS;
-		s = 1;
-	}
-
-
-	for (i = 0; i < amd_iommus_present; ++i) {
-		if (!domain->dev_iommu[i])
-			continue;
-
-		/*
-		 * Devices of this domain are behind this IOMMU
-		 * We need a TLB flush
-		 */
-		iommu_queue_inv_iommu_pages(amd_iommus[i], address,
-					    domain->id, pde, s);
-	}
-
-	return;
-}
-
-/* Flush the whole IO/TLB for a given protection domain - including PDE */
-static void iommu_flush_tlb_pde(struct protection_domain *domain)
-{
-	__iommu_flush_pages(domain, 0, CMD_INV_IOMMU_ALL_PAGES_ADDRESS, 1);
-}
-
 /* Flush a range of pages in the domain */
 static void iommu_update_domain_tlb(struct protection_domain *domain, bool pde)
 {
@@ -678,6 +634,13 @@ static void iommu_update_domain_tlb(struct protection_domain *domain, bool pde)
 	domain->flush.tlb = false;
 }
 
+/* Flush the whole IO/TLB for a given protection domain - including PDE */
+static void iommu_flush_tlb_pde(struct protection_domain *domain)
+{
+	update_flush_info_tlb(domain, 0, ~0);
+	iommu_update_domain_tlb(domain, true);
+}
+
 /*
  * This function flushes the DTEs for all devices in domain
  */
-- 
1.6.6

Re: [PATCH 1/7] x86/amd-iommu: Add flush_info to protection domains

[Don Dutile]

Joerg Roedel wrote:
> This patch adds a new sub-struct to protection domains which
> is used to keep information about what parts of the domain
> needs to be flushed on the hardware side.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/include/asm/amd_iommu_types.h |   11 +++++++++++
>  arch/x86/kernel/amd_iommu.c            |   23 +++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
> index ba19ad4..30c4410 100644
> --- a/arch/x86/include/asm/amd_iommu_types.h
> +++ b/arch/x86/include/asm/amd_iommu_types.h
> @@ -230,6 +230,16 @@ extern bool amd_iommu_np_cache;
>  #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
>  
>  /*
> + * This struct holds information about the parts of a protection domain that
> + * needs to be flushed on the IOMMU hardware.
> + */
> +struct flush_info {
> +	bool tlb;
> +	u64 start;
> +	u64 end;
> +};
> +
> +/*
>   * This structure contains generic data for  IOMMU protection domains
>   * independent of their use.
>   */
> @@ -244,6 +254,7 @@ struct protection_domain {
>  	bool updated;		/* complete domain flush required */
>  	unsigned dev_cnt;	/* devices assigned to this domain */
>  	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
> +	struct flush_info flush;
>  	void *priv;		/* private data */
>  
>  };
> diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> index adb0ba0..fcb85e8 100644
> --- a/arch/x86/kernel/amd_iommu.c
> +++ b/arch/x86/kernel/amd_iommu.c
> @@ -78,6 +78,19 @@ static struct iommu_dev_data *get_dev_data(struct device *dev)
>  	return dev->archdata.iommu;
>  }
>  
> +static void update_flush_info_tlb(struct protection_domain *domain,
> +				  u64 start, u64 end)
> +{
> +	if (!domain->flush.tlb) {
> +		domain->flush.tlb   = true;
> +		domain->flush.start = start;
> +		domain->flush.end   = end;
> +	} else {
> +		domain->flush.start = min(start, domain->flush.start);
> +		domain->flush.end   = max(end  , domain->flush.end);
> +	}
> +}
> +

the code has start/end here.... but callers below.... 

>  /*
>   * In this function the list of preallocated protection domains is traversed to
>   * find the domain for a specific device
> @@ -1849,6 +1862,9 @@ retry:
>  
>  	ADD_STATS_COUNTER(alloced_io_mem, size);
>  
> +	if (unlikely(amd_iommu_np_cache))
> +		update_flush_info_tlb(&dma_dom->domain, start, size);
> +

use start/size .... 
>  	if (unlikely(dma_dom->need_flush && !amd_iommu_unmap_flush)) {
>  		iommu_flush_tlb(&dma_dom->domain);
>  		dma_dom->need_flush = false;
> @@ -1895,6 +1911,8 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
>  		start += PAGE_SIZE;
>  	}
>  
> +	update_flush_info_tlb(&dma_dom->domain, dma_addr, size);
> +
use start/size .... 

so is end a size in update_flush_info_tlb() , or should size be dma_addr+size 
in the callers of update_flush_info_tlb() ?

>  	SUB_STATS_COUNTER(alloced_io_mem, size);
>  
>  	dma_ops_free_addresses(dma_dom, dma_addr, pages);
> @@ -2465,6 +2483,9 @@ static int amd_iommu_map_range(struct iommu_domain *dom,
>  		paddr += PAGE_SIZE;
>  	}
>  
> +	if (unlikely(amd_iommu_np_cache))
> +		update_flush_info_tlb(domain, iova, iova + size);
> +
>  	return 0;
>  }
>  
> @@ -2482,6 +2503,8 @@ static void amd_iommu_unmap_range(struct iommu_domain *dom,
>  		iova  += PAGE_SIZE;
>  	}
>  
> +	update_flush_info_tlb(domain, iova, iova + size);
> +
>  	iommu_flush_tlb_pde(domain);
>  }
>  

Re: [PATCH 1/7] x86/amd-iommu: Add flush_info to protection domains

[Joerg Roedel]

On Thu, Feb 11, 2010 at 11:40:02AM -0500, Don Dutile wrote:
> Joerg Roedel wrote:
> > This patch adds a new sub-struct to protection domains which
> > is used to keep information about what parts of the domain
> > needs to be flushed on the hardware side.
> > 
> > Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> > ---
> >  arch/x86/include/asm/amd_iommu_types.h |   11 +++++++++++
> >  arch/x86/kernel/amd_iommu.c            |   23 +++++++++++++++++++++++
> >  2 files changed, 34 insertions(+), 0 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/amd_iommu_types.h b/arch/x86/include/asm/amd_iommu_types.h
> > index ba19ad4..30c4410 100644
> > --- a/arch/x86/include/asm/amd_iommu_types.h
> > +++ b/arch/x86/include/asm/amd_iommu_types.h
> > @@ -230,6 +230,16 @@ extern bool amd_iommu_np_cache;
> >  #define APERTURE_PAGE_INDEX(a)	(((a) >> 21) & 0x3fULL)
> >  
> >  /*
> > + * This struct holds information about the parts of a protection domain that
> > + * needs to be flushed on the IOMMU hardware.
> > + */
> > +struct flush_info {
> > +	bool tlb;
> > +	u64 start;
> > +	u64 end;
> > +};
> > +
> > +/*
> >   * This structure contains generic data for  IOMMU protection domains
> >   * independent of their use.
> >   */
> > @@ -244,6 +254,7 @@ struct protection_domain {
> >  	bool updated;		/* complete domain flush required */
> >  	unsigned dev_cnt;	/* devices assigned to this domain */
> >  	unsigned dev_iommu[MAX_IOMMUS]; /* per-IOMMU reference count */
> > +	struct flush_info flush;
> >  	void *priv;		/* private data */
> >  
> >  };
> > diff --git a/arch/x86/kernel/amd_iommu.c b/arch/x86/kernel/amd_iommu.c
> > index adb0ba0..fcb85e8 100644
> > --- a/arch/x86/kernel/amd_iommu.c
> > +++ b/arch/x86/kernel/amd_iommu.c
> > @@ -78,6 +78,19 @@ static struct iommu_dev_data *get_dev_data(struct device *dev)
> >  	return dev->archdata.iommu;
> >  }
> >  
> > +static void update_flush_info_tlb(struct protection_domain *domain,
> > +				  u64 start, u64 end)
> > +{
> > +	if (!domain->flush.tlb) {
> > +		domain->flush.tlb   = true;
> > +		domain->flush.start = start;
> > +		domain->flush.end   = end;
> > +	} else {
> > +		domain->flush.start = min(start, domain->flush.start);
> > +		domain->flush.end   = max(end  , domain->flush.end);
> > +	}
> > +}
> > +
> 
> the code has start/end here.... but callers below.... 
> 
> >  /*
> >   * In this function the list of preallocated protection domains is traversed to
> >   * find the domain for a specific device
> > @@ -1849,6 +1862,9 @@ retry:
> >  
> >  	ADD_STATS_COUNTER(alloced_io_mem, size);
> >  
> > +	if (unlikely(amd_iommu_np_cache))
> > +		update_flush_info_tlb(&dma_dom->domain, start, size);
> > +
> 
> use start/size .... 
> >  	if (unlikely(dma_dom->need_flush && !amd_iommu_unmap_flush)) {
> >  		iommu_flush_tlb(&dma_dom->domain);
> >  		dma_dom->need_flush = false;
> > @@ -1895,6 +1911,8 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
> >  		start += PAGE_SIZE;
> >  	}
> >  
> > +	update_flush_info_tlb(&dma_dom->domain, dma_addr, size);
> > +
> use start/size .... 
> 
> so is end a size in update_flush_info_tlb() , or should size be dma_addr+size 
> in the callers of update_flush_info_tlb() ?

Right, this is a bug. Thanks for pointing that out.

	Joerg