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

[Christophe Leroy]

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