Re: [PATCH v2 0/4] arm64/ftrace: move to DYNAMIC_FTRACE_WITH_ARGS

[Google]

On Thu,  3 Nov 2022 17:05:16 +0000
Mark Rutland <mark.rutland@xxxxxxx> wrote:

> This series replaces arm64's support for FTRACE_WITH_REGS with support
> for FTRACE_WITH_ARGS. This removes some overhead and complexity, and
> removes some latent issues with inconsistent presentation of struct
> pt_regs (which can only be reliably saved/restored at exception
> boundaries).
> 
> The existing FTRACE_WITH_REGS support was added for two major reasons:
> 
> (1) To make it possible to use the ftrace graph tracer with pointer
>     authentication, where it's necessary to snapshot/manipulate the LR
>     before it is signed by the instrumented function.
> 
> (2) To make it possible to implement LIVEPATCH in future, where we need
>     to hook function entry before an instrumented function manipulates
>     the stack or argument registers. Practically speaking, we need to
>     preserve the argument/return registers, PC, LR, and SP.
> 
> Neither of these requires the full set of pt_regs, and only requires us
> to save/restore a subset of registers used for passing
> arguments/return-values and context/return information (which is the
> minimum set we always need to save/restore today).
> 
> As there is no longer a need to save different sets of registers for
> different features, we no longer need distinct `ftrace_caller` and
> `ftrace_regs_caller` trampolines. This allows the trampoline assembly to
> be simpler, and simplifies code which previously had to handle the two
> trampolines.
> 
> I've tested this with the ftrace selftests, where there are no
> unexpected failures.
> 
> I plan to build atop this with subsequent patches to add per-callsite
> ftrace_ops, and I'm sending these patches on their own as I think they
> make sense regardless.

Thanks! this series looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

So it is the good time to rewrite the new fprobe event handler
based on these interfaces :)

> 
> Since v1 [1]:
> * Change ifdeferry per Steve's request
> * Add ftrace_regs_query_register_offset() per Masami's request
> * Fix a bunch of typos
> 
> [1] https://lore.kernel.org/lkml/20221024140846.3555435-1-mark.rutland@xxxxxxx
> 
> This series can be found in my 'arm64/ftrace/minimal-regs' branch on
> kernel.org:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/
>   git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git
> 
> This version is tagged as:
> 
>   arm64-ftrace-minimal-regs-20221103
> 
> Thanks,
> Mark.
> 
> Mark Rutland (4):
>   ftrace: pass fregs to arch_ftrace_set_direct_caller()
>   ftrace: rename ftrace_instruction_pointer_set() ->
>     ftrace_regs_set_instruction_pointer()
>   ftrace: abstract DYNAMIC_FTRACE_WITH_ARGS accesses
>   ftrace: arm64: move from REGS to ARGS
> 
>  arch/arm64/Kconfig                |  18 +++--
>  arch/arm64/Makefile               |   2 +-
>  arch/arm64/include/asm/ftrace.h   |  72 ++++++++++++++++--
>  arch/arm64/kernel/asm-offsets.c   |  13 ++++
>  arch/arm64/kernel/entry-ftrace.S  | 117 ++++++++++++------------------
>  arch/arm64/kernel/ftrace.c        |  82 ++++++++++++---------
>  arch/arm64/kernel/module.c        |   3 -
>  arch/powerpc/include/asm/ftrace.h |  24 +++++-
>  arch/s390/include/asm/ftrace.h    |  29 +++++++-
>  arch/x86/include/asm/ftrace.h     |  49 +++++++++----
>  include/linux/ftrace.h            |  47 +++++++++---
>  kernel/livepatch/patch.c          |   2 +-
>  kernel/trace/Kconfig              |   6 +-
>  kernel/trace/ftrace.c             |   3 +-
>  14 files changed, 309 insertions(+), 158 deletions(-)
> 
> -- 
> 2.30.2
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>