On Tue, Jan 19, 2016 at 7:58 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 18 January 2016 at 07:12, Peter Crosthwaite
> <crosthwaitepe...@gmail.com> wrote:
>> From: Peter Crosthwaite <peter.crosthwa...@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.crosthwa...@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
