Re: [PATCH v4 RESEND 1/7] riscv: stacktrace: Add frame record metadata
From: Shuai Xue <xueshuai@linux.alibaba.com>
Date: 2026-07-08 08:00:07
Also in:
linux-kselftest, linux-perf-users, linux-riscv, live-patching, lkml
On 6/29/26 3:27 PM, Wang Han wrote:
Reliable frame-pointer unwinding needs an explicit way to identify
exception boundaries and the final entry frame. The existing unwinder
infers those boundaries from return addresses, which is too loose for a
future reliable unwinder.
Add a small metadata frame record to pt_regs and initialize it on
exception entry, kernel stack overflow, kernel thread fork, user fork,
and early idle task setup. The record uses a zero {fp, ra} sentinel plus
a type field so a later unwinder can distinguish a final user-to-kernel
boundary from a nested kernel pt_regs boundary.
This follows the arm64 metadata frame-record model, adapted to the
RISC-V {fp, ra} frame record convention.
The metadata is established at the RISC-V entry boundaries that need an
explicit unwind marker:
* exception entry clears the metadata {fp, ra} pair and uses SPP
(or MPP in M-mode) to record whether the pt_regs frame is the final
user-to-kernel boundary or a nested kernel boundary;
* the kernel stack overflow path builds a nested pt_regs metadata
record on the overflow stack so an unwinder can resume from the
pre-overflow s0 saved in PT_S0;
* _start_kernel builds the init task's final metadata record, while
the secondary CPU path sets up s0 before smp_callin() so idle-task
unwinding does not inherit an undefined caller frame;
* copy_thread creates matching final metadata records for new kernel
and user tasks, and keeps s0 available for the frame-pointer chain.
Keep the embedded metadata-record field offsets distinct from the
s0-relative STACKFRAME_* offsets used by call_on_irq_stack(), because
the latter describe a frame record relative to s0 rather than to the
record base.
These changes keep s0 reserved for the frame-pointer chain at task and
exception boundaries.Overall the patch looks good to me. One nit below:
quoted hunk ↗ jump to hunk
Signed-off-by: Wang Han <redacted> --- arch/riscv/include/asm/ptrace.h | 9 ++++ arch/riscv/include/asm/stacktrace/frame.h | 53 +++++++++++++++++++++++ arch/riscv/kernel/asm-offsets.c | 6 +++ arch/riscv/kernel/entry.S | 39 ++++++++++++++++- arch/riscv/kernel/head.S | 23 ++++++++++ arch/riscv/kernel/process.c | 33 +++++++++++++- 6 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 arch/riscv/include/asm/stacktrace/frame.hdiff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h index addc8188152f..4b9b0f279214 100644 --- a/arch/riscv/include/asm/ptrace.h +++ b/arch/riscv/include/asm/ptrace.h@@ -8,6 +8,7 @@ #include <uapi/asm/ptrace.h> #include <asm/csr.h> +#include <asm/stacktrace/frame.h> #include <linux/compiler.h> #ifndef __ASSEMBLER__@@ -53,6 +54,14 @@ struct pt_regs { unsigned long cause; /* a0 value before the syscall */ unsigned long orig_a0; + + /* + * This frame record is entirely zeroed on exception entry, allowing the + * unwinder to identify exception boundaries. The type field encodes + * whether the exception was taken from user (FINAL) or kernel (PT_REGS) + * mode. + */ + struct frame_record_meta stackframe; }; #define PTRACE_SYSEMU 0x1fdiff --git a/arch/riscv/include/asm/stacktrace/frame.h b/arch/riscv/include/asm/stacktrace/frame.h new file mode 100644 index 000000000000..5720a6c65fe8 --- /dev/null +++ b/arch/riscv/include/asm/stacktrace/frame.h@@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef __ASM_RISCV_STACKTRACE_FRAME_H +#define __ASM_RISCV_STACKTRACE_FRAME_H + +/* + * See: arch/arm64/include/asm/stacktrace/frame.h for the reference + * implementation. + */ + +/* + * - FRAME_META_TYPE_NONE + * + * This value is reserved. + * + * - FRAME_META_TYPE_FINAL + * + * The record is the last entry on the stack. + * Unwinding should terminate successfully. + * + * - FRAME_META_TYPE_PT_REGS + * + * The record is embedded within a struct pt_regs, recording the registers at + * an arbitrary point in time. + * Unwinding should consume pt_regs::epc, followed by pt_regs::ra. + * + * Note: all other values are reserved and should result in unwinding + * terminating with an error. + */ +#define FRAME_META_TYPE_NONE 0 +#define FRAME_META_TYPE_FINAL 1 +#define FRAME_META_TYPE_PT_REGS 2 + +#ifndef __ASSEMBLER__ +/* + * A standard RISC-V frame record. + */ +struct frame_record { + unsigned long fp; + unsigned long ra; +}; + +/* + * A metadata frame record indicating a special unwind. + * The record::{fp,ra} fields must be zero to indicate the presence of + * metadata. + */ +struct frame_record_meta { + struct frame_record record; + unsigned long type; +}; +#endif /* __ASSEMBLER__ */ + +#endif /* __ASM_RISCV_STACKTRACE_FRAME_H */diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c index a75f0cfea1e9..bc8e8cd7130a 100644 --- a/arch/riscv/kernel/asm-offsets.c +++ b/arch/riscv/kernel/asm-offsets.c@@ -131,6 +131,9 @@ void asm_offsets(void) OFFSET(PT_BADADDR, pt_regs, badaddr); OFFSET(PT_CAUSE, pt_regs, cause); + DEFINE(S_STACKFRAME, offsetof(struct pt_regs, stackframe)); + DEFINE(S_STACKFRAME_TYPE, offsetof(struct pt_regs, stackframe.type)); + OFFSET(SUSPEND_CONTEXT_REGS, suspend_context, regs); OFFSET(HIBERN_PBE_ADDR, pbe, address);@@ -503,6 +506,9 @@ void asm_offsets(void) DEFINE(STACKFRAME_SIZE_ON_STACK, ALIGN(sizeof(struct stackframe), STACK_ALIGN)); DEFINE(STACKFRAME_FP, offsetof(struct stackframe, fp) - sizeof(struct stackframe)); DEFINE(STACKFRAME_RA, offsetof(struct stackframe, ra) - sizeof(struct stackframe)); + DEFINE(STACKFRAME_RECORD_SIZE, sizeof(struct stackframe));
This should be sizeof(struct frame_record) rather than
sizeof(struct stackframe). STACKFRAME_RECORD_SIZE is used to skip past
the embedded frame_record_meta.record field:
addi s0, sp, S_STACKFRAME + STACKFRAME_RECORD_SIZE
The two structs happen to be identical today, but they are independent
types serving different purposes — struct stackframe is the existing
unwinder frame record, while struct frame_record is the one embedded
in struct frame_record_meta introduced by this patch. If struct
stackframe ever gains an extra field, this offset would silently break.
With that fixed:
Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
Thanks.
Shuai