On 29 February 2012 15:40, Paul Brook <p...@codesourcery.com> wrote:
> Add support for ARM BE8 userspace binaries.
> i.e. big-endian data and little-endian code.
> In principle LE8 mode is also possible, but AFAIK has never actually
> been implemented/used.

There seems to have been an LE8 flag in the ARM ELF spec at
one point but it doesn't exist in the current version of the
spec. I'm not entirely sure what it was originally intended to
mean: the ARM ARM only talks about BE-8, BE-32 and LE.
(You probably know better than me here since you were around at
the time...)

There is a theoretical configuration involving an R profile core
with SCTLR.IE=1 SCTLR.EE=1 but CPSR.E=0, which would be an OS
running fully big-endian, user process flipped to read data little
endian but code still big-endian. I can't find anything in the
ARM ARM that rules that out but it would be pretty weird.

> @@ -3648,6 +3656,10 @@ int main(int argc, char **argv, char **envp)
>         for(i = 0; i < 16; i++) {
>             env->regs[i] = regs->uregs[i];
>         }
> +        /* Enable BE8.  */
> +        if ((info->elf_flags >> 24) >= 4 && (info->elf_flags & 0x800000)) {
> +            env->bswap_code = 1;
> +        }
>     }

If we updated the #defines in elf.h based on a newer linux kernel header
we could say
    if ((info->elf_flags & EF_ARM_EABI_MASK) >= EF_ARM_EABI_VER4
        && (info->elf_flags & EF_ARM_BE8)) {

> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -216,6 +216,9 @@ typedef struct CPUARMState {
>         uint32_t cregs[16];
>     } iwmmxt;
>
> +    /* For mixed endian mode.  */
> +    int bswap_code;

If we make this a fixed-width type to start with it will avoid
having to change it if/when we implement system mode dynamic
endianness and need to put it into the vmstate save/load.

>  #if defined(CONFIG_USER_ONLY)
>     /* For usermode syscall translation.  */
>     int eabi;
> @@ -490,6 +493,8 @@ static inline void cpu_clone_regs(CPUState *env, 
> target_ulong newsp)
>  #define ARM_TBFLAG_VFPEN_MASK       (1 << ARM_TBFLAG_VFPEN_SHIFT)
>  #define ARM_TBFLAG_CONDEXEC_SHIFT   8
>  #define ARM_TBFLAG_CONDEXEC_MASK    (0xff << ARM_TBFLAG_CONDEXEC_SHIFT)
> +#define ARM_TBFLAG_BSWAP_CODE_SHIFT 16
> +#define ARM_TBFLAG_BSWAP_CODE_MASK  (1 << ARM_TBFLAG_BSWAP_CODE_SHIFT)
>  /* Bits 31..16 are currently unused. */

We're using bit 16 now so this comment needs updating.

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 4929372..34507b1 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -846,6 +846,9 @@ static void do_interrupt_v7m(CPUARMState *env)
>         if (semihosting_enabled) {
>             int nr;
>             nr = lduw_code(env->regs[15]) & 0xff;
> +            if (env->bswap_code) {
> +                nr = bswap16(nr);
> +            }
>             if (nr == 0xab) {
>                 env->regs[15] += 2;
>                 env->regs[0] = do_arm_semihosting(env);

[and several similar hunks]

It's kind of ugly to have all these places doing a "ld*_code()
and then bswap it". Also it has resulted in bugs as here where
we're doing a mask before the bswap rather than afterwards.
Trying to make the generic qemu ld*_code macros handle wrong-endian
insns for the sake of ARM BE8 seems like overkill though. Maybe
we should just have target-arm/cpu.h macros/inline functions to
do the load-and-maybe-bswap ?

(As you pointed out on IRC we can't actually currently get to any
of these places with bswap_code true since they're all system
mode only code, so they're only latent bugs rather than real ones.)

-- PMM

Reply via email to