From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41392) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEpH9-0005ea-BC for qemu-devel@nongnu.org; Fri, 23 Jan 2015 20:12:36 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YEpH5-0007z3-8k for qemu-devel@nongnu.org; Fri, 23 Jan 2015 20:12:35 -0500 Received: from mail-lb0-f170.google.com ([209.85.217.170]:56719) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YEpH5-0007yq-1R for qemu-devel@nongnu.org; Fri, 23 Jan 2015 20:12:31 -0500 Received: by mail-lb0-f170.google.com with SMTP id w7so434989lbi.1 for ; Fri, 23 Jan 2015 17:12:30 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1422037228-5363-1-git-send-email-peter.maydell@linaro.org> <1422037228-5363-5-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Sat, 24 Jan 2015 01:12:09 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 04/11] target-arm: Define correct mmu_idx values and pass them in TB flags List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Bellows Cc: "Edgar E. Iglesias" , Andrew Jones , =?UTF-8?B?QWxleCBCZW5uw6ll?= , QEMU Developers , Patch Tracking On 23 January 2015 at 21:44, Greg Bellows wrote: > On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell > wrote: >> +typedef enum ARMMMUIdx { >> + ARMMMUIdx_S12NSE0 = 0, >> + ARMMMUIdx_S12NSE1 = 1, >> + ARMMMUIdx_S1E2 = 2, >> + ARMMMUIdx_S1E3 = 3, >> + ARMMMUIdx_S1SE0 = 4, >> + ARMMMUIdx_S1SE1 = 5, >> + ARMMMUIdx_S2NS = 6, >> + /* Indexes below here don't have TLBs and are used only for AT system >> + * instructions or for the first stage of an S12 page table walk. >> + */ >> + ARMMMUIdx_S1NSE0 = 7, >> + ARMMMUIdx_S1NSE1 = 8, >> +} ARMMMUIdx; >> + >> #define MMU_MODE0_SUFFIX _user >> #define MMU_MODE1_SUFFIX _kernel >> #define MMU_USER_IDX 0 >> + >> +/* Return the exception level we're running at if this is our mmu_idx */ >> +static inline int arm_mmu_idx_to_el(ARMMMUIdx mmu_idx) >> +{ >> + assert(mmu_idx < ARMMMUIdx_S2NS); >> + return mmu_idx & 3; >> +} >> + >> +/* Determine the current mmu_idx to use for normal loads/stores */ >> static inline int cpu_mmu_index (CPUARMState *env) >> { >> - return arm_current_el(env); >> + int el = arm_current_el(env); >> + >> + if (el < 3 && arm_is_secure_below_el3(env)) { > > We bypass the secure check for EL3 but not EL2. We should either > circumvent both EL2 & 3 or neither. Not sure what you mean here. The point of this code is to return a suitable MMU idx for the current CPU state. If we are in EL2 or EL3 then just "el" is correct (being ARMMMUIdx_S1E2 and ARMMMUIdx_S1E3). We can't be in this condition with el == 2 because if we're in EL2 then the NS bit must be set (there's no such thing as Secure EL2) and so arm_is_secure_below_el3() will have returned false. So we know here that el is 0 or 1, and this addition below: >> + return ARMMMUIdx_S1SE0 + el; means we end up with either _S1SE0 or _S1SE1, as required. (the condition could equally well be written if (el < 2 && arm_is_secure_below_el3(env)) as the two are true in exactly the same set of cases. < 3 seemed marginally better to me, since it's expressing "if we're secure and not in EL3" which is the set of cases where we need to change the mmu_idx from the 0/1/2/3 values.) >> + } >> + return el; Anything else (_S12NSE0, _S12NSE1, _S1E2, _S1E3) is this return. >> } thanks -- PMM