From nobody Thu Apr 2 17:15:16 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4DBDB1E7C23; Thu, 12 Feb 2026 01:10:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770858617; cv=none; b=kPlIJR53R7D/ycKQ4Bdec4AKY1spdkUd+XEzrjRvnyGJRLju1yTveIhN3FSwEaMyl5tGn0RYTRGjUB1Xuf7lLlqVdTkp/YeM2voUUoFoim6OszD3JUpyCiSCdnzRqyL7XMIlNaf5p921FjdYVlpW4onSFtiQo0XF9BISUgdFaGI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770858617; c=relaxed/simple; bh=K2jh1daCdWMh9KQ1Cb8fDMYXLSHsnsx88Z2bTMtspTA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=H811rqLTnY9Yfz+qz/NbUZiQ0z8VycqVz2wvyCERXipquyKidA5LYRZhJh+u72+0ci4+dv3pSfLmjsvh6mEl7MzPnt556qjh/jLGO/obkiYV4i7Hpo0IRvO0aNjZEFGXGVPKqsfKGMG9HWPw5CVQzzJbdTEucDIeZ5oIS38mda4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=br+j/l9I; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="br+j/l9I" Received: by smtp.kernel.org (Postfix) with ESMTPSA id EE400C16AAE; Thu, 12 Feb 2026 01:10:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770858617; bh=K2jh1daCdWMh9KQ1Cb8fDMYXLSHsnsx88Z2bTMtspTA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=br+j/l9IYi/ovdwH/TU270uZYwZ70Xa4wpO3tqHyUuXcQ0qA4WfFulz2SgujyAFfU ZJphpLT/MHOC3I8F+AVGvtVV+zw7Va5xP4y5o9howIyY9EKItnew0RpYS1W0FLGmdu nm84VCMzrrdXlWuqn4SZo/Ov8+A6xA2Pa9zIDuu0Ea40xmSGh3pNtTjdBjGdqSXMyX C7LKCAT99t6Uf8C/R1asTGLVrSTooJ891Qh5TWOneJbTga7WYGHEbHYURHy1+p2aFP UNBmHpatzkpy+eUzjPYBHSQABjfzdkUT9+I033JRJ8l95myDL8OWK+Zcw0iYJGMUk8 sz8R+0+4ap2iA== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Sebastian Andrzej Siewior , Thomas Gleixner , Sasha Levin , jassisinghbrar@gmail.com, clrkwllms@kernel.org, rostedt@goodmis.org, linux-kernel@vger.kernel.org, linux-rt-devel@lists.linux.dev Subject: [PATCH AUTOSEL 6.19-5.10] mailbox: bcm-ferxrm-mailbox: Use default primary handler Date: Wed, 11 Feb 2026 20:09:35 -0500 Message-ID: <20260212010955.3480391-12-sashal@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260212010955.3480391-1-sashal@kernel.org> References: <20260212010955.3480391-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 6.19 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable From: Sebastian Andrzej Siewior [ Upstream commit 03843d95a4a4e0ba22ad4fcda65ccf21822b104c ] 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 flag 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 Signed-off-by: Thomas Gleixner Link: https://patch.msgid.link/20260128095540.863589-5-bigeasy@linutronix.de Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: ## Complete Analysis ### 1. COMMIT MESSAGE ANALYSIS The subject says "Use default primary handler" which sounds like a cleanup, but the commit body reveals a real bug: > "The lack of the IRQF_ONESHOT flag 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." The authors are Sebastian Andrzej Siewior (bigeasy, a core IRQ/RT developer) and Thomas Gleixner (the IRQ subsystem maintainer and co- author of the `__setup_irq` protection). This is coming from the very people who understand threaded IRQ semantics best. ### 2. CODE CHANGE ANALYSIS The change is small and surgical =E2=80=94 two distinct modifications: **A) Remove the redundant `flexrm_irq_event` primary handler:** ```1176:1182:drivers/mailbox/bcm-flexrm-mailbox.c 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; } ``` This function is **identical** in behavior to `irq_default_primary_handler()` in `kernel/irq/manage.c`: ```976:979:kernel/irq/manage.c static irqreturn_t irq_default_primary_handler(int irq, void *dev_id) { return IRQ_WAKE_THREAD; } ``` Both simply return `IRQ_WAKE_THREAD`. There's zero functional difference. **B) Change the `request_threaded_irq()` call:** Old code: ``` request_threaded_irq(ring->irq, flexrm_irq_event, flexrm_irq_thread, 0, ...) ``` New code: ``` request_threaded_irq(ring->irq, NULL, flexrm_irq_thread, IRQF_ONESHOT, ...) ``` ### 3. THE BUG MECHANISM This is a real bug with two dimensions: **Dimension 1: Missing IRQF_ONESHOT on non-ONESHOT_SAFE interrupts** The bcm-flexrm-mailbox driver uses **platform MSI** (via `platform_device_msi_init_and_alloc_irqs()`), NOT PCI MSI. I verified that while PCI MSI irqchips have `IRQCHIP_ONESHOT_SAFE` set (in `drivers/pci/msi/irqdomain.c`), platform MSI does NOT. This means the IRQ subsystem's safety net =E2=80=94 automatically stripping `IRQF_ONESHOT`= for chips that don't need it =E2=80=94 does not apply here. Without `IRQF_ONESHOT`, the interrupt line is **not masked** while the threaded handler (`flexrm_irq_thread`) runs. On a **level-triggered** interrupt line, this creates an interrupt storm: 1. Interrupt fires =E2=86=92 primary handler returns `IRQ_WAKE_THREAD` 2. Interrupt line is re-enabled immediately (no masking) 3. Device still has the line asserted =E2=86=92 interrupt fires again immediately 4. Goto 1 =E2=80=94 the thread never gets to run, the system is stuck in ha= rd IRQ context The commit message explicitly describes this: "especially on LEVEL typed interrupt lines, the interrupt can fire again before the threaded handler had a chance to run." **Dimension 2: Forced threading bypass** The old code provided an explicit primary handler (`flexrm_irq_event`), even though it's functionally identical to `irq_default_primary_handler`. This is problematic because `irq_setup_forced_threading()` has a special check: ```1302:1303:kernel/irq/manage.c if (new->handler =3D=3D irq_default_primary_handler) return 0; ``` When `handler !=3D irq_default_primary_handler` (i.e., it's the driver's custom `flexrm_irq_event`), forced threading proceeds and creates a **secondary action** with the original thread handler, and converts the primary handler to run in a thread too. This is wasteful and changes the behavior on PREEMPT_RT kernels =E2=80=94 instead of a simple wake-and-handle flow, it creates an unnecessary secondary handler chain. But more critically, with the old code and forced threading, the check at line 1295: ```1295:1296:kernel/irq/manage.c if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT)) return 0; ``` Since flags=3D0 (no IRQF_ONESHOT), forced threading continues and sets `IRQF_ONESHOT` at line 1305. But without forced threading (normal kernels), IRQF_ONESHOT is never set, and the interrupt runs without masking. **Why the old code doesn't hit the __setup_irq safety check:** The `__setup_irq` code at line 1664-1684 rejects `handler=3D=3DNULL` without `IRQF_ONESHOT` by checking `new->handler =3D=3D irq_default_primary_handler`. But since the old code passes `flexrm_irq_event` (a different function pointer that does the same thing), this safety check is **bypassed**. The driver sneaks past the protection that Thomas Gleixner himself added in commit 1c6c69525b40e ("genirq: Reject bogus threaded irq requests"). ### 4. CLASSIFICATION This is a **bug fix** =E2=80=94 specifically fixing a potential interrupt s= torm / system hang on level-triggered interrupt configurations. It's disguised as cleanup but addresses a real correctness issue. ### 5. SCOPE AND RISK ASSESSMENT - **Lines changed:** ~10 lines removed, ~2 lines changed =E2=80=94 extremely small - **Files touched:** 1 file (`drivers/mailbox/bcm-flexrm-mailbox.c`) - **Risk:** Very low. The change is: - Removing dead code (a function identical to `irq_default_primary_handler`) - Passing `NULL` + `IRQF_ONESHOT` instead of a custom handler + flags=3D0 - This is the canonical correct way to request a threaded IRQ with no real primary handler - **Could it break something?** No. The behavior with `IRQF_ONESHOT` is strictly safer =E2=80=94 the interrupt is masked during threaded handler execution. The primary handler behavior is identical (`IRQ_WAKE_THREAD`). ### 6. USER IMPACT - This driver is used on Broadcom iProc SoCs (embedded ARM) for FlexRM offload engine mailbox operations - The bug manifests as an **interrupt storm causing system hang** on level-triggered interrupt configurations - Even on edge-triggered (MSI) configurations, the missing `IRQF_ONESHOT` creates a window where the interrupt can re-fire before the thread handler runs, potentially causing lost completions or spurious interrupt warnings ### 7. STABILITY INDICATORS - **Author:** Sebastian Andrzej Siewior =E2=80=94 a core kernel developer, especially for PREEMPT_RT and IRQ subsystem - **Acked by:** Thomas Gleixner =E2=80=94 the IRQ subsystem maintainer and creator of the `__setup_irq` safety checks - The fix follows a well-established pattern used across many drivers - The patch is self-contained with no dependencies ### 8. DEPENDENCY CHECK The code being modified exists in all stable trees that have this driver. The driver `bcm-flexrm-mailbox.c` has been in the kernel since at least v4.14. The `request_threaded_irq()` with `NULL` + `IRQF_ONESHOT` pattern has been supported since the genirq safety check was added in 2012 (commit 1c6c69525b40e). This patch applies cleanly to any stable tree. ### CONCLUSION This commit fixes a real bug: a missing `IRQF_ONESHOT` flag that can cause an interrupt storm and system hang on level-triggered interrupt lines. The old code also inadvertently bypassed the kernel's own safety check for this exact scenario (by providing a custom handler identical to the default one). The fix is small (net -10 lines), self-contained, authored by core IRQ subsystem developers, and follows the canonical pattern for threaded interrupts. It has zero risk of regression =E2=80=94 `IRQF_ONESHOT` is strictly correct and the removed handler was functionally identical to the default. **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-fle= xrm-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) =20 /* =3D=3D=3D=3D=3D=3D FlexRM interrupt handler =3D=3D=3D=3D=3D */ =20 -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 =3D -ENODEV; goto fail_free_cmpl_memory; } - ret =3D request_threaded_irq(ring->irq, - flexrm_irq_event, - flexrm_irq_thread, - 0, dev_name(ring->mbox->dev), ring); + ret =3D 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); --=20 2.51.0