lkml.org 
[lkml]   [2016]   [Dec]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from user mode
From
Date

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

\
 
 \ /
  Last update: 2016-12-05 17:54    [W:0.133 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site