lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 09 Feb 2021 12:06:05 +1000
From:   Nicholas Piggin <npiggin@...il.com>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Michael Ellerman <mpe@...erman.id.au>, msuchanek@...e.de,
        Paul Mackerras <paulus@...ba.org>
Cc:     linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of
 system_call_exception()

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@...roup.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
> 
> 

Powered by blists - more mailing lists