linux-kernel - Re: [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of system_call

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]

Message-ID: <bd96ae5e-d4f7-8b3c-7617-adc15ab9e73e@csgroup.eu>
Date:   Tue, 9 Feb 2021 15:32:15 +0100
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Nicholas Piggin <npiggin@...il.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        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()



Le 09/02/2021 à 03:06, Nicholas Piggin a écrit :
> 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.

Yes indeed. I reword the commit log and tell that.

> 
> 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
>>
>>