Re: PCI MSI issue with reinserting a driver

[John Garry]

> > Here's my suspicion: two of the interrupts are mapped in the low-level
> > domain (the ITS, I'd expect in your case), but they have never been
> > mapped at the higher level.
> >
> > On teardown, we only get rid of the 30 that were actually mapped, and
> > leave the last two dangling in the ITS domain, and thus the ITS device
> > resources are never freed. On reload, we request another 32
> > interrupts, which can't be satisfied for this device.
> >
> > Assuming I got it right, the question is: why weren't these interrupts
> > mapped in the PCI domain the first place. And if I got it wrong, I'm
> > even more curious!
>
> Not sure. I also now notice an error for the SAS PCI driver on D06 when 
> nr_cpus < 16, which means number of MSI vectors allocated < 32, so looks 
> the same problem. There we try to allocate 16 + max(nr cpus, 16) MSI.
>
> Anyway, let me have a look today to see what is going wrong.
Could this be the problem:

nr_cpus=11

In alloc path, we have:
	its_alloc_device_irq(nvecs=27 = 16+11)
	  bitmap_find_free_region(order = 5);
In free path, we have:
	its_irq_domain_free(nvecs = 1) and free each 27 vecs
	  bitmap_release_region(order = 0)

So we allocate 32 bits, but only free 27. And 2nd alloc for 32 fails.

FWIW, this hack seems to fix it:

---->8-----

diff --git a/drivers/irqchip/irq-gic-v3-its.c 
b/drivers/irqchip/irq-gic-v3-its.c
index ac5412b284e6..458ea0ebea2b 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c

@@ -3533,34 +3534,39 @@ static int its_irq_domain_alloc(struct 
irq_domain *domain, unsigned int virq,
 	struct its_device *its_dev = info->scratchpad[0].ptr;
 	struct its_node *its = its_dev->its;
 	struct irq_data *irqd;
-	irq_hw_number_t hwirq;
+	irq_hw_number_t hwirq[nr_irqs]; //vla :(
 	int err;
 	int i;

-	err = its_alloc_device_irq(its_dev, nr_irqs, &hwirq);
+	for (i = 0; i < nr_irqs; i++) {
+		err = its_alloc_device_irq(its_dev, 1, &hwirq[i]);
+		if (err) //tidy
+			return err;
+	}
+	
-	if (err)
-		return err;

 	err = iommu_dma_prepare_msi(info->desc, its->get_msi_base(its_dev));
 	if (err)
 		return err;

 	for (i = 0; i < nr_irqs; i++) {
-		err = its_irq_gic_domain_alloc(domain, virq + i, hwirq + i);
+		err = its_irq_gic_domain_alloc(domain, virq + i, hwirq[i]);
 		if (err)
 			return err;

 		irq_domain_set_hwirq_and_chip(domain, virq + i,
-					      hwirq + i, &its_irq_chip, its_dev);
+					      hwirq[i], &its_irq_chip, its_dev);
 		irqd = irq_get_irq_data(virq + i);
 		irqd_set_single_target(irqd);
 		irqd_set_affinity_on_activate(irqd);
 		pr_debug("ID:%d pID:%d vID:%d\n",
-			 (int)(hwirq + i - its_dev->event_map.lpi_base),
-			 (int)(hwirq + i), virq + i);
+			 (int)(hwirq[i] - its_dev->event_map.lpi_base),
+			 (int)(hwirq[i]), virq + i);
 	}
----8<-----


But I'm not sure that we have any requirement for those map bits to be 
consecutive.

Thanks,
John