Re: [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of system_call_exception()

Nicholas Piggin
2021-02-09T02:07:09Z

Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> Combine all tests of regs->msr into a single logical one.

Okay by me unless we choose to do the config option and put these all
under it. I think I would prefer that because sometimes the registers
are in a state you can't easily see what the values in the expression
were. In this case it doesn't matter so much because they should be in
regs in the interrupt frame.

Thanks,
Nick

>
> Before the patch:
>
> 0: 81 6a 00 84 lwz r11,132(r10)
> 4: 90 6a 00 88 stw r3,136(r10)
> 8: 69 60 00 02 xori r0,r11,2
> c: 54 00 ff fe rlwinm r0,r0,31,31,31
> 10: 0f 00 00 00 twnei r0,0
> 14: 69 63 40 00 xori r3,r11,16384
> 18: 54 63 97 fe rlwinm r3,r3,18,31,31
> 1c: 0f 03 00 00 twnei r3,0
> 20: 69 6b 80 00 xori r11,r11,32768
> 24: 55 6b 8f fe rlwinm r11,r11,17,31,31
> 28: 0f 0b 00 00 twnei r11,0
>
> After the patch:
>
> 0: 81 6a 00 84 lwz r11,132(r10)
> 4: 90 6a 00 88 stw r3,136(r10)
> 8: 7d 6b 58 f8 not r11,r11
> c: 71 6b c0 02 andi. r11,r11,49154
> 10: 0f 0b 00 00 twnei r11,0
>
> 6 cycles less on powerpc 8xx (328 => 322 cycles).
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/kernel/interrupt.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 55e1aa18cdb9..8c38e8c95be2 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -28,6 +28,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> unsigned long r0, struct pt_regs *regs)
> {
> syscall_fn f;
> + unsigned long expected_msr;
>
> regs->orig_gpr3 = r3;
>
> @@ -39,10 +40,13 @@ notrace long system_call_exception(long r3, long r4, long r5,
>
> trace_hardirqs_off(); /* finish reconciling */
>
> + expected_msr = MSR_PR;
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> - BUG_ON(!(regs->msr & MSR_RI));
> - BUG_ON(!(regs->msr & MSR_PR));
> - BUG_ON(arch_irq_disabled_regs(regs));
> + expected_msr |= MSR_RI;
> + if (IS_ENABLED(CONFIG_PPC32))
> + expected_msr |= MSR_EE;
> + BUG_ON((regs->msr & expected_msr) ^ expected_msr);
> + BUG_ON(IS_ENABLED(CONFIG_PPC64) && arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> if (mmu_has_feature(MMU_FTR_PKEY)) {
> --
> 2.25.0
>
>