| Message ID | 850817333cc76593699032e8e9a70d8c36e1af1e.1640017960.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Implement livepatch on PPC32 and more | expand |
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
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 */ +#endif /* 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 /* ftrace_call(r3, r4, r5, r6) */ .globl ftrace_call @@ -189,6 +194,10 @@ ftrace_call: ld r3, _NIP(r1) mtctr r3 +#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 /* Handle function_graph or go back */ b ftrace_caller_common
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(-)