[Qemu-devel] [PATCH 0/9] target-arm: A64 decoder set 3: loads, stores, misc integer

[Qemu-devel] [PATCH 0/9] target-arm: A64 decoder set 3: loads, stores, misc integer

[Peter Maydell]

Welcome back to the latest installment in this thrilling
series of A64 decoder patches. In this episode, we have:

 * support for most kinds of load and store (still to come are
   load-literal, the exclusives and the SIMD structure ld/st)
 * addition and subtraction (but not adc/sbc)
 * some other minor integer instructions

(Tune in next time for set 4, which is likely to contain
FMOV, the user-space permitted parts of MSR/MRS, load
literal, and exclusives; since that's sufficient to get
through libc startup and run simple binaries we'll also
throw in the 'actually enable the target' patches.)

Git tree (with v7-cpu-host/mach-virt, v8 kvm control,
 and A64 set one & two all underneath these patches):
 git://git.linaro.org/people/peter.maydell/qemu-arm.git a64-third-set
web UI:
 https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/a64-second-set

thanks
-- PMM

Alex Bennée (7):
  target-arm: A64: add support for stp (store pair)
  target-arm: A64: add support for ldp (load pair)
  target-arm: A64: add support for ld/st unsigned imm
  target-arm: A64: add support for ld/st with reg offset
  target-arm: A64: add support for ld/st with index
  target-arm: A64: add support for add, addi, sub, subi
  target-arm: A64: add support for move wide instructions

Alexander Graf (2):
  target-arm: A64: add support for 3 src data proc insns
  target-arm: A64: implement SVC, BRK

 target-arm/helper-a64.c    |   14 +
 target-arm/helper-a64.h    |    2 +
 target-arm/translate-a64.c | 1255 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 1251 insertions(+), 20 deletions(-)

-- 
1.8.5

[Qemu-devel] [PATCH 1/9] target-arm: A64: add support for stp (store pair)

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

This patch support the basic store-pair instructions and includes
the generic store helper functions:

  * do_gpr_st()
  * do_fp_st()
  * gen_check_sp_alignment()

The last function gen_check_sp_alignment() is a NULL op currently but
put in place to make it easy to add SP alignment checking later.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 220 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 218 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0a76130..56f2d6b 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -99,6 +99,16 @@ void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
     cpu_fprintf(f, "\n");
 }
 
+
+static int get_mem_index(DisasContext *s)
+{
+#ifdef CONFIG_USER_ONLY
+    return 1;
+#else
+    return s->user;
+#endif
+}
+
 void gen_a64_set_pc_im(uint64_t val)
 {
     tcg_gen_movi_i64(cpu_pc, val);
@@ -278,6 +288,94 @@ static inline void gen_logic_CC(int sf, TCGv_i64 result)
 }
 
 /*
+ * Load/Store generators
+ */
+
+/*
+  Store from GPR register to memory
+*/
+static void do_gpr_st(DisasContext *s, TCGv_i64 source,
+                      TCGv_i64 tcg_addr, int size)
+{
+    switch (size) {
+    case 0:
+        tcg_gen_qemu_st8(source, tcg_addr, get_mem_index(s));
+        break;
+    case 1:
+        tcg_gen_qemu_st16(source, tcg_addr, get_mem_index(s));
+        break;
+    case 2:
+        tcg_gen_qemu_st32(source, tcg_addr, get_mem_index(s));
+        break;
+    case 3:
+        tcg_gen_qemu_st64(source, tcg_addr, get_mem_index(s));
+        break;
+    default:
+        /* Bad size */
+        g_assert(false);
+        break;
+    }
+}
+
+/*
+ * Store from FP register to memory
+ */
+static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size)
+{
+    /* This writes the bottom N bits of a 128 bit wide vector to memory */
+    int freg_offs = offsetof(CPUARMState, vfp.regs[srcidx * 2]);
+    TCGv_i64 tmp = tcg_temp_new_i64();
+
+    switch (size) {
+    case 0:
+        tcg_gen_ld8u_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st8(tmp, tcg_addr, get_mem_index(s));
+        break;
+    case 1:
+        tcg_gen_ld16u_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st16(tmp, tcg_addr, get_mem_index(s));
+        break;
+    case 2:
+        tcg_gen_ld32u_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st32(tmp, tcg_addr, get_mem_index(s));
+        break;
+    case 3:
+        tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s));
+        break;
+    case 4:
+    {
+        TCGv_i64 tcg_hiaddr = tcg_temp_new_i64();
+        tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s));
+        tcg_gen_ld_i64(tmp, cpu_env, freg_offs + sizeof(float64));
+        tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
+        tcg_gen_qemu_st64(tmp, tcg_hiaddr, get_mem_index(s));
+        tcg_temp_free_i64(tcg_hiaddr);
+        break;
+    }
+    default:
+        g_assert(false);
+        break;
+    }
+
+    tcg_temp_free_i64(tmp);
+}
+
+static inline void gen_check_sp_alignment(DisasContext *s)
+{
+    /* The AArch64 architecture mandates that (if enabled via PSTATE
+     * or SCTLR bits) there is a check that SP is 16-aligned on every
+     * SP-relative load or store (with an exception generated if it is not).
+     * In line with general QEMU practice regarding misaligned accesses,
+     * we omit these checks for the sake of guest program performance.
+     * This function is provided as a hook so we can more easily add these
+     * checks in future (possibly as a "favour catching guest program bugs
+     * over speed" user selectable option).
+     */
+}
+
+/*
  * the instruction disassembly implemented here matches
  * the instruction encoding classifications in chapter 3 (C3)
  * of the ARM Architecture Reference Manual (DDI0487A_a)
@@ -620,10 +718,128 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Load/store pair (all forms) */
+/*
+ * C5.6.177 STP (Store Pair - non vector)
+ * C6.3.284 STP (Store Pair of SIMD&FP)
+ *
+ *  31 30 29   27 26  25   23 22 21   15 14   10 9    5 4    0
+ * +-----+-------+---+-------+--+-----------------------------+
+ * | opc | 1 0 1 | V | index | 0|  imm7 |  Rt2  |  Rn  | Rt   |
+ * +-----+-------+---+-------+--+-------+-------+------+------+
+ *
+ * opc: STP           00 -> 32 bit, 10 -> 64 bit
+ *      STP (SIMD&FP) 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit
+ * idx: 001 -> post-index, 011 -> pre-index, 010 -> signed off
+ *
+ * Rt, Rt2 = GPR or SIMD registers to be stored
+ * Rn = general purpose register containing address
+ * imm7 = signed offset (multiple of 4 or 8 depending on size)
+ */
+static void handle_stp(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int rt2 = extract32(insn, 10, 5);
+    int64_t offset = sextract32(insn, 15, 7);
+    int type = extract32(insn, 23, 2);
+    bool is_vector = extract32(insn, 26, 1);
+    int opc = extract32(insn, 30, 2);
+
+    TCGv_i64 tcg_rt = cpu_reg(s, rt);
+    TCGv_i64 tcg_rt2 = cpu_reg(s, rt2);
+    TCGv_i64 tcg_addr; /* calculated address */
+    bool postindex = false;
+    bool wback = false;
+    int size;
+
+    if (is_vector) {
+        if (opc == 3) {
+            unallocated_encoding(s);
+            return;
+        }
+        size = 2 + opc;
+    } else {
+        size = 2 + extract32(opc, 1, 1);
+        if (opc & 1) {
+            unallocated_encoding(s);
+            return;
+        }
+    }
+
+    switch (type) {
+    case 1: /* STP (post-index) */
+        postindex = true;
+        wback = true;
+        break;
+    case 2: /* STP (signed offset), rn not updated */
+        postindex = false;
+        break;
+    case 3: /* STP (pre-index) */
+        postindex = false;
+        wback = true;
+        break;
+    default: /* Failed decoder tree? */
+        unallocated_encoding(s);
+        break;
+    }
+
+    offset <<= size;
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+
+    tcg_addr = tcg_temp_new_i64();
+    tcg_gen_mov_i64(tcg_addr, cpu_reg_sp(s, rn));
+
+    if (!postindex) {
+        tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+    }
+
+    if (is_vector) {
+        do_fp_st(s, rt, tcg_addr, size);
+    } else {
+        do_gpr_st(s, tcg_rt, tcg_addr, size);
+    }
+    tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
+    if (is_vector) {
+        do_fp_st(s, rt2, tcg_addr, size);
+    } else {
+        do_gpr_st(s, tcg_rt2, tcg_addr, size);
+    }
+
+    if (wback) {
+        if (postindex) {
+            tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size));
+        } else {
+            tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
+        }
+        tcg_gen_mov_i64(cpu_reg_sp(s, rn), tcg_addr);
+    }
+
+    tcg_temp_free_i64(tcg_addr);
+}
+
+
+/* C2.2.3 Load/store pair (all non vector forms)
+ *
+ *  31 30 29   27 26  25   23  22 21   15 14   10 9    5 4    0
+ * +-----+-------+---+-------+---+-----------------------------+
+ * | opc | 1 0 1 | V | index | L |  imm7 |  Rt2  |  Rn  | Rt   |
+ * +-----+-------+---+-------+---+-------+-------+------+------+
+ *
+ * L = 0 -> Store, 1 -> Load
+ * V = 0 -> non-vector, 1 -> vector (SIMD & FP)
+ */
 static void disas_ldst_pair(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int is_load = extract32(insn, 22, 1);
+
+    if (is_load) {
+        unsupported_encoding(s, insn);
+    } else {
+        handle_stp(s, insn);
+    }
 }
 
 /* Load/store register (all forms) */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 1/9] target-arm: A64: add support for stp (store pair)

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> +static void do_gpr_st(DisasContext *s, TCGv_i64 source,
> +                      TCGv_i64 tcg_addr, int size)
> +{
> +    switch (size) {
> +    case 0:
> +        tcg_gen_qemu_st8(source, tcg_addr, get_mem_index(s));
> +        break;
> +    case 1:
> +        tcg_gen_qemu_st16(source, tcg_addr, get_mem_index(s));
> +        break;
> +    case 2:
> +        tcg_gen_qemu_st32(source, tcg_addr, get_mem_index(s));
> +        break;
> +    case 3:
> +        tcg_gen_qemu_st64(source, tcg_addr, get_mem_index(s));
> +        break;

Please use the new ldst entry points.  In this case,

  tcg_gen_qemu_st_i64(source, tcg_addr, get_mem_index(s), MO_TE + size)

since size is already log2.

> +static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size)
> +{
> +    /* This writes the bottom N bits of a 128 bit wide vector to memory */
> +    int freg_offs = offsetof(CPUARMState, vfp.regs[srcidx * 2]);
> +    TCGv_i64 tmp = tcg_temp_new_i64();
> +
> +    switch (size) {
> +    case 0:
> +        tcg_gen_ld8u_i64(tmp, cpu_env, freg_offs);
> +        tcg_gen_qemu_st8(tmp, tcg_addr, get_mem_index(s));
> +        break;
> +    case 1:
> +        tcg_gen_ld16u_i64(tmp, cpu_env, freg_offs);
> +        tcg_gen_qemu_st16(tmp, tcg_addr, get_mem_index(s));
> +        break;
> +    case 2:
> +        tcg_gen_ld32u_i64(tmp, cpu_env, freg_offs);
> +        tcg_gen_qemu_st32(tmp, tcg_addr, get_mem_index(s));
> +        break;
> +    case 3:
> +        tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
> +        tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s));
> +        break;
> +    case 4:
> +    {
> +        TCGv_i64 tcg_hiaddr = tcg_temp_new_i64();
> +        tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
> +        tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s));
> +        tcg_gen_ld_i64(tmp, cpu_env, freg_offs + sizeof(float64));
> +        tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
> +        tcg_gen_qemu_st64(tmp, tcg_hiaddr, get_mem_index(s));
> +        tcg_temp_free_i64(tcg_hiaddr);
> +        break;
> +    }

You'll certainly have to continue to special-case the 128-bit store, but the
other sizes need not be.

> +/*
> + * C5.6.177 STP (Store Pair - non vector)
> + * C6.3.284 STP (Store Pair of SIMD&FP)
> + *
> + *  31 30 29   27 26  25   23 22 21   15 14   10 9    5 4    0
> + * +-----+-------+---+-------+--+-----------------------------+
> + * | opc | 1 0 1 | V | index | 0|  imm7 |  Rt2  |  Rn  | Rt   |
> + * +-----+-------+---+-------+--+-------+-------+------+------+
> + *
> + * opc: STP           00 -> 32 bit, 10 -> 64 bit
> + *      STP (SIMD&FP) 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit
> + * idx: 001 -> post-index, 011 -> pre-index, 010 -> signed off
> + *
> + * Rt, Rt2 = GPR or SIMD registers to be stored
> + * Rn = general purpose register containing address
> + * imm7 = signed offset (multiple of 4 or 8 depending on size)
> + */
> +static void handle_stp(DisasContext *s, uint32_t insn)
> +{
> +    int rt = extract32(insn, 0, 5);
> +    int rn = extract32(insn, 5, 5);
> +    int rt2 = extract32(insn, 10, 5);
> +    int64_t offset = sextract32(insn, 15, 7);
> +    int type = extract32(insn, 23, 2);
> +    bool is_vector = extract32(insn, 26, 1);
> +    int opc = extract32(insn, 30, 2);
> +
> +    TCGv_i64 tcg_rt = cpu_reg(s, rt);
> +    TCGv_i64 tcg_rt2 = cpu_reg(s, rt2);

If you're going to combine vector and non-vector STP, then I think it would be
cleaner if you didn't load these registers so early, when we're not even sure
if we're talking about general registers.

> +    if (wback) {
> +        if (postindex) {
> +            tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size));
> +        } else {
> +            tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
> +        }

Perhaps better as

    tcg_gen_addi_i64(tcg_addr, tcg_addr,
                     (postindex ? offset : 0) - (1 << size));


?


r~

Re: [Qemu-devel] [PATCH 1/9] target-arm: A64: add support for stp (store pair)

[Alex Bennée]

rth@twiddle.net writes:

> On 12/09/2013 10:12 AM, Peter Maydell wrote:
<snip>
>> +    if (wback) {
>> +        if (postindex) {
>> +            tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size));
>> +        } else {
>> +            tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
>> +        }
>
> Perhaps better as
>
>     tcg_gen_addi_i64(tcg_addr, tcg_addr,
>                      (postindex ? offset : 0) - (1 << size));

I'm not so sure it's easier to follow despite my general proclivity for
ternary operator.

>
>
> ?
>
>
> r~

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro

[Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

Implement the LDP instruciton. This includes some generic helper
functions for doing loads:

  * do_gpr_ld()
  * do_fp_ld()

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 197 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 196 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 56f2d6b..600cf63 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -318,6 +318,49 @@ static void do_gpr_st(DisasContext *s, TCGv_i64 source,
 }
 
 /*
+ * Load from memory to GPR register
+ */
+static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr,
+                      int size, bool is_signed, bool extend)
+{
+    switch (size) {
+    case 0:
+        if (is_signed) {
+            tcg_gen_qemu_ld8s(dest, tcg_addr, get_mem_index(s));
+        } else {
+            tcg_gen_qemu_ld8u(dest, tcg_addr, get_mem_index(s));
+        }
+        break;
+    case 1:
+        if (is_signed) {
+            tcg_gen_qemu_ld16s(dest, tcg_addr, get_mem_index(s));
+        } else {
+            tcg_gen_qemu_ld16u(dest, tcg_addr, get_mem_index(s));
+        }
+        break;
+    case 2:
+        if (is_signed) {
+            tcg_gen_qemu_ld32s(dest, tcg_addr, get_mem_index(s));
+        } else {
+            tcg_gen_qemu_ld32u(dest, tcg_addr, get_mem_index(s));
+        }
+        break;
+    case 3:
+        tcg_gen_qemu_ld64(dest, tcg_addr, get_mem_index(s));
+        break;
+    default:
+        /* Bad size */
+        g_assert(false);
+        break;
+    }
+
+    if (extend && is_signed) {
+        g_assert(size < 3);
+        tcg_gen_ext32u_i64(dest, dest);
+    }
+}
+
+/*
  * Store from FP register to memory
  */
 static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size)
@@ -362,6 +405,57 @@ static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size)
     tcg_temp_free_i64(tmp);
 }
 
+/* Load from memory to FP register */
+static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size)
+{
+    /* This always zero-extends and writes to a full 128 bit wide vector */
+    int freg_offs = offsetof(CPUARMState, vfp.regs[destidx * 2]);
+    TCGv_i64 tmplo = tcg_temp_new_i64();
+    TCGv_i64 tmphi;
+
+    switch (size) {
+    case 0:
+        tcg_gen_qemu_ld8u(tmplo, tcg_addr, get_mem_index(s));
+        break;
+    case 1:
+        tcg_gen_qemu_ld16u(tmplo, tcg_addr, get_mem_index(s));
+        break;
+    case 2:
+        tcg_gen_qemu_ld32u(tmplo, tcg_addr, get_mem_index(s));
+        break;
+    case 3:
+    case 4:
+        tcg_gen_qemu_ld64(tmplo, tcg_addr, get_mem_index(s));
+        break;
+    default:
+        g_assert(false);
+        break;
+    }
+
+    switch (size) {
+    case 4:
+    {
+        TCGv_i64 tcg_hiaddr;
+
+        tmphi = tcg_temp_new_i64();
+        tcg_hiaddr = tcg_temp_new_i64();
+        tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
+        tcg_gen_qemu_ld64(tmphi, tcg_hiaddr, get_mem_index(s));
+        tcg_temp_free_i64(tcg_hiaddr);
+        break;
+    }
+    default:
+        tmphi = tcg_const_i64(0);
+        break;
+    }
+
+    tcg_gen_st_i64(tmplo, cpu_env, freg_offs);
+    tcg_gen_st_i64(tmphi, cpu_env, freg_offs + sizeof(float64));
+
+    tcg_temp_free_i64(tmplo);
+    tcg_temp_free_i64(tmphi);
+}
+
 static inline void gen_check_sp_alignment(DisasContext *s)
 {
     /* The AArch64 architecture mandates that (if enabled via PSTATE
@@ -719,6 +813,107 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
 }
 
 /*
+ * C5.6.81 LDP (Load Pair - non vector)
+ * C5.6.82 LDPSW (Load Pair Signed Word - non vector)
+ * C6.3.165 LDP (Load Pair of SIMD&FP)
+ *
+ *  31 30 29   27  26 25   23 22 21   15 14   10 9    5 4    0
+ * +-----+-------+---+-------+--+-----------------------------+
+ * | opc | 1 0 1 | V | index | 1|  imm7 |  Rt2  |  Rn  | Rt   |
+ * +-----+-------+---+-------+--+-------+-------+------+------+
+ *                             L
+ * opc: LDP           00 -> 32 bit, 10 -> 64 bit
+ *      LDPSW         01
+ *      LDP (SIMD&FP) 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit
+ * idx: 001 -> post-index, 011 -> pre-index, 010 -> signed off
+ *
+ * Rt, Rt2 = GPR or SIMD registers to be stored
+ * Rn = general purpose register containing address
+ * imm7 = signed offset (multiple of 4 or 8 depending on size)
+ */
+static void handle_ldp(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int rt2 = extract32(insn, 10, 5);
+    int64_t offset = sextract32(insn, 15, 7);
+    int idx = extract32(insn, 23, 3);
+    bool is_signed = false;
+    bool is_vector = extract32(insn, 26, 1);
+    int opc = extract32(insn, 30, 2);
+    int size;
+    bool postindex = true;
+    bool wback = false;
+
+    TCGv_i64 tcg_rt = cpu_reg(s, rt);
+    TCGv_i64 tcg_rt2 = cpu_reg(s, rt2);
+    TCGv_i64 tcg_addr;
+
+    if (opc == 3) {
+        unallocated_encoding(s);
+        return;
+    }
+    if (is_vector) {
+        size = 2 + opc;
+    } else {
+        is_signed = opc & 1;
+        size = 2 + extract32(opc, 1, 1);
+    }
+
+    switch (idx) {
+    case 1: /* post-index */
+        postindex = true;
+        wback = true;
+        break;
+    case 2: /* signed offset, rn not updated */
+        postindex = false;
+        break;
+    case 3: /* STP (pre-index) */
+        postindex = false;
+        wback = true;
+        break;
+    default: /* Failed decoder tree? */
+        unallocated_encoding(s);
+        break;
+    }
+
+    offset <<= size;
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+
+    tcg_addr = tcg_temp_new_i64();
+    tcg_gen_mov_i64(tcg_addr, cpu_reg_sp(s, rn));
+
+    if (!postindex) {
+        tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+    }
+
+    if (is_vector) {
+        do_fp_ld(s, rt, tcg_addr, size);
+    } else {
+        do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false);
+    }
+    tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
+    if (is_vector) {
+        do_fp_ld(s, rt2, tcg_addr, size);
+    } else {
+        do_gpr_ld(s, tcg_rt2, tcg_addr, size, is_signed, false);
+    }
+
+    if (wback) {
+        if (postindex) {
+            tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size));
+        } else {
+            tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
+        }
+        tcg_gen_mov_i64(cpu_reg_sp(s, rn), tcg_addr);
+    }
+    tcg_temp_free_i64(tcg_addr);
+}
+
+/*
  * C5.6.177 STP (Store Pair - non vector)
  * C6.3.284 STP (Store Pair of SIMD&FP)
  *
@@ -836,7 +1031,7 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
     int is_load = extract32(insn, 22, 1);
 
     if (is_load) {
-        unsupported_encoding(s, insn);
+        handle_ldp(s, insn);
     } else {
         handle_stp(s, insn);
     }
-- 
1.8.5

Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> +static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr,
> +                      int size, bool is_signed, bool extend)
> +{
> +    switch (size) {
> +    case 0:
> +        if (is_signed) {
> +            tcg_gen_qemu_ld8s(dest, tcg_addr, get_mem_index(s));
> +        } else {
> +            tcg_gen_qemu_ld8u(dest, tcg_addr, get_mem_index(s));
> +        }
> +        break;
> +    case 1:
> +        if (is_signed) {
> +            tcg_gen_qemu_ld16s(dest, tcg_addr, get_mem_index(s));
> +        } else {
> +            tcg_gen_qemu_ld16u(dest, tcg_addr, get_mem_index(s));
> +        }
> +        break;
> +    case 2:
> +        if (is_signed) {
> +            tcg_gen_qemu_ld32s(dest, tcg_addr, get_mem_index(s));
> +        } else {
> +            tcg_gen_qemu_ld32u(dest, tcg_addr, get_mem_index(s));
> +        }
> +        break;
> +    case 3:
> +        tcg_gen_qemu_ld64(dest, tcg_addr, get_mem_index(s));
> +        break;

Likewise, combine all of this with tcg_gen_qemu_ld_i64.

> +    if (extend && is_signed) {
> +        g_assert(size < 3);
> +        tcg_gen_ext32u_i64(dest, dest);
> +    }

Is it worth noticing in size==2 && !extend that is_signed can be forced false
to avoid the extra extension.

> +static void handle_ldp(DisasContext *s, uint32_t insn)
> +{
> +    int rt = extract32(insn, 0, 5);
> +    int rn = extract32(insn, 5, 5);
> +    int rt2 = extract32(insn, 10, 5);
> +    int64_t offset = sextract32(insn, 15, 7);
> +    int idx = extract32(insn, 23, 3);
> +    bool is_signed = false;
> +    bool is_vector = extract32(insn, 26, 1);
> +    int opc = extract32(insn, 30, 2);
> +    int size;
> +    bool postindex = true;
> +    bool wback = false;
> +
> +    TCGv_i64 tcg_rt = cpu_reg(s, rt);
> +    TCGv_i64 tcg_rt2 = cpu_reg(s, rt2);
> +    TCGv_i64 tcg_addr;
> +
> +    if (opc == 3) {
> +        unallocated_encoding(s);
> +        return;
> +    }
> +    if (is_vector) {
> +        size = 2 + opc;
> +    } else {
> +        is_signed = opc & 1;
> +        size = 2 + extract32(opc, 1, 1);
> +    }
> +
> +    switch (idx) {
> +    case 1: /* post-index */
> +        postindex = true;
> +        wback = true;
> +        break;
> +    case 2: /* signed offset, rn not updated */
> +        postindex = false;
> +        break;
> +    case 3: /* STP (pre-index) */
> +        postindex = false;
> +        wback = true;
> +        break;
> +    default: /* Failed decoder tree? */
> +        unallocated_encoding(s);
> +        break;
> +    }
> +
> +    offset <<= size;
> +
> +    if (rn == 31) {
> +        gen_check_sp_alignment(s);
> +    }
> +
> +    tcg_addr = tcg_temp_new_i64();
> +    tcg_gen_mov_i64(tcg_addr, cpu_reg_sp(s, rn));
> +
> +    if (!postindex) {
> +        tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
> +    }
> +
> +    if (is_vector) {
> +        do_fp_ld(s, rt, tcg_addr, size);
> +    } else {
> +        do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false);
> +    }
> +    tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
> +    if (is_vector) {
> +        do_fp_ld(s, rt2, tcg_addr, size);
> +    } else {
> +        do_gpr_ld(s, tcg_rt2, tcg_addr, size, is_signed, false);
> +    }
> +
> +    if (wback) {
> +        if (postindex) {
> +            tcg_gen_addi_i64(tcg_addr, tcg_addr, offset - (1 << size));
> +        } else {
> +            tcg_gen_subi_i64(tcg_addr, tcg_addr, 1 << size);
> +        }
> +        tcg_gen_mov_i64(cpu_reg_sp(s, rn), tcg_addr);
> +    }
> +    tcg_temp_free_i64(tcg_addr);
> +}

There's so much overlap with STP, I think these ought not be separate
functions.  Just merge the middle bit where the actual load/store happens.


r~

Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

[Alex Bennée]

rth@twiddle.net writes:

> On 12/09/2013 10:12 AM, Peter Maydell wrote:
>> +static void do_gpr_ld(DisasContext *s, TCGv_i64 dest, TCGv_i64 tcg_addr,
>> +                      int size, bool is_signed, bool extend)
>> +{
<snip>
>
> Likewise, combine all of this with tcg_gen_qemu_ld_i64.

Cool, that cleans up a bunch of logic.

>
>> +    if (extend && is_signed) {
>> +        g_assert(size < 3);
>> +        tcg_gen_ext32u_i64(dest, dest);
>> +    }
>
> Is it worth noticing in size==2 && !extend that is_signed can be forced false
> to avoid the extra extension.

Sorry I don't quite follow, the extension only occurs if it's an explict
extension into a 64 bit register. Or are you talking about avoiding
using the extension logic in the generic tcg_gen_qemu_ld_i64 code?

>
>> +static void handle_ldp(DisasContext *s, uint32_t insn)
>> +{
<snip>
>> +}
>
> There's so much overlap with STP, I think these ought not be separate
> functions.  Just merge the middle bit where the actual load/store happens.

Yeah that's a function of the way the patches were ported across
piecemeal as required for risu testing. I'll merge the two together.

There are other potential common parts that could be cleaned up later.
However the original ldst patches did get a little gnarly thanks to the
structure of the original decoder and I wanted to avoid things like imm
fields being optionally chopped apart depending on the decode.

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro

Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

[Richard Henderson]

On 12/10/2013 05:59 AM, Alex Bennée wrote:
>>> +    if (extend && is_signed) {
>>> +        g_assert(size < 3);
>>> +        tcg_gen_ext32u_i64(dest, dest);
>>> +    }
>>
>> Is it worth noticing in size==2 && !extend that is_signed can be forced false
>> to avoid the extra extension.
> 
> Sorry I don't quite follow, the extension only occurs if it's an explict
> extension into a 64 bit register. Or are you talking about avoiding
> using the extension logic in the generic tcg_gen_qemu_ld_i64 code?

It's not obvious from this patch, since its more about the later gpr loads.
I was thinking about size==4, signed, 32-bit.

I.e. size=10, opc=11.  But reading closer that's unallocated_encoding.  So the
answer for this patch is "no it's not worth it".

But looking forward through the other patches I see

> +        if (size == 3 && opc == 2) {
> +            /* PRFM - prefetch */
> +            return;
> +        }
> +        is_store = (opc == 0);
> +        is_signed = opc & (1<<1);
> +        is_extended = (size < 3) && (opc & 1);

whereas from the ARM I see

if opc<1> == '0' then
  // store or zero-extending load
else
  if size == '11' then
     memop = MemOp_PREFETCH;
     if opc<0> == '1' then UnallocatedEncoding();
  else
     // sign-extending load
     memop = MemOp_LOAD;
     if size == '10' && opc<0> == '1' then UnallocatedEncoding();

I.e. two undiagnosed unallocated_encoding.

BTW, I find (1<<1) harder to scan here than 2, but I wasn't going to mention
that while there was nothing else in the code to fix.


r~

Re: [Qemu-devel] [PATCH 2/9] target-arm: A64: add support for ldp (load pair)

[Alex Bennée]

rth@twiddle.net writes:

> On 12/10/2013 05:59 AM, Alex Bennée wrote:
>>>> +    if (extend && is_signed) {
>>>> +        g_assert(size < 3);
>>>> +        tcg_gen_ext32u_i64(dest, dest);
>>>> +    }
>>>
>>> Is it worth noticing in size==2 && !extend that is_signed can be forced false
>>> to avoid the extra extension.
>> 
>> Sorry I don't quite follow, the extension only occurs if it's an explict
>> extension into a 64 bit register. Or are you talking about avoiding
>> using the extension logic in the generic tcg_gen_qemu_ld_i64 code?
>
> It's not obvious from this patch, since its more about the later gpr loads.
> I was thinking about size==4, signed, 32-bit.
>
> I.e. size=10, opc=11.  But reading closer that's unallocated_encoding.  So the
> answer for this patch is "no it's not worth it".
>
> But looking forward through the other patches I see
>
>> +        if (size == 3 && opc == 2) {
>> +            /* PRFM - prefetch */
>> +            return;
>> +        }
>> +        is_store = (opc == 0);
>> +        is_signed = opc & (1<<1);
>> +        is_extended = (size < 3) && (opc & 1);
>
> whereas from the ARM I see
>
> if opc<1> == '0' then
>   // store or zero-extending load
> else
>   if size == '11' then
>      memop = MemOp_PREFETCH;
>      if opc<0> == '1' then UnallocatedEncoding();
>   else
>      // sign-extending load
>      memop = MemOp_LOAD;
>      if size == '10' && opc<0> == '1' then UnallocatedEncoding();
>
> I.e. two undiagnosed unallocated_encoding.
>
> BTW, I find (1<<1) harder to scan here than 2, but I wasn't going to mention
> that while there was nothing else in the code to fix.

Yeah it's uglier than it could be, I'll see if I can make it cleaner and
catch those missing unallocs. I've just finished the lpd/stp today and it's
running validation overnight so I'll go through the rest of the patches
tomorrow.

Cheers,

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro

[Qemu-devel] [PATCH 3/9] target-arm: A64: add support for ld/st unsigned imm

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

This adds support for the forms of ld/st with a 12 bit
unsigned immediate offset.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 95 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 600cf63..ea3abc3 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -260,6 +260,17 @@ static TCGv_i64 read_cpu_reg(DisasContext *s, int reg, int sf)
     return v;
 }
 
+static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
+{
+    TCGv_i64 v = new_tmp_a64(s);
+    if (sf) {
+        tcg_gen_mov_i64(v, cpu_X[reg]);
+    } else {
+        tcg_gen_ext32u_i64(v, cpu_X[reg]);
+    }
+    return v;
+}
+
 /* Set ZF and NF based on a 64 bit result. This is alas fiddlier
  * than the 32 bit equivalent.
  */
@@ -1037,10 +1048,92 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
     }
 }
 
+/*
+ * C3.3.13 Load/store (unsigned immediate)
+ *
+ * 31 30 29   27  26 25 24 23 22 21        10 9     5
+ * +----+-------+---+-----+-----+------------+-------+------+
+ * |size| 1 1 1 | V | 0 1 | opc |   imm12    |  Rn   |  Rt  |
+ * +----+-------+---+-----+-----+------------+-------+------+
+ *
+ * For non-vector:
+ *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
+ *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ * For vector:
+ *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
+ *   opc<0>: 0 -> store, 1 -> load
+ * Rn: base address register (inc SP)
+ * Rt: target register
+ */
+static void handle_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    unsigned int imm12 = extract32(insn, 10, 12);
+    bool is_vector = extract32(insn, 26, 1);
+    int size = extract32(insn, 30, 2);
+    int opc = extract32(insn, 22, 2);
+    unsigned int offset;
+
+    TCGv_i64 tcg_addr;
+
+    bool is_store;
+    bool is_signed = false;
+    bool is_extended = false;
+
+    if (is_vector) {
+        size |= (opc & 2) << 1;
+        if (size > 4) {
+            unallocated_encoding(s);
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        is_store = (opc == 0);
+        is_signed = opc & (1<<1);
+        is_extended = (size < 3) && (opc & 1);
+    }
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+    tcg_addr = read_cpu_reg_sp(s, rn, 1);
+    offset = imm12 << size;
+    tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+
+    if (is_vector) {
+        if (is_store) {
+            do_fp_st(s, rt, tcg_addr, size);
+        } else {
+            do_fp_ld(s, rt, tcg_addr, size);
+        }
+    } else {
+        TCGv_i64 tcg_rt = cpu_reg(s, rt);
+        if (is_store) {
+            do_gpr_st(s, tcg_rt, tcg_addr, size);
+        } else {
+            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+        }
+    }
+}
+
 /* Load/store register (all forms) */
 static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    switch (extract32(insn, 24, 2)) {
+    case 0:
+        unsupported_encoding(s, insn);
+        break;
+    case 1:
+        handle_ldst_reg_unsigned_imm(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
 }
 
 /* AdvSIMD load/store multiple structures */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 3/9] target-arm: A64: add support for ld/st unsigned imm

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> This adds support for the forms of ld/st with a 12 bit
> unsigned immediate offset.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 95 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 600cf63..ea3abc3 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -260,6 +260,17 @@ static TCGv_i64 read_cpu_reg(DisasContext *s, int reg, int sf)
>      return v;
>  }
>  
> +static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
> +{
> +    TCGv_i64 v = new_tmp_a64(s);
> +    if (sf) {
> +        tcg_gen_mov_i64(v, cpu_X[reg]);
> +    } else {
> +        tcg_gen_ext32u_i64(v, cpu_X[reg]);
> +    }
> +    return v;
> +}

Did you want to use this in for the load/store pair insns too?

r~

Re: [Qemu-devel] [PATCH 3/9] target-arm: A64: add support for ld/st unsigned imm

[Alex Bennée]

rth@twiddle.net writes:

> On 12/09/2013 10:12 AM, Peter Maydell wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
<snip>
>>  
>> +static TCGv_i64 read_cpu_reg_sp(DisasContext *s, int reg, int sf)
>> +{
>> +    TCGv_i64 v = new_tmp_a64(s);
>> +    if (sf) {
>> +        tcg_gen_mov_i64(v, cpu_X[reg]);
>> +    } else {
>> +        tcg_gen_ext32u_i64(v, cpu_X[reg]);
>> +    }
>> +    return v;
>> +}
>
> Did you want to use this in for the load/store pair insns too?

Yes. Originally I was doing a load of:

tcg_rn = cpu_reg_sp(...)
tcg_addr = tcg_temp_new_i64();
tcg_gen_mov_i64(tcg_addr, tcg_rn...

Before we had a clear API for the CPU reg stuff. Now I tend to just do:

tcg_addr = read_cpu_reg_sp(...)

I'll hoist that into the first patch now I've re-done it.

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro

[Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

This adds support for the load/store forms using a register offset.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 141 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index ea3abc3..64f98a7 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -467,6 +467,54 @@ static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size)
     tcg_temp_free_i64(tmphi);
 }
 
+/*
+ * This utility function is for doing register extension with an
+ * optional shift. You will likely want to pass a temporary for the
+ * destination register. See DecodeRegExtend() in the ARM ARM.
+ */
+static void ext_and_shift_reg(TCGv_i64 tcg_out, TCGv_i64 tcg_in,
+                              int option, unsigned int shift)
+{
+    int extsize = extract32(option, 0, 2);
+    bool is_signed = extract32(option, 2, 1);
+
+    if (is_signed) {
+        switch (extsize) {
+        case 0:
+            tcg_gen_ext8s_i64(tcg_out, tcg_in);
+            break;
+        case 1:
+            tcg_gen_ext16s_i64(tcg_out, tcg_in);
+            break;
+        case 2:
+            tcg_gen_ext32s_i64(tcg_out, tcg_in);
+            break;
+        case 3:
+            tcg_gen_mov_i64(tcg_out, tcg_in);
+            break;
+        }
+    } else {
+        switch (extsize) {
+        case 0:
+            tcg_gen_ext8u_i64(tcg_out, tcg_in);
+            break;
+        case 1:
+            tcg_gen_ext16u_i64(tcg_out, tcg_in);
+            break;
+        case 2:
+            tcg_gen_ext32u_i64(tcg_out, tcg_in);
+            break;
+        case 3:
+            tcg_gen_mov_i64(tcg_out, tcg_in);
+            break;
+        }
+    }
+
+    if (shift) {
+        tcg_gen_shli_i64(tcg_out, tcg_out, shift);
+    }
+}
+
 static inline void gen_check_sp_alignment(DisasContext *s)
 {
     /* The AArch64 architecture mandates that (if enabled via PSTATE
@@ -1048,6 +1096,93 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
     }
 }
 
+
+
+/*
+ * C3.3.10 Load/store (register offset)
+ *
+ * 31 30 29   27  26 25 24 23 22 21  20  16 15 13 12 11 10 9  5 4  0
+ * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
+ * |size| 1 1 1 | V | 0 0 | opc | 1 |  Rm  | opt | S| 1 0 | Rn | Rt |
+ * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
+ *
+ * For non-vector:
+ *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
+ *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ * For vector:
+ *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
+ *   opc<0>: 0 -> store, 1 -> load
+ * V: 1 -> vector/simd
+ * opt: extend encoding (see DecodeRegExtend)
+ * S: if S=1 then scale (essentially index by sizeof(size))
+ * Rt: register to transfer into/out of
+ * Rn: address register or SP for base
+ * Rm: offset register or ZR for offset
+ */
+static void handle_ldst_reg_roffset(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int shift = extract32(insn, 12, 1);
+    int rm = extract32(insn, 16, 5);
+    int opc = extract32(insn, 22, 2);
+    int opt = extract32(insn, 13, 3);
+    int size = extract32(insn, 30, 2);
+    bool is_signed = false;
+    bool is_store = false;
+    bool is_extended = false;
+    bool is_vector = extract32(insn, 26, 1);
+
+    TCGv_i64 tcg_rm;
+    TCGv_i64 tcg_addr;
+
+    if (extract32(opt, 1, 1) == 0) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    if (is_vector) {
+        size |= (opc & 2) << 1;
+        if (size > 4) {
+            unallocated_encoding(s);
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        is_store = (opc == 0);
+        is_signed = opc & (1<<1);
+        is_extended = (size < 3) && (opc & 1);
+    }
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+    tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+    tcg_rm = read_cpu_reg(s, rm, 1);
+    ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
+
+    tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
+
+    if (is_vector) {
+        if (is_store) {
+            do_fp_st(s, rt, tcg_addr, size);
+        } else {
+            do_fp_ld(s, rt, tcg_addr, size);
+        }
+    } else {
+        TCGv_i64 tcg_rt = cpu_reg(s, rt);
+        if (is_store) {
+            do_gpr_st(s, tcg_rt, tcg_addr, size);
+        } else {
+            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+        }
+    }
+}
+
 /*
  * C3.3.13 Load/store (unsigned immediate)
  *
@@ -1125,7 +1260,11 @@ static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
     switch (extract32(insn, 24, 2)) {
     case 0:
-        unsupported_encoding(s, insn);
+        if (extract32(insn, 21, 1) == 1 && extract32(insn, 10, 2) == 2) {
+            handle_ldst_reg_roffset(s, insn);
+        } else {
+            unsupported_encoding(s, insn);
+        }
         break;
     case 1:
         handle_ldst_reg_unsigned_imm(s, insn);
-- 
1.8.5

Re: [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> This adds support for the load/store forms using a register offset.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 141 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 140 insertions(+), 1 deletion(-)
> 
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index ea3abc3..64f98a7 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -467,6 +467,54 @@ static void do_fp_ld(DisasContext *s, int destidx, TCGv_i64 tcg_addr, int size)
>      tcg_temp_free_i64(tmphi);
>  }
>  
> +/*
> + * This utility function is for doing register extension with an
> + * optional shift. You will likely want to pass a temporary for the
> + * destination register. See DecodeRegExtend() in the ARM ARM.
> + */
> +static void ext_and_shift_reg(TCGv_i64 tcg_out, TCGv_i64 tcg_in,
> +                              int option, unsigned int shift)
> +{
> +    int extsize = extract32(option, 0, 2);
> +    bool is_signed = extract32(option, 2, 1);
> +
> +    if (is_signed) {
> +        switch (extsize) {
> +        case 0:
> +            tcg_gen_ext8s_i64(tcg_out, tcg_in);
> +            break;
> +        case 1:
> +            tcg_gen_ext16s_i64(tcg_out, tcg_in);
> +            break;
> +        case 2:
> +            tcg_gen_ext32s_i64(tcg_out, tcg_in);
> +            break;
> +        case 3:
> +            tcg_gen_mov_i64(tcg_out, tcg_in);
> +            break;
> +        }
> +    } else {
> +        switch (extsize) {
> +        case 0:
> +            tcg_gen_ext8u_i64(tcg_out, tcg_in);
> +            break;
> +        case 1:
> +            tcg_gen_ext16u_i64(tcg_out, tcg_in);
> +            break;
> +        case 2:
> +            tcg_gen_ext32u_i64(tcg_out, tcg_in);
> +            break;
> +        case 3:
> +            tcg_gen_mov_i64(tcg_out, tcg_in);
> +            break;
> +        }
> +    }
> +
> +    if (shift) {
> +        tcg_gen_shli_i64(tcg_out, tcg_out, shift);
> +    }
> +}
> +
>  static inline void gen_check_sp_alignment(DisasContext *s)
>  {
>      /* The AArch64 architecture mandates that (if enabled via PSTATE
> @@ -1048,6 +1096,93 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
>      }
>  }
>  
> +
> +
> +/*
> + * C3.3.10 Load/store (register offset)
> + *
> + * 31 30 29   27  26 25 24 23 22 21  20  16 15 13 12 11 10 9  5 4  0
> + * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
> + * |size| 1 1 1 | V | 0 0 | opc | 1 |  Rm  | opt | S| 1 0 | Rn | Rt |
> + * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
> + *
> + * For non-vector:
> + *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
> + *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
> + * For vector:
> + *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
> + *   opc<0>: 0 -> store, 1 -> load
> + * V: 1 -> vector/simd
> + * opt: extend encoding (see DecodeRegExtend)
> + * S: if S=1 then scale (essentially index by sizeof(size))
> + * Rt: register to transfer into/out of
> + * Rn: address register or SP for base
> + * Rm: offset register or ZR for offset
> + */
> +static void handle_ldst_reg_roffset(DisasContext *s, uint32_t insn)
> +{
> +    int rt = extract32(insn, 0, 5);
> +    int rn = extract32(insn, 5, 5);
> +    int shift = extract32(insn, 12, 1);
> +    int rm = extract32(insn, 16, 5);
> +    int opc = extract32(insn, 22, 2);
> +    int opt = extract32(insn, 13, 3);
> +    int size = extract32(insn, 30, 2);
> +    bool is_signed = false;
> +    bool is_store = false;
> +    bool is_extended = false;
> +    bool is_vector = extract32(insn, 26, 1);
> +
> +    TCGv_i64 tcg_rm;
> +    TCGv_i64 tcg_addr;
> +
> +    if (extract32(opt, 1, 1) == 0) {
> +        unallocated_encoding(s);
> +        return;
> +    }
> +
> +    if (is_vector) {
> +        size |= (opc & 2) << 1;
> +        if (size > 4) {
> +            unallocated_encoding(s);
> +        }
> +        is_store = ((opc & 1) == 0);
> +    } else {
> +        if (size == 3 && opc == 2) {
> +            /* PRFM - prefetch */
> +            return;
> +        }
> +        is_store = (opc == 0);
> +        is_signed = opc & (1<<1);
> +        is_extended = (size < 3) && (opc & 1);
> +    }
> +
> +    if (rn == 31) {
> +        gen_check_sp_alignment(s);
> +    }
> +    tcg_addr = read_cpu_reg_sp(s, rn, 1);
> +
> +    tcg_rm = read_cpu_reg(s, rm, 1);
> +    ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
> +
> +    tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
> +
> +    if (is_vector) {
> +        if (is_store) {
> +            do_fp_st(s, rt, tcg_addr, size);
> +        } else {
> +            do_fp_ld(s, rt, tcg_addr, size);
> +        }
> +    } else {
> +        TCGv_i64 tcg_rt = cpu_reg(s, rt);
> +        if (is_store) {
> +            do_gpr_st(s, tcg_rt, tcg_addr, size);
> +        } else {
> +            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
> +        }
> +    }
> +}

I wonder if it would be better to merge this function with the one from the
previous patch.  There are only 3-4 lines that are different.


r~

Re: [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

[Alex Bennée]

rth@twiddle.net writes:

> On 12/09/2013 10:12 AM, Peter Maydell wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>> 
>> This adds support for the load/store forms using a register offset.
>> 
<snip>
>> +/*
>> + * C3.3.10 Load/store (register offset)
>> + *
>> + * 31 30 29   27  26 25 24 23 22 21  20  16 15 13 12 11 10 9  5 4  0
>> + * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
>> + * |size| 1 1 1 | V | 0 0 | opc | 1 |  Rm  | opt | S| 1 0 | Rn | Rt |
>> + * +----+-------+---+-----+-----+---+------+-----+--+-----+----+----+
>> + *
>> + * For non-vector:
>> + *   size: 00-> byte, 01 -> 16 bit, 10 -> 32bit, 11 -> 64bit
>> + *   opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
>> + * For vector:
>> + *   size is opc<1>:size<1:0> so 100 -> 128 bit; 110 and 111 unallocated
>> + *   opc<0>: 0 -> store, 1 -> load
>> + * V: 1 -> vector/simd
>> + * opt: extend encoding (see DecodeRegExtend)
>> + * S: if S=1 then scale (essentially index by sizeof(size))
>> + * Rt: register to transfer into/out of
>> + * Rn: address register or SP for base
>> + * Rm: offset register or ZR for offset
>> + */
>> +static void handle_ldst_reg_roffset(DisasContext *s, uint32_t insn)
>> +{
>> +    int rt = extract32(insn, 0, 5);
>> +    int rn = extract32(insn, 5, 5);
>> +    int shift = extract32(insn, 12, 1);
>> +    int rm = extract32(insn, 16, 5);
>> +    int opc = extract32(insn, 22, 2);
>> +    int opt = extract32(insn, 13, 3);
>> +    int size = extract32(insn, 30, 2);
>> +    bool is_signed = false;
>> +    bool is_store = false;
>> +    bool is_extended = false;
>> +    bool is_vector = extract32(insn, 26, 1);
>> +
>> +    TCGv_i64 tcg_rm;
>> +    TCGv_i64 tcg_addr;
>> +
>> +    if (extract32(opt, 1, 1) == 0) {
>> +        unallocated_encoding(s);
>> +        return;
>> +    }
>> +
>> +    if (is_vector) {
>> +        size |= (opc & 2) << 1;
>> +        if (size > 4) {
>> +            unallocated_encoding(s);
>> +        }
>> +        is_store = ((opc & 1) == 0);
>> +    } else {
>> +        if (size == 3 && opc == 2) {
>> +            /* PRFM - prefetch */
>> +            return;
>> +        }
>> +        is_store = (opc == 0);
>> +        is_signed = opc & (1<<1);
>> +        is_extended = (size < 3) && (opc & 1);
>> +    }
>> +
>> +    if (rn == 31) {
>> +        gen_check_sp_alignment(s);
>> +    }
>> +    tcg_addr = read_cpu_reg_sp(s, rn, 1);
>> +
>> +    tcg_rm = read_cpu_reg(s, rm, 1);
>> +    ext_and_shift_reg(tcg_rm, tcg_rm, opt, shift ? size : 0);
>> +
>> +    tcg_gen_add_i64(tcg_addr, tcg_addr, tcg_rm);
>> +
>> +    if (is_vector) {
>> +        if (is_store) {
>> +            do_fp_st(s, rt, tcg_addr, size);
>> +        } else {
>> +            do_fp_ld(s, rt, tcg_addr, size);
>> +        }
>> +    } else {
>> +        TCGv_i64 tcg_rt = cpu_reg(s, rt);
>> +        if (is_store) {
>> +            do_gpr_st(s, tcg_rt, tcg_addr, size);
>> +        } else {
>> +            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
>> +        }
>> +    }
>> +}
>
> I wonder if it would be better to merge this function with the one from the
> previous patch.  There are only 3-4 lines that are different.

I was trying to avoid having it jump through special cases when decoding
the instruction where Rm, opt, S class with the imm field in the other
form. But certainly there is an argument for the rest of the common code
to be shared (size/opc decoding and final tcg_addr based load/store).

However my preference unless there is a strong objection would be to
clean that up in later patches. For one thing the more instructions each
patch handles the longer it takes to run the instruction validation on
the rather slow models to have good coverage of the decoder!

>
>
> r~

-- 
Alex Bennée
QEMU/KVM Hacker for Linaro

Re: [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

[Richard Henderson]

On 12/10/2013 06:16 AM, Alex Bennée wrote:
> However my preference unless there is a strong objection would be to
> clean that up in later patches. For one thing the more instructions each
> patch handles the longer it takes to run the instruction validation on
> the rather slow models to have good coverage of the decoder!

That'd be ok by me.


r~

Re: [Qemu-devel] [PATCH 4/9] target-arm: A64: add support for ld/st with reg offset

[Alex Bennée]

rth@twiddle.net writes:

> On 12/10/2013 06:16 AM, Alex Bennée wrote:
>> However my preference unless there is a strong objection would be to
>> clean that up in later patches. For one thing the more instructions each
>> patch handles the longer it takes to run the instruction validation on
>> the rather slow models to have good coverage of the decoder!
>
> That'd be ok by me.

I had a play trying to see what pulling out the common parts of decode
and unallocated handling based on the ARM ARM pseudo-code into a
separate function would look like. Unfortunately what I ended up
with was a horrible function full of pass-by-reference parameters and a
not particularly cleaner or shorter call-sites in each handler.

I did briefly consider if I could construct a macro which would make for
less duplicated code but suspect that won't help in the long run.

This is certainly something I think that requires more thought. In the
meantime I've addresses your other review comments and Peter should be
pushing a new set of patches soon.


Cheers,

--
Alex Bennée
QEMU/KVM Hacker for Linaro

[Qemu-devel] [PATCH 5/9] target-arm: A64: add support for ld/st with index

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

This adds support for the pre/post-index ld/st forms with immediate
offsets as well as the un-scaled immediate form (which are all
variations on the same 9-bit immediate instruction form).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 119 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 64f98a7..b0f2478 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1096,6 +1096,104 @@ static void disas_ldst_pair(DisasContext *s, uint32_t insn)
     }
 }
 
+/*
+ * C3.3.8 Load/store (immediate post-indexed)
+ * C3.3.9 Load/store (immediate pre-indexed)
+ * C3.3.12 Load/store (unscaled immediate)
+ *
+ * 31 30 29   27  26 25 24 23 22 21  20    12 11 10 9    5 4    0
+ * +----+-------+---+-----+-----+---+--------+-----+------+------+
+ * |size| 1 1 1 | V | 0 0 | opc | 0 |  imm9  | idx |  Rn  |  Rt  |
+ * +----+-------+---+-----+-----+---+--------+-----+------+------+
+ *
+ * idx = 01 -> post-indexed, 11 pre-indexed, 00 unscaled imm. (no writeback)
+ * V = 0 -> non-vector
+ * size: 00 -> 8 bit, 01 -> 16 bit, 10 -> 32 bit, 11 -> 64bit
+ * opc: 00 -> store, 01 -> loadu, 10 -> loads 64, 11 -> loads 32
+ */
+static void handle_ldst_reg_imm9(DisasContext *s, uint32_t insn)
+{
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int imm9 = sextract32(insn, 12, 9);
+    int opc = extract32(insn, 22, 2);
+    int size = extract32(insn, 30, 2);
+    int idx = extract32(insn, 10, 2);
+    bool is_signed = false;
+    bool is_store = false;
+    bool is_extended = false;
+    bool is_vector = extract32(insn, 26, 1);
+    bool post_index;
+    bool writeback;
+
+    TCGv_i64 tcg_addr;
+
+    if (is_vector) {
+        size |= (opc & 2) << 1;
+        if (size > 4) {
+            unallocated_encoding(s);
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        is_store = (opc == 0);
+        is_signed = opc & (1<<1);
+        is_extended = (size < 3) && (opc & 1);
+    }
+
+    switch (idx) {
+    case 0:
+        post_index = false;
+        writeback = false;
+        break;
+    case 1:
+        post_index = true;
+        writeback = true;
+        break;
+    case 3:
+        post_index = false;
+        writeback = true;
+        break;
+    case 2:
+        g_assert(false);
+        break;
+    }
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+    tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+    if (!post_index) {
+        tcg_gen_addi_i64(tcg_addr, tcg_addr, imm9);
+    }
+
+    if (is_vector) {
+        if (is_store) {
+            do_fp_st(s, rt, tcg_addr, size);
+        } else {
+            do_fp_ld(s, rt, tcg_addr, size);
+        }
+    } else {
+        TCGv_i64 tcg_rt = cpu_reg(s, rt);
+        if (is_store) {
+            do_gpr_st(s, tcg_rt, tcg_addr, size);
+        } else {
+            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, is_extended);
+        }
+    }
+
+    if (writeback) {
+        TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
+        if (post_index) {
+            tcg_gen_addi_i64(tcg_addr, tcg_addr, imm9);
+        }
+        tcg_gen_mov_i64(tcg_rn, tcg_addr);
+    }
+}
 
 
 /*
@@ -1255,6 +1353,25 @@ static void handle_ldst_reg_unsigned_imm(DisasContext *s, uint32_t insn)
     }
 }
 
+/* Load/store register (immediate forms) */
+static void disas_ldst_reg_imm(DisasContext *s, uint32_t insn)
+{
+    switch (extract32(insn, 10, 2)) {
+    case 0: case 1: case 3:
+        /* Load/store register (unscaled immediate) */
+        /* Load/store immediate pre/post-indexed */
+        handle_ldst_reg_imm9(s, insn);
+        break;
+    case 2:
+        /* Load/store register unprivileged */
+        unsupported_encoding(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
+}
+
 /* Load/store register (all forms) */
 static void disas_ldst_reg(DisasContext *s, uint32_t insn)
 {
@@ -1263,7 +1380,7 @@ static void disas_ldst_reg(DisasContext *s, uint32_t insn)
         if (extract32(insn, 21, 1) == 1 && extract32(insn, 10, 2) == 2) {
             handle_ldst_reg_roffset(s, insn);
         } else {
-            unsupported_encoding(s, insn);
+            disas_ldst_reg_imm(s, insn);
         }
         break;
     case 1:
-- 
1.8.5

[Qemu-devel] [PATCH 6/9] target-arm: A64: add support for add, addi, sub, subi

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

Implement the non-carry forms of addition and subtraction
(immediate, extended register and shifted register).
This includes the code to calculate NZCV if the instruction
calls for setting the flags.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 300 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 290 insertions(+), 10 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index b0f2478..294de4f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -298,6 +298,102 @@ static inline void gen_logic_CC(int sf, TCGv_i64 result)
     tcg_gen_movi_i32(cpu_VF, 0);
 }
 
+/* dest = T0 + T1; compute C, N, V and Z flags */
+static void gen_add_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+    if (sf) {
+        TCGv_i64 result, flag, tmp;
+        result = tcg_temp_new_i64();
+        flag = tcg_temp_new_i64();
+        tmp = tcg_temp_new_i64();
+
+        tcg_gen_movi_i64(tmp, 0);
+        tcg_gen_add2_i64(result, flag, t0, tmp, t1, tmp);
+
+        tcg_gen_trunc_i64_i32(cpu_CF, flag);
+
+        gen_set_NZ64(result);
+
+        tcg_gen_xor_i64(flag, result, t0);
+        tcg_gen_xor_i64(tmp, t0, t1);
+        tcg_gen_andc_i64(flag, flag, tmp);
+        tcg_temp_free_i64(tmp);
+        tcg_gen_shri_i64(flag, flag, 32);
+        tcg_gen_trunc_i64_i32(cpu_VF, flag);
+
+        tcg_gen_mov_i64(dest, result);
+        tcg_temp_free_i64(result);
+        tcg_temp_free_i64(flag);
+    } else {
+        /* 32 bit arithmetic */
+        TCGv_i32 t0_32 = tcg_temp_new_i32();
+        TCGv_i32 t1_32 = tcg_temp_new_i32();
+        TCGv_i32 tmp = tcg_temp_new_i32();
+
+        tcg_gen_movi_i32(tmp, 0);
+        tcg_gen_trunc_i64_i32(t0_32, t0);
+        tcg_gen_trunc_i64_i32(t1_32, t1);
+        tcg_gen_add2_i32(cpu_NF, cpu_CF, t0_32, tmp, t1_32, tmp);
+        tcg_gen_mov_i32(cpu_ZF, cpu_NF);
+        tcg_gen_xor_i32(cpu_VF, cpu_NF, t0_32);
+        tcg_gen_xor_i32(tmp, t0_32, t1_32);
+        tcg_gen_andc_i32(cpu_VF, cpu_VF, tmp);
+        tcg_gen_extu_i32_i64(dest, cpu_NF);
+
+        tcg_temp_free_i32(tmp);
+        tcg_temp_free_i32(t0_32);
+        tcg_temp_free_i32(t1_32);
+    }
+}
+
+/* dest = T0 - T1; compute C, N, V and Z flags */
+static void gen_sub_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+    if (sf) {
+        /* 64 bit arithmetic */
+        TCGv_i64 result, flag, tmp;
+
+        result = tcg_temp_new_i64();
+        flag = tcg_temp_new_i64();
+        tcg_gen_sub_i64(result, t0, t1);
+
+        gen_set_NZ64(result);
+
+        tcg_gen_setcond_i64(TCG_COND_GEU, flag, t0, t1);
+        tcg_gen_trunc_i64_i32(cpu_CF, flag);
+
+        tcg_gen_xor_i64(flag, result, t0);
+        tmp = tcg_temp_new_i64();
+        tcg_gen_xor_i64(tmp, t0, t1);
+        tcg_gen_and_i64(flag, flag, tmp);
+        tcg_temp_free_i64(tmp);
+        tcg_gen_shri_i64(flag, flag, 32);
+        tcg_gen_trunc_i64_i32(cpu_VF, flag);
+        tcg_gen_mov_i64(dest, result);
+        tcg_temp_free_i64(flag);
+        tcg_temp_free_i64(result);
+    } else {
+        /* 32 bit arithmetic */
+        TCGv_i32 t0_32 = tcg_temp_new_i32();
+        TCGv_i32 t1_32 = tcg_temp_new_i32();
+        TCGv_i32 tmp;
+
+        tcg_gen_trunc_i64_i32(t0_32, t0);
+        tcg_gen_trunc_i64_i32(t1_32, t1);
+        tcg_gen_sub_i32(cpu_NF, t0_32, t1_32);
+        tcg_gen_mov_i32(cpu_ZF, cpu_NF);
+        tcg_gen_setcond_i32(TCG_COND_GEU, cpu_CF, t0_32, t1_32);
+        tcg_gen_xor_i32(cpu_VF, cpu_NF, t0_32);
+        tmp = tcg_temp_new_i32();
+        tcg_gen_xor_i32(tmp, t0_32, t1_32);
+        tcg_temp_free_i32(t0_32);
+        tcg_temp_free_i32(t1_32);
+        tcg_gen_and_i32(cpu_VF, cpu_VF, tmp);
+        tcg_temp_free_i32(tmp);
+        tcg_gen_extu_i32_i64(dest, cpu_NF);
+    }
+}
+
 /*
  * Load/Store generators
  */
@@ -1461,10 +1557,67 @@ static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
     tcg_gen_movi_i64(cpu_reg(s, rd), base + offset);
 }
 
-/* Add/subtract (immediate) */
+/*
+ * C3.4.1 Add/subtract (immediate)
+ *
+ *  31 30 29 28       24 23 22 21         10 9   5 4   0
+ * +--+--+--+-----------+-----+-------------+-----+-----+
+ * |sf|op| S| 1 0 0 0 1 |shift|    imm12    |  Rn | Rd  |
+ * +--+--+--+-----------+-----+-------------+-----+-----+
+ *
+ *    sf: 0 -> 32bit, 1 -> 64bit
+ *    op: 0 -> add  , 1 -> sub
+ *     S: 1 -> set flags
+ * shift: 00 -> LSL imm by 0, 01 -> LSL imm by 12
+ */
 static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    uint64_t imm = extract32(insn, 10, 12);
+    int shift = extract32(insn, 22, 2);
+    bool setflags = extract32(insn, 29, 1);
+    bool sub_op = extract32(insn, 30, 1);
+    bool is_64bit = extract32(insn, 31, 1);
+
+    TCGv_i64 tcg_rn = cpu_reg_sp(s, rn);
+    TCGv_i64 tcg_rd = setflags ? cpu_reg(s, rd) : cpu_reg_sp(s, rd);
+    TCGv_i64 tcg_result;
+
+    switch (shift) {
+    case 0x0:
+        break;
+    case 0x1:
+        imm <<= 12;
+        break;
+    default:
+        unallocated_encoding(s);
+    }
+
+    tcg_result = tcg_temp_new_i64();
+    if (!setflags) {
+        if (sub_op) {
+            tcg_gen_subi_i64(tcg_result, tcg_rn, imm);
+        } else {
+            tcg_gen_addi_i64(tcg_result, tcg_rn, imm);
+        }
+    } else {
+        TCGv_i64 tcg_imm = tcg_const_i64(imm);
+        if (sub_op) {
+            gen_sub_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
+        } else {
+            gen_add_CC(is_64bit, tcg_result, tcg_rn, tcg_imm);
+        }
+        tcg_temp_free_i64(tcg_imm);
+    }
+
+    if (is_64bit) {
+        tcg_gen_mov_i64(tcg_rd, tcg_result);
+    } else {
+        tcg_gen_ext32u_i64(tcg_rd, tcg_result);
+    }
+
+    tcg_temp_free_i64(tcg_result);
 }
 
 /* The input should be a value in the bottom e bits (with higher
@@ -1922,16 +2075,143 @@ static void disas_logic_reg(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Add/subtract (extended register) */
-static void disas_add_sub_ext_reg(DisasContext *s, uint32_t insn)
+/*
+ * C3.5.1 Add/subtract (extended register)
+ *
+ *  31|30|29|28       24|23 22|21|20   16|15  13|12  10|9  5|4  0|
+ * +--+--+--+-----------+-----+--+-------+------+------+----+----+
+ * |sf|op| S| 0 1 0 1 1 | opt | 1|  Rm   |option| imm3 | Rn | Rd |
+ * +--+--+--+-----------+-----+--+-------+------+------+----+----+
+ *
+ *  sf: 0 -> 32bit, 1 -> 64bit
+ *  op: 0 -> add  , 1 -> sub
+ *   S: 1 -> set flags
+ * opt: 00
+ * option: extension type (see DecodeRegExtend)
+ * imm3: optional shift to Rm
+ *
+ * Rd = Rn + LSL(extend(Rm), amount)
+ */
+static void handle_add_sub_ext_reg(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int imm3 = extract32(insn, 10, 3);
+    int option = extract32(insn, 13, 3);
+    int rm = extract32(insn, 16, 5);
+    bool setflags = extract32(insn, 29, 1);
+    bool sub_op = extract32(insn, 30, 1);
+    bool sf = extract32(insn, 31, 1);
+
+    TCGv_i64 tcg_rm, tcg_rn; /* temps */
+    TCGv_i64 tcg_rd;
+    TCGv_i64 tcg_result;
+
+    if (imm3 > 4) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    /* non-flag setting ops may use SP */
+    if (!setflags) {
+        tcg_rn = read_cpu_reg_sp(s, rn, sf);
+        tcg_gen_mov_i64(tcg_rn, cpu_reg_sp(s, rn));
+        tcg_rd = cpu_reg_sp(s, rd);
+    } else {
+        tcg_rn = read_cpu_reg(s, rn, sf);
+        tcg_rd = cpu_reg(s, rd);
+    }
+
+    tcg_rm = read_cpu_reg(s, rm, sf);
+    ext_and_shift_reg(tcg_rm, tcg_rm, option, imm3);
+
+    tcg_result = tcg_temp_new_i64();
+
+    if (!setflags) {
+        if (sub_op) {
+            tcg_gen_sub_i64(tcg_result, tcg_rn, tcg_rm);
+        } else {
+            tcg_gen_add_i64(tcg_result, tcg_rn, tcg_rm);
+        }
+    } else {
+        if (sub_op) {
+            gen_sub_CC(sf, tcg_result, tcg_rn, tcg_rm);
+        } else {
+            gen_add_CC(sf, tcg_result, tcg_rn, tcg_rm);
+        }
+    }
+
+    if (sf) {
+        tcg_gen_mov_i64(tcg_rd, tcg_result);
+    } else {
+        tcg_gen_ext32u_i64(tcg_rd, tcg_result);
+    }
+
+    tcg_temp_free_i64(tcg_result);
 }
 
-/* Add/subtract (shifted register) */
-static void disas_add_sub_reg(DisasContext *s, uint32_t insn)
+/*
+ * C3.5.2 Add/subtract (shifted register)
+ *
+ *  31 30 29 28       24 23 22 21 20   16 15     10 9    5 4    0
+ * +--+--+--+-----------+-----+--+-------+---------+------+------+
+ * |sf|op| S| 0 1 0 1 1 |shift| 0|  Rm   |  imm6   |  Rn  |  Rd  |
+ * +--+--+--+-----------+-----+--+-------+---------+------+------+
+ *
+ *    sf: 0 -> 32bit, 1 -> 64bit
+ *    op: 0 -> add  , 1 -> sub
+ *     S: 1 -> set flags
+ * shift: 00 -> LSL, 01 -> LSR, 10 -> ASR, 11 -> RESERVED
+ *  imm6: Shift amount to apply to Rm before the add/sub
+ */
+static void handle_add_sub_reg(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int imm6 = extract32(insn, 10, 6);
+    int rm = extract32(insn, 16, 5);
+    int shift_type = extract32(insn, 22, 2);
+    bool setflags = extract32(insn, 29, 1);
+    bool sub_op = extract32(insn, 30, 1);
+    bool sf = extract32(insn, 31, 1);
+
+    TCGv_i64 tcg_rd = cpu_reg(s, rd);
+    TCGv_i64 tcg_rn, tcg_rm;
+    TCGv_i64 tcg_result;
+
+    if ((shift_type == 3) || (!sf && (imm6 > 31))) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    tcg_rn = read_cpu_reg(s, rn, sf);
+    tcg_rm = read_cpu_reg(s, rm, sf);
+
+    shift_reg_imm(tcg_rm, tcg_rm, sf, shift_type, imm6);
+
+    tcg_result = tcg_temp_new_i64();
+
+    if (!setflags) {
+        if (sub_op) {
+            tcg_gen_sub_i64(tcg_result, tcg_rn, tcg_rm);
+        } else {
+            tcg_gen_add_i64(tcg_result, tcg_rn, tcg_rm);
+        }
+    } else {
+        if (sub_op) {
+            gen_sub_CC(sf, tcg_result, tcg_rn, tcg_rm);
+        } else {
+            gen_add_CC(sf, tcg_result, tcg_rn, tcg_rm);
+        }
+    }
+
+    if (sf) {
+        tcg_gen_mov_i64(tcg_rd, tcg_result);
+    } else {
+        tcg_gen_ext32u_i64(tcg_rd, tcg_result);
+    }
+
+    tcg_temp_free_i64(tcg_result);
 }
 
 /* Data-processing (3 source) */
@@ -2295,9 +2575,9 @@ static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
         break;
     case 0x0b: /* Add/subtract */
         if (insn & (1 << 21)) { /* (extended register) */
-            disas_add_sub_ext_reg(s, insn);
+            handle_add_sub_ext_reg(s, insn);
         } else {
-            disas_add_sub_reg(s, insn);
+            handle_add_sub_reg(s, insn);
         }
         break;
     case 0x1b: /* Data-processing (3 source) */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 6/9] target-arm: A64: add support for add, addi, sub, subi

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> +        tcg_rn = read_cpu_reg_sp(s, rn, sf);
> +        tcg_gen_mov_i64(tcg_rn, cpu_reg_sp(s, rn));

Duplicate copy from the register.

Otherwise,

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


r~

[Qemu-devel] [PATCH 7/9] target-arm: A64: add support for move wide instructions

[Peter Maydell]

From: Alex Bennée <alex.bennee@linaro.org>

This patch adds emulation for the mov wide instructions
(MOVN, MOVZ, MOVK).

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 51 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 294de4f..731e779 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1776,10 +1776,57 @@ static void disas_logic_imm(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Move wide (immediate) */
+/*
+ * C3.4.5 Move wide (immediate)
+ *
+ *  31 30 29 28         23 22 21 20             5 4    0
+ * +--+-----+-------------+-----+----------------+------+
+ * |sf| opc | 1 0 0 1 0 1 |  hw |  imm16         |  Rd  |
+ * +--+-----+-------------+-----+----------------+------+
+ *
+ * sf: 0 -> 32 bit, 1 -> 64 bit
+ * opc: 00 -> N, 10 -> Z, 11 -> K
+ * hw: shift/16 (0,16, and sf only 32, 48)
+ */
 static void disas_movw_imm(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    uint64_t imm = extract32(insn, 5, 16);
+    int sf = extract32(insn, 31, 1);
+    int opc = extract32(insn, 29, 2);
+    int pos = extract32(insn, 21, 2) << 4;
+    TCGv_i64 tcg_rd = cpu_reg(s, rd);
+    TCGv_i64 tcg_imm;
+
+    if (!sf && (pos >= 32)) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    switch (opc) {
+    case 0: /* MOVN */
+    case 2: /* MOVZ */
+        imm <<= pos;
+        if (opc == 0) {
+            imm = ~imm;
+        }
+        if (!sf) {
+            imm &= 0xffffffffu;
+        }
+        tcg_gen_movi_i64(tcg_rd, imm);
+        break;
+    case 3: /* MOVK */
+        tcg_imm = tcg_const_i64(imm);
+        tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_imm, pos, 16);
+        tcg_temp_free_i64(tcg_imm);
+        if (!sf) {
+            tcg_gen_ext32u_i64(tcg_rd, tcg_rd);
+        }
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
 }
 
 /* C3.4.2 Bitfield
-- 
1.8.5

Re: [Qemu-devel] [PATCH 7/9] target-arm: A64: add support for move wide instructions

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> This patch adds emulation for the mov wide instructions
> (MOVN, MOVZ, MOVK).
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 51 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 49 insertions(+), 2 deletions(-)

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


r~

[Qemu-devel] [PATCH 8/9] target-arm: A64: add support for 3 src data proc insns

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

This patch adds emulation for the "Data-processing (3 source)"
family of instructions, namely MADD, MSUB, SMADDL, SMSUBL, SMULH,
UMADDL, UMSUBL, UMULH.

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper-a64.c    | 14 ++++++++
 target-arm/helper-a64.h    |  2 ++
 target-arm/translate-a64.c | 85 ++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index d3f7067..691d56b 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -77,3 +77,17 @@ uint64_t HELPER(rbit64)(uint64_t x)
 
     return x;
 }
+
+uint64_t HELPER(umulh)(uint64_t n, uint64_t m)
+{
+    uint64_t rl, rh;
+    mulu64(&rl, &rh, n, m);
+    return rh;
+}
+
+uint64_t HELPER(smulh)(uint64_t n, uint64_t m)
+{
+    uint64_t rl, rh;
+    muls64(&rl, &rh, n, m);
+    return rh;
+}
diff --git a/target-arm/helper-a64.h b/target-arm/helper-a64.h
index a163a94..275da96 100644
--- a/target-arm/helper-a64.h
+++ b/target-arm/helper-a64.h
@@ -22,3 +22,5 @@ DEF_HELPER_FLAGS_1(clz64, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(cls64, TCG_CALL_NO_RWG_SE, i64, i64)
 DEF_HELPER_FLAGS_1(cls32, TCG_CALL_NO_RWG_SE, i32, i32)
 DEF_HELPER_FLAGS_1(rbit64, TCG_CALL_NO_RWG_SE, i64, i64)
+DEF_HELPER_FLAGS_2(umulh, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(smulh, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 731e779..1ab264f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2261,10 +2261,91 @@ static void handle_add_sub_reg(DisasContext *s, uint32_t insn)
     tcg_temp_free_i64(tcg_result);
 }
 
-/* Data-processing (3 source) */
+/* C3.5.9 Data-processing (3 source)
+
+   31 30  29 28       24 23 21  20  16  15  14  10 9    5 4    0
+  +--+------+-----------+------+------+----+------+------+------+
+  |sf| op54 | 1 1 0 1 1 | op31 |  Rm  | o0 |  Ra  |  Rn  |  Rd  |
+  +--+------+-----------+------+------+----+------+------+------+
+
+ */
 static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int ra = extract32(insn, 10, 5);
+    int rm = extract32(insn, 16, 5);
+    int op_id = (extract32(insn, 29, 3) << 4) |
+        (extract32(insn, 21, 3) << 1) |
+        extract32(insn, 15, 1);
+    bool is_32bit = !extract32(insn, 31, 1);
+    bool is_sub = extract32(op_id, 0, 1);
+    bool is_high = extract32(op_id, 2, 1);
+    bool is_signed = false;
+    TCGv_i64 tcg_op1;
+    TCGv_i64 tcg_op2;
+    TCGv_i64 tcg_tmp;
+
+    /* Note that op_id is sf:op54:op31:o0 so it includes the 32/64 size flag */
+    switch (op_id) {
+    case 0x42: /* SMADDL */
+    case 0x43: /* SMSUBL */
+    case 0x44: /* SMULH */
+        is_signed = true;
+        break;
+    case 0x0: /* MADD (32bit) */
+    case 0x1: /* MSUB (32bit) */
+    case 0x40: /* MADD (64bit) */
+    case 0x41: /* MSUB (64bit) */
+    case 0x4a: /* UMADDL */
+    case 0x4b: /* UMSUBL */
+    case 0x4c: /* UMULH */
+        break;
+    default:
+        unallocated_encoding(s);
+    }
+
+    if (is_high) {
+        /* SMULH and UMULH go via helpers for the 64x64->128 multiply */
+        if (is_signed) {
+            gen_helper_smulh(cpu_reg(s, rd), cpu_reg(s, rn), cpu_reg(s, rm));
+        } else {
+            gen_helper_umulh(cpu_reg(s, rd), cpu_reg(s, rn), cpu_reg(s, rm));
+        }
+        return;
+    }
+
+    tcg_op1 = tcg_temp_new_i64();
+    tcg_op2 = tcg_temp_new_i64();
+    tcg_tmp = tcg_temp_new_i64();
+
+    if (op_id < 0x42) {
+        tcg_gen_mov_i64(tcg_op1, cpu_reg(s, rn));
+        tcg_gen_mov_i64(tcg_op2, cpu_reg(s, rm));
+    } else {
+        if (is_signed) {
+            tcg_gen_ext32s_i64(tcg_op1, cpu_reg(s, rn));
+            tcg_gen_ext32s_i64(tcg_op2, cpu_reg(s, rm));
+        } else {
+            tcg_gen_ext32u_i64(tcg_op1, cpu_reg(s, rn));
+            tcg_gen_ext32u_i64(tcg_op2, cpu_reg(s, rm));
+        }
+    }
+
+    tcg_gen_mul_i64(tcg_tmp, tcg_op1, tcg_op2);
+    if (is_sub) {
+        tcg_gen_sub_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+    } else {
+        tcg_gen_add_i64(cpu_reg(s, rd), cpu_reg(s, ra), tcg_tmp);
+    }
+
+    if (is_32bit) {
+        tcg_gen_ext32u_i64(cpu_reg(s, rd), cpu_reg(s, rd));
+    }
+
+    tcg_temp_free_i64(tcg_op1);
+    tcg_temp_free_i64(tcg_op2);
+    tcg_temp_free_i64(tcg_tmp);
 }
 
 /* Add/subtract (with carry) */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 8/9] target-arm: A64: add support for 3 src data proc insns

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> +    if (is_high) {
> +        /* SMULH and UMULH go via helpers for the 64x64->128 multiply */
> +        if (is_signed) {
> +            gen_helper_smulh(cpu_reg(s, rd), cpu_reg(s, rn), cpu_reg(s, rm));
> +        } else {
> +            gen_helper_umulh(cpu_reg(s, rd), cpu_reg(s, rn), cpu_reg(s, rm));
> +        }

Should use tcg_gen_mul[su]2_i64, with a dummy temp for the first arg; the
highpart result is placed into the second arg.



r~

[Qemu-devel] [PATCH 9/9] target-arm: A64: implement SVC, BRK

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Add decoding for the exception generating instructions, and implement
SVC (syscalls) and BRK (software breakpoint).

Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 51 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 1ab264f..c07cb30 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -871,10 +871,57 @@ static void disas_system(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Exception generation */
+/* C3.2.3 Exception generation
+ *
+ *  31             24 23 21 20                     5 4   2 1  0
+ * +-----------------+-----+------------------------+-----+----+
+ * | 1 1 0 1 0 1 0 0 | opc |          imm16         | op2 | LL |
+ * +-----------------------+------------------------+----------+
+ */
 static void disas_exc(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int opc = extract32(insn, 21, 3);
+    int op2_ll = extract32(insn, 0, 5);
+
+    switch (opc) {
+    case 0:
+        /* SVC, HVC, SMC; since we don't support the Virtualization
+         * or TrustZone extensions these all UNDEF except SVC.
+         */
+        if (op2_ll != 1) {
+            unallocated_encoding(s);
+            break;
+        }
+        gen_exception_insn(s, 0, EXCP_SWI);
+        break;
+    case 1:
+        if (op2_ll != 0) {
+            unallocated_encoding(s);
+            break;
+        }
+        /* BRK */
+        gen_exception_insn(s, 0, EXCP_BKPT);
+        break;
+    case 2:
+        if (op2_ll != 0) {
+            unallocated_encoding(s);
+            break;
+        }
+        /* HLT */
+        unsupported_encoding(s, insn);
+        break;
+    case 5:
+        if (op2_ll < 1 || op2_ll > 3) {
+            unallocated_encoding(s);
+            break;
+        }
+        /* DCPS1, DCPS2, DCPS3 */
+        unsupported_encoding(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
 }
 
 /* C3.2.7 Unconditional branch (register)
-- 
1.8.5

Re: [Qemu-devel] [PATCH 9/9] target-arm: A64: implement SVC, BRK

[Richard Henderson]

On 12/09/2013 10:12 AM, Peter Maydell wrote:
> +    case 0:
> +        /* SVC, HVC, SMC; since we don't support the Virtualization
> +         * or TrustZone extensions these all UNDEF except SVC.
> +         */
> +        if (op2_ll != 1) {
> +            unallocated_encoding(s);
> +            break;
> +        }
> +        gen_exception_insn(s, 0, EXCP_SWI);
> +        break;

Should the imm16 should be stored somewhere, for exception.syndrome?

I can only presume from this that the AA64 userland doesn't encode its syscall
number in the insn...

Otherwise,

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


r~

Re: [Qemu-devel] [PATCH 9/9] target-arm: A64: implement SVC, BRK

[Peter Maydell]

On 9 December 2013 21:58, Richard Henderson <rth@twiddle.net> wrote:
> On 12/09/2013 10:12 AM, Peter Maydell wrote:
>> +    case 0:
>> +        /* SVC, HVC, SMC; since we don't support the Virtualization
>> +         * or TrustZone extensions these all UNDEF except SVC.
>> +         */
>> +        if (op2_ll != 1) {
>> +            unallocated_encoding(s);
>> +            break;
>> +        }
>> +        gen_exception_insn(s, 0, EXCP_SWI);
>> +        break;
>
> Should the imm16 should be stored somewhere, for exception.syndrome?
>
> I can only presume from this that the AA64 userland doesn't encode its syscall
> number in the insn...

It doesn't; it goes in X8 (we already have the aarch64 cpu_loop() in
linux-user/main.c). I thought about adding a comment about the
syndrome register, but decided against on the basis that for the
system emulation work we're going to have to update every point
where we take an exception to record syndrome information,
and this one didn't seem special enough to be worth noting.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/9] target-arm: A64 decoder set 3: loads, stores, misc integer

[Peter Maydell]

On 9 December 2013 18:12, Peter Maydell <peter.maydell@linaro.org> wrote:
> Git tree (with v7-cpu-host/mach-virt, v8 kvm control,
>  and A64 set one & two all underneath these patches):
>  git://git.linaro.org/people/peter.maydell/qemu-arm.git a64-third-set
> web UI:
>  https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/a64-second-set

Cut-n-paste error, obviously: web UI URL should be
https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/a64-third-set

thanks
-- PMM