Re: [PATCH v3 2/8] arm64, unwind: build kernel with sframe V3 info

[Jens Remus <jremus@linux.ibm.com>]

On 4/6/2026 8:49 PM, Dylan Hatch wrote:
> Build with -Wa,--gsframe-3 flags to generate a .sframe section. This
> will be used for in-kernel reliable stacktrace in cases where the frame
> pointer alone is insufficient.
> 
> Currently, the sframe format only supports arm64, x86_64 and s390x
> architectures.
> 
> Signed-off-by: Weinan Liu <wnliu@google.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>
> Reviewed-by: Prasanna Kumar T S M <ptsm@linux.microsoft.com>
> diff --git a/MAINTAINERS b/MAINTAINERS

> @@ -27561,6 +27561,7 @@ STACK UNWINDING
>  M:	Josh Poimboeuf <jpoimboe@kernel.org>
>  M:	Steven Rostedt <rostedt@goodmis.org>
>  S:	Maintained
> +F:	arch/*/include/asm/unwind_sframe.h
>  F:	include/linux/sframe.h
>  F:	include/linux/unwind*.h
>  F:	kernel/unwind/

Good catch!  Should we rather add the following in the series you are
basing on, as there are already arch-specific unwind_user.h and
unwind_user_sframe.h?

F:	arch/*/include/asm/unwind*.h

On the other hand I wonder whether the arch-specific headers should
remain maintained by the respective arch maintainers?  How is that
handled in general?

> diff --git a/arch/Kconfig b/arch/Kconfig

> @@ -520,6 +520,13 @@ config SFRAME_VALIDATION
>  
>  	  If unsure, say N.
>  
> +config ARCH_SUPPORTS_SFRAME_UNWINDER
> +	bool
> +	help
> +	  An architecture can select this if it  enables the sframe (Simple
> +	  Frame) unwinder for unwinding kernel stack traces. It uses unwind
> +	  table that is directly generatedby toolchain based on DWARF CFI information.

Nit: s/sframe/SFrame/

> +
>  config HAVE_PERF_REGS
>  	bool
>  	help

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig

> @@ -112,6 +112,7 @@ config ARM64
>  	select ARCH_SUPPORTS_SCHED_SMT
>  	select ARCH_SUPPORTS_SCHED_CLUSTER
>  	select ARCH_SUPPORTS_SCHED_MC
> +	select ARCH_SUPPORTS_SFRAME_UNWINDER
>  	select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
>  	select ARCH_WANT_COMPAT_IPC_PARSE_VERSION if COMPAT
>  	select ARCH_WANT_DEFAULT_BPF_JIT

> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug

> @@ -20,4 +20,17 @@ config ARM64_RELOC_TEST
>  	depends on m
>  	tristate "Relocation testing module"
>  
> +config SFRAME_UNWINDER

Why do you introduce this for arm64 (and debug) only?  If s390 were to
add support (as replacement for s390 backchain), this would have to be
moved or duplicated.  It would not suffice to enable
ARCH_SUPPORTS_SFRAME_UNWINDER to also enable SFRAME_UNWINDER.

As mentioned in my feedback on the previous patch in this series:
Would it make sense to align the naming to the existing 
HAVE_UNWIND_USER_SFRAME, for instance:

  HAVE_UNWIND_KERNEL_SFRAME

> +	bool "Sframe unwinder"
> +	depends on AS_SFRAME3
> +	depends on 64BIT
> +	depends on ARCH_SUPPORTS_SFRAME_UNWINDER
> +	select SFRAME_LOOKUP
> +	help
> +	  This option enables the sframe (Simple Frame) unwinder for unwinding
> +	  kernel stack traces. It uses unwind table that is directly generated
> +	  by toolchain based on DWARF CFI information. In, practice this can
> +	  provide more reliable stacktrace results than unwinding with frame
> +	  pointers alone.

Nit: s/sframe/SFrame/

> +
>  source "drivers/hwtracing/coresight/Kconfig"

You are introducing two new Kconfig options (SFRAME_UNWINDER and
ARCH_SUPPORTS_SFRAME_UNWINDER).  I wonder whether they could somehow be
combined into a single new option.  Although I am not sure how an option
can be both selectable and depending at the same time, so that the ARM64
config could select it, but it would also depend on the above.

> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h

> @@ -491,6 +491,8 @@
>  		*(.rodata1)						\
>  	}								\
>  									\
> +	SFRAME								\
> +									\
>  	/* PCI quirks */						\
>  	.pci_fixup        : AT(ADDR(.pci_fixup) - LOAD_OFFSET) {	\
>  		BOUNDED_SECTION_PRE_LABEL(.pci_fixup_early,  _pci_fixups_early,  __start, __end) \
> @@ -911,6 +913,19 @@
>  #define TRACEDATA
>  #endif
>  
> +#ifdef CONFIG_SFRAME_UNWINDER
> +#define SFRAME							\
> +	/* sframe */						\
> +	.sframe : AT(ADDR(.sframe) - LOAD_OFFSET) {		\
> +		__start_sframe_header = .;			\

		__start_sframe[_section] = .;

> +		KEEP(*(.sframe))				\
> +		KEEP(*(.init.sframe))				\
> +		__stop_sframe_header = .;			\

		__stop_sframe[_section] = .;

Unless I am missing something both are not the start/stop of the .sframe
header (in the .sframe section) but the .sframe section itself (see also
your subsequent "[PATCH v3 4/8] sframe: Provide PC lookup for vmlinux
.sframe section." where you assign both to kernel_sfsec.sframe_start
and kernel_sfsec.sframe_end.

> +	}
> +#else
> +#define SFRAME
> +#endif
> +
>  #ifdef CONFIG_PRINTK_INDEX
>  #define PRINTK_INDEX							\
>  	.printk_index : AT(ADDR(.printk_index) - LOAD_OFFSET) {		\

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/