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

2026-05-18 Thread Dylan Hatch
On Fri, May 15, 2026 at 1:58 AM Mark Rutland  wrote:
>
> On Thu, May 14, 2026 at 08:30:43PM -0700, Dylan Hatch wrote:
> > On Wed, Apr 29, 2026 at 8:26 AM Mark Rutland  wrote:
> > > On Tue, Apr 28, 2026 at 06:36:38PM +, Dylan Hatch wrote:
> > > > From: Weinan Liu 
> > > >
> > > > 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 minimal CFI to enable stacktracing using SFrame for kernel
> > > > exception entries through el1*_64_*() paths
> > >
> > > I thought we were only consuming SFrame when unwinding an exeption
> > > boundary?
> > >
> > > We shouldn't be taking exceptions _from_ the entry assembly functions
> > > unless something has gone horribly wrong, and so I don't see why we'd
> > > need CFI entries for the entry assembly functions.
> > >
> > > Am I missing some reason we need CFI entries for the entry assembly
> > > functions? I strongly suspect it is not necessary to add these, and I'd
> > > prefer to omit them.
> >
> > I believe the el1 entry functions are called in an exception, and are
> > called before call_on_irq_stack.
>
> Yes, but I don't think that matters. See below for more details.
>
> > Example stacktrace segment:
> >
> > [  262.119564]  handle_percpu_devid_irq+0xb4/0x348
> > [  262.119913]  handle_irq_desc+0x3c/0x68
> > [  262.120196]  generic_handle_domain_irq+0x20/0x40
> > [  262.120678]  gic_handle_irq+0x48/0xe0
> > [  262.121005]  call_on_irq_stack+0x30/0x48
> > [  262.121412]  do_interrupt_handler+0x88/0xa0
> > [  262.121779]  el1_interrupt+0x38/0x58
> > [  262.122089]  el1h_64_irq_handler+0x18/0x30
> > [  262.122617]  el1h_64_irq+0x6c/0x70
>
> The segment immediately above can be unwound using FP, as frame records
> were created consistently, and there are no exception boundaries. No CFI
> needed.

Ah right -- with the logic in stacktrace.c changed to use SFrame only
when recovering the next frame from a pt_regs, the FP alone is
sufficient if we know these entry functions won't take an exception.
This patch was originally implemented with an SFrame-only unwinder in
mind, so my mental model still hadn't back-propagated the new logic to
this patch :)

>
> It's legitimate to take an exception from parts of call_on_irq_stack(),
> so it makes sense for that to have CFI, but for the specific unwind
> segment above, CFI isn't necessary.
>
> Everything in the stacktrace segment above was executed *after* HW took
> the exception.
>
> << EXCEPTION BOUNDARY HERE >>
>
> Everything in the stacktrace segment(s) below was executed *before* HW
> took the exception.
>
> The unwinder knows that it has crossed this exception boundary by virtue
> of finding a FRAME_META_TYPE_PT_REGS frame record.
>
> > [  262.123159]  _raw_spin_unlock_irq+0x10/0x60 (P)
>
> The unwinder knows that the value of pt_regs::pc was *definitely* the PC
> at the time the exception was taken, so that entry is reliable. No CFI
> needed.
>
> > [  262.123720]  __filemap_add_folio+0x200/0x580 (L)
>
> The unwinder doesn't know whether the LR should be used, and needs CFI
> to determine that.
>
> After this point, an FP unwind can be used until we encounter the next
> exception boundary.

Right, and this is what is implemented in the final patch of this series.

>
> > [  262.124145]  filemap_add_folio+0xec/0x300
> > [  262.124674]  page_cache_ra_unbounded+0x128/0x368
> > [  262.125338]  do_page_cache_ra+0x70/0x98
> > [  262.125875]  page_cache_ra_order+0x460/0x4e0
>
> The segment immediately above can be unwound using FP. No CFI needed.
>
> > Here, el1h_64_irq is the last function that appears in the exception
> > stack before _raw_spin_unlock_irq and __filemap_add_folio are
> > recovered from the saved PC and LR, respectively. So we therefore need
> > the CFI annotations in order to unwind through the full exception
> > boundary.
> >
> > Is my interpretation here correct?
>
> Given you say "full exception boundary" here, I think we might be using
> the term "exception boundary" to mean different things.
>
> As per the example above, I'm using "exception boundary" to mean the a
> point between two entries in the stacktrace where HW took an exception.
>
> Did my comments on the example help?

I admit I may have been using the term "exception boundary" with a
vague definition, which was partly the source of my confusion. Thanks
for the example, it did help.

>
> > > > and irq entries through call_on_irq_stack()
> > >
> > > Needing some sort of unwind annotations for call_on_irq_stack() makes
> > > sense to me, but don't we need something for other assembly functions
> > > too?
> > >
> > > We can interrupt things like memset(); I assume we'll treat those as
> > > unreliable until annotated?
> >
> > While looking into adding these

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

2026-05-15 Thread Mark Rutland
On Thu, May 14, 2026 at 08:30:43PM -0700, Dylan Hatch wrote:
> On Wed, Apr 29, 2026 at 8:26 AM Mark Rutland  wrote:
> > On Tue, Apr 28, 2026 at 06:36:38PM +, Dylan Hatch wrote:
> > > From: Weinan Liu 
> > >
> > > 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 minimal CFI to enable stacktracing using SFrame for kernel
> > > exception entries through el1*_64_*() paths
> >
> > I thought we were only consuming SFrame when unwinding an exeption
> > boundary?
> >
> > We shouldn't be taking exceptions _from_ the entry assembly functions
> > unless something has gone horribly wrong, and so I don't see why we'd
> > need CFI entries for the entry assembly functions.
> >
> > Am I missing some reason we need CFI entries for the entry assembly
> > functions? I strongly suspect it is not necessary to add these, and I'd
> > prefer to omit them.
> 
> I believe the el1 entry functions are called in an exception, and are
> called before call_on_irq_stack. 

Yes, but I don't think that matters. See below for more details.

> Example stacktrace segment:
> 
> [  262.119564]  handle_percpu_devid_irq+0xb4/0x348
> [  262.119913]  handle_irq_desc+0x3c/0x68
> [  262.120196]  generic_handle_domain_irq+0x20/0x40
> [  262.120678]  gic_handle_irq+0x48/0xe0
> [  262.121005]  call_on_irq_stack+0x30/0x48
> [  262.121412]  do_interrupt_handler+0x88/0xa0
> [  262.121779]  el1_interrupt+0x38/0x58
> [  262.122089]  el1h_64_irq_handler+0x18/0x30
> [  262.122617]  el1h_64_irq+0x6c/0x70

The segment immediately above can be unwound using FP, as frame records
were created consistently, and there are no exception boundaries. No CFI
needed.

It's legitimate to take an exception from parts of call_on_irq_stack(),
so it makes sense for that to have CFI, but for the specific unwind
segment above, CFI isn't necessary.

Everything in the stacktrace segment above was executed *after* HW took
the exception.

<< EXCEPTION BOUNDARY HERE >>

Everything in the stacktrace segment(s) below was executed *before* HW
took the exception.

The unwinder knows that it has crossed this exception boundary by virtue
of finding a FRAME_META_TYPE_PT_REGS frame record.

> [  262.123159]  _raw_spin_unlock_irq+0x10/0x60 (P)

The unwinder knows that the value of pt_regs::pc was *definitely* the PC
at the time the exception was taken, so that entry is reliable. No CFI
needed.

> [  262.123720]  __filemap_add_folio+0x200/0x580 (L)

The unwinder doesn't know whether the LR should be used, and needs CFI
to determine that.

After this point, an FP unwind can be used until we encounter the next
exception boundary.

> [  262.124145]  filemap_add_folio+0xec/0x300
> [  262.124674]  page_cache_ra_unbounded+0x128/0x368
> [  262.125338]  do_page_cache_ra+0x70/0x98
> [  262.125875]  page_cache_ra_order+0x460/0x4e0

The segment immediately above can be unwound using FP. No CFI needed.

> Here, el1h_64_irq is the last function that appears in the exception
> stack before _raw_spin_unlock_irq and __filemap_add_folio are
> recovered from the saved PC and LR, respectively. So we therefore need
> the CFI annotations in order to unwind through the full exception
> boundary.
> 
> Is my interpretation here correct?

Given you say "full exception boundary" here, I think we might be using
the term "exception boundary" to mean different things.

As per the example above, I'm using "exception boundary" to mean the a
point between two entries in the stacktrace where HW took an exception.

Did my comments on the example help?

> > > and irq entries through call_on_irq_stack()
> >
> > Needing some sort of unwind annotations for call_on_irq_stack() makes
> > sense to me, but don't we need something for other assembly functions
> > too?
> >
> > We can interrupt things like memset(); I assume we'll treat those as
> > unreliable until annotated?
> 
> While looking into adding these annotations, I noticed a pattern where
> a sibling call is made to a local function:
> 
> SYM_FUNC_START(__pi_memset)
> alternative_if_not ARM64_HAS_MOPS
> b   __pi_memset_generic
> alternative_else_nop_endif
> 
> mov dst, dstin
> setp[dst]!, count!, val_x
> setm[dst]!, count!, val_x
> sete[dst]!, count!, val_x
> ret
> SYM_FUNC_END(__pi_memset)
> 
> In this case, do we consider the stacktrace unreliable since
> __pi_memset may not appear in the trace?

This is a tail-call, and __pi_memset_generic() will not return to
__pi_memset(). Once the branch to __pi_memset_generic() has been
executed, it's fine for __pi_memset() to not show up in the trace.

The key thing is that no more instructions from __pi_memset() itself
will be executed unless it was called again (from its en

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

2026-05-14 Thread Dylan Hatch
On Wed, Apr 29, 2026 at 8:26 AM Mark Rutland  wrote:
>
> Hi Dylan,
>
> On Tue, Apr 28, 2026 at 06:36:38PM +, Dylan Hatch wrote:
> > From: Weinan Liu 
> >
> > 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 minimal CFI to enable stacktracing using SFrame for kernel
> > exception entries through el1*_64_*() paths
>
> I thought we were only consuming SFrame when unwinding an exeption
> boundary?
>
> We shouldn't be taking exceptions _from_ the entry assembly functions
> unless something has gone horribly wrong, and so I don't see why we'd
> need CFI entries for the entry assembly functions.
>
> Am I missing some reason we need CFI entries for the entry assembly
> functions? I strongly suspect it is not necessary to add these, and I'd
> prefer to omit them.

I believe the el1 entry functions are called in an exception, and are
called before call_on_irq_stack. Example stacktrace segment:

[  262.119564]  handle_percpu_devid_irq+0xb4/0x348
[  262.119913]  handle_irq_desc+0x3c/0x68
[  262.120196]  generic_handle_domain_irq+0x20/0x40
[  262.120678]  gic_handle_irq+0x48/0xe0
[  262.121005]  call_on_irq_stack+0x30/0x48
[  262.121412]  do_interrupt_handler+0x88/0xa0
[  262.121779]  el1_interrupt+0x38/0x58
[  262.122089]  el1h_64_irq_handler+0x18/0x30
[  262.122617]  el1h_64_irq+0x6c/0x70
[  262.123159]  _raw_spin_unlock_irq+0x10/0x60 (P)
[  262.123720]  __filemap_add_folio+0x200/0x580 (L)
[  262.124145]  filemap_add_folio+0xec/0x300
[  262.124674]  page_cache_ra_unbounded+0x128/0x368
[  262.125338]  do_page_cache_ra+0x70/0x98
[  262.125875]  page_cache_ra_order+0x460/0x4e0

Here, el1h_64_irq is the last function that appears in the exception
stack before _raw_spin_unlock_irq and __filemap_add_folio are
recovered from the saved PC and LR, respectively. So we therefore need
the CFI annotations in order to unwind through the full exception
boundary.

Is my interpretation here correct?

>
> > and irq entries through call_on_irq_stack()
>
> Needing some sort of unwind annotations for call_on_irq_stack() makes
> sense to me, but don't we need something for other assembly functions
> too?
>
> We can interrupt things like memset(); I assume we'll treat those as
> unreliable until annotated?

While looking into adding these annotations, I noticed a pattern where
a sibling call is made to a local function:

SYM_FUNC_START(__pi_memset)
alternative_if_not ARM64_HAS_MOPS
b   __pi_memset_generic
alternative_else_nop_endif

mov dst, dstin
setp[dst]!, count!, val_x
setm[dst]!, count!, val_x
sete[dst]!, count!, val_x
ret
SYM_FUNC_END(__pi_memset)

In this case, do we consider the stacktrace unreliable since
__pi_memset may not appear in the trace? Or is this not important
because assembly functions cannot be directly livepatched anyway?

Thanks,
Dylan



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

2026-04-29 Thread Mark Rutland
Hi Dylan,

On Tue, Apr 28, 2026 at 06:36:38PM +, Dylan Hatch wrote:
> From: Weinan Liu 
> 
> 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 minimal CFI to enable stacktracing using SFrame for kernel
> exception entries through el1*_64_*() paths 

I thought we were only consuming SFrame when unwinding an exeption
boundary?

We shouldn't be taking exceptions _from_ the entry assembly functions
unless something has gone horribly wrong, and so I don't see why we'd
need CFI entries for the entry assembly functions.

Am I missing some reason we need CFI entries for the entry assembly
functions? I strongly suspect it is not necessary to add these, and I'd
prefer to omit them.

> and irq entries through call_on_irq_stack()

Needing some sort of unwind annotations for call_on_irq_stack() makes
sense to me, but don't we need something for other assembly functions
too?

We can interrupt things like memset(); I assume we'll treat those as
unreliable until annotated?

Mark.

> Signed-off-by: Weinan Liu 
> Suggested-by: Jens Remus 
> Reviewed-by: Jens Remus 
> Signed-off-by: Dylan Hatch 
> ---
>  arch/arm64/kernel/entry.S | 23 +++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index f8018b5c1f9a..dc55b0b19cfa 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -30,6 +30,12 @@
>  #include 
>  #include 
>  
> +/*
> + * Do not generate .eh_frame.  Only generate .debug_frame and optionally
> + * .sframe (via assembler option --gsframe[-N]).
> + */
> + .cfi_sections .debug_frame
> +
>   .macro  clear_gp_regs
>   .irp
> n,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29
>   mov x\n, xzr
> @@ -575,7 +581,16 @@ SYM_CODE_START_LOCAL(el\el\ht\()_\regsize\()_\label)
>   .if \el == 0
>   b   ret_to_user
>   .else
> + /*
> +  * Minimal DWARF CFI for unwinding across the call above.
> +  * Enable unwinding for el1*_64_*() path only.
> +  */
> + .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
> @@ -872,6 +887,7 @@ NOKPROBE(ret_from_fork)
>   * Calls func(regs) using this CPU's irq stack and shadow irq stack.
>   */
>  SYM_FUNC_START(call_on_irq_stack)
> + .cfi_startproc
>   save_and_disable_daif x9
>  #ifdef CONFIG_SHADOW_CALL_STACK
>   get_current_task x16
> @@ -882,6 +898,9 @@ SYM_FUNC_START(call_on_irq_stack)
>   /* 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
>  
>   ldr_this_cpu x16, irq_stack_ptr, x17
>  
> @@ -897,9 +916,13 @@ SYM_FUNC_START(call_on_irq_stack)
>*/
>   mov sp, x29
>   ldp x29, x30, [sp], #16
> + .cfi_restore 29
> + .cfi_restore 30
> + .cfi_def_cfa 31, 0
>   scs_load_current
>   restore_irq x9
>   ret
> + .cfi_endproc
>  SYM_FUNC_END(call_on_irq_stack)
>  NOKPROBE(call_on_irq_stack)
>  
> -- 
> 2.54.0.545.g6539524ca2-goog
>