[PATCH] irqchip/gic-v3-its: Lock its device list during find and create its device

From: Zheng Xiang
Date: Sat Jan 26 2019 - 01:19:38 EST


Currently each PCI device under a PCI Bridge shares the same device id
and ITS device. Assume there are two PCI devices call its_msi_prepare
concurrently and they are both going to find and create their ITS
device. There is a chance that the later one couldn't find ITS device
before the other one creating the ITS device. It will cause the later
one to create a different ITS device even if they have the same
device_id.

Signed-off-by: Zheng Xiang <zhengxiang9@xxxxxxxxxx>
---
drivers/irqchip/irq-gic-v3-its.c | 52 +++++++++++++++-------------------------
1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index db20e99..397edc8 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -2205,25 +2205,6 @@ static void its_cpu_init_collections(void)
raw_spin_unlock(&its_lock);
}

-static struct its_device *its_find_device(struct its_node *its, u32 dev_id)
-{
- struct its_device *its_dev = NULL, *tmp;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&its->lock, flags);
-
- list_for_each_entry(tmp, &its->its_device_list, entry) {
- if (tmp->device_id == dev_id) {
- its_dev = tmp;
- break;
- }
- }
-
- raw_spin_unlock_irqrestore(&its->lock, flags);
-
- return its_dev;
-}
-
static struct its_baser *its_get_baser(struct its_node *its, u32 type)
{
int i;
@@ -2321,7 +2302,7 @@ static bool its_alloc_vpe_table(u32 vpe_id)
static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
int nvecs, bool alloc_lpis)
{
- struct its_device *dev;
+ struct its_device *dev = NULL, *tmp;
unsigned long *lpi_map = NULL;
unsigned long flags;
u16 *col_map = NULL;
@@ -2331,6 +2312,24 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
int nr_ites;
int sz;

+ raw_spin_lock_irqsave(&its->lock, flags);
+ list_for_each_entry(tmp, &its->its_device_list, entry) {
+ if (tmp->device_id == dev_id) {
+ dev = tmp;
+ break;
+ }
+ }
+ if (dev) {
+ /*
+ * We already have seen this ID, probably through
+ * another alias (PCI bridge of some sort). No need to
+ * create the device.
+ */
+ pr_debug("Reusing ITT for devID %x\n", dev_id);
+ raw_spin_unlock_irqrestore(&its->lock, flags);
+ return dev;
+ }
+
if (!its_alloc_device_table(its, dev_id))
return NULL;

@@ -2378,7 +2377,6 @@ static struct its_device *its_create_device(struct its_node *its, u32 dev_id,
dev->device_id = dev_id;
INIT_LIST_HEAD(&dev->entry);

- raw_spin_lock_irqsave(&its->lock, flags);
list_add(&dev->entry, &its->its_device_list);
raw_spin_unlock_irqrestore(&its->lock, flags);

@@ -2443,23 +2441,11 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
return -EINVAL;
}

- its_dev = its_find_device(its, dev_id);
- if (its_dev) {
- /*
- * We already have seen this ID, probably through
- * another alias (PCI bridge of some sort). No need to
- * create the device.
- */
- pr_debug("Reusing ITT for devID %x\n", dev_id);
- goto out;
- }
-
its_dev = its_create_device(its, dev_id, nvec, true);
if (!its_dev)
return -ENOMEM;

pr_debug("ITT %d entries, %d bits\n", nvec, ilog2(nvec));
-out:
info->scratchpad[0].ptr = its_dev;
return 0;
}
--
1.8.3.1