> diff --git a/arch/loongarch/kernel/mcount_dyn.S 
> b/arch/loongarch/kernel/mcount_dyn.S
> index d6b474ad1d5e..5729c20e5b8b 100644
> --- a/arch/loongarch/kernel/mcount_dyn.S
> +++ b/arch/loongarch/kernel/mcount_dyn.S
> @@ -94,7 +94,6 @@ SYM_INNER_LABEL(ftrace_graph_call, SYM_L_GLOBAL)
>   * at the callsite, so there is no need to restore the T series regs.
>   */
>  ftrace_common_return:
> -     PTR_L           ra, sp, PT_R1
>       PTR_L           a0, sp, PT_R4
>       PTR_L           a1, sp, PT_R5
>       PTR_L           a2, sp, PT_R6
> @@ -104,12 +103,17 @@ ftrace_common_return:
>       PTR_L           a6, sp, PT_R10
>       PTR_L           a7, sp, PT_R11
>       PTR_L           fp, sp, PT_R22
> -     PTR_L           t0, sp, PT_ERA
>       PTR_L           t1, sp, PT_R13
> -     PTR_ADDI        sp, sp, PT_SIZE
>       bnez            t1, .Ldirect
> +
> +     PTR_L           ra, sp, PT_R1
> +     PTR_L           t0, sp, PT_ERA
> +     PTR_ADDI        sp, sp, PT_SIZE
>       jr              t0
>  .Ldirect:
> +     PTR_L           t0, sp, PT_R1
> +     PTR_L           ra, sp, PT_ERA
                     ^^^^^^^^^^^^^^^^^^^
Does this break existing ftrace direct trampolines?

Before this change, direct trampolines received:
  ra = parent return address (from PT_R1)
  t0 = traced function address (from PT_ERA)

After this change, direct trampolines receive:
  t0 = parent return address (from PT_R1)
  ra = traced function address (from PT_ERA)

The sample trampolines in samples/ftrace/ were written for the old
convention. For example, samples/ftrace/ftrace-direct.c does:

  my_tramp:
    st.d  $t0, $sp, 8    # Save what it thinks is traced func addr
    st.d  $ra, $sp, 16   # Save what it thinks is parent addr
    bl    my_direct_func
    ld.d  $t0, $sp, 8    # Restore
    ld.d  $ra, $sp, 16   # Restore
    jr    $t0            # Jump to what it thinks is traced function

With the new convention, the sample would save parent address in the
t0 slot, restore it to t0, and then jump to the parent instead of the
traced function, skipping the traced function entirely.

This was fixed later in commit 055e392f3be7 "LoongArch: ftrace: Adjust
register stack restore order in direct call trampolines" which updated
all the samples to match the new convention. However, any out-of-tree
direct trampolines written before this commit would be broken by this
change.

Was this ABI break intentional? The commit message mentions matching
"the state when ftrace was entered" but doesn't explain why breaking
compatibility was necessary. RISC-V uses the opposite convention
(t0 = traced func, ra = parent), which the original LoongArch code
matched.

> +     PTR_ADDI        sp, sp, PT_SIZE
>       jr              t1
>  SYM_CODE_END(ftrace_common)

[ ... ]

> @@ -161,6 +165,8 @@ SYM_CODE_END(return_to_handler)
>  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>  SYM_CODE_START(ftrace_stub_direct_tramp)
>       UNWIND_HINT_UNDEFINED
> -     jr              t0
> +     move            t1, ra
> +     move            ra, t0
> +     jr              t1
>  SYM_CODE_END(ftrace_stub_direct_tramp)
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20293821855

Reply via email to