Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751752AbcLEQxb (ORCPT ); Mon, 5 Dec 2016 11:53:31 -0500 Received: from mailapp01.imgtec.com ([195.59.15.196]:14971 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751388AbcLEQx1 (ORCPT ); Mon, 5 Dec 2016 11:53:27 -0500 Subject: Re: [PATCH 3/5] MIPS: Only change $28 to thread_info if coming from user mode To: "Maciej W. Rozycki" References: <1480685957-18809-1-git-send-email-matt.redfearn@imgtec.com> <1480685957-18809-4-git-send-email-matt.redfearn@imgtec.com> CC: Ralf Baechle , , "Jason A . Donenfeld" , Thomas Gleixner , , James Hogan , Paul Burton From: Matt Redfearn Message-ID: <9579f0c6-9f67-cf1d-94c1-34d14f38afd6@imgtec.com> Date: Mon, 5 Dec 2016 16:52:54 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.150.130.83] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1669 Lines: 47 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 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