Message ID | 18ce6708d6f8c71d87436f9c6019f04df4125128.1652074503.git.christophe.leroy@csgroup.eu (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | powerpc: ftrace optimisation and cleanup and more [v3] | expand |
Christophe Leroy wrote: > A lot of #ifdefs can be replaced by IS_ENABLED() > > Do so. > > This requires to have kernel_toc_addr() defined at all time > as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() > --- > arch/powerpc/include/asm/code-patching.h | 2 - > arch/powerpc/include/asm/module.h | 2 - > arch/powerpc/include/asm/sections.h | 24 +-- > arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ > 4 files changed, 103 insertions(+), 107 deletions(-) > <snip> > @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) > > #ifdef CONFIG_PPC64 > #define PACATOC offsetof(struct paca_struct, kernel_toc) > +#else > +#define PACATOC 0 > +#endif This conflicts with my fix for the ftrace init tramp: https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can get rid of the PACATOC. Here is an incremental diff: diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index da1a2f8ebb72f3..28169a1ccc7377 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -701,11 +701,6 @@ void arch_ftrace_update_code(int command) } #ifdef CONFIG_PPC64 -#define PACATOC offsetof(struct paca_struct, kernel_toc) -#else -#define PACATOC 0 -#endif - extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; void ftrace_free_init_tramp(void) @@ -724,7 +719,7 @@ int __init ftrace_dyn_arch_init(void) int i; unsigned int *tramp[] = { ftrace_tramp_text, ftrace_tramp_init }; u32 stub_insns[] = { - PPC_RAW_LD(_R12, _R13, PACATOC), + PPC_RAW_LD(_R12, _R13, offsetof(struct paca_struct, kernel_toc)), PPC_RAW_ADDIS(_R12, _R12, 0), PPC_RAW_ADDI(_R12, _R12, 0), PPC_RAW_MTCTR(_R12), @@ -733,9 +728,6 @@ int __init ftrace_dyn_arch_init(void) unsigned long addr; long reladdr; - if (IS_ENABLED(CONFIG_PPC32)) - return 0; - addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR); reladdr = addr - kernel_toc_addr(); @@ -754,6 +746,7 @@ int __init ftrace_dyn_arch_init(void) return 0; } +#endif #ifdef CONFIG_FUNCTION_GRAPH_TRACER - Naveen
Le 18/05/2022 à 11:45, Naveen N. Rao a écrit : > Christophe Leroy wrote: >> A lot of #ifdefs can be replaced by IS_ENABLED() >> >> Do so. >> >> This requires to have kernel_toc_addr() defined at all time >> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v2: Moved the setup of pop outside of the big if()/else() in >> __ftrace_make_nop() >> --- >> arch/powerpc/include/asm/code-patching.h | 2 - >> arch/powerpc/include/asm/module.h | 2 - >> arch/powerpc/include/asm/sections.h | 24 +-- >> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >> 4 files changed, 103 insertions(+), 107 deletions(-) >> > > <snip> > >> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >> >> #ifdef CONFIG_PPC64 >> #define PACATOC offsetof(struct paca_struct, kernel_toc) >> +#else >> +#define PACATOC 0 >> +#endif > > This conflicts with my fix for the ftrace init tramp: > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ > > > It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can > get rid of the PACATOC. Here is an incremental diff: Ah yes, it makes sense. Initial purpose was to de-duplicate ftrace_dyn_arch_init(), but as ftrace_dyn_arch_init() is defined as a weak nop function in kernel/trace/ftrace.c we don't need it for PPC32 at all. And then kernel_toc_addr() could remain inside #ifdef CONFIG_PPC64 in asm/section.h > > diff --git a/arch/powerpc/kernel/trace/ftrace.c > b/arch/powerpc/kernel/trace/ftrace.c > index da1a2f8ebb72f3..28169a1ccc7377 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -701,11 +701,6 @@ void arch_ftrace_update_code(int command) > } > > #ifdef CONFIG_PPC64 > -#define PACATOC offsetof(struct paca_struct, kernel_toc) > -#else > -#define PACATOC 0 > -#endif > - > extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; > > void ftrace_free_init_tramp(void) > @@ -724,7 +719,7 @@ int __init ftrace_dyn_arch_init(void) > int i; > unsigned int *tramp[] = { ftrace_tramp_text, ftrace_tramp_init }; > u32 stub_insns[] = { > - PPC_RAW_LD(_R12, _R13, PACATOC), > + PPC_RAW_LD(_R12, _R13, offsetof(struct paca_struct, kernel_toc)), > PPC_RAW_ADDIS(_R12, _R12, 0), > PPC_RAW_ADDI(_R12, _R12, 0), > PPC_RAW_MTCTR(_R12), > @@ -733,9 +728,6 @@ int __init ftrace_dyn_arch_init(void) > unsigned long addr; > long reladdr; > > - if (IS_ENABLED(CONFIG_PPC32)) > - return 0; > - > addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR); > reladdr = addr - kernel_toc_addr(); > > @@ -754,6 +746,7 @@ int __init ftrace_dyn_arch_init(void) > > return 0; > } > +#endif > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > > - Naveen
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: > Christophe Leroy wrote: >> A lot of #ifdefs can be replaced by IS_ENABLED() >> >> Do so. >> >> This requires to have kernel_toc_addr() defined at all time >> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() >> --- >> arch/powerpc/include/asm/code-patching.h | 2 - >> arch/powerpc/include/asm/module.h | 2 - >> arch/powerpc/include/asm/sections.h | 24 +-- >> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >> 4 files changed, 103 insertions(+), 107 deletions(-) >> > > <snip> > >> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >> >> #ifdef CONFIG_PPC64 >> #define PACATOC offsetof(struct paca_struct, kernel_toc) >> +#else >> +#define PACATOC 0 >> +#endif > > This conflicts with my fix for the ftrace init tramp: > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ > > It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can > get rid of the PACATOC. Here is an incremental diff: Where is the incremental diff meant to apply? It doesn't apply on top of patch 19, or at the end of the series. cheers > diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c > index da1a2f8ebb72f3..28169a1ccc7377 100644 > --- a/arch/powerpc/kernel/trace/ftrace.c > +++ b/arch/powerpc/kernel/trace/ftrace.c > @@ -701,11 +701,6 @@ void arch_ftrace_update_code(int command) > } > > #ifdef CONFIG_PPC64 > -#define PACATOC offsetof(struct paca_struct, kernel_toc) > -#else > -#define PACATOC 0 > -#endif > - > extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; > > void ftrace_free_init_tramp(void) > @@ -724,7 +719,7 @@ int __init ftrace_dyn_arch_init(void) > int i; > unsigned int *tramp[] = { ftrace_tramp_text, ftrace_tramp_init }; > u32 stub_insns[] = { > - PPC_RAW_LD(_R12, _R13, PACATOC), > + PPC_RAW_LD(_R12, _R13, offsetof(struct paca_struct, kernel_toc)), > PPC_RAW_ADDIS(_R12, _R12, 0), > PPC_RAW_ADDI(_R12, _R12, 0), > PPC_RAW_MTCTR(_R12), > @@ -733,9 +728,6 @@ int __init ftrace_dyn_arch_init(void) > unsigned long addr; > long reladdr; > > - if (IS_ENABLED(CONFIG_PPC32)) > - return 0; > - > addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR); > reladdr = addr - kernel_toc_addr(); > > @@ -754,6 +746,7 @@ int __init ftrace_dyn_arch_init(void) > > return 0; > } > +#endif > > #ifdef CONFIG_FUNCTION_GRAPH_TRACER > > > - Naveen
Michael Ellerman <mpe@ellerman.id.au> writes: > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: >> Christophe Leroy wrote: >>> A lot of #ifdefs can be replaced by IS_ENABLED() >>> >>> Do so. >>> >>> This requires to have kernel_toc_addr() defined at all time >>> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() >>> --- >>> arch/powerpc/include/asm/code-patching.h | 2 - >>> arch/powerpc/include/asm/module.h | 2 - >>> arch/powerpc/include/asm/sections.h | 24 +-- >>> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >>> 4 files changed, 103 insertions(+), 107 deletions(-) >>> >> >> <snip> >> >>> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >>> >>> #ifdef CONFIG_PPC64 >>> #define PACATOC offsetof(struct paca_struct, kernel_toc) >>> +#else >>> +#define PACATOC 0 >>> +#endif >> >> This conflicts with my fix for the ftrace init tramp: >> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ >> >> It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can >> get rid of the PACATOC. Here is an incremental diff: > > Where is the incremental diff meant to apply? > > It doesn't apply on top of patch 19, or at the end of the series. I think I worked out what you meant. Can you check what's in next-test: https://github.com/linuxppc/linux/commits/next-test cheers
Le 18/05/2022 à 13:19, Michael Ellerman a écrit : > "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: >> Christophe Leroy wrote: >>> A lot of #ifdefs can be replaced by IS_ENABLED() >>> >>> Do so. >>> >>> This requires to have kernel_toc_addr() defined at all time >>> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() >>> --- >>> arch/powerpc/include/asm/code-patching.h | 2 - >>> arch/powerpc/include/asm/module.h | 2 - >>> arch/powerpc/include/asm/sections.h | 24 +-- >>> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >>> 4 files changed, 103 insertions(+), 107 deletions(-) >>> >> >> <snip> >> >>> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >>> >>> #ifdef CONFIG_PPC64 >>> #define PACATOC offsetof(struct paca_struct, kernel_toc) >>> +#else >>> +#define PACATOC 0 >>> +#endif >> >> This conflicts with my fix for the ftrace init tramp: >> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ >> >> It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can >> get rid of the PACATOC. Here is an incremental diff: > > Where is the incremental diff meant to apply? > > It doesn't apply on top of patch 19, or at the end of the series. White space damage it seems. I'll send you a proper fixup for that patch in a few minutes. > > cheers > >> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c >> index da1a2f8ebb72f3..28169a1ccc7377 100644 >> --- a/arch/powerpc/kernel/trace/ftrace.c >> +++ b/arch/powerpc/kernel/trace/ftrace.c >> @@ -701,11 +701,6 @@ void arch_ftrace_update_code(int command) >> } >> >> #ifdef CONFIG_PPC64 >> -#define PACATOC offsetof(struct paca_struct, kernel_toc) >> -#else >> -#define PACATOC 0 >> -#endif >> - >> extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; >> >> void ftrace_free_init_tramp(void) >> @@ -724,7 +719,7 @@ int __init ftrace_dyn_arch_init(void) >> int i; >> unsigned int *tramp[] = { ftrace_tramp_text, ftrace_tramp_init }; >> u32 stub_insns[] = { >> - PPC_RAW_LD(_R12, _R13, PACATOC), >> + PPC_RAW_LD(_R12, _R13, offsetof(struct paca_struct, kernel_toc)), >> PPC_RAW_ADDIS(_R12, _R12, 0), >> PPC_RAW_ADDI(_R12, _R12, 0), >> PPC_RAW_MTCTR(_R12), >> @@ -733,9 +728,6 @@ int __init ftrace_dyn_arch_init(void) >> unsigned long addr; >> long reladdr; >> >> - if (IS_ENABLED(CONFIG_PPC32)) >> - return 0; >> - >> addr = ppc_global_function_entry((void *)FTRACE_REGS_ADDR); >> reladdr = addr - kernel_toc_addr(); >> >> @@ -754,6 +746,7 @@ int __init ftrace_dyn_arch_init(void) >> >> return 0; >> } >> +#endif >> >> #ifdef CONFIG_FUNCTION_GRAPH_TRACER >> >> >> - Naveen
Le 18/05/2022 à 14:03, Michael Ellerman a écrit : > Michael Ellerman <mpe@ellerman.id.au> writes: >> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: >>> Christophe Leroy wrote: >>>> A lot of #ifdefs can be replaced by IS_ENABLED() >>>> >>>> Do so. >>>> >>>> This requires to have kernel_toc_addr() defined at all time >>>> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >>>> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>> --- >>>> v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() >>>> --- >>>> arch/powerpc/include/asm/code-patching.h | 2 - >>>> arch/powerpc/include/asm/module.h | 2 - >>>> arch/powerpc/include/asm/sections.h | 24 +-- >>>> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >>>> 4 files changed, 103 insertions(+), 107 deletions(-) >>>> >>> >>> <snip> >>> >>>> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >>>> >>>> #ifdef CONFIG_PPC64 >>>> #define PACATOC offsetof(struct paca_struct, kernel_toc) >>>> +#else >>>> +#define PACATOC 0 >>>> +#endif >>> >>> This conflicts with my fix for the ftrace init tramp: >>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ >>> >>> It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can >>> get rid of the PACATOC. Here is an incremental diff: >> >> Where is the incremental diff meant to apply? >> >> It doesn't apply on top of patch 19, or at the end of the series. > > I think I worked out what you meant. > > Can you check what's in next-test: > > https://github.com/linuxppc/linux/commits/next-test Yes that looks fine. As Naveen mentioned we can also get rid of PACATOC completely and use offsetof(struct paca_struct, kernel_toc) directly at the only place PACATOC is used. Thanks Christophe
Christophe Leroy wrote: > > > Le 18/05/2022 à 14:03, Michael Ellerman a écrit : >> Michael Ellerman <mpe@ellerman.id.au> writes: >>> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes: >>>> Christophe Leroy wrote: >>>>> A lot of #ifdefs can be replaced by IS_ENABLED() >>>>> >>>>> Do so. >>>>> >>>>> This requires to have kernel_toc_addr() defined at all time >>>>> as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. >>>>> >>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>>> --- >>>>> v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() >>>>> --- >>>>> arch/powerpc/include/asm/code-patching.h | 2 - >>>>> arch/powerpc/include/asm/module.h | 2 - >>>>> arch/powerpc/include/asm/sections.h | 24 +-- >>>>> arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ >>>>> 4 files changed, 103 insertions(+), 107 deletions(-) >>>>> >>>> >>>> <snip> >>>> >>>>> @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) >>>>> >>>>> #ifdef CONFIG_PPC64 >>>>> #define PACATOC offsetof(struct paca_struct, kernel_toc) >>>>> +#else >>>>> +#define PACATOC 0 >>>>> +#endif >>>> >>>> This conflicts with my fix for the ftrace init tramp: >>>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20220516071422.463738-1-naveen.n.rao@linux.vnet.ibm.com/ >>>> >>>> It probably makes sense to retain #ifdef CONFIG_PPC64, so that we can >>>> get rid of the PACATOC. Here is an incremental diff: >>> >>> Where is the incremental diff meant to apply? >>> >>> It doesn't apply on top of patch 19, or at the end of the series. Ugh, sorry. I had an additional patch that converts those ftrace_[regs_]_caller uses to FTRACE_REGS_ADDR, which prevented one of the hunks from applying. >> >> I think I worked out what you meant. >> >> Can you check what's in next-test: >> >> https://github.com/linuxppc/linux/commits/next-test > > Yes that looks fine. +1 > > As Naveen mentioned we can also get rid of PACATOC completely and use > offsetof(struct paca_struct, kernel_toc) directly at the only place > PACATOC is used. Yes, or we can send it out as a separate cleanup. Thanks, Naveen
diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h index 8b1a10868275..3f881548fb61 100644 --- a/arch/powerpc/include/asm/code-patching.h +++ b/arch/powerpc/include/asm/code-patching.h @@ -217,7 +217,6 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name) return addr; } -#ifdef CONFIG_PPC64 /* * Some instruction encodings commonly used in dynamic ftracing * and function live patching. @@ -234,6 +233,5 @@ static inline unsigned long ppc_kallsyms_lookup_name(const char *name) /* usually preceded by a mflr r0 */ #define PPC_INST_STD_LR PPC_RAW_STD(_R0, _R1, PPC_LR_STKOFF) -#endif /* CONFIG_PPC64 */ #endif /* _ASM_POWERPC_CODE_PATCHING_H */ diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h index 857d9ff24295..09e2ffd360bb 100644 --- a/arch/powerpc/include/asm/module.h +++ b/arch/powerpc/include/asm/module.h @@ -41,9 +41,7 @@ struct mod_arch_specific { #ifdef CONFIG_DYNAMIC_FTRACE unsigned long tramp; -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS unsigned long tramp_regs; -#endif #endif /* List of BUG addresses, source line numbers and filenames */ diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h index 8be2c491c733..6980eaeb16fe 100644 --- a/arch/powerpc/include/asm/sections.h +++ b/arch/powerpc/include/asm/sections.h @@ -29,18 +29,6 @@ extern char start_virt_trampolines[]; extern char end_virt_trampolines[]; #endif -/* - * This assumes the kernel is never compiled -mcmodel=small or - * the total .toc is always less than 64k. - */ -static inline unsigned long kernel_toc_addr(void) -{ - unsigned long toc_ptr; - - asm volatile("mr %0, 2" : "=r" (toc_ptr)); - return toc_ptr; -} - static inline int overlaps_interrupt_vector_text(unsigned long start, unsigned long end) { @@ -60,5 +48,17 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end) #endif +/* + * This assumes the kernel is never compiled -mcmodel=small or + * the total .toc is always less than 64k. + */ +static inline unsigned long kernel_toc_addr(void) +{ + unsigned long toc_ptr; + + asm volatile("mr %0, 2" : "=r" (toc_ptr)); + return toc_ptr; +} + #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_SECTIONS_H */ diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c index c34cb394f8a8..5e7a4ed7ad22 100644 --- a/arch/powerpc/kernel/trace/ftrace.c +++ b/arch/powerpc/kernel/trace/ftrace.c @@ -150,26 +150,39 @@ __ftrace_make_nop(struct module *mod, return -EINVAL; } - /* When using -mprofile-kernel or PPC32 there is no load to jump over */ - pop = ppc_inst(PPC_RAW_NOP()); + if (IS_ENABLED(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; + } -#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; - } + /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */ + if (!ppc_inst_equal(op, ppc_inst(PPC_RAW_MFLR(_R0))) && + !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) { + pr_err("Unexpected instruction %s around bl _mcount\n", + ppc_inst_as_str(op)); + return -EINVAL; + } + } else if (IS_ENABLED(CONFIG_PPC64)) { + /* + * Check what is in the next instruction. We can see ld r2,40(r1), but + * on first pass after boot we will see mflr r0. + */ + if (copy_inst_from_kernel_nofault(&op, (void *)(ip + 4))) { + pr_err("Fetching op failed.\n"); + return -EFAULT; + } - /* We expect either a mflr r0, or a std r0, LRSAVE(r1) */ - if (!ppc_inst_equal(op, ppc_inst(PPC_RAW_MFLR(_R0))) && - !ppc_inst_equal(op, ppc_inst(PPC_INST_STD_LR))) { - pr_err("Unexpected instruction %s around bl _mcount\n", - ppc_inst_as_str(op)); - return -EINVAL; + if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) { + pr_err("Expected %08lx found %s\n", PPC_INST_LD_TOC, ppc_inst_as_str(op)); + return -EINVAL; + } } -#else + /* - * Our original call site looks like: + * When using -mprofile-kernel or PPC32 there is no load to jump over. + * + * Otherwise our original call site looks like: * * bl <tramp> * ld r2,XX(r1) @@ -181,24 +194,10 @@ __ftrace_make_nop(struct module *mod, * * Use a b +8 to jump over the load. */ - - pop = ppc_inst(PPC_RAW_BRANCH(8)); /* b +8 */ - - /* - * Check what is in the next instruction. We can see ld r2,40(r1), but - * on first pass after boot we will see mflr r0. - */ - if (copy_inst_from_kernel_nofault(&op, (void *)(ip + 4))) { - pr_err("Fetching op failed.\n"); - return -EFAULT; - } - - if (!ppc_inst_equal(op, ppc_inst(PPC_INST_LD_TOC))) { - pr_err("Expected %08lx found %s\n", PPC_INST_LD_TOC, ppc_inst_as_str(op)); - return -EINVAL; - } -#endif /* CONFIG_MPROFILE_KERNEL */ -#endif /* PPC64 */ + if (IS_ENABLED(CONFIG_MPROFILE_KERNEL) || IS_ENABLED(CONFIG_PPC32)) + pop = ppc_inst(PPC_RAW_NOP()); + else + pop = ppc_inst(PPC_RAW_BRANCH(8)); /* b +8 */ if (patch_instruction((u32 *)ip, pop)) { pr_err("Patching NOP failed.\n"); @@ -207,6 +206,11 @@ __ftrace_make_nop(struct module *mod, return 0; } +#else +static int __ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec, unsigned long addr) +{ + return 0; +} #endif /* CONFIG_MODULES */ static unsigned long find_ftrace_tramp(unsigned long ip) @@ -279,11 +283,11 @@ static int setup_mcount_compiler_tramp(unsigned long tramp) } /* Let's re-write the tramp to go to ftrace_[regs_]caller */ -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - ptr = ppc_global_function_entry((void *)ftrace_regs_caller); -#else - ptr = ppc_global_function_entry((void *)ftrace_caller); -#endif + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS)) + ptr = ppc_global_function_entry((void *)ftrace_regs_caller); + else + ptr = ppc_global_function_entry((void *)ftrace_caller); + if (patch_branch((u32 *)tramp, ptr, 0)) { pr_debug("REL24 out of range!\n"); return -1; @@ -352,10 +356,12 @@ int ftrace_make_nop(struct module *mod, old = ftrace_call_replace(ip, addr, 1); new = ppc_inst(PPC_RAW_NOP()); return ftrace_modify_code(ip, old, new); - } else if (core_kernel_text(ip)) + } else if (core_kernel_text(ip)) { return __ftrace_make_nop_kernel(rec, addr); + } else if (!IS_ENABLED(CONFIG_MODULES)) { + return -EINVAL; + } -#ifdef CONFIG_MODULES /* * Out of range jumps are called from modules. * We should either already have a pointer to the module @@ -378,10 +384,6 @@ int ftrace_make_nop(struct module *mod, mod = rec->arch.mod; return __ftrace_make_nop(mod, rec, addr); -#else - /* We should not get here without modules */ - return -EINVAL; -#endif /* CONFIG_MODULES */ } #ifdef CONFIG_MODULES @@ -411,10 +413,9 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) if (copy_inst_from_kernel_nofault(op, ip)) return -EFAULT; -#ifdef CONFIG_PPC64_ELF_ABI_V1 - if (copy_inst_from_kernel_nofault(op + 1, ip + 4)) + if (IS_ENABLED(CONFIG_PPC64_ELF_ABI_V1) && + copy_inst_from_kernel_nofault(op + 1, ip + 4)) return -EFAULT; -#endif if (!expected_nop_sequence(ip, op[0], op[1])) { pr_err("Unexpected call sequence at %p: %s %s\n", @@ -423,20 +424,15 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) } /* If we never set up ftrace trampoline(s), then bail */ -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - if (!mod->arch.tramp || !mod->arch.tramp_regs) { -#else - if (!mod->arch.tramp) { -#endif + if (!mod->arch.tramp || + (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && !mod->arch.tramp_regs)) { pr_err("No ftrace trampoline\n"); return -EINVAL; } -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - if (rec->flags & FTRACE_FL_REGS) + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && rec->flags & FTRACE_FL_REGS) tramp = mod->arch.tramp_regs; else -#endif tramp = mod->arch.tramp; if (module_trampoline_target(mod, tramp, &ptr)) { @@ -460,6 +456,11 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) return 0; } +#else +static int __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) +{ + return 0; +} #endif /* CONFIG_MODULES */ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr) @@ -472,16 +473,12 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr) entry = ppc_global_function_entry((void *)ftrace_caller); ptr = ppc_global_function_entry((void *)addr); - if (ptr != entry) { -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS + if (ptr != entry && IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS)) entry = ppc_global_function_entry((void *)ftrace_regs_caller); - if (ptr != entry) { -#endif - pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr); - return -EINVAL; -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - } -#endif + + if (ptr != entry) { + pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr); + return -EINVAL; } /* Make sure we have a nop */ @@ -524,10 +521,13 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) old = ppc_inst(PPC_RAW_NOP()); new = ftrace_call_replace(ip, addr, 1); return ftrace_modify_code(ip, old, new); - } else if (core_kernel_text(ip)) + } else if (core_kernel_text(ip)) { return __ftrace_make_call_kernel(rec, addr); + } else if (!IS_ENABLED(CONFIG_MODULES)) { + /* We should not get here without modules */ + return -EINVAL; + } -#ifdef CONFIG_MODULES /* * Out of range jumps are called from modules. * Being that we are converting from nop, it had better @@ -539,10 +539,6 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr) } return __ftrace_make_call(rec, addr); -#else - /* We should not get here without modules */ - return -EINVAL; -#endif /* CONFIG_MODULES */ } #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS @@ -633,6 +629,11 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, return 0; } +#else +static int __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) +{ + return 0; +} #endif int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, @@ -657,9 +658,11 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, * variant, so there is nothing to do here */ return 0; + } else if (!IS_ENABLED(CONFIG_MODULES)) { + /* We should not get here without modules */ + return -EINVAL; } -#ifdef CONFIG_MODULES /* * Out of range jumps are called from modules. */ @@ -669,10 +672,6 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, } return __ftrace_modify_call(rec, old_addr, addr); -#else - /* We should not get here without modules */ - return -EINVAL; -#endif /* CONFIG_MODULES */ } #endif @@ -686,15 +685,13 @@ int ftrace_update_ftrace_func(ftrace_func_t func) new = ftrace_call_replace(ip, (unsigned long)func, 1); ret = ftrace_modify_code(ip, old, new); -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS /* Also update the regs callback function */ - if (!ret) { + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS) && !ret) { ip = (unsigned long)(&ftrace_regs_call); old = ppc_inst_read((u32 *)&ftrace_regs_call); new = ftrace_call_replace(ip, (unsigned long)func, 1); ret = ftrace_modify_code(ip, old, new); } -#endif return ret; } @@ -710,6 +707,9 @@ void arch_ftrace_update_code(int command) #ifdef CONFIG_PPC64 #define PACATOC offsetof(struct paca_struct, kernel_toc) +#else +#define PACATOC 0 +#endif extern unsigned int ftrace_tramp_text[], ftrace_tramp_init[]; @@ -724,12 +724,18 @@ int __init ftrace_dyn_arch_init(void) PPC_RAW_MTCTR(_R12), PPC_RAW_BCTR() }; -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - unsigned long addr = ppc_global_function_entry((void *)ftrace_regs_caller); -#else - unsigned long addr = ppc_global_function_entry((void *)ftrace_caller); -#endif - long reladdr = addr - kernel_toc_addr(); + unsigned long addr; + long reladdr; + + if (IS_ENABLED(CONFIG_PPC32)) + return 0; + + if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_REGS)) + addr = ppc_global_function_entry((void *)ftrace_regs_caller); + else + addr = ppc_global_function_entry((void *)ftrace_caller); + + reladdr = addr - kernel_toc_addr(); if (reladdr >= SZ_2G || reladdr < -SZ_2G) { pr_err("Address of %ps out of range of kernel_toc.\n", @@ -746,12 +752,6 @@ int __init ftrace_dyn_arch_init(void) return 0; } -#else -int __init ftrace_dyn_arch_init(void) -{ - return 0; -} -#endif #ifdef CONFIG_FUNCTION_GRAPH_TRACER
A lot of #ifdefs can be replaced by IS_ENABLED() Do so. This requires to have kernel_toc_addr() defined at all time as well as PPC_INST_LD_TOC and PPC_INST_STD_LR. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: Moved the setup of pop outside of the big if()/else() in __ftrace_make_nop() --- arch/powerpc/include/asm/code-patching.h | 2 - arch/powerpc/include/asm/module.h | 2 - arch/powerpc/include/asm/sections.h | 24 +-- arch/powerpc/kernel/trace/ftrace.c | 182 +++++++++++------------ 4 files changed, 103 insertions(+), 107 deletions(-)