Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 19/22] powerpc/syscall: Optimise checks in beginning of
 system_call_exception()
To:     Nicholas Piggin <npiggin@gmail.com>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        Michael Ellerman <mpe@ellerman.id.au>, msuchanek@suse.de,
        Paul Mackerras <paulus@samba.org>
Cc:     linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
References: <cover.1612796617.git.christophe.leroy@csgroup.eu>
 <3e48bb439357c6f72ae4343bf93bd29f0980eeb1.1612796617.git.christophe.leroy@csgroup.eu>
 <1612836170.502t0sssvi.astroid@bobo.none>
From:   Christophe Leroy <christophe.leroy@csgroup.eu>
Message-ID: <bd96ae5e-d4f7-8b3c-7617-adc15ab9e73e@csgroup.eu>
Date:   Tue, 9 Feb 2021 15:32:15 +0100
User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
MIME-Version: 1.0
In-Reply-To: <1612836170.502t0sssvi.astroid@bobo.none>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: fr
Content-Transfer-Encoding: 8bit
Precedence: bulk


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