Re: [PATCH v2 08/13] powerpc/ftrace: Prepare PPC64's ftrace_caller() for CONFIG_DYNAMIC_FTRACE_WITH_ARGS
From: Naveen N. Rao <hidden>
Date: 2022-02-14 15:20:46
Also in:
live-patching, lkml
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 <redacted> --- .../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...
quoted hunk ↗ jump to hunk
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
quoted hunk ↗ jump to hunk
/* 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.
quoted hunk ↗ jump to hunk
+#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