Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1659895 > unrolled thread

[PATCH 0/7] iommu/amd: Optimize iova queue flushing

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] iommu/amd: Optimize iova queue flushing Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:00 +0200
    [PATCH 1/7] iommu/amd: Rip out old queue flushing code Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:00 +0200
    [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:00 +0200
    [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:10 +0200
    [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:10 +0200
    [PATCH 7/7] iommu/amd: Remove queue_release() function Joerg Roedel <joro@8bytes.org> - 2017-06-07 17:10 +0200

#1659895 — [PATCH 0/7] iommu/amd: Optimize iova queue flushing

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

[toc] | [next] | [standalone]

#1659902 — [PATCH 1/7] iommu/amd: Rip out old queue flushing code

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

[toc] | [prev] | [next] | [standalone]

#1659905 — [PATCH 4/7] iommu/amd: Add locking to per-domain flush-queue

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

[toc] | [prev] | [next] | [standalone]

#1659908 — [PATCH 3/7] iommu/amd: Make use of the per-domain flush queue

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

[toc] | [prev] | [next] | [standalone]

#1659913 — [PATCH 2/7] iommu/amd: Add per-domain flush-queue data structures

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

[toc] | [prev] | [next] | [standalone]

#1659919 — [PATCH 7/7] iommu/amd: Remove queue_release() function

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web