Messages in this thread | | | Subject | Re: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from user mode | From | Matt Redfearn <> | Date | Mon, 5 Dec 2016 16:52:54 +0000 |
| |
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
| |