diff mbox series

[v2,11/13] powerpc/ftrace: directly call of function graph tracer by ftrace caller

Message ID 04d196585ff81bde06a000bd9c633a33a5b21130.1640017960.git.christophe.leroy@csgroup.eu (mailing list archive)
State Accepted
Headers show
Series Implement livepatch on PPC32 and more | expand

Commit Message

Christophe Leroy Dec. 20, 2021, 4:38 p.m. UTC 
Modify function graph tracer to be handled directly by the standard
ftrace caller.

This is made possible as powerpc now supports
CONFIG_DYNAMIC_FTRACE_WITH_ARGS.

This change simplifies the call of function graph ftrace.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/ftrace.h             |  6 ++
 arch/powerpc/kernel/trace/ftrace.c            | 11 ++++
 arch/powerpc/kernel/trace/ftrace_32.S         | 53 +--------------
 .../powerpc/kernel/trace/ftrace_64_mprofile.S | 64 +------------------
 4 files changed, 20 insertions(+), 114 deletions(-)

Comments

Naveen N. Rao Feb. 14, 2022, 5:24 p.m. UTC | #1 
Christophe Leroy wrote:
> Modify function graph tracer to be handled directly by the standard
> ftrace caller.
> 
> This is made possible as powerpc now supports
> CONFIG_DYNAMIC_FTRACE_WITH_ARGS.
> 
> This change simplifies the call of function graph ftrace.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/include/asm/ftrace.h             |  6 ++
>  arch/powerpc/kernel/trace/ftrace.c            | 11 ++++
>  arch/powerpc/kernel/trace/ftrace_32.S         | 53 +--------------
>  .../powerpc/kernel/trace/ftrace_64_mprofile.S | 64 +------------------
>  4 files changed, 20 insertions(+), 114 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
> index 45c3d6f11daa..70b457097098 100644
> --- a/arch/powerpc/include/asm/ftrace.h
> +++ b/arch/powerpc/include/asm/ftrace.h
> @@ -38,6 +38,12 @@ static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
>  {
>  	regs_set_return_ip(&fregs->regs, ip);
>  }
> +
> +struct ftrace_ops;
> +
> +#define ftrace_graph_func ftrace_graph_func
> +void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> +		       struct ftrace_ops *op, struct ftrace_regs *fregs);
>  #endif
>  #endif /* __ASSEMBLY__ */
>  
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index ce673764cb69..74a176e394ef 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -917,6 +917,9 @@ static int ftrace_modify_ftrace_graph_caller(bool enable)
>  	unsigned long stub = (unsigned long)(&ftrace_graph_stub);
>  	ppc_inst_t old, new;
>  
> +	if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS))
> +		return 0;
> +
>  	old = ftrace_call_replace(ip, enable ? stub : addr, 0);
>  	new = ftrace_call_replace(ip, enable ? addr : stub, 0);
>  
> @@ -955,6 +958,14 @@ unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
>  out:
>  	return parent;
>  }

For x86, commit 0c0593b45c9b4e ("x86/ftrace: Make function graph use 
ftrace directly") also adds recursion check before the call to 
function_graph_enter() in prepare_ftrace_return(). Do we need that on
powerpc as well?


- Naveen
Steven Rostedt Feb. 14, 2022, 7:03 p.m. UTC | #2 
On Mon, 14 Feb 2022 22:54:23 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> For x86, commit 0c0593b45c9b4e ("x86/ftrace: Make function graph use 
> ftrace directly") also adds recursion check before the call to 
> function_graph_enter() in prepare_ftrace_return(). Do we need that on
> powerpc as well?

Yes. The function_graph_enter() does not provide any recursion protection,
so if it were to call something that gets function graph traced, it will
crash the machine.

-- Steve
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/ftrace.h b/arch/powerpc/include/asm/ftrace.h
index 45c3d6f11daa..70b457097098 100644
--- a/arch/powerpc/include/asm/ftrace.h
+++ b/arch/powerpc/include/asm/ftrace.h
@@ -38,6 +38,12 @@  static __always_inline void ftrace_instruction_pointer_set(struct ftrace_regs *f
 {
 	regs_set_return_ip(&fregs->regs, ip);
 }
+
+struct ftrace_ops;
+
+#define ftrace_graph_func ftrace_graph_func
+void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
+		       struct ftrace_ops *op, struct ftrace_regs *fregs);
 #endif
 #endif /* __ASSEMBLY__ */
 
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index ce673764cb69..74a176e394ef 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -917,6 +917,9 @@  static int ftrace_modify_ftrace_graph_caller(bool enable)
 	unsigned long stub = (unsigned long)(&ftrace_graph_stub);
 	ppc_inst_t old, new;
 
+	if (IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_ARGS))
+		return 0;
+
 	old = ftrace_call_replace(ip, enable ? stub : addr, 0);
 	new = ftrace_call_replace(ip, enable ? addr : stub, 0);
 
@@ -955,6 +958,14 @@  unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
 out:
 	return parent;
 }
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
+void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
+		       struct ftrace_ops *op, struct ftrace_regs *fregs)
+{
+	fregs->regs.link = prepare_ftrace_return(parent_ip, ip, fregs->regs.gpr[1]);
+}
+#endif
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
 #ifdef PPC64_ELF_ABI_v1
diff --git a/arch/powerpc/kernel/trace/ftrace_32.S b/arch/powerpc/kernel/trace/ftrace_32.S
index c4055b41af5f..2b425da97a6b 100644
--- a/arch/powerpc/kernel/trace/ftrace_32.S
+++ b/arch/powerpc/kernel/trace/ftrace_32.S
@@ -59,13 +59,6 @@  ftrace_call:
 	mtlr	r0
 
 	addi	r1, r1, INT_FRAME_SIZE
-ftrace_caller_common:
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
 	/* old link register ends up in ctr reg */
 	bctr
 
@@ -135,52 +128,10 @@  ftrace_regs_call:
 
 	/* Pop our stack frame */
 	addi r1, r1, INT_FRAME_SIZE
-
-	b	ftrace_caller_common
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-_GLOBAL(ftrace_graph_caller)
-	stwu	r1,-48(r1)
-	stw	r3, 12(r1)
-	stw	r4, 16(r1)
-	stw	r5, 20(r1)
-	stw	r6, 24(r1)
-	stw	r7, 28(r1)
-	stw	r8, 32(r1)
-	stw	r9, 36(r1)
-	stw	r10,40(r1)
-
-	addi	r5, r1, 48
-	mfctr	r4		/* ftrace_caller has moved local addr here */
-	stw	r4, 44(r1)
-	mflr	r3		/* ftrace_caller has restored LR from stack */
-	subi	r4, r4, MCOUNT_INSN_SIZE
-
-	bl	prepare_ftrace_return
-	nop
-
-        /*
-         * prepare_ftrace_return gives us the address we divert to.
-         * Change the LR in the callers stack frame to this.
-         */
-	stw	r3,52(r1)
-	mtlr	r3
-	lwz	r0,44(r1)
-	mtctr	r0
-
-	lwz	r3, 12(r1)
-	lwz	r4, 16(r1)
-	lwz	r5, 20(r1)
-	lwz	r6, 24(r1)
-	lwz	r7, 28(r1)
-	lwz	r8, 32(r1)
-	lwz	r9, 36(r1)
-	lwz	r10,40(r1)
-
-	addi	r1, r1, 48
-
+	/* old link register ends up in ctr reg */
 	bctr
 
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
 _GLOBAL(return_to_handler)
 	/* need to save return values */
 	stwu	r1, -16(r1)
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index f6f787819273..6071e0122797 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -124,15 +124,6 @@  ftrace_regs_call:
         /* Based on the cmpd above, if the NIP was altered handle livepatch */
 	bne-	livepatch_handler
 #endif
-
-ftrace_caller_common:
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-
 	bctr			/* jump after _mcount site */
 
 _GLOBAL(ftrace_stub)
@@ -216,8 +207,7 @@  ftrace_call:
         /* 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
+	bctr			/* jump after _mcount site */
 
 #ifdef CONFIG_LIVEPATCH
 	/*
@@ -286,55 +276,3 @@  livepatch_handler:
 	/* Return to original caller of live patched function */
 	blr
 #endif /* CONFIG_LIVEPATCH */
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-_GLOBAL(ftrace_graph_caller)
-	stdu	r1, -112(r1)
-	/* with -mprofile-kernel, parameter regs are still alive at _mcount */
-	std	r10, 104(r1)
-	std	r9, 96(r1)
-	std	r8, 88(r1)
-	std	r7, 80(r1)
-	std	r6, 72(r1)
-	std	r5, 64(r1)
-	std	r4, 56(r1)
-	std	r3, 48(r1)
-
-	/* Save callee's TOC in the ABI compliant location */
-	std	r2, 24(r1)
-	ld	r2, PACATOC(r13)	/* get kernel TOC in r2 */
-
-	addi	r5, r1, 112
-	mfctr	r4		/* ftrace_caller has moved local addr here */
-	std	r4, 40(r1)
-	mflr	r3		/* ftrace_caller has restored LR from stack */
-	subi	r4, r4, MCOUNT_INSN_SIZE
-
-	bl	prepare_ftrace_return
-	nop
-
-	/*
-	 * prepare_ftrace_return gives us the address we divert to.
-	 * Change the LR to this.
-	 */
-	mtlr	r3
-
-	ld	r0, 40(r1)
-	mtctr	r0
-	ld	r10, 104(r1)
-	ld	r9, 96(r1)
-	ld	r8, 88(r1)
-	ld	r7, 80(r1)
-	ld	r6, 72(r1)
-	ld	r5, 64(r1)
-	ld	r4, 56(r1)
-	ld	r3, 48(r1)
-
-	/* Restore callee's TOC */
-	ld	r2, 24(r1)
-
-	addi	r1, r1, 112
-	mflr	r0
-	std	r0, LRSAVE(r1)
-	bctr
-#endif /* CONFIG_FUNCTION_GRAPH_TRACER */