irqchip/gic-v3-its: Implement .msi_teardown() callback

The ITS driver currently nukes the structure representing an endpoint
device translating via an ITS on freeing the last LPI allocated for it.

That's an unfortunate state of affair, as it is pretty common for a driver
to allocate a single MSI, do something clever, teardown this MSI, and
reallocate a whole bunch of them. The NVME driver does exactly that,
amongst others.

What happens in that case is that the core code is accidentaly issuing
another .msi_prepare() call, even if it shouldn't.  This luckily cancels
the above behaviour and hides the problem.

In order to fix the core code, start by implementing the new
.msi_teardown() callback. Nothing calls it yet, so a side effect is that
the its_dev structure will not be freed and that the DID will stay
mapped. Not a big deal, and this will be solved in following patches.

Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250513163144.2215824-3-maz@kernel.org

diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 68f9ba4085ce50555c4f05b7413b8bfba51c38ec..958736622fa5739d4db122661f9f9389945e558b 100644 (file)
--- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c
+++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
@@ -167,6 +167,14 @@ static int its_pmsi_prepare(struct irq_domain *domain, struct device *dev,
                                          dev, nvec, info);
 }
 
+static void its_msi_teardown(struct irq_domain *domain, msi_alloc_info_t *info)
+{
+       struct msi_domain_info *msi_info;
+
+       msi_info = msi_get_domain_info(domain->parent);
+       msi_info->ops->msi_teardown(domain->parent, info);
+}
+
 static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                                  struct irq_domain *real_parent, struct msi_domain_info *info)
 {
@@ -190,6 +198,7 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                 * %MSI_MAX_INDEX.
                 */
                info->ops->msi_prepare = its_pci_msi_prepare;
+               info->ops->msi_teardown = its_msi_teardown;
                break;
        case DOMAIN_BUS_DEVICE_MSI:
        case DOMAIN_BUS_WIRED_TO_MSI:
@@ -198,6 +207,7 @@ static bool its_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
                 * size is also known at domain creation time.
                 */
                info->ops->msi_prepare = its_pmsi_prepare;
+               info->ops->msi_teardown = its_msi_teardown;
                break;
        default:
                /* Confused. How did the lib return true? */

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index fd6e7c170d37e888f58f2f07da82c95e7febaf8b..a77f11e23ad6c0b6440b083019ba0bbf57f353ff 100644 (file)
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3620,8 +3620,33 @@ out:
        return err;
 }
 
+static void its_msi_teardown(struct irq_domain *domain, msi_alloc_info_t *info)
+{
+       struct its_device *its_dev = info->scratchpad[0].ptr;
+
+       guard(mutex)(&its_dev->its->dev_alloc_lock);
+
+       /* If the device is shared, keep everything around */
+       if (its_dev->shared)
+               return;
+
+       /* LPIs should have been already unmapped at this stage */
+       if (WARN_ON_ONCE(!bitmap_empty(its_dev->event_map.lpi_map,
+                                      its_dev->event_map.nr_lpis)))
+               return;
+
+       its_lpi_free(its_dev->event_map.lpi_map,
+                    its_dev->event_map.lpi_base,
+                    its_dev->event_map.nr_lpis);
+
+       /* Unmap device/itt, and get rid of the tracking */
+       its_send_mapd(its_dev, 0);
+       its_free_device(its_dev);
+}
+
 static struct msi_domain_ops its_msi_domain_ops = {
        .msi_prepare    = its_msi_prepare,
+       .msi_teardown   = its_msi_teardown,
 };
 
 static int its_irq_gic_domain_alloc(struct irq_domain *domain,
@@ -3722,7 +3747,6 @@ 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 its_device *its_dev = irq_data_get_irq_chip_data(d);
-       struct its_node *its = its_dev->its;
        int i;
 
        bitmap_release_region(its_dev->event_map.lpi_map,
@@ -3736,26 +3760,6 @@ static void its_irq_domain_free(struct irq_domain *domain, unsigned int virq,
                irq_domain_reset_irq_data(data);
        }
 
-       mutex_lock(&its->dev_alloc_lock);
-
-       /*
-        * If all interrupts have been freed, start mopping the
-        * floor. This is conditioned 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_lpi_free(its_dev->event_map.lpi_map,
-                            its_dev->event_map.lpi_base,
-                            its_dev->event_map.nr_lpis);
-
-               /* Unmap device/itt */
-               its_send_mapd(its_dev, 0);
-               its_free_device(its_dev);
-       }
-
-       mutex_unlock(&its->dev_alloc_lock);
-
        irq_domain_free_irqs_parent(domain, virq, nr_irqs);
 }
 

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 7f0dfe0e5ccb233c2779a31ca94c597019f22e24..00f4d8758a2afd3b19c328c7cc90bb9c20520df1 100644 (file)
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -795,8 +795,7 @@ static int msi_domain_ops_prepare(struct irq_domain *domain, struct device *dev,
        return 0;
 }
 
-static void msi_domain_ops_teardown(struct irq_domain *domain,
-                                   msi_alloc_info_t *arg)
+static void msi_domain_ops_teardown(struct irq_domain *domain, msi_alloc_info_t *arg)
 {
 }