arm64: cross-CPU NMI via SDEI
v4 cover arm64: cross-CPU NMI via SDEI
From: "Kiryl Shutsemau (Meta)" <[email protected]>
A class of debug/observability features needs to interrupt a CPU that has its interrupts locally masked: the all-CPU backtrace behind sysrq-l / RCU-stall / hung-task / hard-lockup dumps, and crash_smp_send_stop() capturing a stuck CPU's state into the vmcore. On arm64 these need a mechanism that reaches a CPU spinning with DAIF masked, which a normal IPI cannot.
arm64 has two such mechanisms today:
- GICv3 pseudo-NMI (interrupt priority masking). Its cost is on the interrupt mask/unmask hot path: local_irq_enable() becomes an ICC_PMR_EL1 write plus a synchronising barrier, and exception entry/exit save and restore the PMR, paid on every CPU whether or not an NMI is ever delivered. In our measurements, enabling pseudo-NMI costs up to ~5% on real workloads, and ~66% on a syscall-in-a-loop microbenchmark. A fleet-wide ~5% regression is not acceptable, so these systems run with pseudo-NMI disabled.
- FEAT_NMI (Armv8.8) -- the architectural fix, but absent from deployed silicon and from most of the fleet for years to come.
For deployments that do not run pseudo-NMI, the backtrace and crash paths are degraded: a plain IPI can't reach the masked CPU, so the backtrace of the CPU you care about comes back empty and the kdump is missing the culprit's registers. The hard-lockup detector on these systems is the software buddy detector (HARDLOCKUP_DETECTOR_BUDDY): it detects a stall from a neighbour CPU, but it cannot itself interrupt the wedged CPU, so its report has no stack for the culprit and (with hardlockup_panic) the panic runs on the bystander.
This series adds a third delivery backend that costs nothing on the hot
path: SDEI. Firmware delivers an SDEI event into a CPU regardless of its DAIF state, so interrupt masking stays the cheap PSTATE.DAIF operation and the firmware round-trip is paid only at the rare moment a CPU must be interrupted.
It does not add a hard-lockup detector. Detection stays with the buddy detector (CONFIG_HARDLOCKUP_DETECTOR_PREFER_BUDDY); this series gives the backtrace and crash-stop paths -- including the buddy detector's backtrace of the stalled CPU -- a way to actually reach a masked CPU.
Mechanism
=========
It uses the standard SDEI software-signalled event (event 0) and the SDEI_EVENT_SIGNAL call (DEN0054) -- a spec-defined cross-PE signal, not a vendor extension. The driver registers a handler for event 0 and pokes a target CPU with sdei_event_signal(0, target_mpidr); firmware makes event 0 pending on that PE and dispatches the handler NMI-like.
No firmware change is required beyond SDEI being enabled, which firmware-first RAS (APEI/GHES) deployments already have; the only SDEI-core addition is a thin sdei_event_signal() wrapper over the standard call.
Prior SDEI watchdog work
========================
Out-of-tree SDEI hard-lockup watchdogs exist (e.g. in the openEuler and Anolis kernels). They bind the secure physical timer as an SDEI event, so firmware delivers a periodic self-CPU tick that drives a detector. That requires a new SDEI interrupt-binding API, pushes the watchdog period into firmware, and adds secure-timer EOI handling on the kexec path. This series instead uses only the standard software-signalled event 0, keeps all timing in the kernel (the buddy detector), and the same delivery primitive serves the backtrace and crash-stop users, not just lockup reporting.
Not included / follow-ups
=========================
- No SDEI hard-lockup-detector backend. v1 had one; it is dropped here. The buddy detector plus this series' backtrace already cover the no-pseudo-NMI case, and a dedicated SDEI backend duplicated the perf-NMI detector it had to compile-exclude. Run PREFER_BUDDY.
- A CPU stopped by the SDEI rung is parked, not powered off via PSCI CPU_OFF. Reaching and dumping the wedged CPU -- the point of the series -- works, and it matches the shared stop path's own park fallback when CPU_OFF is unavailable. The consequence is that an SMP crash-capture kernel cannot re-online such a CPU (it stays "already on"); the capture kernel boots and runs on the remaining CPUs. Powering the stopped CPU off so a capture kernel can reclaim it requires completing the SDEI event and then CPU_OFF, which hit a firmware-specific issue still under investigation; it is left as a follow-up and does not affect the dump's contents.
Testing
=======
Developed on QEMU 'virt' (Trusted Firmware-A with SDEI enabled) and validated on NVIDIA Grace (Neoverse V2) hardware, under irqchip.gicv3_pseudo_nmi=0 with HARDLOCKUP_DETECTOR_PREFER_BUDDY=y:
- sysrq-l backtrace of an interrupt-masked CPU returns its real stack,
pstate showing DAIF set -- proof SDEI delivered into the masked CPU;
- buddy detector catches a hard lockup (LKDTM) and the wedged CPU's
stack is fetched via the SDEI backtrace; - reboot/halt and the panic/kdump crash stop reach a wedged CPU via the SDEI rung ("SMP: retry stop with SDEI NMI for CPUs N"), and the kdump captures the wedged CPU's registers in the vmcore;
- with SDEI absent (plain QEMU 'virt', no firmware support) the driver stays inert: no event registration and no boot-time warning.
Changes since v3
================
- New sdei_is_present() patch; the NMI initcall now skips registration (and its boot warning) on non-SDEI systems (Puranjay Mohan).
- Fixed a NULL deref on a parallel-panic crash stop and the CONFIG_KEXEC_CORE=n build (Puranjay Mohan).
- kernel-doc + barrier comments on the stop path; reordered the two arm_sdei core patches (Doug Anderson).
Changes since v2
================
- Unified the CPU-stop paths into one arm64_nmi_cpu_stop(regs, die_on_crash), dropping local_cpu_stop()/ipi_cpu_crash_stop().
- SDEI rung tests sdei_nmi_active() first; sdei_nmi_stop_cpus() is void.
- Replaced the per-CPU stop cpumask with a write-once flag.
- Commented the SDEI-park / no-CPU_OFF rationale.
Changes since v1
================
- Dropped the SDEI hard-lockup-detector patch; use the buddy detector.
- Reworked crash-stop into a third rung of smp_send_stop().
- Renamed the driver to arm_sdei_nmi.c; widened the MAINTAINERS glob.
v3: https://lore.kernel.org/all/[email protected]
v2: https://lore.kernel.org/all/[email protected]
v1: https://lore.kernel.org/all/[email protected]
Also available at:
git://git.kernel.org/pub/scm/linux/kernel/git/kas/linux.git sdei-nmi/v4
Kiryl Shutsemau (Meta) (4):
firmware: arm_sdei: add sdei_is_present()
firmware: arm_sdei: add SDEI_EVENT_SIGNAL support drivers/firmware: add SDEI cross-CPU NMI service for arm64
arm64: escalate smp_send_stop() to an SDEI NMI as a last resort
MAINTAINERS | 2 +- arch/arm64/include/asm/nmi.h | 48 +++++++ arch/arm64/kernel/smp.c | 124 ++++++++++++----- drivers/firmware/Kconfig | 21 +++ drivers/firmware/Makefile | 1 + drivers/firmware/arm_sdei.c | 22 +++ drivers/firmware/arm_sdei_nmi.c | 238 ++++++++++++++++++++++++++++++++ include/linux/arm_sdei.h | 9 ++ include/uapi/linux/arm_sdei.h | 1 + 9 files changed, 427 insertions(+), 39 deletions(-) create mode 100644 arch/arm64/include/asm/nmi.h create mode 100644 drivers/firmware/arm_sdei_nmi.c
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6 --
2.54.0
PATCH v4 1/4 firmware: arm_sdei: add sdei_is_present()
From: "Kiryl Shutsemau (Meta)" <[email protected]>
invoke_sdei_fn() returns -EIO when no SDEI conduit was probed, and the core warns ("Failed to create event ...") on any registration that hits that. An optional consumer that registers an event from an unconditional initcall would therefore make every boot on a non-SDEI system emit that warning for what is simply absent firmware.
Expose whether SDEI firmware is present so such a consumer can skip registration -- and the warning -- when there is nothing to talk to.
--- drivers/firmware/arm_sdei.c | 10 ++++++++++ include/linux/arm_sdei.h | 3 +++ 2 files changed, 13 insertions(+)
drivers/firmware/arm_sdei.c +10 -0
@@ -339,6 +339,16 @@ static void _ipi_unmask_cpu(void *ignored)sdei_unmask_local_cpu (); } +/* +* Was SDEI firmware probed and is it usable? Lets optional consumers skip +* registering an event -- and the warning a failed registration emits -- on +* systems with no SDEI. +*/ +bool sdei_is_present (void ) +{ +return sdei_firmware_call; +} +static void _ipi_private_reset (void *ignored) {int err;
include/linux/arm_sdei.h +3 -0
@@ -37,6 +37,9 @@ int sdei_event_unregister(u32 event_num);int sdei_event_enable (u32 event_num);int sdei_event_disable (u32 event_num); +/* Was SDEI firmware probed and usable? */ +bool sdei_is_present (void ); +/* GHES register/unregister helpers */ int sdei_register_ghes (struct ghes *ghes,sdei_event_callback *normal_cb,sdei_event_callback *critical_cb);
Signature · 1 line
2.54.0
PATCH v4 2/4 firmware: arm_sdei: add SDEI_EVENT_SIGNAL support
In reply to Kiryl ShutsemauFrom: "Kiryl Shutsemau (Meta)" <[email protected]>
Add sdei_event_signal(), a thin wrapper over the SDEI_EVENT_SIGNAL call (DEN0054) that makes the software-signalled event (event 0) pending on a target PE -- delivered NMI-like even when that PE has interrupts masked. It takes no locks, so it is safe to call from NMI / crash context.
--- drivers/firmware/arm_sdei.c | 12 ++++++++++++ include/linux/arm_sdei.h | 6 ++++++ include/uapi/linux/arm_sdei.h | 1 + 3 files changed, 19 insertions(+)
drivers/firmware/arm_sdei.c +12 -0
@@ -339,6 +339,18 @@ static void _ipi_unmask_cpu(void *ignored)sdei_unmask_local_cpu (); } +/* +* Signal the software-signalled event (event 0) to @mpidr. Does nothing +* but the SMC -- no locks, no event lookup -- so it is safe from NMI / +* crash context (e.g. the cross-CPU NMI service). +*/ +int sdei_event_signal (u32 event_num,u64 mpidr) +{ +return invoke_sdei_fn (SDEI_1_0_FN_SDEI_EVENT_SIGNAL , event_num, + mpidr,0 ,0 ,0 ,NULL ); +} +NOKPROBE_SYMBOL (sdei_event_signal); +/* * Was SDEI firmware probed and is it usable? Lets optional consumers skip * registering an event -- and the warning a failed registration emits -- on
include/linux/arm_sdei.h +6 -0
@@ -37,6 +37,12 @@ int sdei_event_unregister(u32 event_num);int sdei_event_enable (u32 event_num);int sdei_event_disable (u32 event_num); +/* +* Signal the software-signalled event (event 0) to another PE, NMI-like. +* @mpidr is the target's MPIDR affinity. +*/ +int sdei_event_signal (u32 event_num,u64 mpidr); +/* Was SDEI firmware probed and usable? */ bool sdei_is_present (void );
include/uapi/linux/arm_sdei.h +1 -0
@@ -22,6 +22,7 @@#define SDEI_1_0_FN_SDEI_PE_UNMASK SDEI_1_0_FN(0x0C)#define SDEI_1_0_FN_SDEI_INTERRUPT_BIND SDEI_1_0_FN(0x0D)#define SDEI_1_0_FN_SDEI_INTERRUPT_RELEASE SDEI_1_0_FN(0x0E) +#define SDEI_1_0_FN_SDEI_EVENT_SIGNAL SDEI_1_0_FN(0x0F)#define SDEI_1_0_FN_SDEI_PRIVATE_RESET SDEI_1_0_FN(0x11)#define SDEI_1_0_FN_SDEI_SHARED_RESET SDEI_1_0_FN(0x12)
Signature · 1 line
2.54.0
PATCH v4 3/4 drivers/firmware: add SDEI cross-CPU NMI service for arm64
In reply to Kiryl ShutsemauFrom: "Kiryl Shutsemau (Meta)" <[email protected]>
Deliver an NMI-like event to an interrupt-masked arm64 CPU via the standard SDEI software-signalled event (event 0), without the pseudo-NMI hot-path cost: register a handler for event 0 and poke a target with sdei_event_signal(0, mpidr).
First user is arch_trigger_cpumask_backtrace() (sysrq-l, RCU stalls, hung-task/soft-lockup dumps), which otherwise rides an IPI that can't reach a masked CPU. Falls back to the IPI path when SDEI is absent; no watchdog backend yet, so the stock detector is untouched.
--- MAINTAINERS | 2 +- arch/arm64/include/asm/nmi.h | 24 +++++ arch/arm64/kernel/smp.c | 11 +++ drivers/firmware/Kconfig | 19 ++++ drivers/firmware/Makefile | 1 + drivers/firmware/arm_sdei_nmi.c | 152 ++++++++++++++++++++++++++++++++ 6 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 arch/arm64/include/asm/nmi.h create mode 100644 drivers/firmware/arm_sdei_nmi.c
MAINTAINERS +1 -1
@@ -24797,7 +24797,7 @@ M: James Morse <[email protected]> L: [email protected] (moderated for non-subscribers) S: Maintained F: Documentation/devicetree/bindings/arm/firmware/sdei.txt -F: drivers/firmware/arm_sdei.c +F: drivers/firmware/arm_sdei* F: include/linux/arm_sdei.h F: include/uapi/linux/arm_sdei.h
arch/arm64/include/asm/nmi.h new file +24 -0
@@ -0,0 +1,24 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_NMI_H +#define __ASM_NMI_H + +#include <linux/cpumask.h> + +/* +* Cross-CPU NMI provider hooks, consulted by the arm64 arch code before +* its regular-IRQ / pseudo-NMI IPI paths. The SDEI provider in +* drivers/firmware/arm_sdei_nmi.c implements them when active; a future +* FEAT_NMI provider could slot in here too. The stubs let callers stay +* unconditional when ARM_SDEI_NMI is off. +*/ +#ifdef CONFIG_ARM_SDEI_NMI +bool sdei_nmi_trigger_cpumask_backtrace (const cpumask_t *mask,int exclude_cpu); +#else +static inline bool sdei_nmi_trigger_cpumask_backtrace (const cpumask_t *mask, +int exclude_cpu) +{ +return false; +} +#endif + +#endif /* __ASM_NMI_H */
arch/arm64/kernel/smp.c +11 -0
@@ -45,6 +45,7 @@#include <asm/daifflags.h> #include <asm/kvm_mmu.h> #include <asm/mmu_context.h> +#include <asm/nmi.h> #include <asm/numa.h> #include <asm/processor.h> #include <asm/smp_plat.h>
@@ -927,6 +928,16 @@ static void arm64_backtrace_ipi(cpumask_t *mask)void arch_trigger_cpumask_backtrace (const cpumask_t *mask,int exclude_cpu) { +/* +* Prefer the SDEI cross-CPU NMI provider when active: firmware +* dispatches the event out of EL3 and reaches CPUs that have +* interrupts locally masked, without the per-IRQ-mask cost that +* pseudo-NMI pays for the same reach. The plain IPI path below +* can't reach such a CPU unless pseudo-NMI is enabled. +*/ +if (sdei_nmi_trigger_cpumask_backtrace (mask, exclude_cpu)) +return ; +/* * NOTE: though nmi_trigger_cpumask_backtrace() has "nmi_" in the name, * nothing about it truly needs to be implemented using an NMI, it's
drivers/firmware/Kconfig +19 -0
@@ -36,6 +36,25 @@ config ARM_SDE_INTERFACE standard for registering callbacks from the platform firmware into the OS. This is typically used to implement RAS notifications. +config ARM_SDEI_NMI + bool "SDEI-based cross-CPU NMI service (arm64)" + depends on ARM64 && ARM_SDE_INTERFACE + help + Provides SDEI-based cross-CPU NMI delivery for hooks that need + to reach interrupt-masked CPUs on silicon that lacks FEAT_NMI: + + - arch_trigger_cpumask_backtrace() (sysrq-l, RCU stalls, + hardlockup_all_cpu_backtrace, soft-lockup secondary dumps, + hung-task auxiliary dumps) + + The driver registers a handler for the SDEI software-signalled + event (event 0) and reaches a target CPU by signalling it with + SDEI_EVENT_SIGNAL. Firmware delivers the event out of EL3 + regardless of the target's PSTATE.DAIF -- forced delivery into a + CPU wedged with interrupts locally masked. + + If unsure, say N. + config EDD tristate "BIOS Enhanced Disk Drive calls determine boot disk" depends on X86
drivers/firmware/Makefile +1 -0
@@ -4,6 +4,7 @@ # obj-$(CONFIG_ARM_SCPI_PROTOCOL) += arm_scpi.o obj-$(CONFIG_ARM_SDE_INTERFACE) += arm_sdei.o +obj-$(CONFIG_ARM_SDEI_NMI) += arm_sdei_nmi.o obj-$(CONFIG_DMI) += dmi_scan.o obj-$(CONFIG_DMI_SYSFS) += dmi-sysfs.o obj-$(CONFIG_EDD) += edd.o
drivers/firmware/arm_sdei_nmi.c new file +152 -0
152 lines not on this page · raw message
Signature · 1 line
2.54.0
PATCH v4 4/4 arm64: escalate smp_send_stop() to an SDEI NMI as a last resort
In reply to Kiryl ShutsemauFrom: "Kiryl Shutsemau (Meta)" <[email protected]>
A CPU wedged with interrupts masked ignores the stop IPI, and without pseudo-NMI there is no NMI IPI to escalate to: a reboot proceeds with the CPU still running, and a kdump misses its registers.
Add a third rung to smp_send_stop(): once the IPI (and pseudo-NMI IPI, if enabled) rungs have run, signal SDEI event 0 at whatever stayed online. Firmware delivers it regardless of the target's DAIF, so it reaches a CPU a plain IPI cannot; the target acks by going offline, which the caller already polls for.
Fold the stop bookkeeping into one arm64_nmi_cpu_stop(regs, die_on_crash), shared by the stop IPI handlers, panic_smp_self_stop() and the SDEI handler, replacing the near-duplicate local_cpu_stop() and ipi_cpu_crash_stop(). @die_on_crash is the only difference: the IPI handlers pass true and PSCI CPU_OFF the CPU on a crash stop so a capture kernel can reclaim it; the SDEI handler and self-stop pass false and park. The SDEI park is required, not conservative -- its handler runs inside an SDEI event that is never completed (completing it resumes the wedged context), and a CPU_OFF from that unfinished-event context wedges EL3 on some firmware (left as a follow-up). The dump is unaffected; only re-onlining the CPU in an SMP capture kernel is lost.
--- arch/arm64/include/asm/nmi.h | 24 +++++++ arch/arm64/kernel/smp.c | 113 +++++++++++++++++++++----------- drivers/firmware/Kconfig | 2 + drivers/firmware/arm_sdei_nmi.c | 86 ++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 38 deletions(-)
arch/arm64/include/asm/nmi.h +24 -0
@@ -4,21 +4,45 @@#include <linux/cpumask.h> +struct pt_regs ; +/* * Cross-CPU NMI provider hooks, consulted by the arm64 arch code before * its regular-IRQ / pseudo-NMI IPI paths. The SDEI provider in * drivers/firmware/arm_sdei_nmi.c implements them when active; a future * FEAT_NMI provider could slot in here too. The stubs let callers stay * unconditional when ARM_SDEI_NMI is off. +* +* sdei_nmi_active() lets a caller test for the service before committing +* to (and waiting on) the SDEI stop rung; sdei_nmi_stop_cpus() then signals +* the targets, which ack by going offline. */ #ifdef CONFIG_ARM_SDEI_NMI bool sdei_nmi_trigger_cpumask_backtrace (const cpumask_t *mask,int exclude_cpu); +bool sdei_nmi_active (void ); +void sdei_nmi_stop_cpus (const cpumask_t *mask);#else static inline bool sdei_nmi_trigger_cpumask_backtrace (const cpumask_t *mask,int exclude_cpu) {return false; } + +static inline bool sdei_nmi_active (void ) +{ +return false; +} + +static inline void sdei_nmi_stop_cpus (const cpumask_t *mask) { }#endif +/* +* The common "stop this CPU" entry every arm64 stop path funnels through: +* the regular/pseudo-NMI stop IPI handlers, panic_smp_self_stop(), and the +* SDEI cross-CPU NMI handler. @die_on_crash powers the CPU off on the kdump +* crash path (IPI handlers) instead of parking it (SDEI / self-stop). +* Defined in arch/arm64/kernel/smp.c. +*/ +void __noreturn arm64_nmi_cpu_stop (struct pt_regs *regs,bool die_on_crash); +#endif /* __ASM_NMI_H */
arch/arm64/kernel/smp.c +75 -38
149 lines not on this page · raw message
drivers/firmware/Kconfig +2 -0
@@ -46,6 +46,8 @@ config ARM_SDEI_NMI - arch_trigger_cpumask_backtrace() (sysrq-l, RCU stalls, hardlockup_all_cpu_backtrace, soft-lockup secondary dumps, hung-task auxiliary dumps) + - smp_send_stop() escalation (reboot/halt and the + panic / kdump crash stop) The driver registers a handler for the SDEI software-signalled event (event 0) and reaches a target CPU by signalling it with
drivers/firmware/arm_sdei_nmi.c +86 -0
106 lines not on this page · raw message
Signature · 1 line
2.54.0
Re: [PATCH v4 4/4] arm64: escalate smp_send_stop() to an SDEI NMI as a last resort
Hi,
Quoted 25 lines · Kiryl Shutsemau
From: "Kiryl Shutsemau (Meta)" <[email protected]>
A CPU wedged with interrupts masked ignores the stop IPI, and without pseudo-NMI there is no NMI IPI to escalate to: a reboot proceeds with the CPU still running, and a kdump misses its registers.
Add a third rung to
smp_send_stop(): once the IPI (and pseudo-NMI IPI, if enabled) rungs have run, signal SDEI event 0 at whatever stayed online. Firmware delivers it regardless of the target's DAIF, so it reaches a CPU a plain IPI cannot; the target acks by going offline, which the caller already polls for.Fold the stop bookkeeping into one
arm64_nmi_cpu_stop(regs, die_on_crash), shared by the stop IPI handlers,panic_smp_self_stop()and the SDEI handler, replacing the near-duplicatelocal_cpu_stop()andipi_cpu_crash_stop(). @die_on_crash is the only difference: the IPI handlers pass true and PSCICPU_OFFthe CPU on a crash stop so a capture kernel can reclaim it; the SDEI handler and self-stop pass false and park. The SDEI park is required, not conservative -- its handler runs inside an SDEI event that is never completed (completing it resumes the wedged context), and aCPU_OFFfrom that unfinished-event context wedges EL3 on some firmware (left as a follow-up). The dump is unaffected; only re-onlining the CPU in an SMP capture kernel is lost.
Quoted 8 lines
Suggested-by Douglas Anderson <[email protected]>Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>--- arch/arm64/include/asm/nmi.h | 24 +++++++ arch/arm64/kernel/smp.c | 113 +++++++++++++++++++++----------- drivers/firmware/Kconfig | 2 + drivers/firmware/arm_sdei_nmi.c | 86 ++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 38 deletions(-)
Re: [PATCH v4 1/4] firmware: arm_sdei: add sdei_is_present()
In reply to Kiryl ShutsemauHi,
Quoted 9 lines · Kiryl Shutsemau
From: "Kiryl Shutsemau (Meta)" <[email protected]>
invoke_sdei_fn()returns-EIOwhen no SDEI conduit was probed, and the core warns ("Failed to create event ...") on any registration that hits that. An optional consumer that registers an event from an unconditional initcall would therefore make every boot on a non-SDEI system emit that warning for what is simply absent firmware.
Quoted 8 lines
Expose whether SDEI firmware is present so such a consumer can skip registration -- and the warning -- when there is nothing to talk to.
Signed-off-by Kiryl Shutsemau (Meta) <[email protected]>--- drivers/firmware/arm_sdei.c | 10 ++++++++++ include/linux/arm_sdei.h | 3 +++ 2 files changed, 13 insertions(+)
From a non-expert in SDEI, but FWIW:
Re: [PATCH v4 3/4] drivers/firmware: add SDEI cross-CPU NMI service for arm64
In reply to Kiryl ShutsemauHi Kiryl,
drivers/firmware/Kconfig config ARM_SDE_INTERFACE · line 41
Quoted 3 lines · Kiryl Shutsemau
drivers/firmware/Kconfig +3
-0+config ARM_SDEI_NMI + bool "SDEI-based cross-CPU NMI service (arm64)" + depends on ARM64 && ARM_SDE_INTERFACE
The dependency on ARM64 is redundant here since you already have the dependency on ARM_SDE_INTERFACE. Maybe a comment instead, though I think it's pretty clear from the prompt...
- Julian Braha
Re: [PATCH v4 3/4] drivers/firmware: add SDEI cross-CPU NMI service for arm64
Quoted 9 lines · Julian Braha
Hi Kiryl,
drivers/firmware/Kconfig config ARM_SDE_INTERFACE · line 41
Quoted 3 lines · Kiryl Shutsemau
drivers/firmware/Kconfig +3
-0+config ARM_SDEI_NMI + bool "SDEI-based cross-CPU NMI service (arm64)" + depends on ARM64 && ARM_SDE_INTERFACEThe dependency on ARM64 is redundant here since you already have the dependency on
ARM_SDE_INTERFACE.
Fair. Will remove ARM64 in the next version.
Quoted 2 lines
Maybe a comment instead, though I think
it's pretty clear from the prompt...
I don't think the comment would contribute much value.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Catalin Marinas
maintainerHi Kiryl,
Quoted 8 lines · Kiryl Shutsemau
- GICv3 pseudo-NMI (interrupt priority masking). Its cost is on the interrupt mask/unmask hot path:
local_irq_enable()becomes anICC_PMR_EL1write plus a synchronising barrier, and exception entry/exit save and restore the PMR, paid on every CPU whether or not an NMI is ever delivered. In our measurements, enabling pseudo-NMI costs up to ~5% on real workloads, and ~66% on a syscall-in-a-loop microbenchmark. A fleet-wide ~5% regression is not acceptable, so these systems run with pseudo-NMI disabled.
Does your firmware set ICC_CTLR_EL1.PMHE? I'd be curious to see the numbers if the DSB was omitted on the enable path.
Quoted 5 lines
This series adds a third delivery backend that costs nothing on the hot
path: SDEI. Firmware delivers an SDEI event into a CPU regardless of its DAIF state, so interrupt masking stays the cheap PSTATE.DAIF operation and the firmware round-trip is paid only at the rare moment a CPU must be interrupted.
The direction of travel is to deprecated SDEI. I wouldn't add more stuff on top of this interface.
(I haven't looked at the patches yet; Marc/Mark/James are more knowledgeable than me in this area)
Signature · 1 line
Catalin
Quoted 13 lines · Catalin Marinas
Hi Kiryl,
Quoted 8 lines · Kiryl Shutsemau
- GICv3 pseudo-NMI (interrupt priority masking). Its cost is on the interrupt mask/unmask hot path:
local_irq_enable()becomes anICC_PMR_EL1write plus a synchronising barrier, and exception entry/exit save and restore the PMR, paid on every CPU whether or not an NMI is ever delivered. In our measurements, enabling pseudo-NMI costs up to ~5% on real workloads, and ~66% on a syscall-in-a-loop microbenchmark. A fleet-wide ~5% regression is not acceptable, so these systems run with pseudo-NMI disabled.
Quoted 2 lines
Does your firmware set ICC_CTLR_EL1.PMHE? I'd be curious to see the numbers if the DSB was omitted on the enable path.
I certainly don't observe this sort of overhead on the HW I have access to, and would like to understand where this is coming from with actual profiling data.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Quoted 6 lines · Marc Zyngier
Quoted 2 lines
Does your firmware set ICC_CTLR_EL1.PMHE? I'd be curious to see the numbers if the DSB was omitted on the enable path.
I certainly don't observe this sort of overhead on the HW I have access to, and would like to understand where this is coming from with actual profiling data.
Full disclosure: the ~66% figures come from internal testing about a year ago. I no longer have the details of the machine it ran on and can't confirm whether ICC_CTLR_EL1.PMHE was set there -- it may well have been. I shouldn't have carried those numbers forward without being able to stand behind them, so please disregard them.
Here are fresh numbers from NVIDIA Grace (Neoverse V2). Importantly, this box reports:
GICv3: Pseudo-NMIs enabled using relaxed ICC_PMR_EL1 synchronisation
i.e. PMHE == 0, so the synchronising DSB on the unmask path is already patched to a NOP (ARM64_HAS_GIC_PRIO_RELAXED_SYNC). What's left is the floor cost of PMR-based masking itself plus the PMR save/restore on exception entry/exit -- not the DSB. So this is the case Catalin asked about (DSB omitted), and there is still a measurable cost.
A trivial single-threaded gettid() loop (1e6 calls, median of 5, performance governor, ASLR off):
pseudo_nmi=0 (DAIF): 178.4 ns/call
pseudo_nmi=1 (PMR): 252.5 ns/call
delta: +74.1 ns/call (~230-250 cycles)
+41.5% wall time / 0.706 throughput --- u-bench.c ---
#include <unistd.h>
#include <sys/syscall.h>
#include <time.h>
#include <stdio.h>
int main(void) {
struct timespec a, b;
clock_gettime(CLOCK_MONOTONIC, &a);
for (long i = 0; i < 1000000; i++)
syscall(SYS_gettid);
clock_gettime(CLOCK_MONOTONIC, &b);
printf("%f ns\n", (b.tv_sec-a.tv_sec)*1e9 + (b.tv_nsec-a.tv_nsec));
return 0;
}will-it-scale agrees independently. sched_yield (ops/s, median of 5):
1 task 72 tasks
pseudo_nmi=0 3,195,656 230,824,534 pseudo_nmi=1 2,253,753 163,914,837
ratio 0.705 0.710
The ratio is flat across the whole 1-to-72 sweep, so -- relevant to the scalability question -- it's a constant per-syscall tax, not a contention effect. The impact tracks syscall/exception density: page_fault1, a more realistic workload, stays within ~5%.
Quoted 2 lines
The direction of travel is to deprecate SDEI. I wouldn't add more stuff on top of this interface.
I understand FEAT_NMI is the long-term answer, and I'm not arguing against deprecating SDEI. My concern is the gap in between. By our estimate it's 10+ years before the last non-FEAT_NMI machine retires from the fleet -- for scale, we're still running Skylake today. So there's roughly a decade where a large installed base has neither FEAT_NMI nor affordable pseudo-NMI, and no way to reach a DAIF-masked CPU for an all-CPU backtrace or to capture a wedged CPU in a crash dump. That's the functional gap this series tries to cover.
Given the deprecation direction, I deliberately kept the SDEI footprint as small as I could. The series adds no new firmware interface and no vendor SMC -- it uses only the standard software-signalled event (event 0) via SDEI_EVENT_SIGNAL, which is already present on these systems for firmware-first RAS (APEI/GHES). And SDEI is only ever invoked in a "bad state": to deliver a backtrace signal to a CPU that a normal IPI can't reach, or to stop a CPU that ignored the stop IPIs. Nothing on any hot or steady-state path touches it.
If even that minimal use is unacceptable on a deprecated interface, I'd rather know now and redirect the effort -- but I'd appreciate a pointer to what should cover this gap for existing silicon in the meantime.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Hi,
Quoted 5 lines · Kiryl Shutsemau
The ratio is flat across the whole 1-to-72 sweep, so -- relevant to the scalability question -- it's a constant per-syscall tax, not a contention effect. The impact tracks syscall/exception density:
page_fault1, a more realistic workload, stays within ~5%.
FWIW, I'm not sure 5% is truly a "realistic" measure of the overhead here. While it cannot be denied that pseudo-NMI has some overhead, every time I've seen someone study it in "real world" scenarios it ends up being fairly negligible. Not zero, mind you. ...but some amount below 1%. ...and by this, I mean put real-world workloads on a system with pseudo NMI enabled and pick your favorite metric. Perhaps measure the amount of work completed in a given time. I dunno if this is possible for you to do in this case.
Quoted 21 lines
Quoted 2 lines
The direction of travel is to deprecate SDEI. I wouldn't add more stuff on top of this interface.
I understand
FEAT_NMIis the long-term answer, and I'm not arguing against deprecating SDEI. My concern is the gap in between. By our estimate it's 10+ years before the last non-FEAT_NMI machine retires from the fleet -- for scale, we're still running Skylake today. So there's roughly a decade where a large installed base has neitherFEAT_NMInor affordable pseudo-NMI, and no way to reach a DAIF-masked CPU for an all-CPU backtrace or to capture a wedged CPU in a crash dump. That's the functional gap this series tries to cover.Given the deprecation direction, I deliberately kept the SDEI footprint as small as I could. The series adds no new firmware interface and no vendor SMC -- it uses only the standard software-signalled event (event 0) via
SDEI_EVENT_SIGNAL, which is already present on these systems for firmware-first RAS (APEI/GHES). And SDEI is only ever invoked in a "bad state": to deliver a backtrace signal to a CPU that a normal IPI can't reach, or to stop a CPU that ignored the stop IPIs. Nothing on any hot or steady-state path touches it.
Quoted 3 lines
If even that minimal use is unacceptable on a deprecated interface, I'd rather know now and redirect the effort -- but I'd appreciate a pointer to what should cover this gap for existing silicon in the meantime.
FWIW, despite the fact that pseudo-NMI (in my experience) ends up being mostly negligible, personally I'd be in favor of landing Kiryl's patches. I dunno how many times I've had the discussion of pseudo-NMI overhead over the years and it's certainly very easy to show that there is _some_ overhead and that in pathological cases there is a lot of overhead. As Kiryl says, the patches don't add a ton of extra complexity and they even combine some of the stop/crash-stop code, which is nice. Having them as a stop-gap until true NMI is available seems nice to me. ...of course, I'm not an ARM maintainer, so it's obviously not up to me. :-)
-Doug
Hi Kirill
Quoted 25 lines · Kiryl Shutsemau
Quoted 6 lines · Marc Zyngier
Quoted 2 lines
Does your firmware set ICC_CTLR_EL1.PMHE? I'd be curious to see the numbers if the DSB was omitted on the enable path.
I certainly don't observe this sort of overhead on the HW I have access to, and would like to understand where this is coming from with actual profiling data.
Full disclosure: the ~66% figures come from internal testing about a year ago. I no longer have the details of the machine it ran on and can't confirm whether ICC_CTLR_EL1.PMHE was set there -- it may well have been. I shouldn't have carried those numbers forward without being able to stand behind them, so please disregard them.
Here are fresh numbers from NVIDIA Grace (Neoverse V2). Importantly, this box reports:
GICv3: Pseudo-NMIs enabled using relaxed
ICC_PMR_EL1synchronisationi.e. PMHE == 0, so the synchronising DSB on the unmask path is already patched to a NOP (
ARM64_HAS_GIC_PRIO_RELAXED_SYNC). What's left is the floor cost of PMR-based masking itself plus the PMR save/restore on exception entry/exit -- not the DSB. So this is the case Catalin asked about (DSB omitted), and there is still a measurable cost.
Quoted 7 lines
A trivial single-threaded
gettid()loop (1e6 calls, median of 5, performance governor, ASLR off):pseudo_nmi=0 (DAIF): 178.4 ns/call pseudo_nmi=1 (PMR): 252.5 ns/call delta: +74.1 ns/call (~230-250 cycles) +41.5% wall time / 0.706 throughput
I tested the u-bench.c on a Neoverse N2 based arm64 server. The result is as following:
pseudo_nmi=0 (DAIF): 96.3 ns/call pseudo_nmi=1 (PMR): 169.8 ns/call delta: +73.5 ns/call
Quoted 49 lines
--- u-bench.c --- #include <unistd.h> #include <sys/syscall.h> #include <time.h> #include <stdio.h> int main(void) { struct timespec a, b; clock_gettime(CLOCK_MONOTONIC, &a); for (long i = 0; i < 1000000; i++) syscall(SYS_gettid); clock_gettime(CLOCK_MONOTONIC, &b); printf("%f ns\n", (b.tv_sec-a.tv_sec)*1e9 + (b.tv_nsec-a.tv_nsec)); return 0; }will-it-scale agrees independently.
sched_yield(ops/s, median of 5):1 task 72 tasks
pseudo_nmi=0 3,195,656 230,824,534 pseudo_nmi=1 2,253,753 163,914,837ratio 0.705 0.710
The ratio is flat across the whole 1-to-72 sweep, so -- relevant to the scalability question -- it's a constant per-syscall tax, not a contention effect. The impact tracks syscall/exception density:
page_fault1, a more realistic workload, stays within ~5%.Quoted 2 lines
The direction of travel is to deprecate SDEI. I wouldn't add more stuff on top of this interface.
I understand
FEAT_NMIis the long-term answer, and I'm not arguing against deprecating SDEI. My concern is the gap in between. By our estimate it's 10+ years before the last non-FEAT_NMI machine retires from the fleet -- for scale, we're still running Skylake today. So there's roughly a decade where a large installed base has neitherFEAT_NMInor affordable pseudo-NMI, and no way to reach a DAIF-masked CPU for an all-CPU backtrace or to capture a wedged CPU in a crash dump. That's the functional gap this series tries to cover.Given the deprecation direction, I deliberately kept the SDEI footprint as small as I could. The series adds no new firmware interface and no vendor SMC -- it uses only the standard software-signalled event (event 0) via
SDEI_EVENT_SIGNAL, which is already present on these systems for firmware-first RAS (APEI/GHES). And SDEI is only ever invoked in a "bad state": to deliver a backtrace signal to a CPU that a normal IPI can't reach, or to stop a CPU that ignored the stop IPIs. Nothing on any hot or steady-state path touches it.
Quoted 3 lines
If even that minimal use is unacceptable on a deprecated interface, I'd rather know now and redirect the effort -- but I'd appreciate a pointer to what should cover this gap for existing silicon in the meantime.
I couldn't agree more: We need a solution for existing system. And like to see this patchset merged. Thanks.
Regards
Yin, Fengwei
Quoted 4 lines
Signature · 2 lines
-- Kiryl Shutsemau / Kirill A. Shutemov
Quoted 2 lines · Doug Anderson
Having them as a stop-gap until true NMI is available seems nice to me
I completely agree. Should these patches unfortunately not make it upstream, my plan is to maintain them downstream at Meta kernels until the Meta fleet no longer contains any non-FEAT_NMI hosts.
The trade-offs under consideration are:
1) Accept the performance overhead from pseudo-NMI
2) Lose debuggability by disabling NMI entirely
3) Maintain downstream patches
Of these three options, carrying downstream patches appears to be the least disruptive path for our business needs.
Catalin Marinas
maintainerQuoted 10 lines · Kiryl Shutsemau
- A CPU stopped by the SDEI rung is parked, not powered off via PSCI
CPU_OFF. Reaching and dumping the wedged CPU -- the point of the series -- works, and it matches the shared stop path's own park fallback whenCPU_OFFis unavailable. The consequence is that an SMP crash-capture kernel cannot re-online such a CPU (it stays "already on"); the capture kernel boots and runs on the remaining CPUs. Powering the stopped CPU off so a capture kernel can reclaim it requires completing the SDEI event and thenCPU_OFF, which hit a firmware-specific issue still under investigation; it is left as a follow-up and does not affect the dump's contents.
Just to understand, your firmware cannot cope with a PSCI CPU_OFF from the SDEI handler? This is one of the required calls to be supported.
Signature · 1 line
Catalin
Catalin Marinas
maintainerRe: [PATCH v4 4/4] arm64: escalate smp_send_stop() to an SDEI NMI as a last resort
In reply to Kiryl ShutsemauQuoted 7 lines · Kiryl Shutsemau
drivers/firmware/arm_sdei_nmi.c +7
-0+void sdei_nmi_stop_cpus (const cpumask_t *mask) +{ +unsigned int cpu; + +WRITE_ONCE (sdei_nmi_stopping, true); + +/*
drivers/firmware/arm_sdei_nmi.c bool sdei_nmi_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu) · line 202
Quoted 10 lines
drivers/firmware/arm_sdei_nmi.c +10
-0+* Publish the flag before signalling. The SMC is a context-sync +* event, not a barrier, so WRITE_ONCE() alone could let the store be +* observed after the event it triggers. The barrier is cumulative: a +* target that sees the event is guaranteed to see the flag. +*/ +smp_wmb (); + +for_each_cpu (cpu, mask) +sdei_nmi_fire (cpu); +}
The smp_wmb() is not sufficient here. In the GIC IPI code we use a dsb(ishst). It should be similar here. I think TF-A does this already but it's unclear from the SDEI spec that it is mandated.
Signature · 1 line
Catalin
Catalin Marinas
maintainerRe: [PATCH v4 3/4] drivers/firmware: add SDEI cross-CPU NMI service for arm64
In reply to Kiryl Shutsemaudrivers/firmware/arm_sdei_nmi.c line 94
Quoted 12 lines · Kiryl Shutsemau
drivers/firmware/arm_sdei_nmi.c new file +12
-0+/* +* Raise callback for nmi_trigger_cpumask_backtrace(): signal event 0 +* at every CPU still pending in @mask. The framework excludes the local +* CPU from @mask before calling us. +*/ +static void sdei_nmi_raise_backtrace (cpumask_t *mask) +{ +unsigned int cpu; + +for_each_cpu (cpu, mask) +sdei_nmi_fire (cpu); +}
Does this need a barrier as in patch 4? Sashiko spotted that backtrace_mask won't be visible without a dsb(ishst). Again, as per my comment in patch 4, we get away with this because of the TF-A implementation but that's not guaranteed (we could ask for the SDEI spec to be updated assuming all firmware implementations do this).
Signature · 1 line
Catalin
Quoted 12 lines · Catalin Marinas
Quoted 10 lines · Kiryl Shutsemau
- A CPU stopped by the SDEI rung is parked, not powered off via PSCI
CPU_OFF. Reaching and dumping the wedged CPU -- the point of the series -- works, and it matches the shared stop path's own park fallback whenCPU_OFFis unavailable. The consequence is that an SMP crash-capture kernel cannot re-online such a CPU (it stays "already on"); the capture kernel boots and runs on the remaining CPUs. Powering the stopped CPU off so a capture kernel can reclaim it requires completing the SDEI event and thenCPU_OFF, which hit a firmware-specific issue still under investigation; it is left as a follow-up and does not affect the dump's contents.
Quoted 2 lines
Just to understand, your firmware cannot cope with a PSCI
CPU_OFFfrom the SDEI handler? This is one of the required calls to be supported.
I did chase it a fair bit. Bisecting on Grace: completing the event and parking (no CPU_OFF) works, and so does the stack-switch + C-call setup on its own. The hang only appears once I call PSCI CPU_OFF after the event -- and it persists even with DAIF masked and the GIC PMR reset first, so it isn't leftover interrupt/priority state from the dispatch. It's a silent wedge: no TF-A exception report, nothing after the last console line.
But I have not tried calling CPU_OFF directly, without completing the event. I assumed it is required. Will give it a try when I have time.
Either way this is a side quest: it only lets a crash kernel reclaim the stopped CPU. The dump itself is complete, so it's nice-to-have, not required for the series.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v4 4/4] arm64: escalate smp_send_stop() to an SDEI NMI as a last resort
In reply to Catalin MarinasQuoted 14 lines · Catalin Marinas
Quoted 13 lines · Kiryl Shutsemau
drivers/firmware/arm_sdei_nmi.c +13
-0+void sdei_nmi_stop_cpus (const cpumask_t *mask) +{ +unsigned int cpu; + +WRITE_ONCE (sdei_nmi_stopping, true); + +/* +* Publish the flag before signalling. The SMC is a context-sync +* event, not a barrier, so WRITE_ONCE() alone could let the store be +* observed after the event it triggers. The barrier is cumulative: a +* target that sees the event is guaranteed to see the flag. +*/ +smp_wmb ();
Quoted 8 lines
drivers/firmware/arm_sdei_nmi.c bool sdei_nmi_trigger_cpumask_backtrace(const cpumask_t *mask, int exclude_cpu) · line 202
Quoted 4 lines
drivers/firmware/arm_sdei_nmi.c +4
-0+ +for_each_cpu (cpu, mask) +sdei_nmi_fire (cpu); +}The
smp_wmb()is not sufficient here. In the GIC IPI code we use a dsb(ishst). It should be similar here. I think TF-A does this already but it's unclear from the SDEI spec that it is mandated.
You're right, I'll switch it to dsb(ishst) to match gic_ipi_send_mask().
And as you pointed, the xame fix is needed in 3/4: the backtrace path publishes backtrace_mask before signalling and currently has no barrier at all (as the bot spotted), so it needs the dsb(ishst) there too.
I'll also drop the now-incorrect "cumulative wmb" comment.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Quoted 2 lines · Kiryl Shutsemau
But I have not tried calling
CPU_OFFdirectly, without completing the event. I assumed it is required. Will give it a try when I have time.
Tried it now, and it doesn't work either -- in a more interesting way.
Calling PSCI CPU_OFF directly from the SDEI handler (event left uncompleted) reproducibly breaks the kdump capture kernel, and this reproduces under QEMU's TF-A, not just on Grace -- so it isn't a Grace firmware quirk.
The test: a CPU wedged with interrupts masked is stopped via the SDEI rung; its handler calls __cpu_try_die() instead of parking. A/B in QEMU, changing only that wedged CPU's handling (everything else identical):
- park it (current series): capture kernel boots fully to a shell.
- CPU_OFF from the handler: capture kernel hangs in early boot, around SDEI re-init, never reaches a shell.
Powering the PE off while its SDEI event is still active leaves EL3's dispatch state dangling, and the capture kernel trips over it. Completing the event first and then CPU_OFF -- what I tried originally -- silently wedges EL3 on Grace instead.
So both routes off fail, and the CPU stays parked. The dump is complete either way; only re-onlining the stopped CPU in an SMP capture kernel is lost. It's a cheap QEMU repro now if anyone wants to dig into the EL3 side.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Catalin Marinas
maintainerQuoted 10 lines · Kiryl Shutsemau
Quoted 2 lines · Kiryl Shutsemau
But I have not tried calling
CPU_OFFdirectly, without completing the event. I assumed it is required. Will give it a try when I have time.Tried it now, and it doesn't work either -- in a more interesting way.
Calling PSCI
CPU_OFFdirectly from the SDEI handler (event left uncompleted) reproducibly breaks the kdump capture kernel, and this reproduces under QEMU's TF-A, not just on Grace -- so it isn't a Grace firmware quirk.
I had a quick grep (with the help of claude) through the TF-A code and it doesn't seem to be compliant with the spec. It should subscribe to the PSCI CPU_OFF event and complete the SDEI but it doesn't. It seems to handle CPU_ON but that may not be sufficient. It only EIOs the SGI once the OS completed the event, which doesn't happen if you issue CPU_OFF.
Quoted 13 lines
The test: a CPU wedged with interrupts masked is stopped via the SDEI rung; its handler calls
__cpu_try_die()instead of parking. A/B in QEMU, changing only that wedged CPU's handling (everything else identical):- park it (current series): capture kernel boots fully to a shell.
-CPU_OFFfrom the handler: capture kernel hangs in early boot, around SDEI re-init, never reaches a shell.Powering the PE off while its SDEI event is still active leaves EL3's dispatch state dangling, and the capture kernel trips over it. Completing the event first and then
CPU_OFF-- what I tried originally -- silently wedges EL3 on Grace instead.
Quoted 4 lines
So both routes off fail, and the CPU stays parked. The dump is complete either way; only re-onlining the stopped CPU in an SMP capture kernel is lost. It's a cheap QEMU repro now if anyone wants to dig into the EL3 side.
Have you tried SDEI_EVENT_COMPLETE_AND_RESUME instead? Just COMPLETE won't return to the kernel. We have sdei_handler_abort() to complete the event and, hopefully, you can continue with the CPU_OFF. It's a work around the TF-A non-compliance but I think this is useful even if you don't issue the CPU_OFF (e.g. no CPU hotplug, just the park loop).
Signature · 1 line
Catalin
Quoted 5 lines · Catalin Marinas
Have you tried
SDEI_EVENT_COMPLETE_AND_RESUMEinstead? Just COMPLETE won't return to the kernel. We havesdei_handler_abort()to complete the event and, hopefully, you can continue with theCPU_OFF. It's a work around the TF-A non-compliance but I think this is useful even if you don't issue theCPU_OFF(e.g. no CPU hotplug, just the park loop).
Tried it. The result is the opposite of what I expected, and it argues against doing the complete at all under QEMU's TF-A.
Same A/B harness as before -- a CPU wedged with interrupts masked is stopped via the SDEI rung; I only vary how its handler ends. kdump capture kernel boot under QEMU:
- park, event left uncompleted (current series): boots to a shell - sdei_handler_abort() then CPU_OFF: hangs at SDEI re-init - sdei_handler_abort() then park (no CPU_OFF): hangs at SDEI re-init - CPU_OFF, event left uncompleted: hangs at SDEI re-init
The third line is the surprising one: completing the event via sdei_handler_abort() and then just parking -- no CPU_OFF at all -- breaks the capture kernel too, at the same point as CPU_OFF does. The only variant that boots is the one that leaves the event uncompleted and parks. So here it's completing the event that the capture kernel trips over, not the dangling dispatch -- backwards from the non-compliance theory, where completing should be the safe thing and "useful even without CPU_OFF".
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Quoted 9 lines · Kiryl Shutsemau
Quoted 5 lines · Catalin Marinas
Have you tried
SDEI_EVENT_COMPLETE_AND_RESUMEinstead? Just COMPLETE won't return to the kernel. We havesdei_handler_abort()to complete the event and, hopefully, you can continue with theCPU_OFF. It's a work around the TF-A non-compliance but I think this is useful even if you don't issue theCPU_OFF(e.g. no CPU hotplug, just the park loop).Tried it. The result is the opposite of what I expected, and it argues against doing the complete at all under QEMU's TF-A.
I have to walk back on this. My test setup was broken. I will be back to you with proper data.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov
Quoted 8 lines · Kiryl Shutsemau
Quoted 7 lines · Kiryl Shutsemau
Quoted 5 lines · Catalin Marinas
Have you tried
SDEI_EVENT_COMPLETE_AND_RESUMEinstead? Just COMPLETE won't return to the kernel. We havesdei_handler_abort()to complete the event and, hopefully, you can continue with theCPU_OFF. It's a work around the TF-A non-compliance but I think this is useful even if you don't issue theCPU_OFF(e.g. no CPU hotplug, just the park loop).
Quoted 5 lines
Quoted 2 lines
Tried it. The result is the opposite of what I expected, and it argues against doing the complete at all under QEMU's TF-A.
I have to walk back on this. My test setup was broken. I will be back to you with proper data.
Back with proper data. Short version: CPU_OFF from the SDEI stop does not work on Grace, whether or not the event is completed first, so the series stays with the park. Details below.
What was broken before
======================
My QEMU kdump harness had three faults that together made CPU_OFF look like it broke the capture kernel:
- a race between the sysrq-c crash and the buddy detector -- the script that wedges the CPU wasn't pinned off it, so which crash path ran (and thus the outcome) was random;
- the capture kernel's console went quiet at the boot-console handover, so "nothing after Bye!" was mostly lost output, not a hang (keep_bootcon shows it booting on);
- the one genuine early stall was the capture kernel's CFI probe of the secure NOR flash -- it reproduces with the whole series compiled out (CONFIG_ARM_SDEI_NMI=n) and disappears with initcall_blacklist=physmap_init, i.e. nothing to do with SDEI.
With those fixed, QEMU/TF-A boots the capture kernel to userspace for both park and raw CPU_OFF, and CPU_OFF really powers the PE off. But QEMU can't decide the question that matters: its post-kexec hold-pen can't revive any CPU_OFF'd core (even a cleanly hotplug-offlined one), so re-onlining is undecidable there. Hence Grace.
Grace (Neoverse V2, EL3 TF-A)
=============================
- Series as posted (park): the SDEI rung stops the wedged CPU, its context lands in the vmcore, the capture kernel boots. Works. Bringing up secondary CPUs complains that it cannot get the CPU up, but proceeds.
- raw CPU_OFF from the uncompleted SDEI event: the capture kernel boots but hangs at "smp: Bringing up secondary CPUs ..." -- it cannot re-online the CPU that was CPU_OFF'd. This is exactly the TF-A point you raised: the SDEI dispatch is left dangling on CPU_OFF and the PE won't come back cleanly.
- complete-then-CPU_OFF: same hang at secondary bring-up. Completing the event first does not help.
For that last one I did not use sdei_handler_abort(): it can't express "complete, then CPU_OFF" from the handler, because its COMPLETE_AND_RESUME lands on the "1: ret" trampoline, which resumes the *interrupted* (wedged) context -- so nothing after it runs. I used a COMPLETE_AND_RESUME whose resume PC is a PSCI CPU_OFF stub, so the event is genuinely completed and then the PE powers off. It still can't be re-onlined. (I suspect the "complete-then-CPU_OFF wedged EL3" I mentioned earlier was in fact the sdei_handler_abort() path resuming the wedged loop, i.e. never doing the CPU_OFF at all.)
Conclusion
==========CPU_OFF from the SDEI stop is unusable on current TF-A, with or without completing the event first -- a concrete real-HW consequence of the non-compliance you flagged (TF-A subscribes SDEI to PSCI CPU_ON and suspend-wakeup, but not CPU_OFF). So the series keeps the park: the dump is complete either way, and only re-onlining the SDEI-stopped CPU in an SMP capture kernel is lost -- the same fallback the shared stop path already takes when CPU_OFF is unavailable. Powering the PE off for reuse is a firmware follow-up (TF-A completing or tearing down the SDEI dispatch on PSCI CPU_OFF), not something the kernel can paper over.
Please consider applying v5.
Signature · 1 line
Kiryl Shutsemau / Kirill A. Shutemov