All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] powerpc: split ftrace bits into separate files
@ 2017-02-21 19:01 Naveen N. Rao
  2017-02-21 19:01 ` [PATCH v3 1/2] powerpc: split ftrace bits into a separate file Naveen N. Rao
  2017-02-21 19:01 ` [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel Naveen N. Rao
  0 siblings, 2 replies; 17+ messages in thread
From: Naveen N. Rao @ 2017-02-21 19:01 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt; +Cc: linuxppc-dev

v2: https://patchwork.ozlabs.org/patch/697787/

Michael,
Sorry this took as long as it did, but here's a take at v3. This series
conflicts with the KPROBES_ON_FTRACE patchset, but I'm posting this so
as to get feedback. I will rework these patches as needed.

Thanks,
Naveen

Naveen N. Rao (2):
  powerpc: split ftrace bits into a separate file
  powerpc: ftrace_64: split further based on -mprofile-kernel

 arch/powerpc/kernel/Makefile                   |   9 +-
 arch/powerpc/kernel/entry_32.S                 | 107 -------
 arch/powerpc/kernel/entry_64.S                 | 380 -------------------------
 arch/powerpc/kernel/trace/Makefile             |  29 ++
 arch/powerpc/kernel/{ => trace}/ftrace.c       |   0
 arch/powerpc/kernel/trace/ftrace_32.S          | 118 ++++++++
 arch/powerpc/kernel/trace/ftrace_64.S          |  85 ++++++
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 274 ++++++++++++++++++
 arch/powerpc/kernel/trace/ftrace_64_pg.S       |  68 +++++
 arch/powerpc/kernel/{ => trace}/trace_clock.c  |   0
 10 files changed, 575 insertions(+), 495 deletions(-)
 create mode 100644 arch/powerpc/kernel/trace/Makefile
 rename arch/powerpc/kernel/{ => trace}/ftrace.c (100%)
 create mode 100644 arch/powerpc/kernel/trace/ftrace_32.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_mprofile.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_pg.S
 rename arch/powerpc/kernel/{ => trace}/trace_clock.c (100%)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-21 19:01 [PATCH v3 0/2] powerpc: split ftrace bits into separate files Naveen N. Rao
@ 2017-02-21 19:01 ` Naveen N. Rao
  2017-02-27 15:36   ` Steven Rostedt
  2017-02-21 19:01 ` [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel Naveen N. Rao
  1 sibling, 1 reply; 17+ messages in thread
From: Naveen N. Rao @ 2017-02-21 19:01 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt; +Cc: linuxppc-dev

entry_*.S now includes a lot more than just kernel entry/exit code. As a
first step at cleaning this up, let's split out the ftrace bits into
separate files. Also move all related tracing code into a new trace/
subdirectory.

No functional changes.

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/Makefile                  |   9 +-
 arch/powerpc/kernel/entry_32.S                | 107 -------
 arch/powerpc/kernel/entry_64.S                | 380 -------------------------
 arch/powerpc/kernel/trace/Makefile            |  24 ++
 arch/powerpc/kernel/{ => trace}/ftrace.c      |   0
 arch/powerpc/kernel/trace/ftrace_32.S         | 118 ++++++++
 arch/powerpc/kernel/trace/ftrace_64.S         | 391 ++++++++++++++++++++++++++
 arch/powerpc/kernel/{ => trace}/trace_clock.c |   0
 8 files changed, 534 insertions(+), 495 deletions(-)
 create mode 100644 arch/powerpc/kernel/trace/Makefile
 rename arch/powerpc/kernel/{ => trace}/ftrace.c (100%)
 create mode 100644 arch/powerpc/kernel/trace/ftrace_32.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64.S
 rename arch/powerpc/kernel/{ => trace}/trace_clock.c (100%)

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a048b37b9b27..e1a04237f09d 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -29,8 +29,6 @@ CFLAGS_REMOVE_cputable.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_prom_init.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_btext.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_prom.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
-# do not trace tracer code
-CFLAGS_REMOVE_ftrace.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 # timers used by tracing
 CFLAGS_REMOVE_time.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
 endif
@@ -122,10 +120,7 @@ obj64-$(CONFIG_AUDIT)		+= compat_audit.o
 
 obj-$(CONFIG_PPC_IO_WORKAROUNDS)	+= io-workarounds.o
 
-obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
-obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
-obj-$(CONFIG_FTRACE_SYSCALLS)	+= ftrace.o
-obj-$(CONFIG_TRACING)		+= trace_clock.o
+obj-y				+= trace/
 
 ifneq ($(CONFIG_PPC_INDIRECT_PIO),y)
 obj-y				+= iomap.o
@@ -146,8 +141,6 @@ obj-$(CONFIG_KVM_GUEST)		+= kvm.o kvm_emul.o
 # Disable GCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
 UBSAN_SANITIZE_prom_init.o := n
-GCOV_PROFILE_ftrace.o := n
-UBSAN_SANITIZE_ftrace.o := n
 GCOV_PROFILE_machine_kexec_64.o := n
 UBSAN_SANITIZE_machine_kexec_64.o := n
 GCOV_PROFILE_machine_kexec_32.o := n
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index f3e4fc1c1b4d..5a2a13bd2193 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -31,7 +31,6 @@
 #include <asm/ppc_asm.h>
 #include <asm/asm-offsets.h>
 #include <asm/unistd.h>
-#include <asm/ftrace.h>
 #include <asm/ptrace.h>
 #include <asm/export.h>
 
@@ -1319,109 +1318,3 @@ machine_check_in_rtas:
 	/* XXX load up BATs and panic */
 
 #endif /* CONFIG_PPC_RTAS */
-
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-	/*
-	 * It is required that _mcount on PPC32 must preserve the
-	 * link register. But we have r0 to play with. We use r0
-	 * to push the return address back to the caller of mcount
-	 * into the ctr register, restore the link register and
-	 * then jump back using the ctr register.
-	 */
-	mflr	r0
-	mtctr	r0
-	lwz	r0, 4(r1)
-	mtlr	r0
-	bctr
-
-_GLOBAL(ftrace_caller)
-	MCOUNT_SAVE_FRAME
-	/* r3 ends up with link register */
-	subi	r3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-	bl	ftrace_stub
-	nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-	MCOUNT_RESTORE_FRAME
-	/* old link register ends up in ctr reg */
-	bctr
-#else
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-
-	MCOUNT_SAVE_FRAME
-
-	subi	r3, r3, MCOUNT_INSN_SIZE
-	LOAD_REG_ADDR(r5, ftrace_trace_function)
-	lwz	r5,0(r5)
-
-	mtctr	r5
-	bctrl
-	nop
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-	b	ftrace_graph_caller
-#endif
-	MCOUNT_RESTORE_FRAME
-	bctr
-#endif
-EXPORT_SYMBOL(_mcount)
-
-_GLOBAL(ftrace_stub)
-	blr
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-_GLOBAL(ftrace_graph_caller)
-	/* load r4 with local address */
-	lwz	r4, 44(r1)
-	subi	r4, r4, MCOUNT_INSN_SIZE
-
-	/* Grab the LR out of the caller stack frame */
-	lwz	r3,52(r1)
-
-	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)
-
-	MCOUNT_RESTORE_FRAME
-	/* old link register ends up in ctr reg */
-	bctr
-
-_GLOBAL(return_to_handler)
-	/* need to save return values */
-	stwu	r1, -32(r1)
-	stw	r3, 20(r1)
-	stw	r4, 16(r1)
-	stw	r31, 12(r1)
-	mr	r31, r1
-
-	bl	ftrace_return_to_handler
-	nop
-
-	/* return value has real return address */
-	mtlr	r3
-
-	lwz	r3, 20(r1)
-	lwz	r4, 16(r1)
-	lwz	r31,12(r1)
-	lwz	r1, 0(r1)
-
-	/* Jump back to real return address */
-	blr
-#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#endif /* CONFIG_FUNCTION_TRACER */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 6432d4bf08c8..9b541d22595a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -20,7 +20,6 @@
 
 #include <linux/errno.h>
 #include <linux/err.h>
-#include <linux/magic.h>
 #include <asm/unistd.h>
 #include <asm/processor.h>
 #include <asm/page.h>
@@ -33,7 +32,6 @@
 #include <asm/bug.h>
 #include <asm/ptrace.h>
 #include <asm/irqflags.h>
-#include <asm/ftrace.h>
 #include <asm/hw_irq.h>
 #include <asm/context_tracking.h>
 #include <asm/tm.h>
@@ -1173,381 +1171,3 @@ _GLOBAL(enter_prom)
 	ld	r0,16(r1)
 	mtlr    r0
         blr
-
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-EXPORT_SYMBOL(_mcount)
-	mflr	r12
-	mtctr	r12
-	mtlr	r0
-	bctr
-
-#ifndef CC_USING_MPROFILE_KERNEL
-_GLOBAL_TOC(ftrace_caller)
-	/* Taken from output of objdump from lib64/glibc */
-	mflr	r3
-	ld	r11, 0(r1)
-	stdu	r1, -112(r1)
-	std	r3, 128(r1)
-	ld	r4, 16(r11)
-	subi	r3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-	bl	ftrace_stub
-	nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-	ld	r0, 128(r1)
-	mtlr	r0
-	addi	r1, r1, 112
-
-#else /* CC_USING_MPROFILE_KERNEL */
-/*
- *
- * ftrace_caller() is the function that replaces _mcount() when ftrace is
- * active.
- *
- * We arrive here after a function A calls function B, and we are the trace
- * function for B. When we enter r1 points to A's stack frame, B has not yet
- * had a chance to allocate one yet.
- *
- * Additionally r2 may point either to the TOC for A, or B, depending on
- * whether B did a TOC setup sequence before calling us.
- *
- * On entry the LR points back to the _mcount() call site, and r0 holds the
- * saved LR as it was on entry to B, ie. the original return address at the
- * call site in A.
- *
- * Our job is to save the register state into a struct pt_regs (on the stack)
- * and then arrange for the ftrace function to be called.
- */
-_GLOBAL(ftrace_caller)
-	/* Save the original return address in A's stack frame */
-	std	r0,LRSAVE(r1)
-
-	/* Create our stack frame + pt_regs */
-	stdu	r1,-SWITCH_FRAME_SIZE(r1)
-
-	/* Save all gprs to pt_regs */
-	SAVE_8GPRS(0,r1)
-	SAVE_8GPRS(8,r1)
-	SAVE_8GPRS(16,r1)
-	SAVE_8GPRS(24,r1)
-
-	/* Load special regs for save below */
-	mfmsr   r8
-	mfctr   r9
-	mfxer   r10
-	mfcr	r11
-
-	/* Get the _mcount() call site out of LR */
-	mflr	r7
-	/* Save it as pt_regs->nip & pt_regs->link */
-	std     r7, _NIP(r1)
-	std     r7, _LINK(r1)
-
-	/* Save callee's TOC in the ABI compliant location */
-	std	r2, 24(r1)
-	ld	r2,PACATOC(r13)	/* get kernel TOC in r2 */
-
-	addis	r3,r2,function_trace_op@toc@ha
-	addi	r3,r3,function_trace_op@toc@l
-	ld	r5,0(r3)
-
-#ifdef CONFIG_LIVEPATCH
-	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 */
-	mr	r4, r0
-
-	/* Save special regs */
-	std     r8, _MSR(r1)
-	std     r9, _CTR(r1)
-	std     r10, _XER(r1)
-	std     r11, _CCR(r1)
-
-	/* Load &pt_regs in r6 for call below */
-	addi    r6, r1 ,STACK_FRAME_OVERHEAD
-
-	/* ftrace_call(r3, r4, r5, r6) */
-.globl ftrace_call
-ftrace_call:
-	bl	ftrace_stub
-	nop
-
-	/* Load ctr with the possibly modified NIP */
-	ld	r3, _NIP(r1)
-	mtctr	r3
-#ifdef CONFIG_LIVEPATCH
-	cmpd	r14,r3		/* has NIP been altered? */
-#endif
-
-	/* Restore gprs */
-	REST_8GPRS(0,r1)
-	REST_8GPRS(8,r1)
-	REST_8GPRS(16,r1)
-	REST_8GPRS(24,r1)
-
-	/* Restore callee's TOC */
-	ld	r2, 24(r1)
-
-	/* Pop our stack frame */
-	addi r1, r1, SWITCH_FRAME_SIZE
-
-	/* Restore original LR for return to B */
-	ld	r0, LRSAVE(r1)
-	mtlr	r0
-
-#ifdef CONFIG_LIVEPATCH
-        /* Based on the cmpd above, if the NIP was altered handle livepatch */
-	bne-	livepatch_handler
-#endif
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-	stdu	r1, -112(r1)
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-	addi	r1, r1, 112
-#endif
-
-	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
-	mtlr	r0
-	bctr			/* jump after _mcount site */
-#endif /* CC_USING_MPROFILE_KERNEL */
-
-_GLOBAL(ftrace_stub)
-	blr
-
-#ifdef CONFIG_LIVEPATCH
-	/*
-	 * This function runs in the mcount context, between two functions. As
-	 * such it can only clobber registers which are volatile and used in
-	 * function linkage.
-	 *
-	 * We get here when a function A, calls another function B, but B has
-	 * been live patched with a new function C.
-	 *
-	 * On entry:
-	 *  - we have no stack frame and can not allocate one
-	 *  - LR points back to the original caller (in A)
-	 *  - CTR holds the new NIP in C
-	 *  - r0 & r12 are free
-	 *
-	 * r0 can't be used as the base register for a DS-form load or store, so
-	 * we temporarily shuffle r1 (stack pointer) into r0 and then put it back.
-	 */
-livepatch_handler:
-	CURRENT_THREAD_INFO(r12, r1)
-
-	/* Save stack pointer into r0 */
-	mr	r0, r1
-
-	/* Allocate 3 x 8 bytes */
-	ld	r1, TI_livepatch_sp(r12)
-	addi	r1, r1, 24
-	std	r1, TI_livepatch_sp(r12)
-
-	/* Save toc & real LR on livepatch stack */
-	std	r2,  -24(r1)
-	mflr	r12
-	std	r12, -16(r1)
-
-	/* Store stack end marker */
-	lis     r12, STACK_END_MAGIC@h
-	ori     r12, r12, STACK_END_MAGIC@l
-	std	r12, -8(r1)
-
-	/* Restore real stack pointer */
-	mr	r1, r0
-
-	/* Put ctr in r12 for global entry and branch there */
-	mfctr	r12
-	bctrl
-
-	/*
-	 * Now we are returning from the patched function to the original
-	 * caller A. We are free to use r0 and r12, and we can use r2 until we
-	 * restore it.
-	 */
-
-	CURRENT_THREAD_INFO(r12, r1)
-
-	/* Save stack pointer into r0 */
-	mr	r0, r1
-
-	ld	r1, TI_livepatch_sp(r12)
-
-	/* Check stack marker hasn't been trashed */
-	lis     r2,  STACK_END_MAGIC@h
-	ori     r2,  r2, STACK_END_MAGIC@l
-	ld	r12, -8(r1)
-1:	tdne	r12, r2
-	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
-
-	/* Restore LR & toc from livepatch stack */
-	ld	r12, -16(r1)
-	mtlr	r12
-	ld	r2,  -24(r1)
-
-	/* Pop livepatch stack frame */
-	CURRENT_THREAD_INFO(r12, r0)
-	subi	r1, r1, 24
-	std	r1, TI_livepatch_sp(r12)
-
-	/* Restore real stack pointer */
-	mr	r1, r0
-
-	/* Return to original caller of live patched function */
-	blr
-#endif
-
-
-#else
-_GLOBAL_TOC(_mcount)
-EXPORT_SYMBOL(_mcount)
-	/* Taken from output of objdump from lib64/glibc */
-	mflr	r3
-	ld	r11, 0(r1)
-	stdu	r1, -112(r1)
-	std	r3, 128(r1)
-	ld	r4, 16(r11)
-
-	subi	r3, r3, MCOUNT_INSN_SIZE
-	LOAD_REG_ADDR(r5,ftrace_trace_function)
-	ld	r5,0(r5)
-	ld	r5,0(r5)
-	mtctr	r5
-	bctrl
-	nop
-
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-	b	ftrace_graph_caller
-#endif
-	ld	r0, 128(r1)
-	mtlr	r0
-	addi	r1, r1, 112
-_GLOBAL(ftrace_stub)
-	blr
-
-#endif /* CONFIG_DYNAMIC_FTRACE */
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-#ifndef CC_USING_MPROFILE_KERNEL
-_GLOBAL(ftrace_graph_caller)
-	/* load r4 with local address */
-	ld	r4, 128(r1)
-	subi	r4, r4, MCOUNT_INSN_SIZE
-
-	/* Grab the LR out of the caller stack frame */
-	ld	r11, 112(r1)
-	ld	r3, 16(r11)
-
-	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.
-	 */
-	ld	r11, 112(r1)
-	std	r3, 16(r11)
-
-	ld	r0, 128(r1)
-	mtlr	r0
-	addi	r1, r1, 112
-	blr
-
-#else /* CC_USING_MPROFILE_KERNEL */
-_GLOBAL(ftrace_graph_caller)
-	/* 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 */
-
-	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 /* CC_USING_MPROFILE_KERNEL */
-
-_GLOBAL(return_to_handler)
-	/* need to save return values */
-	std	r4,  -32(r1)
-	std	r3,  -24(r1)
-	/* save TOC */
-	std	r2,  -16(r1)
-	std	r31, -8(r1)
-	mr	r31, r1
-	stdu	r1, -112(r1)
-
-	/*
-	 * We might be called from a module.
-	 * Switch to our TOC to run inside the core kernel.
-	 */
-	ld	r2, PACATOC(r13)
-
-	bl	ftrace_return_to_handler
-	nop
-
-	/* return value has real return address */
-	mtlr	r3
-
-	ld	r1, 0(r1)
-	ld	r4,  -32(r1)
-	ld	r3,  -24(r1)
-	ld	r2,  -16(r1)
-	ld	r31, -8(r1)
-
-	/* Jump back to real return address */
-	blr
-#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-#endif /* CONFIG_FUNCTION_TRACER */
diff --git a/arch/powerpc/kernel/trace/Makefile b/arch/powerpc/kernel/trace/Makefile
new file mode 100644
index 000000000000..5f5a35254a9b
--- /dev/null
+++ b/arch/powerpc/kernel/trace/Makefile
@@ -0,0 +1,24 @@
+#
+# Makefile for the powerpc trace subsystem
+#
+
+subdir-ccflags-$(CONFIG_PPC_WERROR)	:= -Werror
+
+ifdef CONFIG_FUNCTION_TRACER
+# do not trace tracer code
+CFLAGS_REMOVE_ftrace.o = -mno-sched-epilog $(CC_FLAGS_FTRACE)
+endif
+
+obj32-$(CONFIG_FUNCTION_TRACER)		+= ftrace_32.o
+obj64-$(CONFIG_FUNCTION_TRACER)		+= ftrace_64.o
+obj-$(CONFIG_DYNAMIC_FTRACE)		+= ftrace.o
+obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
+obj-$(CONFIG_FTRACE_SYSCALLS)		+= ftrace.o
+obj-$(CONFIG_TRACING)			+= trace_clock.o
+
+obj-$(CONFIG_PPC64)			+= $(obj64-y)
+obj-$(CONFIG_PPC32)			+= $(obj32-y)
+
+# Disable GCOV & sanitizers in odd or sensitive code
+GCOV_PROFILE_ftrace.o := n
+UBSAN_SANITIZE_ftrace.o := n
diff --git a/arch/powerpc/kernel/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
similarity index 100%
rename from arch/powerpc/kernel/ftrace.c
rename to arch/powerpc/kernel/trace/ftrace.c
diff --git a/arch/powerpc/kernel/trace/ftrace_32.S b/arch/powerpc/kernel/trace/ftrace_32.S
new file mode 100644
index 000000000000..afef2c076282
--- /dev/null
+++ b/arch/powerpc/kernel/trace/ftrace_32.S
@@ -0,0 +1,118 @@
+/*
+ * Split from entry_32.S
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/magic.h>
+#include <asm/reg.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/ftrace.h>
+#include <asm/export.h>
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+_GLOBAL(mcount)
+_GLOBAL(_mcount)
+	/*
+	 * It is required that _mcount on PPC32 must preserve the
+	 * link register. But we have r0 to play with. We use r0
+	 * to push the return address back to the caller of mcount
+	 * into the ctr register, restore the link register and
+	 * then jump back using the ctr register.
+	 */
+	mflr	r0
+	mtctr	r0
+	lwz	r0, 4(r1)
+	mtlr	r0
+	bctr
+
+_GLOBAL(ftrace_caller)
+	MCOUNT_SAVE_FRAME
+	/* r3 ends up with link register */
+	subi	r3, r3, MCOUNT_INSN_SIZE
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+#endif
+	MCOUNT_RESTORE_FRAME
+	/* old link register ends up in ctr reg */
+	bctr
+#else
+_GLOBAL(mcount)
+_GLOBAL(_mcount)
+
+	MCOUNT_SAVE_FRAME
+
+	subi	r3, r3, MCOUNT_INSN_SIZE
+	LOAD_REG_ADDR(r5, ftrace_trace_function)
+	lwz	r5,0(r5)
+
+	mtctr	r5
+	bctrl
+	nop
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	b	ftrace_graph_caller
+#endif
+	MCOUNT_RESTORE_FRAME
+	bctr
+#endif
+EXPORT_SYMBOL(_mcount)
+
+_GLOBAL(ftrace_stub)
+	blr
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+_GLOBAL(ftrace_graph_caller)
+	/* load r4 with local address */
+	lwz	r4, 44(r1)
+	subi	r4, r4, MCOUNT_INSN_SIZE
+
+	/* Grab the LR out of the caller stack frame */
+	lwz	r3,52(r1)
+
+	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)
+
+	MCOUNT_RESTORE_FRAME
+	/* old link register ends up in ctr reg */
+	bctr
+
+_GLOBAL(return_to_handler)
+	/* need to save return values */
+	stwu	r1, -32(r1)
+	stw	r3, 20(r1)
+	stw	r4, 16(r1)
+	stw	r31, 12(r1)
+	mr	r31, r1
+
+	bl	ftrace_return_to_handler
+	nop
+
+	/* return value has real return address */
+	mtlr	r3
+
+	lwz	r3, 20(r1)
+	lwz	r4, 16(r1)
+	lwz	r31,12(r1)
+	lwz	r1, 0(r1)
+
+	/* Jump back to real return address */
+	blr
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/powerpc/kernel/trace/ftrace_64.S b/arch/powerpc/kernel/trace/ftrace_64.S
new file mode 100644
index 000000000000..df47433b8e13
--- /dev/null
+++ b/arch/powerpc/kernel/trace/ftrace_64.S
@@ -0,0 +1,391 @@
+/*
+ * Split from entry_64.S
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/magic.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/ftrace.h>
+#include <asm/ppc-opcode.h>
+#include <asm/export.h>
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+_GLOBAL(mcount)
+_GLOBAL(_mcount)
+EXPORT_SYMBOL(_mcount)
+	mflr	r12
+	mtctr	r12
+	mtlr	r0
+	bctr
+
+#ifndef CC_USING_MPROFILE_KERNEL
+_GLOBAL_TOC(ftrace_caller)
+	/* Taken from output of objdump from lib64/glibc */
+	mflr	r3
+	ld	r11, 0(r1)
+	stdu	r1, -112(r1)
+	std	r3, 128(r1)
+	ld	r4, 16(r11)
+	subi	r3, r3, MCOUNT_INSN_SIZE
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+#endif
+	ld	r0, 128(r1)
+	mtlr	r0
+	addi	r1, r1, 112
+
+#else /* CC_USING_MPROFILE_KERNEL */
+/*
+ *
+ * ftrace_caller() is the function that replaces _mcount() when ftrace is
+ * active.
+ *
+ * We arrive here after a function A calls function B, and we are the trace
+ * function for B. When we enter r1 points to A's stack frame, B has not yet
+ * had a chance to allocate one yet.
+ *
+ * Additionally r2 may point either to the TOC for A, or B, depending on
+ * whether B did a TOC setup sequence before calling us.
+ *
+ * On entry the LR points back to the _mcount() call site, and r0 holds the
+ * saved LR as it was on entry to B, ie. the original return address at the
+ * call site in A.
+ *
+ * Our job is to save the register state into a struct pt_regs (on the stack)
+ * and then arrange for the ftrace function to be called.
+ */
+_GLOBAL(ftrace_caller)
+	/* Save the original return address in A's stack frame */
+	std	r0,LRSAVE(r1)
+
+	/* Create our stack frame + pt_regs */
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
+
+	/* Save all gprs to pt_regs */
+	SAVE_8GPRS(0,r1)
+	SAVE_8GPRS(8,r1)
+	SAVE_8GPRS(16,r1)
+	SAVE_8GPRS(24,r1)
+
+	/* Load special regs for save below */
+	mfmsr   r8
+	mfctr   r9
+	mfxer   r10
+	mfcr	r11
+
+	/* Get the _mcount() call site out of LR */
+	mflr	r7
+	/* Save it as pt_regs->nip & pt_regs->link */
+	std     r7, _NIP(r1)
+	std     r7, _LINK(r1)
+
+	/* Save callee's TOC in the ABI compliant location */
+	std	r2, 24(r1)
+	ld	r2,PACATOC(r13)	/* get kernel TOC in r2 */
+
+	addis	r3,r2,function_trace_op@toc@ha
+	addi	r3,r3,function_trace_op@toc@l
+	ld	r5,0(r3)
+
+#ifdef CONFIG_LIVEPATCH
+	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 */
+	mr	r4, r0
+
+	/* Save special regs */
+	std     r8, _MSR(r1)
+	std     r9, _CTR(r1)
+	std     r10, _XER(r1)
+	std     r11, _CCR(r1)
+
+	/* Load &pt_regs in r6 for call below */
+	addi    r6, r1 ,STACK_FRAME_OVERHEAD
+
+	/* ftrace_call(r3, r4, r5, r6) */
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+
+	/* Load ctr with the possibly modified NIP */
+	ld	r3, _NIP(r1)
+	mtctr	r3
+#ifdef CONFIG_LIVEPATCH
+	cmpd	r14,r3		/* has NIP been altered? */
+#endif
+
+	/* Restore gprs */
+	REST_8GPRS(0,r1)
+	REST_8GPRS(8,r1)
+	REST_8GPRS(16,r1)
+	REST_8GPRS(24,r1)
+
+	/* Restore callee's TOC */
+	ld	r2, 24(r1)
+
+	/* Pop our stack frame */
+	addi r1, r1, SWITCH_FRAME_SIZE
+
+	/* Restore original LR for return to B */
+	ld	r0, LRSAVE(r1)
+	mtlr	r0
+
+#ifdef CONFIG_LIVEPATCH
+        /* Based on the cmpd above, if the NIP was altered handle livepatch */
+	bne-	livepatch_handler
+#endif
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	stdu	r1, -112(r1)
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+	addi	r1, r1, 112
+#endif
+
+	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
+	mtlr	r0
+	bctr			/* jump after _mcount site */
+#endif /* CC_USING_MPROFILE_KERNEL */
+
+_GLOBAL(ftrace_stub)
+	blr
+
+#ifdef CONFIG_LIVEPATCH
+	/*
+	 * This function runs in the mcount context, between two functions. As
+	 * such it can only clobber registers which are volatile and used in
+	 * function linkage.
+	 *
+	 * We get here when a function A, calls another function B, but B has
+	 * been live patched with a new function C.
+	 *
+	 * On entry:
+	 *  - we have no stack frame and can not allocate one
+	 *  - LR points back to the original caller (in A)
+	 *  - CTR holds the new NIP in C
+	 *  - r0 & r12 are free
+	 *
+	 * r0 can't be used as the base register for a DS-form load or store, so
+	 * we temporarily shuffle r1 (stack pointer) into r0 and then put it back.
+	 */
+livepatch_handler:
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	/* Allocate 3 x 8 bytes */
+	ld	r1, TI_livepatch_sp(r12)
+	addi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Save toc & real LR on livepatch stack */
+	std	r2,  -24(r1)
+	mflr	r12
+	std	r12, -16(r1)
+
+	/* Store stack end marker */
+	lis     r12, STACK_END_MAGIC@h
+	ori     r12, r12, STACK_END_MAGIC@l
+	std	r12, -8(r1)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Put ctr in r12 for global entry and branch there */
+	mfctr	r12
+	bctrl
+
+	/*
+	 * Now we are returning from the patched function to the original
+	 * caller A. We are free to use r0 and r12, and we can use r2 until we
+	 * restore it.
+	 */
+
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	ld	r1, TI_livepatch_sp(r12)
+
+	/* Check stack marker hasn't been trashed */
+	lis     r2,  STACK_END_MAGIC@h
+	ori     r2,  r2, STACK_END_MAGIC@l
+	ld	r12, -8(r1)
+1:	tdne	r12, r2
+	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
+
+	/* Restore LR & toc from livepatch stack */
+	ld	r12, -16(r1)
+	mtlr	r12
+	ld	r2,  -24(r1)
+
+	/* Pop livepatch stack frame */
+	CURRENT_THREAD_INFO(r12, r0)
+	subi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Return to original caller of live patched function */
+	blr
+#endif
+
+
+#else
+_GLOBAL_TOC(_mcount)
+EXPORT_SYMBOL(_mcount)
+	/* Taken from output of objdump from lib64/glibc */
+	mflr	r3
+	ld	r11, 0(r1)
+	stdu	r1, -112(r1)
+	std	r3, 128(r1)
+	ld	r4, 16(r11)
+
+	subi	r3, r3, MCOUNT_INSN_SIZE
+	LOAD_REG_ADDR(r5,ftrace_trace_function)
+	ld	r5,0(r5)
+	ld	r5,0(r5)
+	mtctr	r5
+	bctrl
+	nop
+
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	b	ftrace_graph_caller
+#endif
+	ld	r0, 128(r1)
+	mtlr	r0
+	addi	r1, r1, 112
+_GLOBAL(ftrace_stub)
+	blr
+
+#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+#ifndef CC_USING_MPROFILE_KERNEL
+_GLOBAL(ftrace_graph_caller)
+	/* load r4 with local address */
+	ld	r4, 128(r1)
+	subi	r4, r4, MCOUNT_INSN_SIZE
+
+	/* Grab the LR out of the caller stack frame */
+	ld	r11, 112(r1)
+	ld	r3, 16(r11)
+
+	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.
+	 */
+	ld	r11, 112(r1)
+	std	r3, 16(r11)
+
+	ld	r0, 128(r1)
+	mtlr	r0
+	addi	r1, r1, 112
+	blr
+
+#else /* CC_USING_MPROFILE_KERNEL */
+_GLOBAL(ftrace_graph_caller)
+	/* 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 */
+
+	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 /* CC_USING_MPROFILE_KERNEL */
+
+_GLOBAL(return_to_handler)
+	/* need to save return values */
+	std	r4,  -32(r1)
+	std	r3,  -24(r1)
+	/* save TOC */
+	std	r2,  -16(r1)
+	std	r31, -8(r1)
+	mr	r31, r1
+	stdu	r1, -112(r1)
+
+	/*
+	 * We might be called from a module.
+	 * Switch to our TOC to run inside the core kernel.
+	 */
+	ld	r2, PACATOC(r13)
+
+	bl	ftrace_return_to_handler
+	nop
+
+	/* return value has real return address */
+	mtlr	r3
+
+	ld	r1, 0(r1)
+	ld	r4,  -32(r1)
+	ld	r3,  -24(r1)
+	ld	r2,  -16(r1)
+	ld	r31, -8(r1)
+
+	/* Jump back to real return address */
+	blr
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/powerpc/kernel/trace_clock.c b/arch/powerpc/kernel/trace/trace_clock.c
similarity index 100%
rename from arch/powerpc/kernel/trace_clock.c
rename to arch/powerpc/kernel/trace/trace_clock.c
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel
  2017-02-21 19:01 [PATCH v3 0/2] powerpc: split ftrace bits into separate files Naveen N. Rao
  2017-02-21 19:01 ` [PATCH v3 1/2] powerpc: split ftrace bits into a separate file Naveen N. Rao
@ 2017-02-21 19:01 ` Naveen N. Rao
  2017-02-27 15:40   ` Steven Rostedt
  1 sibling, 1 reply; 17+ messages in thread
From: Naveen N. Rao @ 2017-02-21 19:01 UTC (permalink / raw)
  To: Michael Ellerman, Steven Rostedt; +Cc: linuxppc-dev

Split ftrace_64.S further retaining the core ftrace 64-bit aspects
in ftrace_64.S and moving ftrace_caller() and ftrace_graph_caller() into
separate files based on -mprofile-kernel. The livepatch routines are all
now contained within the mprofile file.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/Makefile             |   5 +
 arch/powerpc/kernel/trace/ftrace_64.S          | 308 +------------------------
 arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 274 ++++++++++++++++++++++
 arch/powerpc/kernel/trace/ftrace_64_pg.S       |  68 ++++++
 4 files changed, 348 insertions(+), 307 deletions(-)
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_mprofile.S
 create mode 100644 arch/powerpc/kernel/trace/ftrace_64_pg.S

diff --git a/arch/powerpc/kernel/trace/Makefile b/arch/powerpc/kernel/trace/Makefile
index 5f5a35254a9b..729dffc5f7bc 100644
--- a/arch/powerpc/kernel/trace/Makefile
+++ b/arch/powerpc/kernel/trace/Makefile
@@ -11,6 +11,11 @@ endif
 
 obj32-$(CONFIG_FUNCTION_TRACER)		+= ftrace_32.o
 obj64-$(CONFIG_FUNCTION_TRACER)		+= ftrace_64.o
+ifdef CONFIG_MPROFILE_KERNEL
+obj64-$(CONFIG_FUNCTION_TRACER)		+= ftrace_64_mprofile.o
+else
+obj64-$(CONFIG_FUNCTION_TRACER)		+= ftrace_64_pg.o
+endif
 obj-$(CONFIG_DYNAMIC_FTRACE)		+= ftrace.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)	+= ftrace.o
 obj-$(CONFIG_FTRACE_SYSCALLS)		+= ftrace.o
diff --git a/arch/powerpc/kernel/trace/ftrace_64.S b/arch/powerpc/kernel/trace/ftrace_64.S
index df47433b8e13..e5ccea19821e 100644
--- a/arch/powerpc/kernel/trace/ftrace_64.S
+++ b/arch/powerpc/kernel/trace/ftrace_64.S
@@ -23,236 +23,7 @@ EXPORT_SYMBOL(_mcount)
 	mtlr	r0
 	bctr
 
-#ifndef CC_USING_MPROFILE_KERNEL
-_GLOBAL_TOC(ftrace_caller)
-	/* Taken from output of objdump from lib64/glibc */
-	mflr	r3
-	ld	r11, 0(r1)
-	stdu	r1, -112(r1)
-	std	r3, 128(r1)
-	ld	r4, 16(r11)
-	subi	r3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-	bl	ftrace_stub
-	nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-	ld	r0, 128(r1)
-	mtlr	r0
-	addi	r1, r1, 112
-
-#else /* CC_USING_MPROFILE_KERNEL */
-/*
- *
- * ftrace_caller() is the function that replaces _mcount() when ftrace is
- * active.
- *
- * We arrive here after a function A calls function B, and we are the trace
- * function for B. When we enter r1 points to A's stack frame, B has not yet
- * had a chance to allocate one yet.
- *
- * Additionally r2 may point either to the TOC for A, or B, depending on
- * whether B did a TOC setup sequence before calling us.
- *
- * On entry the LR points back to the _mcount() call site, and r0 holds the
- * saved LR as it was on entry to B, ie. the original return address at the
- * call site in A.
- *
- * Our job is to save the register state into a struct pt_regs (on the stack)
- * and then arrange for the ftrace function to be called.
- */
-_GLOBAL(ftrace_caller)
-	/* Save the original return address in A's stack frame */
-	std	r0,LRSAVE(r1)
-
-	/* Create our stack frame + pt_regs */
-	stdu	r1,-SWITCH_FRAME_SIZE(r1)
-
-	/* Save all gprs to pt_regs */
-	SAVE_8GPRS(0,r1)
-	SAVE_8GPRS(8,r1)
-	SAVE_8GPRS(16,r1)
-	SAVE_8GPRS(24,r1)
-
-	/* Load special regs for save below */
-	mfmsr   r8
-	mfctr   r9
-	mfxer   r10
-	mfcr	r11
-
-	/* Get the _mcount() call site out of LR */
-	mflr	r7
-	/* Save it as pt_regs->nip & pt_regs->link */
-	std     r7, _NIP(r1)
-	std     r7, _LINK(r1)
-
-	/* Save callee's TOC in the ABI compliant location */
-	std	r2, 24(r1)
-	ld	r2,PACATOC(r13)	/* get kernel TOC in r2 */
-
-	addis	r3,r2,function_trace_op@toc@ha
-	addi	r3,r3,function_trace_op@toc@l
-	ld	r5,0(r3)
-
-#ifdef CONFIG_LIVEPATCH
-	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 */
-	mr	r4, r0
-
-	/* Save special regs */
-	std     r8, _MSR(r1)
-	std     r9, _CTR(r1)
-	std     r10, _XER(r1)
-	std     r11, _CCR(r1)
-
-	/* Load &pt_regs in r6 for call below */
-	addi    r6, r1 ,STACK_FRAME_OVERHEAD
-
-	/* ftrace_call(r3, r4, r5, r6) */
-.globl ftrace_call
-ftrace_call:
-	bl	ftrace_stub
-	nop
-
-	/* Load ctr with the possibly modified NIP */
-	ld	r3, _NIP(r1)
-	mtctr	r3
-#ifdef CONFIG_LIVEPATCH
-	cmpd	r14,r3		/* has NIP been altered? */
-#endif
-
-	/* Restore gprs */
-	REST_8GPRS(0,r1)
-	REST_8GPRS(8,r1)
-	REST_8GPRS(16,r1)
-	REST_8GPRS(24,r1)
-
-	/* Restore callee's TOC */
-	ld	r2, 24(r1)
-
-	/* Pop our stack frame */
-	addi r1, r1, SWITCH_FRAME_SIZE
-
-	/* Restore original LR for return to B */
-	ld	r0, LRSAVE(r1)
-	mtlr	r0
-
-#ifdef CONFIG_LIVEPATCH
-        /* Based on the cmpd above, if the NIP was altered handle livepatch */
-	bne-	livepatch_handler
-#endif
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-	stdu	r1, -112(r1)
-.globl ftrace_graph_call
-ftrace_graph_call:
-	b	ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-	addi	r1, r1, 112
-#endif
-
-	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
-	mtlr	r0
-	bctr			/* jump after _mcount site */
-#endif /* CC_USING_MPROFILE_KERNEL */
-
-_GLOBAL(ftrace_stub)
-	blr
-
-#ifdef CONFIG_LIVEPATCH
-	/*
-	 * This function runs in the mcount context, between two functions. As
-	 * such it can only clobber registers which are volatile and used in
-	 * function linkage.
-	 *
-	 * We get here when a function A, calls another function B, but B has
-	 * been live patched with a new function C.
-	 *
-	 * On entry:
-	 *  - we have no stack frame and can not allocate one
-	 *  - LR points back to the original caller (in A)
-	 *  - CTR holds the new NIP in C
-	 *  - r0 & r12 are free
-	 *
-	 * r0 can't be used as the base register for a DS-form load or store, so
-	 * we temporarily shuffle r1 (stack pointer) into r0 and then put it back.
-	 */
-livepatch_handler:
-	CURRENT_THREAD_INFO(r12, r1)
-
-	/* Save stack pointer into r0 */
-	mr	r0, r1
-
-	/* Allocate 3 x 8 bytes */
-	ld	r1, TI_livepatch_sp(r12)
-	addi	r1, r1, 24
-	std	r1, TI_livepatch_sp(r12)
-
-	/* Save toc & real LR on livepatch stack */
-	std	r2,  -24(r1)
-	mflr	r12
-	std	r12, -16(r1)
-
-	/* Store stack end marker */
-	lis     r12, STACK_END_MAGIC@h
-	ori     r12, r12, STACK_END_MAGIC@l
-	std	r12, -8(r1)
-
-	/* Restore real stack pointer */
-	mr	r1, r0
-
-	/* Put ctr in r12 for global entry and branch there */
-	mfctr	r12
-	bctrl
-
-	/*
-	 * Now we are returning from the patched function to the original
-	 * caller A. We are free to use r0 and r12, and we can use r2 until we
-	 * restore it.
-	 */
-
-	CURRENT_THREAD_INFO(r12, r1)
-
-	/* Save stack pointer into r0 */
-	mr	r0, r1
-
-	ld	r1, TI_livepatch_sp(r12)
-
-	/* Check stack marker hasn't been trashed */
-	lis     r2,  STACK_END_MAGIC@h
-	ori     r2,  r2, STACK_END_MAGIC@l
-	ld	r12, -8(r1)
-1:	tdne	r12, r2
-	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
-
-	/* Restore LR & toc from livepatch stack */
-	ld	r12, -16(r1)
-	mtlr	r12
-	ld	r2,  -24(r1)
-
-	/* Pop livepatch stack frame */
-	CURRENT_THREAD_INFO(r12, r0)
-	subi	r1, r1, 24
-	std	r1, TI_livepatch_sp(r12)
-
-	/* Restore real stack pointer */
-	mr	r1, r0
-
-	/* Return to original caller of live patched function */
-	blr
-#endif
-
-
-#else
+#else /* CONFIG_DYNAMIC_FTRACE */
 _GLOBAL_TOC(_mcount)
 EXPORT_SYMBOL(_mcount)
 	/* Taken from output of objdump from lib64/glibc */
@@ -270,7 +41,6 @@ EXPORT_SYMBOL(_mcount)
 	bctrl
 	nop
 
-
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 	b	ftrace_graph_caller
 #endif
@@ -279,85 +49,9 @@ EXPORT_SYMBOL(_mcount)
 	addi	r1, r1, 112
 _GLOBAL(ftrace_stub)
 	blr
-
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
-#ifndef CC_USING_MPROFILE_KERNEL
-_GLOBAL(ftrace_graph_caller)
-	/* load r4 with local address */
-	ld	r4, 128(r1)
-	subi	r4, r4, MCOUNT_INSN_SIZE
-
-	/* Grab the LR out of the caller stack frame */
-	ld	r11, 112(r1)
-	ld	r3, 16(r11)
-
-	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.
-	 */
-	ld	r11, 112(r1)
-	std	r3, 16(r11)
-
-	ld	r0, 128(r1)
-	mtlr	r0
-	addi	r1, r1, 112
-	blr
-
-#else /* CC_USING_MPROFILE_KERNEL */
-_GLOBAL(ftrace_graph_caller)
-	/* 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 */
-
-	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 /* CC_USING_MPROFILE_KERNEL */
-
 _GLOBAL(return_to_handler)
 	/* need to save return values */
 	std	r4,  -32(r1)
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
new file mode 100644
index 000000000000..ffb1c3564cd9
--- /dev/null
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -0,0 +1,274 @@
+/*
+ * Split from ftrace_64.S
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/magic.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/ftrace.h>
+#include <asm/ppc-opcode.h>
+#include <asm/export.h>
+#include <asm/thread_info.h>
+#include <asm/bug.h>
+#include <asm/ptrace.h>
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+/*
+ *
+ * ftrace_caller() is the function that replaces _mcount() when ftrace is
+ * active.
+ *
+ * We arrive here after a function A calls function B, and we are the trace
+ * function for B. When we enter r1 points to A's stack frame, B has not yet
+ * had a chance to allocate one yet.
+ *
+ * Additionally r2 may point either to the TOC for A, or B, depending on
+ * whether B did a TOC setup sequence before calling us.
+ *
+ * On entry the LR points back to the _mcount() call site, and r0 holds the
+ * saved LR as it was on entry to B, ie. the original return address at the
+ * call site in A.
+ *
+ * Our job is to save the register state into a struct pt_regs (on the stack)
+ * and then arrange for the ftrace function to be called.
+ */
+_GLOBAL(ftrace_caller)
+	/* Save the original return address in A's stack frame */
+	std	r0,LRSAVE(r1)
+
+	/* Create our stack frame + pt_regs */
+	stdu	r1,-SWITCH_FRAME_SIZE(r1)
+
+	/* Save all gprs to pt_regs */
+	SAVE_8GPRS(0,r1)
+	SAVE_8GPRS(8,r1)
+	SAVE_8GPRS(16,r1)
+	SAVE_8GPRS(24,r1)
+
+	/* Load special regs for save below */
+	mfmsr   r8
+	mfctr   r9
+	mfxer   r10
+	mfcr	r11
+
+	/* Get the _mcount() call site out of LR */
+	mflr	r7
+	/* Save it as pt_regs->nip & pt_regs->link */
+	std     r7, _NIP(r1)
+	std     r7, _LINK(r1)
+
+	/* Save callee's TOC in the ABI compliant location */
+	std	r2, 24(r1)
+	ld	r2,PACATOC(r13)	/* get kernel TOC in r2 */
+
+	addis	r3,r2,function_trace_op@toc@ha
+	addi	r3,r3,function_trace_op@toc@l
+	ld	r5,0(r3)
+
+#ifdef CONFIG_LIVEPATCH
+	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 */
+	mr	r4, r0
+
+	/* Save special regs */
+	std     r8, _MSR(r1)
+	std     r9, _CTR(r1)
+	std     r10, _XER(r1)
+	std     r11, _CCR(r1)
+
+	/* Load &pt_regs in r6 for call below */
+	addi    r6, r1 ,STACK_FRAME_OVERHEAD
+
+	/* ftrace_call(r3, r4, r5, r6) */
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+
+	/* Load ctr with the possibly modified NIP */
+	ld	r3, _NIP(r1)
+	mtctr	r3
+#ifdef CONFIG_LIVEPATCH
+	cmpd	r14,r3		/* has NIP been altered? */
+#endif
+
+	/* Restore gprs */
+	REST_8GPRS(0,r1)
+	REST_8GPRS(8,r1)
+	REST_8GPRS(16,r1)
+	REST_8GPRS(24,r1)
+
+	/* Restore callee's TOC */
+	ld	r2, 24(r1)
+
+	/* Pop our stack frame */
+	addi r1, r1, SWITCH_FRAME_SIZE
+
+	/* Restore original LR for return to B */
+	ld	r0, LRSAVE(r1)
+	mtlr	r0
+
+#ifdef CONFIG_LIVEPATCH
+        /* Based on the cmpd above, if the NIP was altered handle livepatch */
+	bne-	livepatch_handler
+#endif
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	stdu	r1, -112(r1)
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+	addi	r1, r1, 112
+#endif
+
+	ld	r0,LRSAVE(r1)	/* restore callee's lr at _mcount site */
+	mtlr	r0
+	bctr			/* jump after _mcount site */
+
+_GLOBAL(ftrace_stub)
+	blr
+
+#ifdef CONFIG_LIVEPATCH
+	/*
+	 * This function runs in the mcount context, between two functions. As
+	 * such it can only clobber registers which are volatile and used in
+	 * function linkage.
+	 *
+	 * We get here when a function A, calls another function B, but B has
+	 * been live patched with a new function C.
+	 *
+	 * On entry:
+	 *  - we have no stack frame and can not allocate one
+	 *  - LR points back to the original caller (in A)
+	 *  - CTR holds the new NIP in C
+	 *  - r0 & r12 are free
+	 *
+	 * r0 can't be used as the base register for a DS-form load or store, so
+	 * we temporarily shuffle r1 (stack pointer) into r0 and then put it back.
+	 */
+livepatch_handler:
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	/* Allocate 3 x 8 bytes */
+	ld	r1, TI_livepatch_sp(r12)
+	addi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Save toc & real LR on livepatch stack */
+	std	r2,  -24(r1)
+	mflr	r12
+	std	r12, -16(r1)
+
+	/* Store stack end marker */
+	lis     r12, STACK_END_MAGIC@h
+	ori     r12, r12, STACK_END_MAGIC@l
+	std	r12, -8(r1)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Put ctr in r12 for global entry and branch there */
+	mfctr	r12
+	bctrl
+
+	/*
+	 * Now we are returning from the patched function to the original
+	 * caller A. We are free to use r0 and r12, and we can use r2 until we
+	 * restore it.
+	 */
+
+	CURRENT_THREAD_INFO(r12, r1)
+
+	/* Save stack pointer into r0 */
+	mr	r0, r1
+
+	ld	r1, TI_livepatch_sp(r12)
+
+	/* Check stack marker hasn't been trashed */
+	lis     r2,  STACK_END_MAGIC@h
+	ori     r2,  r2, STACK_END_MAGIC@l
+	ld	r12, -8(r1)
+1:	tdne	r12, r2
+	EMIT_BUG_ENTRY 1b, __FILE__, __LINE__ - 1, 0
+
+	/* Restore LR & toc from livepatch stack */
+	ld	r12, -16(r1)
+	mtlr	r12
+	ld	r2,  -24(r1)
+
+	/* Pop livepatch stack frame */
+	CURRENT_THREAD_INFO(r12, r0)
+	subi	r1, r1, 24
+	std	r1, TI_livepatch_sp(r12)
+
+	/* Restore real stack pointer */
+	mr	r1, r0
+
+	/* Return to original caller of live patched function */
+	blr
+#endif /* CONFIG_LIVEPATCH */
+
+#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+_GLOBAL(ftrace_graph_caller)
+	/* 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 */
+
+	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 */
diff --git a/arch/powerpc/kernel/trace/ftrace_64_pg.S b/arch/powerpc/kernel/trace/ftrace_64_pg.S
new file mode 100644
index 000000000000..f095358da96e
--- /dev/null
+++ b/arch/powerpc/kernel/trace/ftrace_64_pg.S
@@ -0,0 +1,68 @@
+/*
+ * Split from ftrace_64.S
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/magic.h>
+#include <asm/ppc_asm.h>
+#include <asm/asm-offsets.h>
+#include <asm/ftrace.h>
+#include <asm/ppc-opcode.h>
+#include <asm/export.h>
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+_GLOBAL_TOC(ftrace_caller)
+	/* Taken from output of objdump from lib64/glibc */
+	mflr	r3
+	ld	r11, 0(r1)
+	stdu	r1, -112(r1)
+	std	r3, 128(r1)
+	ld	r4, 16(r11)
+	subi	r3, r3, MCOUNT_INSN_SIZE
+.globl ftrace_call
+ftrace_call:
+	bl	ftrace_stub
+	nop
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.globl ftrace_graph_call
+ftrace_graph_call:
+	b	ftrace_graph_stub
+_GLOBAL(ftrace_graph_stub)
+#endif
+	ld	r0, 128(r1)
+	mtlr	r0
+	addi	r1, r1, 112
+
+_GLOBAL(ftrace_stub)
+	blr
+#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+_GLOBAL(ftrace_graph_caller)
+	/* load r4 with local address */
+	ld	r4, 128(r1)
+	subi	r4, r4, MCOUNT_INSN_SIZE
+
+	/* Grab the LR out of the caller stack frame */
+	ld	r11, 112(r1)
+	ld	r3, 16(r11)
+
+	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.
+	 */
+	ld	r11, 112(r1)
+	std	r3, 16(r11)
+
+	ld	r0, 128(r1)
+	mtlr	r0
+	addi	r1, r1, 112
+	blr
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-21 19:01 ` [PATCH v3 1/2] powerpc: split ftrace bits into a separate file Naveen N. Rao
@ 2017-02-27 15:36   ` Steven Rostedt
  2017-02-28  4:04     ` Michael Ellerman
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2017-02-27 15:36 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On Wed, 22 Feb 2017 00:31:01 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> entry_*.S now includes a lot more than just kernel entry/exit code. As a
> first step at cleaning this up, let's split out the ftrace bits into
> separate files. Also move all related tracing code into a new trace/
> subdirectory.
> 
> No functional changes.

I wonder if we should stay consistent among archs, and call these files
"mcount_*.S". Or perhaps we should change x86 from mcount_64.S to
ftrace_64.S?

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel
  2017-02-21 19:01 ` [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel Naveen N. Rao
@ 2017-02-27 15:40   ` Steven Rostedt
  2017-02-28  3:58     ` Michael Ellerman
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2017-02-27 15:40 UTC (permalink / raw)
  To: Naveen N. Rao; +Cc: Michael Ellerman, linuxppc-dev

On Wed, 22 Feb 2017 00:31:02 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Split ftrace_64.S further retaining the core ftrace 64-bit aspects
> in ftrace_64.S and moving ftrace_caller() and ftrace_graph_caller() into
> separate files based on -mprofile-kernel. The livepatch routines are all
> now contained within the mprofile file.

Why? I would think that it may be possible to share code if they stay
in the same file. I haven't had the chance to look deeply into the
current code, but there may be ways to make macros and consolidate
duplicate code like we did in x86.

-- Steve


> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel
  2017-02-27 15:40   ` Steven Rostedt
@ 2017-02-28  3:58     ` Michael Ellerman
  0 siblings, 0 replies; 17+ messages in thread
From: Michael Ellerman @ 2017-02-28  3:58 UTC (permalink / raw)
  To: Steven Rostedt, Naveen N. Rao; +Cc: linuxppc-dev

Steven Rostedt <rostedt@goodmis.org> writes:

> On Wed, 22 Feb 2017 00:31:02 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> Split ftrace_64.S further retaining the core ftrace 64-bit aspects
>> in ftrace_64.S and moving ftrace_caller() and ftrace_graph_caller() into
>> separate files based on -mprofile-kernel. The livepatch routines are all
>> now contained within the mprofile file.
>
> Why?

Because we have two ABIs that are basically entirely different.

Splitting them into separate files makes it easier to work on one or the
other without getting muddled.

It also makes it clearer that the livepatch code only relates to the
mprofile ABI for example.

> I would think that it may be possible to share code if they stay
> in the same file.

We can share code even if they're in separate files. But I don't think
there's much that can be shared TBH.

> I haven't had the chance to look deeply into the
> current code, but there may be ways to make macros and consolidate
> duplicate code like we did in x86.

I'd obviously be happy to remove duplicate code, but as I said above
there's not much that is obviously shared.

There's a few shared lines such as:

  .globl ftrace_graph_call
  ftrace_graph_call:
  	b	ftrace_graph_stub


Which could go in a macro I guess, but it's almost not worth the
trouble.

cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-27 15:36   ` Steven Rostedt
@ 2017-02-28  4:04     ` Michael Ellerman
  2017-02-28 14:11       ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Ellerman @ 2017-02-28  4:04 UTC (permalink / raw)
  To: Steven Rostedt, Naveen N. Rao
  Cc: linuxppc-dev, Ingo Molnar, Thomas Gleixner, H. Peter Anvin

Steven Rostedt <rostedt@goodmis.org> writes:

> On Wed, 22 Feb 2017 00:31:01 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> entry_*.S now includes a lot more than just kernel entry/exit code. As a
>> first step at cleaning this up, let's split out the ftrace bits into
>> separate files. Also move all related tracing code into a new trace/
>> subdirectory.
>> 
>> No functional changes.
>
> I wonder if we should stay consistent among archs, and call these files
> "mcount_*.S". Or perhaps we should change x86 from mcount_64.S to
> ftrace_64.S?

I prefer ftrace_64.S, there's a lot more in those files than just the
mcount() implementation, and it also makes the link to
kernel/trace/ftrace.c more obvious.

I don't know if it's really worth keeping the names the same across
arches, especially as we already have:

  arch/arm64/kernel/entry-ftrace.S
  arch/arm/kernel/entry-ftrace.S
  arch/blackfin/kernel/ftrace-entry.S
  arch/metag/kernel/ftrace_stub.S

But we can rename it if you feel strongly about it.

cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-28  4:04     ` Michael Ellerman
@ 2017-02-28 14:11       ` Steven Rostedt
  2017-03-01 15:23         ` Naveen N. Rao
  2017-03-02  9:38         ` Michael Ellerman
  0 siblings, 2 replies; 17+ messages in thread
From: Steven Rostedt @ 2017-02-28 14:11 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Naveen N. Rao, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On Tue, 28 Feb 2017 15:04:15 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

kernel/trace/ftrace.c more obvious.
> 
> I don't know if it's really worth keeping the names the same across
> arches, especially as we already have:
> 
>   arch/arm64/kernel/entry-ftrace.S
>   arch/arm/kernel/entry-ftrace.S
>   arch/blackfin/kernel/ftrace-entry.S
>   arch/metag/kernel/ftrace_stub.S
> 
> But we can rename it if you feel strongly about it.

Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
the "mcount.S" name.

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-28 14:11       ` Steven Rostedt
@ 2017-03-01 15:23         ` Naveen N. Rao
  2017-03-02  9:38         ` Michael Ellerman
  1 sibling, 0 replies; 17+ messages in thread
From: Naveen N. Rao @ 2017-03-01 15:23 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On 2017/02/28 09:11AM, Steven Rostedt wrote:
> On Tue, 28 Feb 2017 15:04:15 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> kernel/trace/ftrace.c more obvious.
> > 
> > I don't know if it's really worth keeping the names the same across
> > arches, especially as we already have:
> > 
> >   arch/arm64/kernel/entry-ftrace.S
> >   arch/arm/kernel/entry-ftrace.S
> >   arch/blackfin/kernel/ftrace-entry.S
> >   arch/metag/kernel/ftrace_stub.S
> > 
> > But we can rename it if you feel strongly about it.
> 
> Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
> the "mcount.S" name.

Ok, just to be sure I get this right -- do you want me to name the 
powerpc files entry_ftrace_64.S (and subsequently to a slightly long 
entry_ftrace_64_mprofile.S), or rename the x86 mcount* files? Or both?

Thanks for the review,
- Naveen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-02-28 14:11       ` Steven Rostedt
  2017-03-01 15:23         ` Naveen N. Rao
@ 2017-03-02  9:38         ` Michael Ellerman
  2017-03-10 15:08           ` Naveen N. Rao
  2017-03-10 15:45           ` Steven Rostedt
  1 sibling, 2 replies; 17+ messages in thread
From: Michael Ellerman @ 2017-03-02  9:38 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Naveen N. Rao, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

Steven Rostedt <rostedt@goodmis.org> writes:

> On Tue, 28 Feb 2017 15:04:15 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> kernel/trace/ftrace.c more obvious.
>> 
>> I don't know if it's really worth keeping the names the same across
>> arches, especially as we already have:
>> 
>>   arch/arm64/kernel/entry-ftrace.S
>>   arch/arm/kernel/entry-ftrace.S
>>   arch/blackfin/kernel/ftrace-entry.S
>>   arch/metag/kernel/ftrace_stub.S
>> 
>> But we can rename it if you feel strongly about it.
>
> Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
> the "mcount.S" name.

Except what does the "entry" part mean?

Traditionally entry.S has been for the code that "enters" the kernel,
ie. from userspace or elsewhere. But that's not the case with any of the
ftrace code, it's kernel code called from the kernel. So using "entry"
is a bit wrong IMHO.

So if we drop that we're left with ftrace.S - which seems perfect to me.

cheers

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-02  9:38         ` Michael Ellerman
@ 2017-03-10 15:08           ` Naveen N. Rao
  2017-03-10 15:45           ` Steven Rostedt
  1 sibling, 0 replies; 17+ messages in thread
From: Naveen N. Rao @ 2017-03-10 15:08 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linuxppc-dev, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
	Michael Ellerman

On 2017/03/02 08:38PM, Michael Ellerman wrote:
> Steven Rostedt <rostedt@goodmis.org> writes:
> 
> > On Tue, 28 Feb 2017 15:04:15 +1100
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > kernel/trace/ftrace.c more obvious.
> >> 
> >> I don't know if it's really worth keeping the names the same across
> >> arches, especially as we already have:
> >> 
> >>   arch/arm64/kernel/entry-ftrace.S
> >>   arch/arm/kernel/entry-ftrace.S
> >>   arch/blackfin/kernel/ftrace-entry.S
> >>   arch/metag/kernel/ftrace_stub.S
> >> 
> >> But we can rename it if you feel strongly about it.
> >
> > Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
> > the "mcount.S" name.
> 
> Except what does the "entry" part mean?
> 
> Traditionally entry.S has been for the code that "enters" the kernel,
> ie. from userspace or elsewhere. But that's not the case with any of the
> ftrace code, it's kernel code called from the kernel. So using "entry"
> is a bit wrong IMHO.
> 
> So if we drop that we're left with ftrace.S - which seems perfect to me.

Hi Steve,
Are you ok with this? I'd prefer to not add the 'entry-' prefix too, 
seeing as it will make the file names quite long without necessarily 
adding much.

Thanks,
Naveen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-02  9:38         ` Michael Ellerman
  2017-03-10 15:08           ` Naveen N. Rao
@ 2017-03-10 15:45           ` Steven Rostedt
  2017-03-10 16:08             ` Naveen N. Rao
  1 sibling, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2017-03-10 15:45 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Naveen N. Rao, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On Thu, 02 Mar 2017 20:38:53 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Steven Rostedt <rostedt@goodmis.org> writes:
> 
> > On Tue, 28 Feb 2017 15:04:15 +1100
> > Michael Ellerman <mpe@ellerman.id.au> wrote:
> >
> > kernel/trace/ftrace.c more obvious.  
> >> 
> >> I don't know if it's really worth keeping the names the same across
> >> arches, especially as we already have:
> >> 
> >>   arch/arm64/kernel/entry-ftrace.S
> >>   arch/arm/kernel/entry-ftrace.S
> >>   arch/blackfin/kernel/ftrace-entry.S
> >>   arch/metag/kernel/ftrace_stub.S
> >> 
> >> But we can rename it if you feel strongly about it.  
> >
> > Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
> > the "mcount.S" name.  
> 
> Except what does the "entry" part mean?
> 
> Traditionally entry.S has been for the code that "enters" the kernel,
> ie. from userspace or elsewhere. But that's not the case with any of the
> ftrace code, it's kernel code called from the kernel. So using "entry"
> is a bit wrong IMHO.
> 
> So if we drop that we're left with ftrace.S - which seems perfect to me.

Yeah, I agree. But then there's the problem that ftrace.c and ftrace.S
will get the same ftrace.o. Maybe make it ftrace-hook.S ?

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-10 15:45           ` Steven Rostedt
@ 2017-03-10 16:08             ` Naveen N. Rao
  2017-03-10 16:54               ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Naveen N. Rao @ 2017-03-10 16:08 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On 2017/03/10 10:45AM, Steven Rostedt wrote:
> On Thu, 02 Mar 2017 20:38:53 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > Steven Rostedt <rostedt@goodmis.org> writes:
> > 
> > > On Tue, 28 Feb 2017 15:04:15 +1100
> > > Michael Ellerman <mpe@ellerman.id.au> wrote:
> > >
> > > kernel/trace/ftrace.c more obvious.  
> > >> 
> > >> I don't know if it's really worth keeping the names the same across
> > >> arches, especially as we already have:
> > >> 
> > >>   arch/arm64/kernel/entry-ftrace.S
> > >>   arch/arm/kernel/entry-ftrace.S
> > >>   arch/blackfin/kernel/ftrace-entry.S
> > >>   arch/metag/kernel/ftrace_stub.S
> > >> 
> > >> But we can rename it if you feel strongly about it.  
> > >
> > > Hmm, perhaps "entry-ftrace.S" would be the better name. I never liked
> > > the "mcount.S" name.  
> > 
> > Except what does the "entry" part mean?
> > 
> > Traditionally entry.S has been for the code that "enters" the kernel,
> > ie. from userspace or elsewhere. But that's not the case with any of the
> > ftrace code, it's kernel code called from the kernel. So using "entry"
> > is a bit wrong IMHO.
> > 
> > So if we drop that we're left with ftrace.S - which seems perfect to me.
> 
> Yeah, I agree. But then there's the problem that ftrace.c and ftrace.S
> will get the same ftrace.o. Maybe make it ftrace-hook.S ?

I've avoided that issue by naming the files ftrace_32.S and ftrace_64.S 
(which gets further split up).

Thanks,
Naveen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-10 16:08             ` Naveen N. Rao
@ 2017-03-10 16:54               ` Steven Rostedt
  2017-03-15  9:05                 ` Naveen N. Rao
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2017-03-10 16:54 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On Fri, 10 Mar 2017 21:38:53 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> On 2017/03/10 10:45AM, Steven Rostedt wrote:
> > On Thu, 02 Mar 2017 20:38:53 +1100
> > Michael Ellerman <mpe@ellerman.id.au> wrote:

> > > So if we drop that we're left with ftrace.S - which seems perfect to me.  
> > 
> > Yeah, I agree. But then there's the problem that ftrace.c and ftrace.S
> > will get the same ftrace.o. Maybe make it ftrace-hook.S ?  
> 
> I've avoided that issue by naming the files ftrace_32.S and ftrace_64.S 
> (which gets further split up).

That's what I looked at doing for x86 as well. But not all archs have
32 / 64 splits. Should we look to have something that all archs can be
consistent with?

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-10 16:54               ` Steven Rostedt
@ 2017-03-15  9:05                 ` Naveen N. Rao
  2017-03-15 13:11                   ` Steven Rostedt
  0 siblings, 1 reply; 17+ messages in thread
From: Naveen N. Rao @ 2017-03-15  9:05 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On 2017/03/10 11:54AM, Steven Rostedt wrote:
> On Fri, 10 Mar 2017 21:38:53 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > On 2017/03/10 10:45AM, Steven Rostedt wrote:
> > > On Thu, 02 Mar 2017 20:38:53 +1100
> > > Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> > > > So if we drop that we're left with ftrace.S - which seems perfect to me.  
> > > 
> > > Yeah, I agree. But then there's the problem that ftrace.c and ftrace.S
> > > will get the same ftrace.o. Maybe make it ftrace-hook.S ?  
> > 
> > I've avoided that issue by naming the files ftrace_32.S and ftrace_64.S 
> > (which gets further split up).
> 
> That's what I looked at doing for x86 as well. But not all archs have
> 32 / 64 splits. Should we look to have something that all archs can be
> consistent with?

I don't have a strong opinion about this, but I feel that x86 can simply 
use ftrace_64.S, seeing as the current name is mcount_64.S.

Other architectures could do something similar too, or fall back to 
ftrace_hook.S. That way, all ftrace low-level code can simply be 
referred to as arch/*/ftrace_*.S

- Naveen

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-15  9:05                 ` Naveen N. Rao
@ 2017-03-15 13:11                   ` Steven Rostedt
  2017-03-15 13:57                     ` Naveen N. Rao
  0 siblings, 1 reply; 17+ messages in thread
From: Steven Rostedt @ 2017-03-15 13:11 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On Wed, 15 Mar 2017 14:35:16 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> I don't have a strong opinion about this, but I feel that x86 can simply 
> use ftrace_64.S, seeing as the current name is mcount_64.S.
> 
> Other architectures could do something similar too, or fall back to 
> ftrace_hook.S. That way, all ftrace low-level code can simply be 
> referred to as arch/*/ftrace_*.S

Just to clarify, I'm currently working on patches to clean up the
ftrace.S code in x86, and I renamed mcount_64.S to ftrace_64.S. I'm
also moving the ftrace code out of entry_32.S into a ftrace_32.S file
as well. Patches will hopefully be posted soon.

-- Steve

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/2] powerpc: split ftrace bits into a separate file
  2017-03-15 13:11                   ` Steven Rostedt
@ 2017-03-15 13:57                     ` Naveen N. Rao
  0 siblings, 0 replies; 17+ messages in thread
From: Naveen N. Rao @ 2017-03-15 13:57 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Michael Ellerman, linuxppc-dev, Ingo Molnar, Thomas Gleixner,
	H. Peter Anvin

On 2017/03/15 09:11AM, Steven Rostedt wrote:
> On Wed, 15 Mar 2017 14:35:16 +0530
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > I don't have a strong opinion about this, but I feel that x86 can simply 
> > use ftrace_64.S, seeing as the current name is mcount_64.S.
> > 
> > Other architectures could do something similar too, or fall back to 
> > ftrace_hook.S. That way, all ftrace low-level code can simply be 
> > referred to as arch/*/ftrace_*.S
> 
> Just to clarify, I'm currently working on patches to clean up the
> ftrace.S code in x86, and I renamed mcount_64.S to ftrace_64.S. I'm
> also moving the ftrace code out of entry_32.S into a ftrace_32.S file
> as well. Patches will hopefully be posted soon.

Good to know, thanks for clarifying.

In light of that, I hope that you're ok with the changes proposed to the 
ftrace bits under arch/powerpc in this patchset?

- Naveen

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2017-03-15 13:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-21 19:01 [PATCH v3 0/2] powerpc: split ftrace bits into separate files Naveen N. Rao
2017-02-21 19:01 ` [PATCH v3 1/2] powerpc: split ftrace bits into a separate file Naveen N. Rao
2017-02-27 15:36   ` Steven Rostedt
2017-02-28  4:04     ` Michael Ellerman
2017-02-28 14:11       ` Steven Rostedt
2017-03-01 15:23         ` Naveen N. Rao
2017-03-02  9:38         ` Michael Ellerman
2017-03-10 15:08           ` Naveen N. Rao
2017-03-10 15:45           ` Steven Rostedt
2017-03-10 16:08             ` Naveen N. Rao
2017-03-10 16:54               ` Steven Rostedt
2017-03-15  9:05                 ` Naveen N. Rao
2017-03-15 13:11                   ` Steven Rostedt
2017-03-15 13:57                     ` Naveen N. Rao
2017-02-21 19:01 ` [PATCH v3 2/2] powerpc: ftrace_64: split further based on -mprofile-kernel Naveen N. Rao
2017-02-27 15:40   ` Steven Rostedt
2017-02-28  3:58     ` Michael Ellerman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.