Re: [Qemu-devel] [PATCH 03/12] ARM: Prepare translation for AArch64 code

[Laurent Desnogues]

On Wed, Mar 6, 2013 at 10:36 AM, Alexander Graf <address@hidden> wrote:
>
>
> Am 06.03.2013 um 08:11 schrieb Laurent Desnogues <address@hidden>:
>
>> On Wed, Mar 6, 2013 at 3:01 AM, Alexander Graf <address@hidden> wrote:
>>> This patch adds all the prerequisites for AArch64 support that didn't
>>> fit into split up patches. It extends important bits in the core cpu
>>> headers to also take AArch64 mode into account.
>>>
>>> Signed-off-by: Alexander Graf <address@hidden>
>>> ---
>>> include/elf.h          |    2 +
>>> target-arm/cpu.h       |  103 
>>> ++++++++++++++++++++++++++++++++++++------------
>>> target-arm/translate.c |   42 +++++++++++--------
>>> 3 files changed, 103 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/include/elf.h b/include/elf.h
>>> index a21ea53..0ff0ea6 100644
>>> --- a/include/elf.h
>>> +++ b/include/elf.h
>>> @@ -109,6 +109,8 @@ typedef int64_t  Elf64_Sxword;
>>> #define EM_OPENRISC     92        /* OpenCores OpenRISC */
>>>
>>> #define EM_UNICORE32    110     /* UniCore32 */
>>> +#define EM_AARCH64      183     /* ARM 64-bit architecture */
>>> +
>>>
>>> /*
>>>  * This is an interim value that we will use until the committee comes
>>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> index c28a0d9..ec292c9 100644
>>> --- a/target-arm/cpu.h
>>> +++ b/target-arm/cpu.h
>>> @@ -19,13 +19,19 @@
>>> #ifndef CPU_ARM_H
>>> #define CPU_ARM_H
>>>
>>> -#define TARGET_LONG_BITS 32
>>> +#include "config.h"
>>>
>>> -#define ELF_MACHINE    EM_ARM
>>> +#if defined (TARGET_ARM64)
>>> +  /* AArch64 definitions */
>>> +#  define TARGET_LONG_BITS 64
>>> +#  define ELF_MACHINE  EM_AARCH64
>>> +#else
>>> +#  define TARGET_LONG_BITS 32
>>> +#  define ELF_MACHINE  EM_ARM
>>> +#endif
>>>
>>> #define CPUArchState struct CPUARMState
>>>
>>> -#include "config.h"
>>> #include "qemu-common.h"
>>> #include "exec/cpu-defs.h"
>>>
>>> @@ -79,6 +85,13 @@ struct arm_boot_info;
>>> typedef struct CPUARMState {
>>>     /* Regs for current mode.  */
>>>     uint32_t regs[16];
>>> +
>>> +    /* Regs for A64 mode.  */
>>> +    uint64_t xregs[32];
>>
>> I'm not sure it makes sense to allocate space for xregs[31].
>> If it is the zero register then you would anyway have special
>> cases in translation code to discard writes to reg 31 and so
>> it could be argued you could also special case reads from
>> reg 31 (and you should so that the optimizer knows it's zero).
>>
>> Perhaps you could use xregs[31] as SP, which after all is
>> a more "regular" register than xzr.
>
> Sp is a separate env field in my patch set. So yeah, I should just drop 
> xregs[31] and always special case it I suppose. That's the best way to avoid 
> accidents I hope :)

I think that using xregs[31] as sp would ease code generation.
No need to special case both xzr and sp.

OTOH if you only declare 31 regs and you access xregs[31]
by accident you'd read/write pc which might help uncover
bugs faster, though I wouldn't want to use that "feature" to
ease development :-)


Laurent

> Alex
>
>>
>>
>> Laurent
>>
>>> +    uint64_t pc;
>>> +    uint64_t sp;
>>> +    uint32_t pstate;
>>> +
>>>     /* Frequently accessed CPSR bits are stored separately for efficiency.
>>>        This contains all the other bits.  Use cpsr_{read,write} to access
>>>        the whole CPSR.  */
>>> @@ -154,6 +167,11 @@ typedef struct CPUARMState {
>>>         uint32_t c15_power_control; /* power control */
>>>     } cp15;
>>>
>>> +    /* System registers (AArch64) */
>>> +    struct {
>>> +        uint64_t tpidr_el0;
>>> +    } sr;
>>> +
>>>     struct {
>>>         uint32_t other_sp;
>>>         uint32_t vecbase;
>>> @@ -170,7 +188,7 @@ typedef struct CPUARMState {
>>>
>>>     /* VFP coprocessor state.  */
>>>     struct {
>>> -        float64 regs[32];
>>> +        float64 regs[64];
>>>
>>>         uint32_t xregs[16];
>>>         /* We store these fpcsr fields separately for convenience.  */
>>> @@ -241,6 +259,24 @@ int bank_number(int mode);
>>> void switch_mode(CPUARMState *, int);
>>> uint32_t do_arm_semihosting(CPUARMState *env);
>>>
>>> +static inline bool is_a64(CPUARMState *env)
>>> +{
>>> +#ifdef TARGET_ARM64
>>> +    return true;
>>> +#else
>>> +    return false;
>>> +#endif
>>> +}
>>> +
>>> +#define PSTATE_N_SHIFT 3
>>> +#define PSTATE_N  (1 << PSTATE_N_SHIFT)
>>> +#define PSTATE_Z_SHIFT 2
>>> +#define PSTATE_Z  (1 << PSTATE_Z_SHIFT)
>>> +#define PSTATE_C_SHIFT 1
>>> +#define PSTATE_C  (1 << PSTATE_C_SHIFT)
>>> +#define PSTATE_V_SHIFT 0
>>> +#define PSTATE_V  (1 << PSTATE_V_SHIFT)
>>> +
>>> /* you can call this signal handler from your SIGBUS and SIGSEGV
>>>    signal handlers to inform the virtual CPU of exceptions. non zero
>>>    is returned if the signal was handled by the virtual CPU.  */
>>> @@ -624,8 +660,13 @@ static inline bool cp_access_ok(CPUARMState *env,
>>> #define TARGET_PAGE_BITS 10
>>> #endif
>>>
>>> -#define TARGET_PHYS_ADDR_SPACE_BITS 40
>>> -#define TARGET_VIRT_ADDR_SPACE_BITS 32
>>> +#if defined (TARGET_ARM64)
>>> +#  define TARGET_PHYS_ADDR_SPACE_BITS 64
>>> +#  define TARGET_VIRT_ADDR_SPACE_BITS 64
>>> +#else
>>> +#  define TARGET_PHYS_ADDR_SPACE_BITS 40
>>> +#  define TARGET_VIRT_ADDR_SPACE_BITS 32
>>> +#endif
>>>
>>> static inline CPUARMState *cpu_init(const char *cpu_model)
>>> {
>>> @@ -699,25 +740,31 @@ static inline void cpu_clone_regs(CPUARMState *env, 
>>> target_ulong newsp)
>>> static inline void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>>>                                         target_ulong *cs_base, int *flags)
>>> {
>>> -    int privmode;
>>> -    *pc = env->regs[15];
>>> -    *cs_base = 0;
>>> -    *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
>>> -        | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
>>> -        | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
>>> -        | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT)
>>> -        | (env->bswap_code << ARM_TBFLAG_BSWAP_CODE_SHIFT);
>>> -    if (arm_feature(env, ARM_FEATURE_M)) {
>>> -        privmode = !((env->v7m.exception == 0) && (env->v7m.control & 1));
>>> +    if (is_a64(env)) {
>>> +        *pc = env->pc;
>>> +        *flags = 0;
>>>     } else {
>>> -        privmode = (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR;
>>> -    }
>>> -    if (privmode) {
>>> -        *flags |= ARM_TBFLAG_PRIV_MASK;
>>> -    }
>>> -    if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
>>> -        *flags |= ARM_TBFLAG_VFPEN_MASK;
>>> +        int privmode;
>>> +        *pc = env->regs[15];
>>> +        *flags = (env->thumb << ARM_TBFLAG_THUMB_SHIFT)
>>> +            | (env->vfp.vec_len << ARM_TBFLAG_VECLEN_SHIFT)
>>> +            | (env->vfp.vec_stride << ARM_TBFLAG_VECSTRIDE_SHIFT)
>>> +            | (env->condexec_bits << ARM_TBFLAG_CONDEXEC_SHIFT)
>>> +            | (env->bswap_code << ARM_TBFLAG_BSWAP_CODE_SHIFT);
>>> +        if (arm_feature(env, ARM_FEATURE_M)) {
>>> +            privmode = !((env->v7m.exception == 0) && (env->v7m.control & 
>>> 1));
>>> +        } else {
>>> +            privmode = (env->uncached_cpsr & CPSR_M) != ARM_CPU_MODE_USR;
>>> +        }
>>> +        if (privmode) {
>>> +            *flags |= ARM_TBFLAG_PRIV_MASK;
>>> +        }
>>> +        if (env->vfp.xregs[ARM_VFP_FPEXC] & (1 << 30)) {
>>> +            *flags |= ARM_TBFLAG_VFPEN_MASK;
>>> +        }
>>>     }
>>> +
>>> +    *cs_base = 0;
>>> }
>>>
>>> static inline bool cpu_has_work(CPUState *cpu)
>>> @@ -732,11 +779,15 @@ static inline bool cpu_has_work(CPUState *cpu)
>>>
>>> static inline void cpu_pc_from_tb(CPUARMState *env, TranslationBlock *tb)
>>> {
>>> -    env->regs[15] = tb->pc;
>>> +    if (is_a64(env)) {
>>> +        env->pc = tb->pc;
>>> +    } else {
>>> +        env->regs[15] = tb->pc;
>>> +    }
>>> }
>>>
>>> /* Load an instruction and return it in the standard little-endian order */
>>> -static inline uint32_t arm_ldl_code(CPUARMState *env, uint32_t addr,
>>> +static inline uint32_t arm_ldl_code(CPUARMState *env, target_ulong addr,
>>>                                     bool do_swap)
>>> {
>>>     uint32_t insn = cpu_ldl_code(env, addr);
>>> @@ -747,7 +798,7 @@ static inline uint32_t arm_ldl_code(CPUARMState *env, 
>>> uint32_t addr,
>>> }
>>>
>>> /* Ditto, for a halfword (Thumb) instruction */
>>> -static inline uint16_t arm_lduw_code(CPUARMState *env, uint32_t addr,
>>> +static inline uint16_t arm_lduw_code(CPUARMState *env, target_ulong addr,
>>>                                      bool do_swap)
>>> {
>>>     uint16_t insn = cpu_lduw_code(env, addr);
>>> diff --git a/target-arm/translate.c b/target-arm/translate.c
>>> index f8838f3..de04a0c 100644
>>> --- a/target-arm/translate.c
>>> +++ b/target-arm/translate.c
>>> @@ -9749,7 +9749,7 @@ static inline void 
>>> gen_intermediate_code_internal(CPUARMState *env,
>>>     uint16_t *gen_opc_end;
>>>     int j, lj;
>>>     target_ulong pc_start;
>>> -    uint32_t next_page_start;
>>> +    target_ulong next_page_start;
>>>     int num_insns;
>>>     int max_insns;
>>>
>>> @@ -9833,24 +9833,26 @@ static inline void 
>>> gen_intermediate_code_internal(CPUARMState *env,
>>>         store_cpu_field(tmp, condexec_bits);
>>>       }
>>>     do {
>>> +        if (!is_a64(env)) {
>>> #ifdef CONFIG_USER_ONLY
>>> -        /* Intercept jump to the magic kernel page.  */
>>> -        if (dc->pc >= 0xffff0000) {
>>> -            /* We always get here via a jump, so know we are not in a
>>> -               conditional execution block.  */
>>> -            gen_exception(EXCP_KERNEL_TRAP);
>>> -            dc->is_jmp = DISAS_UPDATE;
>>> -            break;
>>> -        }
>>> +            /* Intercept jump to the magic kernel page.  */
>>> +            if (dc->pc >= 0xffff0000) {
>>> +                /* We always get here via a jump, so know we are not in a
>>> +                   conditional execution block.  */
>>> +                gen_exception(EXCP_KERNEL_TRAP);
>>> +                dc->is_jmp = DISAS_UPDATE;
>>> +                break;
>>> +            }
>>> #else
>>> -        if (dc->pc >= 0xfffffff0 && IS_M(env)) {
>>> -            /* We always get here via a jump, so know we are not in a
>>> -               conditional execution block.  */
>>> -            gen_exception(EXCP_EXCEPTION_EXIT);
>>> -            dc->is_jmp = DISAS_UPDATE;
>>> -            break;
>>> -        }
>>> +            if (dc->pc >= 0xfffffff0 && IS_M(env)) {
>>> +                /* We always get here via a jump, so know we are not in a
>>> +                   conditional execution block.  */
>>> +                gen_exception(EXCP_EXCEPTION_EXIT);
>>> +                dc->is_jmp = DISAS_UPDATE;
>>> +                break;
>>> +            }
>>> #endif
>>> +        }
>>>
>>>         if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) {
>>>             QTAILQ_FOREACH(bp, &env->breakpoints, entry) {
>>> @@ -9904,7 +9906,7 @@ static inline void 
>>> gen_intermediate_code_internal(CPUARMState *env,
>>>         }
>>>
>>>         if (tcg_check_temp_count()) {
>>> -            fprintf(stderr, "TCG temporary leak before %08x\n", dc->pc);
>>> +            fprintf(stderr, "TCG temporary leak before "TARGET_FMT_lx"\n", 
>>> dc->pc);
>>>         }
>>>
>>>         /* Translation stops when a conditional branch is encountered.
>>> @@ -10074,6 +10076,10 @@ void cpu_dump_state(CPUARMState *env, FILE *f, 
>>> fprintf_function cpu_fprintf,
>>>
>>> void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb, int 
>>> pc_pos)
>>> {
>>> -    env->regs[15] = tcg_ctx.gen_opc_pc[pc_pos];
>>> +    if (is_a64(env)) {
>>> +        env->pc = tcg_ctx.gen_opc_pc[pc_pos];
>>> +    } else {
>>> +        env->regs[15] = tcg_ctx.gen_opc_pc[pc_pos];
>>> +    }
>>>     env->condexec_bits = gen_opc_condexec_bits[pc_pos];
>>> }
>>> --
>>> 1.6.0.2
>>>
>>>