Re: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from user mode

[Matt Redfearn]

Hi Maciej,

On 05/12/16 16:20, Maciej W. Rozycki wrote:
> On Fri, 2 Dec 2016, Matt Redfearn wrote:
>
> > diff --git a/arch/mips/include/asm/stackframe.h b/arch/mips/include/asm/stackframe.h
> > index eebf39549606..5782fa3d63be 100644
> > --- a/arch/mips/include/asm/stackframe.h
> > +++ b/arch/mips/include/asm/stackframe.h
> > @@ -216,12 +216,22 @@
> >   		LONG_S	$25, PT_R25(sp)
> >   		LONG_S	$28, PT_R28(sp)
> >   		LONG_S	$31, PT_R31(sp)
> > +
> > +		/* Set thread_info if we're coming from user mode */
> > +		.set	reorder
> > +		mfc0	k0, CP0_STATUS
> > +		sll	k0, 3		/* extract cu0 bit */
> > +		.set	noreorder
> > +		bltz	k0, 9f
> > +		 nop
>   This code is already `.set reorder', although a badly applied CONFIG_EVA
> change made things slightly less obvious.  So why do you need this `.set
> reorder' in the first place, and then why do you switch code that follows
> to `.set noreorder'?

Ah yes, I missed the .set reorder above the EVA ifdef and just included 
the .set reorder as the similar snippet here:
http://lxr.free-electrons.com/source/arch/mips/include/asm/stackframe.h#L149

I'll revise that in V2.

Thanks,
Matt

>   Overall I think all <asm/stackframe.h> code should be using the (default)
> `.set reorder' mode, perhaps forced explicitly in case these macros are
> pasted into `.set noreorder' code, to make it easier to avoid subtle data
> dependency bugs, and also to make R6 porting easier.  Except maybe for the
> RFE sequence, for readability's sake, although even there GAS will do the
> right thing.  Surely the BLTZ/MOVE piece does not have to be `.set
> noreorder' as GAS will schedule that delay slot automatically if allowed
> to.
>
>    Maciej