From: Marc Zyngier <maz@kernel.org>
To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: Thomas Gleixner <tglx@linutronix.de>,
Lorenzo Pieralisi <lpieralisi@kernel.org>,
Sascha Bischoff <sascha.bischoff@arm.com>,
Timothy Hayes <timothy.hayes@arm.com>
Subject: [PATCH v2 2/5] irqchip/gic-v3-its: Implement .msi_teardown() callback
Date: Tue, 13 May 2025 17:31:41 +0100 [thread overview]
Message-ID: <20250513163144.2215824-3-maz@kernel.org> (raw)
In-Reply-To: <20250513163144.2215824-1-maz@kernel.org>
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 buggy enough
to issue 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>
---
drivers/irqchip/irq-gic-v3-its-msi-parent.c | 10 +++++
drivers/irqchip/irq-gic-v3-its.c | 46 +++++++++++----------
kernel/irq/msi.c | 3 +-
3 files changed, 36 insertions(+), 23 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c
index 68f9ba4085ce5..958736622fa57 100644
--- 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 fd6e7c170d37e..a77f11e23ad6c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -3620,8 +3620,33 @@ static int its_msi_prepare(struct irq_domain *domain, struct device *dev,
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 a3b34c3c599be..31378a2535fb9 100644
--- 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)
{
}
--
2.39.2
next prev parent reply other threads:[~2025-05-13 16:44 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-13 16:31 [PATCH v2 0/5] genirq/msi: Fix device MSI prepare/alloc sequencing Marc Zyngier
2025-05-13 16:31 ` [PATCH v2 1/5] genirq/msi: Add .msi_teardown() callback as the reverse of .msi_prepare() Marc Zyngier
2025-05-16 10:02 ` [tip: irq/msi] " tip-bot2 for Marc Zyngier
2025-05-13 16:31 ` Marc Zyngier [this message]
2025-05-16 10:02 ` [tip: irq/msi] irqchip/gic-v3-its: Implement .msi_teardown() callback tip-bot2 for Marc Zyngier
2025-05-13 16:31 ` [PATCH v2 3/5] genirq/msi: Move prepare() call to per-device allocation Marc Zyngier
2025-05-16 10:02 ` [tip: irq/msi] " tip-bot2 for Marc Zyngier
2025-06-03 8:22 ` [PATCH v2 3/5] " Zenghui Yu
2025-06-03 9:35 ` Lorenzo Pieralisi
2025-06-03 13:09 ` Marc Zyngier
2025-06-03 14:37 ` Lorenzo Pieralisi
2025-06-04 1:52 ` Zenghui Yu
2025-06-03 12:50 ` Marc Zyngier
2025-06-03 13:07 ` Zenghui Yu
2025-06-03 13:27 ` Marc Zyngier
2025-06-03 13:29 ` Lorenzo Pieralisi
2025-06-03 14:03 ` Marc Zyngier
2025-06-03 14:08 ` Marc Zyngier
2025-05-13 16:31 ` [PATCH v2 4/5] genirq/msi: Engage the .msi_teardown() callback on domain removal Marc Zyngier
2025-05-16 10:02 ` [tip: irq/msi] " tip-bot2 for Marc Zyngier
2025-05-13 16:31 ` [PATCH v2 5/5] irqchip/gic-v3-its: Use allocation size from the prepare call Marc Zyngier
2025-05-16 10:02 ` [tip: irq/msi] " tip-bot2 for Marc Zyngier
2025-05-18 18:53 ` [PATCH v2 5/5] " Thomas Gleixner
2025-05-19 10:15 ` Marc Zyngier
2025-05-19 12:16 ` Thomas Gleixner
2025-05-19 14:28 ` Marc Zyngier
2025-05-21 15:29 ` Thomas Gleixner
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=20250513163144.2215824-3-maz@kernel.org \
--to=maz@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=sascha.bischoff@arm.com \
--cc=tglx@linutronix.de \
--cc=timothy.hayes@arm.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.