Re: [Qemu-arm] [PATCH v1 09/17] target-arm: introduce tbflag for endianness

[Peter Maydell]

On 18 January 2016 at 07:12, Peter Crosthwaite
<address@hidden> wrote:
> From: Peter Crosthwaite <address@hidden>
>
> Introduce a tbflags for endianness, set based upon the CPUs current
> endianness. This in turn propagates through to the disas endianness
> flag.
>
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
>  target-arm/cpu.h           | 7 +++++++
>  target-arm/translate-a64.c | 2 +-
>  target-arm/translate.c     | 2 +-
>  3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 54675c7..74048d1 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1857,6 +1857,8 @@ static bool arm_cpu_is_big_endian(CPUARMState *env)
>   */
>  #define ARM_TBFLAG_NS_SHIFT         19
>  #define ARM_TBFLAG_NS_MASK          (1 << ARM_TBFLAG_NS_SHIFT)
> +#define ARM_TBFLAG_MOE_SHIFT        20
> +#define ARM_TBFLAG_MOE_MASK         (1 << ARM_TBFLAG_MOE_SHIFT)

Can we call the flag bit something that makes clear that it's the
data endianness (and that the sense is 'bit set for BE data'),
please? (We probably don't need MO in the name.)

>  /* Bit usage when in AArch64 state: currently we have no A64 specific bits */
>
> @@ -1887,6 +1889,8 @@ static bool arm_cpu_is_big_endian(CPUARMState *env)
>      (((F) & ARM_TBFLAG_XSCALE_CPAR_MASK) >> ARM_TBFLAG_XSCALE_CPAR_SHIFT)
>  #define ARM_TBFLAG_NS(F) \
>      (((F) & ARM_TBFLAG_NS_MASK) >> ARM_TBFLAG_NS_SHIFT)
> +#define ARM_TBFLAG_MOE(F) \
> +    (((F) & ARM_TBFLAG_MOE_MASK) >> ARM_TBFLAG_MOE_SHIFT)
>
>  /* Return the exception level to which FP-disabled exceptions should
>   * be taken, or 0 if FP is enabled.
> @@ -2018,6 +2022,9 @@ static inline void cpu_get_tb_cpu_state(CPUARMState 
> *env, target_ulong *pc,
>              }
>          }
>      }
> +    if (arm_cpu_is_big_endian(env)) {
> +        *flags |= ARM_TBFLAG_MOE_MASK;
> +    }

Doesn't this break BE32 linux-user? That wanted MO_TE and got it before this
patch, and now it will end up with MO_LE because arm_cpu_is_big_endian()
is only looking at BE8 related CPU status flags.

>      *flags |= fp_exception_el(env) << ARM_TBFLAG_FPEXC_EL_SHIFT;
>
>      *cs_base = 0;
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 59026b6..db68662 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -11044,7 +11044,7 @@ void gen_intermediate_code_a64(ARMCPU *cpu, 
> TranslationBlock *tb)
>                                 !arm_el_is_aa64(env, 3);
>      dc->thumb = 0;
>      dc->bswap_code = 0;
> -    dc->mo_endianness = MO_TE;
> +    dc->mo_endianness = ARM_TBFLAG_MOE(tb->flags) ? MO_BE : MO_LE;
>      dc->condexec_mask = 0;
>      dc->condexec_cond = 0;
>      dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index e1679d3..cb925ef 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -11274,7 +11274,7 @@ void gen_intermediate_code(CPUARMState *env, 
> TranslationBlock *tb)
>                                 !arm_el_is_aa64(env, 3);
>      dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
>      dc->bswap_code = ARM_TBFLAG_BSWAP_CODE(tb->flags);
> -    dc->mo_endianness = MO_TE;
> +    dc->mo_endianness = ARM_TBFLAG_MOE(tb->flags) ? MO_BE : MO_LE;
>      dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
>      dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
>      dc->mmu_idx = ARM_TBFLAG_MMUIDX(tb->flags);

thanks
-- PMM