[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251218012656.GA75051@chenghao-pc>
Date: Thu, 18 Dec 2025 09:26:56 +0800
From: Chenghao Duan <duanchenghao@...inos.cn>
To: bot+bpf-ci@...nel.org
Cc: yangtiezhu@...ngson.cn, rostedt@...dmis.org, mhiramat@...nel.org,
mark.rutland@....com, hengqi.chen@...il.com, chenhuacai@...nel.org,
kernel@...0n.name, zhangtianyang@...ngson.cn, masahiroy@...nel.org,
linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev,
bpf@...r.kernel.org, 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
On Wed, Dec 17, 2025 at 06:55:00AM +0000, bot+bpf-ci@...nel.org wrote:
> > 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.
Chenghao
>
> > + 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