Commit 9791ec7d authored by Marc Zyngier's avatar Marc Zyngier

irqchip/gic-v3-its: Plug allocation race for devices sharing a DevID

On systems or VMs where multiple devices share a single DevID
(because they sit behind a PCI bridge, or because the HW is
broken in funky ways), we reuse the save its_device structure
in order to reflect this.

It turns out that there is a distinct lack of locking when looking
up the its_device, and two device being probed concurrently can result
in double allocations. That's obviously not nice.

A solution for this is to have a per-ITS mutex that serializes device
allocation.

A similar issue exists on the freeing side, which can run concurrently
with the allocation. On top of now taking the appropriate lock, we
also make sure that a shared device is never freed, as we have no way
to currently track the life cycle of such object.
Reported-by: default avatarZheng Xiang <zhengxiang9@huawei.com>
Tested-by: default avatarZheng Xiang <zhengxiang9@huawei.com>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarMarc Zyngier <marc.zyngier@arm.com>
parent 6479450f
...@@ -97,9 +97,14 @@ struct its_device; ...@@ -97,9 +97,14 @@ struct its_device;
* The ITS structure - contains most of the infrastructure, with the * The ITS structure - contains most of the infrastructure, with the
* top-level MSI domain, the command queue, the collections, and the * top-level MSI domain, the command queue, the collections, and the
* list of devices writing to it. * list of devices writing to it.
*
* dev_alloc_lock has to be taken for device allocations, while the
* spinlock must be taken to parse data structures such as the device
* list.
*/ */
struct its_node { struct its_node {
raw_spinlock_t lock; raw_spinlock_t lock;
struct mutex dev_alloc_lock;
struct list_head entry; struct list_head entry;
void __iomem *base; void __iomem *base;
phys_addr_t phys_base; phys_addr_t phys_base;
...@@ -156,6 +161,7 @@ struct its_device { ...@@ -156,6 +161,7 @@ struct its_device {
void *itt; void *itt;
u32 nr_ites; u32 nr_ites;
u32 device_id; u32 device_id;
bool shared;
}; };
static struct { static struct {
...@@ -2469,6 +2475,7 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev, ...@@ -2469,6 +2475,7 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
struct its_device *its_dev; struct its_device *its_dev;
struct msi_domain_info *msi_info; struct msi_domain_info *msi_info;
u32 dev_id; u32 dev_id;
int err = 0;
/* /*
* We ignore "dev" entierely, and rely on the dev_id that has * We ignore "dev" entierely, and rely on the dev_id that has
...@@ -2491,6 +2498,7 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev, ...@@ -2491,6 +2498,7 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
return -EINVAL; return -EINVAL;
} }
mutex_lock(&its->dev_alloc_lock);
its_dev = its_find_device(its, dev_id); its_dev = its_find_device(its, dev_id);
if (its_dev) { if (its_dev) {
/* /*
...@@ -2498,18 +2506,22 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev, ...@@ -2498,18 +2506,22 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
* another alias (PCI bridge of some sort). No need to * another alias (PCI bridge of some sort). No need to
* create the device. * create the device.
*/ */
its_dev->shared = true;
pr_debug("Reusing ITT for devID %x\n", dev_id); pr_debug("Reusing ITT for devID %x\n", dev_id);
goto out; goto out;
} }
its_dev = its_create_device(its, dev_id, nvec, true); its_dev = its_create_device(its, dev_id, nvec, true);
if (!its_dev) if (!its_dev) {
return -ENOMEM; err = -ENOMEM;
goto out;
}
pr_debug("ITT %d entries, %d bits\n", nvec, ilog2(nvec)); pr_debug("ITT %d entries, %d bits\n", nvec, ilog2(nvec));
out: out:
mutex_unlock(&its->dev_alloc_lock);
info->scratchpad[0].ptr = its_dev; info->scratchpad[0].ptr = its_dev;
return 0; return err;
} }
static struct msi_domain_ops its_msi_domain_ops = { static struct msi_domain_ops its_msi_domain_ops = {
...@@ -2613,6 +2625,7 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq, ...@@ -2613,6 +2625,7 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
{ {
struct irq_data *d = irq_domain_get_irq_data(domain, virq); struct irq_data *d = irq_domain_get_irq_data(domain, virq);
struct its_device *its_dev = irq_data_get_irq_chip_data(d); struct its_device *its_dev = irq_data_get_irq_chip_data(d);
struct its_node *its = its_dev->its;
int i; int i;
for (i = 0; i < nr_irqs; i++) { for (i = 0; i < nr_irqs; i++) {
...@@ -2627,8 +2640,14 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq, ...@@ -2627,8 +2640,14 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
irq_domain_reset_irq_data(data); irq_domain_reset_irq_data(data);
} }
/* If all interrupts have been freed, start mopping the floor */ mutex_lock(&its->dev_alloc_lock);
if (bitmap_empty(its_dev->event_map.lpi_map,
/*
* If all interrupts have been freed, start mopping the
* floor. This is conditionned on the device not being shared.
*/
if (!its_dev->shared &&
bitmap_empty(its_dev->event_map.lpi_map,
its_dev->event_map.nr_lpis)) { its_dev->event_map.nr_lpis)) {
its_lpi_free(its_dev->event_map.lpi_map, its_lpi_free(its_dev->event_map.lpi_map,
its_dev->event_map.lpi_base, its_dev->event_map.lpi_base,
...@@ -2640,6 +2659,8 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq, ...@@ -2640,6 +2659,8 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
its_free_device(its_dev); its_free_device(its_dev);
} }
mutex_unlock(&its->dev_alloc_lock);
irq_domain_free_irqs_parent(domain, virq, nr_irqs); irq_domain_free_irqs_parent(domain, virq, nr_irqs);
} }
...@@ -3549,6 +3570,7 @@ static int __init its_probe_one(struct resource *res, ...@@ -3549,6 +3570,7 @@ static int __init its_probe_one(struct resource *res,
} }
raw_spin_lock_init(&its->lock); raw_spin_lock_init(&its->lock);
mutex_init(&its->dev_alloc_lock);
INIT_LIST_HEAD(&its->entry); INIT_LIST_HEAD(&its->entry);
INIT_LIST_HEAD(&its->its_device_list); INIT_LIST_HEAD(&its->its_device_list);
typer = gic_read_typer(its_base + GITS_TYPER); typer = gic_read_typer(its_base + GITS_TYPER);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment