Linux-RISC-V Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-6.18] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run
       [not found] <20260212010955.3480391-1-sashal@kernel.org>
@ 2026-02-12  1:09 ` Sasha Levin
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-5.15] char: tpm: cr50: Remove IRQF_ONESHOT Sasha Levin
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] irqchip/riscv-imsic: Add a CPU pm notifier to restore the IMSIC on exit Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2026-02-12  1:09 UTC (permalink / raw)
  To: patches, stable
  Cc: Jiri Olsa, Mahe Tardy, Andrii Nakryiko, Steven Rostedt (Google),
	Will Deacon, Sasha Levin, song, ast, daniel, kpsingh,
	mattbobrowski, mhiramat, pjw, palmer, aou, svens, bpf,
	linux-kernel, linux-trace-kernel, linux-riscv

From: Jiri Olsa <jolsa@kernel.org>

[ Upstream commit 276f3b6daf6024ae2742afd161e7418a5584a660 ]

Mahe reported issue with bpf_override_return helper not working when
executed from kprobe.multi bpf program on arm.

The problem is that on arm we use alternate storage for pt_regs object
that is passed to bpf_prog_run and if any register is changed (which
is the case of bpf_override_return) it's not propagated back to actual
pt_regs object.

Fixing this by introducing and calling ftrace_partial_regs_update function
to propagate the values of changed registers (ip and stack).

Reported-by: Mahe Tardy <mahe.tardy@gmail.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Acked-by: Will Deacon <will@kernel.org>
Link: https://lore.kernel.org/bpf/20260112121157.854473-1-jolsa@kernel.org
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a complete picture. Let me provide the full analysis.

---

## Comprehensive Analysis

### 1. COMMIT MESSAGE ANALYSIS

The commit message is clear: `bpf_override_return` helper does not work
when executed from a **kprobe.multi** BPF program on arm64. This was
**reported by a real user** (Mahe Tardy), which indicates it's a real-
world bug affecting actual users. The fix has been **reviewed by Steven
Rostedt** (ftrace maintainer) and **acked by Will Deacon** (arm64
maintainer), two highly authoritative figures.

### 2. THE BUG MECHANISM (Detailed)

On arm64, the `ftrace_regs` structure (`struct __arch_ftrace_regs` in
`arch/arm64/include/asm/ftrace.h`) is **NOT** a wrapper around `pt_regs`
- it's a separate, smaller structure containing only x0-x8, fp, lr, sp,
pc. Arm64 defines `HAVE_ARCH_FTRACE_REGS`.

When `kprobe_multi_link_prog_run()` is called (in
`kernel/trace/bpf_trace.c`):

1. `ftrace_partial_regs(fregs, bpf_kprobe_multi_pt_regs_ptr())` copies
   data **from** `ftrace_regs` **to** a per-CPU `pt_regs` buffer. On
   arm64, this creates a **separate copy**.
2. `bpf_prog_run(link->link.prog, regs)` passes this `pt_regs` copy to
   the BPF program.
3. When `bpf_override_return` is called inside the BPF program, it
   modifies the `pt_regs` copy:
   - `regs_set_return_value(regs, rc)` - sets the return value
   - `override_function_with_return(regs)` - sets `regs->pc = regs->lr`
     (on arm64), redirecting execution to skip the probed function
4. **THE BUG**: After `bpf_prog_run()` returns, the modified `pt_regs`
   copy is simply discarded. The changes are **never propagated back**
   to the original `ftrace_regs`, so the instruction pointer override
   and return value changes are lost.

On x86_64 (and s390, powerpc, loongarch), this bug does NOT manifest
because they define `CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS`, meaning
`ftrace_partial_regs()` returns a pointer directly into the
`ftrace_regs` structure — changes to the `pt_regs` automatically update
the `ftrace_regs`.

### 3. THE FIX

The fix introduces a new function `ftrace_partial_regs_update()` with
two variants:

- **For architectures without `HAVE_ARCH_FTRACE_REGS`** (x86, etc.):
  Empty no-op, because `pt_regs` is embedded in `ftrace_regs` directly.
- **For architectures with `HAVE_ARCH_FTRACE_REGS`** (arm64, riscv):
  Copies the instruction pointer and return value back from the separate
  `pt_regs` to the `ftrace_regs`:

```c
ftrace_regs_set_instruction_pointer(fregs, instruction_pointer(regs));
ftrace_regs_set_return_value(fregs, regs_return_value(regs));
```

And in `bpf_trace.c`, the fix calls this function after
`bpf_prog_run()`:
```c
ftrace_partial_regs_update(fregs, bpf_kprobe_multi_pt_regs_ptr());
```

### 4. SCOPE AND RISK ASSESSMENT

- **Files changed**: 2 files
- **Lines added**: ~25 (mostly documentation)
- **Lines removed**: 0
- **Actual code changes**: ~5 lines of actual logic
- **Risk**: Very LOW. The no-op version for non-arm64 architectures
  means zero impact on x86. The arm64 version just copies two values
  back.

### 5. DEPENDENCY CHECK - CRITICAL ISSUE

This is where the analysis gets complex. The affected code —
`kprobe_multi_link_prog_run()` using `ftrace_regs` and
`ftrace_partial_regs()` — was introduced as part of a massive fprobe
rewrite in the **v6.14 merge window**:

- `b9b55c8912ce1` (v6.14): `tracing: Add ftrace_partial_regs() for
  converting ftrace_regs to pt_regs`
- `46bc082388560` (v6.14): `fprobe: Use ftrace_regs in fprobe entry
  handler`
- `8e2759da9378` (v6.14): `bpf: Enable kprobe_multi feature if
  CONFIG_FPROBE is enabled`
- `4346ba1604093` (v6.14): `fprobe: Rewrite fprobe on function-graph
  tracer`

In **v6.13 and earlier** (6.12 LTS, 6.6 LTS, 6.1 LTS, 5.15 LTS),
`kprobe_multi_link_prog_run()` takes `struct pt_regs *regs` directly —
there is no `ftrace_regs` / `ftrace_partial_regs()` indirection. **The
bug simply does not exist in those older stable trees.**

The bug exists in: **v6.14, v6.15, v6.18** stable trees (all currently
maintained).

### 6. BACKPORT FEASIBILITY

The patch should apply cleanly to:
- **v6.14.y**: The code at `bpf_trace.c` is very similar (just needs
  adjustment for `migrate_disable/enable` which was still present in
  v6.14)
- **v6.15.y**: Very similar to HEAD
- **v6.18.y**: Identical to HEAD (`include/linux/ftrace_regs.h`
  unchanged since v6.14)

The `include/linux/ftrace_regs.h` change should apply cleanly to all
three. The `bpf_trace.c` change might need minor adjustment for v6.14
(which still has `migrate_disable()` calls), but the relevant line is
the same.

### 7. USER IMPACT

This bug makes `bpf_override_return` completely non-functional on arm64
when used from kprobe.multi programs. This is a significant feature
regression:
- `bpf_override_return` is used by BPF-based error injection frameworks
- arm64 is a major platform (server, embedded, Android)
- The bug was reported by an actual user

### 8. CLASSIFICATION

This is a **clear bug fix** for a **functional regression** introduced
in v6.14. It:
- Fixes a real, user-reported bug
- Is small and surgical (~5 lines of actual logic)
- Has been reviewed by the ftrace maintainer (Rostedt) and arm64
  maintainer (Will Deacon)
- Has clear scope and low regression risk
- Only affects arm64 and riscv (no impact on x86)

The fix meets all stable kernel criteria:
1. Obviously correct and tested (reviewed/acked by subsystem
   maintainers)
2. Fixes a real bug (user-reported, bpf_override_return completely
   broken on arm64)
3. Important issue (complete feature breakage on a major architecture)
4. Small and contained (2 files, ~5 lines of logic)
5. Does not introduce new features (just propagates existing register
   values back)

**YES**

 include/linux/ftrace_regs.h | 25 +++++++++++++++++++++++++
 kernel/trace/bpf_trace.c    |  1 +
 2 files changed, 26 insertions(+)

diff --git a/include/linux/ftrace_regs.h b/include/linux/ftrace_regs.h
index 15627ceea9bcc..386fa48c4a957 100644
--- a/include/linux/ftrace_regs.h
+++ b/include/linux/ftrace_regs.h
@@ -33,6 +33,31 @@ struct ftrace_regs;
 #define ftrace_regs_get_frame_pointer(fregs) \
 	frame_pointer(&arch_ftrace_regs(fregs)->regs)
 
+static __always_inline void
+ftrace_partial_regs_update(struct ftrace_regs *fregs, struct pt_regs *regs) { }
+
+#else
+
+/*
+ * ftrace_partial_regs_update - update the original ftrace_regs from regs
+ * @fregs: The ftrace_regs to update from @regs
+ * @regs: The partial regs from ftrace_partial_regs() that was updated
+ *
+ * Some architectures have the partial regs living in the ftrace_regs
+ * structure, whereas other architectures need to make a different copy
+ * of the @regs. If a partial @regs is retrieved by ftrace_partial_regs() and
+ * if the code using @regs updates a field (like the instruction pointer or
+ * stack pointer) it may need to propagate that change to the original @fregs
+ * it retrieved the partial @regs from. Use this function to guarantee that
+ * update happens.
+ */
+static __always_inline void
+ftrace_partial_regs_update(struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	ftrace_regs_set_instruction_pointer(fregs, instruction_pointer(regs));
+	ftrace_regs_set_return_value(fregs, regs_return_value(regs));
+}
+
 #endif /* HAVE_ARCH_FTRACE_REGS */
 
 /* This can be overridden by the architectures */
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index fe28d86f7c357..2a5dabda8b5c2 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2564,6 +2564,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 	old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
 	err = bpf_prog_run(link->link.prog, regs);
 	bpf_reset_run_ctx(old_run_ctx);
+	ftrace_partial_regs_update(fregs, bpf_kprobe_multi_pt_regs_ptr());
 	rcu_read_unlock();
 
  out:
-- 
2.51.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH AUTOSEL 6.19-5.15] char: tpm: cr50: Remove IRQF_ONESHOT
       [not found] <20260212010955.3480391-1-sashal@kernel.org>
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run Sasha Levin
@ 2026-02-12  1:09 ` Sasha Levin
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] irqchip/riscv-imsic: Add a CPU pm notifier to restore the IMSIC on exit Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2026-02-12  1:09 UTC (permalink / raw)
  To: patches, stable
  Cc: Sebastian Andrzej Siewior, Thomas Gleixner, Jarkko Sakkinen,
	Sasha Levin, peterhuewe, clrkwllms, rostedt, dlan,
	linux-integrity, linux-rt-devel, linux-riscv, spacemit

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

[ Upstream commit 1affd29ffbd50125a5492c6be1dbb1f04be18d4f ]

Passing IRQF_ONESHOT ensures that the interrupt source is masked until
the secondary (threaded) handler is done. If only a primary handler is
used then the flag makes no sense because the interrupt can not fire
(again) while its handler is running.

The flag also prevents force-threading of the primary handler and the
irq-core will warn about this.

Remove IRQF_ONESHOT from irqflags.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Link: https://patch.msgid.link/20260128095540.863589-10-bigeasy@linutronix.de
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a complete understanding. Let me write up the comprehensive
analysis.

---

## Comprehensive Analysis

### 1. Commit Message Analysis

The commit subject is "char: tpm: cr50: Remove IRQF_ONESHOT" and the
message explains three points:
1. `IRQF_ONESHOT` is for masking the interrupt source until the
   **threaded** (secondary) handler completes
2. These drivers only have primary handlers, so the flag is meaningless
3. The flag **prevents force-threading** of the primary handler, and the
   irq-core will warn about this

The commit is authored by **Sebastian Andrzej Siewior** (bigeasy), the
PREEMPT_RT maintainer, and signed off by **Thomas Gleixner**, the IRQ
subsystem maintainer and co-maintainer of the Linux kernel. It was
reviewed by **Jarkko Sakkinen**, the TPM subsystem maintainer.

### 2. Code Change Analysis

The change is minimal and surgical:
- **`tpm_tis_i2c_cr50.c`**: Removes `IRQF_ONESHOT` from the flags in
  `devm_request_irq()`, keeping `IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN`
- **`tpm_tis_spi_cr50.c`**: Removes `IRQF_ONESHOT` from the flags in
  `devm_request_irq()`, keeping `IRQF_TRIGGER_RISING`

Both interrupt handlers (`tpm_cr50_i2c_int_handler` at line 74 and
`cr50_spi_irq_handler` at line 65) are trivially simple - they just call
`complete()` and return `IRQ_HANDLED`. There is no thread_fn.
`devm_request_irq()` is a wrapper that calls
`devm_request_threaded_irq()` with `thread_fn = NULL`.

### 3. The Real Bug

The companion commit **`aef30c8d569c`** ("genirq: Warn about using
IRQF_ONESHOT without a threaded handler") was merged on 2026-01-12 and
adds a `WARN_ON_ONCE()` in `__setup_irq()`:

```c
WARN_ON_ONCE(new->flags & IRQF_ONESHOT && !new->thread_fn);
```

This means that **without this cr50 fix**, every time the cr50 TPM
driver probes on a system with the updated IRQ core, it will emit a
`WARN_ON_ONCE` kernel warning at boot. This is a real runtime issue that
would affect all Chromebook and other systems using cr50/ti50 TPM chips.

More importantly, the core technical issue is that `IRQF_ONESHOT`
prevents force-threading of the primary handler. From
`irq_setup_forced_threading()` in `kernel/irq/manage.c`:

```c
if (new->flags & (IRQF_NO_THREAD | IRQF_PERCPU | IRQF_ONESHOT))
    return 0;  // Skip force-threading!
```

On **PREEMPT_RT kernels** (where `force_irqthreads()` returns `true`),
this means the cr50 interrupt handler runs in hardirq context instead of
being force-threaded. While the handler itself (`complete()`) is safe in
hardirq context, this defeats the PREEMPT_RT design goal of having all
interrupt handlers run in thread context. On non-RT systems with
`threadirqs` boot parameter, the same issue occurs.

### 4. Classification

This is a **bug fix** that addresses:
1. **A spurious kernel warning** triggered by the new `WARN_ON_ONCE`
   check added in `aef30c8d569c`
2. **Incorrect IRQ flags** - `IRQF_ONESHOT` has never been semantically
   correct for these drivers since they only use primary handlers
3. **PREEMPT_RT compatibility** - the flag prevents force-threading,
   which can be problematic

The fix is not a feature addition - it removes an incorrect flag that
was always wrong.

### 5. Scope and Risk Assessment

- **Lines changed**: 2 files, 2 lines total (removing `IRQF_ONESHOT`
  from two flag constants)
- **Risk**: Extremely low. `IRQF_ONESHOT` had no functional effect on
  these drivers since they use only primary handlers. The interrupt
  cannot fire while its primary handler is already running (hardware
  guarantee), so removing the flag changes nothing about interrupt
  masking behavior.
- **Subsystem**: TPM drivers, well-understood, mature code
- **Testing**: Reviewed by TPM maintainer (Jarkko Sakkinen), authored by
  PREEMPT_RT maintainer, signed off by IRQ core maintainer

### 6. User Impact

- **Who is affected**: All users with cr50/ti50 TPM chips (primarily
  Chromebooks and Google-based embedded devices)
- **Severity without fix**: Kernel `WARN_ON_ONCE` at boot if the
  `aef30c8d569c` warning commit is also backported; PREEMPT_RT
  degradation
- **Severity of regression risk**: Essentially zero - the flag had no
  functional purpose

### 7. Dependencies

This commit is **self-contained**. It does not require any other patches
to apply cleanly. However, the motivating warning comes from
`aef30c8d569c` - if that commit is backported to stable, this fix
becomes essential to avoid warnings. Even without the warning commit,
this fix is correct and beneficial (enables proper force-threading on RT
systems).

The i2c-spacemit driver already got a similar fix (`e351836a54e3`) with
`Cc: stable@vger.kernel.org` tag, establishing precedent that these
IRQF_ONESHOT removal patches are considered stable material.

### 8. Stability Indicators

- Author: Sebastian Andrzej Siewior (PREEMPT_RT maintainer) - domain
  expert
- Signed-off-by: Thomas Gleixner (IRQ subsystem maintainer) - domain
  expert
- Reviewed-by: Jarkko Sakkinen (TPM maintainer) - domain expert
- Part of a systematic cleanup pattern with prior art (i2c-exynos5,
  i2c-hix5hd2, i2c-spacemit, drm/msm)

### Conclusion

This is a small, surgical, obviously correct bug fix. The `IRQF_ONESHOT`
flag was always semantically wrong for these primary-only handlers. The
fix removes a flag that had no functional benefit but actively prevented
force-threading on PREEMPT_RT systems and will trigger a `WARN_ON_ONCE`
with the companion IRQ core warning check. The risk is near-zero, the
change is trivial (2 lines across 2 files), it was reviewed by all three
relevant subsystem maintainers, and there is clear precedent for
identical fixes being marked as stable material (the i2c-spacemit fix
had `Cc: stable@vger.kernel.org`).

**YES**

 drivers/char/tpm/tpm_tis_i2c_cr50.c | 3 +--
 drivers/char/tpm/tpm_tis_spi_cr50.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_i2c_cr50.c b/drivers/char/tpm/tpm_tis_i2c_cr50.c
index fc6891a0b6936..b48cacacc0664 100644
--- a/drivers/char/tpm/tpm_tis_i2c_cr50.c
+++ b/drivers/char/tpm/tpm_tis_i2c_cr50.c
@@ -749,8 +749,7 @@ static int tpm_cr50_i2c_probe(struct i2c_client *client)
 
 	if (client->irq > 0) {
 		rc = devm_request_irq(dev, client->irq, tpm_cr50_i2c_int_handler,
-				      IRQF_TRIGGER_FALLING | IRQF_ONESHOT |
-				      IRQF_NO_AUTOEN,
+				      IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN,
 				      dev->driver->name, chip);
 		if (rc < 0) {
 			dev_err(dev, "Failed to probe IRQ %d\n", client->irq);
diff --git a/drivers/char/tpm/tpm_tis_spi_cr50.c b/drivers/char/tpm/tpm_tis_spi_cr50.c
index f4937280e9406..32920b4cecfb4 100644
--- a/drivers/char/tpm/tpm_tis_spi_cr50.c
+++ b/drivers/char/tpm/tpm_tis_spi_cr50.c
@@ -287,7 +287,7 @@ int cr50_spi_probe(struct spi_device *spi)
 	if (spi->irq > 0) {
 		ret = devm_request_irq(&spi->dev, spi->irq,
 				       cr50_spi_irq_handler,
-				       IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+				       IRQF_TRIGGER_RISING,
 				       "cr50_spi", cr50_phy);
 		if (ret < 0) {
 			if (ret == -EPROBE_DEFER)
-- 
2.51.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH AUTOSEL 6.19-6.18] irqchip/riscv-imsic: Add a CPU pm notifier to restore the IMSIC on exit
       [not found] <20260212010955.3480391-1-sashal@kernel.org>
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run Sasha Levin
  2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-5.15] char: tpm: cr50: Remove IRQF_ONESHOT Sasha Levin
@ 2026-02-12  1:09 ` Sasha Levin
  2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2026-02-12  1:09 UTC (permalink / raw)
  To: patches, stable
  Cc: Nick Hu, Thomas Gleixner, Yong-Xuan Wang, Cyan Yang, Anup Patel,
	Nutty Liu, Sasha Levin, tglx, pjw, palmer, aou, linux-riscv,
	linux-kernel

From: Nick Hu <nick.hu@sifive.com>

[ Upstream commit f48b4bd0915bf61ac12b8c65c7939ebd03bc8abf ]

The IMSIC might be reset when the system enters a low power state, but on
exit nothing restores the registers, which prevents interrupt delivery.

Solve this by registering a CPU power management notifier, which restores
the IMSIC on exit.

Signed-off-by: Nick Hu <nick.hu@sifive.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Yong-Xuan Wang <yongxuan.wang@sifive.com>
Reviewed-by: Cyan Yang <cyan.yang@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Reviewed-by: Nutty Liu <liujingqi@lanxincomputing.com>
Link: https://patch.msgid.link/20251202-preserve-aplic-imsic-v3-1-1844fbf1fe92@sifive.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a comprehensive understanding. Let me summarize the analysis.

---

## Detailed Analysis

### 1. Commit Message Analysis

The commit message is clear and directly describes a **bug fix**: "The
IMSIC might be reset when the system enters a low power state, but on
exit nothing restores the registers, which prevents interrupt delivery."
This is unambiguous — without the fix, **interrupt delivery is broken**
after returning from a low power state.

The patch is from Nick Hu at SiFive (a major RISC-V silicon vendor),
signed-off by Thomas Gleixner (the overall irqchip maintainer), and
reviewed by:
- Anup Patel (original IMSIC driver author/maintainer at Ventana Micro)
- Yong-Xuan Wang, Cyan Yang (SiFive reviewers)
- Nutty Liu (Lanxin Computing)

This level of review by domain experts demonstrates the fix is well-
vetted.

### 2. Code Change Analysis

The change is minimal (31 lines added, 8 removed, single file) and
consists of three logical parts:

**a) Refactoring — extracting `imsic_hw_states_init()`:**

The existing code from `imsic_starting_cpu()` that handles hardware
register initialization is moved into a new helper:

```126:147:drivers/irqchip/irq-riscv-imsic-early.c
static int imsic_starting_cpu(unsigned int cpu)
{
        /* Mark per-CPU IMSIC state as online */
        imsic_state_online();

        /* Enable per-CPU parent interrupt */
        enable_percpu_irq(imsic_parent_irq,
irq_get_trigger_type(imsic_parent_irq));

        /* Setup IPIs */
        imsic_ipi_starting_cpu();

        /*
  - Interrupts identities might have been enabled/disabled while
  - this CPU was not running so sync-up local enable/disable state.
         */
        imsic_local_sync_all(true);

        /* Enable local interrupt delivery */
        imsic_local_delivery(true);

        return 0;
}
```

The three operations (`imsic_ipi_starting_cpu()`,
`imsic_local_sync_all(true)`, `imsic_local_delivery(true)`) are
extracted into `imsic_hw_states_init()`, which is then called from both
`imsic_starting_cpu()` and the new PM notifier.

**b) Adding the CPU PM notifier:**

A new `imsic_pm_notifier` function handles `CPU_PM_EXIT` by calling
`imsic_hw_states_init()`. This is the **exact same pattern** used by
GICv3 (`irq-gic-v3.c:1482`) and GIC (`irq-gic.c`), which have been
stable for years.

**c) Registering the notifier:**

The `imsic_early_probe()` return is changed from `return 0` to `return
cpu_pm_register_notifier(&imsic_pm_notifier_block)`.

### 3. Bug Mechanism and Severity

**The bug**: On RISC-V systems with SBI-based cpuidle (the standard CPU
idle mechanism), when a CPU enters a deep idle state, the SBI firmware
may power down the IMSIC. The `cpuidle-riscv-sbi.c` driver calls
`cpu_pm_enter()` before and `cpu_pm_exit()` after the idle transition.
`cpu_pm_exit()` fires `CPU_PM_EXIT` notifications to all registered
handlers. Without this patch, no handler exists for IMSIC, so:

1. `imsic_local_delivery` — the EIDELIVERY/EITHRESHOLD CSRs may be
   reset, disabling all interrupt delivery
2. `imsic_local_sync_all` — individual interrupt enable bits may be
   reset, leaving all interrupt sources disabled
3. `imsic_ipi_starting_cpu` — the IPI enable bit may be reset, breaking
   inter-processor interrupts

**Impact**: The CPU effectively becomes deaf to all interrupts — IPIs,
device interrupts, timer interrupts routed through IMSIC — after
returning from a deep idle state. This can cause:
- System hangs (no timer interrupts)
- IPI failures (scheduler hangs, RCU stalls)
- Device interrupt loss (I/O failures, network timeouts)

This bug affects any RISC-V system using IMSIC with cpuidle deep states
— which includes SiFive boards and other production RISC-V hardware.

### 4. Dependency Check

The patch is **self-contained**. It depends only on:
- `imsic_ipi_starting_cpu()` — exists in all stable trees with IMSIC
  (6.12.y+)
- `imsic_local_sync_all()` — exists in all stable trees with IMSIC
  (6.12.y+)
- `imsic_local_delivery()` — exists in all stable trees with IMSIC
  (6.12.y+)
- `cpu_pm_register_notifier()` — exists in all kernel versions (core
  kernel PM API)
- `cpuidle-riscv-sbi.c` — exists in all stable trees with IMSIC, and
  calls `cpu_pm_enter()`/`cpu_pm_exit()`

The companion APLIC patch (95a8ddde36601) touches a different driver
entirely and is not a dependency.

### 5. Backport Applicability

- **IMSIC driver exists**: 6.12.y through 6.19.y (added in v6.10)
- **Clean backport**: The `imsic_starting_cpu` function is byte-
  identical between the mainline pre-image and 6.12.y. Only minor
  context conflict in the `#include` section (6.12.y lacks
  `<linux/export.h>` that mainline has), trivially resolved.
- **Bug triggerable**: The cpuidle-riscv-sbi driver in 6.12.y already
  calls `cpu_pm_enter()`/`cpu_pm_exit()`, meaning the IMSIC registers
  get reset with no restoration.

### 6. Risk Assessment

**Very low risk**:
- The refactored code (`imsic_hw_states_init`) performs exactly the same
  operations as before — just from an additional call site
- The GICv3 driver has used this exact pattern (CPU PM notifier
  restoring interrupt controller state) for over a decade
- No behavior changes to the existing CPU hotplug path
- Well-reviewed by all key stakeholders

### 7. Conclusion

This is a clear-cut bug fix for a critical issue: **complete loss of
interrupt delivery** after RISC-V CPUs return from deep idle states. The
fix is:
- Small and surgical (31 lines added to a single file)
- Based on a well-established pattern (identical to GICv3/GIC CPU PM
  handling)
- Thoroughly reviewed by domain experts
- Self-contained with no dependencies on other patches
- Cleanly backportable to all stable trees containing IMSIC (6.12.y+)

**YES**

 drivers/irqchip/irq-riscv-imsic-early.c | 39 ++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-riscv-imsic-early.c b/drivers/irqchip/irq-riscv-imsic-early.c
index 6bac67cc0b6d9..ba903fa689bd5 100644
--- a/drivers/irqchip/irq-riscv-imsic-early.c
+++ b/drivers/irqchip/irq-riscv-imsic-early.c
@@ -7,6 +7,7 @@
 #define pr_fmt(fmt) "riscv-imsic: " fmt
 #include <linux/acpi.h>
 #include <linux/cpu.h>
+#include <linux/cpu_pm.h>
 #include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/init.h>
@@ -123,14 +124,8 @@ static void imsic_handle_irq(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static int imsic_starting_cpu(unsigned int cpu)
+static void imsic_hw_states_init(void)
 {
-	/* Mark per-CPU IMSIC state as online */
-	imsic_state_online();
-
-	/* Enable per-CPU parent interrupt */
-	enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq));
-
 	/* Setup IPIs */
 	imsic_ipi_starting_cpu();
 
@@ -142,6 +137,18 @@ static int imsic_starting_cpu(unsigned int cpu)
 
 	/* Enable local interrupt delivery */
 	imsic_local_delivery(true);
+}
+
+static int imsic_starting_cpu(unsigned int cpu)
+{
+	/* Mark per-CPU IMSIC state as online */
+	imsic_state_online();
+
+	/* Enable per-CPU parent interrupt */
+	enable_percpu_irq(imsic_parent_irq, irq_get_trigger_type(imsic_parent_irq));
+
+	/* Initialize the IMSIC registers to enable the interrupt delivery */
+	imsic_hw_states_init();
 
 	return 0;
 }
@@ -157,6 +164,22 @@ static int imsic_dying_cpu(unsigned int cpu)
 	return 0;
 }
 
+static int imsic_pm_notifier(struct notifier_block *self, unsigned long cmd, void *v)
+{
+	switch (cmd) {
+	case CPU_PM_EXIT:
+		/* Initialize the IMSIC registers to enable the interrupt delivery */
+		imsic_hw_states_init();
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block imsic_pm_notifier_block = {
+	.notifier_call = imsic_pm_notifier,
+};
+
 static int __init imsic_early_probe(struct fwnode_handle *fwnode)
 {
 	struct irq_domain *domain;
@@ -194,7 +217,7 @@ static int __init imsic_early_probe(struct fwnode_handle *fwnode)
 	cpuhp_setup_state(CPUHP_AP_IRQ_RISCV_IMSIC_STARTING, "irqchip/riscv/imsic:starting",
 			  imsic_starting_cpu, imsic_dying_cpu);
 
-	return 0;
+	return cpu_pm_register_notifier(&imsic_pm_notifier_block);
 }
 
 static int __init imsic_early_dt_init(struct device_node *node, struct device_node *parent)
-- 
2.51.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, other threads:[~2026-02-12  1:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260212010955.3480391-1-sashal@kernel.org>
2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] arm64/ftrace,bpf: Fix partial regs after bpf_prog_run Sasha Levin
2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-5.15] char: tpm: cr50: Remove IRQF_ONESHOT Sasha Levin
2026-02-12  1:09 ` [PATCH AUTOSEL 6.19-6.18] irqchip/riscv-imsic: Add a CPU pm notifier to restore the IMSIC on exit Sasha Levin

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