2022-02-17 13:08:36

by Christophe Leroy

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

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 <[email protected]>
---
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


2022-02-17 15:06:49

by Christophe Leroy

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

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

Refactor them using GAS macros.

Signed-off-by: Christophe Leroy <[email protected]>
---
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

2022-02-17 15:50:16

by Christophe Leroy

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

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

Signed-off-by: Christophe Leroy <[email protected]>
---
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

2022-02-17 18:12:10

by Christophe Leroy

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

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 <[email protected]>
---
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

2022-03-03 19:28:10

by Naveen N. Rao

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

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 <[email protected]>
> ---
> 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 <[email protected]>


- Naveen

2022-03-04 01:07:05

by Christophe Leroy

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



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 <[email protected]>
>> ---
>>  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

2022-03-08 12:17:46

by Michael Ellerman

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

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

2022-03-09 15:23:49

by Naveen N. Rao

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

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