On Thu, Mar 16, 2023 at 12:51 AM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > On Tue, 7 Feb 2023 19:21:31 +0100 > Florent Revest <revest@xxxxxxxxxxxx> wrote: > > > From: Mark Rutland <mark.rutland@xxxxxxx> > > > > The ftrace selftest code has a trace_direct_tramp() function which it > > uses as a direct call trampoline. This happens to work on x86, since the > > direct call's return address is in the usual place, and can be returned > > to via a RET, but in general the calling convention for direct calls is > > different from regular function calls, and requires a trampoline written > > in assembly. > > > > On s390, regular function calls place the return address in %r14, and an > > ftrace patch-site in an instrumented function places the trampoline's > > return address (which is within the instrumented function) in %r0, > > preserving the original %r14 value in-place. As a regular C function > > will return to the address in %r14, using a C function as the trampoline > > results in the trampoline returning to the caller of the instrumented > > function, skipping the body of the instrumented function. > > > > Note that the s390 issue is not detcted by the ftrace selftest code, as > > the instrumented function is trivial, and returning back into the caller > > happens to be equivalent. > > > > On arm64, regular function calls place the return address in x30, and > > an ftrace patch-site in an instrumented function saves this into r9 > > and places the trampoline's return address (within the instrumented > > function) in x30. A regular C function will return to the address in > > x30, but will not restore x9 into x30. Consequently, using a C function > > as the trampoline results in returning to the trampoline's return > > address having corrupted x30, such that when the instrumented function > > returns, it will return back into itself. > > > > To avoid future issues in this area, remove the trace_direct_tramp() > > function, and require that each architecture with direct calls provides > > a stub trampoline, named ftrace_stub_direct_tramp. This can be written > > to handle the architecture's trampoline calling convention, and in > > future could be used elsewhere (e.g. in the ftrace ops sample, to > > measure the overhead of direct calls), so we may as well always build it > > in. > > > > Signed-off-by: Mark Rutland <mark.rutland@xxxxxxx> > > Cc: Li Huafei <lihuafei1@xxxxxxxxxx> > > Cc: Xu Kuohai <xukuohai@xxxxxxxxxx> > > Cc: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx> > > Cc: Florent Revest <revest@xxxxxxxxxxxx> > > Signed-off-by: Florent Revest <revest@xxxxxxxxxxxx> > > > Care to respin with my update requests? I can take up to this patch and > base it directly on v6.3-rc3 when it comes out. I'm expecting that to have > the fixes in other code that is breaking my tests. Okay! :) I'll send you a subset of this series (first 6 patches with your review addressed and rebased on v6.3-rc2 for now). > Then I'll push it out after it passes all my tests, and you can take it > and add the arm64 specific bits on top. I'm currently running these patches > as is on my tests to see if they fail (with a patched kernel for the other > code that's breaking my tests). > > Does that sound OK? Sounds good to me, yes!
