On Fri, May 15, 2026 at 1:58 AM Mark Rutland <[email protected]> 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 <[email protected]> wrote: > > > On Tue, Apr 28, 2026 at 06:36:38PM +0000, Dylan Hatch wrote: > > > > From: Weinan Liu <[email protected]> > > > > > > > > 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 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 entry point). > > > Or is this not important because assembly functions cannot be directly > > livepatched anyway? > > To the best of my knowledge, reliable stacktrace is only used to > determine whether any thread is still within an old version of a > patchable function (including where it's within a callee thereof). > > I am not aware of a case where we'd need to detect whether a thread is > still within a non-patchable function, but I can't rule out that as a > possibility. > > That's more of a question for the livepatching maintainers. > > Thanks, > Mark. Thanks, Dylan
