Messages in this thread | | | Date | Mon, 18 Apr 2022 12:10:04 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH v1 08/22] powerpc/ftrace: Make __ftrace_make_{nop/call}() common to PPC32 and PPC64 |
| |
Christophe Leroy wrote: > Since c93d4f6ecf4b ("powerpc/ftrace: Add module_trampoline_target() > for PPC32"), __ftrace_make_nop() for PPC32 is very similar to the > one for PPC64. > > Same for __ftrace_make_call(). > > Make them common. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/kernel/trace/ftrace.c | 108 +++-------------------------- > 1 file changed, 8 insertions(+), 100 deletions(-) > > diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c > index 1b05d33f96c6..2c7e42e439bb 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -114,7 +114,6 @@ static unsigned long find_bl_target(unsigned long ip, ppc_inst_t op) > } > > #ifdef CONFIG_MODULES > -#ifdef CONFIG_PPC64 > static int > __ftrace_make_nop(struct module *mod, > struct dyn_ftrace *rec, unsigned long addr) > @@ -154,10 +153,11 @@ __ftrace_make_nop(struct module *mod, > return -EINVAL; > } > > -#ifdef CONFIG_MPROFILE_KERNEL > - /* When using -mkernel_profile there is no load to jump over */ > + /* When using -mkernel_profile or PPC32 there is no load to jump over */ -mprofile-kernel
Since you are modifying that line anyway ^^
> pop = ppc_inst(PPC_RAW_NOP()); > > +#ifdef CONFIG_PPC64 > +#ifdef CONFIG_MPROFILE_KERNEL > if (copy_inst_from_kernel_nofault(&op, (void *)(ip - 4))) { > pr_err("Fetching instruction at %lx failed.\n", ip - 4); > return -EFAULT; > @@ -201,6 +201,7 @@ __ftrace_make_nop(struct module *mod, > return -EINVAL; > } > #endif /* CONFIG_MPROFILE_KERNEL */ > +#endif /* PPC64 */ > > if (patch_instruction((u32 *)ip, pop)) { > pr_err("Patching NOP failed.\n"); > @@ -209,48 +210,6 @@ __ftrace_make_nop(struct module *mod, > > return 0; > } > - > -#else /* !PPC64 */ > -static int > -__ftrace_make_nop(struct module *mod, > - struct dyn_ftrace *rec, unsigned long addr) > -{ > - ppc_inst_t op; > - unsigned long ip = rec->ip; > - unsigned long tramp, ptr; > - > - if (copy_from_kernel_nofault(&op, (void *)ip, MCOUNT_INSN_SIZE)) > - return -EFAULT; > - > - /* Make sure that that this is still a 24bit jump */ > - if (!is_bl_op(op)) { > - pr_err("Not expected bl: opcode is %s\n", ppc_inst_as_str(op)); > - return -EINVAL; > - } > - > - /* lets find where the pointer goes */ > - tramp = find_bl_target(ip, op); > - > - /* Find where the trampoline jumps to */ > - if (module_trampoline_target(mod, tramp, &ptr)) { > - pr_err("Failed to get trampoline target\n"); > - return -EFAULT; > - } > - > - if (ptr != addr) { > - pr_err("Trampoline location %08lx does not match addr\n", > - tramp); > - return -EINVAL; > - } > - > - op = ppc_inst(PPC_RAW_NOP()); > - > - if (patch_instruction((u32 *)ip, op)) > - return -EPERM; > - > - return 0; > -} > -#endif /* PPC64 */ > #endif /* CONFIG_MODULES */ > > static unsigned long find_ftrace_tramp(unsigned long ip) > @@ -437,13 +396,12 @@ int ftrace_make_nop(struct module *mod, > } > > #ifdef CONFIG_MODULES > -#ifdef CONFIG_PPC64 > /* > * Examine the existing instructions for __ftrace_make_call. > * They should effectively be a NOP, and follow formal constraints, > * depending on the ABI. Return false if they don't. > */ > -#ifndef CONFIG_MPROFILE_KERNEL > +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS
It is better to gate this on PPC64_ELF_ABI_v1
> static int > expected_nop_sequence(void *ip, ppc_inst_t op0, ppc_inst_t op1) > { > @@ -465,7 +423,7 @@ expected_nop_sequence(void *ip, ppc_inst_t op0, ppc_inst_t op1) > static int > expected_nop_sequence(void *ip, ppc_inst_t op0, ppc_inst_t op1) > { > - /* look for patched "NOP" on ppc64 with -mprofile-kernel */ > + /* look for patched "NOP" on ppc64 with -mprofile-kernel or ppc32 */ > if (!ppc_inst_equal(op0, ppc_inst(PPC_RAW_NOP()))) > return 0; > return 1; > @@ -484,8 +442,10 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) > if (copy_inst_from_kernel_nofault(op, ip)) > return -EFAULT; > > +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS > if (copy_inst_from_kernel_nofault(op + 1, ip + 4)) > return -EFAULT; > +#endif
Here too...
- Naveen
| |