Re: [PATCH v4 1/7] LoongArch: ftrace: Refactor register restoration in ftrace_common_return
From: Chris Mason <hidden>
Date: 2025-12-18 15:29:22
Also in:
bpf, lkml, loongarch
On 12/17/25 8:26 PM, Chenghao Duan wrote:
On Wed, Dec 17, 2025 at 06:55:00AM +0000, bot+bpf-ci@kernel.org wrote:quoted
quoted
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.The patch "LoongArch: ftrace: Refactor register restoration in ftrace_common_return" changed the conventions for t0 and ra after entering the direct call function, which is exactly the reason forthe changes made in this patch.
I'll try to adjust the kinds of ABI breakage AI comments on. It did catch the other related changes from this series, but the additional commentary wasn't useful. Thanks, Chris