| Message ID | fb6f0c034f8a1d36961247f56337aca1f22b3653.1453100525.git.crosthwaite.peter@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 18 January 2016 at 07:12, Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote: > From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > 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 <peter.crosthwaite@xilinx.com> > --- > > 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
On Tue, Jan 19, 2016 at 7:58 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 18 January 2016 at 07:12, Peter Crosthwaite > <crosthwaitepeter@gmail.com> wrote: >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >> 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 <peter.crosthwaite@xilinx.com> >> --- >> >> 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? > Reverted >> + /* Set new mode endianess */ > > "endianness" > Fixed >> + 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; > } > Fixed. >> 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? > Fixed. > PS: this patch will conflict with the multi-ases series but only > fairly trivially. > Resolved. Regards, Peter > thanks > -- PMM
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; + /* Set new mode endianess */ + env->uncached_cpsr = (env->uncached_cpsr & ~(CPSR_E)) | + (env->cp15.sctlr_el[arm_current_el(env)] & SCTLR_EE ? CPSR_E : 0); 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); + } } static uint64_t arm_ldq_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; @@ -6290,7 +6303,11 @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure, if (fi->s1ptw) { return 0; } - return address_space_ldq(cs->as, addr, attrs, NULL); + if (be) { + return address_space_ldq_be(cs->as, addr, attrs, NULL); + } else { + return address_space_ldq_le(cs->as, addr, attrs, NULL); + } } static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, @@ -6318,7 +6335,8 @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, goto do_fault; } desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx), - mmu_idx, fsr, fi); + mmu_idx, fsr, fi, + regime_translation_big_endian(env, mmu_idx)); type = (desc & 3); domain = (desc >> 5) & 0x0f; if (regime_el(env, mmu_idx) == 1) { @@ -6355,7 +6373,8 @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address, table = (desc & 0xfffff000) | ((address >> 8) & 0xffc); } desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx), - mmu_idx, fsr, fi); + mmu_idx, fsr, fi, + regime_translation_big_endian(env, mmu_idx)); switch (desc & 3) { case 0: /* Page translation fault. */ code = 7; @@ -6437,7 +6456,8 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, goto do_fault; } desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx), - mmu_idx, fsr, fi); + mmu_idx, fsr, fi, + regime_translation_big_endian(env, mmu_idx)); type = (desc & 3); if (type == 0 || (type == 3 && !arm_feature(env, ARM_FEATURE_PXN))) { /* Section translation fault, or attempt to use the encoding @@ -6489,7 +6509,8 @@ static bool get_phys_addr_v6(CPUARMState *env, uint32_t address, /* Lookup l2 entry. */ table = (desc & 0xfffffc00) | ((address >> 10) & 0x3fc); desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx), - mmu_idx, fsr, fi); + mmu_idx, fsr, fi, + regime_translation_big_endian(env, mmu_idx)); ap = ((desc >> 4) & 3) | ((desc >> 7) & 4); switch (desc & 3) { case 0: /* Page translation fault. */ @@ -6862,7 +6883,8 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, descaddr |= (address >> (stride * (4 - level))) & descmask; descaddr &= ~7ULL; nstable = extract32(tableattrs, 4, 1); - descriptor = arm_ldq_ptw(cs, descaddr, !nstable, mmu_idx, fsr, fi); + descriptor = arm_ldq_ptw(cs, descaddr, !nstable, mmu_idx, fsr, fi, + regime_translation_big_endian(env, mmu_idx)); if (fi->s1ptw) { goto do_fault; }