Re: [PATCH] riscv: ftrace: Fix to pass correct ftrace_regs to ftrace_func_t functions
Hi, Sorry please ignore this. I found that the arch/riscv doesn't support HAVE_DYNAMIC_FTRACE_WITH_ARGS. Thus this might add a new feature support. Let me update it. Thank you, On Sun, 15 Oct 2023 23:15:23 +0900 Masami Hiramatsu (Google) wrote: > Hi, > > Gentry ping. > > I think this should be an important fix because if a fprobe handler without > FTRACE_OPS_FL_SAVE_REGS tries to access any register via ftrace_regs, that > will get a wrong value. > > Thank you, > > On Mon, 2 Oct 2023 21:50:34 +0900 > "Masami Hiramatsu (Google)" wrote: > > > From: Masami Hiramatsu (Google) > > > > Since ftrace_func_t requires to pass 'struct ftrace_regs *' as the 4th > > argument even if FTRACE_OPS_FL_SAVE_REGS is not set, ftrace_caller must > > pass 'struct ftrace_regs *', which is a partial pt_regs, on the stack > > to the ftrace_func_t functions, so that the ftrace_func_t functions can > > access some partial registers. > > > > Fix to allocate 'struct ftrace_regs' (which has the same size of 'struct > > pt_regs') on the stack and save partial (argument) registers on it > > instead of reduced size custom data structure. > > > > Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of > > MCOUNT") > > Signed-off-by: Masami Hiramatsu (Google) > > --- > > arch/riscv/kernel/mcount-dyn.S | 65 > > +--- > > 1 file changed, 28 insertions(+), 37 deletions(-) > > > > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > > index 669b8697aa38..84963680eff4 100644 > > --- a/arch/riscv/kernel/mcount-dyn.S > > +++ b/arch/riscv/kernel/mcount-dyn.S > > @@ -14,46 +14,37 @@ > > .text > > > > #define FENTRY_RA_OFFSET 8 > > -#define ABI_SIZE_ON_STACK 80 > > -#define ABI_A0 0 > > -#define ABI_A1 8 > > -#define ABI_A2 16 > > -#define ABI_A3 24 > > -#define ABI_A4 32 > > -#define ABI_A5 40 > > -#define ABI_A6 48 > > -#define ABI_A7 56 > > -#define ABI_T0 64 > > -#define ABI_RA 72 > > > > .macro SAVE_ABI > > - addisp, sp, -ABI_SIZE_ON_STACK > > - > > - REG_S a0, ABI_A0(sp) > > - REG_S a1, ABI_A1(sp) > > - REG_S a2, ABI_A2(sp) > > - REG_S a3, ABI_A3(sp) > > - REG_S a4, ABI_A4(sp) > > - REG_S a5, ABI_A5(sp) > > - REG_S a6, ABI_A6(sp) > > - REG_S a7, ABI_A7(sp) > > - REG_S t0, ABI_T0(sp) > > - REG_S ra, ABI_RA(sp) > > + addisp, sp, -PT_SIZE_ON_STACK > > + > > + /* Save t0 as epc for ftrace_regs_get_instruction_pointer() */ > > + REG_S t0, PT_EPC(sp) > > + REG_S a0, PT_A0(sp) > > + REG_S a1, PT_A1(sp) > > + REG_S a2, PT_A2(sp) > > + REG_S a3, PT_A3(sp) > > + REG_S a4, PT_A4(sp) > > + REG_S a5, PT_A5(sp) > > + REG_S a6, PT_A6(sp) > > + REG_S a7, PT_A7(sp) > > + REG_S t0, PT_T0(sp) > > + REG_S ra, PT_RA(sp) > > .endm > > > > .macro RESTORE_ABI > > - REG_L a0, ABI_A0(sp) > > - REG_L a1, ABI_A1(sp) > > - REG_L a2, ABI_A2(sp) > > - REG_L a3, ABI_A3(sp) > > - REG_L a4, ABI_A4(sp) > > - REG_L a5, ABI_A5(sp) > > - REG_L a6, ABI_A6(sp) > > - REG_L a7, ABI_A7(sp) > > - REG_L t0, ABI_T0(sp) > > - REG_L ra, ABI_RA(sp) > > - > > - addisp, sp, ABI_SIZE_ON_STACK > > + REG_L a0, PT_A0(sp) > > + REG_L a1, PT_A1(sp) > > + REG_L a2, PT_A2(sp) > > + REG_L a3, PT_A3(sp) > > + REG_L a4, PT_A4(sp) > > + REG_L a5, PT_A5(sp) > > + REG_L a6, PT_A6(sp) > > + REG_L a7, PT_A7(sp) > > + REG_L t0, PT_T0(sp) > > + REG_L ra, PT_RA(sp) > > + > > + addisp, sp, PT_SIZE_ON_STACK > > .endm > > > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS > > @@ -96,8 +87,8 @@ ftrace_call: > > callftrace_stub > > > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > - addia0, sp, ABI_RA > > - REG_L a1, ABI_T0(sp) > > + addia0, sp, PT_RA > > + REG_L a1, PT_T0(sp) > > addia1, a1, -FENTRY_RA_OFFSET > > #ifdef HAVE_FUNCTION_GRAPH_FP_TEST > > mv a2, s0 > > > > > > > -- > Masami Hiramatsu (Google) -- Masami Hiramatsu (Google)
Re: [PATCH] riscv: ftrace: Fix to pass correct ftrace_regs to ftrace_func_t functions
Hi, Gentry ping. I think this should be an important fix because if a fprobe handler without FTRACE_OPS_FL_SAVE_REGS tries to access any register via ftrace_regs, that will get a wrong value. Thank you, On Mon, 2 Oct 2023 21:50:34 +0900 "Masami Hiramatsu (Google)" wrote: > From: Masami Hiramatsu (Google) > > Since ftrace_func_t requires to pass 'struct ftrace_regs *' as the 4th > argument even if FTRACE_OPS_FL_SAVE_REGS is not set, ftrace_caller must > pass 'struct ftrace_regs *', which is a partial pt_regs, on the stack > to the ftrace_func_t functions, so that the ftrace_func_t functions can > access some partial registers. > > Fix to allocate 'struct ftrace_regs' (which has the same size of 'struct > pt_regs') on the stack and save partial (argument) registers on it > instead of reduced size custom data structure. > > Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of > MCOUNT") > Signed-off-by: Masami Hiramatsu (Google) > --- > arch/riscv/kernel/mcount-dyn.S | 65 > +--- > 1 file changed, 28 insertions(+), 37 deletions(-) > > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S > index 669b8697aa38..84963680eff4 100644 > --- a/arch/riscv/kernel/mcount-dyn.S > +++ b/arch/riscv/kernel/mcount-dyn.S > @@ -14,46 +14,37 @@ > .text > > #define FENTRY_RA_OFFSET 8 > -#define ABI_SIZE_ON_STACK80 > -#define ABI_A0 0 > -#define ABI_A1 8 > -#define ABI_A2 16 > -#define ABI_A3 24 > -#define ABI_A4 32 > -#define ABI_A5 40 > -#define ABI_A6 48 > -#define ABI_A7 56 > -#define ABI_T0 64 > -#define ABI_RA 72 > > .macro SAVE_ABI > - addisp, sp, -ABI_SIZE_ON_STACK > - > - REG_S a0, ABI_A0(sp) > - REG_S a1, ABI_A1(sp) > - REG_S a2, ABI_A2(sp) > - REG_S a3, ABI_A3(sp) > - REG_S a4, ABI_A4(sp) > - REG_S a5, ABI_A5(sp) > - REG_S a6, ABI_A6(sp) > - REG_S a7, ABI_A7(sp) > - REG_S t0, ABI_T0(sp) > - REG_S ra, ABI_RA(sp) > + addisp, sp, -PT_SIZE_ON_STACK > + > + /* Save t0 as epc for ftrace_regs_get_instruction_pointer() */ > + REG_S t0, PT_EPC(sp) > + REG_S a0, PT_A0(sp) > + REG_S a1, PT_A1(sp) > + REG_S a2, PT_A2(sp) > + REG_S a3, PT_A3(sp) > + REG_S a4, PT_A4(sp) > + REG_S a5, PT_A5(sp) > + REG_S a6, PT_A6(sp) > + REG_S a7, PT_A7(sp) > + REG_S t0, PT_T0(sp) > + REG_S ra, PT_RA(sp) > .endm > > .macro RESTORE_ABI > - REG_L a0, ABI_A0(sp) > - REG_L a1, ABI_A1(sp) > - REG_L a2, ABI_A2(sp) > - REG_L a3, ABI_A3(sp) > - REG_L a4, ABI_A4(sp) > - REG_L a5, ABI_A5(sp) > - REG_L a6, ABI_A6(sp) > - REG_L a7, ABI_A7(sp) > - REG_L t0, ABI_T0(sp) > - REG_L ra, ABI_RA(sp) > - > - addisp, sp, ABI_SIZE_ON_STACK > + REG_L a0, PT_A0(sp) > + REG_L a1, PT_A1(sp) > + REG_L a2, PT_A2(sp) > + REG_L a3, PT_A3(sp) > + REG_L a4, PT_A4(sp) > + REG_L a5, PT_A5(sp) > + REG_L a6, PT_A6(sp) > + REG_L a7, PT_A7(sp) > + REG_L t0, PT_T0(sp) > + REG_L ra, PT_RA(sp) > + > + addisp, sp, PT_SIZE_ON_STACK > .endm > > #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS > @@ -96,8 +87,8 @@ ftrace_call: > callftrace_stub > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > - addia0, sp, ABI_RA > - REG_L a1, ABI_T0(sp) > + addia0, sp, PT_RA > + REG_L a1, PT_T0(sp) > addia1, a1, -FENTRY_RA_OFFSET > #ifdef HAVE_FUNCTION_GRAPH_FP_TEST > mv a2, s0 > > -- Masami Hiramatsu (Google)
[PATCH] riscv: ftrace: Fix to pass correct ftrace_regs to ftrace_func_t functions
From: Masami Hiramatsu (Google) Since ftrace_func_t requires to pass 'struct ftrace_regs *' as the 4th argument even if FTRACE_OPS_FL_SAVE_REGS is not set, ftrace_caller must pass 'struct ftrace_regs *', which is a partial pt_regs, on the stack to the ftrace_func_t functions, so that the ftrace_func_t functions can access some partial registers. Fix to allocate 'struct ftrace_regs' (which has the same size of 'struct pt_regs') on the stack and save partial (argument) registers on it instead of reduced size custom data structure. Fixes: afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") Signed-off-by: Masami Hiramatsu (Google) --- arch/riscv/kernel/mcount-dyn.S | 65 +--- 1 file changed, 28 insertions(+), 37 deletions(-) diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S index 669b8697aa38..84963680eff4 100644 --- a/arch/riscv/kernel/mcount-dyn.S +++ b/arch/riscv/kernel/mcount-dyn.S @@ -14,46 +14,37 @@ .text #define FENTRY_RA_OFFSET 8 -#define ABI_SIZE_ON_STACK 80 -#define ABI_A0 0 -#define ABI_A1 8 -#define ABI_A2 16 -#define ABI_A3 24 -#define ABI_A4 32 -#define ABI_A5 40 -#define ABI_A6 48 -#define ABI_A7 56 -#define ABI_T0 64 -#define ABI_RA 72 .macro SAVE_ABI - addisp, sp, -ABI_SIZE_ON_STACK - - REG_S a0, ABI_A0(sp) - REG_S a1, ABI_A1(sp) - REG_S a2, ABI_A2(sp) - REG_S a3, ABI_A3(sp) - REG_S a4, ABI_A4(sp) - REG_S a5, ABI_A5(sp) - REG_S a6, ABI_A6(sp) - REG_S a7, ABI_A7(sp) - REG_S t0, ABI_T0(sp) - REG_S ra, ABI_RA(sp) + addisp, sp, -PT_SIZE_ON_STACK + + /* Save t0 as epc for ftrace_regs_get_instruction_pointer() */ + REG_S t0, PT_EPC(sp) + REG_S a0, PT_A0(sp) + REG_S a1, PT_A1(sp) + REG_S a2, PT_A2(sp) + REG_S a3, PT_A3(sp) + REG_S a4, PT_A4(sp) + REG_S a5, PT_A5(sp) + REG_S a6, PT_A6(sp) + REG_S a7, PT_A7(sp) + REG_S t0, PT_T0(sp) + REG_S ra, PT_RA(sp) .endm .macro RESTORE_ABI - REG_L a0, ABI_A0(sp) - REG_L a1, ABI_A1(sp) - REG_L a2, ABI_A2(sp) - REG_L a3, ABI_A3(sp) - REG_L a4, ABI_A4(sp) - REG_L a5, ABI_A5(sp) - REG_L a6, ABI_A6(sp) - REG_L a7, ABI_A7(sp) - REG_L t0, ABI_T0(sp) - REG_L ra, ABI_RA(sp) - - addisp, sp, ABI_SIZE_ON_STACK + REG_L a0, PT_A0(sp) + REG_L a1, PT_A1(sp) + REG_L a2, PT_A2(sp) + REG_L a3, PT_A3(sp) + REG_L a4, PT_A4(sp) + REG_L a5, PT_A5(sp) + REG_L a6, PT_A6(sp) + REG_L a7, PT_A7(sp) + REG_L t0, PT_T0(sp) + REG_L ra, PT_RA(sp) + + addisp, sp, PT_SIZE_ON_STACK .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS @@ -96,8 +87,8 @@ ftrace_call: callftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addia0, sp, ABI_RA - REG_L a1, ABI_T0(sp) + addia0, sp, PT_RA + REG_L a1, PT_T0(sp) addia1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0