[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <5c7c7239a6f89e685f69093ff84fe686c75cf6b9b051dcbf812841f00ab3bc31@mail.kernel.org>
Date: Wed, 17 Dec 2025 06:55:00 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: duanchenghao@...inos.cn,yangtiezhu@...ngson.cn,rostedt@...dmis.org,mhiramat@...nel.org,mark.rutland@....com,hengqi.chen@...il.com,chenhuacai@...nel.org
Cc: kernel@...0n.name,zhangtianyang@...ngson.cn,masahiroy@...nel.org,linux-kernel@...r.kernel.org,loongarch@...ts.linux.dev,bpf@...r.kernel.org,duanchenghao@...inos.cn,youling.tang@...ux.dev,jianghaoran@...inos.cn,vincent.mc.li@...il.com,linux-trace-kernel@...r.kernel.org,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH v4 1/7] LoongArch: ftrace: Refactor register restoration in ftrace_common_return
> 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
Powered by blists - more mailing lists