From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39516) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGAd8-0008Mr-MB for qemu-devel@nongnu.org; Tue, 27 Jan 2015 13:12:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGAd3-0003gi-Sj for qemu-devel@nongnu.org; Tue, 27 Jan 2015 13:12:50 -0500 Received: from mail-la0-f49.google.com ([209.85.215.49]:40877) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGAd3-0003gb-Ly for qemu-devel@nongnu.org; Tue, 27 Jan 2015 13:12:45 -0500 Received: by mail-la0-f49.google.com with SMTP id gf13so14699411lab.8 for ; Tue, 27 Jan 2015 10:12:45 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1422037228-5363-1-git-send-email-peter.maydell@linaro.org> <1422037228-5363-10-git-send-email-peter.maydell@linaro.org> From: Peter Maydell Date: Tue, 27 Jan 2015 18:12:24 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 09/11] target-arm: Use mmu_idx in get_phys_addr() 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 27 January 2015 at 17:57, Greg Bellows wrote: > > > On Fri, Jan 23, 2015 at 12:20 PM, Peter Maydell > wrote: >> +/* Return the exception level which controls this address translation >> regime */ >> +static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case ARMMMUIdx_S2NS: >> + case ARMMMUIdx_S1E2: >> + return 2; >> + case ARMMMUIdx_S1E3: >> + return 3; >> + case ARMMMUIdx_S1SE0: >> + return arm_el_is_aa64(env, 3) ? 1 : 3; >> + case ARMMMUIdx_S1SE1: > > > I think this should be handled the same way as S1SE0 as Secure EL1 maps to > EL3 when EL3 is AArch32. If EL3 is AArch32 then you'll never be using this MMU index. By definition the S1SE1 index is for code executing at Secure EL1, and there isn't any of that unless EL3 is 64 bit. (Secure EL1 doesn't "map to" anything, it just doesn't exist/have any code running in it.) >> + case ARMMMUIdx_S1NSE0: >> + case ARMMMUIdx_S1NSE1: >> + return 1; >> + default: >> + g_assert_not_reached(); >> + } >> +} >> + >> +/* Return the SCTLR value which controls this address translation regime >> */ >> +static inline uint32_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + return env->cp15.sctlr_el[regime_el(env, mmu_idx)]; > > > Given the above regime_el(), for S1SE1, this would return the non-secure > SCTLR bank on a secure translation. Same below for TCR and all uses > thereafter. That's correct, because S1SE1 implies "secure EL1 under a 64 bit EL3", in which case there is no system register banking and both Secure and NonSecure use the same underlying register. Compare the way that A32_BANKED_CURRENT_REG_GET/SET always use the NS version if arm_el_is_aa64(env, 3). >> +static inline bool regime_is_user(CPUARMState *env, ARMMMUIdx mmu_idx) >> +{ >> + switch (mmu_idx) { >> + case ARMMMUIdx_S1SE0: >> + case ARMMMUIdx_S1NSE0: > > > The get_phys_addr path filters out ARMMMUIdx_S12NSE0 so calls to this > function in this context don't matter, but what if it is called outside this > context? How should it handle this index? g_assert_not_reached(), but it didn't seem worth cluttering the switch with a bunch of extra labels just to assert that they weren't reachable. thanks -- PMM