Re: [Qemu-arm] [PATCH v1 14/17] target-arm: implement BE32 mode in system emulation

[Peter Maydell]

On 18 January 2016 at 07:12, Peter Crosthwaite
<address@hidden> wrote:
> From: Paolo Bonzini <address@hidden>
>
> System emulation only has a little-endian target; BE32 mode
> is implemented by adjusting the low bits of the address
> for every byte and halfword load and store.  64-bit accesses
> flip the low and high words.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> [PC changes:
>   * rebased against master (Jan 2016)
> ]
> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
>
>  target-arm/cpu.h       |  5 ++-
>  target-arm/translate.c | 86 
> +++++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 73 insertions(+), 18 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 96b1e99..5814019 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -1925,9 +1925,8 @@ static inline bool bswap_code(bool sctlr_b)
>  #endif
>          sctlr_b;
>  #else
> -    /* We do not implement BE32 mode for system-mode emulation, but
> -     * anyway it would always do little-endian accesses with
> -     * TARGET_WORDS_BIGENDIAN = 0.
> +    /* All code access in ARM is little endian, and there are no loaders
> +     * doing swaps that need to be reversed
>       */
>      return 0;
>  #endif
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 44c3ac9..2d80bb2 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -914,6 +914,12 @@ static inline void store_reg_from_load(DisasContext *s, 
> int reg, TCGv_i32 var)
>      }
>  }
>
> +#ifdef CONFIG_USER_ONLY
> +#define IS_USER_ONLY 1
> +#else
> +#define IS_USER_ONLY 0
> +#endif
> +
>  /* Abstractions of "generate code to do a guest load/store for
>   * AArch32", where a vaddr is always 32 bits (and is zero
>   * extended if we're a 64 bit core) and  data is also
> @@ -923,19 +929,35 @@ static inline void store_reg_from_load(DisasContext *s, 
> int reg, TCGv_i32 var)
>   */
>  #if TARGET_LONG_BITS == 32
>
> -#define DO_GEN_LD(SUFF, OPC)                                             \
> +#define DO_GEN_LD(SUFF, OPC, BE32_XOR)                                   \
>  static inline void gen_aa32_ld##SUFF(DisasContext *s, TCGv_i32 val,      \
>                                       TCGv_i32 addr, int index)           \
>  {                                                                        \
>      TCGMemOp opc = (OPC) | s->mo_endianness;                             \
> +    /* Not needed for user-mode BE32, where we use MO_BE instead.  */    \
> +    if (!IS_USER_ONLY && s->sctlr_b && BE32_XOR) {                       \
> +        TCGv addr_be = tcg_temp_new();                                   \
> +        tcg_gen_xori_i32(addr_be, addr, BE32_XOR);                       \
> +        tcg_gen_qemu_ld_i32(val, addr_be, index, opc);                   \
> +        tcg_temp_free(addr_be);                                          \
> +        return;                                                          \
> +    }                                                                    \
>      tcg_gen_qemu_ld_i32(val, addr, index, opc);                          \
>  }
>
> -#define DO_GEN_ST(SUFF, OPC)                                             \
> +#define DO_GEN_ST(SUFF, OPC, BE32_XOR)                                   \
>  static inline void gen_aa32_st##SUFF(DisasContext *s, TCGv_i32 val,      \
>                                       TCGv_i32 addr, int index)           \
>  {                                                                        \
>      TCGMemOp opc = (OPC) | s->mo_endianness;                             \
> +    /* Not needed for user-mode BE32, where we use MO_BE instead.  */    \
> +    if (!IS_USER_ONLY && s->sctlr_b && BE32_XOR) {                       \
> +        TCGv addr_be = tcg_temp_new();                                   \
> +        tcg_gen_xori_i32(addr_be, addr, BE32_XOR);                       \
> +        tcg_gen_qemu_ld_i32(val, addr_be, index, opc);                   \

Cut-n-paste error -- this should be a store...

> +        tcg_temp_free(addr_be);                                          \
> +        return;                                                          \
> +    }                                                                    \
>      tcg_gen_qemu_st_i32(val, addr, index, opc);                          \
>  }
>

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM