public inbox for linux-rt-devel@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler
@ 2026-02-15 17:41 Sasha Levin
  2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.1] mailbox: pcc: Remove spurious IRQF_ONESHOT usage Sasha Levin
  0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2026-02-15 17:41 UTC (permalink / raw)
  To: patches, stable
  Cc: Sebastian Andrzej Siewior, Jassi Brar, Sasha Levin, clrkwllms,
	rostedt, linux-kernel, linux-rt-devel

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

[ Upstream commit fa84883d44422208b45869a67c0265234fdce1f0 ]

request_threaded_irq() is invoked with a primary and a secondary handler
and no flags are passed. The primary handler is the same as
irq_default_primary_handler() so there is no need to have an identical
copy.
The lack of the IRQF_ONESHOT can be dangerous because the interrupt
source is not masked while the threaded handler is active. This means,
especially on LEVEL typed interrupt lines, the interrupt can fire again
before the threaded handler had a chance to run.

Use the default primary interrupt handler by specifying NULL and set
IRQF_ONESHOT so the interrupt source is masked until the secondary
handler is done.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Both commits are the same patch. The one being evaluated (fa84883d44422,
signed by Jassi Brar) is the one from the mailbox tree.

## Summary of Analysis

### What the commit fixes

The commit fixes a missing `IRQF_ONESHOT` flag in
`request_threaded_irq()` for the BCM FlexRM mailbox driver. The existing
code had a custom primary handler (`flexrm_irq_event`) that was
functionally identical to the kernel's `irq_default_primary_handler()` —
it simply returned `IRQ_WAKE_THREAD`. However, because the code didn't
set `IRQF_ONESHOT`, the interrupt line is not masked while the threaded
handler runs. On level-triggered interrupt lines, this can cause an
interrupt storm (the interrupt fires repeatedly before the threaded
handler gets to run).

### Why it matters

1. **Correctness**: Without `IRQF_ONESHOT`, if the interrupt is level-
   triggered (or chained through a level-triggered parent), the
   interrupt will continuously fire between the primary handler
   returning and the threaded handler completing. This is a potential
   interrupt storm / soft lockup.

2. **Author credibility**: Sebastian Siewior is the PREEMPT_RT
   maintainer and a leading expert on interrupt handling in Linux.
   Thomas Gleixner, the IRQ subsystem maintainer, also signed off on
   this patch via the tip tree.

3. **Pattern**: This is part of a systematic series fixing the same
   pattern across multiple drivers (efct, btintel_pcie, fsl-mc, etc.),
   all from the same author.

### Stable kernel criteria

- **Obviously correct**: Yes - removes an exact duplicate of
  `irq_default_primary_handler()` and adds the missing `IRQF_ONESHOT`
  flag. This is the canonical pattern for threaded IRQs.
- **Fixes a real bug**: Yes - missing `IRQF_ONESHOT` can cause interrupt
  storms on level-triggered lines.
- **Small and contained**: Yes - 1 file changed, 2 insertions, 12
  deletions. The change is strictly self-contained.
- **No new features**: Correct - this is purely a bug fix.
- **Risk**: Very low. The behavior is identical except that the
  interrupt line is now properly masked during threaded handler
  execution, which prevents the storm.

### Nuance: MSI vs Level-Triggered

The driver uses platform MSI interrupts for completions (the comment
says "We only have MSI for completions"). MSI interrupts are typically
edge-triggered and may not suffer from the re-assertion problem.
However:
1. Platform MSIs may be chained through parent controllers that ARE
   level-triggered.
2. The underlying IRQ chip may not have `IRQCHIP_ONESHOT_SAFE` set (only
   PCI MSI explicitly sets this).
3. Even if the risk is lower for MSI, adding `IRQF_ONESHOT` is the
   correct defensive practice and has zero downside.
4. The kernel enforcement check (`__setup_irq`) would actually reject
   the new code (handler=NULL without IRQF_ONESHOT) if the chip lacks
   `IRQCHIP_ONESHOT_SAFE`, so `IRQF_ONESHOT` is needed for the code to
   even work.

### Verification

- **Verified** `flexrm_irq_event()` returns only `IRQ_WAKE_THREAD` at
  line 1176-1182 of bcm-flexrm-mailbox.c, identical to
  `irq_default_primary_handler()` at kernel/irq/manage.c:976-979.
- **Verified** `request_threaded_irq()` with handler=NULL uses
  `irq_default_primary_handler` (kernel/irq/manage.c:2128-2132).
- **Verified** `__setup_irq()` requires IRQF_ONESHOT when
  handler==irq_default_primary_handler and chip lacks
  IRQCHIP_ONESHOT_SAFE (kernel/irq/manage.c:1664-1685).
- **Verified** platform MSI irq chips do NOT automatically set
  `IRQCHIP_ONESHOT_SAFE` - only PCI MSI does
  (drivers/pci/msi/irqdomain.c:133,187). No irqchip drivers set it
  either.
- **Verified** the driver has existed since v4.12 (commit
  dbc049eee7300), present in all active stable trees.
- **Verified** the fix is self-contained: only touches 1 file, no
  dependencies on other commits.
- **Verified** author Sebastian Siewior is the PREEMPT_RT maintainer
  with deep IRQ expertise, and this is part of a systematic series.
- **Verified** the commit also went through Thomas Gleixner's tip tree
  (commit 03843d95a4a4e) with his Signed-off-by, confirming IRQ
  subsystem maintainer review.

This is a small, safe, well-understood bug fix from a domain expert that
prevents a potential interrupt storm. It meets all stable kernel
criteria.

**YES**

 drivers/mailbox/bcm-flexrm-mailbox.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/mailbox/bcm-flexrm-mailbox.c b/drivers/mailbox/bcm-flexrm-mailbox.c
index 41f79e51d9e5a..4255fefc3a5a0 100644
--- a/drivers/mailbox/bcm-flexrm-mailbox.c
+++ b/drivers/mailbox/bcm-flexrm-mailbox.c
@@ -1173,14 +1173,6 @@ static int flexrm_debugfs_stats_show(struct seq_file *file, void *offset)
 
 /* ====== FlexRM interrupt handler ===== */
 
-static irqreturn_t flexrm_irq_event(int irq, void *dev_id)
-{
-	/* We only have MSI for completions so just wakeup IRQ thread */
-	/* Ring related errors will be informed via completion descriptors */
-
-	return IRQ_WAKE_THREAD;
-}
-
 static irqreturn_t flexrm_irq_thread(int irq, void *dev_id)
 {
 	flexrm_process_completions(dev_id);
@@ -1271,10 +1263,8 @@ static int flexrm_startup(struct mbox_chan *chan)
 		ret = -ENODEV;
 		goto fail_free_cmpl_memory;
 	}
-	ret = request_threaded_irq(ring->irq,
-				   flexrm_irq_event,
-				   flexrm_irq_thread,
-				   0, dev_name(ring->mbox->dev), ring);
+	ret = request_threaded_irq(ring->irq, NULL, flexrm_irq_thread,
+				   IRQF_ONESHOT, dev_name(ring->mbox->dev), ring);
 	if (ret) {
 		dev_err(ring->mbox->dev,
 			"failed to request ring%d IRQ\n", ring->num);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.19-6.1] mailbox: pcc: Remove spurious IRQF_ONESHOT usage
  2026-02-15 17:41 [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler Sasha Levin
@ 2026-02-15 17:41 ` Sasha Levin
  0 siblings, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-15 17:41 UTC (permalink / raw)
  To: patches, stable
  Cc: Mark Brown, Aishwarya TCV, Sudeep Holla, Jassi Brar, Sasha Levin,
	sudeep.holla, bigeasy, clrkwllms, rostedt, linux-acpi,
	linux-kernel, linux-rt-devel

From: Mark Brown <broonie@kernel.org>

[ Upstream commit 673327028cd61db68a1e0c708be2e302c082adf9 ]

The PCC code currently specifies IRQF_ONESHOT if the interrupt could
potentially be shared but doesn't actually use request_threaded_irq() and
the interrupt handler does not use IRQ_WAKE_THREAD so IRQF_ONESHOT is
never relevant. Since commit aef30c8d569c ("genirq: Warn about using
IRQF_ONESHOT without a threaded handler") specifying it has resulted in a
WARN_ON(), fix this by removing IRQF_ONESHOT.

Reported-by: Aishwarya TCV <Aishwarya.TCV@arm.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Jassi Brar <jassisinghbrar@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Summary of Analysis

### What the commit fixes

The PCC mailbox driver (`drivers/mailbox/pcc.c`) incorrectly specifies
`IRQF_ONESHOT` when requesting a shared interrupt. `IRQF_ONESHOT` is
only meaningful for threaded interrupt handlers (registered via
`request_threaded_irq()`), but this driver uses `devm_request_irq()`
with a hardcoded primary handler (`pcc_mbox_irq`) and no thread
function.

Since commit `aef30c8d569c` (merged 2026-01-13 into v6.19 cycle), the
generic IRQ code now emits a `WARN_ON_ONCE()` when `IRQF_ONESHOT` is
specified without a threaded handler. This means on any system with a
PCC shared interrupt, the kernel produces a warning splat at
boot/runtime — a visible regression for users.

### Stable kernel criteria assessment

1. **Obviously correct and tested**: Yes — the change removes a flag
   that has no effect (since the driver doesn't use threaded IRQs).
   Reviewed by the SCMI/PCC co-maintainer Sudeep Holla.

2. **Fixes a real bug**: Yes — it fixes a `WARN_ON()` that fires at
   runtime. WARN_ONs are treated as bugs by many distributions and can
   be flagged by automated testing.

3. **Important issue**: Moderate — a WARN_ON at boot is visible to users
   and breaks CI/testing systems. On PREEMPT_RT systems, the underlying
   issue (IRQF_ONESHOT without threading) could also be problematic
   since the handler is exempt from forced-threading.

4. **Small and contained**: Yes — single line change, single file, no
   side effects.

5. **No new features**: Correct — this purely removes a spurious flag.

### Dependencies

This fix only makes sense if commit `aef30c8d569c` ("genirq: Warn about
using IRQF_ONESHOT without a threaded handler") is also present in the
stable tree. That commit was merged in the v6.19 cycle (January 2026).
If `aef30c8d569c` is NOT backported to stable, then the `IRQF_ONESHOT`
flag is harmless (no WARN_ON fires, and the flag is simply ignored).
However, the `IRQF_ONESHOT` was always incorrect — removing it is safe
regardless.

The PCC shared interrupt support was added by commit `3db174e478cb0b` in
September 2023 (v6.7 cycle), so this code exists in stable trees 6.6.y
and later.

### Risk assessment

**Risk: Extremely low.** Removing an unused flag from an IRQ
registration call cannot break anything. The flag was never functional
since the driver doesn't use threaded IRQs.

**Benefit: Eliminates WARN_ON** splat on systems with shared PCC
interrupts, which affects ACPI-based ARM64 and x86 platforms using PCC
for CPPC/SCMI.

### Verification

- **Verified**: Commit `aef30c8d569c` exists in tree and adds
  `WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn)` in
  `kernel/irq/manage.c` (dated 2026-01-13).
- **Verified**: `drivers/mailbox/pcc.c` uses `devm_request_irq()` (line
  556), NOT `request_threaded_irq()`.
- **Verified**: No `IRQ_WAKE_THREAD` or `request_threaded_irq` usage in
  `pcc.c` (grep returned no matches).
- **Verified**: The `IRQF_ONESHOT` was introduced by commit
  `3db174e478cb0b` ("mailbox: pcc: Support shared interrupt for multiple
  subspaces", 2023-09-11), which is in the v6.7 cycle, making it present
  in 6.6.y stable and later.
- **Verified**: The change is a single flag removal (`IRQF_SHARED |
  IRQF_ONESHOT` → `IRQF_SHARED`), one line modified.
- **Verified**: Reviewed by Sudeep Holla (PCC/SCMI co-maintainer),
  reported by an ARM engineer.

### Conclusion

This is a straightforward fix for a WARN_ON triggered at runtime. The
change is minimal (removing one unused flag), obviously correct
(verified by code inspection that no threaded handler exists), reviewed
by a domain expert, and has zero risk of regression. It meets all stable
kernel criteria.

The only nuance is that the WARN_ON itself comes from a very recent
commit (`aef30c8d569c`), which may or may not be present in all stable
trees. However, even without that commit, removing the spurious
`IRQF_ONESHOT` is correct — the flag was never needed and was always a
no-op in this context. Removing incorrect flags proactively prevents
future issues and is good practice.

**YES**

 drivers/mailbox/pcc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index ff292b9e0be9e..060489e5ae6de 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -552,7 +552,7 @@ static int pcc_startup(struct mbox_chan *chan)
 
 	if (pchan->plat_irq > 0) {
 		irqflags = pcc_chan_plat_irq_can_be_shared(pchan) ?
-						IRQF_SHARED | IRQF_ONESHOT : 0;
+						IRQF_SHARED : 0;
 		rc = devm_request_irq(chan->mbox->dev, pchan->plat_irq, pcc_mbox_irq,
 				      irqflags, MBOX_IRQ_NAME, chan);
 		if (unlikely(rc)) {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-02-15 17:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-15 17:41 [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler Sasha Levin
2026-02-15 17:41 ` [PATCH AUTOSEL 6.19-6.1] mailbox: pcc: Remove spurious IRQF_ONESHOT usage Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox