iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Sebastian Andrzej Siewior
	<bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Subject: [PATCH 08/10] iommu/amd: drop the lock while allocating new irq remap table
Date: Fri, 23 Feb 2018 23:27:34 +0100	[thread overview]
Message-ID: <20180223222736.18542-9-bigeasy@linutronix.de> (raw)
In-Reply-To: <20180223222736.18542-1-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>

The irq_remap_table is allocated while the iommu_table_lock is held with
interrupts disabled. While this works it makes RT scream very loudly.
>From looking at the call sites, all callers are in the early device
initialisation (apic_bsp_setup(), pci_enable_device(),
pci_enable_msi()) so make sense to drop the lock which also enables
interrupts and try to allocate that memory with GFP_KERNEL instead
GFP_ATOMIC.

Since during the allocation the iommu_table_lock is dropped, we need to
recheck if table exists after the lock has been reacquired. I *think*
that it is impossible that the "devid" entry appears in irq_lookup_table
while the lock is dropped since the same device can only be probed once.
It is more likely that another device added an `alias' entry. However I
check for both cases, just to be sure.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 drivers/iommu/amd_iommu.c | 65 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b763fcbd790d..de6cc41d6cd2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3588,6 +3588,30 @@ static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
 	amd_iommu_dev_table[devid].data[2] = dte;
 }
 
+static struct irq_remap_table *alloc_irq_table(void)
+{
+	struct irq_remap_table *table;
+
+	table = kzalloc(sizeof(*table), GFP_ATOMIC);
+	if (!table)
+		return NULL;
+
+	table->table = kmem_cache_alloc(amd_iommu_irq_cache, GFP_KERNEL);
+	if (!table->table) {
+		kfree(table);
+		return NULL;
+	}
+	spin_lock_init(&table->lock);
+
+	if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
+		memset(table->table, 0,
+		       MAX_IRQS_PER_TABLE * sizeof(u32));
+	else
+		memset(table->table, 0,
+		       (MAX_IRQS_PER_TABLE * (sizeof(u64) * 2)));
+	return table;
+}
+
 static void set_remap_table_entry(struct amd_iommu *iommu, u16 devid,
 				  struct irq_remap_table *table)
 {
@@ -3599,6 +3623,7 @@ static void set_remap_table_entry(struct amd_iommu *iommu, u16 devid,
 static struct irq_remap_table *get_irq_table(u16 devid)
 {
 	struct irq_remap_table *table = NULL;
+	struct irq_remap_table *new_table = NULL;
 	struct amd_iommu *iommu;
 	unsigned long flags;
 	u16 alias;
@@ -3617,42 +3642,44 @@ static struct irq_remap_table *get_irq_table(u16 devid)
 	table = irq_lookup_table[alias];
 	if (table) {
 		set_remap_table_entry(iommu, devid, table);
-		goto out;
+		goto out_wait;
 	}
+	raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
 
 	/* Nothing there yet, allocate new irq remapping table */
-	table = kzalloc(sizeof(*table), GFP_ATOMIC);
-	if (!table)
+	new_table = alloc_irq_table();
+	if (!new_table)
+		return NULL;
+
+	raw_spin_lock_irqsave(&iommu_table_lock, flags);
+
+	table = irq_lookup_table[devid];
+	if (table)
 		goto out_unlock;
 
-	/* Initialize table spin-lock */
-	spin_lock_init(&table->lock);
-
-	table->table = kmem_cache_alloc(amd_iommu_irq_cache, GFP_ATOMIC);
-	if (!table->table) {
-		kfree(table);
-		table = NULL;
-		goto out_unlock;
+	table = irq_lookup_table[alias];
+	if (table) {
+		set_remap_table_entry(iommu, devid, table);
+		goto out_wait;
 	}
 
-	if (!AMD_IOMMU_GUEST_IR_GA(amd_iommu_guest_ir))
-		memset(table->table, 0,
-		       MAX_IRQS_PER_TABLE * sizeof(u32));
-	else
-		memset(table->table, 0,
-		       (MAX_IRQS_PER_TABLE * (sizeof(u64) * 2)));
-
+	table = new_table;
+	new_table = NULL;
 
 	set_remap_table_entry(iommu, devid, table);
 	if (devid != alias)
 		set_remap_table_entry(iommu, alias, table);
 
-out:
+out_wait:
 	iommu_completion_wait(iommu);
 
 out_unlock:
 	raw_spin_unlock_irqrestore(&iommu_table_lock, flags);
 
+	if (new_table) {
+		kmem_cache_free(amd_iommu_irq_cache, new_table->table);
+		kfree(new_table);
+	}
 	return table;
 }
 
-- 
2.16.1

  parent reply	other threads:[~2018-02-23 22:27 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-23 22:27 iommu/amd: lock splitting & GFP_KERNEL allocation Sebastian Andrzej Siewior
     [not found] ` <20180223222736.18542-1-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-02-23 22:27   ` [PATCH 01/10] iommu/amd: take into account that alloc_dev_data() may return NULL Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 02/10] iommu/amd: turn dev_data_list into a lock less list Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 03/10] iommu/amd: split domain id out of amd_iommu_devtable_lock Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 04/10] iommu/amd: split irq_lookup_table out of the amd_iommu_devtable_lock Sebastian Andrzej Siewior
     [not found]     ` <20180223222736.18542-5-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-03-15 12:54       ` Joerg Roedel
2018-02-23 22:27   ` [PATCH 05/10] iommu/amd: remove the special case from get_irq_table() Sebastian Andrzej Siewior
2018-03-15 12:53     ` Joerg Roedel
     [not found]       ` <20180315125342.q74wurk6bdbhl6hy-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2018-03-15 13:01         ` Sebastian Andrzej Siewior
     [not found]           ` <20180315130153.3ba2loj5oplogdew-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-03-15 13:07             ` Joerg Roedel
     [not found]               ` <20180315130723.dunmpmbkswwvq54g-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2018-03-15 14:15                 ` Sebastian Andrzej Siewior
     [not found]                   ` <20180315141541.6g3xc6yi4siafuwr-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-03-15 15:19                     ` Joerg Roedel
     [not found]                       ` <20180315151917.pgheszruqnnjbqb4-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
2018-03-15 15:25                         ` Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 06/10] iommu/amd: use `table' instead `irt' as variable name in amd_iommu_update_ga() Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 07/10] iommu/amd: factor out setting the remap table for a devid Sebastian Andrzej Siewior
2018-02-23 22:27   ` Sebastian Andrzej Siewior [this message]
     [not found]     ` <20180223222736.18542-9-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-03-15 12:58       ` [PATCH 08/10] iommu/amd: drop the lock while allocating new irq remap table Joerg Roedel
2018-02-23 22:27   ` [PATCH 09/10] iommu/amd: declare irq_remap_table's and amd_iommu's lock as a raw_spin_lock Sebastian Andrzej Siewior
2018-02-23 22:27   ` [PATCH 10/10] iommu/amd: make amd_iommu_devtable_lock a spin_lock Sebastian Andrzej Siewior
2018-03-15 13:01   ` iommu/amd: lock splitting & GFP_KERNEL allocation Joerg Roedel
  -- strict thread matches above, loose matches on Subject: below --
2018-03-22 15:22 [PATCH 00/10 v3] " Sebastian Andrzej Siewior
     [not found] ` <20180322152242.12705-1-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2018-03-22 15:22   ` [PATCH 08/10] iommu/amd: drop the lock while allocating new irq remap table Sebastian Andrzej Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180223222736.18542-9-bigeasy@linutronix.de \
    --to=bigeasy-hfztesqfncyowbw4kg4ksq@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).