Messages in this thread | | | Date | Mon, 14 Feb 2022 20:49:57 +0530 | From | "Naveen N. Rao" <> | Subject | Re: [PATCH v2 08/13] powerpc/ftrace: Prepare PPC64's ftrace_caller() for CONFIG_DYNAMIC_FTRACE_WITH_ARGS |
| |
Hi Christophe, Thanks for your work enabling DYNAMIC_FTRACE_WITH_ARGS on powerpc. Sorry for the late review on this series, but I have a few comments below.
Christophe Leroy wrote: > In order to implement CONFIG_DYNAMIC_FTRACE_WITH_ARGS, change ftrace_caller() > to handle LIVEPATCH the same way as frace_caller_regs(). > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > .../powerpc/kernel/trace/ftrace_64_mprofile.S | 25 ++++++++++++++----- > 1 file changed, 19 insertions(+), 6 deletions(-)
I think we also need to save r1 into pt_regs so that the stack pointer is available in the callbacks.
Other than that, a few minor nits below...
> > diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > index d636fc755f60..f6f787819273 100644 > --- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > +++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S > @@ -172,14 +172,19 @@ _GLOBAL(ftrace_caller) > addi r3, r3, function_trace_op@toc@l > ld r5, 0(r3) > > +#ifdef CONFIG_LIVEPATCH_64 > + SAVE_GPR(14, r1) > + mr r14,r7 /* remember old NIP */ ^ add a space > +#endif
Please add a blank line here, to match the formatting for the rest of this file.
> /* Calculate ip from nip-4 into r3 for call below */ > subi r3, r7, MCOUNT_INSN_SIZE > > /* Put the original return address in r4 as parent_ip */ > + std r0, _LINK(r1) > mr r4, r0 > > - /* Set pt_regs to NULL */ > - li r6, 0 > + /* Load &pt_regs in r6 for call below */ > + addi r6, r1 ,STACK_FRAME_OVERHEAD ^^ incorrect spacing > > /* ftrace_call(r3, r4, r5, r6) */ > .globl ftrace_call > @@ -189,6 +194,10 @@ ftrace_call: > > ld r3, _NIP(r1) > mtctr r3
Another blank line here.
> +#ifdef CONFIG_LIVEPATCH_64 > + cmpd r14, r3 /* has NIP been altered? */ > + REST_GPR(14, r1) > +#endif > > /* Restore gprs */ > REST_GPRS(3, 10, r1) > @@ -196,13 +205,17 @@ ftrace_call: > /* Restore callee's TOC */ > ld r2, 24(r1) > > + /* Restore possibly modified LR */ > + ld r0, _LINK(r1) > + mtlr r0 > + > /* Pop our stack frame */ > addi r1, r1, SWITCH_FRAME_SIZE > > - /* Reload original LR */ > - ld r0, LRSAVE(r1) > - mtlr r0 > - > +#ifdef CONFIG_LIVEPATCH_64 > + /* Based on the cmpd above, if the NIP was altered handle livepatch */ > + bne- livepatch_handler > +#endif
Here too.
> /* Handle function_graph or go back */ > b ftrace_caller_common >
- Naveen
| |