From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35751) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLYfy-0002cV-FG for qemu-devel@nongnu.org; Tue, 19 Jan 2016 10:58:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLYfx-0005Jb-7m for qemu-devel@nongnu.org; Tue, 19 Jan 2016 10:58:34 -0500 Received: from mail-vk0-x233.google.com ([2607:f8b0:400c:c05::233]:34885) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLYfw-0005Ir-PG for qemu-devel@nongnu.org; Tue, 19 Jan 2016 10:58:33 -0500 Received: by mail-vk0-x233.google.com with SMTP id k1so350548287vkb.2 for ; Tue, 19 Jan 2016 07:58:32 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: From: Peter Maydell Date: Tue, 19 Jan 2016 15:58:12 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v1 04/17] target-arm: implement SCTLR.EE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Crosthwaite , QEMU Developers , Alistair Francis , sridhar kulkarni , qemu-arm , Paolo Bonzini , =?UTF-8?Q?Piotr_Kr=C3=B3l?= On 18 January 2016 at 07:12, Peter Crosthwaite wrote: > From: Peter Crosthwaite > > Implement SCTLR.EE bit which controls data endianess for exceptions > and page table translations. SCTLR.EE is mirrored to the CPSR.E bit > on exception entry. > > Signed-off-by: Peter Crosthwaite > --- > > target-arm/helper.c | 42 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 32 insertions(+), 10 deletions(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 59d5a41..afac1b2 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -5889,7 +5889,10 @@ void arm_cpu_do_interrupt(CPUState *cs) > /* Clear IT bits. */ > env->condexec_bits = 0; > /* Switch to the new mode, and to the correct instruction set. */ > - env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode; > + env->uncached_cpsr = (env->uncached_cpsr & ~(CPSR_M)) | new_mode; Why change this line? > + /* Set new mode endianess */ "endianness" > + env->uncached_cpsr = (env->uncached_cpsr & ~(CPSR_E)) | > + (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE ? CPSR_E : 0); This is a bit confusing. I think just splitting it into multiple statements would help: env->uncached_cpsr &= ~CPSR_E; if (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE) { env->uncached_cpsr |= CPSR_E; } > env->daif |= mask; > /* this is a lie, as the was no c1_sys on V4T/V5, but who cares > * and we should just guard the thumb mode on V4 */ > @@ -5958,6 +5961,12 @@ static inline bool regime_translation_disabled(CPUARMState *env, > return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0; > } > > +static inline bool regime_translation_big_endian(CPUARMState *env, > + ARMMMUIdx mmu_idx) > +{ > + return (regime_sctlr(env, mmu_idx) & SCTLR_EE) != 0; > +} > + > /* Return the TCR controlling this translation regime */ > static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx) > { > @@ -6263,7 +6272,7 @@ static hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx, > */ > static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure, > ARMMMUIdx mmu_idx, uint32_t *fsr, > - ARMMMUFaultInfo *fi) > + ARMMMUFaultInfo *fi, bool be) > { > ARMCPU *cpu = ARM_CPU(cs); > CPUARMState *env = &cpu->env; > @@ -6274,12 +6283,16 @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure, > if (fi->s1ptw) { > return 0; > } > - return address_space_ldl(cs->as, addr, attrs, NULL); > + if (be) { > + return address_space_ldl_be(cs->as, addr, attrs, NULL); > + } else { > + return address_space_ldl_le(cs->as, addr, attrs, NULL); > + } > } Why not just call regime_translation_big_endian() inside arm_ldl_ptw() and arm_ldq_ptw(), rather than having every call site making the call and passing in the result? PS: this patch will conflict with the multi-ases series but only fairly trivially. thanks -- PMM