Re: [PATCH v3 3/8] arm64: entry: add unwind info for various kernel entries

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

Hello Dylan and Weinan!

On 4/6/2026 8:49 PM, Dylan Hatch wrote:
> From: Weinan Liu <wnliu@google.com>
> 
> DWARF CFI (Call Frame Information) specifies how to recover the return
> address and callee-saved registers at each PC in a given function.
> Compilers are able to generate the CFI annotations when they compile
> the code to assembly language. For handcrafted assembly, we need to
> annotate them by hand.
> 
> Annotate CFI unwind info for assembly for interrupt and exception
> handlers.

It took me a while to figure, why CFI annotations are uncommonly only
added to selected instruction (ranges) and not the whole functions.  I
guess you only want to enable stacktracing using SFrame through
el1*_64_*() (from el1*_64_*_handler()) and call_on_irq_stack(), that is
why the added CFI annotations start after the bl/blr instructions, so
that whenever an unwound return address points after those bl/blr
SFrame can recover the stack pointer, frame pointer, and return address.

Wouldn't that be worth to be documented in the commit message?

> Signed-off-by: Weinan Liu <wnliu@google.com>
> Signed-off-by: Dylan Hatch <dylanbhatch@google.com>

> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S

> @@ -575,7 +575,12 @@ SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label)
>  	.if \el == 0
>  	b	ret_to_user
>  	.else

	/* Minimal DWARF CFI for unwinding across call above. */

> +	.cfi_startproc
> +	.cfi_def_cfa_offset PT_REGS_SIZE
> +	.cfi_offset 29, S_FP - PT_REGS_SIZE
> +	.cfi_offset 30, S_LR - PT_REGS_SIZE
>  	b	ret_to_kernel
> +	.cfi_endproc
>  	.endif
>  SYM_CODE_END(el\el\ht\()_\regsize\()_\label)
>  	.endm

> @@ -889,6 +894,10 @@ SYM_FUNC_START(call_on_irq_stack)
>  	add	sp, x16, #IRQ_STACK_SIZE
>  	restore_irq x9
>  	blr	x1

	/* Minimal DWARF CFI for unwinding across indirect call above. */

> +	.cfi_startproc
> +	.cfi_def_cfa 29, 16
> +	.cfi_offset 29, -16
> +	.cfi_offset 30, -8
>  
>  	save_and_disable_daif x9
>  	/*
> @@ -900,6 +909,7 @@ SYM_FUNC_START(call_on_irq_stack)
>  	scs_load_current
>  	restore_irq x9
>  	ret
> +	.cfi_endproc
>  SYM_FUNC_END(call_on_irq_stack)
>  NOKPROBE(call_on_irq_stack)

While above minimal DWARF CFI works for your use case, the following
minor issue should probably be better corrected (excerpt from
call_on_irq_stack with your patch applied):

	blr	x1
	.cfi_startproc
	.cfi_def_cfa 29, 16   <-- CFA is defined as FP + 16
	.cfi_offset 29, -16
	.cfi_offset 30, -8

	save_and_disable_daif x9
	/*
	 * Restore the SP from the FP, and restore the FP and LR from the frame
	 * record.
	 */
	mov	sp, x29
	ldp	x29, x30, [sp], #16   <-- FP is restored, so that the CFA definition is no longer valid
[CORRECTION]
	.cfi_restore 29
	.cfi_restore 30
	.cfi_def_cfa 31, 0
[/CORRECTION]
	scs_load_current
	restore_irq x9
	ret
	.cfi_endproc
SYM_FUNC_END(call_on_irq_stack)


Would it alternatively make sense to add complete DWARF CFI annotations
to call_on_irq_stack()?  I think the following would do:

SYM_FUNC_START(call_on_irq_stack)
	.cfi_startproc
...
	/* Create a frame record to save our LR and SP (implicit in FP) */
	stp	x29, x30, [sp, #-16]!
	mov	x29, sp
	.cfi_def_cfa 29, 16
	.cfi_offset 29, -16
	.cfi_offset 30, -8
...
	/*
	 * Restore the SP from the FP, and restore the FP and LR from the frame
	 * record.
	 */
	mov	sp, x29
	ldp	x29, x30, [sp], #16
	.cfi_restore 29
	.cfi_restore 30
	.cfi_def_cfa 31, 0
...
	ret
	.cfi_endproc
SYM_FUNC_END(call_on_irq_stack)

Thanks and 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/