[PATCH v1 1/4] powerpc/ftrace: Don't use lmw/stmw in ftrace_regs_caller()

[PATCH v1 1/4] powerpc/ftrace: Don't use lmw/stmw in ftrace_regs_caller()

[Christophe Leroy]

For the same reason as commit a85c728cb5e1 ("powerpc/32: Don't use
lmw/stmw for saving/restoring non volatile regs"), don't use
lmw/stmw in ftrace_regs_caller().

Use the same macros for PPC32 and PPC64.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/trace/ftrace_mprofile.S | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index 89639e64acd1..76dab07fd8fd 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -43,18 +43,16 @@ _GLOBAL(ftrace_regs_caller)
 
 	/* Save all gprs to pt_regs */
 	SAVE_GPR(0, r1)
-#ifdef CONFIG_PPC64
 	SAVE_GPRS(2, 11, r1)
 
+#ifdef CONFIG_PPC64
 	/* Ok to continue? */
 	lbz	r3, PACA_FTRACE_ENABLED(r13)
 	cmpdi	r3, 0
 	beq	ftrace_no_trace
+#endif
 
 	SAVE_GPRS(12, 31, r1)
-#else
-	stmw	r2, GPR2(r1)
-#endif
 
 	/* Save previous stack pointer (r1) */
 	addi	r8, r1, SWITCH_FRAME_SIZE
@@ -120,11 +118,7 @@ ftrace_regs_call:
 #endif
 
 	/* Restore gprs */
-#ifdef CONFIG_PPC64
 	REST_GPRS(2, 31, r1)
-#else
-	lmw	r2, GPR2(r1)
-#endif
 
 	/* Restore possibly modified LR */
 	PPC_LL	r0, _LINK(r1)
-- 
2.34.1

[PATCH v1 2/4] powerpc/ftrace: Refactor ftrace_{regs_}caller

[Christophe Leroy]

ftrace_caller() and frace_regs_caller() have now a lot in common.

Refactor them using GAS macros.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/trace/ftrace_mprofile.S | 147 ++++++--------------
 1 file changed, 45 insertions(+), 102 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index 76dab07fd8fd..630b2de9957b 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -32,7 +32,7 @@
  * 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_regs_caller)
+.macro	ftrace_regs_entry allregs
 	/* Save the original return address in A's stack frame */
 #ifdef CONFIG_MPROFILE_KERNEL
 	PPC_STL	r0,LRSAVE(r1)
@@ -43,7 +43,7 @@ _GLOBAL(ftrace_regs_caller)
 
 	/* Save all gprs to pt_regs */
 	SAVE_GPR(0, r1)
-	SAVE_GPRS(2, 11, r1)
+	SAVE_GPRS(3, 10, r1)
 
 #ifdef CONFIG_PPC64
 	/* Ok to continue? */
@@ -52,17 +52,29 @@ _GLOBAL(ftrace_regs_caller)
 	beq	ftrace_no_trace
 #endif
 
-	SAVE_GPRS(12, 31, r1)
+	.if \allregs == 1
+	SAVE_GPR(2, r1)
+	SAVE_GPRS(11, 31, r1)
+	.else
+#ifdef CONFIG_LIVEPATCH_64
+	SAVE_GPR(14, r1)
+#endif
+	.endif
 
 	/* Save previous stack pointer (r1) */
 	addi	r8, r1, SWITCH_FRAME_SIZE
 	PPC_STL	r8, GPR1(r1)
 
+	.if \allregs == 1
 	/* Load special regs for save below */
 	mfmsr   r8
 	mfctr   r9
 	mfxer   r10
 	mfcr	r11
+	.else
+	/* Clear MSR to flag as ftrace_caller versus frace_regs_caller */
+	li	r8, 0
+	.endif
 
 	/* Get the _mcount() call site out of LR */
 	mflr	r7
@@ -96,19 +108,17 @@ _GLOBAL(ftrace_regs_caller)
 
 	/* Save special regs */
 	PPC_STL	r8, _MSR(r1)
+	.if \allregs == 1
 	PPC_STL	r9, _CTR(r1)
 	PPC_STL	r10, _XER(r1)
 	PPC_STL	r11, _CCR(r1)
+	.endif
 
 	/* Load &pt_regs in r6 for call below */
 	addi    r6, r1, STACK_FRAME_OVERHEAD
+.endm
 
-	/* ftrace_call(r3, r4, r5, r6) */
-.globl ftrace_regs_call
-ftrace_regs_call:
-	bl	ftrace_stub
-	nop
-
+.macro	ftrace_regs_exit allregs
 	/* Load ctr with the possibly modified NIP */
 	PPC_LL	r3, _NIP(r1)
 	mtctr	r3
@@ -118,7 +128,14 @@ ftrace_regs_call:
 #endif
 
 	/* Restore gprs */
+	.if \allregs == 1
 	REST_GPRS(2, 31, r1)
+	.else
+	REST_GPRS(3, 10, r1)
+#ifdef CONFIG_LIVEPATCH_64
+	REST_GPR(14, r1)
+#endif
+	.endif
 
 	/* Restore possibly modified LR */
 	PPC_LL	r0, _LINK(r1)
@@ -137,113 +154,39 @@ ftrace_regs_call:
 	bne-	livepatch_handler
 #endif
 	bctr			/* jump after _mcount site */
+.endm
 
-_GLOBAL(ftrace_stub)
-	blr
-
-#ifdef CONFIG_PPC64
-ftrace_no_trace:
-	mflr	r3
-	mtctr	r3
-	REST_GPR(3, r1)
-	addi	r1, r1, SWITCH_FRAME_SIZE
-	mtlr	r0
-	bctr
-#endif
+_GLOBAL(ftrace_regs_caller)
+	ftrace_regs_entry 1
+	/* ftrace_call(r3, r4, r5, r6) */
+.globl ftrace_regs_call
+ftrace_regs_call:
+	bl	ftrace_stub
+	nop
+	ftrace_regs_exit 1
 
 _GLOBAL(ftrace_caller)
-	/* Save the original return address in A's stack frame */
-#ifdef CONFIG_MPROFILE_KERNEL
-	PPC_STL	r0, LRSAVE(r1)
-#endif
-
-	/* Create our stack frame + pt_regs */
-	PPC_STLU	r1, -SWITCH_FRAME_SIZE(r1)
-
-	/* Save all gprs to pt_regs */
-	SAVE_GPRS(3, 10, r1)
-
-#ifdef CONFIG_PPC64
-	lbz	r3, PACA_FTRACE_ENABLED(r13)
-	cmpdi	r3, 0
-	beq	ftrace_no_trace
-#endif
-
-	/* Save previous stack pointer (r1) */
-	addi	r8, r1, SWITCH_FRAME_SIZE
-	PPC_STL	r8, GPR1(r1)
-
-	/* Get the _mcount() call site out of LR */
-	mflr	r7
-	PPC_STL     r7, _NIP(r1)
-
-#ifdef CONFIG_PPC64
-	/* 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)
-#else
-	lis	r3,function_trace_op@ha
-	lwz	r5,function_trace_op@l(r3)
-#endif
-
-#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 */
-	PPC_STL	r0, _LINK(r1)
-	mr	r4, r0
-
-	/* Clear MSR to flag as ftrace_caller versus frace_regs_caller */
-	li	r8, 0
-	PPC_STL	r8, _MSR(r1)
-
-	/* Load &pt_regs in r6 for call below */
-	addi    r6, r1, STACK_FRAME_OVERHEAD
-
+	ftrace_regs_entry 0
 	/* ftrace_call(r3, r4, r5, r6) */
 .globl ftrace_call
 ftrace_call:
 	bl	ftrace_stub
 	nop
+	ftrace_regs_exit 0
 
-	PPC_LL	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)
+_GLOBAL(ftrace_stub)
+	blr
 
 #ifdef CONFIG_PPC64
-	/* Restore callee's TOC */
-	ld	r2, 24(r1)
-#endif
-
-	/* Restore possibly modified LR */
-	PPC_LL	r0, _LINK(r1)
-	mtlr	r0
-
-	/* Pop our stack frame */
+ftrace_no_trace:
+	mflr	r3
+	mtctr	r3
+	REST_GPR(3, r1)
 	addi	r1, r1, SWITCH_FRAME_SIZE
-
-#ifdef CONFIG_LIVEPATCH_64
-        /* Based on the cmpd above, if the NIP was altered handle livepatch */
-	bne-	livepatch_handler
+	mtlr	r0
+	bctr
 #endif
 
-	bctr			/* jump after _mcount site */
-
 #ifdef CONFIG_LIVEPATCH_64
 	/*
 	 * This function runs in the mcount context, between two functions. As
-- 
2.34.1

[PATCH v1 3/4] powerpc/ftrace: Regroup PPC64 specific operations in ftrace_mprofile.S

[Christophe Leroy]

CONFIG_MPROFILE_KERNEL is only for PPC64 and ftrace_mprofile.o
is build on PPC64 only when CONFIG_MPROFILE_KERNEL is defined.

Move saving of r0 inside #ifdef PPC64

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/trace/ftrace_mprofile.S | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index 630b2de9957b..f5d31c458e6b 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -33,11 +33,6 @@
  * and then arrange for the ftrace function to be called.
  */
 .macro	ftrace_regs_entry allregs
-	/* Save the original return address in A's stack frame */
-#ifdef CONFIG_MPROFILE_KERNEL
-	PPC_STL	r0,LRSAVE(r1)
-#endif
-
 	/* Create our stack frame + pt_regs */
 	PPC_STLU	r1,-SWITCH_FRAME_SIZE(r1)
 
@@ -46,6 +41,8 @@
 	SAVE_GPRS(3, 10, r1)
 
 #ifdef CONFIG_PPC64
+	/* Save the original return address in A's stack frame */
+	std	r0, LRSAVE+SWITCH_FRAME_SIZE(r1)
 	/* Ok to continue? */
 	lbz	r3, PACA_FTRACE_ENABLED(r13)
 	cmpdi	r3, 0
-- 
2.34.1

[PATCH v1 4/4] powerpc/ftrace: Use STK_GOT in ftrace_mprofile.S

[Christophe Leroy]

Instead of open coding offset value 24, use STK_GOT when
accessing got register in stack.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/kernel/trace/ftrace_mprofile.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
index f5d31c458e6b..4fa23e260cab 100644
--- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
@@ -82,7 +82,7 @@
 
 #ifdef CONFIG_PPC64
 	/* Save callee's TOC in the ABI compliant location */
-	std	r2, 24(r1)
+	std	r2, STK_GOT(r1)
 	ld	r2,PACATOC(r13)	/* get kernel TOC in r2 */
 
 	addis	r3,r2,function_trace_op@toc@ha
@@ -140,7 +140,7 @@
 
 #ifdef CONFIG_PPC64
 	/* Restore callee's TOC */
-	ld	r2, 24(r1)
+	ld	r2, STK_GOT(r1)
 #endif
 
 	/* Pop our stack frame */
-- 
2.34.1

Re: [PATCH v1 2/4] powerpc/ftrace: Refactor ftrace_{regs_}caller

[Naveen N. Rao]

Christophe Leroy wrote:
> ftrace_caller() and frace_regs_caller() have now a lot in common.
> 
> Refactor them using GAS macros.

Thanks for doing this - this looks much better.

> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/kernel/trace/ftrace_mprofile.S | 147 ++++++--------------
>  1 file changed, 45 insertions(+), 102 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S b/arch/powerpc/kernel/trace/ftrace_mprofile.S
> index 76dab07fd8fd..630b2de9957b 100644
> --- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
> +++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
> @@ -32,7 +32,7 @@
>   * 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_regs_caller)
> +.macro	ftrace_regs_entry allregs
>  	/* Save the original return address in A's stack frame */
>  #ifdef CONFIG_MPROFILE_KERNEL
>  	PPC_STL	r0,LRSAVE(r1)
> @@ -43,7 +43,7 @@ _GLOBAL(ftrace_regs_caller)
> 
>  	/* Save all gprs to pt_regs */
>  	SAVE_GPR(0, r1)
> -	SAVE_GPRS(2, 11, r1)
> +	SAVE_GPRS(3, 10, r1)
> 
>  #ifdef CONFIG_PPC64
>  	/* Ok to continue? */
> @@ -52,17 +52,29 @@ _GLOBAL(ftrace_regs_caller)
>  	beq	ftrace_no_trace
>  #endif

The ability to disable ftrace in certain code paths through 
paca_struct->ftrace_enabled will also be relevant on ppc32 - it will be 
nice if it can be introduced there.

For this series though:
Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>


- Naveen

Re: [PATCH v1 2/4] powerpc/ftrace: Refactor ftrace_{regs_}caller

[Christophe Leroy]

Le 03/03/2022 à 17:59, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>> ftrace_caller() and frace_regs_caller() have now a lot in common.
>>
>> Refactor them using GAS macros.
> 
> Thanks for doing this - this looks much better.
> 
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>>  arch/powerpc/kernel/trace/ftrace_mprofile.S | 147 ++++++--------------
>>  1 file changed, 45 insertions(+), 102 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/trace/ftrace_mprofile.S 
>> b/arch/powerpc/kernel/trace/ftrace_mprofile.S
>> index 76dab07fd8fd..630b2de9957b 100644
>> --- a/arch/powerpc/kernel/trace/ftrace_mprofile.S
>> +++ b/arch/powerpc/kernel/trace/ftrace_mprofile.S
>> @@ -32,7 +32,7 @@
>>   * 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_regs_caller)
>> +.macro    ftrace_regs_entry allregs
>>      /* Save the original return address in A's stack frame */
>>  #ifdef CONFIG_MPROFILE_KERNEL
>>      PPC_STL    r0,LRSAVE(r1)
>> @@ -43,7 +43,7 @@ _GLOBAL(ftrace_regs_caller)
>>
>>      /* Save all gprs to pt_regs */
>>      SAVE_GPR(0, r1)
>> -    SAVE_GPRS(2, 11, r1)
>> +    SAVE_GPRS(3, 10, r1)
>>
>>  #ifdef CONFIG_PPC64
>>      /* Ok to continue? */
>> @@ -52,17 +52,29 @@ _GLOBAL(ftrace_regs_caller)
>>      beq    ftrace_no_trace
>>  #endif
> 
> The ability to disable ftrace in certain code paths through 
> paca_struct->ftrace_enabled will also be relevant on ppc32 - it will be 
> nice if it can be introduced there.

Ah ? I understood from commit ea678ac627e0 ("powerpc64/ftrace: Add a 
field in paca to disable ftrace in unsafe code paths") that it was for 
when it runs in real mode. PPC32 doesn't run any C code in real mode.

Are there any other situations that real_mode where we'd like to disable 
it ? If so we could use the thread_struct as we don't have paca on PPC32.


Christophe

Re: [PATCH v1 1/4] powerpc/ftrace: Don't use lmw/stmw in ftrace_regs_caller()

[Michael Ellerman]

On Thu, 17 Feb 2022 13:01:56 +0100, Christophe Leroy wrote:
> For the same reason as commit a85c728cb5e1 ("powerpc/32: Don't use
> lmw/stmw for saving/restoring non volatile regs"), don't use
> lmw/stmw in ftrace_regs_caller().
> 
> Use the same macros for PPC32 and PPC64.
> 
> 
> [...]

Applied to powerpc/next.

[1/4] powerpc/ftrace: Don't use lmw/stmw in ftrace_regs_caller()
      https://git.kernel.org/powerpc/c/9bdb2eec3dde4d66b71cb4bbaebde0caed2cf0e3
[2/4] powerpc/ftrace: Refactor ftrace_{regs_}caller
      https://git.kernel.org/powerpc/c/228216716cb5f9a19d70bfc40bdc0d6a7fb7e72f
[3/4] powerpc/ftrace: Regroup PPC64 specific operations in ftrace_mprofile.S
      https://git.kernel.org/powerpc/c/a5f04d1f2724db036ba4087873c0691881932bc9
[4/4] powerpc/ftrace: Use STK_GOT in ftrace_mprofile.S
      https://git.kernel.org/powerpc/c/2ca48dbb210767b9e7166d7d47febc8fcd1da6e1

cheers

Re: [PATCH v1 2/4] powerpc/ftrace: Refactor ftrace_{regs_}caller

[Naveen N. Rao]

Christophe Leroy wrote:
> 
> 
> Le 03/03/2022 à 17:59, Naveen N. Rao a écrit :
>> Christophe Leroy wrote:
>>
>> The ability to disable ftrace in certain code paths through 
>> paca_struct->ftrace_enabled will also be relevant on ppc32 - it will be 
>> nice if it can be introduced there.
> 
> Ah ? I understood from commit ea678ac627e0 ("powerpc64/ftrace: Add a 
> field in paca to disable ftrace in unsafe code paths") that it was for 
> when it runs in real mode. PPC32 doesn't run any C code in real mode.

It likely isn't necessary in that case.

> 
> Are there any other situations that real_mode where we'd like to disable 
> it ? If so we could use the thread_struct as we don't have paca on PPC32.

For ppc64, we use this flag to disable certain paths in kvm, kexec, 
mce/hmi and idle/hotplug. If none of those are problematic on ppc32, 
then this isn't necessary.


Thanks,
- Naveen