linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] iommu/amd: Optimize iova queue flushing
@ 2017-06-07 14:58 Joerg Roedel
  2017-06-07 14:58 ` [PATCH 1/7] iommu/amd: Rip out old queue flushing code Joerg Roedel
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

Hi,

here is a patch-set to optimize the queue flushing in the
AMD IOMMU driver. It removes the global per-cpu flush queues
and implements queue-rings which are per-domain and per-cpu.

This allows some good optimizations to lower the flush-rate
of a domain and also lowers the contention on the queue
locks.

Regards,

	Joerg

Joerg Roedel (7):
  iommu/amd: Rip out old queue flushing code
  iommu/amd: Add per-domain flush-queue data structures
  iommu/amd: Make use of the per-domain flush queue
  iommu/amd: Add locking to per-domain flush-queue
  iommu/amd: Add flush counters to struct dma_ops_domain
  iommu/amd: Add per-domain timer to flush per-cpu queues
  iommu/amd: Remove queue_release() function

 drivers/iommu/amd_iommu.c | 363 +++++++++++++++++++++++++++++-----------------
 1 file changed, 227 insertions(+), 136 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/7] iommu/amd: Rip out old queue flushing code
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures Joerg Roedel
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

The queue flushing is pretty inefficient when it flushes the
queues for all cpus at once. Further it flushes all domains
from all IOMMUs for all CPUs, which is overkill as well.

Rip it out to make room for something more efficient.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 143 ++--------------------------------------------
 1 file changed, 6 insertions(+), 137 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 63cacf5..6304a6e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -89,25 +89,6 @@ LIST_HEAD(ioapic_map);
 LIST_HEAD(hpet_map);
 LIST_HEAD(acpihid_map);
 
-#define FLUSH_QUEUE_SIZE 256
-
-struct flush_queue_entry {
-	unsigned long iova_pfn;
-	unsigned long pages;
-	struct dma_ops_domain *dma_dom;
-};
-
-struct flush_queue {
-	spinlock_t lock;
-	unsigned next;
-	struct flush_queue_entry *entries;
-};
-
-static DEFINE_PER_CPU(struct flush_queue, flush_queue);
-
-static atomic_t queue_timer_on;
-static struct timer_list queue_timer;
-
 /*
  * Domain for untranslated devices - only allocated
  * if iommu=pt passed on kernel cmd line.
@@ -2225,92 +2206,6 @@ static struct iommu_group *amd_iommu_device_group(struct device *dev)
  *
  *****************************************************************************/
 
-static void __queue_flush(struct flush_queue *queue)
-{
-	struct protection_domain *domain;
-	unsigned long flags;
-	int idx;
-
-	/* First flush TLB of all known domains */
-	spin_lock_irqsave(&amd_iommu_pd_lock, flags);
-	list_for_each_entry(domain, &amd_iommu_pd_list, list)
-		domain_flush_tlb(domain);
-	spin_unlock_irqrestore(&amd_iommu_pd_lock, flags);
-
-	/* Wait until flushes have completed */
-	domain_flush_complete(NULL);
-
-	for (idx = 0; idx < queue->next; ++idx) {
-		struct flush_queue_entry *entry;
-
-		entry = queue->entries + idx;
-
-		free_iova_fast(&entry->dma_dom->iovad,
-				entry->iova_pfn,
-				entry->pages);
-
-		/* Not really necessary, just to make sure we catch any bugs */
-		entry->dma_dom = NULL;
-	}
-
-	queue->next = 0;
-}
-
-static void queue_flush_all(void)
-{
-	int cpu;
-
-	for_each_possible_cpu(cpu) {
-		struct flush_queue *queue;
-		unsigned long flags;
-
-		queue = per_cpu_ptr(&flush_queue, cpu);
-		spin_lock_irqsave(&queue->lock, flags);
-		if (queue->next > 0)
-			__queue_flush(queue);
-		spin_unlock_irqrestore(&queue->lock, flags);
-	}
-}
-
-static void queue_flush_timeout(unsigned long unsused)
-{
-	atomic_set(&queue_timer_on, 0);
-	queue_flush_all();
-}
-
-static void queue_add(struct dma_ops_domain *dma_dom,
-		      unsigned long address, unsigned long pages)
-{
-	struct flush_queue_entry *entry;
-	struct flush_queue *queue;
-	unsigned long flags;
-	int idx;
-
-	pages     = __roundup_pow_of_two(pages);
-	address >>= PAGE_SHIFT;
-
-	queue = get_cpu_ptr(&flush_queue);
-	spin_lock_irqsave(&queue->lock, flags);
-
-	if (queue->next == FLUSH_QUEUE_SIZE)
-		__queue_flush(queue);
-
-	idx   = queue->next++;
-	entry = queue->entries + idx;
-
-	entry->iova_pfn = address;
-	entry->pages    = pages;
-	entry->dma_dom  = dma_dom;
-
-	spin_unlock_irqrestore(&queue->lock, flags);
-
-	if (atomic_cmpxchg(&queue_timer_on, 0, 1) == 0)
-		mod_timer(&queue_timer, jiffies + msecs_to_jiffies(10));
-
-	put_cpu_ptr(&flush_queue);
-}
-
-
 /*
  * In the dma_ops path we only have the struct device. This function
  * finds the corresponding IOMMU, the protection domain and the
@@ -2462,7 +2357,10 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 		domain_flush_tlb(&dma_dom->domain);
 		domain_flush_complete(&dma_dom->domain);
 	} else {
-		queue_add(dma_dom, dma_addr, pages);
+		/* Keep the if() around, we need it later again */
+		dma_ops_free_iova(dma_dom, dma_addr, pages);
+		domain_flush_tlb(&dma_dom->domain);
+		domain_flush_complete(&dma_dom->domain);
 	}
 }
 
@@ -2797,7 +2695,7 @@ static int init_reserved_iova_ranges(void)
 
 int __init amd_iommu_init_api(void)
 {
-	int ret, cpu, err = 0;
+	int ret, err = 0;
 
 	ret = iova_cache_get();
 	if (ret)
@@ -2807,18 +2705,6 @@ int __init amd_iommu_init_api(void)
 	if (ret)
 		return ret;
 
-	for_each_possible_cpu(cpu) {
-		struct flush_queue *queue = per_cpu_ptr(&flush_queue, cpu);
-
-		queue->entries = kzalloc(FLUSH_QUEUE_SIZE *
-					 sizeof(*queue->entries),
-					 GFP_KERNEL);
-		if (!queue->entries)
-			goto out_put_iova;
-
-		spin_lock_init(&queue->lock);
-	}
-
 	err = bus_set_iommu(&pci_bus_type, &amd_iommu_ops);
 	if (err)
 		return err;
@@ -2830,23 +2716,12 @@ int __init amd_iommu_init_api(void)
 	err = bus_set_iommu(&platform_bus_type, &amd_iommu_ops);
 	if (err)
 		return err;
-	return 0;
-
-out_put_iova:
-	for_each_possible_cpu(cpu) {
-		struct flush_queue *queue = per_cpu_ptr(&flush_queue, cpu);
 
-		kfree(queue->entries);
-	}
-
-	return -ENOMEM;
+	return 0;
 }
 
 int __init amd_iommu_init_dma_ops(void)
 {
-	setup_timer(&queue_timer, queue_flush_timeout, 0);
-	atomic_set(&queue_timer_on, 0);
-
 	swiotlb        = iommu_pass_through ? 1 : 0;
 	iommu_detected = 1;
 
@@ -3002,12 +2877,6 @@ static void amd_iommu_domain_free(struct iommu_domain *dom)
 
 	switch (dom->type) {
 	case IOMMU_DOMAIN_DMA:
-		/*
-		 * First make sure the domain is no longer referenced from the
-		 * flush queue
-		 */
-		queue_flush_all();
-
 		/* Now release the domain */
 		dma_dom = to_dma_ops_domain(domain);
 		dma_ops_domain_free(dma_dom);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
  2017-06-07 14:58 ` [PATCH 1/7] iommu/amd: Rip out old queue flushing code Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue Joerg Roedel
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

Make the flush-queue per dma-ops domain and add code
allocate and free the flush-queues;

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6304a6e..71c688a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -134,6 +134,18 @@ static void update_domain(struct protection_domain *domain);
 static int protection_domain_init(struct protection_domain *domain);
 static void detach_device(struct device *dev);
 
+#define FLUSH_QUEUE_SIZE 256
+
+struct flush_queue_entry {
+	unsigned long iova_pfn;
+	unsigned long pages;
+};
+
+struct flush_queue {
+	struct flush_queue_entry *entries;
+	unsigned head, tail;
+};
+
 /*
  * Data container for a dma_ops specific protection domain
  */
@@ -143,6 +155,8 @@ struct dma_ops_domain {
 
 	/* IOVA RB-Tree */
 	struct iova_domain iovad;
+
+	struct flush_queue __percpu *flush_queue;
 };
 
 static struct iova_domain reserved_iova_ranges;
@@ -1714,6 +1728,56 @@ static void free_gcr3_table(struct protection_domain *domain)
 	free_page((unsigned long)domain->gcr3_tbl);
 }
 
+static void dma_ops_domain_free_flush_queue(struct dma_ops_domain *dom)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu) {
+		struct flush_queue *queue;
+
+		queue = per_cpu_ptr(dom->flush_queue, cpu);
+		kfree(queue->entries);
+	}
+
+	free_percpu(dom->flush_queue);
+
+	dom->flush_queue = NULL;
+}
+
+static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
+{
+	int cpu;
+
+	dom->flush_queue = alloc_percpu(struct flush_queue);
+	if (!dom->flush_queue)
+		return -ENOMEM;
+
+	/* First make sure everything is cleared */
+	for_each_possible_cpu(cpu) {
+		struct flush_queue *queue;
+
+		queue = per_cpu_ptr(dom->flush_queue, cpu);
+		queue->head    = 0;
+		queue->tail    = 0;
+		queue->entries = NULL;
+	}
+
+	/* Now start doing the allocation */
+	for_each_possible_cpu(cpu) {
+		struct flush_queue *queue;
+
+		queue = per_cpu_ptr(dom->flush_queue, cpu);
+		queue->entries = kzalloc(FLUSH_QUEUE_SIZE * sizeof(*queue->entries),
+					 GFP_KERNEL);
+		if (!queue->entries) {
+			dma_ops_domain_free_flush_queue(dom);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * Free a domain, only used if something went wrong in the
  * allocation path and we need to free an already allocated page table
@@ -1725,6 +1789,8 @@ static void dma_ops_domain_free(struct dma_ops_domain *dom)
 
 	del_domain_from_list(&dom->domain);
 
+	dma_ops_domain_free_flush_queue(dom);
+
 	put_iova_domain(&dom->iovad);
 
 	free_pagetable(&dom->domain);
@@ -1763,6 +1829,9 @@ static struct dma_ops_domain *dma_ops_domain_alloc(void)
 	/* Initialize reserved ranges */
 	copy_reserved_iova(&reserved_iova_ranges, &dma_dom->iovad);
 
+	if (dma_ops_domain_alloc_flush_queue(dma_dom))
+		goto free_dma_dom;
+
 	add_domain_to_list(&dma_dom->domain);
 
 	return dma_dom;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
  2017-06-07 14:58 ` [PATCH 1/7] iommu/amd: Rip out old queue flushing code Joerg Roedel
  2017-06-07 14:58 ` [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue Joerg Roedel
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

Fill the flush-queue on unmap and only flush the IOMMU and
device TLBs when a per-cpu queue gets full.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 60 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 56 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 71c688a..6a5c858 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1778,6 +1778,61 @@ static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
 	return 0;
 }
 
+static inline bool queue_ring_full(struct flush_queue *queue)
+{
+	return (((queue->tail + 1) % FLUSH_QUEUE_SIZE) == queue->head);
+}
+
+#define queue_ring_for_each(i, q) \
+	for (i = (q)->head; i != (q)->tail; i = (i + 1) % FLUSH_QUEUE_SIZE)
+
+static void queue_release(struct dma_ops_domain *dom,
+			  struct flush_queue *queue)
+{
+	unsigned i;
+
+	queue_ring_for_each(i, queue)
+		free_iova_fast(&dom->iovad,
+			       queue->entries[i].iova_pfn,
+			       queue->entries[i].pages);
+
+	queue->head = queue->tail = 0;
+}
+
+static inline unsigned queue_ring_add(struct flush_queue *queue)
+{
+	unsigned idx = queue->tail;
+
+	queue->tail = (idx + 1) % FLUSH_QUEUE_SIZE;
+
+	return idx;
+}
+
+static void queue_add(struct dma_ops_domain *dom,
+		      unsigned long address, unsigned long pages)
+{
+	struct flush_queue *queue;
+	int idx;
+
+	pages     = __roundup_pow_of_two(pages);
+	address >>= PAGE_SHIFT;
+
+	queue = get_cpu_ptr(dom->flush_queue);
+
+	if (queue_ring_full(queue)) {
+		domain_flush_tlb(&dom->domain);
+		domain_flush_complete(&dom->domain);
+		queue_release(dom, queue);
+	}
+
+	idx = queue_ring_add(queue);
+
+	queue->entries[idx].iova_pfn = address;
+	queue->entries[idx].pages    = pages;
+
+	put_cpu_ptr(dom->flush_queue);
+}
+
 /*
  * Free a domain, only used if something went wrong in the
  * allocation path and we need to free an already allocated page table
@@ -2426,10 +2481,7 @@ static void __unmap_single(struct dma_ops_domain *dma_dom,
 		domain_flush_tlb(&dma_dom->domain);
 		domain_flush_complete(&dma_dom->domain);
 	} else {
-		/* Keep the if() around, we need it later again */
-		dma_ops_free_iova(dma_dom, dma_addr, pages);
-		domain_flush_tlb(&dma_dom->domain);
-		domain_flush_complete(&dma_dom->domain);
+		queue_add(dma_dom, dma_addr, pages);
 	}
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
                   ` (2 preceding siblings ...)
  2017-06-07 14:58 ` [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 5/7] iommu/amd: Add flush counters to struct dma_ops_domain Joerg Roedel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

With locking we can safely access the flush-queues of other
cpus.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6a5c858..9aa2735 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -144,6 +144,7 @@ struct flush_queue_entry {
 struct flush_queue {
 	struct flush_queue_entry *entries;
 	unsigned head, tail;
+	spinlock_t lock;
 };
 
 /*
@@ -1773,6 +1774,8 @@ static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
 			dma_ops_domain_free_flush_queue(dom);
 			return -ENOMEM;
 		}
+
+		spin_lock_init(&queue->lock);
 	}
 
 	return 0;
@@ -1780,6 +1783,8 @@ static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
 
 static inline bool queue_ring_full(struct flush_queue *queue)
 {
+	assert_spin_locked(&queue->lock);
+
 	return (((queue->tail + 1) % FLUSH_QUEUE_SIZE) == queue->head);
 }
 
@@ -1791,6 +1796,8 @@ static void queue_release(struct dma_ops_domain *dom,
 {
 	unsigned i;
 
+	assert_spin_locked(&queue->lock);
+
 	queue_ring_for_each(i, queue)
 		free_iova_fast(&dom->iovad,
 			       queue->entries[i].iova_pfn,
@@ -1803,6 +1810,7 @@ static inline unsigned queue_ring_add(struct flush_queue *queue)
 {
 	unsigned idx = queue->tail;
 
+	assert_spin_locked(&queue->lock);
 	queue->tail = (idx + 1) % FLUSH_QUEUE_SIZE;
 
 	return idx;
@@ -1812,12 +1820,14 @@ static void queue_add(struct dma_ops_domain *dom,
 		      unsigned long address, unsigned long pages)
 {
 	struct flush_queue *queue;
+	unsigned long flags;
 	int idx;
 
 	pages     = __roundup_pow_of_two(pages);
 	address >>= PAGE_SHIFT;
 
 	queue = get_cpu_ptr(dom->flush_queue);
+	spin_lock_irqsave(&queue->lock, flags);
 
 	if (queue_ring_full(queue)) {
 		domain_flush_tlb(&dom->domain);
@@ -1830,6 +1840,7 @@ static void queue_add(struct dma_ops_domain *dom,
 	queue->entries[idx].iova_pfn = address;
 	queue->entries[idx].pages    = pages;
 
+	spin_unlock_irqrestore(&queue->lock, flags);
 	put_cpu_ptr(dom->flush_queue);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/7] iommu/amd: Add flush counters to struct dma_ops_domain
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
                   ` (3 preceding siblings ...)
  2017-06-07 14:58 ` [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 6/7] iommu/amd: Add per-domain timer to flush per-cpu queues Joerg Roedel
  2017-06-07 14:58 ` [PATCH 7/7] iommu/amd: Remove queue_release() function Joerg Roedel
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

The counters are increased every time the TLB for a given
domain is flushed. We also store the current value of that
counter into newly added entries of the flush-queue, so that
we can tell wheter this entry is already flushed.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9aa2735..1ad2866 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -139,6 +139,7 @@ static void detach_device(struct device *dev);
 struct flush_queue_entry {
 	unsigned long iova_pfn;
 	unsigned long pages;
+	u64 counter; /* Flush counter when this entry was added to the queue */
 };
 
 struct flush_queue {
@@ -158,6 +159,27 @@ struct dma_ops_domain {
 	struct iova_domain iovad;
 
 	struct flush_queue __percpu *flush_queue;
+
+	/*
+	 * We need two counter here to be race-free wrt. IOTLB flushing and
+	 * adding entries to the flush queue.
+	 *
+	 * The flush_start_cnt is incremented _before_ the IOTLB flush starts.
+	 * New entries added to the flush ring-buffer get their 'counter' value
+	 * from here. This way we can make sure that entries added to the queue
+	 * (or other per-cpu queues of the same domain) while the TLB is about
+	 * to be flushed are not considered to be flushed already.
+	 */
+	atomic64_t flush_start_cnt;
+
+	/*
+	 * The flush_finish_cnt is incremented when an IOTLB flush is complete.
+	 * This value is always smaller than flush_start_cnt. The queue_add
+	 * function frees all IOVAs that have a counter value smaller than
+	 * flush_finish_cnt. This makes sure that we only free IOVAs that are
+	 * flushed out of the IOTLB of the domain.
+	 */
+	atomic64_t flush_finish_cnt;
 };
 
 static struct iova_domain reserved_iova_ranges;
@@ -1749,6 +1771,9 @@ static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
 {
 	int cpu;
 
+	atomic64_set(&dom->flush_start_cnt,  0);
+	atomic64_set(&dom->flush_finish_cnt, 0);
+
 	dom->flush_queue = alloc_percpu(struct flush_queue);
 	if (!dom->flush_queue)
 		return -ENOMEM;
@@ -1816,22 +1841,48 @@ static inline unsigned queue_ring_add(struct flush_queue *queue)
 	return idx;
 }
 
+static inline void queue_ring_remove_head(struct flush_queue *queue)
+{
+	assert_spin_locked(&queue->lock);
+	queue->head = (queue->head + 1) % FLUSH_QUEUE_SIZE;
+}
+
 static void queue_add(struct dma_ops_domain *dom,
 		      unsigned long address, unsigned long pages)
 {
 	struct flush_queue *queue;
 	unsigned long flags;
+	u64 counter;
 	int idx;
 
 	pages     = __roundup_pow_of_two(pages);
 	address >>= PAGE_SHIFT;
 
+	counter = atomic64_read(&dom->flush_finish_cnt);
+
 	queue = get_cpu_ptr(dom->flush_queue);
 	spin_lock_irqsave(&queue->lock, flags);
 
+	queue_ring_for_each(idx, queue) {
+		/*
+		 * This assumes that counter values in the ring-buffer are
+		 * monotonously rising.
+		 */
+		if (queue->entries[idx].counter >= counter)
+			break;
+
+		free_iova_fast(&dom->iovad,
+			       queue->entries[idx].iova_pfn,
+			       queue->entries[idx].pages);
+
+		queue_ring_remove_head(queue);
+	}
+
 	if (queue_ring_full(queue)) {
+		atomic64_inc(&dom->flush_start_cnt);
 		domain_flush_tlb(&dom->domain);
 		domain_flush_complete(&dom->domain);
+		atomic64_inc(&dom->flush_finish_cnt);
 		queue_release(dom, queue);
 	}
 
@@ -1839,6 +1890,7 @@ static void queue_add(struct dma_ops_domain *dom,
 
 	queue->entries[idx].iova_pfn = address;
 	queue->entries[idx].pages    = pages;
+	queue->entries[idx].counter  = atomic64_read(&dom->flush_start_cnt);
 
 	spin_unlock_irqrestore(&queue->lock, flags);
 	put_cpu_ptr(dom->flush_queue);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/7] iommu/amd: Add per-domain timer to flush per-cpu queues
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
                   ` (4 preceding siblings ...)
  2017-06-07 14:58 ` [PATCH 5/7] iommu/amd: Add flush counters to struct dma_ops_domain Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  2017-06-07 14:58 ` [PATCH 7/7] iommu/amd: Remove queue_release() function Joerg Roedel
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

Add a timer to each dma_ops domain so that we flush unused
IOTLB entries regularily, even if the queues don't get full
all the time.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 84 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 67 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1ad2866..2bdfabf 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -180,6 +180,13 @@ struct dma_ops_domain {
 	 * flushed out of the IOTLB of the domain.
 	 */
 	atomic64_t flush_finish_cnt;
+
+	/*
+	 * Timer to make sure we don't keep IOVAs around unflushed
+	 * for too long
+	 */
+	struct timer_list flush_timer;
+	atomic_t flush_timer_on;
 };
 
 static struct iova_domain reserved_iova_ranges;
@@ -1806,6 +1813,14 @@ static int dma_ops_domain_alloc_flush_queue(struct dma_ops_domain *dom)
 	return 0;
 }
 
+static void dma_ops_domain_flush_tlb(struct dma_ops_domain *dom)
+{
+	atomic64_inc(&dom->flush_start_cnt);
+	domain_flush_tlb(&dom->domain);
+	domain_flush_complete(&dom->domain);
+	atomic64_inc(&dom->flush_finish_cnt);
+}
+
 static inline bool queue_ring_full(struct flush_queue *queue)
 {
 	assert_spin_locked(&queue->lock);
@@ -1847,22 +1862,12 @@ static inline void queue_ring_remove_head(struct flush_queue *queue)
 	queue->head = (queue->head + 1) % FLUSH_QUEUE_SIZE;
 }
 
-static void queue_add(struct dma_ops_domain *dom,
-		      unsigned long address, unsigned long pages)
+static void queue_ring_free_flushed(struct dma_ops_domain *dom,
+				    struct flush_queue *queue)
 {
-	struct flush_queue *queue;
-	unsigned long flags;
-	u64 counter;
+	u64 counter = atomic64_read(&dom->flush_finish_cnt);
 	int idx;
 
-	pages     = __roundup_pow_of_two(pages);
-	address >>= PAGE_SHIFT;
-
-	counter = atomic64_read(&dom->flush_finish_cnt);
-
-	queue = get_cpu_ptr(dom->flush_queue);
-	spin_lock_irqsave(&queue->lock, flags);
-
 	queue_ring_for_each(idx, queue) {
 		/*
 		 * This assumes that counter values in the ring-buffer are
@@ -1877,12 +1882,25 @@ static void queue_add(struct dma_ops_domain *dom,
 
 		queue_ring_remove_head(queue);
 	}
+}
+
+static void queue_add(struct dma_ops_domain *dom,
+		      unsigned long address, unsigned long pages)
+{
+	struct flush_queue *queue;
+	unsigned long flags;
+	int idx;
+
+	pages     = __roundup_pow_of_two(pages);
+	address >>= PAGE_SHIFT;
+
+	queue = get_cpu_ptr(dom->flush_queue);
+	spin_lock_irqsave(&queue->lock, flags);
+
+	queue_ring_free_flushed(dom, queue);
 
 	if (queue_ring_full(queue)) {
-		atomic64_inc(&dom->flush_start_cnt);
-		domain_flush_tlb(&dom->domain);
-		domain_flush_complete(&dom->domain);
-		atomic64_inc(&dom->flush_finish_cnt);
+		dma_ops_domain_flush_tlb(dom);
 		queue_release(dom, queue);
 	}
 
@@ -1893,9 +1911,33 @@ static void queue_add(struct dma_ops_domain *dom,
 	queue->entries[idx].counter  = atomic64_read(&dom->flush_start_cnt);
 
 	spin_unlock_irqrestore(&queue->lock, flags);
+
+	if (atomic_cmpxchg(&dom->flush_timer_on, 0, 1) == 0)
+		mod_timer(&dom->flush_timer, jiffies + msecs_to_jiffies(10));
+
 	put_cpu_ptr(dom->flush_queue);
 }
 
+static void queue_flush_timeout(unsigned long data)
+{
+	struct dma_ops_domain *dom = (struct dma_ops_domain *)data;
+	int cpu;
+
+	atomic_set(&dom->flush_timer_on, 0);
+
+	dma_ops_domain_flush_tlb(dom);
+
+	for_each_possible_cpu(cpu) {
+		struct flush_queue *queue;
+		unsigned long flags;
+
+		queue = per_cpu_ptr(dom->flush_queue, cpu);
+		spin_lock_irqsave(&queue->lock, flags);
+		queue_ring_free_flushed(dom, queue);
+		spin_unlock_irqrestore(&queue->lock, flags);
+	}
+}
+
 /*
  * Free a domain, only used if something went wrong in the
  * allocation path and we need to free an already allocated page table
@@ -1907,6 +1949,9 @@ static void dma_ops_domain_free(struct dma_ops_domain *dom)
 
 	del_domain_from_list(&dom->domain);
 
+	if (timer_pending(&dom->flush_timer))
+		del_timer(&dom->flush_timer);
+
 	dma_ops_domain_free_flush_queue(dom);
 
 	put_iova_domain(&dom->iovad);
@@ -1950,6 +1995,11 @@ static struct dma_ops_domain *dma_ops_domain_alloc(void)
 	if (dma_ops_domain_alloc_flush_queue(dma_dom))
 		goto free_dma_dom;
 
+	setup_timer(&dma_dom->flush_timer, queue_flush_timeout,
+		    (unsigned long)dma_dom);
+
+	atomic_set(&dma_dom->flush_timer_on, 0);
+
 	add_domain_to_list(&dma_dom->domain);
 
 	return dma_dom;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 7/7] iommu/amd: Remove queue_release() function
  2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
                   ` (5 preceding siblings ...)
  2017-06-07 14:58 ` [PATCH 6/7] iommu/amd: Add per-domain timer to flush per-cpu queues Joerg Roedel
@ 2017-06-07 14:58 ` Joerg Roedel
  6 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2017-06-07 14:58 UTC (permalink / raw)
  To: iommu; +Cc: linux-kernel, Tom Lendacky, Arindam Nath, Joerg Roedel

From: Joerg Roedel <jroedel@suse.de>

We can use queue_ring_free_flushed() instead, so remove this
redundancy.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/amd_iommu.c | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 2bdfabf..80efa72 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -1831,21 +1831,6 @@ static inline bool queue_ring_full(struct flush_queue *queue)
 #define queue_ring_for_each(i, q) \
 	for (i = (q)->head; i != (q)->tail; i = (i + 1) % FLUSH_QUEUE_SIZE)
 
-static void queue_release(struct dma_ops_domain *dom,
-			  struct flush_queue *queue)
-{
-	unsigned i;
-
-	assert_spin_locked(&queue->lock);
-
-	queue_ring_for_each(i, queue)
-		free_iova_fast(&dom->iovad,
-			       queue->entries[i].iova_pfn,
-			       queue->entries[i].pages);
-
-	queue->head = queue->tail = 0;
-}
-
 static inline unsigned queue_ring_add(struct flush_queue *queue)
 {
 	unsigned idx = queue->tail;
@@ -1897,12 +1882,15 @@ static void queue_add(struct dma_ops_domain *dom,
 	queue = get_cpu_ptr(dom->flush_queue);
 	spin_lock_irqsave(&queue->lock, flags);
 
-	queue_ring_free_flushed(dom, queue);
-
-	if (queue_ring_full(queue)) {
+	/*
+	 * When ring-queue is full, flush the entries from the IOTLB so
+	 * that we can free all entries with queue_ring_free_flushed()
+	 * below.
+	 */
+	if (queue_ring_full(queue))
 		dma_ops_domain_flush_tlb(dom);
-		queue_release(dom, queue);
-	}
+
+	queue_ring_free_flushed(dom, queue);
 
 	idx = queue_ring_add(queue);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-06-07 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-07 14:58 [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel
2017-06-07 14:58 ` [PATCH 1/7] iommu/amd: Rip out old queue flushing code Joerg Roedel
2017-06-07 14:58 ` [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures Joerg Roedel
2017-06-07 14:58 ` [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue Joerg Roedel
2017-06-07 14:58 ` [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue Joerg Roedel
2017-06-07 14:58 ` [PATCH 5/7] iommu/amd: Add flush counters to struct dma_ops_domain Joerg Roedel
2017-06-07 14:58 ` [PATCH 6/7] iommu/amd: Add per-domain timer to flush per-cpu queues Joerg Roedel
2017-06-07 14:58 ` [PATCH 7/7] iommu/amd: Remove queue_release() function Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).