Thread (12 messages) 12 messages, 2 authors, 14d ago

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.h
diff --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			0x1f
diff --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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help