[Qemu-devel] [PATCH 00/12] target-arm: A64 decoder, foundation plus branches

[Qemu-devel] [PATCH 00/12] target-arm: A64 decoder, foundation plus branches

[Peter Maydell]

Hi; this patchset represents the first bit of output of the work
we've been doing in Linaro to help get the SuSE A64 instruction
emulation upstream. Since code review on the 60-patch set Alex
posted suggested that we should rework the decoder skeleton to more
closely match the ARM ARM documentation, it seemed to me like a good
idea to get this first set of patches out now for review, even though
you can't do very much with just branch instructions. Plus I'm hoping
that a set of dozen-patch mouthfuls will go down more smoothly than
if we drop the whole thing on the list at once :-)

Contents:
 * the new decoder skeleton,
 * gdbstub support for FP insns
 * a patch from me which gives the A64 decoder its own
   gen_intermediate_code_internal() loop for simplicity
 * the branch related patches from Alex's series, inserted into
   the new decoder skeleton

These patches sit on top of the v8 KVM control patchset I posted
last week. You can find a git tree with them here:
 git://git.linaro.org/people/pmaydell/qemu-arm.git a64-first-set
web UI:
 https://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/a64-first-set

I think these patches are ready to commit to mainline [ie they have
passed my personal code review]; further review appreciated.
My plan is to put them into the target-arm queue in these smallish
chunks as they pass review.

I've checked and I believe I have the attributions and signoffs
correct here, but it's easy for them to get accidentally mangled
in the course of rebasing, so please say if I missed one.

(The next set is probably going to be a bunch of integer instructions;
we now have a setup which will let us do some decent testing of these
with risu.)


Alexander Graf (7):
  target-arm: A64: add set_pc cpu method
  target-arm: A64: add stubs for a64 specific helpers
  target-arm: A64: add support for B and BL insns
  target-arm: A64: add support for BR, BLR and RET insns
  target-arm: A64: add support for conditional branches
  target-arm: A64: add support for 'test and branch' imm
  target-arm: A64: add support for compare and branch imm

Claudio Fontana (2):
  target-arm: A64: provide skeleton for a64 insn decoding
  target-arm: A64: expand decoding skeleton for system instructions

Peter Maydell (3):
  target-arm: Split A64 from A32/T32 gen_intermediate_code_internal()
  target-arm: A64: provide functions for accessing FPCR and FPSR
  target-arm: Support fp registers in gdb stub

 configure                  |    2 +-
 gdb-xml/aarch64-fpu.xml    |   86 +++++
 target-arm/Makefile.objs   |    2 +-
 target-arm/cpu.h           |   28 ++
 target-arm/cpu64.c         |   11 +
 target-arm/helper-a64.c    |   25 ++
 target-arm/helper-a64.h    |   18 +
 target-arm/helper.c        |   48 ++-
 target-arm/helper.h        |    4 +
 target-arm/translate-a64.c |  877 +++++++++++++++++++++++++++++++++++++++++++-
 target-arm/translate.c     |   76 ++--
 target-arm/translate.h     |   25 +-
 12 files changed, 1141 insertions(+), 61 deletions(-)
 create mode 100644 gdb-xml/aarch64-fpu.xml
 create mode 100644 target-arm/helper-a64.c
 create mode 100644 target-arm/helper-a64.h

-- 
1.7.9.5

[Qemu-devel] [PATCH 01/12] target-arm: Split A64 from A32/T32 gen_intermediate_code_internal()

[Peter Maydell]

The A32/T32 gen_intermediate_code_internal() is complicated because it
has to deal with:
 * conditionally executed instructions
 * Thumb IT blocks
 * kernel helper page
 * M profile exception-exit special casing

None of these apply to A64, so putting the "this is A64 so
call the A64 decoder" check in the middle of the A32/T32
loop is confusing and means the A64 decoder's handling of
things like conditional jump and singlestepping has to take
account of the conditional-execution jumps the main loop
might emit.

Refactor the code to give A64 its own gen_intermediate_code_internal
function instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |  209 ++++++++++++++++++++++++++++++++++++++++++--
 target-arm/translate.c     |   62 +++++--------
 target-arm/translate.h     |   20 ++++-
 3 files changed, 246 insertions(+), 45 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 932b601..a713137 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -28,6 +28,8 @@
 #include "translate.h"
 #include "qemu/host-utils.h"
 
+#include "exec/gen-icount.h"
+
 #include "helper.h"
 #define GEN_HELPER 1
 #include "helper.h"
@@ -106,7 +108,42 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp)
 {
     gen_a64_set_pc_im(s->pc - offset);
     gen_exception(excp);
-    s->is_jmp = DISAS_JUMP;
+    s->is_jmp = DISAS_EXC;
+}
+
+static inline bool use_goto_tb(DisasContext *s, int n, uint64_t dest)
+{
+    /* No direct tb linking with singlestep or deterministic io */
+    if (s->singlestep_enabled || (s->tb->cflags & CF_LAST_IO)) {
+        return false;
+    }
+
+    /* Only link tbs from inside the same guest page */
+    if ((s->tb->pc & TARGET_PAGE_MASK) != (dest & TARGET_PAGE_MASK)) {
+        return false;
+    }
+
+    return true;
+}
+
+static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
+{
+    TranslationBlock *tb;
+
+    tb = s->tb;
+    if (use_goto_tb(s, n, dest)) {
+        tcg_gen_goto_tb(n);
+        gen_a64_set_pc_im(dest);
+        tcg_gen_exit_tb((tcg_target_long)tb + n);
+        s->is_jmp = DISAS_TB_JUMP;
+    } else {
+        gen_a64_set_pc_im(dest);
+        if (s->singlestep_enabled) {
+            gen_exception(EXCP_DEBUG);
+        }
+        tcg_gen_exit_tb(0);
+        s->is_jmp = DISAS_JUMP;
+    }
 }
 
 static void real_unallocated_encoding(DisasContext *s)
@@ -120,7 +157,7 @@ static void real_unallocated_encoding(DisasContext *s)
     real_unallocated_encoding(s); \
     } while (0)
 
-void disas_a64_insn(CPUARMState *env, DisasContext *s)
+static void disas_a64_insn(CPUARMState *env, DisasContext *s)
 {
     uint32_t insn;
 
@@ -133,9 +170,171 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s)
         unallocated_encoding(s);
         break;
     }
+}
 
-    if (unlikely(s->singlestep_enabled) && (s->is_jmp == DISAS_TB_JUMP)) {
-        /* go through the main loop for single step */
-        s->is_jmp = DISAS_JUMP;
+void gen_intermediate_code_internal_a64(ARMCPU *cpu,
+                                        TranslationBlock *tb,
+                                        bool search_pc)
+{
+    CPUState *cs = CPU(cpu);
+    CPUARMState *env = &cpu->env;
+    DisasContext dc1, *dc = &dc1;
+    CPUBreakpoint *bp;
+    uint16_t *gen_opc_end;
+    int j, lj;
+    target_ulong pc_start;
+    target_ulong next_page_start;
+    int num_insns;
+    int max_insns;
+
+    pc_start = tb->pc;
+
+    dc->tb = tb;
+
+    gen_opc_end = tcg_ctx.gen_opc_buf + OPC_MAX_SIZE;
+
+    dc->is_jmp = DISAS_NEXT;
+    dc->pc = pc_start;
+    dc->singlestep_enabled = cs->singlestep_enabled;
+    dc->condjmp = 0;
+
+    dc->aarch64 = 1;
+    dc->thumb = 0;
+    dc->bswap_code = 0;
+    dc->condexec_mask = 0;
+    dc->condexec_cond = 0;
+#if !defined(CONFIG_USER_ONLY)
+    dc->user = 0;
+#endif
+    dc->vfp_enabled = 0;
+    dc->vec_len = 0;
+    dc->vec_stride = 0;
+
+    next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE;
+    lj = -1;
+    num_insns = 0;
+    max_insns = tb->cflags & CF_COUNT_MASK;
+    if (max_insns == 0) {
+        max_insns = CF_COUNT_MASK;
+    }
+
+    gen_tb_start();
+
+    tcg_clear_temp_count();
+
+    do {
+        if (unlikely(!QTAILQ_EMPTY(&env->breakpoints))) {
+            QTAILQ_FOREACH(bp, &env->breakpoints, entry) {
+                if (bp->pc == dc->pc) {
+                    gen_exception_insn(dc, 0, EXCP_DEBUG);
+                    /* Advance PC so that clearing the breakpoint will
+                       invalidate this TB.  */
+                    dc->pc += 2;
+                    goto done_generating;
+                }
+            }
+        }
+
+        if (search_pc) {
+            j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
+            if (lj < j) {
+                lj++;
+                while (lj < j) {
+                    tcg_ctx.gen_opc_instr_start[lj++] = 0;
+                }
+            }
+            tcg_ctx.gen_opc_pc[lj] = dc->pc;
+            tcg_ctx.gen_opc_instr_start[lj] = 1;
+            tcg_ctx.gen_opc_icount[lj] = num_insns;
+        }
+
+        if (num_insns + 1 == max_insns && (tb->cflags & CF_LAST_IO)) {
+            gen_io_start();
+        }
+
+        if (unlikely(qemu_loglevel_mask(CPU_LOG_TB_OP | CPU_LOG_TB_OP_OPT))) {
+            tcg_gen_debug_insn_start(dc->pc);
+        }
+
+        disas_a64_insn(env, dc);
+
+        if (tcg_check_temp_count()) {
+            fprintf(stderr, "TCG temporary leak before "TARGET_FMT_lx"\n",
+                    dc->pc);
+        }
+
+        /* Translation stops when a conditional branch is encountered.
+         * Otherwise the subsequent code could get translated several times.
+         * Also stop translation when a page boundary is reached.  This
+         * ensures prefetch aborts occur at the right place.
+         */
+        num_insns++;
+    } while (!dc->is_jmp && tcg_ctx.gen_opc_ptr < gen_opc_end &&
+             !cs->singlestep_enabled &&
+             !singlestep &&
+             dc->pc < next_page_start &&
+             num_insns < max_insns);
+
+    if (tb->cflags & CF_LAST_IO) {
+        gen_io_end();
+    }
+
+    if (unlikely(cs->singlestep_enabled) && dc->is_jmp != DISAS_EXC) {
+        /* Note that this means single stepping WFI doesn't halt the CPU.
+         * For conditional branch insns this is harmless unreachable code as
+         * gen_goto_tb() has already handled emitting the debug exception
+         * (and thus a tb-jump is not possible when singlestepping).
+         */
+        assert(dc->is_jmp != DISAS_TB_JUMP);
+        if (dc->is_jmp != DISAS_JUMP) {
+            gen_a64_set_pc_im(dc->pc);
+        }
+        gen_exception(EXCP_DEBUG);
+    } else {
+        switch (dc->is_jmp) {
+        case DISAS_NEXT:
+            gen_goto_tb(dc, 1, dc->pc);
+            break;
+        default:
+        case DISAS_JUMP:
+        case DISAS_UPDATE:
+            /* indicate that the hash table must be used to find the next TB */
+            tcg_gen_exit_tb(0);
+            break;
+        case DISAS_TB_JUMP:
+        case DISAS_EXC:
+        case DISAS_SWI:
+            break;
+        case DISAS_WFI:
+            /* This is a special case because we don't want to just halt the CPU
+             * if trying to debug across a WFI.
+             */
+            gen_helper_wfi(cpu_env);
+            break;
+        }
+    }
+
+done_generating:
+    gen_tb_end(tb, num_insns);
+    *tcg_ctx.gen_opc_ptr = INDEX_op_end;
+
+#ifdef DEBUG_DISAS
+    if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM)) {
+        qemu_log("----------------\n");
+        qemu_log("IN: %s\n", lookup_symbol(pc_start));
+        log_target_disas(env, pc_start, dc->pc - pc_start,
+                         dc->thumb | (dc->bswap_code << 1));
+        qemu_log("\n");
+    }
+#endif
+    if (search_pc) {
+        j = tcg_ctx.gen_opc_ptr - tcg_ctx.gen_opc_buf;
+        lj++;
+        while (lj <= j) {
+            tcg_ctx.gen_opc_instr_start[lj++] = 0;
+        }
+    } else {
+        tb->size = dc->pc - pc_start;
+        tb->icount = num_insns;
     }
 }
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5f003e7..553bded 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -56,11 +56,6 @@ static uint32_t gen_opc_condexec_bits[OPC_BUF_SIZE];
 #define IS_USER(s) (s->user)
 #endif
 
-/* These instructions trap after executing, so defer them until after the
-   conditional execution state has been updated.  */
-#define DISAS_WFI 4
-#define DISAS_SWI 5
-
 TCGv_ptr cpu_env;
 /* We reuse the same 64-bit temporaries for efficiency.  */
 static TCGv_i64 cpu_V0, cpu_V1, cpu_M0;
@@ -906,11 +901,7 @@ DO_GEN_ST(st32)
 
 static inline void gen_set_pc_im(DisasContext *s, target_ulong val)
 {
-    if (s->aarch64) {
-        gen_a64_set_pc_im(val);
-    } else {
-        tcg_gen_movi_i32(cpu_R[15], val);
-    }
+    tcg_gen_movi_i32(cpu_R[15], val);
 }
 
 /* Force a TB lookup after an instruction that changes the CPU state.  */
@@ -10005,6 +9996,15 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     int max_insns;
 
     /* generate intermediate code */
+
+    /* The A64 decoder has its own top level loop, because it doesn't need
+     * the A32/T32 complexity to do with conditional execution/IT blocks/etc.
+     */
+    if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) {
+        gen_intermediate_code_internal_a64(cpu, tb, search_pc);
+        return;
+    }
+
     pc_start = tb->pc;
 
     dc->tb = tb;
@@ -10016,31 +10016,18 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     dc->singlestep_enabled = cs->singlestep_enabled;
     dc->condjmp = 0;
 
-    if (ARM_TBFLAG_AARCH64_STATE(tb->flags)) {
-        dc->aarch64 = 1;
-        dc->thumb = 0;
-        dc->bswap_code = 0;
-        dc->condexec_mask = 0;
-        dc->condexec_cond = 0;
+    dc->aarch64 = 0;
+    dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
+    dc->bswap_code = ARM_TBFLAG_BSWAP_CODE(tb->flags);
+    dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
+    dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
 #if !defined(CONFIG_USER_ONLY)
-        dc->user = 0;
+    dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
 #endif
-        dc->vfp_enabled = 0;
-        dc->vec_len = 0;
-        dc->vec_stride = 0;
-    } else {
-        dc->aarch64 = 0;
-        dc->thumb = ARM_TBFLAG_THUMB(tb->flags);
-        dc->bswap_code = ARM_TBFLAG_BSWAP_CODE(tb->flags);
-        dc->condexec_mask = (ARM_TBFLAG_CONDEXEC(tb->flags) & 0xf) << 1;
-        dc->condexec_cond = ARM_TBFLAG_CONDEXEC(tb->flags) >> 4;
-#if !defined(CONFIG_USER_ONLY)
-        dc->user = (ARM_TBFLAG_PRIV(tb->flags) == 0);
-#endif
-        dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
-        dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
-        dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
-    }
+    dc->vfp_enabled = ARM_TBFLAG_VFPEN(tb->flags);
+    dc->vec_len = ARM_TBFLAG_VECLEN(tb->flags);
+    dc->vec_stride = ARM_TBFLAG_VECSTRIDE(tb->flags);
+
     cpu_F0s = tcg_temp_new_i32();
     cpu_F1s = tcg_temp_new_i32();
     cpu_F0d = tcg_temp_new_i64();
@@ -10102,7 +10089,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     do {
 #ifdef CONFIG_USER_ONLY
         /* Intercept jump to the magic kernel page.  */
-        if (!dc->aarch64 && dc->pc >= 0xffff0000) {
+        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);
@@ -10150,9 +10137,7 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
             tcg_gen_debug_insn_start(dc->pc);
         }
 
-        if (dc->aarch64) {
-            disas_a64_insn(env, dc);
-        } else if (dc->thumb) {
+        if (dc->thumb) {
             disas_thumb_insn(env, dc);
             if (dc->condexec_mask) {
                 dc->condexec_cond = (dc->condexec_cond & 0xe)
@@ -10347,8 +10332,9 @@ void restore_state_to_opc(CPUARMState *env, TranslationBlock *tb, int pc_pos)
 {
     if (is_a64(env)) {
         env->pc = tcg_ctx.gen_opc_pc[pc_pos];
+        env->condexec_bits = 0;
     } else {
         env->regs[15] = tcg_ctx.gen_opc_pc[pc_pos];
+        env->condexec_bits = gen_opc_condexec_bits[pc_pos];
     }
-    env->condexec_bits = gen_opc_condexec_bits[pc_pos];
 }
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 67c7760..8789181 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -28,16 +28,32 @@ typedef struct DisasContext {
 
 extern TCGv_ptr cpu_env;
 
+/* target-specific extra values for is_jmp */
+/* These instructions trap after executing, so the A32/T32 decoder must
+ * defer them until after the conditional execution state has been updated.
+ * WFI also needs special handling when single-stepping.
+ */
+#define DISAS_WFI 4
+#define DISAS_SWI 5
+/* For instructions which unconditionally cause an exception we can skip
+ * emitting unreachable code at the end of the TB in the A64 decoder
+ */
+#define DISAS_EXC 6
+
 #ifdef TARGET_AARCH64
 void a64_translate_init(void);
-void disas_a64_insn(CPUARMState *env, DisasContext *s);
+void gen_intermediate_code_internal_a64(ARMCPU *cpu,
+                                        TranslationBlock *tb,
+                                        bool search_pc);
 void gen_a64_set_pc_im(uint64_t val);
 #else
 static inline void a64_translate_init(void)
 {
 }
 
-static inline void disas_a64_insn(CPUARMState *env, DisasContext *s)
+static inline void gen_intermediate_code_internal_a64(ARMCPU *cpu,
+                                                      TranslationBlock *tb,
+                                                      bool search_pc)
 {
 }
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 01/12] target-arm: Split A64 from A32/T32 gen_intermediate_code_internal()

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> The A32/T32 gen_intermediate_code_internal() is complicated because it
> has to deal with:
>  * conditionally executed instructions
>  * Thumb IT blocks
>  * kernel helper page
>  * M profile exception-exit special casing
> 
> None of these apply to A64, so putting the "this is A64 so
> call the A64 decoder" check in the middle of the A32/T32
> loop is confusing and means the A64 decoder's handling of
> things like conditional jump and singlestepping has to take
> account of the conditional-execution jumps the main loop
> might emit.
> 
> Refactor the code to give A64 its own gen_intermediate_code_internal
> function instead.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |  209 ++++++++++++++++++++++++++++++++++++++++++--
>  target-arm/translate.c     |   62 +++++--------
>  target-arm/translate.h     |   20 ++++-
>  3 files changed, 246 insertions(+), 45 deletions(-)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 02/12] target-arm: A64: add set_pc cpu method

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

When executing translation blocks we need to be able to recover
our program counter. Add a method to set it for AArch64 CPUs.
This covers user-mode, but for system mode emulation we will
need to check if the CPU is in an AArch32 execution state.

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu64.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 3e99c21..04ce879 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -68,11 +68,22 @@ static void aarch64_cpu_finalizefn(Object *obj)
 {
 }
 
+static void aarch64_cpu_set_pc(CPUState *cs, vaddr value)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+    /*
+     * TODO: this will need updating for system emulation,
+     * when the core may be in AArch32 mode.
+     */
+    cpu->env.pc = value;
+}
+
 static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
 {
     CPUClass *cc = CPU_CLASS(oc);
 
     cc->dump_state = aarch64_cpu_dump_state;
+    cc->set_pc = aarch64_cpu_set_pc;
     cc->gdb_read_register = aarch64_cpu_gdb_read_register;
     cc->gdb_write_register = aarch64_cpu_gdb_write_register;
     cc->gdb_num_core_regs = 34;
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 02/12] target-arm: A64: add set_pc cpu method

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> When executing translation blocks we need to be able to recover
> our program counter. Add a method to set it for AArch64 CPUs.
> This covers user-mode, but for system mode emulation we will
> need to check if the CPU is in an AArch32 execution state.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu64.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 03/12] target-arm: A64: provide functions for accessing FPCR and FPSR

[Peter Maydell]

The information which AArch32 holds in the FPSCR is split for
AArch64 into two logically distinct registers, FPSR and FPCR.
Since they are carefully arranged to use non-overlapping bits,
we leave the underlying state in the same place, and provide
accessor functions which just update the appropriate bits
via vfp_get_fpscr() and vfp_set_fpscr().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index ff7aac5..4807354 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -445,6 +445,34 @@ static inline void xpsr_write(CPUARMState *env, uint32_t val, uint32_t mask)
 uint32_t vfp_get_fpscr(CPUARMState *env);
 void vfp_set_fpscr(CPUARMState *env, uint32_t val);
 
+/* For A64 the FPSCR is split into two logically distinct registers,
+ * FPCR and FPSR. However since they still use non-overlapping bits
+ * we store the underlying state in fpscr and just mask on read/write.
+ */
+#define FPSR_MASK 0xf800009f
+#define FPCR_MASK 0x07f79f00
+static inline uint32_t vfp_get_fpsr(CPUARMState *env)
+{
+    return vfp_get_fpscr(env) & FPSR_MASK;
+}
+
+static inline void vfp_set_fpsr(CPUARMState *env, uint32_t val)
+{
+    uint32_t new_fpscr = (vfp_get_fpscr(env) & ~FPSR_MASK) | (val & FPSR_MASK);
+    vfp_set_fpscr(env, new_fpscr);
+}
+
+static inline uint32_t vfp_get_fpcr(CPUARMState *env)
+{
+    return vfp_get_fpscr(env) & FPCR_MASK;
+}
+
+static inline void vfp_set_fpcr(CPUARMState *env, uint32_t val)
+{
+    uint32_t new_fpscr = (vfp_get_fpscr(env) & ~FPCR_MASK) | (val & FPCR_MASK);
+    vfp_set_fpscr(env, new_fpscr);
+}
+
 enum arm_cpu_mode {
   ARM_CPU_MODE_USR = 0x10,
   ARM_CPU_MODE_FIQ = 0x11,
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 03/12] target-arm: A64: provide functions for accessing FPCR and FPSR

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> The information which AArch32 holds in the FPSCR is split for
> AArch64 into two logically distinct registers, FPSR and FPCR.
> Since they are carefully arranged to use non-overlapping bits,
> we leave the underlying state in the same place, and provide
> accessor functions which just update the appropriate bits
> via vfp_get_fpscr() and vfp_set_fpscr().
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h |   28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 04/12] target-arm: Support fp registers in gdb stub

[Peter Maydell]

Register the aarch64-fpu XML and implement the necessary
read/write handlers so we can support reading and writing
of FP registers in the gdb stub.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure               |    2 +-
 gdb-xml/aarch64-fpu.xml |   86 +++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/helper.c     |   48 +++++++++++++++++++++++++-
 3 files changed, 134 insertions(+), 2 deletions(-)
 create mode 100644 gdb-xml/aarch64-fpu.xml

diff --git a/configure b/configure
index 3317013..c9ad1de 100755
--- a/configure
+++ b/configure
@@ -4401,7 +4401,7 @@ case "$target_name" in
   aarch64)
     TARGET_BASE_ARCH=arm
     bflt="yes"
-    gdb_xml_files="aarch64-core.xml"
+    gdb_xml_files="aarch64-core.xml aarch64-fpu.xml"
   ;;
   cris)
   ;;
diff --git a/gdb-xml/aarch64-fpu.xml b/gdb-xml/aarch64-fpu.xml
new file mode 100644
index 0000000..997197e
--- /dev/null
+++ b/gdb-xml/aarch64-fpu.xml
@@ -0,0 +1,86 @@
+<?xml version="1.0"?>
+<!-- Copyright (C) 2009-2012 Free Software Foundation, Inc.
+     Contributed by ARM Ltd.
+
+     Copying and distribution of this file, with or without modification,
+     are permitted in any medium without royalty provided the copyright
+     notice and this notice are preserved.  -->
+
+<!DOCTYPE feature SYSTEM "gdb-target.dtd">
+<feature name="org.gnu.gdb.aarch64.fpu">
+  <vector id="v2d" type="ieee_double" count="2"/>
+  <vector id="v2u" type="uint64" count="2"/>
+  <vector id="v2i" type="int64" count="2"/>
+  <vector id="v4f" type="ieee_single" count="4"/>
+  <vector id="v4u" type="uint32" count="4"/>
+  <vector id="v4i" type="int32" count="4"/>
+  <vector id="v8u" type="uint16" count="8"/>
+  <vector id="v8i" type="int16" count="8"/>
+  <vector id="v16u" type="uint8" count="16"/>
+  <vector id="v16i" type="int8" count="16"/>
+  <vector id="v1u" type="uint128" count="1"/>
+  <vector id="v1i" type="int128" count="1"/>
+  <union id="vnd">
+    <field name="f" type="v2d"/>
+    <field name="u" type="v2u"/>
+    <field name="s" type="v2i"/>
+  </union>
+  <union id="vns">
+    <field name="f" type="v4f"/>
+    <field name="u" type="v4u"/>
+    <field name="s" type="v4i"/>
+  </union>
+  <union id="vnh">
+    <field name="u" type="v8u"/>
+    <field name="s" type="v8i"/>
+  </union>
+  <union id="vnb">
+    <field name="u" type="v16u"/>
+    <field name="s" type="v16i"/>
+  </union>
+  <union id="vnq">
+    <field name="u" type="v1u"/>
+    <field name="s" type="v1i"/>
+  </union>
+  <union id="aarch64v">
+    <field name="d" type="vnd"/>
+    <field name="s" type="vns"/>
+    <field name="h" type="vnh"/>
+    <field name="b" type="vnb"/>
+    <field name="q" type="vnq"/>
+  </union>
+  <reg name="v0" bitsize="128" type="aarch64v" regnum="34"/>
+  <reg name="v1" bitsize="128" type="aarch64v" />
+  <reg name="v2" bitsize="128" type="aarch64v" />
+  <reg name="v3" bitsize="128" type="aarch64v" />
+  <reg name="v4" bitsize="128" type="aarch64v" />
+  <reg name="v5" bitsize="128" type="aarch64v" />
+  <reg name="v6" bitsize="128" type="aarch64v" />
+  <reg name="v7" bitsize="128" type="aarch64v" />
+  <reg name="v8" bitsize="128" type="aarch64v" />
+  <reg name="v9" bitsize="128" type="aarch64v" />
+  <reg name="v10" bitsize="128" type="aarch64v"/>
+  <reg name="v11" bitsize="128" type="aarch64v"/>
+  <reg name="v12" bitsize="128" type="aarch64v"/>
+  <reg name="v13" bitsize="128" type="aarch64v"/>
+  <reg name="v14" bitsize="128" type="aarch64v"/>
+  <reg name="v15" bitsize="128" type="aarch64v"/>
+  <reg name="v16" bitsize="128" type="aarch64v"/>
+  <reg name="v17" bitsize="128" type="aarch64v"/>
+  <reg name="v18" bitsize="128" type="aarch64v"/>
+  <reg name="v19" bitsize="128" type="aarch64v"/>
+  <reg name="v20" bitsize="128" type="aarch64v"/>
+  <reg name="v21" bitsize="128" type="aarch64v"/>
+  <reg name="v22" bitsize="128" type="aarch64v"/>
+  <reg name="v23" bitsize="128" type="aarch64v"/>
+  <reg name="v24" bitsize="128" type="aarch64v"/>
+  <reg name="v25" bitsize="128" type="aarch64v"/>
+  <reg name="v26" bitsize="128" type="aarch64v"/>
+  <reg name="v27" bitsize="128" type="aarch64v"/>
+  <reg name="v28" bitsize="128" type="aarch64v"/>
+  <reg name="v29" bitsize="128" type="aarch64v"/>
+  <reg name="v30" bitsize="128" type="aarch64v"/>
+  <reg name="v31" bitsize="128" type="aarch64v"/>
+  <reg name="fpsr" bitsize="32"/>
+  <reg name="fpcr" bitsize="32"/>
+</feature>
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 263dbbf..73c97e8 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -65,6 +65,48 @@ static int vfp_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
     return 0;
 }
 
+static int aarch64_fpu_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+    switch (reg) {
+    case 0 ... 31:
+        /* 128 bit FP register */
+        stfq_le_p(buf, env->vfp.regs[reg * 2]);
+        stfq_le_p(buf + 8, env->vfp.regs[reg * 2 + 1]);
+        return 16;
+    case 32:
+        /* FPSR */
+        stl_p(buf, vfp_get_fpsr(env));
+        return 4;
+    case 33:
+        /* FPCR */
+        stl_p(buf, vfp_get_fpcr(env));
+        return 4;
+    default:
+        return 0;
+    }
+}
+
+static int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+    switch (reg) {
+    case 0 ... 31:
+        /* 128 bit FP register */
+        env->vfp.regs[reg * 2] = ldfq_le_p(buf);
+        env->vfp.regs[reg * 2 + 1] = ldfq_le_p(buf + 8);
+        return 16;
+    case 32:
+        /* FPSR */
+        vfp_set_fpsr(env, ldl_p(buf));
+        return 4;
+    case 33:
+        /* FPCR */
+        vfp_set_fpcr(env, ldl_p(buf));
+        return 4;
+    default:
+        return 0;
+    }
+}
+
 static int raw_read(CPUARMState *env, const ARMCPRegInfo *ri,
                     uint64_t *value)
 {
@@ -1785,7 +1827,11 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
     CPUState *cs = CPU(cpu);
     CPUARMState *env = &cpu->env;
 
-    if (arm_feature(env, ARM_FEATURE_NEON)) {
+    if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+        gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg,
+                                 aarch64_fpu_gdb_set_reg,
+                                 34, "aarch64-fpu.xml", 0);
+    } else if (arm_feature(env, ARM_FEATURE_NEON)) {
         gdb_register_coprocessor(cs, vfp_gdb_get_reg, vfp_gdb_set_reg,
                                  51, "arm-neon.xml", 0);
     } else if (arm_feature(env, ARM_FEATURE_VFP3)) {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 04/12] target-arm: Support fp registers in gdb stub

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> Register the aarch64-fpu XML and implement the necessary
> read/write handlers so we can support reading and writing
> of FP registers in the gdb stub.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure               |    2 +-
>  gdb-xml/aarch64-fpu.xml |   86 +++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/helper.c     |   48 +++++++++++++++++++++++++-
>  3 files changed, 134 insertions(+), 2 deletions(-)
>  create mode 100644 gdb-xml/aarch64-fpu.xml


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 05/12] target-arm: A64: add stubs for a64 specific helpers

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

We will need helpers that only make sense with AArch64. Add
helper-a64.{c,h} files as stubs that we can fill with these
helpers in the following patches.

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/Makefile.objs |    2 +-
 target-arm/helper-a64.c  |   25 +++++++++++++++++++++++++
 target-arm/helper-a64.h  |   18 ++++++++++++++++++
 target-arm/helper.h      |    4 ++++
 4 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 target-arm/helper-a64.c
 create mode 100644 target-arm/helper-a64.h

diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index 5493a4c..a5914e9 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -5,6 +5,6 @@ obj-$(call lnot,$(CONFIG_KVM)) += kvm-stub.o
 obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += neon_helper.o iwmmxt_helper.o
 obj-y += gdbstub.o
-obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o gdbstub64.o
+obj-$(TARGET_AARCH64) += cpu64.o translate-a64.o helper-a64.o gdbstub64.o
 obj-$(call land,$(CONFIG_KVM),$(call lnot,$(TARGET_AARCH64))) += kvm32.o
 obj-$(call land,$(CONFIG_KVM),$(TARGET_AARCH64)) += kvm64.o
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
new file mode 100644
index 0000000..adb8428
--- /dev/null
+++ b/target-arm/helper-a64.c
@@ -0,0 +1,25 @@
+/*
+ *  AArch64 specific helpers
+ *
+ *  Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "cpu.h"
+#include "exec/gdbstub.h"
+#include "helper.h"
+#include "qemu/host-utils.h"
+#include "sysemu/sysemu.h"
+#include "qemu/bitops.h"
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
new file mode 100644
index 0000000..dd28306
--- /dev/null
+++ b/target-arm/helper-a64.h
@@ -0,0 +1,18 @@
+/*
+ *  AArch64 specific helper definitions
+ *
+ *  Copyright (c) 2013 Alexander Graf <agraf@suse.de>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
diff --git a/target-arm/helper.h b/target-arm/helper.h
index cac9564..f3727a5 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -458,4 +458,8 @@ DEF_HELPER_3(neon_qzip8, void, env, i32, i32)
 DEF_HELPER_3(neon_qzip16, void, env, i32, i32)
 DEF_HELPER_3(neon_qzip32, void, env, i32, i32)
 
+#ifdef TARGET_AARCH64
+#include "helper-a64.h"
+#endif
+
 #include "exec/def-helper.h"
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 05/12] target-arm: A64: add stubs for a64 specific helpers

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> We will need helpers that only make sense with AArch64. Add
> helper-a64.{c,h} files as stubs that we can fill with these
> helpers in the following patches.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/Makefile.objs |    2 +-
>  target-arm/helper-a64.c  |   25 +++++++++++++++++++++++++
>  target-arm/helper-a64.h  |   18 ++++++++++++++++++
>  target-arm/helper.h      |    4 ++++
>  4 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100644 target-arm/helper-a64.c
>  create mode 100644 target-arm/helper-a64.h

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 06/12] target-arm: A64: provide skeleton for a64 insn decoding

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

Provide a skeleton for a64 instruction decoding in translate-a64.c,
by dividing instructions into the classes defined by the
ARM Architecture Reference Manual(DDI0487A_a) section C3.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |  370 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 362 insertions(+), 8 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index a713137..8e16cb1 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -146,17 +146,348 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
     }
 }
 
-static void real_unallocated_encoding(DisasContext *s)
+static void unallocated_encoding(DisasContext *s)
 {
-    fprintf(stderr, "Unknown instruction: %#x\n", s->insn);
     gen_exception_insn(s, 4, EXCP_UDEF);
 }
 
-#define unallocated_encoding(s) do { \
-    fprintf(stderr, "unallocated encoding at line: %d\n", __LINE__); \
-    real_unallocated_encoding(s); \
-    } while (0)
+#define unsupported_encoding(s, insn)                                    \
+    do {                                                                 \
+        qemu_log_mask(LOG_UNIMP,                                         \
+                      "%s:%d: unsupported instruction encoding 0x%08x "  \
+                      "at pc=%016" PRIx64 "\n",                          \
+                      __FILE__, __LINE__, insn, s->pc - 4);              \
+        unallocated_encoding(s);                                         \
+    } while (0);
 
+/*
+ * the instruction disassembly implemented here matches
+ * the instruction encoding classifications in chapter 3 (C3)
+ * of the ARM Architecture Reference Manual (DDI0487A_a)
+ */
+
+/* Unconditional branch (immediate) */
+static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Compare & branch (immediate) */
+static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Test & branch (immediate) */
+static void disas_test_b_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Conditional branch (immediate) */
+static void disas_cond_b_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* System */
+static void disas_system(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Exception generation */
+static void disas_exc(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Unconditional branch (register) */
+static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.2 Branches, exception generating and system instructions */
+static void disas_b_exc_sys(DisasContext *s, uint32_t insn)
+{
+    switch (extract32(insn, 25, 7)) {
+    case 0x0a: case 0x0b:
+    case 0x4a: case 0x4b: /* Unconditional branch (immediate) */
+        disas_uncond_b_imm(s, insn);
+        break;
+    case 0x1a: case 0x5a: /* Compare & branch (immediate) */
+        disas_comp_b_imm(s, insn);
+        break;
+    case 0x1b: case 0x5b: /* Test & branch (immediate) */
+        disas_test_b_imm(s, insn);
+        break;
+    case 0x2a: /* Conditional branch (immediate) */
+        disas_cond_b_imm(s, insn);
+        break;
+    case 0x6a: /* Exception generation / System */
+        if (insn & (1 << 24)) {
+            disas_system(s, insn);
+        } else {
+            disas_exc(s, insn);
+        }
+        break;
+    case 0x6b: /* Unconditional branch (register) */
+        disas_uncond_b_reg(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
+}
+
+/* Load/store exclusive */
+static void disas_ldst_excl(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Load register (literal) */
+static void disas_ld_lit(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Load/store pair (all forms) */
+static void disas_ldst_pair(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Load/store register (all forms) */
+static void disas_ldst_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* AdvSIMD load/store multiple structures */
+static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* AdvSIMD load/store single structure */
+static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.3 Loads and stores */
+static void disas_ldst(DisasContext *s, uint32_t insn)
+{
+    switch (extract32(insn, 24, 6)) {
+    case 0x08: /* Load/store exclusive */
+        disas_ldst_excl(s, insn);
+        break;
+    case 0x18: case 0x1c: /* Load register (literal) */
+        disas_ld_lit(s, insn);
+        break;
+    case 0x28: case 0x29:
+    case 0x2c: case 0x2d: /* Load/store pair (all forms) */
+        disas_ldst_pair(s, insn);
+        break;
+    case 0x38: case 0x39:
+    case 0x3c: case 0x3d: /* Load/store register (all forms) */
+        disas_ldst_reg(s, insn);
+        break;
+    case 0x0c: /* AdvSIMD load/store multiple structures */
+        disas_ldst_multiple_struct(s, insn);
+        break;
+    case 0x0d: /* AdvSIMD load/store single structure */
+        disas_ldst_single_struct(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
+}
+
+/* PC-rel. addressing */
+static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Add/subtract (immediate) */
+static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Logical (immediate) */
+static void disas_logic_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Move wide (immediate) */
+static void disas_movw_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Bitfield */
+static void disas_bitfield(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Extract */
+static void disas_extract(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.4 Data processing - immediate */
+static void disas_data_proc_imm(DisasContext *s, uint32_t insn)
+{
+    switch (extract32(insn, 23, 6)) {
+    case 0x20: case 0x21: /* PC-rel. addressing */
+        disas_pc_rel_adr(s, insn);
+        break;
+    case 0x22: case 0x23: /* Add/subtract (immediate) */
+        disas_add_sub_imm(s, insn);
+        break;
+    case 0x24: /* Logical (immediate) */
+        disas_logic_imm(s, insn);
+        break;
+    case 0x25: /* Move wide (immediate) */
+        disas_movw_imm(s, insn);
+        break;
+    case 0x26: /* Bitfield */
+        disas_bitfield(s, insn);
+        break;
+    case 0x27: /* Extract */
+        disas_extract(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
+}
+
+/* Logical (shifted register) */
+static void disas_logic_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Add/subtract (extended register) */
+static void disas_add_sub_ext_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Add/subtract (shifted register) */
+static void disas_add_sub_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Data-processing (3 source) */
+static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Add/subtract (with carry) */
+static void disas_adc_sbc(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Conditional compare (immediate) */
+static void disas_cc_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Conditional compare (register) */
+static void disas_cc_reg(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Conditional select */
+static void disas_cond_select(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Data-processing (1 source) */
+static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* Data-processing (2 source) */
+static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.5 Data processing - register */
+static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
+{
+    switch (extract32(insn, 24, 5)) {
+    case 0x0a: /* Logical (shifted register) */
+        disas_logic_reg(s, insn);
+        break;
+    case 0x0b: /* Add/subtract */
+        if (insn & (1 << 21)) { /* (extended register) */
+            disas_add_sub_ext_reg(s, insn);
+        } else {
+            disas_add_sub_reg(s, insn);
+        }
+        break;
+    case 0x1b: /* Data-processing (3 source) */
+        disas_data_proc_3src(s, insn);
+        break;
+    case 0x1a:
+        switch (extract32(insn, 21, 3)) {
+        case 0x0: /* Add/subtract (with carry) */
+            disas_adc_sbc(s, insn);
+            break;
+        case 0x2: /* Conditional compare */
+            if (insn & (1 << 11)) { /* (immediate) */
+                disas_cc_imm(s, insn);
+            } else {            /* (register) */
+                disas_cc_reg(s, insn);
+            }
+            break;
+        case 0x4: /* Conditional select */
+            disas_cond_select(s, insn);
+            break;
+        case 0x6: /* Data-processing */
+            if (insn & (1 << 30)) { /* (1 source) */
+                disas_data_proc_1src(s, insn);
+            } else {            /* (2 source) */
+                disas_data_proc_2src(s, insn);
+            }
+            break;
+        default:
+            unallocated_encoding(s);
+            break;
+        }
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
+}
+
+/* C3.6 Data processing - SIMD and floating point */
+static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.1 A64 instruction index by encoding */
 static void disas_a64_insn(CPUARMState *env, DisasContext *s)
 {
     uint32_t insn;
@@ -165,10 +496,33 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s)
     s->insn = insn;
     s->pc += 4;
 
-    switch ((insn >> 24) & 0x1f) {
-    default:
+    switch (extract32(insn, 25, 4)) {
+    case 0x0: case 0x1: case 0x2: case 0x3: /* UNALLOCATED */
         unallocated_encoding(s);
         break;
+    case 0x8: case 0x9: /* Data processing - immediate */
+        disas_data_proc_imm(s, insn);
+        break;
+    case 0xa: case 0xb: /* Branch, exception generation and system insns */
+        disas_b_exc_sys(s, insn);
+        break;
+    case 0x4:
+    case 0x6:
+    case 0xc:
+    case 0xe:      /* Loads and stores */
+        disas_ldst(s, insn);
+        break;
+    case 0x5:
+    case 0xd:      /* Data processing - register */
+        disas_data_proc_reg(s, insn);
+        break;
+    case 0x7:
+    case 0xf:      /* Data processing - SIMD and floating point */
+        disas_data_proc_simd_fp(s, insn);
+        break;
+    default:
+        assert(FALSE); /* all 15 cases should be handled above */
+        break;
     }
 }
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 06/12] target-arm: A64: provide skeleton for a64 insn decoding

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> Provide a skeleton for a64 instruction decoding in translate-a64.c,
> by dividing instructions into the classes defined by the
> ARM Architecture Reference Manual(DDI0487A_a) section C3.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |  370 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 362 insertions(+), 8 deletions(-)


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 07/12] target-arm: A64: expand decoding skeleton for system instructions

[Peter Maydell]

From: Claudio Fontana <claudio.fontana@linaro.org>

Decode the various kinds of system instructions:
 hints (HINT), which include NOP, YIELD, WFE, WFI, SEV, SEL
 sync instructions, which include CLREX, DSB, DMB, ISB
 msr_i, which move immediate to processor state field
 sys, which include all SYS and SYSL instructions
 msr, which move from a gp register to a system register
 mrs, which move from a system register to a gp register

Provide implementations where they are trivial nops.

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |  130 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 128 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8e16cb1..29f001d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -190,12 +190,138 @@ static void disas_cond_b_imm(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* System */
-static void disas_system(DisasContext *s, uint32_t insn)
+/* C5.6.68 HINT */
+static void handle_hint(DisasContext *s, uint32_t insn,
+                        unsigned int op1, unsigned int op2, unsigned int crm)
+{
+    unsigned int selector = crm << 3 | op2;
+
+    if (op1 != 3) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    switch (selector) {
+    case 0: /* NOP */
+        return;
+    case 1: /* YIELD */
+    case 2: /* WFE */
+    case 3: /* WFI */
+    case 4: /* SEV */
+    case 5: /* SEVL */
+        /* we treat all as NOP at least for now */
+        return;
+    default:
+        /* default specified as NOP equivalent */
+        return;
+    }
+}
+
+/* CLREX, DSB, DMB, ISB */
+static void handle_sync(DisasContext *s, uint32_t insn,
+                        unsigned int op1, unsigned int op2, unsigned int crm)
+{
+    if (op1 != 3) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    switch (op2) {
+    case 2: /* CLREX */
+        unsupported_encoding(s, insn);
+        return;
+    case 4: /* DSB */
+    case 5: /* DMB */
+    case 6: /* ISB */
+        /* We don't emulate caches so barriers are no-ops */
+        return;
+    default:
+        unallocated_encoding(s);
+        return;
+    }
+}
+
+/* C5.6.130 MSR (immediate) - move immediate to processor state field */
+static void handle_msr_i(DisasContext *s, uint32_t insn,
+                         unsigned int op1, unsigned int op2, unsigned int crm)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C5.6.204 SYS */
+static void handle_sys(DisasContext *s, uint32_t insn, unsigned int l,
+                       unsigned int op1, unsigned int op2,
+                       unsigned int crn, unsigned int crm, unsigned int rt)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C5.6.129 MRS - move from system register */
+static void handle_mrs(DisasContext *s, uint32_t insn, unsigned int op0,
+                       unsigned int op1, unsigned int op2,
+                       unsigned int crn, unsigned int crm, unsigned int rt)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C5.6.131 MSR (register) - move to system register */
+static void handle_msr(DisasContext *s, uint32_t insn, unsigned int op0,
+                       unsigned int op1, unsigned int op2,
+                       unsigned int crn, unsigned int crm, unsigned int rt)
 {
     unsupported_encoding(s, insn);
 }
 
+/* C3.2.4 System */
+static void disas_system(DisasContext *s, uint32_t insn)
+{
+    /*
+     * 31 30 29 28 27 26 25 24 23 22 21 20 19 18   16 15   12 11   8 7   5 4  0
+     *  1  1  0  1  0  1  0  1  0  0  L  op0    op1     CRn     CRm   op2   Rt
+     */
+    unsigned int l, op0, op1, crn, crm, op2, rt;
+    l = extract32(insn, 21, 1);
+    op0 = extract32(insn, 19, 2);
+    op1 = extract32(insn, 16, 3);
+    crn = extract32(insn, 12, 4);
+    crm = extract32(insn, 8, 4);
+    op2 = extract32(insn, 5, 3);
+    rt = extract32(insn, 0, 5);
+
+    if (op0 == 0) {
+        if (l || rt != 31) {
+            unallocated_encoding(s);
+            return;
+        }
+        switch (crn) {
+        case 2: /* C5.6.68 HINT */
+            handle_hint(s, insn, op1, op2, crm);
+            break;
+        case 3: /* CLREX, DSB, DMB, ISB */
+            handle_sync(s, insn, op1, op2, crm);
+            break;
+        case 4: /* C5.6.130 MSR (immediate) */
+            handle_msr_i(s, insn, op1, op2, crm);
+            break;
+        default:
+            unallocated_encoding(s);
+            break;
+        }
+        return;
+    }
+
+    if (op0 == 1) {
+        /* C5.6.204 SYS */
+        handle_sys(s, insn, l, op1, op2, crn, crm, rt);
+    } else if (l) { /* op0 > 1 */
+        /* C5.6.129 MRS - move from system register */
+        handle_mrs(s, insn, op0, op1, op2, crn, crm, rt);
+    } else {
+        /* C5.6.131 MSR (register) - move to system register */
+        handle_msr(s, insn, op0, op1, op2, crn, crm, rt);
+    }
+}
+
 /* Exception generation */
 static void disas_exc(DisasContext *s, uint32_t insn)
 {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 07/12] target-arm: A64: expand decoding skeleton for system instructions

[Christopher Covington]

Hi Claudio, Peter,

On 12/03/2013 04:51 PM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> Decode the various kinds of system instructions:
>  hints (HINT), which include NOP, YIELD, WFE, WFI, SEV, SEL
>  sync instructions, which include CLREX, DSB, DMB, ISB
>  msr_i, which move immediate to processor state field
>  sys, which include all SYS and SYSL instructions
>  msr, which move from a gp register to a system register
>  mrs, which move from a system register to a gp register
> 
> Provide implementations where they are trivial nops.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |  130 +++++++++++++++++++++++++++++++++++++++++++-
[...]
> +/* C3.2.4 System */
> +static void disas_system(DisasContext *s, uint32_t insn)
> +{
> +    /*
> +     * 31 30 29 28 27 26 25 24 23 22 21 20 19 18   16 15   12 11   8 7   5 4  0
> +     *  1  1  0  1  0  1  0  1  0  0  L  op0    op1     CRn     CRm   op2   Rt
> +     */

[...]

Could this opcode legend get a pretty box like the others?

Thanks,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.

Re: [Qemu-devel] [PATCH 07/12] target-arm: A64: expand decoding skeleton for system instructions

[Peter Maydell]

On 3 December 2013 23:15, Christopher Covington <cov@codeaurora.org> wrote:
> Hi Claudio, Peter,
>
> On 12/03/2013 04:51 PM, Peter Maydell wrote:
>> From: Claudio Fontana <claudio.fontana@linaro.org>
>>
>> Decode the various kinds of system instructions:
>>  hints (HINT), which include NOP, YIELD, WFE, WFI, SEV, SEL
>>  sync instructions, which include CLREX, DSB, DMB, ISB
>>  msr_i, which move immediate to processor state field
>>  sys, which include all SYS and SYSL instructions
>>  msr, which move from a gp register to a system register
>>  mrs, which move from a system register to a gp register
>>
>> Provide implementations where they are trivial nops.
>>
>> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  target-arm/translate-a64.c |  130 +++++++++++++++++++++++++++++++++++++++++++-
> [...]
>> +/* C3.2.4 System */
>> +static void disas_system(DisasContext *s, uint32_t insn)
>> +{
>> +    /*
>> +     * 31 30 29 28 27 26 25 24 23 22 21 20 19 18   16 15   12 11   8 7   5 4  0
>> +     *  1  1  0  1  0  1  0  1  0  0  L  op0    op1     CRn     CRm   op2   Rt
>> +     */
>
> [...]
>
> Could this opcode legend get a pretty box like the others?

Rats, I thought I caught them all in my review pass. Will fix.

-- PMM

Re: [Qemu-devel] [PATCH 07/12] target-arm: A64: expand decoding skeleton for system instructions

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> Decode the various kinds of system instructions:
>  hints (HINT), which include NOP, YIELD, WFE, WFI, SEV, SEL
>  sync instructions, which include CLREX, DSB, DMB, ISB
>  msr_i, which move immediate to processor state field
>  sys, which include all SYS and SYSL instructions
>  msr, which move from a gp register to a system register
>  mrs, which move from a system register to a gp register
> 
> Provide implementations where they are trivial nops.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |  130 +++++++++++++++++++++++++++++++++++++++++++-


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 08/12] target-arm: A64: add support for B and BL insns

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Implement the B and BL instructions (PC relative branches and calls).

For convenience in managing TCG temporaries which might be generated
if a source register is the zero-register XZR, we provide a simple
mechanism for creating a new temp which is automatically freed at the
end of decode of the instruction.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: renamed functions, adapted to new decoder layout]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   45 ++++++++++++++++++++++++++++++++++++++++++--
 target-arm/translate.h     |    3 +++
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 29f001d..8535803 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -160,16 +160,53 @@ static void unallocated_encoding(DisasContext *s)
         unallocated_encoding(s);                                         \
     } while (0);
 
+static void free_tmp_a64(DisasContext *s)
+{
+    int i;
+    for (i = 0; i < s->tmp_a64_count; i++) {
+        tcg_temp_free_i64(s->tmp_a64[i]);
+    }
+    s->tmp_a64_count = 0;
+}
+
+static TCGv_i64 new_tmp_a64_zero(DisasContext *s)
+{
+    assert(s->tmp_a64_count < TMP_A64_MAX);
+    return s->tmp_a64[s->tmp_a64_count++] = tcg_const_i64(0);
+}
+
+static TCGv_i64 cpu_reg(DisasContext *s, int reg)
+{
+    if (reg == 31) {
+        return new_tmp_a64_zero(s);
+    } else {
+        return cpu_X[reg];
+    }
+}
+
 /*
  * the instruction disassembly implemented here matches
  * the instruction encoding classifications in chapter 3 (C3)
  * of the ARM Architecture Reference Manual (DDI0487A_a)
  */
 
-/* Unconditional branch (immediate) */
+/* C3.2.7 Unconditional branch (immediate)
+ *   31  30       26 25                                  0
+ * +----+-----------+-------------------------------------+
+ * | op | 0 0 1 0 1 |                 imm26               |
+ * +----+-----------+-------------------------------------+
+ */
 static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    uint64_t addr = s->pc + sextract32(insn, 0, 26) * 4 - 4;
+
+    if (insn & (1 << 31)) {
+        /* C5.6.26 BL Branch with link */
+        tcg_gen_movi_i64(cpu_reg(s, 30), s->pc);
+    }
+
+    /* C5.6.20 B Branch / C5.6.26 BL Branch with link */
+    gen_goto_tb(s, 0, addr);
 }
 
 /* Compare & branch (immediate) */
@@ -650,6 +687,9 @@ static void disas_a64_insn(CPUARMState *env, DisasContext *s)
         assert(FALSE); /* all 15 cases should be handled above */
         break;
     }
+
+    /* if we allocated any temporaries, free them here */
+    free_tmp_a64(s);
 }
 
 void gen_intermediate_code_internal_a64(ARMCPU *cpu,
@@ -679,6 +719,7 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
     dc->condjmp = 0;
 
     dc->aarch64 = 1;
+    dc->tmp_a64_count = 0;
     dc->thumb = 0;
     dc->bswap_code = 0;
     dc->condexec_mask = 0;
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 8789181..23a45da 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -24,6 +24,9 @@ typedef struct DisasContext {
     int vec_len;
     int vec_stride;
     int aarch64;
+#define TMP_A64_MAX 16
+    int tmp_a64_count;
+    TCGv_i64 tmp_a64[TMP_A64_MAX];
 } DisasContext;
 
 extern TCGv_ptr cpu_env;
-- 
1.7.9.5

[Qemu-devel] [PATCH 09/12] target-arm: A64: add support for BR, BLR and RET insns

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Implement BR, BLR and RET. This is all of the 'unconditional
branch (register)' instruction category except for ERET
and DPRS (which are system mode only).

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: reimplemented on top of new decoder structure]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 8535803..48281ff 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -365,10 +365,49 @@ static void disas_exc(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Unconditional branch (register) */
+/* C3.2.7 Unconditional branch (register)
+ *  31           25 24   21 20   16 15   10 9    5 4     0
+ * +---------------+-------+-------+-------+------+-------+
+ * | 1 1 0 1 0 1 1 |  opc  |  op2  |  op3  |  Rn  |  op4  |
+ * +---------------+-------+-------+-------+------+-------+
+ */
 static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int opc, op2, op3, rn, op4;
+
+    opc = extract32(insn, 21, 4);
+    op2 = extract32(insn, 16, 5);
+    op3 = extract32(insn, 10, 6);
+    rn = extract32(insn, 5, 5);
+    op4 = extract32(insn, 0, 5);
+
+    if (op4 != 0x0 || op3 != 0x0 || op2 != 0x1f) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    switch (opc) {
+    case 0: /* BR */
+    case 2: /* RET */
+        break;
+    case 1: /* BLR */
+        tcg_gen_movi_i64(cpu_reg(s, 30), s->pc);
+        break;
+    case 4: /* ERET */
+    case 5: /* DRPS */
+        if (rn != 0x1f) {
+            unallocated_encoding(s);
+        } else {
+            unsupported_encoding(s, insn);
+        }
+        return;
+    default:
+        unallocated_encoding(s);
+        return;
+    }
+
+    tcg_gen_mov_i64(cpu_pc, cpu_reg(s, rn));
+    s->is_jmp = DISAS_JUMP;
 }
 
 /* C3.2 Branches, exception generating and system instructions */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 09/12] target-arm: A64: add support for BR, BLR and RET insns

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> Implement BR, BLR and RET. This is all of the 'unconditional
> branch (register)' instruction category except for ERET
> and DPRS (which are system mode only).
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> [claudio: reimplemented on top of new decoder structure]
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c |   43 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

[Qemu-devel] [PATCH 10/12] target-arm: A64: add support for conditional branches

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds emulation for the conditional branch (b.cond) instruction.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder structure,
          reused arm infrastructure for checking the flags]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   29 +++++++++++++++++++++++++++--
 target-arm/translate.c     |   14 +++++++++-----
 target-arm/translate.h     |    2 ++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 48281ff..3c0748d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -221,10 +221,35 @@ static void disas_test_b_imm(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Conditional branch (immediate) */
+/* C3.2.2 / C5.6.19 Conditional branch (immediate)
+ *  31           25  24  23                  5   4  3    0
+ * +---------------+----+---------------------+----+------+
+ * | 0 1 0 1 0 1 0 | o1 |         imm19       | o0 | cond |
+ * +---------------+----+---------------------+----+------+
+ */
 static void disas_cond_b_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int cond;
+    uint64_t addr;
+
+    if ((insn & (1 << 4)) || (insn & (1 << 24))) {
+        unallocated_encoding(s);
+        return;
+    }
+    addr = s->pc + sextract32(insn, 5, 19) * 4 - 4;
+    cond = extract32(insn, 0, 4);
+
+    if (cond < 0x0e) {
+        /* genuinely conditional branches */
+        int label_nomatch = gen_new_label();
+        arm_gen_test_cc(cond ^ 1, label_nomatch);
+        gen_goto_tb(s, 0, addr);
+        gen_set_label(label_nomatch);
+        gen_goto_tb(s, 1, s->pc);
+    } else {
+        /* 0xe and 0xf are both "always" conditions */
+        gen_goto_tb(s, 0, addr);
+    }
 }
 
 /* C5.6.68 HINT */
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 553bded..9e2d1eb 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -671,7 +671,11 @@ static void gen_thumb2_parallel_addsub(int op1, int op2, TCGv_i32 a, TCGv_i32 b)
 }
 #undef PAS_OP
 
-static void gen_test_cc(int cc, int label)
+/*
+ * generate a conditional branch based on ARM condition code cc.
+ * This is common between ARM and Aarch64 targets.
+ */
+void arm_gen_test_cc(int cc, int label)
 {
     TCGv_i32 tmp;
     int inv;
@@ -6903,7 +6907,7 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
         /* if not always execute, we generate a conditional jump to
            next instruction */
         s->condlabel = gen_new_label();
-        gen_test_cc(cond ^ 1, s->condlabel);
+        arm_gen_test_cc(cond ^ 1, s->condlabel);
         s->condjmp = 1;
     }
     if ((insn & 0x0f900000) == 0x03000000) {
@@ -8910,7 +8914,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
                 op = (insn >> 22) & 0xf;
                 /* Generate a conditional jump to next instruction.  */
                 s->condlabel = gen_new_label();
-                gen_test_cc(op ^ 1, s->condlabel);
+                arm_gen_test_cc(op ^ 1, s->condlabel);
                 s->condjmp = 1;
 
                 /* offset[11:1] = insn[10:0] */
@@ -9267,7 +9271,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         cond = s->condexec_cond;
         if (cond != 0x0e) {     /* Skip conditional when condition is AL. */
           s->condlabel = gen_new_label();
-          gen_test_cc(cond ^ 1, s->condlabel);
+          arm_gen_test_cc(cond ^ 1, s->condlabel);
           s->condjmp = 1;
         }
     }
@@ -9940,7 +9944,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s)
         }
         /* generate a conditional jump to next instruction */
         s->condlabel = gen_new_label();
-        gen_test_cc(cond ^ 1, s->condlabel);
+        arm_gen_test_cc(cond ^ 1, s->condlabel);
         s->condjmp = 1;
 
         /* jump to the offset */
diff --git a/target-arm/translate.h b/target-arm/translate.h
index 23a45da..a6f6b3e 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -65,4 +65,6 @@ static inline void gen_a64_set_pc_im(uint64_t val)
 }
 #endif
 
+void arm_gen_test_cc(int cc, int label);
+
 #endif /* TARGET_ARM_TRANSLATE_H */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 10/12] target-arm: A64: add support for conditional branches

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> +    if (cond < 0x0e) {
> +        /* genuinely conditional branches */
> +        int label_nomatch = gen_new_label();
> +        arm_gen_test_cc(cond ^ 1, label_nomatch);
> +        gen_goto_tb(s, 0, addr);
> +        gen_set_label(label_nomatch);
> +        gen_goto_tb(s, 1, s->pc);

Why swap the condition with "^ 1"?  You can just as easily swap the goto_tb.  I
realize the A32 translator does it this way, but need we continue?

Otherwise,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 10/12] target-arm: A64: add support for conditional branches

[Peter Maydell]

On 4 December 2013 00:03, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2013 10:51 AM, Peter Maydell wrote:
>> +    if (cond < 0x0e) {
>> +        /* genuinely conditional branches */
>> +        int label_nomatch = gen_new_label();
>> +        arm_gen_test_cc(cond ^ 1, label_nomatch);
>> +        gen_goto_tb(s, 0, addr);
>> +        gen_set_label(label_nomatch);
>> +        gen_goto_tb(s, 1, s->pc);
>
> Why swap the condition with "^ 1"?  You can just as easily swap the goto_tb.  I
> realize the A32 translator does it this way, but need we continue?

Good point; will change.

thanks
-- PMM

[Qemu-devel] [PATCH 11/12] target-arm: A64: add support for 'test and branch' imm

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds emulation for the test and branch insns,
TBZ and TBNZ.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio:
  adapted for new decoder
  always compare with 0
  remove a TCG temporary
]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3c0748d..bcb59db 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -215,10 +215,36 @@ static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Test & branch (immediate) */
+/* C3.2.5 Test & branch (immediate)
+ *   31  30         25  24  23   19 18          5 4    0
+ * +----+-------------+----+-------+-------------+------+
+ * | b5 | 0 1 1 0 1 1 | op |  b40  |    imm14    |  Rt  |
+ * +----+-------------+----+-------+-------------+------+
+ */
 static void disas_test_b_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int bit_pos, op, rt;
+    uint64_t addr;
+    int label_nomatch;
+    TCGv_i64 tcg_cmp;
+
+    bit_pos = (extract32(insn, 31, 1) << 5) | extract32(insn, 19, 5);
+    op = extract32(insn, 24, 1);
+    addr = s->pc + sextract32(insn, 5, 14) * 4 - 4;
+    rt = extract32(insn, 0, 5);
+
+    tcg_cmp = tcg_temp_new_i64();
+    tcg_gen_andi_i64(tcg_cmp, cpu_reg(s, rt), (1ULL << bit_pos));
+    label_nomatch = gen_new_label();
+    if (op) { /* TBNZ */
+        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
+    } else { /* TBZ */
+        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
+    }
+    tcg_temp_free_i64(tcg_cmp);
+    gen_goto_tb(s, 0, addr);
+    gen_set_label(label_nomatch);
+    gen_goto_tb(s, 1, s->pc);
 }
 
 /* C3.2.2 / C5.6.19 Conditional branch (immediate)
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 11/12] target-arm: A64: add support for 'test and branch' imm

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> +    label_nomatch = gen_new_label();
> +    if (op) { /* TBNZ */
> +        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
> +    } else { /* TBZ */
> +        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
> +    }
> +    tcg_temp_free_i64(tcg_cmp);
> +    gen_goto_tb(s, 0, addr);
> +    gen_set_label(label_nomatch);
> +    gen_goto_tb(s, 1, s->pc);

Similar to B.cond, I think it would be clearer to write positive tests than
negative tests.  Therefore TBNZ -> NE; TBZ -> EQ, and swap the two goto_tb.

Further, you can avoid some repetition of the brcondi with (?:).

Otherwise,

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 11/12] target-arm: A64: add support for 'test and branch' imm

[Peter Maydell]

On 4 December 2013 00:07, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2013 10:51 AM, Peter Maydell wrote:
>> +    label_nomatch = gen_new_label();
>> +    if (op) { /* TBNZ */
>> +        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
>> +    } else { /* TBZ */
>> +        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
>> +    }
>> +    tcg_temp_free_i64(tcg_cmp);
>> +    gen_goto_tb(s, 0, addr);
>> +    gen_set_label(label_nomatch);
>> +    gen_goto_tb(s, 1, s->pc);
>
> Similar to B.cond, I think it would be clearer to write positive tests than
> negative tests.  Therefore TBNZ -> NE; TBZ -> EQ, and swap the two goto_tb.
>
> Further, you can avoid some repetition of the brcondi with (?:).

OK, will fix.

thanks
-- PMM

[Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds emulation for the compare and branch insns,
CBZ and CBNZ.

Signed-off-by: Alexander Graf <agraf@suse.de>
[claudio: adapted to new decoder,
          compare with immediate 0,
	  introduce read_cpu_reg to get the 0 extension on (!sf)]
Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c |   45 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index bcb59db..37dc462 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -184,6 +184,18 @@ static TCGv_i64 cpu_reg(DisasContext *s, int reg)
     }
 }
 
+/* read a cpu register in 32bit/64bit mode to dst */
+static void read_cpu_reg(DisasContext *s, TCGv_i64 dst, int reg, int sf)
+{
+    if (reg == 31) {
+        tcg_gen_movi_i64(dst, 0);
+    } else if (sf) {
+        tcg_gen_mov_i64(dst, cpu_X[reg]);
+    } else { /* (!sf) */
+        tcg_gen_ext32u_i64(dst, cpu_X[reg]);
+    }
+}
+
 /*
  * the instruction disassembly implemented here matches
  * the instruction encoding classifications in chapter 3 (C3)
@@ -209,10 +221,39 @@ static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
     gen_goto_tb(s, 0, addr);
 }
 
-/* Compare & branch (immediate) */
+/* C3.2.1 Compare & branch (immediate)
+ *   31  30         25  24  23                  5 4      0
+ * +----+-------------+----+---------------------+--------+
+ * | sf | 0 1 1 0 1 0 | op |         imm19       |   Rt   |
+ * +----+-------------+----+---------------------+--------+
+ */
 static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, op, rt;
+    uint64_t addr;
+    int label_nomatch;
+    TCGv_i64 tcg_cmp;
+
+    sf = extract32(insn, 31, 1);
+    op = extract32(insn, 24, 1);
+    rt = extract32(insn, 0, 5);
+    addr = s->pc + sextract32(insn, 5, 19) * 4 - 4;
+
+    tcg_cmp = tcg_temp_new_i64();
+    read_cpu_reg(s, tcg_cmp, rt, sf);
+    label_nomatch = gen_new_label();
+
+    if (op) { /* CBNZ */
+        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
+    } else { /* CBZ */
+        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
+    }
+
+    tcg_temp_free_i64(tcg_cmp);
+
+    gen_goto_tb(s, 0, addr);
+    gen_set_label(label_nomatch);
+    gen_goto_tb(s, 1, s->pc);
 }
 
 /* C3.2.5 Test & branch (immediate)
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Richard Henderson]

On 12/04/2013 10:51 AM, Peter Maydell wrote:
> @@ -184,6 +184,18 @@ static TCGv_i64 cpu_reg(DisasContext *s, int reg)
>      }
>  }
>  
> +/* read a cpu register in 32bit/64bit mode to dst */
> +static void read_cpu_reg(DisasContext *s, TCGv_i64 dst, int reg, int sf)
> +{
> +    if (reg == 31) {
> +        tcg_gen_movi_i64(dst, 0);
> +    } else if (sf) {
> +        tcg_gen_mov_i64(dst, cpu_X[reg]);
> +    } else { /* (!sf) */
> +        tcg_gen_ext32u_i64(dst, cpu_X[reg]);
> +    }
> +}

I think this should be more like cpu_reg and return a TCGv instead of force a
copy into a newly generated temporary.  You've got a pool of auto-freeing
temporaries, after all.

> +    if (op) { /* CBNZ */
> +        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
> +    } else { /* CBZ */
> +        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
> +    }

Similar comments to TBNZ/TBZ apply here.


r~

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Peter Maydell]

On 4 December 2013 00:10, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2013 10:51 AM, Peter Maydell wrote:
>> @@ -184,6 +184,18 @@ static TCGv_i64 cpu_reg(DisasContext *s, int reg)
>>      }
>>  }
>>
>> +/* read a cpu register in 32bit/64bit mode to dst */
>> +static void read_cpu_reg(DisasContext *s, TCGv_i64 dst, int reg, int sf)
>> +{
>> +    if (reg == 31) {
>> +        tcg_gen_movi_i64(dst, 0);
>> +    } else if (sf) {
>> +        tcg_gen_mov_i64(dst, cpu_X[reg]);
>> +    } else { /* (!sf) */
>> +        tcg_gen_ext32u_i64(dst, cpu_X[reg]);
>> +    }
>> +}
>
> I think this should be more like cpu_reg and return a TCGv instead of force a
> copy into a newly generated temporary.  You've got a pool of auto-freeing
> temporaries, after all.

You're right that we can just make this function return the TCGv
temp rather than making the caller pass one in. Are you suggesting
the 64-bit case should return cpu_X[reg] rather than a copy of it,
though? I think it would be pretty hard to reason about if you had to
remember that sometimes you got a trashable copy and sometimes
the real register.

(I'm still a bit on the fence about that pool of auto-freeing temporaries.
Manual temp management is certainly a fertile source of decoder bugs,
but in the longer term we might want to push the functionality down into
the common code rather than having an ad-hoc thing in one decoder.)

>> +    if (op) { /* CBNZ */
>> +        tcg_gen_brcondi_i64(TCG_COND_EQ, tcg_cmp, 0, label_nomatch);
>> +    } else { /* CBZ */
>> +        tcg_gen_brcondi_i64(TCG_COND_NE, tcg_cmp, 0, label_nomatch);
>> +    }
>
> Similar comments to TBNZ/TBZ apply here.

Agreed.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Richard Henderson]

On 12/04/2013 01:32 PM, Peter Maydell wrote:
> You're right that we can just make this function return the TCGv
> temp rather than making the caller pass one in. Are you suggesting
> the 64-bit case should return cpu_X[reg] rather than a copy of it,
> though? I think it would be pretty hard to reason about if you had to
> remember that sometimes you got a trashable copy and sometimes
> the real register.

I think that the normal case is to write to the output in one step, and thus
having an input overlap an output isn't your problem but tcg's.  I would think
the case of multi-step output to be fairly rare, and easily solvable by always
allocating an output temporary.

Am I off-base on the multi-output thing?  Modulo flags setting, of course, but
I think that special case ought to be relatively solvable.

> (I'm still a bit on the fence about that pool of auto-freeing temporaries.
> Manual temp management is certainly a fertile source of decoder bugs,
> but in the longer term we might want to push the functionality down into
> the common code rather than having an ad-hoc thing in one decoder.)

See also Sparc and S390.  ;-)

I'm open to suggestions for doing this generically, but I've no great ideas
off-hand.


r~

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Peter Maydell]

On 4 December 2013 00:48, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2013 01:32 PM, Peter Maydell wrote:
>> You're right that we can just make this function return the TCGv
>> temp rather than making the caller pass one in. Are you suggesting
>> the 64-bit case should return cpu_X[reg] rather than a copy of it,
>> though? I think it would be pretty hard to reason about if you had to
>> remember that sometimes you got a trashable copy and sometimes
>> the real register.
>
> I think that the normal case is to write to the output in one step, and thus
> having an input overlap an output isn't your problem but tcg's.  I would think
> the case of multi-step output to be fairly rare, and easily solvable by always
> allocating an output temporary.

Does a copy to temp actually cost us anything, though? I was
under the impression the TCG optimizer would basically throw
it away again.

I suppose you could just treat the semantics of it as "assume you
can't trash this TCGv" in all cases and rely on the auto-free.

> Am I off-base on the multi-output thing?  Modulo flags setting, of course, but
> I think that special case ought to be relatively solvable.
>
>> (I'm still a bit on the fence about that pool of auto-freeing temporaries.
>> Manual temp management is certainly a fertile source of decoder bugs,
>> but in the longer term we might want to push the functionality down into
>> the common code rather than having an ad-hoc thing in one decoder.)
>
> See also Sparc and S390.  ;-)

Ah, I hadn't noticed we were already doing this in other frontends.

-- PMM

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Peter Maydell]

On 4 December 2013 00:48, Richard Henderson <rth@twiddle.net> wrote:
> On 12/04/2013 01:32 PM, Peter Maydell wrote:
>> You're right that we can just make this function return the TCGv
>> temp rather than making the caller pass one in. Are you suggesting
>> the 64-bit case should return cpu_X[reg] rather than a copy of it,
>> though? I think it would be pretty hard to reason about if you had to
>> remember that sometimes you got a trashable copy and sometimes
>> the real register.
>
> I think that the normal case is to write to the output in one step, and thus
> having an input overlap an output isn't your problem but tcg's.  I would think
> the case of multi-step output to be fairly rare, and easily solvable by always
> allocating an output temporary.

So I had a look at this, and it seems to make code like this (from
the handling for logic ops with shifted registers):

    tcg_rm = read_cpu_reg(s, rm, sf);

    if (shift_amount) {
        shift_reg_imm(tcg_rm, tcg_rm, sf,
                      shift_type, shift_amount);
    }

    if (invert) {
        tcg_gen_not_i64(tcg_rm, tcg_rm);
        /* we zero extend later on (!sf) */
    }

a bit awkward if the value returned from read_cpu_reg() isn't a
trashable copy, because of the sequence of "maybe we need
to change the value like this" conditionals. The code basically
needs to copy the return value from read_cpu_reg() into its
own temporary immediately. There's a similar thing in the
conditional-select code where the else-case's value might be
inverted and might be incremented and might be neither.

I think that kind of thing tips the balance in favour of having
read_cpu_reg() always return a trashable temporary.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 12/12] target-arm: A64: add support for compare and branch imm

[Richard Henderson]

On 12/05/2013 06:02 AM, Peter Maydell wrote:
> I think that kind of thing tips the balance in favour of having
> read_cpu_reg() always return a trashable temporary.

Fair enough.


r~