| Message ID | 172615373091.133222.1812791604518973124.stgit@devnote2 (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph | expand |
Can I get an Acked-by from the AARCH64 maintainers for this patch? Thanks! -- Steve On Fri, 13 Sep 2024 00:08:51 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > on function_graph tracer because of simplifying the callback interface. > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > Changes in v8: > - Newly added. > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/ftrace.h | 23 ++++++----------------- > arch/arm64/kernel/asm-offsets.c | 12 ------------ > arch/arm64/kernel/entry-ftrace.S | 32 ++++++++++++++++++-------------- > arch/loongarch/Kconfig | 2 +- > arch/loongarch/include/asm/ftrace.h | 24 ++---------------------- > arch/loongarch/kernel/asm-offsets.c | 12 ------------ > arch/loongarch/kernel/mcount.S | 17 ++++++++++------- > arch/loongarch/kernel/mcount_dyn.S | 14 +++++++------- > arch/riscv/Kconfig | 2 +- > arch/riscv/include/asm/ftrace.h | 26 +++++--------------------- > arch/riscv/kernel/mcount.S | 24 +++++++++++++----------- > arch/s390/Kconfig | 2 +- > arch/s390/include/asm/ftrace.h | 26 +++++++++----------------- > arch/s390/kernel/asm-offsets.c | 6 ------ > arch/s390/kernel/mcount.S | 9 +++++---- > arch/x86/Kconfig | 2 +- > arch/x86/include/asm/ftrace.h | 22 ++-------------------- > arch/x86/kernel/ftrace_32.S | 15 +++++++++------ > arch/x86/kernel/ftrace_64.S | 17 +++++++++-------- > include/linux/ftrace.h | 14 +++++++++++--- > kernel/trace/Kconfig | 4 ++-- > kernel/trace/fgraph.c | 21 +++++++++------------ > 23 files changed, 122 insertions(+), 205 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a2f8ff354ca6..17947f625b06 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -211,6 +211,7 @@ config ARM64 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > select HAVE_FUNCTION_ERROR_INJECTION > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_RETVAL > select HAVE_GCC_PLUGINS > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index dc9cf0bd2a4c..dffaab3dd1f1 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -126,6 +126,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs) > fregs->pc = fregs->lr; > } > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->fp; > +} > + > int ftrace_regs_query_register_offset(const char *name); > > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > @@ -183,23 +189,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym, > > #ifndef __ASSEMBLY__ > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* x0 - x7 */ > - unsigned long regs[8]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > > void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, > unsigned long frame_pointer); > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 27de1dddb0ab..9e03c9a7e5c3 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -201,18 +201,6 @@ int main(void) > DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); > #endif > BLANK(); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - DEFINE(FGRET_REGS_X0, offsetof(struct fgraph_ret_regs, regs[0])); > - DEFINE(FGRET_REGS_X1, offsetof(struct fgraph_ret_regs, regs[1])); > - DEFINE(FGRET_REGS_X2, offsetof(struct fgraph_ret_regs, regs[2])); > - DEFINE(FGRET_REGS_X3, offsetof(struct fgraph_ret_regs, regs[3])); > - DEFINE(FGRET_REGS_X4, offsetof(struct fgraph_ret_regs, regs[4])); > - DEFINE(FGRET_REGS_X5, offsetof(struct fgraph_ret_regs, regs[5])); > - DEFINE(FGRET_REGS_X6, offsetof(struct fgraph_ret_regs, regs[6])); > - DEFINE(FGRET_REGS_X7, offsetof(struct fgraph_ret_regs, regs[7])); > - DEFINE(FGRET_REGS_FP, offsetof(struct fgraph_ret_regs, fp)); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); > #endif > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index f0c16640ef21..169ccf600066 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph) > * @fp is checked against the value passed by ftrace_graph_caller(). > */ > SYM_CODE_START(return_to_handler) > - /* save return value regs */ > - sub sp, sp, #FGRET_REGS_SIZE > - stp x0, x1, [sp, #FGRET_REGS_X0] > - stp x2, x3, [sp, #FGRET_REGS_X2] > - stp x4, x5, [sp, #FGRET_REGS_X4] > - stp x6, x7, [sp, #FGRET_REGS_X6] > - str x29, [sp, #FGRET_REGS_FP] // parent's fp > + /* Make room for ftrace_regs */ > + sub sp, sp, #FREGS_SIZE > + > + /* Save return value regs */ > + stp x0, x1, [sp, #FREGS_X0] > + stp x2, x3, [sp, #FREGS_X2] > + stp x4, x5, [sp, #FREGS_X4] > + stp x6, x7, [sp, #FREGS_X6] > + > + /* Save the callsite's FP */ > + str x29, [sp, #FREGS_FP] > > mov x0, sp > - bl ftrace_return_to_handler // addr = ftrace_return_to_hander(regs); > + bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs); > mov x30, x0 // restore the original return address > > - /* restore return value regs */ > - ldp x0, x1, [sp, #FGRET_REGS_X0] > - ldp x2, x3, [sp, #FGRET_REGS_X2] > - ldp x4, x5, [sp, #FGRET_REGS_X4] > - ldp x6, x7, [sp, #FGRET_REGS_X6] > - add sp, sp, #FGRET_REGS_SIZE > + /* Restore return value regs */ > + ldp x0, x1, [sp, #FREGS_X0] > + ldp x2, x3, [sp, #FREGS_X2] > + ldp x4, x5, [sp, #FREGS_X4] > + ldp x6, x7, [sp, #FREGS_X6] > + add sp, sp, #FREGS_SIZE > > ret > SYM_CODE_END(return_to_handler) > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210b52..974f08f65f63 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -131,7 +131,7 @@ config LOONGARCH > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 6f8517d59954..1a73f35ea9af 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -77,6 +77,8 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + ((fregs)->regs.regs[22]) > > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > @@ -99,26 +101,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > > #endif /* CONFIG_FUNCTION_TRACER */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* a0 - a1 */ > - unsigned long regs[2]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_LOONGARCH_FTRACE_H */ > diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c > index bee9f7a3108f..714f5b5f1956 100644 > --- a/arch/loongarch/kernel/asm-offsets.c > +++ b/arch/loongarch/kernel/asm-offsets.c > @@ -279,18 +279,6 @@ static void __used output_pbe_defines(void) > } > #endif > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -static void __used output_fgraph_ret_regs_defines(void) > -{ > - COMMENT("LoongArch fgraph_ret_regs offsets."); > - OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]); > - OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]); > - OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > - BLANK(); > -} > -#endif > - > static void __used output_kvm_defines(void) > { > COMMENT("KVM/LoongArch Specific offsets."); > diff --git a/arch/loongarch/kernel/mcount.S b/arch/loongarch/kernel/mcount.S > index 3015896016a0..b6850503e061 100644 > --- a/arch/loongarch/kernel/mcount.S > +++ b/arch/loongarch/kernel/mcount.S > @@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller) > SYM_FUNC_END(ftrace_graph_caller) > > SYM_FUNC_START(return_to_handler) > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + /* Save return value regs */ > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > @@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler) > /* Restore the real parent address: a0 -> ra */ > move ra, a0 > > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + /* Restore return value regs */ > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > + > jr ra > SYM_FUNC_END(return_to_handler) > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S > index 0c65cf09110c..d6b474ad1d5e 100644 > --- a/arch/loongarch/kernel/mcount_dyn.S > +++ b/arch/loongarch/kernel/mcount_dyn.S > @@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > /* Save return value regs */ > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > move ra, a0 > > /* Restore return value regs */ > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > > jr ra > SYM_CODE_END(return_to_handler) > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 0f3cd7c3a436..6e8422269ba4 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -140,7 +140,7 @@ config RISCV > select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > select HAVE_EBPF_JIT if MMU > select HAVE_GUP_FAST if MMU > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 2cddd79ff21b..e9f364ce9fe8 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -164,6 +164,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct > return fregs->sp; > } > > +static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->s0; > +} > + > static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, > unsigned int n) > { > @@ -204,25 +209,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long a1; > - unsigned long a0; > - unsigned long s0; > - unsigned long ra; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->a0; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->s0; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_RISCV_FTRACE_H */ > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S > index 3a42f6287909..068168046e0e 100644 > --- a/arch/riscv/kernel/mcount.S > +++ b/arch/riscv/kernel/mcount.S > @@ -12,6 +12,8 @@ > #include <asm/asm-offsets.h> > #include <asm/ftrace.h> > > +#define ABI_SIZE_ON_STACK 80 > + > .text > > .macro SAVE_ABI_STATE > @@ -26,12 +28,12 @@ > * register if a0 was not saved. > */ > .macro SAVE_RET_ABI_STATE > - addi sp, sp, -4*SZREG > - REG_S s0, 2*SZREG(sp) > - REG_S ra, 3*SZREG(sp) > - REG_S a0, 1*SZREG(sp) > - REG_S a1, 0*SZREG(sp) > - addi s0, sp, 4*SZREG > + addi sp, sp, -ABI_SIZE_ON_STACK > + REG_S ra, 1*SZREG(sp) > + REG_S s0, 8*SZREG(sp) > + REG_S a0, 10*SZREG(sp) > + REG_S a1, 11*SZREG(sp) > + addi s0, sp, ABI_SIZE_ON_STACK > .endm > > .macro RESTORE_ABI_STATE > @@ -41,11 +43,11 @@ > .endm > > .macro RESTORE_RET_ABI_STATE > - REG_L ra, 3*SZREG(sp) > - REG_L s0, 2*SZREG(sp) > - REG_L a0, 1*SZREG(sp) > - REG_L a1, 0*SZREG(sp) > - addi sp, sp, 4*SZREG > + REG_L ra, 1*SZREG(sp) > + REG_L s0, 8*SZREG(sp) > + REG_L a0, 10*SZREG(sp) > + REG_L a1, 11*SZREG(sp) > + addi sp, sp, ABI_SIZE_ON_STACK > .endm > > SYM_TYPED_FUNC_START(ftrace_stub) > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index a822f952f64a..12e942cfbcde 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -184,7 +184,7 @@ config S390 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index de76c21eb4a3..9cdd48a46bf7 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -49,23 +49,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * > return NULL; > } > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long gpr2; > - unsigned long fp; > -}; > - > -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->gpr2; > -} > - > -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > - > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > @@ -92,6 +75,15 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > +{ > + unsigned long *sp; > + > + sp = (void *)ftrace_regs_get_stack_pointer(fregs); > + return sp[0]; /* return backchain */ > +} > + > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index ffa0dd2dbaac..d38ed80615d5 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -179,12 +179,6 @@ int main(void) > DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); > DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); > DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - /* function graph return value tracing */ > - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); > - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); > DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > index ae4d4fd9afcd..cda798b976de 100644 > --- a/arch/s390/kernel/mcount.S > +++ b/arch/s390/kernel/mcount.S > @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) > SYM_FUNC_START(return_to_handler) > stmg %r2,%r5,32(%r15) > lgr %r1,%r15 > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > +# Allocate ftrace_regs + backchain on the stack > + aghi %r15,-STACK_FRAME_SIZE_FREGS > stg %r1,__SF_BACKCHAIN(%r15) > la %r3,STACK_FRAME_OVERHEAD(%r15) > - stg %r1,__FGRAPH_RET_FP(%r3) > - stg %r2,__FGRAPH_RET_GPR2(%r3) > + stg %r2,(__SF_GPRS+2*8)(%r15) > + stg %r15,(__SF_GPRS+15*8)(%r15) > lgr %r2,%r3 > brasl %r14,ftrace_return_to_handler > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > + aghi %r15,STACK_FRAME_SIZE_FREGS > lgr %r14,%r2 > lmg %r2,%r5,32(%r15) > BR_EX %r14 > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 007bab9f2a0e..047384e4d93a 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -228,7 +228,7 @@ config X86 > select HAVE_GUP_FAST > select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS if HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_TRACER if X86_32 || (X86_64 && DYNAMIC_FTRACE) > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 78f6a200e15b..669771ef3b5b 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -64,6 +64,8 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > > struct ftrace_ops; > #define ftrace_graph_func ftrace_graph_func > @@ -148,24 +150,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > #endif /* !COMPILE_OFFSETS */ > #endif /* !__ASSEMBLY__ */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long ax; > - unsigned long dx; > - unsigned long bp; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->ax; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->bp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_X86_FTRACE_H */ > diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S > index 58d9ed50fe61..4b265884d06c 100644 > --- a/arch/x86/kernel/ftrace_32.S > +++ b/arch/x86/kernel/ftrace_32.S > @@ -23,6 +23,8 @@ SYM_FUNC_START(__fentry__) > SYM_FUNC_END(__fentry__) > EXPORT_SYMBOL(__fentry__) > > +#define FRAME_SIZE PT_OLDSS+4 > + > SYM_CODE_START(ftrace_caller) > > #ifdef CONFIG_FRAME_POINTER > @@ -187,14 +189,15 @@ SYM_CODE_END(ftrace_graph_caller) > > .globl return_to_handler > return_to_handler: > - pushl $0 > - pushl %edx > - pushl %eax > + subl $(FRAME_SIZE), %esp > + movl $0, PT_EBP(%esp) > + movl %edx, PT_EDX(%esp) > + movl %eax, PT_EAX(%esp) > movl %esp, %eax > call ftrace_return_to_handler > movl %eax, %ecx > - popl %eax > - popl %edx > - addl $4, %esp # skip ebp > + movl %eax, PT_EAX(%esp) > + movl %edx, PT_EDX(%esp) > + addl $(FRAME_SIZE), %esp > JMP_NOSPEC ecx > #endif > diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S > index 214f30e9f0c0..d51647228596 100644 > --- a/arch/x86/kernel/ftrace_64.S > +++ b/arch/x86/kernel/ftrace_64.S > @@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > ANNOTATE_NOENDBR > - subq $24, %rsp > > - /* Save the return values */ > - movq %rax, (%rsp) > - movq %rdx, 8(%rsp) > - movq %rbp, 16(%rsp) > + /* Save ftrace_regs for function exit context */ > + subq $(FRAME_SIZE), %rsp > + > + movq %rax, RAX(%rsp) > + movq %rdx, RDX(%rsp) > + movq %rbp, RBP(%rsp) > movq %rsp, %rdi > > call ftrace_return_to_handler > > movq %rax, %rdi > - movq 8(%rsp), %rdx > - movq (%rsp), %rax > + movq RDX(%rsp), %rdx > + movq RAX(%rsp), %rax > > - addq $24, %rsp > + addq $(FRAME_SIZE), %rsp > /* > * Jump back to the old return address. This cannot be JMP_NOSPEC rdi > * since IBT would demand that contain ENDBR, which simply isn't so for > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 1fe49a28de2d..13987cd63553 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -43,9 +43,8 @@ struct dyn_ftrace; > > char *arch_ftrace_match_adjust(char *str, const char *search); > > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -struct fgraph_ret_regs; > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs); > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs); > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer); > #endif > @@ -134,6 +133,13 @@ extern int ftrace_enabled; > * Also, architecture dependent fields can be used for internal process. > * (e.g. orig_ax on x86_64) > * > + * Basically, ftrace_regs stores the registers related to the context. > + * On function entry, registers for function parameters and hooking the > + * function call are stored, and on function exit, registers for function > + * return value and frame pointers are stored. > + * > + * And also, it dpends on the context that which registers are restored > + * from the ftrace_regs. > * On the function entry, those registers will be restored except for > * the stack pointer, so that user can change the function parameters > * and instruction pointer (e.g. live patching.) > @@ -191,6 +197,8 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) > override_function_with_return(ftrace_get_regs(fregs)) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > #endif > > typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 721c3b221048..ab277eff80dc 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER > help > See Documentation/trace/ftrace-design.rst > > -config HAVE_FUNCTION_GRAPH_RETVAL > +config HAVE_FUNCTION_GRAPH_FREGS > bool > > config HAVE_DYNAMIC_FTRACE > @@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER > > config FUNCTION_GRAPH_RETVAL > bool "Kernel Function Graph Return Value" > - depends on HAVE_FUNCTION_GRAPH_RETVAL > + depends on HAVE_FUNCTION_GRAPH_FREGS > depends on FUNCTION_GRAPH_TRACER > default n > help > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index 0322c5723748..30bebe43607d 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -763,15 +763,12 @@ static struct notifier_block ftrace_suspend_notifier = { > .notifier_call = ftrace_suspend_notifier_call, > }; > > -/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */ > -struct fgraph_ret_regs; > - > /* > * Send the trace to the ring-buffer. > * @return the original return address. > */ > -static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs, > - unsigned long frame_pointer) > +static inline unsigned long > +__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer) > { > struct ftrace_ret_stack *ret_stack; > struct ftrace_graph_ret trace; > @@ -791,7 +788,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > > trace.rettime = trace_clock_local(); > #ifdef CONFIG_FUNCTION_GRAPH_RETVAL > - trace.retval = fgraph_ret_regs_return_value(ret_regs); > + trace.retval = ftrace_regs_get_return_value(fregs); > #endif > > bitmap = get_bitmap_bits(current, offset); > @@ -826,14 +823,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > } > > /* > - * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can > - * leave only ftrace_return_to_handler(ret_regs). > + * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can > + * leave only ftrace_return_to_handler(fregs). > */ > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs) > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs) > { > - return __ftrace_return_to_handler(ret_regs, > - fgraph_ret_regs_frame_pointer(ret_regs)); > + return __ftrace_return_to_handler(fregs, > + ftrace_regs_get_frame_pointer(fregs)); > } > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
Can I get an Acked-by from the RISC-V maintainers for this patch? Thanks! -- Steve On Fri, 13 Sep 2024 00:08:51 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > on function_graph tracer because of simplifying the callback interface. > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > Changes in v8: > - Newly added. > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/ftrace.h | 23 ++++++----------------- > arch/arm64/kernel/asm-offsets.c | 12 ------------ > arch/arm64/kernel/entry-ftrace.S | 32 ++++++++++++++++++-------------- > arch/loongarch/Kconfig | 2 +- > arch/loongarch/include/asm/ftrace.h | 24 ++---------------------- > arch/loongarch/kernel/asm-offsets.c | 12 ------------ > arch/loongarch/kernel/mcount.S | 17 ++++++++++------- > arch/loongarch/kernel/mcount_dyn.S | 14 +++++++------- > arch/riscv/Kconfig | 2 +- > arch/riscv/include/asm/ftrace.h | 26 +++++--------------------- > arch/riscv/kernel/mcount.S | 24 +++++++++++++----------- > arch/s390/Kconfig | 2 +- > arch/s390/include/asm/ftrace.h | 26 +++++++++----------------- > arch/s390/kernel/asm-offsets.c | 6 ------ > arch/s390/kernel/mcount.S | 9 +++++---- > arch/x86/Kconfig | 2 +- > arch/x86/include/asm/ftrace.h | 22 ++-------------------- > arch/x86/kernel/ftrace_32.S | 15 +++++++++------ > arch/x86/kernel/ftrace_64.S | 17 +++++++++-------- > include/linux/ftrace.h | 14 +++++++++++--- > kernel/trace/Kconfig | 4 ++-- > kernel/trace/fgraph.c | 21 +++++++++------------ > 23 files changed, 122 insertions(+), 205 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a2f8ff354ca6..17947f625b06 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -211,6 +211,7 @@ config ARM64 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > select HAVE_FUNCTION_ERROR_INJECTION > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_RETVAL > select HAVE_GCC_PLUGINS > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index dc9cf0bd2a4c..dffaab3dd1f1 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -126,6 +126,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs) > fregs->pc = fregs->lr; > } > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->fp; > +} > + > int ftrace_regs_query_register_offset(const char *name); > > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > @@ -183,23 +189,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym, > > #ifndef __ASSEMBLY__ > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* x0 - x7 */ > - unsigned long regs[8]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > > void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, > unsigned long frame_pointer); > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 27de1dddb0ab..9e03c9a7e5c3 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -201,18 +201,6 @@ int main(void) > DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); > #endif > BLANK(); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - DEFINE(FGRET_REGS_X0, offsetof(struct fgraph_ret_regs, regs[0])); > - DEFINE(FGRET_REGS_X1, offsetof(struct fgraph_ret_regs, regs[1])); > - DEFINE(FGRET_REGS_X2, offsetof(struct fgraph_ret_regs, regs[2])); > - DEFINE(FGRET_REGS_X3, offsetof(struct fgraph_ret_regs, regs[3])); > - DEFINE(FGRET_REGS_X4, offsetof(struct fgraph_ret_regs, regs[4])); > - DEFINE(FGRET_REGS_X5, offsetof(struct fgraph_ret_regs, regs[5])); > - DEFINE(FGRET_REGS_X6, offsetof(struct fgraph_ret_regs, regs[6])); > - DEFINE(FGRET_REGS_X7, offsetof(struct fgraph_ret_regs, regs[7])); > - DEFINE(FGRET_REGS_FP, offsetof(struct fgraph_ret_regs, fp)); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); > #endif > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index f0c16640ef21..169ccf600066 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph) > * @fp is checked against the value passed by ftrace_graph_caller(). > */ > SYM_CODE_START(return_to_handler) > - /* save return value regs */ > - sub sp, sp, #FGRET_REGS_SIZE > - stp x0, x1, [sp, #FGRET_REGS_X0] > - stp x2, x3, [sp, #FGRET_REGS_X2] > - stp x4, x5, [sp, #FGRET_REGS_X4] > - stp x6, x7, [sp, #FGRET_REGS_X6] > - str x29, [sp, #FGRET_REGS_FP] // parent's fp > + /* Make room for ftrace_regs */ > + sub sp, sp, #FREGS_SIZE > + > + /* Save return value regs */ > + stp x0, x1, [sp, #FREGS_X0] > + stp x2, x3, [sp, #FREGS_X2] > + stp x4, x5, [sp, #FREGS_X4] > + stp x6, x7, [sp, #FREGS_X6] > + > + /* Save the callsite's FP */ > + str x29, [sp, #FREGS_FP] > > mov x0, sp > - bl ftrace_return_to_handler // addr = ftrace_return_to_hander(regs); > + bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs); > mov x30, x0 // restore the original return address > > - /* restore return value regs */ > - ldp x0, x1, [sp, #FGRET_REGS_X0] > - ldp x2, x3, [sp, #FGRET_REGS_X2] > - ldp x4, x5, [sp, #FGRET_REGS_X4] > - ldp x6, x7, [sp, #FGRET_REGS_X6] > - add sp, sp, #FGRET_REGS_SIZE > + /* Restore return value regs */ > + ldp x0, x1, [sp, #FREGS_X0] > + ldp x2, x3, [sp, #FREGS_X2] > + ldp x4, x5, [sp, #FREGS_X4] > + ldp x6, x7, [sp, #FREGS_X6] > + add sp, sp, #FREGS_SIZE > > ret > SYM_CODE_END(return_to_handler) > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210b52..974f08f65f63 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -131,7 +131,7 @@ config LOONGARCH > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 6f8517d59954..1a73f35ea9af 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -77,6 +77,8 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + ((fregs)->regs.regs[22]) > > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > @@ -99,26 +101,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > > #endif /* CONFIG_FUNCTION_TRACER */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* a0 - a1 */ > - unsigned long regs[2]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_LOONGARCH_FTRACE_H */ > diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c > index bee9f7a3108f..714f5b5f1956 100644 > --- a/arch/loongarch/kernel/asm-offsets.c > +++ b/arch/loongarch/kernel/asm-offsets.c > @@ -279,18 +279,6 @@ static void __used output_pbe_defines(void) > } > #endif > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -static void __used output_fgraph_ret_regs_defines(void) > -{ > - COMMENT("LoongArch fgraph_ret_regs offsets."); > - OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]); > - OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]); > - OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > - BLANK(); > -} > -#endif > - > static void __used output_kvm_defines(void) > { > COMMENT("KVM/LoongArch Specific offsets."); > diff --git a/arch/loongarch/kernel/mcount.S b/arch/loongarch/kernel/mcount.S > index 3015896016a0..b6850503e061 100644 > --- a/arch/loongarch/kernel/mcount.S > +++ b/arch/loongarch/kernel/mcount.S > @@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller) > SYM_FUNC_END(ftrace_graph_caller) > > SYM_FUNC_START(return_to_handler) > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + /* Save return value regs */ > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > @@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler) > /* Restore the real parent address: a0 -> ra */ > move ra, a0 > > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + /* Restore return value regs */ > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > + > jr ra > SYM_FUNC_END(return_to_handler) > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S > index 0c65cf09110c..d6b474ad1d5e 100644 > --- a/arch/loongarch/kernel/mcount_dyn.S > +++ b/arch/loongarch/kernel/mcount_dyn.S > @@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > /* Save return value regs */ > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > move ra, a0 > > /* Restore return value regs */ > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > > jr ra > SYM_CODE_END(return_to_handler) > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 0f3cd7c3a436..6e8422269ba4 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -140,7 +140,7 @@ config RISCV > select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > select HAVE_EBPF_JIT if MMU > select HAVE_GUP_FAST if MMU > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 2cddd79ff21b..e9f364ce9fe8 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -164,6 +164,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct > return fregs->sp; > } > > +static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->s0; > +} > + > static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, > unsigned int n) > { > @@ -204,25 +209,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long a1; > - unsigned long a0; > - unsigned long s0; > - unsigned long ra; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->a0; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->s0; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_RISCV_FTRACE_H */ > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S > index 3a42f6287909..068168046e0e 100644 > --- a/arch/riscv/kernel/mcount.S > +++ b/arch/riscv/kernel/mcount.S > @@ -12,6 +12,8 @@ > #include <asm/asm-offsets.h> > #include <asm/ftrace.h> > > +#define ABI_SIZE_ON_STACK 80 > + > .text > > .macro SAVE_ABI_STATE > @@ -26,12 +28,12 @@ > * register if a0 was not saved. > */ > .macro SAVE_RET_ABI_STATE > - addi sp, sp, -4*SZREG > - REG_S s0, 2*SZREG(sp) > - REG_S ra, 3*SZREG(sp) > - REG_S a0, 1*SZREG(sp) > - REG_S a1, 0*SZREG(sp) > - addi s0, sp, 4*SZREG > + addi sp, sp, -ABI_SIZE_ON_STACK > + REG_S ra, 1*SZREG(sp) > + REG_S s0, 8*SZREG(sp) > + REG_S a0, 10*SZREG(sp) > + REG_S a1, 11*SZREG(sp) > + addi s0, sp, ABI_SIZE_ON_STACK > .endm > > .macro RESTORE_ABI_STATE > @@ -41,11 +43,11 @@ > .endm > > .macro RESTORE_RET_ABI_STATE > - REG_L ra, 3*SZREG(sp) > - REG_L s0, 2*SZREG(sp) > - REG_L a0, 1*SZREG(sp) > - REG_L a1, 0*SZREG(sp) > - addi sp, sp, 4*SZREG > + REG_L ra, 1*SZREG(sp) > + REG_L s0, 8*SZREG(sp) > + REG_L a0, 10*SZREG(sp) > + REG_L a1, 11*SZREG(sp) > + addi sp, sp, ABI_SIZE_ON_STACK > .endm > > SYM_TYPED_FUNC_START(ftrace_stub) > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index a822f952f64a..12e942cfbcde 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -184,7 +184,7 @@ config S390 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index de76c21eb4a3..9cdd48a46bf7 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -49,23 +49,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * > return NULL; > } > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long gpr2; > - unsigned long fp; > -}; > - > -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->gpr2; > -} > - > -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > - > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > @@ -92,6 +75,15 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > +{ > + unsigned long *sp; > + > + sp = (void *)ftrace_regs_get_stack_pointer(fregs); > + return sp[0]; /* return backchain */ > +} > + > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index ffa0dd2dbaac..d38ed80615d5 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -179,12 +179,6 @@ int main(void) > DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); > DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); > DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - /* function graph return value tracing */ > - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); > - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); > DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > index ae4d4fd9afcd..cda798b976de 100644 > --- a/arch/s390/kernel/mcount.S > +++ b/arch/s390/kernel/mcount.S > @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) > SYM_FUNC_START(return_to_handler) > stmg %r2,%r5,32(%r15) > lgr %r1,%r15 > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > +# Allocate ftrace_regs + backchain on the stack > + aghi %r15,-STACK_FRAME_SIZE_FREGS > stg %r1,__SF_BACKCHAIN(%r15) > la %r3,STACK_FRAME_OVERHEAD(%r15) > - stg %r1,__FGRAPH_RET_FP(%r3) > - stg %r2,__FGRAPH_RET_GPR2(%r3) > + stg %r2,(__SF_GPRS+2*8)(%r15) > + stg %r15,(__SF_GPRS+15*8)(%r15) > lgr %r2,%r3 > brasl %r14,ftrace_return_to_handler > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > + aghi %r15,STACK_FRAME_SIZE_FREGS > lgr %r14,%r2 > lmg %r2,%r5,32(%r15) > BR_EX %r14 > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 007bab9f2a0e..047384e4d93a 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -228,7 +228,7 @@ config X86 > select HAVE_GUP_FAST > select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS if HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_TRACER if X86_32 || (X86_64 && DYNAMIC_FTRACE) > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 78f6a200e15b..669771ef3b5b 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -64,6 +64,8 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > > struct ftrace_ops; > #define ftrace_graph_func ftrace_graph_func > @@ -148,24 +150,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > #endif /* !COMPILE_OFFSETS */ > #endif /* !__ASSEMBLY__ */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long ax; > - unsigned long dx; > - unsigned long bp; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->ax; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->bp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_X86_FTRACE_H */ > diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S > index 58d9ed50fe61..4b265884d06c 100644 > --- a/arch/x86/kernel/ftrace_32.S > +++ b/arch/x86/kernel/ftrace_32.S > @@ -23,6 +23,8 @@ SYM_FUNC_START(__fentry__) > SYM_FUNC_END(__fentry__) > EXPORT_SYMBOL(__fentry__) > > +#define FRAME_SIZE PT_OLDSS+4 > + > SYM_CODE_START(ftrace_caller) > > #ifdef CONFIG_FRAME_POINTER > @@ -187,14 +189,15 @@ SYM_CODE_END(ftrace_graph_caller) > > .globl return_to_handler > return_to_handler: > - pushl $0 > - pushl %edx > - pushl %eax > + subl $(FRAME_SIZE), %esp > + movl $0, PT_EBP(%esp) > + movl %edx, PT_EDX(%esp) > + movl %eax, PT_EAX(%esp) > movl %esp, %eax > call ftrace_return_to_handler > movl %eax, %ecx > - popl %eax > - popl %edx > - addl $4, %esp # skip ebp > + movl %eax, PT_EAX(%esp) > + movl %edx, PT_EDX(%esp) > + addl $(FRAME_SIZE), %esp > JMP_NOSPEC ecx > #endif > diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S > index 214f30e9f0c0..d51647228596 100644 > --- a/arch/x86/kernel/ftrace_64.S > +++ b/arch/x86/kernel/ftrace_64.S > @@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > ANNOTATE_NOENDBR > - subq $24, %rsp > > - /* Save the return values */ > - movq %rax, (%rsp) > - movq %rdx, 8(%rsp) > - movq %rbp, 16(%rsp) > + /* Save ftrace_regs for function exit context */ > + subq $(FRAME_SIZE), %rsp > + > + movq %rax, RAX(%rsp) > + movq %rdx, RDX(%rsp) > + movq %rbp, RBP(%rsp) > movq %rsp, %rdi > > call ftrace_return_to_handler > > movq %rax, %rdi > - movq 8(%rsp), %rdx > - movq (%rsp), %rax > + movq RDX(%rsp), %rdx > + movq RAX(%rsp), %rax > > - addq $24, %rsp > + addq $(FRAME_SIZE), %rsp > /* > * Jump back to the old return address. This cannot be JMP_NOSPEC rdi > * since IBT would demand that contain ENDBR, which simply isn't so for > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 1fe49a28de2d..13987cd63553 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -43,9 +43,8 @@ struct dyn_ftrace; > > char *arch_ftrace_match_adjust(char *str, const char *search); > > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -struct fgraph_ret_regs; > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs); > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs); > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer); > #endif > @@ -134,6 +133,13 @@ extern int ftrace_enabled; > * Also, architecture dependent fields can be used for internal process. > * (e.g. orig_ax on x86_64) > * > + * Basically, ftrace_regs stores the registers related to the context. > + * On function entry, registers for function parameters and hooking the > + * function call are stored, and on function exit, registers for function > + * return value and frame pointers are stored. > + * > + * And also, it dpends on the context that which registers are restored > + * from the ftrace_regs. > * On the function entry, those registers will be restored except for > * the stack pointer, so that user can change the function parameters > * and instruction pointer (e.g. live patching.) > @@ -191,6 +197,8 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) > override_function_with_return(ftrace_get_regs(fregs)) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > #endif > > typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 721c3b221048..ab277eff80dc 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER > help > See Documentation/trace/ftrace-design.rst > > -config HAVE_FUNCTION_GRAPH_RETVAL > +config HAVE_FUNCTION_GRAPH_FREGS > bool > > config HAVE_DYNAMIC_FTRACE > @@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER > > config FUNCTION_GRAPH_RETVAL > bool "Kernel Function Graph Return Value" > - depends on HAVE_FUNCTION_GRAPH_RETVAL > + depends on HAVE_FUNCTION_GRAPH_FREGS > depends on FUNCTION_GRAPH_TRACER > default n > help > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index 0322c5723748..30bebe43607d 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -763,15 +763,12 @@ static struct notifier_block ftrace_suspend_notifier = { > .notifier_call = ftrace_suspend_notifier_call, > }; > > -/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */ > -struct fgraph_ret_regs; > - > /* > * Send the trace to the ring-buffer. > * @return the original return address. > */ > -static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs, > - unsigned long frame_pointer) > +static inline unsigned long > +__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer) > { > struct ftrace_ret_stack *ret_stack; > struct ftrace_graph_ret trace; > @@ -791,7 +788,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > > trace.rettime = trace_clock_local(); > #ifdef CONFIG_FUNCTION_GRAPH_RETVAL > - trace.retval = fgraph_ret_regs_return_value(ret_regs); > + trace.retval = ftrace_regs_get_return_value(fregs); > #endif > > bitmap = get_bitmap_bits(current, offset); > @@ -826,14 +823,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > } > > /* > - * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can > - * leave only ftrace_return_to_handler(ret_regs). > + * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can > + * leave only ftrace_return_to_handler(fregs). > */ > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs) > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs) > { > - return __ftrace_return_to_handler(ret_regs, > - fgraph_ret_regs_frame_pointer(ret_regs)); > + return __ftrace_return_to_handler(fregs, > + ftrace_regs_get_frame_pointer(fregs)); > } > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
Can I get an Acked-by from the S390 maintainers for this patch? Thanks! -- Steve On Fri, 13 Sep 2024 00:08:51 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > on function_graph tracer because of simplifying the callback interface. > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > Changes in v8: > - Newly added. > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/ftrace.h | 23 ++++++----------------- > arch/arm64/kernel/asm-offsets.c | 12 ------------ > arch/arm64/kernel/entry-ftrace.S | 32 ++++++++++++++++++-------------- > arch/loongarch/Kconfig | 2 +- > arch/loongarch/include/asm/ftrace.h | 24 ++---------------------- > arch/loongarch/kernel/asm-offsets.c | 12 ------------ > arch/loongarch/kernel/mcount.S | 17 ++++++++++------- > arch/loongarch/kernel/mcount_dyn.S | 14 +++++++------- > arch/riscv/Kconfig | 2 +- > arch/riscv/include/asm/ftrace.h | 26 +++++--------------------- > arch/riscv/kernel/mcount.S | 24 +++++++++++++----------- > arch/s390/Kconfig | 2 +- > arch/s390/include/asm/ftrace.h | 26 +++++++++----------------- > arch/s390/kernel/asm-offsets.c | 6 ------ > arch/s390/kernel/mcount.S | 9 +++++---- > arch/x86/Kconfig | 2 +- > arch/x86/include/asm/ftrace.h | 22 ++-------------------- > arch/x86/kernel/ftrace_32.S | 15 +++++++++------ > arch/x86/kernel/ftrace_64.S | 17 +++++++++-------- > include/linux/ftrace.h | 14 +++++++++++--- > kernel/trace/Kconfig | 4 ++-- > kernel/trace/fgraph.c | 21 +++++++++------------ > 23 files changed, 122 insertions(+), 205 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a2f8ff354ca6..17947f625b06 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -211,6 +211,7 @@ config ARM64 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > select HAVE_FUNCTION_ERROR_INJECTION > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_RETVAL > select HAVE_GCC_PLUGINS > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index dc9cf0bd2a4c..dffaab3dd1f1 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -126,6 +126,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs) > fregs->pc = fregs->lr; > } > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->fp; > +} > + > int ftrace_regs_query_register_offset(const char *name); > > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > @@ -183,23 +189,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym, > > #ifndef __ASSEMBLY__ > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* x0 - x7 */ > - unsigned long regs[8]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > > void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, > unsigned long frame_pointer); > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 27de1dddb0ab..9e03c9a7e5c3 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -201,18 +201,6 @@ int main(void) > DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); > #endif > BLANK(); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - DEFINE(FGRET_REGS_X0, offsetof(struct fgraph_ret_regs, regs[0])); > - DEFINE(FGRET_REGS_X1, offsetof(struct fgraph_ret_regs, regs[1])); > - DEFINE(FGRET_REGS_X2, offsetof(struct fgraph_ret_regs, regs[2])); > - DEFINE(FGRET_REGS_X3, offsetof(struct fgraph_ret_regs, regs[3])); > - DEFINE(FGRET_REGS_X4, offsetof(struct fgraph_ret_regs, regs[4])); > - DEFINE(FGRET_REGS_X5, offsetof(struct fgraph_ret_regs, regs[5])); > - DEFINE(FGRET_REGS_X6, offsetof(struct fgraph_ret_regs, regs[6])); > - DEFINE(FGRET_REGS_X7, offsetof(struct fgraph_ret_regs, regs[7])); > - DEFINE(FGRET_REGS_FP, offsetof(struct fgraph_ret_regs, fp)); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); > #endif > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index f0c16640ef21..169ccf600066 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph) > * @fp is checked against the value passed by ftrace_graph_caller(). > */ > SYM_CODE_START(return_to_handler) > - /* save return value regs */ > - sub sp, sp, #FGRET_REGS_SIZE > - stp x0, x1, [sp, #FGRET_REGS_X0] > - stp x2, x3, [sp, #FGRET_REGS_X2] > - stp x4, x5, [sp, #FGRET_REGS_X4] > - stp x6, x7, [sp, #FGRET_REGS_X6] > - str x29, [sp, #FGRET_REGS_FP] // parent's fp > + /* Make room for ftrace_regs */ > + sub sp, sp, #FREGS_SIZE > + > + /* Save return value regs */ > + stp x0, x1, [sp, #FREGS_X0] > + stp x2, x3, [sp, #FREGS_X2] > + stp x4, x5, [sp, #FREGS_X4] > + stp x6, x7, [sp, #FREGS_X6] > + > + /* Save the callsite's FP */ > + str x29, [sp, #FREGS_FP] > > mov x0, sp > - bl ftrace_return_to_handler // addr = ftrace_return_to_hander(regs); > + bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs); > mov x30, x0 // restore the original return address > > - /* restore return value regs */ > - ldp x0, x1, [sp, #FGRET_REGS_X0] > - ldp x2, x3, [sp, #FGRET_REGS_X2] > - ldp x4, x5, [sp, #FGRET_REGS_X4] > - ldp x6, x7, [sp, #FGRET_REGS_X6] > - add sp, sp, #FGRET_REGS_SIZE > + /* Restore return value regs */ > + ldp x0, x1, [sp, #FREGS_X0] > + ldp x2, x3, [sp, #FREGS_X2] > + ldp x4, x5, [sp, #FREGS_X4] > + ldp x6, x7, [sp, #FREGS_X6] > + add sp, sp, #FREGS_SIZE > > ret > SYM_CODE_END(return_to_handler) > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210b52..974f08f65f63 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -131,7 +131,7 @@ config LOONGARCH > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 6f8517d59954..1a73f35ea9af 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -77,6 +77,8 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + ((fregs)->regs.regs[22]) > > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > @@ -99,26 +101,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > > #endif /* CONFIG_FUNCTION_TRACER */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* a0 - a1 */ > - unsigned long regs[2]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_LOONGARCH_FTRACE_H */ > diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c > index bee9f7a3108f..714f5b5f1956 100644 > --- a/arch/loongarch/kernel/asm-offsets.c > +++ b/arch/loongarch/kernel/asm-offsets.c > @@ -279,18 +279,6 @@ static void __used output_pbe_defines(void) > } > #endif > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -static void __used output_fgraph_ret_regs_defines(void) > -{ > - COMMENT("LoongArch fgraph_ret_regs offsets."); > - OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]); > - OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]); > - OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > - BLANK(); > -} > -#endif > - > static void __used output_kvm_defines(void) > { > COMMENT("KVM/LoongArch Specific offsets."); > diff --git a/arch/loongarch/kernel/mcount.S b/arch/loongarch/kernel/mcount.S > index 3015896016a0..b6850503e061 100644 > --- a/arch/loongarch/kernel/mcount.S > +++ b/arch/loongarch/kernel/mcount.S > @@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller) > SYM_FUNC_END(ftrace_graph_caller) > > SYM_FUNC_START(return_to_handler) > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + /* Save return value regs */ > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > @@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler) > /* Restore the real parent address: a0 -> ra */ > move ra, a0 > > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + /* Restore return value regs */ > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > + > jr ra > SYM_FUNC_END(return_to_handler) > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S > index 0c65cf09110c..d6b474ad1d5e 100644 > --- a/arch/loongarch/kernel/mcount_dyn.S > +++ b/arch/loongarch/kernel/mcount_dyn.S > @@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > /* Save return value regs */ > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > move ra, a0 > > /* Restore return value regs */ > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > > jr ra > SYM_CODE_END(return_to_handler) > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 0f3cd7c3a436..6e8422269ba4 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -140,7 +140,7 @@ config RISCV > select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > select HAVE_EBPF_JIT if MMU > select HAVE_GUP_FAST if MMU > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 2cddd79ff21b..e9f364ce9fe8 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -164,6 +164,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct > return fregs->sp; > } > > +static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->s0; > +} > + > static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, > unsigned int n) > { > @@ -204,25 +209,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long a1; > - unsigned long a0; > - unsigned long s0; > - unsigned long ra; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->a0; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->s0; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_RISCV_FTRACE_H */ > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S > index 3a42f6287909..068168046e0e 100644 > --- a/arch/riscv/kernel/mcount.S > +++ b/arch/riscv/kernel/mcount.S > @@ -12,6 +12,8 @@ > #include <asm/asm-offsets.h> > #include <asm/ftrace.h> > > +#define ABI_SIZE_ON_STACK 80 > + > .text > > .macro SAVE_ABI_STATE > @@ -26,12 +28,12 @@ > * register if a0 was not saved. > */ > .macro SAVE_RET_ABI_STATE > - addi sp, sp, -4*SZREG > - REG_S s0, 2*SZREG(sp) > - REG_S ra, 3*SZREG(sp) > - REG_S a0, 1*SZREG(sp) > - REG_S a1, 0*SZREG(sp) > - addi s0, sp, 4*SZREG > + addi sp, sp, -ABI_SIZE_ON_STACK > + REG_S ra, 1*SZREG(sp) > + REG_S s0, 8*SZREG(sp) > + REG_S a0, 10*SZREG(sp) > + REG_S a1, 11*SZREG(sp) > + addi s0, sp, ABI_SIZE_ON_STACK > .endm > > .macro RESTORE_ABI_STATE > @@ -41,11 +43,11 @@ > .endm > > .macro RESTORE_RET_ABI_STATE > - REG_L ra, 3*SZREG(sp) > - REG_L s0, 2*SZREG(sp) > - REG_L a0, 1*SZREG(sp) > - REG_L a1, 0*SZREG(sp) > - addi sp, sp, 4*SZREG > + REG_L ra, 1*SZREG(sp) > + REG_L s0, 8*SZREG(sp) > + REG_L a0, 10*SZREG(sp) > + REG_L a1, 11*SZREG(sp) > + addi sp, sp, ABI_SIZE_ON_STACK > .endm > > SYM_TYPED_FUNC_START(ftrace_stub) > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index a822f952f64a..12e942cfbcde 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -184,7 +184,7 @@ config S390 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index de76c21eb4a3..9cdd48a46bf7 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -49,23 +49,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * > return NULL; > } > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long gpr2; > - unsigned long fp; > -}; > - > -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->gpr2; > -} > - > -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > - > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > @@ -92,6 +75,15 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > +{ > + unsigned long *sp; > + > + sp = (void *)ftrace_regs_get_stack_pointer(fregs); > + return sp[0]; /* return backchain */ > +} > + > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index ffa0dd2dbaac..d38ed80615d5 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -179,12 +179,6 @@ int main(void) > DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); > DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); > DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - /* function graph return value tracing */ > - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); > - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); > DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > index ae4d4fd9afcd..cda798b976de 100644 > --- a/arch/s390/kernel/mcount.S > +++ b/arch/s390/kernel/mcount.S > @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) > SYM_FUNC_START(return_to_handler) > stmg %r2,%r5,32(%r15) > lgr %r1,%r15 > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > +# Allocate ftrace_regs + backchain on the stack > + aghi %r15,-STACK_FRAME_SIZE_FREGS > stg %r1,__SF_BACKCHAIN(%r15) > la %r3,STACK_FRAME_OVERHEAD(%r15) > - stg %r1,__FGRAPH_RET_FP(%r3) > - stg %r2,__FGRAPH_RET_GPR2(%r3) > + stg %r2,(__SF_GPRS+2*8)(%r15) > + stg %r15,(__SF_GPRS+15*8)(%r15) > lgr %r2,%r3 > brasl %r14,ftrace_return_to_handler > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > + aghi %r15,STACK_FRAME_SIZE_FREGS > lgr %r14,%r2 > lmg %r2,%r5,32(%r15) > BR_EX %r14 > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 007bab9f2a0e..047384e4d93a 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -228,7 +228,7 @@ config X86 > select HAVE_GUP_FAST > select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS if HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_TRACER if X86_32 || (X86_64 && DYNAMIC_FTRACE) > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 78f6a200e15b..669771ef3b5b 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -64,6 +64,8 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > > struct ftrace_ops; > #define ftrace_graph_func ftrace_graph_func > @@ -148,24 +150,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > #endif /* !COMPILE_OFFSETS */ > #endif /* !__ASSEMBLY__ */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long ax; > - unsigned long dx; > - unsigned long bp; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->ax; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->bp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_X86_FTRACE_H */ > diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S > index 58d9ed50fe61..4b265884d06c 100644 > --- a/arch/x86/kernel/ftrace_32.S > +++ b/arch/x86/kernel/ftrace_32.S > @@ -23,6 +23,8 @@ SYM_FUNC_START(__fentry__) > SYM_FUNC_END(__fentry__) > EXPORT_SYMBOL(__fentry__) > > +#define FRAME_SIZE PT_OLDSS+4 > + > SYM_CODE_START(ftrace_caller) > > #ifdef CONFIG_FRAME_POINTER > @@ -187,14 +189,15 @@ SYM_CODE_END(ftrace_graph_caller) > > .globl return_to_handler > return_to_handler: > - pushl $0 > - pushl %edx > - pushl %eax > + subl $(FRAME_SIZE), %esp > + movl $0, PT_EBP(%esp) > + movl %edx, PT_EDX(%esp) > + movl %eax, PT_EAX(%esp) > movl %esp, %eax > call ftrace_return_to_handler > movl %eax, %ecx > - popl %eax > - popl %edx > - addl $4, %esp # skip ebp > + movl %eax, PT_EAX(%esp) > + movl %edx, PT_EDX(%esp) > + addl $(FRAME_SIZE), %esp > JMP_NOSPEC ecx > #endif > diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S > index 214f30e9f0c0..d51647228596 100644 > --- a/arch/x86/kernel/ftrace_64.S > +++ b/arch/x86/kernel/ftrace_64.S > @@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > ANNOTATE_NOENDBR > - subq $24, %rsp > > - /* Save the return values */ > - movq %rax, (%rsp) > - movq %rdx, 8(%rsp) > - movq %rbp, 16(%rsp) > + /* Save ftrace_regs for function exit context */ > + subq $(FRAME_SIZE), %rsp > + > + movq %rax, RAX(%rsp) > + movq %rdx, RDX(%rsp) > + movq %rbp, RBP(%rsp) > movq %rsp, %rdi > > call ftrace_return_to_handler > > movq %rax, %rdi > - movq 8(%rsp), %rdx > - movq (%rsp), %rax > + movq RDX(%rsp), %rdx > + movq RAX(%rsp), %rax > > - addq $24, %rsp > + addq $(FRAME_SIZE), %rsp > /* > * Jump back to the old return address. This cannot be JMP_NOSPEC rdi > * since IBT would demand that contain ENDBR, which simply isn't so for > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 1fe49a28de2d..13987cd63553 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -43,9 +43,8 @@ struct dyn_ftrace; > > char *arch_ftrace_match_adjust(char *str, const char *search); > > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -struct fgraph_ret_regs; > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs); > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs); > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer); > #endif > @@ -134,6 +133,13 @@ extern int ftrace_enabled; > * Also, architecture dependent fields can be used for internal process. > * (e.g. orig_ax on x86_64) > * > + * Basically, ftrace_regs stores the registers related to the context. > + * On function entry, registers for function parameters and hooking the > + * function call are stored, and on function exit, registers for function > + * return value and frame pointers are stored. > + * > + * And also, it dpends on the context that which registers are restored > + * from the ftrace_regs. > * On the function entry, those registers will be restored except for > * the stack pointer, so that user can change the function parameters > * and instruction pointer (e.g. live patching.) > @@ -191,6 +197,8 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) > override_function_with_return(ftrace_get_regs(fregs)) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > #endif > > typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 721c3b221048..ab277eff80dc 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER > help > See Documentation/trace/ftrace-design.rst > > -config HAVE_FUNCTION_GRAPH_RETVAL > +config HAVE_FUNCTION_GRAPH_FREGS > bool > > config HAVE_DYNAMIC_FTRACE > @@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER > > config FUNCTION_GRAPH_RETVAL > bool "Kernel Function Graph Return Value" > - depends on HAVE_FUNCTION_GRAPH_RETVAL > + depends on HAVE_FUNCTION_GRAPH_FREGS > depends on FUNCTION_GRAPH_TRACER > default n > help > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index 0322c5723748..30bebe43607d 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -763,15 +763,12 @@ static struct notifier_block ftrace_suspend_notifier = { > .notifier_call = ftrace_suspend_notifier_call, > }; > > -/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */ > -struct fgraph_ret_regs; > - > /* > * Send the trace to the ring-buffer. > * @return the original return address. > */ > -static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs, > - unsigned long frame_pointer) > +static inline unsigned long > +__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer) > { > struct ftrace_ret_stack *ret_stack; > struct ftrace_graph_ret trace; > @@ -791,7 +788,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > > trace.rettime = trace_clock_local(); > #ifdef CONFIG_FUNCTION_GRAPH_RETVAL > - trace.retval = fgraph_ret_regs_return_value(ret_regs); > + trace.retval = ftrace_regs_get_return_value(fregs); > #endif > > bitmap = get_bitmap_bits(current, offset); > @@ -826,14 +823,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > } > > /* > - * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can > - * leave only ftrace_return_to_handler(ret_regs). > + * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can > + * leave only ftrace_return_to_handler(fregs). > */ > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs) > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs) > { > - return __ftrace_return_to_handler(ret_regs, > - fgraph_ret_regs_frame_pointer(ret_regs)); > + return __ftrace_return_to_handler(fregs, > + ftrace_regs_get_frame_pointer(fregs)); > } > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
Can I get an Acked-by from the X86 maintainers for this patch? Thanks! -- Steve On Fri, 13 Sep 2024 00:08:51 +0900 "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > on function_graph tracer because of simplifying the callback interface. > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > --- > Changes in v8: > - Newly added. > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/ftrace.h | 23 ++++++----------------- > arch/arm64/kernel/asm-offsets.c | 12 ------------ > arch/arm64/kernel/entry-ftrace.S | 32 ++++++++++++++++++-------------- > arch/loongarch/Kconfig | 2 +- > arch/loongarch/include/asm/ftrace.h | 24 ++---------------------- > arch/loongarch/kernel/asm-offsets.c | 12 ------------ > arch/loongarch/kernel/mcount.S | 17 ++++++++++------- > arch/loongarch/kernel/mcount_dyn.S | 14 +++++++------- > arch/riscv/Kconfig | 2 +- > arch/riscv/include/asm/ftrace.h | 26 +++++--------------------- > arch/riscv/kernel/mcount.S | 24 +++++++++++++----------- > arch/s390/Kconfig | 2 +- > arch/s390/include/asm/ftrace.h | 26 +++++++++----------------- > arch/s390/kernel/asm-offsets.c | 6 ------ > arch/s390/kernel/mcount.S | 9 +++++---- > arch/x86/Kconfig | 2 +- > arch/x86/include/asm/ftrace.h | 22 ++-------------------- > arch/x86/kernel/ftrace_32.S | 15 +++++++++------ > arch/x86/kernel/ftrace_64.S | 17 +++++++++-------- > include/linux/ftrace.h | 14 +++++++++++--- > kernel/trace/Kconfig | 4 ++-- > kernel/trace/fgraph.c | 21 +++++++++------------ > 23 files changed, 122 insertions(+), 205 deletions(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index a2f8ff354ca6..17947f625b06 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -211,6 +211,7 @@ config ARM64 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > select HAVE_FUNCTION_ERROR_INJECTION > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_RETVAL > select HAVE_GCC_PLUGINS > diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h > index dc9cf0bd2a4c..dffaab3dd1f1 100644 > --- a/arch/arm64/include/asm/ftrace.h > +++ b/arch/arm64/include/asm/ftrace.h > @@ -126,6 +126,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs) > fregs->pc = fregs->lr; > } > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->fp; > +} > + > int ftrace_regs_query_register_offset(const char *name); > > int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); > @@ -183,23 +189,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym, > > #ifndef __ASSEMBLY__ > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* x0 - x7 */ > - unsigned long regs[8]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > > void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, > unsigned long frame_pointer); > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c > index 27de1dddb0ab..9e03c9a7e5c3 100644 > --- a/arch/arm64/kernel/asm-offsets.c > +++ b/arch/arm64/kernel/asm-offsets.c > @@ -201,18 +201,6 @@ int main(void) > DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); > #endif > BLANK(); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - DEFINE(FGRET_REGS_X0, offsetof(struct fgraph_ret_regs, regs[0])); > - DEFINE(FGRET_REGS_X1, offsetof(struct fgraph_ret_regs, regs[1])); > - DEFINE(FGRET_REGS_X2, offsetof(struct fgraph_ret_regs, regs[2])); > - DEFINE(FGRET_REGS_X3, offsetof(struct fgraph_ret_regs, regs[3])); > - DEFINE(FGRET_REGS_X4, offsetof(struct fgraph_ret_regs, regs[4])); > - DEFINE(FGRET_REGS_X5, offsetof(struct fgraph_ret_regs, regs[5])); > - DEFINE(FGRET_REGS_X6, offsetof(struct fgraph_ret_regs, regs[6])); > - DEFINE(FGRET_REGS_X7, offsetof(struct fgraph_ret_regs, regs[7])); > - DEFINE(FGRET_REGS_FP, offsetof(struct fgraph_ret_regs, fp)); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); > #endif > diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S > index f0c16640ef21..169ccf600066 100644 > --- a/arch/arm64/kernel/entry-ftrace.S > +++ b/arch/arm64/kernel/entry-ftrace.S > @@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph) > * @fp is checked against the value passed by ftrace_graph_caller(). > */ > SYM_CODE_START(return_to_handler) > - /* save return value regs */ > - sub sp, sp, #FGRET_REGS_SIZE > - stp x0, x1, [sp, #FGRET_REGS_X0] > - stp x2, x3, [sp, #FGRET_REGS_X2] > - stp x4, x5, [sp, #FGRET_REGS_X4] > - stp x6, x7, [sp, #FGRET_REGS_X6] > - str x29, [sp, #FGRET_REGS_FP] // parent's fp > + /* Make room for ftrace_regs */ > + sub sp, sp, #FREGS_SIZE > + > + /* Save return value regs */ > + stp x0, x1, [sp, #FREGS_X0] > + stp x2, x3, [sp, #FREGS_X2] > + stp x4, x5, [sp, #FREGS_X4] > + stp x6, x7, [sp, #FREGS_X6] > + > + /* Save the callsite's FP */ > + str x29, [sp, #FREGS_FP] > > mov x0, sp > - bl ftrace_return_to_handler // addr = ftrace_return_to_hander(regs); > + bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs); > mov x30, x0 // restore the original return address > > - /* restore return value regs */ > - ldp x0, x1, [sp, #FGRET_REGS_X0] > - ldp x2, x3, [sp, #FGRET_REGS_X2] > - ldp x4, x5, [sp, #FGRET_REGS_X4] > - ldp x6, x7, [sp, #FGRET_REGS_X6] > - add sp, sp, #FGRET_REGS_SIZE > + /* Restore return value regs */ > + ldp x0, x1, [sp, #FREGS_X0] > + ldp x2, x3, [sp, #FREGS_X2] > + ldp x4, x5, [sp, #FREGS_X4] > + ldp x6, x7, [sp, #FREGS_X6] > + add sp, sp, #FREGS_SIZE > > ret > SYM_CODE_END(return_to_handler) > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index 70f169210b52..974f08f65f63 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -131,7 +131,7 @@ config LOONGARCH > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h > index 6f8517d59954..1a73f35ea9af 100644 > --- a/arch/loongarch/include/asm/ftrace.h > +++ b/arch/loongarch/include/asm/ftrace.h > @@ -77,6 +77,8 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + ((fregs)->regs.regs[22]) > > #define ftrace_graph_func ftrace_graph_func > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, > @@ -99,26 +101,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) > > #endif /* CONFIG_FUNCTION_TRACER */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - /* a0 - a1 */ > - unsigned long regs[2]; > - > - unsigned long fp; > - unsigned long __unused; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->regs[0]; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_LOONGARCH_FTRACE_H */ > diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c > index bee9f7a3108f..714f5b5f1956 100644 > --- a/arch/loongarch/kernel/asm-offsets.c > +++ b/arch/loongarch/kernel/asm-offsets.c > @@ -279,18 +279,6 @@ static void __used output_pbe_defines(void) > } > #endif > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -static void __used output_fgraph_ret_regs_defines(void) > -{ > - COMMENT("LoongArch fgraph_ret_regs offsets."); > - OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]); > - OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]); > - OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp); > - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); > - BLANK(); > -} > -#endif > - > static void __used output_kvm_defines(void) > { > COMMENT("KVM/LoongArch Specific offsets."); > diff --git a/arch/loongarch/kernel/mcount.S b/arch/loongarch/kernel/mcount.S > index 3015896016a0..b6850503e061 100644 > --- a/arch/loongarch/kernel/mcount.S > +++ b/arch/loongarch/kernel/mcount.S > @@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller) > SYM_FUNC_END(ftrace_graph_caller) > > SYM_FUNC_START(return_to_handler) > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + /* Save return value regs */ > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > @@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler) > /* Restore the real parent address: a0 -> ra */ > move ra, a0 > > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + /* Restore return value regs */ > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > + > jr ra > SYM_FUNC_END(return_to_handler) > #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S > index 0c65cf09110c..d6b474ad1d5e 100644 > --- a/arch/loongarch/kernel/mcount_dyn.S > +++ b/arch/loongarch/kernel/mcount_dyn.S > @@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > /* Save return value regs */ > - PTR_ADDI sp, sp, -FGRET_REGS_SIZE > - PTR_S a0, sp, FGRET_REGS_A0 > - PTR_S a1, sp, FGRET_REGS_A1 > - PTR_S zero, sp, FGRET_REGS_FP > + PTR_ADDI sp, sp, -PT_SIZE > + PTR_S a0, sp, PT_R4 > + PTR_S a1, sp, PT_R5 > + PTR_S zero, sp, PT_R22 > > move a0, sp > bl ftrace_return_to_handler > move ra, a0 > > /* Restore return value regs */ > - PTR_L a0, sp, FGRET_REGS_A0 > - PTR_L a1, sp, FGRET_REGS_A1 > - PTR_ADDI sp, sp, FGRET_REGS_SIZE > + PTR_L a0, sp, PT_R4 > + PTR_L a1, sp, PT_R5 > + PTR_ADDI sp, sp, PT_SIZE > > jr ra > SYM_CODE_END(return_to_handler) > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 0f3cd7c3a436..6e8422269ba4 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -140,7 +140,7 @@ config RISCV > select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL > select HAVE_FUNCTION_GRAPH_TRACER > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION > select HAVE_EBPF_JIT if MMU > select HAVE_GUP_FAST if MMU > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index 2cddd79ff21b..e9f364ce9fe8 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -164,6 +164,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct > return fregs->sp; > } > > +static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) > +{ > + return fregs->s0; > +} > + > static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, > unsigned int n) > { > @@ -204,25 +209,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi > > #endif /* CONFIG_DYNAMIC_FTRACE */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long a1; > - unsigned long a0; > - unsigned long s0; > - unsigned long ra; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->a0; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->s0; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_RISCV_FTRACE_H */ > diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S > index 3a42f6287909..068168046e0e 100644 > --- a/arch/riscv/kernel/mcount.S > +++ b/arch/riscv/kernel/mcount.S > @@ -12,6 +12,8 @@ > #include <asm/asm-offsets.h> > #include <asm/ftrace.h> > > +#define ABI_SIZE_ON_STACK 80 > + > .text > > .macro SAVE_ABI_STATE > @@ -26,12 +28,12 @@ > * register if a0 was not saved. > */ > .macro SAVE_RET_ABI_STATE > - addi sp, sp, -4*SZREG > - REG_S s0, 2*SZREG(sp) > - REG_S ra, 3*SZREG(sp) > - REG_S a0, 1*SZREG(sp) > - REG_S a1, 0*SZREG(sp) > - addi s0, sp, 4*SZREG > + addi sp, sp, -ABI_SIZE_ON_STACK > + REG_S ra, 1*SZREG(sp) > + REG_S s0, 8*SZREG(sp) > + REG_S a0, 10*SZREG(sp) > + REG_S a1, 11*SZREG(sp) > + addi s0, sp, ABI_SIZE_ON_STACK > .endm > > .macro RESTORE_ABI_STATE > @@ -41,11 +43,11 @@ > .endm > > .macro RESTORE_RET_ABI_STATE > - REG_L ra, 3*SZREG(sp) > - REG_L s0, 2*SZREG(sp) > - REG_L a0, 1*SZREG(sp) > - REG_L a1, 0*SZREG(sp) > - addi sp, sp, 4*SZREG > + REG_L ra, 1*SZREG(sp) > + REG_L s0, 8*SZREG(sp) > + REG_L a0, 10*SZREG(sp) > + REG_L a1, 11*SZREG(sp) > + addi sp, sp, ABI_SIZE_ON_STACK > .endm > > SYM_TYPED_FUNC_START(ftrace_stub) > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig > index a822f952f64a..12e942cfbcde 100644 > --- a/arch/s390/Kconfig > +++ b/arch/s390/Kconfig > @@ -184,7 +184,7 @@ config S390 > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_ARG_ACCESS_API > select HAVE_FUNCTION_ERROR_INJECTION > - select HAVE_FUNCTION_GRAPH_RETVAL > + select HAVE_FUNCTION_GRAPH_FREGS > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h > index de76c21eb4a3..9cdd48a46bf7 100644 > --- a/arch/s390/include/asm/ftrace.h > +++ b/arch/s390/include/asm/ftrace.h > @@ -49,23 +49,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * > return NULL; > } > > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long gpr2; > - unsigned long fp; > -}; > - > -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->gpr2; > -} > - > -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->fp; > -} > -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ > - > static __always_inline unsigned long > ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) > { > @@ -92,6 +75,15 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > > +static __always_inline unsigned long > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > +{ > + unsigned long *sp; > + > + sp = (void *)ftrace_regs_get_stack_pointer(fregs); > + return sp[0]; /* return backchain */ > +} > + > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > /* > * When an ftrace registered caller is tracing a function that is > diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c > index ffa0dd2dbaac..d38ed80615d5 100644 > --- a/arch/s390/kernel/asm-offsets.c > +++ b/arch/s390/kernel/asm-offsets.c > @@ -179,12 +179,6 @@ int main(void) > DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); > DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); > DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > - /* function graph return value tracing */ > - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); > - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); > - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); > -#endif > OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); > DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > index ae4d4fd9afcd..cda798b976de 100644 > --- a/arch/s390/kernel/mcount.S > +++ b/arch/s390/kernel/mcount.S > @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) > SYM_FUNC_START(return_to_handler) > stmg %r2,%r5,32(%r15) > lgr %r1,%r15 > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > +# Allocate ftrace_regs + backchain on the stack > + aghi %r15,-STACK_FRAME_SIZE_FREGS > stg %r1,__SF_BACKCHAIN(%r15) > la %r3,STACK_FRAME_OVERHEAD(%r15) > - stg %r1,__FGRAPH_RET_FP(%r3) > - stg %r2,__FGRAPH_RET_GPR2(%r3) > + stg %r2,(__SF_GPRS+2*8)(%r15) > + stg %r15,(__SF_GPRS+15*8)(%r15) > lgr %r2,%r3 > brasl %r14,ftrace_return_to_handler > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > + aghi %r15,STACK_FRAME_SIZE_FREGS > lgr %r14,%r2 > lmg %r2,%r5,32(%r15) > BR_EX %r14 > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 007bab9f2a0e..047384e4d93a 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -228,7 +228,7 @@ config X86 > select HAVE_GUP_FAST > select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE > select HAVE_FTRACE_MCOUNT_RECORD > - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER > + select HAVE_FUNCTION_GRAPH_FREGS if HAVE_FUNCTION_GRAPH_TRACER > select HAVE_FUNCTION_GRAPH_TRACER if X86_32 || (X86_64 && DYNAMIC_FTRACE) > select HAVE_FUNCTION_TRACER > select HAVE_GCC_PLUGINS > diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h > index 78f6a200e15b..669771ef3b5b 100644 > --- a/arch/x86/include/asm/ftrace.h > +++ b/arch/x86/include/asm/ftrace.h > @@ -64,6 +64,8 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) > override_function_with_return(&(fregs)->regs) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > > struct ftrace_ops; > #define ftrace_graph_func ftrace_graph_func > @@ -148,24 +150,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) > #endif /* !COMPILE_OFFSETS */ > #endif /* !__ASSEMBLY__ */ > > -#ifndef __ASSEMBLY__ > -#ifdef CONFIG_FUNCTION_GRAPH_TRACER > -struct fgraph_ret_regs { > - unsigned long ax; > - unsigned long dx; > - unsigned long bp; > -}; > - > -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->ax; > -} > - > -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) > -{ > - return ret_regs->bp; > -} > -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ > -#endif > - > #endif /* _ASM_X86_FTRACE_H */ > diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S > index 58d9ed50fe61..4b265884d06c 100644 > --- a/arch/x86/kernel/ftrace_32.S > +++ b/arch/x86/kernel/ftrace_32.S > @@ -23,6 +23,8 @@ SYM_FUNC_START(__fentry__) > SYM_FUNC_END(__fentry__) > EXPORT_SYMBOL(__fentry__) > > +#define FRAME_SIZE PT_OLDSS+4 > + > SYM_CODE_START(ftrace_caller) > > #ifdef CONFIG_FRAME_POINTER > @@ -187,14 +189,15 @@ SYM_CODE_END(ftrace_graph_caller) > > .globl return_to_handler > return_to_handler: > - pushl $0 > - pushl %edx > - pushl %eax > + subl $(FRAME_SIZE), %esp > + movl $0, PT_EBP(%esp) > + movl %edx, PT_EDX(%esp) > + movl %eax, PT_EAX(%esp) > movl %esp, %eax > call ftrace_return_to_handler > movl %eax, %ecx > - popl %eax > - popl %edx > - addl $4, %esp # skip ebp > + movl %eax, PT_EAX(%esp) > + movl %edx, PT_EDX(%esp) > + addl $(FRAME_SIZE), %esp > JMP_NOSPEC ecx > #endif > diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S > index 214f30e9f0c0..d51647228596 100644 > --- a/arch/x86/kernel/ftrace_64.S > +++ b/arch/x86/kernel/ftrace_64.S > @@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) > SYM_CODE_START(return_to_handler) > UNWIND_HINT_UNDEFINED > ANNOTATE_NOENDBR > - subq $24, %rsp > > - /* Save the return values */ > - movq %rax, (%rsp) > - movq %rdx, 8(%rsp) > - movq %rbp, 16(%rsp) > + /* Save ftrace_regs for function exit context */ > + subq $(FRAME_SIZE), %rsp > + > + movq %rax, RAX(%rsp) > + movq %rdx, RDX(%rsp) > + movq %rbp, RBP(%rsp) > movq %rsp, %rdi > > call ftrace_return_to_handler > > movq %rax, %rdi > - movq 8(%rsp), %rdx > - movq (%rsp), %rax > + movq RDX(%rsp), %rdx > + movq RAX(%rsp), %rax > > - addq $24, %rsp > + addq $(FRAME_SIZE), %rsp > /* > * Jump back to the old return address. This cannot be JMP_NOSPEC rdi > * since IBT would demand that contain ENDBR, which simply isn't so for > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h > index 1fe49a28de2d..13987cd63553 100644 > --- a/include/linux/ftrace.h > +++ b/include/linux/ftrace.h > @@ -43,9 +43,8 @@ struct dyn_ftrace; > > char *arch_ftrace_match_adjust(char *str, const char *search); > > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -struct fgraph_ret_regs; > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs); > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs); > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer); > #endif > @@ -134,6 +133,13 @@ extern int ftrace_enabled; > * Also, architecture dependent fields can be used for internal process. > * (e.g. orig_ax on x86_64) > * > + * Basically, ftrace_regs stores the registers related to the context. > + * On function entry, registers for function parameters and hooking the > + * function call are stored, and on function exit, registers for function > + * return value and frame pointers are stored. > + * > + * And also, it dpends on the context that which registers are restored > + * from the ftrace_regs. > * On the function entry, those registers will be restored except for > * the stack pointer, so that user can change the function parameters > * and instruction pointer (e.g. live patching.) > @@ -191,6 +197,8 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) > override_function_with_return(ftrace_get_regs(fregs)) > #define ftrace_regs_query_register_offset(name) \ > regs_query_register_offset(name) > +#define ftrace_regs_get_frame_pointer(fregs) \ > + frame_pointer(&(fregs)->regs) > #endif > > typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > index 721c3b221048..ab277eff80dc 100644 > --- a/kernel/trace/Kconfig > +++ b/kernel/trace/Kconfig > @@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER > help > See Documentation/trace/ftrace-design.rst > > -config HAVE_FUNCTION_GRAPH_RETVAL > +config HAVE_FUNCTION_GRAPH_FREGS > bool > > config HAVE_DYNAMIC_FTRACE > @@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER > > config FUNCTION_GRAPH_RETVAL > bool "Kernel Function Graph Return Value" > - depends on HAVE_FUNCTION_GRAPH_RETVAL > + depends on HAVE_FUNCTION_GRAPH_FREGS > depends on FUNCTION_GRAPH_TRACER > default n > help > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index 0322c5723748..30bebe43607d 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -763,15 +763,12 @@ static struct notifier_block ftrace_suspend_notifier = { > .notifier_call = ftrace_suspend_notifier_call, > }; > > -/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */ > -struct fgraph_ret_regs; > - > /* > * Send the trace to the ring-buffer. > * @return the original return address. > */ > -static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs, > - unsigned long frame_pointer) > +static inline unsigned long > +__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer) > { > struct ftrace_ret_stack *ret_stack; > struct ftrace_graph_ret trace; > @@ -791,7 +788,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > > trace.rettime = trace_clock_local(); > #ifdef CONFIG_FUNCTION_GRAPH_RETVAL > - trace.retval = fgraph_ret_regs_return_value(ret_regs); > + trace.retval = ftrace_regs_get_return_value(fregs); > #endif > > bitmap = get_bitmap_bits(current, offset); > @@ -826,14 +823,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs > } > > /* > - * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can > - * leave only ftrace_return_to_handler(ret_regs). > + * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can > + * leave only ftrace_return_to_handler(fregs). > */ > -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL > -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs) > +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS > +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs) > { > - return __ftrace_return_to_handler(ret_regs, > - fgraph_ret_regs_frame_pointer(ret_regs)); > + return __ftrace_return_to_handler(fregs, > + ftrace_regs_get_frame_pointer(fregs)); > } > #else > unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
On Sun, Sep 15, 2024 at 05:15:59AM -0400, Steven Rostedt wrote: > > Can I get an Acked-by from the S390 maintainers for this patch? ... > > +static __always_inline unsigned long > > +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) > > +{ > > + unsigned long *sp; > > + > > + sp = (void *)ftrace_regs_get_stack_pointer(fregs); > > + return sp[0]; /* return backchain */ > > +} > > + ... > > diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S > > index ae4d4fd9afcd..cda798b976de 100644 > > --- a/arch/s390/kernel/mcount.S > > +++ b/arch/s390/kernel/mcount.S > > @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) > > SYM_FUNC_START(return_to_handler) > > stmg %r2,%r5,32(%r15) > > lgr %r1,%r15 > > - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) > > +# Allocate ftrace_regs + backchain on the stack > > + aghi %r15,-STACK_FRAME_SIZE_FREGS > > stg %r1,__SF_BACKCHAIN(%r15) > > la %r3,STACK_FRAME_OVERHEAD(%r15) > > - stg %r1,__FGRAPH_RET_FP(%r3) > > - stg %r2,__FGRAPH_RET_GPR2(%r3) > > + stg %r2,(__SF_GPRS+2*8)(%r15) > > + stg %r15,(__SF_GPRS+15*8)(%r15) > > lgr %r2,%r3 > > brasl %r14,ftrace_return_to_handler > > - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE > > + aghi %r15,STACK_FRAME_SIZE_FREGS This does not pass the ftrace selftests. Please merge the patch below into this patch. With that: Acked-by: Heiko Carstens <hca@linux.ibm.com> diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index 7b80ff4d3386..df5a0f8d3445 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -78,10 +78,7 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, static __always_inline unsigned long ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) { - unsigned long *sp; - - sp = (void *)ftrace_regs_get_stack_pointer(fregs); - return sp[0]; /* return backchain */ + return ftrace_regs_get_stack_pointer(fregs); } static __always_inline unsigned long diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S index cda798b976de..10b08e617306 100644 --- a/arch/s390/kernel/mcount.S +++ b/arch/s390/kernel/mcount.S @@ -133,13 +133,12 @@ SYM_CODE_END(ftrace_common) SYM_FUNC_START(return_to_handler) stmg %r2,%r5,32(%r15) lgr %r1,%r15 -# Allocate ftrace_regs + backchain on the stack + # allocate ftrace_regs and stack frame for ftrace_return_to_handler aghi %r15,-STACK_FRAME_SIZE_FREGS stg %r1,__SF_BACKCHAIN(%r15) - la %r3,STACK_FRAME_OVERHEAD(%r15) - stg %r2,(__SF_GPRS+2*8)(%r15) - stg %r15,(__SF_GPRS+15*8)(%r15) - lgr %r2,%r3 + stg %r2,(STACK_FREGS_PTREGS_GPRS+2*8)(%r15) + stg %r1,(STACK_FREGS_PTREGS_GPRS+15*8)(%r15) + la %r2,STACK_FRAME_OVERHEAD(%r15) brasl %r14,ftrace_return_to_handler aghi %r15,STACK_FRAME_SIZE_FREGS lgr %r14,%r2
On Mon, 16 Sep 2024 14:16:56 +0200 Heiko Carstens <hca@linux.ibm.com> wrote: > This does not pass the ftrace selftests. Please merge the patch below > into this patch. With that: > > Acked-by: Heiko Carstens <hca@linux.ibm.com> Thank you very much, this is why I wanted arch maintainers acks before taking anything. There may be other patches in this series that I didn't Cc everyone (yet). Did you look at the other patches? If not, I'll go and do the Cc. It's a manual process. -- Steve
On Mon, Sep 16, 2024 at 12:29:30PM -0400, Steven Rostedt wrote: > On Mon, 16 Sep 2024 14:16:56 +0200 > Heiko Carstens <hca@linux.ibm.com> wrote: > > > This does not pass the ftrace selftests. Please merge the patch below > > into this patch. With that: > > > > Acked-by: Heiko Carstens <hca@linux.ibm.com> > > Thank you very much, this is why I wanted arch maintainers acks before > taking anything. > > There may be other patches in this series that I didn't Cc everyone > (yet). Did you look at the other patches? If not, I'll go and do the Cc. > It's a manual process. I just had a quick look and tried Masami's git tree with the topic/fprobe-on-fgraph branch and tried the ftrace selftests, which resulted in the provided small fix. But now that you ask, I can see that FPROBE is not available on s390 anymore with the full series, since HAVE_FTRACE_GRAPH_FUNC is not available. Also the s390 variant of arch_ftrace_fill_perf_regs() seems to be incorrect. Looks like some work is needed from our side.
On Sun, Sep 15, 2024 at 05:11:44AM -0400, Steven Rostedt wrote: > On Fri, 13 Sep 2024 00:08:51 +0900 > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Use ftrace_regs instead of fgraph_ret_regs for tracing return value > > on function_graph tracer because of simplifying the callback interface. > > > > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by > > CONFIG_HAVE_FUNCTION_GRAPH_FREGS. > > > > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > --- > > Changes in v8: > > - Newly added. > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/include/asm/ftrace.h | 23 ++++++----------------- > > arch/arm64/kernel/asm-offsets.c | 12 ------------ > > arch/arm64/kernel/entry-ftrace.S | 32 ++++++++++++++++++-------------- The arm64 part looks good to me, although passing a partially-populated struct out of asm feels like it's going to cause us hard-to-debug problems down the line if any of those extra fields get used. How hard would it be to poison the unpopulated members of 'ftrace_regs'? Will
On Tue, 17 Sep 2024 10:55:39 +0100 Will Deacon <will@kernel.org> wrote: > The arm64 part looks good to me, although passing a partially-populated > struct out of asm feels like it's going to cause us hard-to-debug > problems down the line if any of those extra fields get used. How hard > would it be to poison the unpopulated members of 'ftrace_regs'? The purpose of creating ftrace_regs was to allow a partially populated pt_regs to be sent around, as Thomas Gleixner and Peter Zijlstra were against using pt_regs that were not fully populated. Hence, I created "ftrace_regs" for this purpose. ftrace_regs should never be accessed via its internal elements but only with its accessor functions, as depending on the arch or functionality used, the content of the structure should never be trusted. The accessor functions will do all the verification needed. I may add some compiler hacks to enforce this. Something like: struct ftrace_regs { void *nothing_to_see_here; }; And then change the arch code to be something like: // in arch/arm64/include/asm/ftrace.h: struct arch_ftrace_regs { /* x0 - x8 */ unsigned long regs[9]; #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS unsigned long direct_tramp; #else unsigned long __unused; #endif unsigned long fp; unsigned long lr; unsigned long sp; unsigned long pc; }; #define get_arch_ftrace_regs(fregs) ((struct arch_ftrace_regs *)(fregs)) static __always_inline void ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long pc) { struct arch_ftrace_regs *afregs = get_ftrace_regs(fregs); afregs->pc = pc; } -- Steve
On Mon, 16 Sep 2024 20:59:05 +0200 Heiko Carstens <hca@linux.ibm.com> wrote: > On Mon, Sep 16, 2024 at 12:29:30PM -0400, Steven Rostedt wrote: > > On Mon, 16 Sep 2024 14:16:56 +0200 > > Heiko Carstens <hca@linux.ibm.com> wrote: > > > > > This does not pass the ftrace selftests. Please merge the patch below > > > into this patch. With that: > > > > > > Acked-by: Heiko Carstens <hca@linux.ibm.com> > > > > Thank you very much, this is why I wanted arch maintainers acks before > > taking anything. > > > > There may be other patches in this series that I didn't Cc everyone > > (yet). Did you look at the other patches? If not, I'll go and do the Cc. > > It's a manual process. > > I just had a quick look and tried Masami's git tree with the > topic/fprobe-on-fgraph branch and tried the ftrace selftests, which > resulted in the provided small fix. Thanks, and sorry, my change on s390 was wrong. I confused that `stack_frame` can be a part of `ftrace_regs`. But at least in this moment, `ftrace_regs` should replace only `fgraph_ret_regs`. > > But now that you ask, I can see that FPROBE is not available on s390 > anymore with the full series, since HAVE_FTRACE_GRAPH_FUNC is not > available. Right, this is required for implementing fprobe. > > Also the s390 variant of arch_ftrace_fill_perf_regs() seems to be > incorrect. Can you tell me what part will be wrong? Thank you, > > Looks like some work is needed from our side.
On Mon, 30 Sep 2024 14:55:48 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 17 Sep 2024 10:55:39 +0100 > Will Deacon <will@kernel.org> wrote: > > > The arm64 part looks good to me, although passing a partially-populated > > struct out of asm feels like it's going to cause us hard-to-debug > > problems down the line if any of those extra fields get used. How hard > > would it be to poison the unpopulated members of 'ftrace_regs'? > > The purpose of creating ftrace_regs was to allow a partially populated > pt_regs to be sent around, as Thomas Gleixner and Peter Zijlstra were > against using pt_regs that were not fully populated. Hence, I created > "ftrace_regs" for this purpose. > > ftrace_regs should never be accessed via its internal elements but only with > its accessor functions, as depending on the arch or functionality used, the > content of the structure should never be trusted. The accessor functions > will do all the verification needed. > > I may add some compiler hacks to enforce this. Something like: > > struct ftrace_regs { > void *nothing_to_see_here; > }; Yeah, OK. But sizeof(fregs) may be changed. (Shouldn't we do too?) > > And then change the arch code to be something like: > > // in arch/arm64/include/asm/ftrace.h: > > struct arch_ftrace_regs { > /* x0 - x8 */ > unsigned long regs[9]; > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > unsigned long direct_tramp; > #else > unsigned long __unused; > #endif > > unsigned long fp; > unsigned long lr; > > unsigned long sp; > unsigned long pc; > }; And if it is pt_regs compatible, #define arch_ftrace_regs pt_regs ? > > #define get_arch_ftrace_regs(fregs) ((struct arch_ftrace_regs *)(fregs)) > > static __always_inline void > ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, > unsigned long pc) > { > struct arch_ftrace_regs *afregs = get_ftrace_regs(fregs); > afregs->pc = pc; > } > > > -- Steve Thanks,
On Wed, 2 Oct 2024 08:10:37 +0900 Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > > I may add some compiler hacks to enforce this. Something like: > > > > struct ftrace_regs { > > void *nothing_to_see_here; > > }; > > Yeah, OK. But sizeof(fregs) may be changed. (Shouldn't we do too?) Honestly, I don't think anything should be doing a sizeof(struct ftrace_regs) Heck, perhaps we should make it totally zero! struct ftrace_regs { long nothing_here[]; }; If someone needs to allocate, then we could provide a: ftrace_regs_size() helper function. > > > > > And then change the arch code to be something like: > > > > // in arch/arm64/include/asm/ftrace.h: > > > > struct arch_ftrace_regs { > > /* x0 - x8 */ > > unsigned long regs[9]; > > > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > unsigned long direct_tramp; > > #else > > unsigned long __unused; > > #endif > > > > unsigned long fp; > > unsigned long lr; > > > > unsigned long sp; > > unsigned long pc; > > }; > > And if it is pt_regs compatible, > > #define arch_ftrace_regs pt_regs > > ? > Only if it is fully pt_regs compatible. -- Steve
On Tue, 1 Oct 2024 19:32:34 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > On Wed, 2 Oct 2024 08:10:37 +0900 > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote: > > > > > > I may add some compiler hacks to enforce this. Something like: > > > > > > struct ftrace_regs { > > > void *nothing_to_see_here; > > > }; > > > > Yeah, OK. But sizeof(fregs) may be changed. (Shouldn't we do too?) > > Honestly, I don't think anything should be doing a sizeof(struct ftrace_regs) > > Heck, perhaps we should make it totally zero! > > struct ftrace_regs { > long nothing_here[]; > }; > > If someone needs to allocate, then we could provide a: > > ftrace_regs_size() > > helper function. Ah, Indeed. > > > > > > > > > And then change the arch code to be something like: > > > > > > // in arch/arm64/include/asm/ftrace.h: > > > > > > struct arch_ftrace_regs { > > > /* x0 - x8 */ > > > unsigned long regs[9]; > > > > > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS > > > unsigned long direct_tramp; > > > #else > > > unsigned long __unused; > > > #endif > > > > > > unsigned long fp; > > > unsigned long lr; > > > > > > unsigned long sp; > > > unsigned long pc; > > > }; > > > > And if it is pt_regs compatible, > > > > #define arch_ftrace_regs pt_regs > > > > ? > > > > Only if it is fully pt_regs compatible. Yeah, OK, this is good idea. Thank you, > > -- Steve
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a2f8ff354ca6..17947f625b06 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -211,6 +211,7 @@ config ARM64 select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_TRACER select HAVE_FUNCTION_ERROR_INJECTION + select HAVE_FUNCTION_GRAPH_FREGS select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_GRAPH_RETVAL select HAVE_GCC_PLUGINS diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index dc9cf0bd2a4c..dffaab3dd1f1 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -126,6 +126,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs) fregs->pc = fregs->lr; } +static __always_inline unsigned long +ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) +{ + return fregs->fp; +} + int ftrace_regs_query_register_offset(const char *name); int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec); @@ -183,23 +189,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym, #ifndef __ASSEMBLY__ #ifdef CONFIG_FUNCTION_GRAPH_TRACER -struct fgraph_ret_regs { - /* x0 - x7 */ - unsigned long regs[8]; - - unsigned long fp; - unsigned long __unused; -}; - -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->regs[0]; -} - -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->fp; -} void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent, unsigned long frame_pointer); diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c index 27de1dddb0ab..9e03c9a7e5c3 100644 --- a/arch/arm64/kernel/asm-offsets.c +++ b/arch/arm64/kernel/asm-offsets.c @@ -201,18 +201,6 @@ int main(void) DEFINE(FTRACE_OPS_FUNC, offsetof(struct ftrace_ops, func)); #endif BLANK(); -#ifdef CONFIG_FUNCTION_GRAPH_TRACER - DEFINE(FGRET_REGS_X0, offsetof(struct fgraph_ret_regs, regs[0])); - DEFINE(FGRET_REGS_X1, offsetof(struct fgraph_ret_regs, regs[1])); - DEFINE(FGRET_REGS_X2, offsetof(struct fgraph_ret_regs, regs[2])); - DEFINE(FGRET_REGS_X3, offsetof(struct fgraph_ret_regs, regs[3])); - DEFINE(FGRET_REGS_X4, offsetof(struct fgraph_ret_regs, regs[4])); - DEFINE(FGRET_REGS_X5, offsetof(struct fgraph_ret_regs, regs[5])); - DEFINE(FGRET_REGS_X6, offsetof(struct fgraph_ret_regs, regs[6])); - DEFINE(FGRET_REGS_X7, offsetof(struct fgraph_ret_regs, regs[7])); - DEFINE(FGRET_REGS_FP, offsetof(struct fgraph_ret_regs, fp)); - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); -#endif #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS DEFINE(FTRACE_OPS_DIRECT_CALL, offsetof(struct ftrace_ops, direct_call)); #endif diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S index f0c16640ef21..169ccf600066 100644 --- a/arch/arm64/kernel/entry-ftrace.S +++ b/arch/arm64/kernel/entry-ftrace.S @@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph) * @fp is checked against the value passed by ftrace_graph_caller(). */ SYM_CODE_START(return_to_handler) - /* save return value regs */ - sub sp, sp, #FGRET_REGS_SIZE - stp x0, x1, [sp, #FGRET_REGS_X0] - stp x2, x3, [sp, #FGRET_REGS_X2] - stp x4, x5, [sp, #FGRET_REGS_X4] - stp x6, x7, [sp, #FGRET_REGS_X6] - str x29, [sp, #FGRET_REGS_FP] // parent's fp + /* Make room for ftrace_regs */ + sub sp, sp, #FREGS_SIZE + + /* Save return value regs */ + stp x0, x1, [sp, #FREGS_X0] + stp x2, x3, [sp, #FREGS_X2] + stp x4, x5, [sp, #FREGS_X4] + stp x6, x7, [sp, #FREGS_X6] + + /* Save the callsite's FP */ + str x29, [sp, #FREGS_FP] mov x0, sp - bl ftrace_return_to_handler // addr = ftrace_return_to_hander(regs); + bl ftrace_return_to_handler // addr = ftrace_return_to_hander(fregs); mov x30, x0 // restore the original return address - /* restore return value regs */ - ldp x0, x1, [sp, #FGRET_REGS_X0] - ldp x2, x3, [sp, #FGRET_REGS_X2] - ldp x4, x5, [sp, #FGRET_REGS_X4] - ldp x6, x7, [sp, #FGRET_REGS_X6] - add sp, sp, #FGRET_REGS_SIZE + /* Restore return value regs */ + ldp x0, x1, [sp, #FREGS_X0] + ldp x2, x3, [sp, #FREGS_X2] + ldp x4, x5, [sp, #FREGS_X4] + ldp x6, x7, [sp, #FREGS_X6] + add sp, sp, #FREGS_SIZE ret SYM_CODE_END(return_to_handler) diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig index 70f169210b52..974f08f65f63 100644 --- a/arch/loongarch/Kconfig +++ b/arch/loongarch/Kconfig @@ -131,7 +131,7 @@ config LOONGARCH select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_ARG_ACCESS_API select HAVE_FUNCTION_ERROR_INJECTION - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER + select HAVE_FUNCTION_GRAPH_FREGS select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER select HAVE_GCC_PLUGINS diff --git a/arch/loongarch/include/asm/ftrace.h b/arch/loongarch/include/asm/ftrace.h index 6f8517d59954..1a73f35ea9af 100644 --- a/arch/loongarch/include/asm/ftrace.h +++ b/arch/loongarch/include/asm/ftrace.h @@ -77,6 +77,8 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip) override_function_with_return(&(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) +#define ftrace_regs_get_frame_pointer(fregs) \ + ((fregs)->regs.regs[22]) #define ftrace_graph_func ftrace_graph_func void ftrace_graph_func(unsigned long ip, unsigned long parent_ip, @@ -99,26 +101,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr) #endif /* CONFIG_FUNCTION_TRACER */ -#ifndef __ASSEMBLY__ -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -struct fgraph_ret_regs { - /* a0 - a1 */ - unsigned long regs[2]; - - unsigned long fp; - unsigned long __unused; -}; - -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->regs[0]; -} - -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->fp; -} -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ -#endif - #endif /* _ASM_LOONGARCH_FTRACE_H */ diff --git a/arch/loongarch/kernel/asm-offsets.c b/arch/loongarch/kernel/asm-offsets.c index bee9f7a3108f..714f5b5f1956 100644 --- a/arch/loongarch/kernel/asm-offsets.c +++ b/arch/loongarch/kernel/asm-offsets.c @@ -279,18 +279,6 @@ static void __used output_pbe_defines(void) } #endif -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -static void __used output_fgraph_ret_regs_defines(void) -{ - COMMENT("LoongArch fgraph_ret_regs offsets."); - OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]); - OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]); - OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp); - DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs)); - BLANK(); -} -#endif - static void __used output_kvm_defines(void) { COMMENT("KVM/LoongArch Specific offsets."); diff --git a/arch/loongarch/kernel/mcount.S b/arch/loongarch/kernel/mcount.S index 3015896016a0..b6850503e061 100644 --- a/arch/loongarch/kernel/mcount.S +++ b/arch/loongarch/kernel/mcount.S @@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller) SYM_FUNC_END(ftrace_graph_caller) SYM_FUNC_START(return_to_handler) - PTR_ADDI sp, sp, -FGRET_REGS_SIZE - PTR_S a0, sp, FGRET_REGS_A0 - PTR_S a1, sp, FGRET_REGS_A1 - PTR_S zero, sp, FGRET_REGS_FP + /* Save return value regs */ + PTR_ADDI sp, sp, -PT_SIZE + PTR_S a0, sp, PT_R4 + PTR_S a1, sp, PT_R5 + PTR_S zero, sp, PT_R22 move a0, sp bl ftrace_return_to_handler @@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler) /* Restore the real parent address: a0 -> ra */ move ra, a0 - PTR_L a0, sp, FGRET_REGS_A0 - PTR_L a1, sp, FGRET_REGS_A1 - PTR_ADDI sp, sp, FGRET_REGS_SIZE + /* Restore return value regs */ + PTR_L a0, sp, PT_R4 + PTR_L a1, sp, PT_R5 + PTR_ADDI sp, sp, PT_SIZE + jr ra SYM_FUNC_END(return_to_handler) #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ diff --git a/arch/loongarch/kernel/mcount_dyn.S b/arch/loongarch/kernel/mcount_dyn.S index 0c65cf09110c..d6b474ad1d5e 100644 --- a/arch/loongarch/kernel/mcount_dyn.S +++ b/arch/loongarch/kernel/mcount_dyn.S @@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller) SYM_CODE_START(return_to_handler) UNWIND_HINT_UNDEFINED /* Save return value regs */ - PTR_ADDI sp, sp, -FGRET_REGS_SIZE - PTR_S a0, sp, FGRET_REGS_A0 - PTR_S a1, sp, FGRET_REGS_A1 - PTR_S zero, sp, FGRET_REGS_FP + PTR_ADDI sp, sp, -PT_SIZE + PTR_S a0, sp, PT_R4 + PTR_S a1, sp, PT_R5 + PTR_S zero, sp, PT_R22 move a0, sp bl ftrace_return_to_handler move ra, a0 /* Restore return value regs */ - PTR_L a0, sp, FGRET_REGS_A0 - PTR_L a1, sp, FGRET_REGS_A1 - PTR_ADDI sp, sp, FGRET_REGS_SIZE + PTR_L a0, sp, PT_R4 + PTR_L a1, sp, PT_R5 + PTR_ADDI sp, sp, PT_SIZE jr ra SYM_CODE_END(return_to_handler) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 0f3cd7c3a436..6e8422269ba4 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -140,7 +140,7 @@ config RISCV select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER + select HAVE_FUNCTION_GRAPH_FREGS select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION select HAVE_EBPF_JIT if MMU select HAVE_GUP_FAST if MMU diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index 2cddd79ff21b..e9f364ce9fe8 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -164,6 +164,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct return fregs->sp; } +static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs) +{ + return fregs->s0; +} + static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs, unsigned int n) { @@ -204,25 +209,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi #endif /* CONFIG_DYNAMIC_FTRACE */ -#ifndef __ASSEMBLY__ -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -struct fgraph_ret_regs { - unsigned long a1; - unsigned long a0; - unsigned long s0; - unsigned long ra; -}; - -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->a0; -} - -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->s0; -} -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ -#endif - #endif /* _ASM_RISCV_FTRACE_H */ diff --git a/arch/riscv/kernel/mcount.S b/arch/riscv/kernel/mcount.S index 3a42f6287909..068168046e0e 100644 --- a/arch/riscv/kernel/mcount.S +++ b/arch/riscv/kernel/mcount.S @@ -12,6 +12,8 @@ #include <asm/asm-offsets.h> #include <asm/ftrace.h> +#define ABI_SIZE_ON_STACK 80 + .text .macro SAVE_ABI_STATE @@ -26,12 +28,12 @@ * register if a0 was not saved. */ .macro SAVE_RET_ABI_STATE - addi sp, sp, -4*SZREG - REG_S s0, 2*SZREG(sp) - REG_S ra, 3*SZREG(sp) - REG_S a0, 1*SZREG(sp) - REG_S a1, 0*SZREG(sp) - addi s0, sp, 4*SZREG + addi sp, sp, -ABI_SIZE_ON_STACK + REG_S ra, 1*SZREG(sp) + REG_S s0, 8*SZREG(sp) + REG_S a0, 10*SZREG(sp) + REG_S a1, 11*SZREG(sp) + addi s0, sp, ABI_SIZE_ON_STACK .endm .macro RESTORE_ABI_STATE @@ -41,11 +43,11 @@ .endm .macro RESTORE_RET_ABI_STATE - REG_L ra, 3*SZREG(sp) - REG_L s0, 2*SZREG(sp) - REG_L a0, 1*SZREG(sp) - REG_L a1, 0*SZREG(sp) - addi sp, sp, 4*SZREG + REG_L ra, 1*SZREG(sp) + REG_L s0, 8*SZREG(sp) + REG_L a0, 10*SZREG(sp) + REG_L a1, 11*SZREG(sp) + addi sp, sp, ABI_SIZE_ON_STACK .endm SYM_TYPED_FUNC_START(ftrace_stub) diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig index a822f952f64a..12e942cfbcde 100644 --- a/arch/s390/Kconfig +++ b/arch/s390/Kconfig @@ -184,7 +184,7 @@ config S390 select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_ARG_ACCESS_API select HAVE_FUNCTION_ERROR_INJECTION - select HAVE_FUNCTION_GRAPH_RETVAL + select HAVE_FUNCTION_GRAPH_FREGS select HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_TRACER select HAVE_GCC_PLUGINS diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h index de76c21eb4a3..9cdd48a46bf7 100644 --- a/arch/s390/include/asm/ftrace.h +++ b/arch/s390/include/asm/ftrace.h @@ -49,23 +49,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs * return NULL; } -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -struct fgraph_ret_regs { - unsigned long gpr2; - unsigned long fp; -}; - -static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->gpr2; -} - -static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->fp; -} -#endif /* CONFIG_FUNCTION_GRAPH_TRACER */ - static __always_inline unsigned long ftrace_regs_get_instruction_pointer(const struct ftrace_regs *fregs) { @@ -92,6 +75,15 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) +static __always_inline unsigned long +ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs) +{ + unsigned long *sp; + + sp = (void *)ftrace_regs_get_stack_pointer(fregs); + return sp[0]; /* return backchain */ +} + #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS /* * When an ftrace registered caller is tracing a function that is diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c index ffa0dd2dbaac..d38ed80615d5 100644 --- a/arch/s390/kernel/asm-offsets.c +++ b/arch/s390/kernel/asm-offsets.c @@ -179,12 +179,6 @@ int main(void) DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size)); DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line)); DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size)); -#ifdef CONFIG_FUNCTION_GRAPH_TRACER - /* function graph return value tracing */ - OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2); - OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp); - DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs)); -#endif OFFSET(__FTRACE_REGS_PT_REGS, ftrace_regs, regs); DEFINE(__FTRACE_REGS_SIZE, sizeof(struct ftrace_regs)); diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S index ae4d4fd9afcd..cda798b976de 100644 --- a/arch/s390/kernel/mcount.S +++ b/arch/s390/kernel/mcount.S @@ -133,14 +133,15 @@ SYM_CODE_END(ftrace_common) SYM_FUNC_START(return_to_handler) stmg %r2,%r5,32(%r15) lgr %r1,%r15 - aghi %r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE) +# Allocate ftrace_regs + backchain on the stack + aghi %r15,-STACK_FRAME_SIZE_FREGS stg %r1,__SF_BACKCHAIN(%r15) la %r3,STACK_FRAME_OVERHEAD(%r15) - stg %r1,__FGRAPH_RET_FP(%r3) - stg %r2,__FGRAPH_RET_GPR2(%r3) + stg %r2,(__SF_GPRS+2*8)(%r15) + stg %r15,(__SF_GPRS+15*8)(%r15) lgr %r2,%r3 brasl %r14,ftrace_return_to_handler - aghi %r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE + aghi %r15,STACK_FRAME_SIZE_FREGS lgr %r14,%r2 lmg %r2,%r5,32(%r15) BR_EX %r14 diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 007bab9f2a0e..047384e4d93a 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -228,7 +228,7 @@ config X86 select HAVE_GUP_FAST select HAVE_FENTRY if X86_64 || DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD - select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER + select HAVE_FUNCTION_GRAPH_FREGS if HAVE_FUNCTION_GRAPH_TRACER select HAVE_FUNCTION_GRAPH_TRACER if X86_32 || (X86_64 && DYNAMIC_FTRACE) select HAVE_FUNCTION_TRACER select HAVE_GCC_PLUGINS diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h index 78f6a200e15b..669771ef3b5b 100644 --- a/arch/x86/include/asm/ftrace.h +++ b/arch/x86/include/asm/ftrace.h @@ -64,6 +64,8 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs) override_function_with_return(&(fregs)->regs) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) +#define ftrace_regs_get_frame_pointer(fregs) \ + frame_pointer(&(fregs)->regs) struct ftrace_ops; #define ftrace_graph_func ftrace_graph_func @@ -148,24 +150,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs) #endif /* !COMPILE_OFFSETS */ #endif /* !__ASSEMBLY__ */ -#ifndef __ASSEMBLY__ -#ifdef CONFIG_FUNCTION_GRAPH_TRACER -struct fgraph_ret_regs { - unsigned long ax; - unsigned long dx; - unsigned long bp; -}; - -static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->ax; -} - -static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs) -{ - return ret_regs->bp; -} -#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */ -#endif - #endif /* _ASM_X86_FTRACE_H */ diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S index 58d9ed50fe61..4b265884d06c 100644 --- a/arch/x86/kernel/ftrace_32.S +++ b/arch/x86/kernel/ftrace_32.S @@ -23,6 +23,8 @@ SYM_FUNC_START(__fentry__) SYM_FUNC_END(__fentry__) EXPORT_SYMBOL(__fentry__) +#define FRAME_SIZE PT_OLDSS+4 + SYM_CODE_START(ftrace_caller) #ifdef CONFIG_FRAME_POINTER @@ -187,14 +189,15 @@ SYM_CODE_END(ftrace_graph_caller) .globl return_to_handler return_to_handler: - pushl $0 - pushl %edx - pushl %eax + subl $(FRAME_SIZE), %esp + movl $0, PT_EBP(%esp) + movl %edx, PT_EDX(%esp) + movl %eax, PT_EAX(%esp) movl %esp, %eax call ftrace_return_to_handler movl %eax, %ecx - popl %eax - popl %edx - addl $4, %esp # skip ebp + movl %eax, PT_EAX(%esp) + movl %edx, PT_EDX(%esp) + addl $(FRAME_SIZE), %esp JMP_NOSPEC ecx #endif diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S index 214f30e9f0c0..d51647228596 100644 --- a/arch/x86/kernel/ftrace_64.S +++ b/arch/x86/kernel/ftrace_64.S @@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__) SYM_CODE_START(return_to_handler) UNWIND_HINT_UNDEFINED ANNOTATE_NOENDBR - subq $24, %rsp - /* Save the return values */ - movq %rax, (%rsp) - movq %rdx, 8(%rsp) - movq %rbp, 16(%rsp) + /* Save ftrace_regs for function exit context */ + subq $(FRAME_SIZE), %rsp + + movq %rax, RAX(%rsp) + movq %rdx, RDX(%rsp) + movq %rbp, RBP(%rsp) movq %rsp, %rdi call ftrace_return_to_handler movq %rax, %rdi - movq 8(%rsp), %rdx - movq (%rsp), %rax + movq RDX(%rsp), %rdx + movq RAX(%rsp), %rax - addq $24, %rsp + addq $(FRAME_SIZE), %rsp /* * Jump back to the old return address. This cannot be JMP_NOSPEC rdi * since IBT would demand that contain ENDBR, which simply isn't so for diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 1fe49a28de2d..13987cd63553 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -43,9 +43,8 @@ struct dyn_ftrace; char *arch_ftrace_match_adjust(char *str, const char *search); -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL -struct fgraph_ret_regs; -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs); +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs); #else unsigned long ftrace_return_to_handler(unsigned long frame_pointer); #endif @@ -134,6 +133,13 @@ extern int ftrace_enabled; * Also, architecture dependent fields can be used for internal process. * (e.g. orig_ax on x86_64) * + * Basically, ftrace_regs stores the registers related to the context. + * On function entry, registers for function parameters and hooking the + * function call are stored, and on function exit, registers for function + * return value and frame pointers are stored. + * + * And also, it dpends on the context that which registers are restored + * from the ftrace_regs. * On the function entry, those registers will be restored except for * the stack pointer, so that user can change the function parameters * and instruction pointer (e.g. live patching.) @@ -191,6 +197,8 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs) override_function_with_return(ftrace_get_regs(fregs)) #define ftrace_regs_query_register_offset(name) \ regs_query_register_offset(name) +#define ftrace_regs_get_frame_pointer(fregs) \ + frame_pointer(&(fregs)->regs) #endif typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip, diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index 721c3b221048..ab277eff80dc 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER help See Documentation/trace/ftrace-design.rst -config HAVE_FUNCTION_GRAPH_RETVAL +config HAVE_FUNCTION_GRAPH_FREGS bool config HAVE_DYNAMIC_FTRACE @@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER config FUNCTION_GRAPH_RETVAL bool "Kernel Function Graph Return Value" - depends on HAVE_FUNCTION_GRAPH_RETVAL + depends on HAVE_FUNCTION_GRAPH_FREGS depends on FUNCTION_GRAPH_TRACER default n help diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c index 0322c5723748..30bebe43607d 100644 --- a/kernel/trace/fgraph.c +++ b/kernel/trace/fgraph.c @@ -763,15 +763,12 @@ static struct notifier_block ftrace_suspend_notifier = { .notifier_call = ftrace_suspend_notifier_call, }; -/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */ -struct fgraph_ret_regs; - /* * Send the trace to the ring-buffer. * @return the original return address. */ -static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs, - unsigned long frame_pointer) +static inline unsigned long +__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer) { struct ftrace_ret_stack *ret_stack; struct ftrace_graph_ret trace; @@ -791,7 +788,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs trace.rettime = trace_clock_local(); #ifdef CONFIG_FUNCTION_GRAPH_RETVAL - trace.retval = fgraph_ret_regs_return_value(ret_regs); + trace.retval = ftrace_regs_get_return_value(fregs); #endif bitmap = get_bitmap_bits(current, offset); @@ -826,14 +823,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs } /* - * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can - * leave only ftrace_return_to_handler(ret_regs). + * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can + * leave only ftrace_return_to_handler(fregs). */ -#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL -unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs) +#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS +unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs) { - return __ftrace_return_to_handler(ret_regs, - fgraph_ret_regs_frame_pointer(ret_regs)); + return __ftrace_return_to_handler(fregs, + ftrace_regs_get_frame_pointer(fregs)); } #else unsigned long ftrace_return_to_handler(unsigned long frame_pointer)