[Qemu-devel] [PATCH 00/21] target-arm: A64 decoder sets 3 and 4: everything but fp & simd

[Qemu-devel] [PATCH 00/21] target-arm: A64 decoder sets 3 and 4: everything but fp & simd

[Peter Maydell]

Hi; the a64-third-set patches haven't all made it through
review yet, but I thought it was worth adding my fourth set
of patches into them, because with these patches we have
support for pretty much all instructions except:
 * FP [we support simple FP register load/store and
   transfer to/from the general purpose registers]
 * Neon
 * system instructions (either ones only available to system
   mode or which only make sense for system mode like LDRT)
(There may also be one or two obscure gaps like the CRC32 instruction.)

So this patchset includes the default-config that enables
building the aarch64-linux target.

This patchset includes support for MRS/MSR for the handful
of registers which are accessible in user mode. I've
replaced the simple user-mode-only implementation in the
initial SuSE patches with a proper integration into the
cpregs hashtable infrastructure, because we're going to need
that for system mode anyway.

The ld/st exclusive code has also been reworked slightly
so that the AArch32 and AArch64 sides coexist a bit more
cleanly.

This patchset sits on top of target-arm.next; git tree
available at:
 git://git.linaro.org/people/peter.maydell/qemu-arm.git a64-third-fourth-set
web UI:
 https://git.linaro.org/people/peter.maydell/qemu-arm.git/shortlog/refs/heads/a64-third-fourth-set

Review welcomed, though I appreciate that we're heading
down towards the Christmas lull...

thanks
-- PMM

Alex Bennée (6):
  target-arm: A64: add support for ld/st 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 (3):
  target-arm: A64: add support for 3 src data proc insns
  target-arm: A64: implement SVC, BRK
  target-arm: aarch64: add support for ld lit

Claudio Fontana (2):
  target-arm: A64: add support for add/sub with carry
  target-arm: A64: add support for conditional compare insns

Michael Matz (1):
  target-arm: A64: support for ld/st/cl exclusive

Peter Maydell (9):
  target-arm: A64: Add decoder skeleton for FP instructions
  target-arm: A64: implement FMOV
  target-arm: Update generic cpreg code for AArch64
  target-arm: Remove ARMCPU/CPUARMState from cpregs APIs used by decoder
  target-arm: A64: Implement MRS/MSR/SYS/SYSL
  target-arm: A64: Implement minimal set of EL0-visible sysregs
  target-arm: Widen thread-local register state fields to 64 bits
  target-arm: Widen exclusive-access support struct fields to 64 bits
  default-configs: Add config for aarch64-linux-user

 default-configs/aarch64-linux-user.mak |    3 +
 linux-user/aarch64/target_cpu.h        |    5 +-
 linux-user/arm/target_cpu.h            |    2 +-
 linux-user/main.c                      |  154 ++-
 target-arm/cpu.h                       |   94 +-
 target-arm/helper.c                    |  151 ++-
 target-arm/kvm-consts.h                |   37 +
 target-arm/machine.c                   |   12 +-
 target-arm/translate-a64.c             | 1927 +++++++++++++++++++++++++++++++-
 target-arm/translate.c                 |   72 +-
 target-arm/translate.h                 |    2 +
 11 files changed, 2320 insertions(+), 139 deletions(-)
 create mode 100644 default-configs/aarch64-linux-user.mak

-- 
1.8.5

[Qemu-devel] [PATCH 01/21] target-arm: A64: add support for ld/st pair

[Peter Maydell]

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

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

  * do_gpr_st()
  * do_fp_st()
  * do_gpr_ld()
  * do_fp_ld()
  * read_cpu_reg_sp()
  * 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 | 277 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 275 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 0a76130..e4ce038 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -99,6 +99,15 @@ 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);
@@ -250,6 +259,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.
  */
@@ -278,6 +298,124 @@ 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)
+{
+    g_assert(size <= 3);
+    tcg_gen_qemu_st_i64(source, tcg_addr, get_mem_index(s), MO_TE + size);
+}
+
+/*
+ * 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)
+{
+    TCGMemOp memop =  MO_TE + size;
+
+    g_assert(size <= 3);
+
+    if (is_signed) {
+        memop += MO_SIGN;
+    }
+
+    tcg_gen_qemu_ld_i64(dest, tcg_addr, get_mem_index(s), memop);
+
+    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)
+{
+    /* 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();
+
+    if (size < 4) {
+        switch (size) {
+        case 0:
+            tcg_gen_ld8u_i64(tmp, cpu_env, freg_offs);
+            break;
+        case 1:
+            tcg_gen_ld16u_i64(tmp, cpu_env, freg_offs);
+            break;
+        case 2:
+            tcg_gen_ld32u_i64(tmp, cpu_env, freg_offs);
+            break;
+        case 3:
+            tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+            break;
+        }
+        tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TE + size);
+    } else {
+        TCGv_i64 tcg_hiaddr = tcg_temp_new_i64();
+        tcg_gen_ld_i64(tmp, cpu_env, freg_offs);
+        tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TEQ);
+        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_st_i64(tmp, tcg_hiaddr, get_mem_index(s), MO_TEQ);
+        tcg_temp_free_i64(tcg_hiaddr);
+    }
+
+    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;
+
+    if (size < 4) {
+        TCGMemOp memop =  MO_TE + size;
+        tmphi = tcg_const_i64(0);
+        tcg_gen_qemu_ld_i64(tmplo, tcg_addr, get_mem_index(s), memop);
+    } else {
+        TCGv_i64 tcg_hiaddr;
+        tmphi = tcg_temp_new_i64();
+        tcg_hiaddr = tcg_temp_new_i64();
+
+        tcg_gen_qemu_ld_i64(tmplo, tcg_addr, get_mem_index(s), MO_TEQ);
+        tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8);
+        tcg_gen_qemu_ld_i64(tmphi, tcg_hiaddr, get_mem_index(s), MO_TEQ);
+        tcg_temp_free_i64(tcg_hiaddr);
+    }
+
+    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
+     * 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 +758,145 @@ static void disas_ld_lit(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Load/store pair (all forms) */
+/*
+ * C5.6.80 LDNP (Load Pair - non-temporal hint)
+ * C5.6.81 LDP (Load Pair - non vector)
+ * C5.6.82 LDPSW (Load Pair Signed Word - non vector)
+ * C5.6.176 STNP (Store Pair - non-temporal hint)
+ * C5.6.177 STP (Store Pair - non vector)
+ * C6.3.165 LDNP (Load Pair of SIMD&FP - non-temporal hint)
+ * C6.3.165 LDP (Load Pair of SIMD&FP)
+ * C6.3.284 STNP (Store Pair of SIMD&FP - non-temporal hint)
+ * C6.3.284 STP (Store Pair of SIMD&FP)
+ *
+ *  31 30 29   27  26  25 24   23  22 21   15 14   10 9    5 4    0
+ * +-----+-------+---+---+-------+---+-----------------------------+
+ * | opc | 1 0 1 | V | 0 | index | L |  imm7 |  Rt2  |  Rn  | Rt   |
+ * +-----+-------+---+---+-------+---+-------+-------+------+------+
+ *
+ * opc: LDP/STP/LDNP/STNP        00 -> 32 bit, 10 -> 64 bit
+ *      LDPSW                    01
+ *      LDP/STP/LDNP/STNP (SIMD) 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit
+ *   V: 0 -> GPR, 1 -> Vector
+ * idx: 00 -> signed offset with non-temporal hint, 01 -> post-index,
+ *      10 -> signed offset, 11 -> pre-index
+ *   L: 0 -> Store 1 -> Load
+ *
+ * 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 disas_ldst_pair(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, 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 index = extract32(insn, 23, 2);
+    bool is_vector = extract32(insn, 26, 1);
+    bool is_load = extract32(insn, 22, 1);
+    int opc = extract32(insn, 30, 2);
+
+    bool is_signed = false;
+    bool postindex = false;
+    bool wback = false;
+
+    TCGv_i64 tcg_addr; /* calculated address */
+    int size;
+
+    if (is_vector) {
+        if (opc == 3) {
+            unallocated_encoding(s);
+            return;
+        }
+        size = 2 + opc;
+    } else {
+        size = 2 + extract32(opc, 1, 1);
+        if (is_load) {
+            is_signed = opc & 1;
+        } else if (opc & 1) {
+            unallocated_encoding(s);
+            return;
+        }
+    }
+
+    switch (index) {
+    case 1: /* post-index */
+        postindex = true;
+        wback = true;
+        break;
+    case 0:
+        /* signed offset with "non-temporal" hint. Since we don't emulate
+         * caches we don't care about hints to the cache system about
+         * data access patterns, and handle this identically to plain
+         * signed offset.
+         */
+        if (is_signed) {
+            /* There is no non-temporal-hint version of LDPSW */
+            unallocated_encoding(s);
+            return;
+        }
+        postindex = false;
+        break;
+    case 2: /* signed offset, rn not updated */
+        postindex = false;
+        break;
+    case 3: /* pre-index */
+        postindex = false;
+        wback = true;
+        break;
+    }
+
+    offset <<= size;
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+
+    tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+    if (!postindex) {
+        tcg_gen_addi_i64(tcg_addr, tcg_addr, offset);
+    }
+
+    if (is_vector) {
+        if (is_load) {
+            do_fp_ld(s, rt, tcg_addr, size);
+        } else {
+            do_fp_st(s, rt, tcg_addr, size);
+        }
+    } else {
+        TCGv_i64 tcg_rt = cpu_reg(s, rt);
+        if (is_load) {
+            do_gpr_ld(s, tcg_rt, tcg_addr, size, is_signed, false);
+        } else {
+            do_gpr_st(s, tcg_rt, tcg_addr, size);
+        }
+    }
+    tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
+    if (is_vector) {
+        if (is_load) {
+            do_fp_ld(s, rt2, tcg_addr, size);
+        } else {
+            do_fp_st(s, rt2, tcg_addr, size);
+        }
+    } else {
+        TCGv_i64 tcg_rt2 = cpu_reg(s, rt2);
+        if (is_load) {
+            do_gpr_ld(s, tcg_rt2, tcg_addr, size, is_signed, false);
+        } 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);
+    }
 }
 
 /* Load/store register (all forms) */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 01/21] target-arm: A64: add support for ld/st pair

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +    TCGMemOp memop =  MO_TE + size;

Watch the double space (multiple occurrences).


> +    if (is_vector) {
> +        if (opc == 3) {
> +            unallocated_encoding(s);
> +            return;
> +        }

In the integer LDP description I see

  if L:opc<0> == '01' || opc == '11' then UnallocatedEncoding();

I think that opc == 3 check should be hoisted outside is_vector.


r~

Re: [Qemu-devel] [PATCH 01/21] target-arm: A64: add support for ld/st pair

[Peter Maydell]

On 19 December 2013 16:58, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> +    TCGMemOp memop =  MO_TE + size;
>
> Watch the double space (multiple occurrences).
>
>
>> +    if (is_vector) {
>> +        if (opc == 3) {
>> +            unallocated_encoding(s);
>> +            return;
>> +        }
>
> In the integer LDP description I see
>
>   if L:opc<0> == '01' || opc == '11' then UnallocatedEncoding();
>
> I think that opc == 3 check should be hoisted outside is_vector.

I thought I'd correctly rolled that into the opc&1 check
in the non-vector case but looking more closely you're
right. Hoisting the opc==3 out is going to be easiest.

thanks
-- PMM

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

[Peter Maydell]

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

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 | 89 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 88 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index e4ce038..3712a6d 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -899,10 +899,97 @@ 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 disas_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);
+            return;
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        if (opc == 3 && size > 1) {
+            unallocated_encoding(s);
+            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:
+        disas_ldst_reg_unsigned_imm(s, insn);
+        break;
+    default:
+        unallocated_encoding(s);
+        break;
+    }
 }
 
 /* AdvSIMD load/store multiple structures */
-- 
1.8.5

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

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +        if (size == 3 && opc == 2) {
> +            /* PRFM - prefetch */
> +            return;
> +        }
> +        if (opc == 3 && size > 1) {
> +            unallocated_encoding(s);
> +            return;
> +        }
> +        is_store = (opc == 0);
> +        is_signed = opc & (1<<1);
> +        is_extended = (size < 3) && (opc & 1);

I thought we'd discussed rearranging this bit of decoding to better match the
ARM?  In particular, opc = 2 && size = 2 should be unallocated.

And please no (1<<1).


r~

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

[Peter Maydell]

On 19 December 2013 17:46, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> +        if (size == 3 && opc == 2) {
>> +            /* PRFM - prefetch */
>> +            return;
>> +        }
>> +        if (opc == 3 && size > 1) {
>> +            unallocated_encoding(s);
>> +            return;
>> +        }
>> +        is_store = (opc == 0);
>> +        is_signed = opc & (1<<1);
>> +        is_extended = (size < 3) && (opc & 1);
>
> I thought we'd discussed rearranging this bit of decoding to better match the
> ARM?

Can't find anything in my email archive but I wouldn't be surprised
if I was just searching on the wrong keywords...

> In particular, opc = 2 && size = 2 should be unallocated.

This is LDRSW (immediate), not unallocated, isn't it?

I agree the decode logic isn't laid out the same as the ARM ARM,
but I'm pretty sure it's correct.

> And please no (1<<1).

Agreed.

-- PMM

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

[Richard Henderson]

On 12/20/2013 08:08 AM, Peter Maydell wrote:
>> In particular, opc = 2 && size = 2 should be unallocated.
> 
> This is LDRSW (immediate), not unallocated, isn't it?
> 
> I agree the decode logic isn't laid out the same as the ARM ARM,
> but I'm pretty sure it's correct.

Oops, typo: opc=3 && size=2.  Basically,


  if opc<1> == '0' then
    // store or zero-extending load
    memop = if opc<0> == '1' then MemOp_LOAD else MemOp_STORE;
    regsize = if size == '11' then 64 else 32;
    signed = FALSE;
  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();

this one  ----^


r~

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

[Peter Maydell]

On 20 December 2013 16:26, Richard Henderson <rth@twiddle.net> wrote:
> On 12/20/2013 08:08 AM, Peter Maydell wrote:
>>> In particular, opc = 2 && size = 2 should be unallocated.
>>
>> This is LDRSW (immediate), not unallocated, isn't it?
>>
>> I agree the decode logic isn't laid out the same as the ARM ARM,
>> but I'm pretty sure it's correct.
>
> Oops, typo: opc=3 && size=2.  Basically,
>
>
>   if opc<1> == '0' then
>     // store or zero-extending load
>     memop = if opc<0> == '1' then MemOp_LOAD else MemOp_STORE;
>     regsize = if size == '11' then 64 else 32;
>     signed = FALSE;
>   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();
>
> this one  ----^

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

That is caught by 'if (opc == 3 && size > 1)'.

thanks
-- PMM

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

[Richard Henderson]

On 12/20/2013 08:29 AM, Peter Maydell wrote:
> On 20 December 2013 16:26, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/20/2013 08:08 AM, Peter Maydell wrote:
>>>> In particular, opc = 2 && size = 2 should be unallocated.
>>>
>>> This is LDRSW (immediate), not unallocated, isn't it?
>>>
>>> I agree the decode logic isn't laid out the same as the ARM ARM,
>>> but I'm pretty sure it's correct.
>>
>> Oops, typo: opc=3 && size=2.  Basically,
>>
>>
>>   if opc<1> == '0' then
>>     // store or zero-extending load
>>     memop = if opc<0> == '1' then MemOp_LOAD else MemOp_STORE;
>>     regsize = if size == '11' then 64 else 32;
>>     signed = FALSE;
>>   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();
>>
>> this one  ----^
> 
> +        if (size == 3 && opc == 2) {
> +            /* PRFM - prefetch */
> +            return;
> +        }
> +        if (opc == 3 && size > 1) {
> +            unallocated_encoding(s);
> +            return;
> +        }
> +        is_store = (opc == 0);
> +        is_signed = opc & (1<<1);
> +        is_extended = (size < 3) && (opc & 1);
> 
> That is caught by 'if (opc == 3 && size > 1)'.

Ah, right.  In which case, patches 2, 3, 4 get

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


r~

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

[Peter Maydell]

On 20 December 2013 16:44, Richard Henderson <rth@twiddle.net> wrote:
> On 12/20/2013 08:29 AM, Peter Maydell wrote:
>> On 20 December 2013 16:26, Richard Henderson <rth@twiddle.net> wrote:
>>> On 12/20/2013 08:08 AM, Peter Maydell wrote:
>>>>> In particular, opc = 2 && size = 2 should be unallocated.
>>>>
>>>> This is LDRSW (immediate), not unallocated, isn't it?
>>>>
>>>> I agree the decode logic isn't laid out the same as the ARM ARM,
>>>> but I'm pretty sure it's correct.
>>>
>>> Oops, typo: opc=3 && size=2.  Basically,
>>>
>>>
>>>   if opc<1> == '0' then
>>>     // store or zero-extending load
>>>     memop = if opc<0> == '1' then MemOp_LOAD else MemOp_STORE;
>>>     regsize = if size == '11' then 64 else 32;
>>>     signed = FALSE;
>>>   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();
>>>
>>> this one  ----^
>>
>> +        if (size == 3 && opc == 2) {
>> +            /* PRFM - prefetch */
>> +            return;
>> +        }
>> +        if (opc == 3 && size > 1) {
>> +            unallocated_encoding(s);
>> +            return;
>> +        }
>> +        is_store = (opc == 0);
>> +        is_signed = opc & (1<<1);
>> +        is_extended = (size < 3) && (opc & 1);
>>
>> That is caught by 'if (opc == 3 && size > 1)'.
>
> Ah, right.  In which case, patches 2, 3, 4 get
>
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks. I've fixed up the 1<<1 &c, so we now read

+    if (is_vector) {
+        size |= (opc & 2) << 1;
+        if (size > 4) {
+            unallocated_encoding(s);
+            return;
+        }
+        is_store = !extract32(opc, 0, 1);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        if (opc == 3 && size > 1) {
+            unallocated_encoding(s);
+            return;
+        }
+        is_store = (opc == 0);
+        is_signed = extract32(opc, 1, 1);
+        is_extended = (size < 3) && extract32(opc, 0, 1);
+    }

Were you planning to review patches 12 ("Remove
ARMCPU/CPUARMState from cpregs APIs used by decoder")
and 19 ("Widen exclusive-access support struct fields
to 64 bits") ? I think those are the only two patches in
this set which I don't either have review comments to fix
or a reviewed-by from you now.

thanks
-- PMM

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

[Richard Henderson]

On 12/20/2013 08:52 AM, Peter Maydell wrote:
> Were you planning to review patches 12 ("Remove
> ARMCPU/CPUARMState from cpregs APIs used by decoder")
> and 19 ("Widen exclusive-access support struct fields
> to 64 bits") ? I think those are the only two patches in
> this set which I don't either have review comments to fix
> or a reviewed-by from you now.

I skipped over those because they're pure target-arm internals, and not really
translator related.  But I could have a look later today if you like.


r~

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

[Peter Maydell]

On 20 December 2013 16:57, Richard Henderson <rth@twiddle.net> wrote:
> On 12/20/2013 08:52 AM, Peter Maydell wrote:
>> Were you planning to review patches 12 ("Remove
>> ARMCPU/CPUARMState from cpregs APIs used by decoder")
>> and 19 ("Widen exclusive-access support struct fields
>> to 64 bits") ? I think those are the only two patches in
>> this set which I don't either have review comments to fix
>> or a reviewed-by from you now.
>
> I skipped over those because they're pure target-arm internals, and not really
> translator related.  But I could have a look later today if you like.

The 'widen exclusive access fields' one could use some eyeballs
to check I didn't mess up when I reworked the generated code from
looking at two 32 bit fields to one 64 bit field. You're right that the
other one is mostly target-arm internals though.

Mostly I just wanted to know if I should be waiting for more
review before sending out a respin :-)

thanks
-- PMM

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

[Peter Maydell]

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

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 | 144 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3712a6d..742c714 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -402,6 +402,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
@@ -900,6 +948,96 @@ 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 disas_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);
+            return;
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        if (opc == 3 && size > 1) {
+            unallocated_encoding(s);
+            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)
  *
  * 31 30 29   27  26 25 24 23 22 21        10 9     5
@@ -981,7 +1119,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) {
+            disas_ldst_reg_roffset(s, insn);
+        } else {
+            unsupported_encoding(s, insn);
+        }
         break;
     case 1:
         disas_ldst_reg_unsigned_imm(s, insn);
-- 
1.8.5

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

[Peter Maydell]

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

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 | 125 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 742c714..1b34a98 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -948,6 +948,110 @@ 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 disas_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);
+            return;
+        }
+        is_store = ((opc & 1) == 0);
+    } else {
+        if (size == 3 && opc == 2) {
+            /* PRFM - prefetch */
+            return;
+        }
+        if (opc == 3 && size > 1) {
+            unallocated_encoding(s);
+            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);
+    }
+}
+
+/*
  * 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
@@ -1114,6 +1218,25 @@ static void disas_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 */
+        disas_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)
 {
@@ -1122,7 +1245,7 @@ static void disas_ldst_reg(DisasContext *s, uint32_t insn)
         if (extract32(insn, 21, 1) == 1 && extract32(insn, 10, 2) == 2) {
             disas_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 05/21] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 target-arm/translate-a64.c | 292 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 286 insertions(+), 6 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 1b34a98..7227bb0 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -297,6 +297,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
  */
@@ -1326,10 +1422,68 @@ 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);
+        return;
+    }
+
+    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
@@ -1787,16 +1941,142 @@ static void disas_logic_reg(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Add/subtract (extended register) */
+/*
+ * 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 disas_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_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) */
+/*
+ * 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 disas_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) */
-- 
1.8.5

[Qemu-devel] [PATCH 06/21] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 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 7227bb0..96ae4e1 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1642,10 +1642,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

[Qemu-devel] [PATCH 07/21] 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/translate-a64.c | 92 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 90 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 96ae4e1..97c9f0b 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2126,10 +2126,98 @@ static void disas_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 sf = 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);
+        return;
+    }
+
+    if (is_high) {
+        TCGv_i64 low_bits = tcg_temp_new_i64(); /* low bits discarded */
+        TCGv_i64 tcg_rd = cpu_reg(s, rd);
+        TCGv_i64 tcg_rn = cpu_reg(s, rn);
+        TCGv_i64 tcg_rm = cpu_reg(s, rm);
+
+        if (is_signed) {
+            tcg_gen_muls2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+        } else {
+            tcg_gen_mulu2_i64(low_bits, tcg_rd, tcg_rn, tcg_rm);
+        }
+
+        tcg_temp_free_i64(low_bits);
+        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 (!sf) {
+        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 07/21] target-arm: A64: add support for 3 src data proc insns

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +    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);
> +    }

Perhaps worth noticing the RA=XZR special case for the MUL alias?

Otherwise,

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


r~

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

[Peter Maydell]

On 19 December 2013 19:29, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> +    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);
>> +    }
>
> Perhaps worth noticing the RA=XZR special case for the MUL alias?

Yeah, makes sense: have adjusted to:
+    if (ra == 31) {
+        /* We special-case rA == XZR as it is the standard MUL alias */
+        tcg_gen_mul_i64(cpu_reg(s, rd), tcg_op1, tcg_op2);
+    } else {
+        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);
+        }
+    }

thanks
-- PMM

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

[Richard Henderson]

On 12/20/2013 05:18 AM, Peter Maydell wrote:
> On 19 December 2013 19:29, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>>> +    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);
>>> +    }
>>
>> Perhaps worth noticing the RA=XZR special case for the MUL alias?
> 
> Yeah, makes sense: have adjusted to:
> +    if (ra == 31) {
> +        /* We special-case rA == XZR as it is the standard MUL alias */
> +        tcg_gen_mul_i64(cpu_reg(s, rd), tcg_op1, tcg_op2);
> +    } else {

You need to handle (or dismiss) is_sub.

Either (ra == 31 && !is_sub) or

           if (is_sub) {
               tcg_gen_neg_i64(tcg_rd, tcg_rd);
           }

with tcg_rd pre-loaded along with tcg_op1 and tcg_op2.

That said, unlike MUL I don't expect MNEG to be common at all.

> +        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);
> +        }
> +    }


r~

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

[Peter Maydell]

On 20 December 2013 14:10, Richard Henderson <rth@twiddle.net> wrote:
> On 12/20/2013 05:18 AM, Peter Maydell wrote:
>> On 19 December 2013 19:29, Richard Henderson <rth@twiddle.net> wrote:
>>> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>>>> +    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);
>>>> +    }
>>>
>>> Perhaps worth noticing the RA=XZR special case for the MUL alias?
>>
>> Yeah, makes sense: have adjusted to:
>> +    if (ra == 31) {
>> +        /* We special-case rA == XZR as it is the standard MUL alias */
>> +        tcg_gen_mul_i64(cpu_reg(s, rd), tcg_op1, tcg_op2);
>> +    } else {
>
> You need to handle (or dismiss) is_sub.
>
> Either (ra == 31 && !is_sub) or
>
>            if (is_sub) {
>                tcg_gen_neg_i64(tcg_rd, tcg_rd);
>            }
>
> with tcg_rd pre-loaded along with tcg_op1 and tcg_op2.
>
> That said, unlike MUL I don't expect MNEG to be common at all.

Oops. Yes, let's just have (ra == 31 && !is_sub).

thanks
-- PMM

[Qemu-devel] [PATCH 08/21] 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>
Reviewed-by: Richard Henderson <rth@twiddle.net>
---
 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 97c9f0b..4cda99f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -806,10 +806,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

[Qemu-devel] [PATCH 09/21] target-arm: A64: Add decoder skeleton for FP instructions

[Peter Maydell]

Add a top level decoder skeleton for FP instructions.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 170 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 169 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 4cda99f..98a11cc 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2663,10 +2663,178 @@ static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
     }
 }
 
+/* C3.6.22 Floating point compare
+ *   31  30  29 28       24 23  22  21 20  16 15 14 13  10    9    5 4     0
+ * +---+---+---+-----------+------+---+------+-----+---------+------+-------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 |  Rm  | op  | 1 0 0 0 |  Rn  |  op2  |
+ * +---+---+---+-----------+------+---+------+-----+---------+------+-------+
+ */
+static void disas_fp_compare(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.23 Floating point conditional compare
+ *   31  30  29 28       24 23  22  21 20  16 15  12 11 10 9    5  4   3    0
+ * +---+---+---+-----------+------+---+------+------+-----+------+----+------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 |  Rm  | cond | 0 1 |  Rn  | op | nzcv |
+ * +---+---+---+-----------+------+---+------+------+-----+------+----+------+
+ */
+static void disas_fp_ccomp(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.24 Floating point conditional select
+ *   31  30  29 28       24 23  22  21 20  16 15  12 11 10 9    5 4    0
+ * +---+---+---+-----------+------+---+------+------+-----+------+------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 |  Rm  | cond | 1 1 |  Rn  |  Rd  |
+ * +---+---+---+-----------+------+---+------+------+-----+------+------+
+ */
+static void disas_fp_csel(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.25 Floating point data-processing (1 source)
+ *   31  30  29 28       24 23  22  21 20    15 14       10 9    5 4    0
+ * +---+---+---+-----------+------+---+--------+-----------+------+------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 | opcode | 1 0 0 0 0 |  Rn  |  Rd  |
+ * +---+---+---+-----------+------+---+--------+-----------+------+------+
+ */
+static void disas_fp_1src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.26 Floating point data-processing (2 source)
+ *   31  30  29 28       24 23  22  21 20  16 15    12 11 10 9    5 4    0
+ * +---+---+---+-----------+------+---+------+--------+-----+------+------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 |  Rm  | opcode | 1 0 |  Rn  |  Rd  |
+ * +---+---+---+-----------+------+---+------+--------+-----+------+------+
+ */
+static void disas_fp_2src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.27 Floating point data-processing (3 source)
+ *   31  30  29 28       24 23  22  21  20  16  15  14  10 9    5 4    0
+ * +---+---+---+-----------+------+----+------+----+------+------+------+
+ * | M | 0 | S | 1 1 1 1 1 | type | o1 |  Rm  | o0 |  Ra  |  Rn  |  Rd  |
+ * +---+---+---+-----------+------+----+------+----+------+------+------+
+ */
+static void disas_fp_3src(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.28 Floating point immediate
+ *   31  30  29 28       24 23  22  21 20        13 12   10 9    5 4    0
+ * +---+---+---+-----------+------+---+------------+-------+------+------+
+ * | M | 0 | S | 1 1 1 1 0 | type | 1 |    imm8    | 1 0 0 | imm5 |  Rd  |
+ * +---+---+---+-----------+------+---+------------+-------+------+------+
+ */
+static void disas_fp_imm(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.29 Floating point <-> fixed point conversions
+ *   31   30  29 28       24 23  22  21 20   19 18    16 15   10 9    5 4    0
+ * +----+---+---+-----------+------+---+-------+--------+-------+------+------+
+ * | sf | 0 | S | 1 1 1 1 0 | type | 0 | rmode | opcode | scale |  Rn  |  Rd  |
+ * +----+---+---+-----------+------+---+-------+--------+-------+------+------+
+ */
+static void disas_fp_fixed_conv(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* C3.6.30 Floating point <-> integer conversions
+ *   31   30  29 28       24 23  22  21 20   19 18 16 15         10 9  5 4  0
+ * +----+---+---+-----------+------+---+-------+-----+-------------+----+----+
+ * | sf | 0 | S | 1 1 1 1 0 | type | 0 | rmode | opc | 0 0 0 0 0 0 | Rn | Rd |
+ * +----+---+---+-----------+------+---+-------+-----+-------------+----+----+
+ */
+static void disas_fp_int_conv(DisasContext *s, uint32_t insn)
+{
+    unsupported_encoding(s, insn);
+}
+
+/* FP-specific subcases of table C3-6 (SIMD and FP data processing)
+ *   31  30  29 28     25 24                          0
+ * +---+---+---+---------+-----------------------------+
+ * |   | 0 |   | 1 1 1 1 |                             |
+ * +---+---+---+---------+-----------------------------+
+ */
+static void disas_data_proc_fp(DisasContext *s, uint32_t insn)
+{
+    if (extract32(insn, 24, 1)) {
+        /* Floating point data-processing (3 source) */
+        disas_fp_3src(s, insn);
+    } else if (extract32(insn, 21, 1) == 0) {
+        /* Floating point to fixed point conversions */
+        disas_fp_fixed_conv(s, insn);
+    } else {
+        switch (extract32(insn, 10, 2)) {
+        case 1:
+            /* Floating point conditional compare */
+            disas_fp_ccomp(s, insn);
+            break;
+        case 2:
+            /* Floating point data-processing (2 source) */
+            disas_fp_2src(s, insn);
+            break;
+        case 3:
+            /* Floating point conditional select */
+            disas_fp_csel(s, insn);
+            break;
+        case 0:
+            switch (ctz32(extract32(insn, 12, 4))) {
+            case 0: /* [15:12] == xxx1 */
+                /* Floating point immediate */
+                disas_fp_imm(s, insn);
+                break;
+            case 1: /* [15:12] == xx10 */
+                /* Floating point compare */
+                disas_fp_compare(s, insn);
+                break;
+            case 2: /* [15:12] == x100 */
+                /* Floating point data-processing (1 source) */
+                disas_fp_1src(s, insn);
+                break;
+            case 3: /* [15:12] == 1000 */
+                unallocated_encoding(s);
+                break;
+            default: /* [15:12] == 0000 */
+                /* Floating point <-> integer conversions */
+                disas_fp_int_conv(s, insn);
+                break;
+            }
+            break;
+        }
+    }
+}
+
+static void disas_data_proc_simd(DisasContext *s, uint32_t insn)
+{
+    /* Note that this is called with all non-FP cases from
+     * table C3-6 so it must UNDEF for entries not specifically
+     * allocated to instructions in that table.
+     */
+    unsupported_encoding(s, insn);
+}
+
 /* C3.6 Data processing - SIMD and floating point */
 static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    if (extract32(insn, 28, 1) == 1 && extract32(insn, 30, 1) == 0) {
+        disas_data_proc_fp(s, insn);
+    } else {
+        /* SIMD, including crypto */
+        disas_data_proc_simd(s, insn);
+    }
 }
 
 /* C3.1 A64 instruction index by encoding */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 09/21] target-arm: A64: Add decoder skeleton for FP instructions

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> Add a top level decoder skeleton for FP instructions.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 170 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 169 insertions(+), 1 deletion(-)

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


r~

[Qemu-devel] [PATCH 10/21] target-arm: A64: implement FMOV

[Peter Maydell]

Implement FMOV, ie non-converting moves between general purpose
registers and floating point registers. This is a subtype of
the floating point <-> integer instruction class.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 86 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 85 insertions(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 98a11cc..add8cc2 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2751,6 +2751,63 @@ static void disas_fp_fixed_conv(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
+static void handle_fmov(DisasContext *s, int rd, int rn, int type, bool itof)
+{
+    /* FMOV: gpr to or from float, double, or top half of quad fp reg,
+     * without conversion.
+     */
+
+    if (itof) {
+        int freg_offs = offsetof(CPUARMState, vfp.regs[rd * 2]);
+        TCGv_i64 tcg_rn = cpu_reg(s, rn);
+
+        switch (type) {
+        case 0:
+        {
+            /* 32 bit */
+            TCGv_i64 tmp = tcg_temp_new_i64();
+            tcg_gen_ext32u_i64(tmp, tcg_rn);
+            tcg_gen_st_i64(tmp, cpu_env, freg_offs);
+            tcg_gen_movi_i64(tmp, 0);
+            tcg_gen_st_i64(tmp, cpu_env, freg_offs + sizeof(float64));
+            tcg_temp_free_i64(tmp);
+            break;
+        }
+        case 1:
+        {
+            /* 64 bit */
+            TCGv_i64 tmp = tcg_const_i64(0);
+            tcg_gen_st_i64(tcg_rn, cpu_env, freg_offs);
+            tcg_gen_st_i64(tmp, cpu_env, freg_offs + sizeof(float64));
+            tcg_temp_free_i64(tmp);
+            break;
+        }
+        case 2:
+            /* 64 bit to top half. */
+            tcg_gen_st_i64(tcg_rn, cpu_env, freg_offs + sizeof(float64));
+            break;
+        }
+    } else {
+        int freg_offs = offsetof(CPUARMState, vfp.regs[rn * 2]);
+        TCGv_i64 tcg_rd = cpu_reg(s, rd);
+
+        switch (type) {
+        case 0:
+            /* 32 bit */
+            tcg_gen_ld32u_i64(tcg_rd, cpu_env, freg_offs);
+            break;
+        case 2:
+            /* 64 bits from top half */
+            freg_offs += sizeof(float64);
+            /* fall through */
+        case 1:
+            /* 64 bit */
+            tcg_gen_ld_i64(tcg_rd, cpu_env, freg_offs);
+            break;
+        }
+    }
+}
+
 /* C3.6.30 Floating point <-> integer conversions
  *   31   30  29 28       24 23  22  21 20   19 18 16 15         10 9  5 4  0
  * +----+---+---+-----------+------+---+-------+-----+-------------+----+----+
@@ -2759,7 +2816,34 @@ static void disas_fp_fixed_conv(DisasContext *s, uint32_t insn)
  */
 static void disas_fp_int_conv(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rd = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int opcode = extract32(insn, 16, 3);
+    int rmode = extract32(insn, 19, 2);
+    int type = extract32(insn, 22, 2);
+    bool sbit = extract32(insn, 29, 1);
+    bool sf = extract32(insn, 31, 1);
+
+    if (!sbit && (rmode < 2) && (opcode > 5)) {
+        /* FMOV */
+        bool itof = opcode & 1;
+
+        switch (sf << 3 | type << 1 | rmode) {
+        case 0x0: /* 32 bit */
+        case 0xa: /* 64 bit */
+        case 0xd: /* 64 bit to top half of quad */
+            break;
+        default:
+            /* all other sf/type/rmode combinations are invalid */
+            unallocated_encoding(s);
+            break;
+        }
+
+        handle_fmov(s, rd, rn, type, itof);
+    } else {
+        /* actual FP conversions */
+        unsupported_encoding(s, insn);
+    }
 }
 
 /* FP-specific subcases of table C3-6 (SIMD and FP data processing)
-- 
1.8.5

Re: [Qemu-devel] [PATCH 10/21] target-arm: A64: implement FMOV

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> Implement FMOV, ie non-converting moves between general purpose
> registers and floating point registers. This is a subtype of
> the floating point <-> integer instruction class.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 86 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 85 insertions(+), 1 deletion(-)

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


r~

[Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

Update the generic cpreg support code to also handle AArch64:
AArch64-visible registers coexist in the same hash table with
AArch32-visible ones, with a bit in the hash key distinguishing
them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h        | 59 ++++++++++++++++++++++++++++++++++++++-----
 target-arm/helper.c     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
 target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 7 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 56ed591..901f882 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq);
  *    or via MRRC/MCRR?)
  * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field.
  * (In this case crn and opc2 should be zero.)
+ * For AArch64, there is no 32/64 bit size distinction;
+ * instead all registers have a 2 bit op0, 3 bit op1 and op2,
+ * and 4 bit CRn and CRm. The encoding patterns are chosen
+ * to be easy to convert to and from the KVM encodings, and also
+ * so that the hashtable can contain both AArch32 and AArch64
+ * registers (to allow for interprocessing where we might run
+ * 32 bit code on a 64 bit core).
  */
+/* This bit is private to our hashtable cpreg; in KVM register
+ * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64
+ * in the upper bits of the 64 bit ID.
+ */
+#define CP_REG_AA64_SHIFT 28
+#define CP_REG_AA64_MASK (1 << CP_REG_AA64_SHIFT)
+
 #define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2)   \
     (((cp) << 16) | ((is64) << 15) | ((crn) << 11) |    \
      ((crm) << 7) | ((opc1) << 3) | (opc2))
 
+#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \
+    (CP_REG_AA64_MASK |                                 \
+     ((cp) << CP_REG_ARM_COPROC_SHIFT) |                \
+     ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) |         \
+     ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) |         \
+     ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) |         \
+     ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) |         \
+     ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT))
+
 /* Convert a full 64 bit KVM register ID to the truncated 32 bit
  * version used as a key for the coprocessor register hashtable
  */
 static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
 {
     uint32_t cpregid = kvmid;
-    if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
+    if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) {
+        cpregid |= CP_REG_AA64_MASK;
+    } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
         cpregid |= (1 << 15);
     }
     return cpregid;
@@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
  */
 static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 {
-    uint64_t kvmid = cpregid & ~(1 << 15);
-    if (cpregid & (1 << 15)) {
-        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
+    uint64_t kvmid;
+
+    if (cpregid & CP_REG_AA64_MASK) {
+        kvmid = cpregid & ~CP_REG_AA64_MASK;
+        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64;
     } else {
-        kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
+        kvmid = cpregid & ~(1 << 15);
+        if (cpregid & (1 << 15)) {
+            kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
+        } else {
+            kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
+        }
     }
     return kvmid;
 }
@@ -626,13 +658,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 #define ARM_CP_OVERRIDE 16
 #define ARM_CP_NO_MIGRATE 32
 #define ARM_CP_IO 64
+#define ARM_CP_AA64 128
 #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
 #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
 #define ARM_LAST_SPECIAL ARM_CP_WFI
 /* Used only as a terminator for ARMCPRegInfo lists */
 #define ARM_CP_SENTINEL 0xffff
 /* Mask of only the flag bits in a type field */
-#define ARM_CP_FLAG_MASK 0x7f
+#define ARM_CP_FLAG_MASK 0xff
 
 /* Return true if cptype is a valid type field. This is used to try to
  * catch errors where the sentinel has been accidentally left off the end
@@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype)
  * (ie anything visible in PL2 is visible in S-PL1, some things are only
  * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the
  * terminology a little and call this PL3.
+ * In AArch64 things are somewhat simpler as the PLx bits line up exactly
+ * with the ELx exception levels.
  *
  * If access permissions for a register are more complex than can be
  * described with these bits, then use a laxer set of restrictions, and
@@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype)
 
 static inline int arm_current_pl(CPUARMState *env)
 {
+    if (env->aarch64) {
+        return extract32(env->pstate, 2, 2);
+    }
+
     if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) {
         return 0;
     }
@@ -713,10 +752,18 @@ struct ARMCPRegInfo {
      * then behave differently on read/write if necessary.
      * For 64 bit registers, only crm and opc1 are relevant; crn and opc2
      * must both be zero.
+     * For AArch64-visible registers, opc0 is also used.
+     * Since there are no "coprocessors" in AArch64, cp is purely used as a
+     * way to distinguish (for KVM's benefit) guest-visible system registers
+     * from demuxed ones provided to preserve the "no side effects on
+     * KVM register read/write from QEMU" semantics. cp==0x13 is guest
+     * visible (to match KVM's encoding); cp==0 will be converted to
+     * cp==0x13 when the ARMCPRegInfo is registered, for convenience.
      */
     uint8_t cp;
     uint8_t crn;
     uint8_t crm;
+    uint8_t opc0;
     uint8_t opc1;
     uint8_t opc2;
     /* Register type: ARM_CP_* bits/values */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6ebd7dc..975a762 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
      * At least one of the original and the second definition should
      * include ARM_CP_OVERRIDE in its type bits -- this is just a guard
      * against accidental use.
+     *
+     * ARM_CP_AA64 is set in the type field to define a register to
+     * be visible when in AArch64 state. In this case r->opc0 may also
+     * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't
+     * be wildcarded.
      */
     int crm, opc1, opc2;
     int crmmin = (r->crm == CP_ANY) ? 0 : r->crm;
@@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
     int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2;
     /* 64 bit registers have only CRm and Opc1 fields */
     assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn)));
+    /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */
+    assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64))
+           != (ARM_CP_64BIT|ARM_CP_AA64));
+    /* op0 only exists in the AArch64 encodings */
+    assert((r->type & ARM_CP_AA64) || (r->opc0 == 0));
+    /* The AArch64 pseudocode CheckSystemAccess() specifies that op1
+     * encodes a minimum access level for the register. We roll this
+     * runtime check into our general permission check code, so check
+     * here that the reginfo's specified permissions are strict enough
+     * to encompass the generic architectural permission check.
+     */
+    if (r->type & ARM_CP_AA64) {
+        int mask = 0;
+        switch (r->opc1) {
+        case 0: case 1: case 2:
+            /* min_EL EL1 */
+            mask = PL1_RW;
+            break;
+        case 3:
+            /* min_EL EL0 */
+            mask = PL0_RW;
+            break;
+        case 4:
+            /* min_EL EL2 */
+            mask = PL2_RW;
+            break;
+        case 5:
+            /* unallocated encoding, so not possible */
+            assert(false);
+            break;
+        case 6:
+            /* min_EL EL3 */
+            mask = PL3_RW;
+            break;
+        case 7:
+            /* min_EL EL1, secure mode only (we don't check the latter) */
+            mask = PL1_RW;
+            break;
+        default:
+            /* broken reginfo with out of range opc1 */
+            assert(false);
+            break;
+        }
+        /* assert our permissions are not too lax (stricter is fine) */
+        assert((r->access & ~mask) == 0);
+    }
+
     /* Check that the register definition has enough info to handle
      * reads and writes if they are permitted.
      */
@@ -1980,7 +2032,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
                 uint32_t *key = g_new(uint32_t, 1);
                 ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo));
                 int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0;
-                *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
+                if (r->type & ARM_CP_AA64) {
+                    /* To allow abbreviation of ARMCPRegInfo
+                     * definitions, we treat cp == 0 as equivalent to
+                     * the value for "standard guest-visible sysreg".
+                     */
+                    if (r->cp == 0) {
+                        r2->cp = CP_REG_ARM64_SYSREG_CP;
+                    }
+                    *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm,
+                                              r->opc0, opc1, opc2);
+                } else {
+                    *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
+                }
                 if (opaque) {
                     r2->opaque = opaque;
                 }
diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h
index 2bba0bd..d006146 100644
--- a/target-arm/kvm-consts.h
+++ b/target-arm/kvm-consts.h
@@ -29,12 +29,14 @@
 #define CP_REG_SIZE_U32        0x0020000000000000ULL
 #define CP_REG_SIZE_U64        0x0030000000000000ULL
 #define CP_REG_ARM             0x4000000000000000ULL
+#define CP_REG_ARCH_MASK       0xff00000000000000ULL
 
 MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT)
 MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK)
 MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32)
 MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64)
 MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM)
+MISMATCH_CHECK(CP_REG_ARCH_MASK, KVM_REG_ARCH_MASK)
 
 #define PSCI_FN_BASE 0x95c1ba5e
 #define PSCI_FN(n) (PSCI_FN_BASE + (n))
@@ -59,6 +61,41 @@ MISMATCH_CHECK(PSCI_FN_MIGRATE, KVM_PSCI_FN_MIGRATE)
 MISMATCH_CHECK(QEMU_KVM_ARM_TARGET_CORTEX_A15, KVM_ARM_TARGET_CORTEX_A15)
 #endif
 
+#define CP_REG_ARM64                   0x6000000000000000ULL
+#define CP_REG_ARM_COPROC_MASK         0x000000000FFF0000
+#define CP_REG_ARM_COPROC_SHIFT        16
+#define CP_REG_ARM64_SYSREG            (0x0013 << CP_REG_ARM_COPROC_SHIFT)
+#define CP_REG_ARM64_SYSREG_OP0_MASK   0x000000000000c000
+#define CP_REG_ARM64_SYSREG_OP0_SHIFT  14
+#define CP_REG_ARM64_SYSREG_OP1_MASK   0x0000000000003800
+#define CP_REG_ARM64_SYSREG_OP1_SHIFT  11
+#define CP_REG_ARM64_SYSREG_CRN_MASK   0x0000000000000780
+#define CP_REG_ARM64_SYSREG_CRN_SHIFT  7
+#define CP_REG_ARM64_SYSREG_CRM_MASK   0x0000000000000078
+#define CP_REG_ARM64_SYSREG_CRM_SHIFT  3
+#define CP_REG_ARM64_SYSREG_OP2_MASK   0x0000000000000007
+#define CP_REG_ARM64_SYSREG_OP2_SHIFT  0
+
+/* No kernel define but it's useful to QEMU */
+#define CP_REG_ARM64_SYSREG_CP (CP_REG_ARM64_SYSREG >> CP_REG_ARM_COPROC_SHIFT)
+
+#ifdef TARGET_AARCH64
+MISMATCH_CHECK(CP_REG_ARM64, KVM_REG_ARM64)
+MISMATCH_CHECK(CP_REG_ARM_COPROC_MASK, KVM_REG_ARM_COPROC_MASK)
+MISMATCH_CHECK(CP_REG_ARM_COPROC_SHIFT, KVM_REG_ARM_COPROC_SHIFT)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG, KVM_REG_ARM64_SYSREG)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK)
+MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT)
+#endif
+
 #undef MISMATCH_CHECK
 
 #endif
-- 
1.8.5

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Update the generic cpreg support code to also handle AArch64:
> AArch64-visible registers coexist in the same hash table with
> AArch32-visible ones, with a bit in the hash key distinguishing
> them.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h        | 59 ++++++++++++++++++++++++++++++++++++++-----
>  target-arm/helper.c     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++
>  3 files changed, 155 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 56ed591..901f882 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq);
>   *    or via MRRC/MCRR?)
>   * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field.
>   * (In this case crn and opc2 should be zero.)
> + * For AArch64, there is no 32/64 bit size distinction;
> + * instead all registers have a 2 bit op0, 3 bit op1 and op2,
> + * and 4 bit CRn and CRm. The encoding patterns are chosen
> + * to be easy to convert to and from the KVM encodings, and also
> + * so that the hashtable can contain both AArch32 and AArch64
> + * registers (to allow for interprocessing where we might run
> + * 32 bit code on a 64 bit core).
>   */
> +/* This bit is private to our hashtable cpreg; in KVM register
> + * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64
> + * in the upper bits of the 64 bit ID.
> + */
> +#define CP_REG_AA64_SHIFT 28
> +#define CP_REG_AA64_MASK (1 << CP_REG_AA64_SHIFT)
> +
>  #define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2)   \
>      (((cp) << 16) | ((is64) << 15) | ((crn) << 11) |    \
>       ((crm) << 7) | ((opc1) << 3) | (opc2))
>
> +#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \
> +    (CP_REG_AA64_MASK |                                 \
> +     ((cp) << CP_REG_ARM_COPROC_SHIFT) |                \
> +     ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) |         \
> +     ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) |         \
> +     ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) |         \
> +     ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) |         \
> +     ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT))
> +
>  /* Convert a full 64 bit KVM register ID to the truncated 32 bit
>   * version used as a key for the coprocessor register hashtable
>   */
>  static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
>  {
>      uint32_t cpregid = kvmid;
> -    if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
> +    if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) {
> +        cpregid |= CP_REG_AA64_MASK;
> +    } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
>          cpregid |= (1 << 15);
>      }
>      return cpregid;
> @@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
>   */
>  static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  {
> -    uint64_t kvmid = cpregid & ~(1 << 15);
> -    if (cpregid & (1 << 15)) {
> -        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
> +    uint64_t kvmid;
> +
> +    if (cpregid & CP_REG_AA64_MASK) {
> +        kvmid = cpregid & ~CP_REG_AA64_MASK;
> +        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64;
>      } else {
> -        kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
> +        kvmid = cpregid & ~(1 << 15);
> +        if (cpregid & (1 << 15)) {
> +            kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
> +        } else {
> +            kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
> +        }
>      }
>      return kvmid;
>  }
> @@ -626,13 +658,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  #define ARM_CP_OVERRIDE 16
>  #define ARM_CP_NO_MIGRATE 32
>  #define ARM_CP_IO 64
> +#define ARM_CP_AA64 128
>  #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
>  #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
>  #define ARM_LAST_SPECIAL ARM_CP_WFI
>  /* Used only as a terminator for ARMCPRegInfo lists */
>  #define ARM_CP_SENTINEL 0xffff
>  /* Mask of only the flag bits in a type field */
> -#define ARM_CP_FLAG_MASK 0x7f
> +#define ARM_CP_FLAG_MASK 0xff
>
>  /* Return true if cptype is a valid type field. This is used to try to
>   * catch errors where the sentinel has been accidentally left off the end
> @@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype)
>   * (ie anything visible in PL2 is visible in S-PL1, some things are only
>   * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the
>   * terminology a little and call this PL3.
> + * In AArch64 things are somewhat simpler as the PLx bits line up exactly
> + * with the ELx exception levels.
>   *
>   * If access permissions for a register are more complex than can be
>   * described with these bits, then use a laxer set of restrictions, and
> @@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype)
>
>  static inline int arm_current_pl(CPUARMState *env)
>  {
> +    if (env->aarch64) {
> +        return extract32(env->pstate, 2, 2);
> +    }
> +
>      if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) {
>          return 0;
>      }
> @@ -713,10 +752,18 @@ struct ARMCPRegInfo {
>       * then behave differently on read/write if necessary.
>       * For 64 bit registers, only crm and opc1 are relevant; crn and opc2
>       * must both be zero.
> +     * For AArch64-visible registers, opc0 is also used.
> +     * Since there are no "coprocessors" in AArch64, cp is purely used as a
> +     * way to distinguish (for KVM's benefit) guest-visible system registers
> +     * from demuxed ones provided to preserve the "no side effects on
> +     * KVM register read/write from QEMU" semantics. cp==0x13 is guest
> +     * visible (to match KVM's encoding); cp==0 will be converted to
> +     * cp==0x13 when the ARMCPRegInfo is registered, for convenience.
>       */
>      uint8_t cp;
>      uint8_t crn;
>      uint8_t crm;
> +    uint8_t opc0;
>      uint8_t opc1;
>      uint8_t opc2;
>      /* Register type: ARM_CP_* bits/values */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 6ebd7dc..975a762 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>       * At least one of the original and the second definition should
>       * include ARM_CP_OVERRIDE in its type bits -- this is just a guard
>       * against accidental use.
> +     *
> +     * ARM_CP_AA64 is set in the type field to define a register to
> +     * be visible when in AArch64 state. In this case r->opc0 may also
> +     * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't
> +     * be wildcarded.
>       */
>      int crm, opc1, opc2;
>      int crmmin = (r->crm == CP_ANY) ? 0 : r->crm;
> @@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>      int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2;
>      /* 64 bit registers have only CRm and Opc1 fields */
>      assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn)));
> +    /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */
> +    assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64))
> +           != (ARM_CP_64BIT|ARM_CP_AA64));
> +    /* op0 only exists in the AArch64 encodings */
> +    assert((r->type & ARM_CP_AA64) || (r->opc0 == 0));
> +    /* The AArch64 pseudocode CheckSystemAccess() specifies that op1
> +     * encodes a minimum access level for the register. We roll this
> +     * runtime check into our general permission check code, so check
> +     * here that the reginfo's specified permissions are strict enough
> +     * to encompass the generic architectural permission check.
> +     */
> +    if (r->type & ARM_CP_AA64) {
> +        int mask = 0;
> +        switch (r->opc1) {
> +        case 0: case 1: case 2:
> +            /* min_EL EL1 */
> +            mask = PL1_RW;
> +            break;
> +        case 3:
> +            /* min_EL EL0 */
> +            mask = PL0_RW;
> +            break;
> +        case 4:
> +            /* min_EL EL2 */
> +            mask = PL2_RW;
> +            break;
> +        case 5:
> +            /* unallocated encoding, so not possible */
> +            assert(false);
> +            break;
> +        case 6:
> +            /* min_EL EL3 */
> +            mask = PL3_RW;
> +            break;
> +        case 7:
> +            /* min_EL EL1, secure mode only (we don't check the latter) */
> +            mask = PL1_RW;
> +            break;
> +        default:
> +            /* broken reginfo with out of range opc1 */

A nit: "out-of-range"

> +            assert(false);
> +            break;
> +        }
> +        /* assert our permissions are not too lax (stricter is fine) */
> +        assert((r->access & ~mask) == 0);
> +    }
> +
>      /* Check that the register definition has enough info to handle
>       * reads and writes if they are permitted.
>       */
> @@ -1980,7 +2032,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>                  uint32_t *key = g_new(uint32_t, 1);
>                  ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo));
>                  int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0;
> -                *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
> +                if (r->type & ARM_CP_AA64) {
> +                    /* To allow abbreviation of ARMCPRegInfo
> +                     * definitions, we treat cp == 0 as equivalent to
> +                     * the value for "standard guest-visible sysreg".
> +                     */
> +                    if (r->cp == 0) {
> +                        r2->cp = CP_REG_ARM64_SYSREG_CP;
> +                    }
> +                    *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm,
> +                                              r->opc0, opc1, opc2);

You have mixed terminology here with "opc" and "op". Should they be
unionised in ARMCPRegInfo?

union {
    uint8_t op1;
    uint8_t opc1;
};

> +                } else {
> +                    *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
> +                }

Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be
possible to define with one ARMCPRegInfo a register that exists in
both 32 and 64 and this code can just double add it to the hash table?
May need two flags to describe existence in either or both schemes
accordingly.

Regards,
Peter

>                  if (opaque) {
>                      r2->opaque = opaque;
>                  }
> diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h
> index 2bba0bd..d006146 100644
> --- a/target-arm/kvm-consts.h
> +++ b/target-arm/kvm-consts.h
> @@ -29,12 +29,14 @@
>  #define CP_REG_SIZE_U32        0x0020000000000000ULL
>  #define CP_REG_SIZE_U64        0x0030000000000000ULL
>  #define CP_REG_ARM             0x4000000000000000ULL
> +#define CP_REG_ARCH_MASK       0xff00000000000000ULL
>
>  MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT)
>  MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK)
>  MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32)
>  MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64)
>  MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM)
> +MISMATCH_CHECK(CP_REG_ARCH_MASK, KVM_REG_ARCH_MASK)
>
>  #define PSCI_FN_BASE 0x95c1ba5e
>  #define PSCI_FN(n) (PSCI_FN_BASE + (n))
> @@ -59,6 +61,41 @@ MISMATCH_CHECK(PSCI_FN_MIGRATE, KVM_PSCI_FN_MIGRATE)
>  MISMATCH_CHECK(QEMU_KVM_ARM_TARGET_CORTEX_A15, KVM_ARM_TARGET_CORTEX_A15)
>  #endif
>
> +#define CP_REG_ARM64                   0x6000000000000000ULL
> +#define CP_REG_ARM_COPROC_MASK         0x000000000FFF0000
> +#define CP_REG_ARM_COPROC_SHIFT        16
> +#define CP_REG_ARM64_SYSREG            (0x0013 << CP_REG_ARM_COPROC_SHIFT)
> +#define CP_REG_ARM64_SYSREG_OP0_MASK   0x000000000000c000
> +#define CP_REG_ARM64_SYSREG_OP0_SHIFT  14
> +#define CP_REG_ARM64_SYSREG_OP1_MASK   0x0000000000003800
> +#define CP_REG_ARM64_SYSREG_OP1_SHIFT  11
> +#define CP_REG_ARM64_SYSREG_CRN_MASK   0x0000000000000780
> +#define CP_REG_ARM64_SYSREG_CRN_SHIFT  7
> +#define CP_REG_ARM64_SYSREG_CRM_MASK   0x0000000000000078
> +#define CP_REG_ARM64_SYSREG_CRM_SHIFT  3
> +#define CP_REG_ARM64_SYSREG_OP2_MASK   0x0000000000000007
> +#define CP_REG_ARM64_SYSREG_OP2_SHIFT  0
> +
> +/* No kernel define but it's useful to QEMU */
> +#define CP_REG_ARM64_SYSREG_CP (CP_REG_ARM64_SYSREG >> CP_REG_ARM_COPROC_SHIFT)
> +
> +#ifdef TARGET_AARCH64
> +MISMATCH_CHECK(CP_REG_ARM64, KVM_REG_ARM64)
> +MISMATCH_CHECK(CP_REG_ARM_COPROC_MASK, KVM_REG_ARM_COPROC_MASK)
> +MISMATCH_CHECK(CP_REG_ARM_COPROC_SHIFT, KVM_REG_ARM_COPROC_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG, KVM_REG_ARM64_SYSREG)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT)
> +#endif
> +
>  #undef MISMATCH_CHECK
>
>  #endif
> --
> 1.8.5
>
>

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 19 December 2013 06:01, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +                    *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm,
>> +                                              r->opc0, opc1, opc2);
>
> You have mixed terminology here with "opc" and "op". Should they be
> unionised in ARMCPRegInfo?
>
> union {
>     uint8_t op1;
>     uint8_t opc1;
> };

That seems pretty ugly to me. The terminology mixing is kind of
inevitable since AArch32 uses opc and AArch64 uses op.

>> +                } else {
>> +                    *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
>> +                }
>
> Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be
> possible to define with one ARMCPRegInfo a register that exists in
> both 32 and 64 and this code can just double add it to the hash table?
> May need two flags to describe existence in either or both schemes
> accordingly.

Almost all the shared registers appear as 32 bit on the AArch32
side and 64 bits on the AArch64 side. This means the required
fieldoffset value is different [or potentially so for bigendian hosts].
So you'd only be able to share registers which were genuinely
64 bit on both sides, which are very rare. So it didn't seem worth
trying to accommodate it.

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 19 December 2013 06:01, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> +                    *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm,
>>> +                                              r->opc0, opc1, opc2);
>>
>> You have mixed terminology here with "opc" and "op". Should they be
>> unionised in ARMCPRegInfo?
>>
>> union {
>>     uint8_t op1;
>>     uint8_t opc1;
>> };
>
> That seems pretty ugly to me. The terminology mixing is kind of
> inevitable since AArch32 uses opc and AArch64 uses op.
>

The union does self-document that fact however.

>>> +                } else {
>>> +                    *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
>>> +                }
>>
>> Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be
>> possible to define with one ARMCPRegInfo a register that exists in
>> both 32 and 64 and this code can just double add it to the hash table?
>> May need two flags to describe existence in either or both schemes
>> accordingly.
>
> Almost all the shared registers appear as 32 bit on the AArch32
> side and 64 bits on the AArch64 side.

Really? Reading v8, there are many 32 bit regs with these op0 AArch64
encodings, for example the documentation of MIDR_EL1:

Configurations
MIDR_EL1 is architecturally mapped to AArch32 register MIDR.
MIDR_EL1 is architecturally mapped to external register MIDR_EL1.
Attributes
MIDR_EL1 is a 32-bit register

CTR, REVIDR, ID_PFRx, CSSELR, CLIDR, DACR32 to name a few more that
are similarly documented and by no means an exhaustive list (I just
randomly picked those from the list).

Regards,
Peter


> This means the required
> fieldoffset value is different [or potentially so for bigendian hosts].
> So you'd only be able to share registers which were genuinely
> 64 bit on both sides, which are very rare. So it didn't seem worth
> trying to accommodate it.
>

The support 32 vs 64 encoding scheme seems quite othogonal to the
backing sotre bit width.

> -- PMM
>

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 04:24, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Almost all the shared registers appear as 32 bit on the AArch32
>> side and 64 bits on the AArch64 side.
>
> Really? Reading v8, there are many 32 bit regs with these op0 AArch64
> encodings, for example the documentation of MIDR_EL1:
>
> Configurations
> MIDR_EL1 is architecturally mapped to AArch32 register MIDR.
> MIDR_EL1 is architecturally mapped to external register MIDR_EL1.
> Attributes
> MIDR_EL1 is a 32-bit register

"32 bit register" in the AArch64 documentation is a shorthand for
"64 bit register with the top 32 bits (currently) RES0". There are
no 32 bit system registers in AArch64 by definition, because there's
no way to load only 32 bits in or out of them: MSR/MRS are always
64 bit transfers.

> CTR, REVIDR, ID_PFRx, CSSELR, CLIDR, DACR32 to name a few more that
> are similarly documented and by no means an exhaustive list (I just
> randomly picked those from the list).

These too are all 64 bits for AArch64.

>> This means the required
>> fieldoffset value is different [or potentially so for bigendian hosts].
>> So you'd only be able to share registers which were genuinely
>> 64 bit on both sides, which are very rare. So it didn't seem worth
>> trying to accommodate it.
>>
>
> The support 32 vs 64 encoding scheme seems quite othogonal to the
> backing sotre bit width.

Well, we could add 4 to the fieldoffset for the copy of the reginfo
that we're using for the 32 bit side if TARGET_WORDS_BIGENDIAN,
I guess.

I'll have a think about this, because there are some wrinkles relating
to reset that might be usefully solved this way.

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> I'll have a think about this, because there are some wrinkles relating
> to reset that might be usefully solved this way.

So my conclusion here is that really this boils down to how we
want to deal with sysregs for AArch64 capable CPUs in general.
There are two models I can think of:

Option 1:
 * we provide new reginfo definitions that describe the AArch64
   views of the system registers
 * for the AArch32 views, we just fall through and use the existing
   definitions we've written for AArch32-only CPUs

Option 2:
 * we provide new reginfo definitions for the AArch64 views of
   the registers, which include annotations to say whether there
   is an AArch32 view of this register
 * for the few registers which aren't neatly arranged so the
   crn/crm/opc1/opc2 line up, we just split up into a separate
   reginfo for AArch64 and AArch32
 * register_cp_regs_for_features() has separate AArch32
   and AArch64 versions

Option 1 is what my initial patch was assuming. Option 2
is what the idea of being able to mark a reginfo as "this
provides both AArch32 and AArch64 register views"
implies, I think.

Having thought about it a bit I actually prefer Option 2
of these -- it means that for a 64 bit CPU the reginfo
dealing with a particular CPUState field will be in one
place rather than two, and we also get a chance to
audit which cp regs go into v8 AArch64 emulated CPUs,
so we don't leak old backwards-compatible dummy
definitions by accident.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I'll have a think about this, because there are some wrinkles relating
>> to reset that might be usefully solved this way.
>
> So my conclusion here is that really this boils down to how we
> want to deal with sysregs for AArch64 capable CPUs in general.
> There are two models I can think of:
>
> Option 1:
>  * we provide new reginfo definitions that describe the AArch64
>    views of the system registers
>  * for the AArch32 views, we just fall through and use the existing
>    definitions we've written for AArch32-only CPUs
>
> Option 2:
>  * we provide new reginfo definitions for the AArch64 views of
>    the registers, which include annotations to say whether there
>    is an AArch32 view of this register

So to clarify, would MIDR and friends be in this bucket? And does t
obsolete the old MIDR def such there is only one CPRegInfo globally?

>  * for the few registers which aren't neatly arranged so the
>    crn/crm/opc1/opc2 line up, we just split up into a separate
>    reginfo for AArch64 and AArch32

ACK, that sounds awkward but there nothing we can do abt it. how many
are there? The few I checked always line up.

>  * register_cp_regs_for_features() has separate AArch32
>    and AArch64 versions
>

Do we need this? With this scheme its still possible to enforce
exclusivity in the CPRegInfo themselves.

> Option 1 is what my initial patch was assuming. Option 2
> is what the idea of being able to mark a reginfo as "this
> provides both AArch32 and AArch64 register views"
> implies, I think.
>
> Having thought about it a bit I actually prefer Option 2
> of these -- it means that for a 64 bit CPU the reginfo
> dealing with a particular CPUState field will be in one
> place rather than two, and we also get a chance to
> audit which cp regs go into v8 AArch64 emulated CPUs,
> so we don't leak old backwards-compatible dummy
> definitions by accident.

Agreed.

Regards,
Peter


>
> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 21:41, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> I'll have a think about this, because there are some wrinkles relating
>>> to reset that might be usefully solved this way.
>>
>> So my conclusion here is that really this boils down to how we
>> want to deal with sysregs for AArch64 capable CPUs in general.
>> There are two models I can think of:
>>
>> Option 1:
>>  * we provide new reginfo definitions that describe the AArch64
>>    views of the system registers
>>  * for the AArch32 views, we just fall through and use the existing
>>    definitions we've written for AArch32-only CPUs
>>
>> Option 2:
>>  * we provide new reginfo definitions for the AArch64 views of
>>    the registers, which include annotations to say whether there
>>    is an AArch32 view of this register
>
> So to clarify, would MIDR and friends be in this bucket? And does t
> obsolete the old MIDR def such there is only one CPRegInfo globally?

I think we end up with a MIDR def for old CPUs and a MIDR
for v8-and-up CPUs which handles both AArch64 and AArch32 views:
            { .name = "MIDR_EL1",
              .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
              .access = PL1_R,
             .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW,
             .resetvalue = cpu->midr },

because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x
registers read as MIDR" and (b) we can ignore the annoying
special case handling for the TI925.

(I wonder if it would be nicer to have a .state field (values
CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH)
rather than shoehorning it into the flags field...)

For cases where we don't have annoying v7-and-earlier legacy
to deal with we can just have one reginfo struct I think.

I think it's the weird stuff that's going to get irritating because
we're going to end up with "not for v8" logic to suppress half of
them.

>>  * for the few registers which aren't neatly arranged so the
>>    crn/crm/opc1/opc2 line up, we just split up into a separate
>>    reginfo for AArch64 and AArch32
>
> ACK, that sounds awkward but there nothing we can do abt it. how many
> are there? The few I checked always line up.

Well, everything in cp14 doesn't have architectural equivalents,
for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2
but that is OSDTRRX_EL1 in AArch64. The TLB and cache
maintenance ops don't always match up because they've been
tidied up, I think. The docs don't make it terribly easy to compile
a list of mismatches though.

>>  * register_cp_regs_for_features() has separate AArch32
>>    and AArch64 versions
>>
>
> Do we need this? With this scheme its still possible to enforce
> exclusivity in the CPRegInfo themselves.

Mmm, I guess we don't really, though it does mean we're
a bit more likely to have dubious stuff lurking around in
the AArch32-visible sysregs. (OTOH that's pretty irrelevant
until we have support for either Hyp or TrustZone, since
otherwise EL1 is always AArch64...)

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 22:07, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 December 2013 21:41, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>  * for the few registers which aren't neatly arranged so the
>>>    crn/crm/opc1/opc2 line up, we just split up into a separate
>>>    reginfo for AArch64 and AArch32
>>
>> ACK, that sounds awkward but there nothing we can do abt it. how many
>> are there? The few I checked always line up.
>
> Well, everything in cp14 doesn't have architectural equivalents,
> for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2
> but that is OSDTRRX_EL1 in AArch64. The TLB and cache
> maintenance ops don't always match up because they've been
> tidied up, I think. The docs don't make it terribly easy to compile
> a list of mismatches though.

More generally I think the way that AArch64 uses op1 to group
the registers by exception-level-access-rights is going to make it
a bit tricky to do the mapping; we either need to
(1) have .opc1 be the AA32 opc1 and infer AA64 op1 from
the permission flags
(2) have .opc1 be the AA64 op1 and insist that the AA32 opc1
is always zero  (or always same as AA64 op1?), and require
split reginfo structs if this isn't so
(3) have both op1 and opc1 fields in the reginfo struct

I don't much like any of these but I guess (2) is least worst.

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 22:16, Peter Maydell <peter.maydell@linaro.org> wrote:
> More generally I think the way that AArch64 uses op1 to group
> the registers by exception-level-access-rights is going to make it
> a bit tricky to do the mapping; we either need to
> (1) have .opc1 be the AA32 opc1 and infer AA64 op1 from
> the permission flags
> (2) have .opc1 be the AA64 op1 and insist that the AA32 opc1
> is always zero  (or always same as AA64 op1?), and require
> split reginfo structs if this isn't so
> (3) have both op1 and opc1 fields in the reginfo struct

Having waded through the docs a bit more I think the correct
answer here is "opc1 is AA64 op1 and AA32 opc1". The
definitions line up in almost all cases. The few exceptions
are where the AA64 definition has been brought into line
with the "opc1 indicates exception level access rights" pattern
but the old AA32 definition had a zero opc1 despite not
being an EL1 register (typically because it was accessible
by EL0; the EL2 registers do match up with the AArch32
encodings). There aren't very many of these, but since the only
registers this patchset really cares about are the EL0-accessible
registers almost all the ones we're going to add here fall into
the "can't share reginfo" category. TPIDR_EL1 (AArch32
TPIDRPRW) is shareable but none of the others are.
So I'm going for the "all of opc1/opc2/crn/crm have to match
on AA32 and AA64 for a shared reginfo", but it's going to
look a little pointless until we get the system emulation
done next year :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Sat, Dec 21, 2013 at 8:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 20 December 2013 21:41, Peter Crosthwaite
> <peter.crosthwaite@xilinx.com> wrote:
>> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> I'll have a think about this, because there are some wrinkles relating
>>>> to reset that might be usefully solved this way.
>>>
>>> So my conclusion here is that really this boils down to how we
>>> want to deal with sysregs for AArch64 capable CPUs in general.
>>> There are two models I can think of:
>>>
>>> Option 1:
>>>  * we provide new reginfo definitions that describe the AArch64
>>>    views of the system registers
>>>  * for the AArch32 views, we just fall through and use the existing
>>>    definitions we've written for AArch32-only CPUs
>>>
>>> Option 2:
>>>  * we provide new reginfo definitions for the AArch64 views of
>>>    the registers, which include annotations to say whether there
>>>    is an AArch32 view of this register
>>
>> So to clarify, would MIDR and friends be in this bucket? And does t
>> obsolete the old MIDR def such there is only one CPRegInfo globally?
>
> I think we end up with a MIDR def for old CPUs and a MIDR
> for v8-and-up CPUs which handles both AArch64 and AArch32 views:
>             { .name = "MIDR_EL1",
>               .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
>               .access = PL1_R,
>              .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW,
>              .resetvalue = cpu->midr },
>
> because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x
> registers read as MIDR"

Ok so MIDR is an annoying special case (Some of the "friends" I listed
early are probably better examples).

> and (b) we can ignore the annoying
> special case handling for the TI925.
>
> (I wonder if it would be nicer to have a .state field (values

Ideally, I think its about which encoding schemes have visibility of
this reg. From what we discussed so far, it seems that there are three
separate encoding schemes:

32/MRC - the current 99% case, but needs to now support access to
widened-to-64 registers (upper RES0 policy)
32/MRRC - 64bit regs accessible from 32
64/MRS - What you are adding here

One hot encode accessibility via each of these in a ".encodings"
field, then the implementations or MRC, MRRC and MRS just check
independently whether they are allowed or not.

> CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH)
> rather than shoehorning it into the flags field...)
>

Agreed - new field.

> For cases where we don't have annoying v7-and-earlier legacy
> to deal with we can just have one reginfo struct I think.
>
> I think it's the weird stuff that's going to get irritating because
> we're going to end up with "not for v8" logic to suppress half of
> them.
>
>>>  * for the few registers which aren't neatly arranged so the
>>>    crn/crm/opc1/opc2 line up, we just split up into a separate
>>>    reginfo for AArch64 and AArch32
>>
>> ACK, that sounds awkward but there nothing we can do abt it. how many
>> are there? The few I checked always line up.
>
> Well, everything in cp14 doesn't have architectural equivalents,
> for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2
> but that is OSDTRRX_EL1 in AArch64. The TLB and cache
> maintenance ops don't always match up because they've been
> tidied up, I think. The docs don't make it terribly easy to compile
> a list of mismatches though.
>
>>>  * register_cp_regs_for_features() has separate AArch32
>>>    and AArch64 versions
>>>
>>
>> Do we need this? With this scheme its still possible to enforce
>> exclusivity in the CPRegInfo themselves.
>
> Mmm, I guess we don't really, though it does mean we're
> a bit more likely to have dubious stuff lurking around in
> the AArch32-visible sysregs. (OTOH that's pretty irrelevant
> until we have support for either Hyp or TrustZone, since
> otherwise EL1 is always AArch64...)

Is there more to it that just enforcing MRS/MRC/MRRC perms? An MRS
only defined CP reg should never have AArch32 visibility unless I am
missing something.

Regards,
Peter

>
> -- PMM
>

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 22:29, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sat, Dec 21, 2013 at 8:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 20 December 2013 21:41, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> So to clarify, would MIDR and friends be in this bucket? And does t
>>> obsolete the old MIDR def such there is only one CPRegInfo globally?
>>
>> I think we end up with a MIDR def for old CPUs and a MIDR
>> for v8-and-up CPUs which handles both AArch64 and AArch32 views:
>>             { .name = "MIDR_EL1",
>>               .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0,
>>               .access = PL1_R,
>>              .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW,
>>              .resetvalue = cpu->midr },
>>
>> because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x
>> registers read as MIDR"
>
> Ok so MIDR is an annoying special case (Some of the "friends" I listed
> early are probably better examples).

I think the friends would probably look much like the MIDR example
I give above.

>> and (b) we can ignore the annoying
>> special case handling for the TI925.
>>
>> (I wonder if it would be nicer to have a .state field (values
>
> Ideally, I think its about which encoding schemes have visibility of
> this reg. From what we discussed so far, it seems that there are three
> separate encoding schemes:
>
> 32/MRC - the current 99% case, but needs to now support access to
> widened-to-64 registers (upper RES0 policy)
> 32/MRRC - 64bit regs accessible from 32
> 64/MRS - What you are adding here
>
> One hot encode accessibility via each of these in a ".encodings"
> field, then the implementations or MRC, MRRC and MRS just check
> independently whether they are allowed or not.

Whether we split into .encodings or use flag fields or whatever
doesn't matter for that -- for "is there a register here via this
encoding scheme?" what we do is just do a hashtable lookup
and we expect to get a match only if the entire "this access
method plus all its op*/cr* fields" matches. This is what
ENCODE_CP_REG() and the new AArch64 equivalent
macro are doing.

How we lay things out in the reginfo struct is purely down
to what makes writing reginfo structs easiest. The code
which populates the hashtable just throws all the relevant
information into the correct ENCODE_CP_REG macro.

>> CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH)
>> rather than shoehorning it into the flags field...)
>>
>
> Agreed - new field.


>>>>  * register_cp_regs_for_features() has separate AArch32
>>>>    and AArch64 versions
>>>>
>>>
>>> Do we need this? With this scheme its still possible to enforce
>>> exclusivity in the CPRegInfo themselves.
>>
>> Mmm, I guess we don't really, though it does mean we're
>> a bit more likely to have dubious stuff lurking around in
>> the AArch32-visible sysregs. (OTOH that's pretty irrelevant
>> until we have support for either Hyp or TrustZone, since
>> otherwise EL1 is always AArch64...)
>
> Is there more to it that just enforcing MRS/MRC/MRRC perms? An MRS
> only defined CP reg should never have AArch32 visibility unless I am
> missing something.

That's correct. The issue is just that if we allow all the
existing "if v6 register these registers, if v5 register these, etc"
code to run then v8 CPUs potentially end up with (AArch32)
sysregs they don't really have. That is however pretty much
masked by the fact you can't actually get at the AArch32
sysregs mostly unless you have an Arch32 EL1, which in
turn you're not going to see in an AArch64 CPU unless
EL2 or EL3 are present and AArch64.

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 20 December 2013 04:24, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 19 December 2013 06:01, Peter Crosthwaite
>> <peter.crosthwaite@xilinx.com> wrote:
>>> On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>>> +                    *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm,
>>>> +                                              r->opc0, opc1, opc2);
>>>
>>> You have mixed terminology here with "opc" and "op". Should they be
>>> unionised in ARMCPRegInfo?
>>>
>>> union {
>>>     uint8_t op1;
>>>     uint8_t opc1;
>>> };
>>
>> That seems pretty ugly to me. The terminology mixing is kind of
>> inevitable since AArch32 uses opc and AArch64 uses op.
>>
>
> The union does self-document that fact however.

I think that would be unnecessarily confusing. I'd rather just
have our code stick with the old style "opc". It's not like it's
a major difference.
> The support 32 vs 64 encoding scheme seems quite othogonal to the
> backing sotre bit width.

At the moment it isn't, though -- 64 bit registers have 64 bit
backing fields (and for AArch32 are accessed via MRRC/MCRR),
32 bit registers have 32 bit backing fields.

Bear in mind also that we have to interact with KVM, which
also treats all AArch64 sysregs as 64 bit (and AArch32
ones as 32 bit for MRC/MCR and 64 bit for MRRC/MCRR).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Crosthwaite]

On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Update the generic cpreg support code to also handle AArch64:
> AArch64-visible registers coexist in the same hash table with
> AArch32-visible ones, with a bit in the hash key distinguishing
> them.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h        | 59 ++++++++++++++++++++++++++++++++++++++-----
>  target-arm/helper.c     | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++
>  3 files changed, 155 insertions(+), 7 deletions(-)
>
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 56ed591..901f882 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq);
>   *    or via MRRC/MCRR?)
>   * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field.
>   * (In this case crn and opc2 should be zero.)
> + * For AArch64, there is no 32/64 bit size distinction;
> + * instead all registers have a 2 bit op0, 3 bit op1 and op2,
> + * and 4 bit CRn and CRm. The encoding patterns are chosen
> + * to be easy to convert to and from the KVM encodings, and also
> + * so that the hashtable can contain both AArch32 and AArch64
> + * registers (to allow for interprocessing where we might run
> + * 32 bit code on a 64 bit core).
>   */
> +/* This bit is private to our hashtable cpreg; in KVM register
> + * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64
> + * in the upper bits of the 64 bit ID.
> + */
> +#define CP_REG_AA64_SHIFT 28
> +#define CP_REG_AA64_MASK (1 << CP_REG_AA64_SHIFT)
> +
>  #define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2)   \
>      (((cp) << 16) | ((is64) << 15) | ((crn) << 11) |    \
>       ((crm) << 7) | ((opc1) << 3) | (opc2))
>
> +#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \
> +    (CP_REG_AA64_MASK |                                 \
> +     ((cp) << CP_REG_ARM_COPROC_SHIFT) |                \
> +     ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) |         \
> +     ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) |         \
> +     ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) |         \
> +     ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) |         \
> +     ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT))
> +
>  /* Convert a full 64 bit KVM register ID to the truncated 32 bit
>   * version used as a key for the coprocessor register hashtable
>   */
>  static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
>  {
>      uint32_t cpregid = kvmid;
> -    if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
> +    if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) {
> +        cpregid |= CP_REG_AA64_MASK;
> +    } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) {
>          cpregid |= (1 << 15);
>      }
>      return cpregid;
> @@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid)
>   */
>  static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  {
> -    uint64_t kvmid = cpregid & ~(1 << 15);
> -    if (cpregid & (1 << 15)) {
> -        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
> +    uint64_t kvmid;
> +
> +    if (cpregid & CP_REG_AA64_MASK) {
> +        kvmid = cpregid & ~CP_REG_AA64_MASK;
> +        kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64;
>      } else {
> -        kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
> +        kvmid = cpregid & ~(1 << 15);
> +        if (cpregid & (1 << 15)) {
> +            kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM;
> +        } else {
> +            kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM;
> +        }
>      }
>      return kvmid;
>  }
> @@ -626,13 +658,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
>  #define ARM_CP_OVERRIDE 16
>  #define ARM_CP_NO_MIGRATE 32
>  #define ARM_CP_IO 64
> +#define ARM_CP_AA64 128
>  #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
>  #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
>  #define ARM_LAST_SPECIAL ARM_CP_WFI
>  /* Used only as a terminator for ARMCPRegInfo lists */
>  #define ARM_CP_SENTINEL 0xffff
>  /* Mask of only the flag bits in a type field */
> -#define ARM_CP_FLAG_MASK 0x7f
> +#define ARM_CP_FLAG_MASK 0xff
>
>  /* Return true if cptype is a valid type field. This is used to try to
>   * catch errors where the sentinel has been accidentally left off the end
> @@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype)
>   * (ie anything visible in PL2 is visible in S-PL1, some things are only
>   * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the
>   * terminology a little and call this PL3.
> + * In AArch64 things are somewhat simpler as the PLx bits line up exactly
> + * with the ELx exception levels.
>   *
>   * If access permissions for a register are more complex than can be
>   * described with these bits, then use a laxer set of restrictions, and
> @@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype)
>
>  static inline int arm_current_pl(CPUARMState *env)
>  {
> +    if (env->aarch64) {
> +        return extract32(env->pstate, 2, 2);
> +    }
> +
>      if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) {
>          return 0;
>      }
> @@ -713,10 +752,18 @@ struct ARMCPRegInfo {
>       * then behave differently on read/write if necessary.
>       * For 64 bit registers, only crm and opc1 are relevant; crn and opc2
>       * must both be zero.
> +     * For AArch64-visible registers, opc0 is also used.
> +     * Since there are no "coprocessors" in AArch64, cp is purely used as a
> +     * way to distinguish (for KVM's benefit) guest-visible system registers
> +     * from demuxed ones provided to preserve the "no side effects on
> +     * KVM register read/write from QEMU" semantics. cp==0x13 is guest
> +     * visible (to match KVM's encoding); cp==0 will be converted to
> +     * cp==0x13 when the ARMCPRegInfo is registered, for convenience.
>       */
>      uint8_t cp;
>      uint8_t crn;
>      uint8_t crm;
> +    uint8_t opc0;
>      uint8_t opc1;
>      uint8_t opc2;
>      /* Register type: ARM_CP_* bits/values */
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 6ebd7dc..975a762 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>       * At least one of the original and the second definition should
>       * include ARM_CP_OVERRIDE in its type bits -- this is just a guard
>       * against accidental use.
> +     *
> +     * ARM_CP_AA64 is set in the type field to define a register to
> +     * be visible when in AArch64 state. In this case r->opc0 may also
> +     * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't
> +     * be wildcarded.
>       */
>      int crm, opc1, opc2;
>      int crmmin = (r->crm == CP_ANY) ? 0 : r->crm;
> @@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu,
>      int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2;
>      /* 64 bit registers have only CRm and Opc1 fields */
>      assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn)));
> +    /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */
> +    assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64))
> +           != (ARM_CP_64BIT|ARM_CP_AA64));
> +    /* op0 only exists in the AArch64 encodings */
> +    assert((r->type & ARM_CP_AA64) || (r->opc0 == 0));
> +    /* The AArch64 pseudocode CheckSystemAccess() specifies that op1
> +     * encodes a minimum access level for the register. We roll this
> +     * runtime check into our general permission check code, so check
> +     * here that the reginfo's specified permissions are strict enough
> +     * to encompass the generic architectural permission check.
> +     */
> +    if (r->type & ARM_CP_AA64) {
> +        int mask = 0;
> +        switch (r->opc1) {
> +        case 0: case 1: case 2:
> +            /* min_EL EL1 */
> +            mask = PL1_RW;
> +            break;
> +        case 3:
> +            /* min_EL EL0 */
> +            mask = PL0_RW;
> +            break;
> +        case 4:
> +            /* min_EL EL2 */
> +            mask = PL2_RW;
> +            break;
> +        case 5:
> +            /* unallocated encoding, so not possible */
> +            assert(false);
> +            break;
> +        case 6:
> +            /* min_EL EL3 */
> +            mask = PL3_RW;
> +            break;
> +        case 7:
> +            /* min_EL EL1, secure mode only (we don't check the latter) */
> +            mask = PL1_RW;
> +            break;
> +        default:
> +            /* broken reginfo with out of range opc1 */

"op1".

Regards,
Peter

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 17 December 2013 15:12, Peter Maydell <peter.maydell@linaro.org> wrote:
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK)
> +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT)

Spot the typo which breaks compilation on aarch64 hosts :-)

-- PMM

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64

[Christoffer Dall]

On Fri, Dec 20, 2013 at 04:43:23PM +0000, Peter Maydell wrote:
> On 17 December 2013 15:12, Peter Maydell <peter.maydell@linaro.org> wrote:
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK)
> > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT)
> 
> Spot the typo which breaks compilation on aarch64 hosts :-)
> 
SYREG, nice one :-)

-Christoffer

[Qemu-devel] [PATCH 12/21] target-arm: Remove ARMCPU/CPUARMState from cpregs APIs used by decoder

[Peter Maydell]

The cpregs APIs used by the decoder (get_arm_cp_reginfo() and
cp_access_ok()) currently take either a CPUARMState* or an ARMCPU*.
This is problematic for the A64 decoder, which doesn't pass the
environment pointer around everywhere the way the 32 bit decoder
does. Adjust the parameters these functions take so that we can
copy only the relevant info from the CPUARMState into the
DisasContext and then use that.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h           |  6 +++---
 target-arm/helper.c        | 12 ++++++------
 target-arm/translate-a64.c |  2 ++
 target-arm/translate.c     |  7 ++++---
 target-arm/translate.h     |  2 ++
 5 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 901f882..5ec74b1 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -837,7 +837,7 @@ static inline void define_one_arm_cp_reg(ARMCPU *cpu, const ARMCPRegInfo *regs)
 {
     define_one_arm_cp_reg_with_opaque(cpu, regs, 0);
 }
-const ARMCPRegInfo *get_arm_cp_reginfo(ARMCPU *cpu, uint32_t encoded_cp);
+const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp);
 
 /* CPWriteFn that can be used to implement writes-ignored behaviour */
 int arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -845,10 +845,10 @@ int arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,
 /* CPReadFn that can be used for read-as-zero behaviour */
 int arm_cp_read_zero(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t *value);
 
-static inline bool cp_access_ok(CPUARMState *env,
+static inline bool cp_access_ok(int current_pl,
                                 const ARMCPRegInfo *ri, int isread)
 {
-    return (ri->access >> ((arm_current_pl(env) * 2) + isread)) & 1;
+    return (ri->access >> ((current_pl * 2) + isread)) & 1;
 }
 
 /**
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 975a762..72f0ee8 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -186,7 +186,7 @@ bool write_cpustate_to_list(ARMCPU *cpu)
         uint32_t regidx = kvm_to_cpreg_id(cpu->cpreg_indexes[i]);
         const ARMCPRegInfo *ri;
         uint64_t v;
-        ri = get_arm_cp_reginfo(cpu, regidx);
+        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
         if (!ri) {
             ok = false;
             continue;
@@ -214,7 +214,7 @@ bool write_list_to_cpustate(ARMCPU *cpu)
         uint64_t readback;
         const ARMCPRegInfo *ri;
 
-        ri = get_arm_cp_reginfo(cpu, regidx);
+        ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
         if (!ri) {
             ok = false;
             continue;
@@ -242,7 +242,7 @@ static void add_cpreg_to_list(gpointer key, gpointer opaque)
     const ARMCPRegInfo *ri;
 
     regidx = *(uint32_t *)key;
-    ri = get_arm_cp_reginfo(cpu, regidx);
+    ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
 
     if (!(ri->type & ARM_CP_NO_MIGRATE)) {
         cpu->cpreg_indexes[cpu->cpreg_array_len] = cpreg_to_kvm_id(regidx);
@@ -258,7 +258,7 @@ static void count_cpreg(gpointer key, gpointer opaque)
     const ARMCPRegInfo *ri;
 
     regidx = *(uint32_t *)key;
-    ri = get_arm_cp_reginfo(cpu, regidx);
+    ri = get_arm_cp_reginfo(cpu->cp_regs, regidx);
 
     if (!(ri->type & ARM_CP_NO_MIGRATE)) {
         cpu->cpreg_array_len++;
@@ -2098,9 +2098,9 @@ void define_arm_cp_regs_with_opaque(ARMCPU *cpu,
     }
 }
 
-const ARMCPRegInfo *get_arm_cp_reginfo(ARMCPU *cpu, uint32_t encoded_cp)
+const ARMCPRegInfo *get_arm_cp_reginfo(GHashTable *cpregs, uint32_t encoded_cp)
 {
-    return g_hash_table_lookup(cpu->cp_regs, &encoded_cp);
+    return g_hash_table_lookup(cpregs, &encoded_cp);
 }
 
 int arm_cp_write_ignore(CPUARMState *env, const ARMCPRegInfo *ri,
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index add8cc2..38017a3 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -3000,6 +3000,8 @@ void gen_intermediate_code_internal_a64(ARMCPU *cpu,
     dc->vfp_enabled = 0;
     dc->vec_len = 0;
     dc->vec_stride = 0;
+    dc->cp_regs = cpu->cp_regs;
+    dc->current_pl = arm_current_pl(env);
 
     init_tmp_a64_array(dc);
 
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 1403ecf..8bfe950 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6498,7 +6498,6 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
 {
     int cpnum, is64, crn, crm, opc1, opc2, isread, rt, rt2;
     const ARMCPRegInfo *ri;
-    ARMCPU *cpu = arm_env_get_cpu(env);
 
     cpnum = (insn >> 8) & 0xf;
     if (arm_feature(env, ARM_FEATURE_XSCALE)
@@ -6541,11 +6540,11 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
     isread = (insn >> 20) & 1;
     rt = (insn >> 12) & 0xf;
 
-    ri = get_arm_cp_reginfo(cpu,
+    ri = get_arm_cp_reginfo(s->cp_regs,
                             ENCODE_CP_REG(cpnum, is64, crn, crm, opc1, opc2));
     if (ri) {
         /* Check access permissions */
-        if (!cp_access_ok(env, ri, isread)) {
+        if (!cp_access_ok(s->current_pl, ri, isread)) {
             return 1;
         }
 
@@ -10269,6 +10268,8 @@ static inline void gen_intermediate_code_internal(ARMCPU *cpu,
     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->cp_regs = cpu->cp_regs;
+    dc->current_pl = arm_current_pl(env);
 
     cpu_F0s = tcg_temp_new_i32();
     cpu_F1s = tcg_temp_new_i32();
diff --git a/target-arm/translate.h b/target-arm/translate.h
index a6f6b3e..67da699 100644
--- a/target-arm/translate.h
+++ b/target-arm/translate.h
@@ -24,6 +24,8 @@ typedef struct DisasContext {
     int vec_len;
     int vec_stride;
     int aarch64;
+    int current_pl;
+    GHashTable *cp_regs;
 #define TMP_A64_MAX 16
     int tmp_a64_count;
     TCGv_i64 tmp_a64[TMP_A64_MAX];
-- 
1.8.5

[Qemu-devel] [PATCH 13/21] target-arm: A64: Implement MRS/MSR/SYS/SYSL

[Peter Maydell]

The AArch64 equivalent of the traditional AArch32
cp15 coprocessor registers is the set of instructions
MRS/MSR/SYS/SYSL, which cover between them both true
system registers and the "operations with side effects"
such as cache maintenance which in AArch32 are mixed
in with other cp15 registers. Implement these instructions
to look in the cpregs hashtable for the register or
operation.

Since we don't yet populate the cpregs hashtable with
any registers with the "AA64" bit set, everything will
still UNDEF at this point.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate-a64.c | 115 +++++++++++++++++++++++++++++++++------------
 1 file changed, 86 insertions(+), 29 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 38017a3..dd37962 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -113,6 +113,13 @@ void gen_a64_set_pc_im(uint64_t val)
     tcg_gen_movi_i64(cpu_pc, val);
 }
 
+/* Force a TB lookup after an instruction that changes the CPU state */
+static inline void gen_lookup_tb(DisasContext *s)
+{
+    gen_a64_set_pc_im(s->pc);
+    s->is_jmp = DISAS_UPDATE;
+}
+
 static void gen_exception(int excp)
 {
     TCGv_i32 tmp = tcg_temp_new_i32();
@@ -731,28 +738,88 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
     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,
+/* C5.6.129 MRS - move from system register
+ * C5.6.131 MSR (register) - move to system register
+ * C5.6.204 SYS
+ * C5.6.205 SYSL
+ * These are all essentially the same insn in 'read' and 'write'
+ * versions, with varying op0 fields.
+ */
+static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
+                       unsigned int op0, unsigned int op1, unsigned int op2,
                        unsigned int crn, unsigned int crm, unsigned int rt)
 {
-    unsupported_encoding(s, insn);
-}
+    const ARMCPRegInfo *ri;
+    TCGv_i64 tcg_rt;
 
-/* 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);
-}
+    ri = get_arm_cp_reginfo(s->cp_regs,
+                            ENCODE_AA64_CP_REG(CP_REG_ARM64_SYSREG_CP,
+                                               crn, crm, op0, op1, op2));
 
-/* 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);
+    if (!ri) {
+        /* Unknown register */
+        unallocated_encoding(s);
+        return;
+    }
+
+    /* Check access permissions */
+    if (!cp_access_ok(env, ri, isread)) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    /* Handle special cases first */
+    switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) {
+    case ARM_CP_NOP:
+        return;
+    default:
+        break;
+    }
+
+    if (use_icount && (ri->type & ARM_CP_IO)) {
+        gen_io_start();
+    }
+
+    tcg_rt = cpu_reg(s, rt);
+
+    if (isread) {
+        if (ri->type & ARM_CP_CONST) {
+            tcg_gen_movi_i64(tcg_rt, ri->resetvalue);
+        } else if (ri->readfn) {
+            TCGv_ptr tmpptr;
+            gen_a64_set_pc_im(s->pc - 4);
+            tmpptr = tcg_const_ptr(ri);
+            gen_helper_get_cp_reg64(tcg_rt, cpu_env, tmpptr);
+            tcg_temp_free_ptr(tmpptr);
+        } else {
+            tcg_gen_ld_i64(tcg_rt, cpu_env, ri->fieldoffset);
+        }
+    } else {
+        if (ri->type & ARM_CP_CONST) {
+            /* If not forbidden by access permissions, treat as WI */
+            return;
+        } else if (ri->writefn) {
+            TCGv_ptr tmpptr;
+            gen_a64_set_pc_im(s->pc - 4);
+            tmpptr = tcg_const_ptr(ri);
+            gen_helper_set_cp_reg64(cpu_env, tmpptr, tcg_rt);
+            tcg_temp_free_ptr(tmpptr);
+        } else {
+            tcg_gen_st_i64(tcg_rt, cpu_env, ri->fieldoffset);
+        }
+    }
+
+    if (use_icount && (ri->type & ARM_CP_IO)) {
+        /* I/O operations must end the TB here (whether read or write) */
+        gen_io_end();
+        gen_lookup_tb(s);
+    } else if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) {
+        /* We default to ending the TB on a coprocessor register write,
+         * but allow this to be suppressed by the register definition
+         * (usually only necessary to work around guest bugs).
+         */
+        gen_lookup_tb(s);
+    }
 }
 
 /* C3.2.4 System
@@ -793,17 +860,7 @@ static void disas_system(DisasContext *s, uint32_t insn)
         }
         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);
-    }
+    handle_sys(s, insn, l, op0, op1, op2, crn, crm, rt);
 }
 
 /* C3.2.3 Exception generation
-- 
1.8.5

Re: [Qemu-devel] [PATCH 13/21] target-arm: A64: Implement MRS/MSR/SYS/SYSL

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +/* Force a TB lookup after an instruction that changes the CPU state */
> +static inline void gen_lookup_tb(DisasContext *s)
> +{
> +    gen_a64_set_pc_im(s->pc);
> +    s->is_jmp = DISAS_UPDATE;
> +}
> +

I'm a bit surprised that this set_pc doesn't happen in
gen_intermediate_code_internal_a64.  Otherwise, what's the point in
distinguishing between DISAS_JUMP and DISAS_UPDATE?

Otherwise,

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


r~

Re: [Qemu-devel] [PATCH 13/21] target-arm: A64: Implement MRS/MSR/SYS/SYSL

[Peter Maydell]

On 19 December 2013 20:30, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> +/* Force a TB lookup after an instruction that changes the CPU state */
>> +static inline void gen_lookup_tb(DisasContext *s)
>> +{
>> +    gen_a64_set_pc_im(s->pc);
>> +    s->is_jmp = DISAS_UPDATE;
>> +}
>> +
>
> I'm a bit surprised that this set_pc doesn't happen in
> gen_intermediate_code_internal_a64.  Otherwise, what's the point in
> distinguishing between DISAS_JUMP and DISAS_UPDATE?

I just borrowed this from the 32 bit decoder, which also sets PC
in gen_lookup_tb(). However you're right that we might as well
do the set pc in the top level loop. I've adjusted that loop to do so
and dropped the gen_lookup_tb() function in favour of just setting
s->is_jmp = DISAS_UPDATE directly at its callsites.

thanks
-- PMM

[Qemu-devel] [PATCH 14/21] target-arm: A64: Implement minimal set of EL0-visible sysregs

[Peter Maydell]

Implement an initial minimal set of EL0-visible system registers:
 * NZCV
 * FPCR
 * FPSR
 * CTR_EL0
 * DCZID_EL0

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h           |  3 ++-
 target-arm/helper.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/translate-a64.c | 54 +++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5ec74b1..32387b0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -661,7 +661,8 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 #define ARM_CP_AA64 128
 #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
 #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
-#define ARM_LAST_SPECIAL ARM_CP_WFI
+#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
+#define ARM_LAST_SPECIAL ARM_CP_NZCV
 /* Used only as a terminator for ARMCPRegInfo lists */
 #define ARM_CP_SENTINEL 0xffff
 /* Mask of only the flag bits in a type field */
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 72f0ee8..c64f618 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1560,6 +1560,61 @@ static const ARMCPRegInfo lpae_cp_reginfo[] = {
     REGINFO_SENTINEL
 };
 
+static int aa64_fpcr_read(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t *value)
+{
+    *value = vfp_get_fpcr(env);
+    return 0;
+}
+
+static int aa64_fpcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                           uint64_t value)
+{
+    vfp_set_fpcr(env, value);
+    return 0;
+}
+
+static int aa64_fpsr_read(CPUARMState *env, const ARMCPRegInfo *ri,
+                          uint64_t *value)
+{
+    *value = vfp_get_fpsr(env);
+    return 0;
+}
+
+static int aa64_fpsr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+                           uint64_t value)
+{
+    vfp_set_fpsr(env, value);
+    return 0;
+}
+
+static const ARMCPRegInfo aarch64_cp_reginfo[] = {
+    /* Minimal set of EL0-visible registers. This will need to be expanded
+     * significantly for system emulation.
+     */
+    { .name = "NZCV", .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 2,
+      .access = PL0_RW, .type = ARM_CP_AA64 | ARM_CP_NZCV },
+    {. name = "FPCR", .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
+     .access = PL0_RW, .type = ARM_CP_AA64,
+     .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write },
+    {. name = "FPSR", .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4,
+     .access = PL0_RW, .type = ARM_CP_AA64,
+     .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write },
+    /* This claims a 32 byte cacheline size for icache and dcache, VIPT icache.
+     * It will eventually need to have a CPU-specified reset value.
+     */
+    { .name = "CTR_EL0", .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
+      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_AA64,
+      .resetvalue = 0x80030003 },
+    /* Prohibit use of DC ZVA. OPTME: implement DC ZVA and allow its use.
+     * For system mode the DZP bit here will need to be computed, not constant.
+     */
+    { .name = "DCZID_EL0", .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
+      .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_AA64,
+      .resetvalue = 0x10 },
+    REGINFO_SENTINEL
+};
+
 static int sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
 {
     env->cp15.c1_sys = value;
@@ -1662,6 +1717,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
     } else {
         define_arm_cp_regs(cpu, not_v7_cp_reginfo);
     }
+    if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+        define_arm_cp_regs(cpu, aarch64_cp_reginfo);
+    }
     if (arm_feature(env, ARM_FEATURE_MPU)) {
         /* These are the MPU registers prior to PMSAv6. Any new
          * PMSA core later than the ARM946 will require that we
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index dd37962..a7d2b60 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -738,6 +738,50 @@ static void handle_msr_i(DisasContext *s, uint32_t insn,
     unsupported_encoding(s, insn);
 }
 
+static void gen_get_nzcv(TCGv_i64 tcg_rt)
+{
+    TCGv_i32 tmp = tcg_temp_new_i32();
+    TCGv_i32 nzcv = tcg_temp_new_i32();
+
+    /* build bit 31, N */
+    tcg_gen_andi_i32(nzcv, cpu_NF, (1 << 31));
+    /* build bit 30, Z */
+    tcg_gen_setcondi_i32(TCG_COND_EQ, tmp, cpu_ZF, 0);
+    tcg_gen_deposit_i32(nzcv, nzcv, tmp, 30, 1);
+    /* build bit 29, C */
+    tcg_gen_deposit_i32(nzcv, nzcv, cpu_CF, 29, 1);
+    /* build bit 28, V */
+    tcg_gen_shri_i32(tmp, cpu_VF, 31);
+    tcg_gen_deposit_i32(nzcv, nzcv, tmp, 28, 1);
+    /* generate result */
+    tcg_gen_extu_i32_i64(tcg_rt, nzcv);
+
+    tcg_temp_free_i32(nzcv);
+    tcg_temp_free_i32(tmp);
+}
+
+static void gen_set_nzcv(TCGv_i64 tcg_rt)
+
+{
+    TCGv_i32 nzcv = tcg_temp_new_i32();
+
+    /* take NZCV from R[t] */
+    tcg_gen_trunc_i64_i32(nzcv, tcg_rt);
+
+    /* bit 31, N */
+    tcg_gen_andi_i32(cpu_NF, nzcv, (1 << 31));
+    /* bit 30, Z */
+    tcg_gen_andi_i32(cpu_ZF, nzcv, (1 << 30));
+    tcg_gen_setcondi_i32(TCG_COND_EQ, cpu_ZF, cpu_ZF, 0);
+    /* bit 29, C */
+    tcg_gen_andi_i32(cpu_CF, nzcv, (1 << 29));
+    tcg_gen_shri_i32(cpu_CF, cpu_CF, 29);
+    /* bit 28, V */
+    tcg_gen_andi_i32(cpu_VF, nzcv, (1 << 28));
+    tcg_gen_shli_i32(cpu_VF, cpu_VF, 3);
+    tcg_temp_free_i32(nzcv);
+}
+
 /* C5.6.129 MRS - move from system register
  * C5.6.131 MSR (register) - move to system register
  * C5.6.204 SYS
@@ -763,7 +807,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
     }
 
     /* Check access permissions */
-    if (!cp_access_ok(env, ri, isread)) {
+    if (!cp_access_ok(s->current_pl, ri, isread)) {
         unallocated_encoding(s);
         return;
     }
@@ -772,6 +816,14 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
     switch (ri->type & ~(ARM_CP_FLAG_MASK & ~ARM_CP_SPECIAL)) {
     case ARM_CP_NOP:
         return;
+    case ARM_CP_NZCV:
+        tcg_rt = cpu_reg(s, rt);
+        if (isread) {
+            gen_get_nzcv(tcg_rt);
+        } else {
+            gen_set_nzcv(tcg_rt);
+        }
+        return;
     default:
         break;
     }
-- 
1.8.5

Re: [Qemu-devel] [PATCH 14/21] target-arm: A64: Implement minimal set of EL0-visible sysregs

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> Implement an initial minimal set of EL0-visible system registers:
>  * NZCV
>  * FPCR
>  * FPSR
>  * CTR_EL0
>  * DCZID_EL0
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h           |  3 ++-
>  target-arm/helper.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>  target-arm/translate-a64.c | 54 +++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 113 insertions(+), 2 deletions(-)

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


r~

Re: [Qemu-devel] [PATCH 14/21] target-arm: A64: Implement minimal set of EL0-visible sysregs

[Peter Maydell]

On 17 December 2013 15:12, Peter Maydell <peter.maydell@linaro.org> wrote:
> @@ -763,7 +807,7 @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,
>      }
>
>      /* Check access permissions */
> -    if (!cp_access_ok(env, ri, isread)) {
> +    if (!cp_access_ok(s->current_pl, ri, isread)) {
>          unallocated_encoding(s);
>          return;
>      }

This hunk should have been squashed into patch 13, which otherwise
won't compile. Will fix in next respin.

thanks
-- PMM

[Qemu-devel] [PATCH 15/21] target-arm: Widen thread-local register state fields to 64 bits

[Peter Maydell]

The common pattern for system registers in a 64-bit capable ARM
CPU is that when in AArch32 the cp15 register is a view of the
bottom 32 bits of the 64-bit AArch64 system register; writes in
AArch32 leave the top half unchanged. The most natural way to
model this is to have the state field in the CPU struct be a
64 bit value, and simply have the AArch32 TCG code operate on
a pointer to its lower half.

For aarch64-linux-user the only registers we need to share like
this are the thread-local-storage ones. Widen their fields to
64 bits and provide the 64 bit reginfo struct to make them
visible in AArch64 state.

Since we're touching almost every line in QEMU that uses the
c13_tls* fields in this patch anyway, we take the opportunity
to rename them in line with the standard ARM architectural names
for these registers.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/aarch64/target_cpu.h |  5 ++++-
 linux-user/arm/target_cpu.h     |  2 +-
 linux-user/main.c               |  2 +-
 target-arm/cpu.h                | 18 +++++++++++++++---
 target-arm/helper.c             | 15 ++++++++++++---
 5 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/linux-user/aarch64/target_cpu.h b/linux-user/aarch64/target_cpu.h
index 6f5539b..21560ef 100644
--- a/linux-user/aarch64/target_cpu.h
+++ b/linux-user/aarch64/target_cpu.h
@@ -29,7 +29,10 @@ static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp)
 
 static inline void cpu_set_tls(CPUARMState *env, target_ulong newtls)
 {
-    env->sr.tpidr_el0 = newtls;
+    /* Note that AArch64 Linux keeps the TLS pointer in TPIDR; this is
+     * different from AArch32 Linux, which uses TPIDRRO.
+     */
+    env->cp15.tpidr_el0 = newtls;
 }
 
 #endif
diff --git a/linux-user/arm/target_cpu.h b/linux-user/arm/target_cpu.h
index ed323c0..39d65b6 100644
--- a/linux-user/arm/target_cpu.h
+++ b/linux-user/arm/target_cpu.h
@@ -29,7 +29,7 @@ static inline void cpu_clone_regs(CPUARMState *env, target_ulong newsp)
 
 static inline void cpu_set_tls(CPUARMState *env, target_ulong newtls)
 {
-    env->cp15.c13_tls2 = newtls;
+    env->cp15.tpidrro_el0 = newtls;
 }
 
 #endif
diff --git a/linux-user/main.c b/linux-user/main.c
index 54f71fe..c0df8b5 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -566,7 +566,7 @@ do_kernel_trap(CPUARMState *env)
         end_exclusive();
         break;
     case 0xffff0fe0: /* __kernel_get_tls */
-        env->regs[0] = env->cp15.c13_tls2;
+        env->regs[0] = env->cp15.tpidrro_el0;
         break;
     case 0xffff0f60: /* __kernel_cmpxchg64 */
         arm_kernel_cmpxchg64_helper(env);
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 32387b0..81c0b1c 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -66,6 +66,18 @@
 /* ARM-specific interrupt pending bits.  */
 #define CPU_INTERRUPT_FIQ   CPU_INTERRUPT_TGT_EXT_1
 
+/* The usual mapping for an AArch64 system register to its AArch32
+ * counterpart is for the 32 bit world to have access to the lower
+ * half only (with writes leaving the upper half untouched). It's
+ * therefore useful to be able to pass TCG the offset of the least
+ * significant half of a uint64_t struct member.
+ */
+#ifdef HOST_WORDS_BIGENDIAN
+#define offsetoflow32(S, M) (offsetof(S, M + sizeof(uint32_t))
+#else
+#define offsetoflow32(S, M) offsetof(S, M)
+#endif
+
 /* Meanings of the ARMCPU object's two inbound GPIO lines */
 #define ARM_CPU_IRQ 0
 #define ARM_CPU_FIQ 1
@@ -188,9 +200,9 @@ typedef struct CPUARMState {
         uint32_t c12_vbar; /* vector base address register */
         uint32_t c13_fcse; /* FCSE PID.  */
         uint32_t c13_context; /* Context ID.  */
-        uint32_t c13_tls1; /* User RW Thread register.  */
-        uint32_t c13_tls2; /* User RO Thread register.  */
-        uint32_t c13_tls3; /* Privileged Thread register.  */
+        uint64_t tpidr_el0; /* User RW Thread register.  */
+        uint64_t tpidrro_el0; /* User RO Thread register.  */
+        uint64_t tpidr_el1; /* Privileged Thread register.  */
         uint32_t c14_cntfrq; /* Counter Frequency register */
         uint32_t c14_cntkctl; /* Timer Control register */
         ARMGenericTimer c14_timer[NUM_GTIMERS];
diff --git a/target-arm/helper.c b/target-arm/helper.c
index c64f618..0754058 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -742,15 +742,15 @@ static const ARMCPRegInfo t2ee_cp_reginfo[] = {
 static const ARMCPRegInfo v6k_cp_reginfo[] = {
     { .name = "TPIDRURW", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2 = 2,
       .access = PL0_RW,
-      .fieldoffset = offsetof(CPUARMState, cp15.c13_tls1),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.tpidr_el0),
       .resetvalue = 0 },
     { .name = "TPIDRURO", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2 = 3,
       .access = PL0_R|PL1_W,
-      .fieldoffset = offsetof(CPUARMState, cp15.c13_tls2),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.tpidrro_el0),
       .resetvalue = 0 },
     { .name = "TPIDRPRW", .cp = 15, .crn = 13, .crm = 0, .opc1 = 0, .opc2 = 4,
       .access = PL1_RW,
-      .fieldoffset = offsetof(CPUARMState, cp15.c13_tls3),
+      .fieldoffset = offsetoflow32(CPUARMState, cp15.tpidr_el1),
       .resetvalue = 0 },
     REGINFO_SENTINEL
 };
@@ -1612,6 +1612,15 @@ static const ARMCPRegInfo aarch64_cp_reginfo[] = {
     { .name = "DCZID_EL0", .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
       .access = PL0_R, .type = ARM_CP_CONST | ARM_CP_AA64,
       .resetvalue = 0x10 },
+    { .name = "TPIDR_EL0", .opc0 = 3, .opc1 = 3, .opc2 = 2, .crn = 13, .crm = 0,
+      .access = PL0_RW, .type = ARM_CP_AA64,
+      .fieldoffset = offsetof(CPUARMState, cp15.tpidr_el0), .resetvalue = 0 },
+    { .name = "TPIDRRO_EL0", .opc0 = 3, .opc1 = 3, .opc2 = 3,
+      .crn = 13, .crm = 0, .access = PL0_R|PL1_W, .type = ARM_CP_AA64,
+      .fieldoffset = offsetof(CPUARMState, cp15.tpidrro_el0), .resetvalue = 0 },
+    { .name = "TPIDR_EL1", .opc0 = 3, .opc1 = 0, .opc2 = 4, .crn = 13, .crm = 0,
+      .access = PL1_RW, .type = ARM_CP_AA64,
+      .fieldoffset = offsetof(CPUARMState, cp15.tpidr_el1), .resetvalue = 0 },
     REGINFO_SENTINEL
 };
 
-- 
1.8.5

Re: [Qemu-devel] [PATCH 15/21] target-arm: Widen thread-local register state fields to 64 bits

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> -        uint32_t c13_tls1; /* User RW Thread register.  */
> -        uint32_t c13_tls2; /* User RO Thread register.  */
> -        uint32_t c13_tls3; /* Privileged Thread register.  */
> +        uint64_t tpidr_el0; /* User RW Thread register.  */
> +        uint64_t tpidrro_el0; /* User RO Thread register.  */
> +        uint64_t tpidr_el1; /* Privileged Thread register.  */

Not target_ulong, continuing to use 32bit slot for pure AA32?


r~

Re: [Qemu-devel] [PATCH 15/21] target-arm: Widen thread-local register state fields to 64 bits

[Peter Maydell]

On 19 December 2013 20:53, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> -        uint32_t c13_tls1; /* User RW Thread register.  */
>> -        uint32_t c13_tls2; /* User RO Thread register.  */
>> -        uint32_t c13_tls3; /* Privileged Thread register.  */
>> +        uint64_t tpidr_el0; /* User RW Thread register.  */
>> +        uint64_t tpidrro_el0; /* User RO Thread register.  */
>> +        uint64_t tpidr_el1; /* Privileged Thread register.  */
>
> Not target_ulong, continuing to use 32bit slot for pure AA32?

It would only be a 32 bit slot for the 32 bit cores in qemu-arm;
the same cores in qemu-aarch64 would be 64 bits. I think
I'd rather have the consistency (and the ability to migrate
between a qemu-arm cortex-a9 and a qemu-aarch64 cortex-a9
doesn't hurt).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 15/21] target-arm: Widen thread-local register state fields to 64 bits

[Richard Henderson]

On 12/19/2013 01:04 PM, Peter Maydell wrote:
> On 19 December 2013 20:53, Richard Henderson <rth@twiddle.net> wrote:
>> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>>> -        uint32_t c13_tls1; /* User RW Thread register.  */
>>> -        uint32_t c13_tls2; /* User RO Thread register.  */
>>> -        uint32_t c13_tls3; /* Privileged Thread register.  */
>>> +        uint64_t tpidr_el0; /* User RW Thread register.  */
>>> +        uint64_t tpidrro_el0; /* User RO Thread register.  */
>>> +        uint64_t tpidr_el1; /* Privileged Thread register.  */
>>
>> Not target_ulong, continuing to use 32bit slot for pure AA32?
> 
> It would only be a 32 bit slot for the 32 bit cores in qemu-arm;
> the same cores in qemu-aarch64 would be 64 bits. I think
> I'd rather have the consistency (and the ability to migrate
> between a qemu-arm cortex-a9 and a qemu-aarch64 cortex-a9
> doesn't hurt).

Fair enough.

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


r~

[Qemu-devel] [PATCH 16/21] target-arm: A64: add support for add/sub with carry

[Peter Maydell]

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

This patch adds support for C3.5.3 Add/subtract (with carry):
instructions ADC, ADCS, SBC, SBCS.

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

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index a7d2b60..4700d6e 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -400,6 +400,71 @@ static void gen_sub_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
     }
 }
 
+/* dest = T0 + T1 + CF; do not compute flags. */
+static void gen_adc(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+    TCGv_i64 flag = tcg_temp_new_i64();
+    tcg_gen_extu_i32_i64(flag, cpu_CF);
+    tcg_gen_add_i64(dest, t0, t1);
+    tcg_gen_add_i64(dest, dest, flag);
+    tcg_temp_free_i64(flag);
+
+    if (!sf) {
+        tcg_gen_ext32u_i64(dest, dest);
+    }
+}
+
+/* dest = T0 + T1 + CF; compute C, N, V and Z flags. */
+static void gen_adc_CC(int sf, TCGv_i64 dest, TCGv_i64 t0, TCGv_i64 t1)
+{
+    if (sf) {
+        TCGv_i64 result, cf_64, vf_64, tmp;
+        result = tcg_temp_new_i64();
+        cf_64 = tcg_temp_new_i64();
+        vf_64 = tcg_temp_new_i64();
+        tmp = tcg_const_i64(0);
+
+        tcg_gen_extu_i32_i64(cf_64, cpu_CF);
+        tcg_gen_add2_i64(result, cf_64, t0, tmp, cf_64, tmp);
+        tcg_gen_add2_i64(result, cf_64, result, cf_64, t1, tmp);
+        tcg_gen_trunc_i64_i32(cpu_CF, cf_64);
+        gen_set_NZ64(result);
+
+        tcg_gen_xor_i64(vf_64, result, t0);
+        tcg_gen_xor_i64(tmp, t0, t1);
+        tcg_gen_andc_i64(vf_64, vf_64, tmp);
+        tcg_gen_shri_i64(vf_64, vf_64, 32);
+        tcg_gen_trunc_i64_i32(cpu_VF, vf_64);
+
+        tcg_gen_mov_i64(dest, result);
+
+        tcg_temp_free_i64(tmp);
+        tcg_temp_free_i64(vf_64);
+        tcg_temp_free_i64(cf_64);
+        tcg_temp_free_i64(result);
+    } else {
+        TCGv_i32 t0_32, t1_32, tmp;
+        t0_32 = tcg_temp_new_i32();
+        t1_32 = tcg_temp_new_i32();
+        tmp = tcg_const_i32(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, cpu_CF, tmp);
+        tcg_gen_add2_i32(cpu_NF, cpu_CF, cpu_NF, cpu_CF, 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(t1_32);
+        tcg_temp_free_i32(t0_32);
+    }
+}
+
 /*
  * Load/Store generators
  */
@@ -2376,10 +2441,46 @@ static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
     tcg_temp_free_i64(tcg_tmp);
 }
 
-/* Add/subtract (with carry) */
+/* C3.5.3 - Add/subtract (with carry)
+ *  31 30 29 28 27 26 25 24 23 22 21  20  16  15   10  9    5 4   0
+ * +--+--+--+------------------------+------+---------+------+-----+
+ * |sf|op| S| 1  1  0  1  0  0  0  0 |  rm  | opcode2 |  Rn  |  Rd |
+ * +--+--+--+------------------------+------+---------+------+-----+
+ *                                            [000000]
+ */
+
 static void disas_adc_sbc(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    unsigned int sf, op, setflags, rm, rn, rd;
+    TCGv_i64 tcg_y, tcg_rn, tcg_rd;
+
+    if (extract32(insn, 10, 6) != 0) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    sf = extract32(insn, 31, 1);
+    op = extract32(insn, 30, 1);
+    setflags = extract32(insn, 29, 1);
+    rm = extract32(insn, 16, 5);
+    rn = extract32(insn, 5, 5);
+    rd = extract32(insn, 0, 5);
+
+    tcg_rd = cpu_reg(s, rd);
+    tcg_rn = cpu_reg(s, rn);
+
+    if (op) {
+        tcg_y = new_tmp_a64(s);
+        tcg_gen_not_i64(tcg_y, cpu_reg(s, rm));
+    } else {
+        tcg_y = cpu_reg(s, rm);
+    }
+
+    if (setflags) {
+        gen_adc_CC(sf, tcg_rd, tcg_rn, tcg_y);
+    } else {
+        gen_adc(sf, tcg_rd, tcg_rn, tcg_y);
+    }
 }
 
 /* Conditional compare (immediate) */
-- 
1.8.5

Re: [Qemu-devel] [PATCH 16/21] target-arm: A64: add support for add/sub with carry

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> From: Claudio Fontana <claudio.fontana@linaro.org>
> 
> This patch adds support for C3.5.3 Add/subtract (with carry):
> instructions ADC, ADCS, SBC, SBCS.
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/translate-a64.c | 105 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 2 deletions(-)

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


r~

[Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Peter Maydell]

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

this patch adds support for C3.5.4 - C3.5.5
Conditional compare (both immediate and register)

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

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 4700d6e..3c18d6f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2483,16 +2483,66 @@ static void disas_adc_sbc(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Conditional compare (immediate) */
-static void disas_cc_imm(DisasContext *s, uint32_t insn)
+/* C3.5.4 - C3.5.5 Conditional compare (immediate / register)
+ *  31 30 29 28 27 26 25 24 23 22 21  20    16 15  12  11  10  9   5  4 3   0
+ * +--+--+--+------------------------+--------+------+----+--+------+--+-----+
+ * |sf|op| S| 1  1  0  1  0  0  1  0 |imm5/rm | cond |i/r |o2|  Rn  |o3|nzcv |
+ * +--+--+--+------------------------+--------+------+----+--+------+--+-----+
+ *        [1]                             y                [0]       [0]
+ */
+static void disas_cc(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
-}
+    unsigned int sf, op, y, cond, rn, nzcv, is_imm;
+    int label_continue;
+    TCGv_i64 tcg_tmp, tcg_y, tcg_rn;
 
-/* Conditional compare (register) */
-static void disas_cc_reg(DisasContext *s, uint32_t insn)
-{
-    unsupported_encoding(s, insn);
+    if (!extract32(insn, 29, 1)) {
+        unallocated_encoding(s);
+        return;
+    }
+    if (insn & (1 << 10 | 1 << 4)) {
+        unallocated_encoding(s);
+        return;
+    }
+    sf = extract32(insn, 31, 1);
+    op = extract32(insn, 30, 1);
+    is_imm = extract32(insn, 11, 1);
+    y = extract32(insn, 16, 5); /* y = rm (reg) or imm5 (imm) */
+    cond = extract32(insn, 12, 4);
+    rn = extract32(insn, 5, 5);
+    nzcv = extract32(insn, 0, 4);
+
+    tcg_tmp = tcg_temp_new_i64();
+
+    if (cond < 0x0e) { /* not always */
+        int label_match = gen_new_label();
+        label_continue = gen_new_label();
+        arm_gen_test_cc(cond, label_match);
+        /* nomatch: */
+        tcg_gen_movi_i64(tcg_tmp, nzcv << 28);
+        gen_set_nzcv(tcg_tmp);
+        tcg_gen_br(label_continue);
+        gen_set_label(label_match);
+    }
+    /* match, or condition is always */
+    if (is_imm) {
+        tcg_y = new_tmp_a64(s);
+        tcg_gen_movi_i64(tcg_y, y);
+    } else {
+        tcg_y = cpu_reg(s, y);
+    }
+    tcg_rn = cpu_reg(s, rn);
+    if (op) {
+        gen_sub_CC(sf, tcg_tmp, tcg_rn, tcg_y);
+    } else {
+        gen_add_CC(sf, tcg_tmp, tcg_rn, tcg_y);
+    }
+
+    tcg_temp_free_i64(tcg_tmp);
+
+    if (cond < 0x0e) { /* continue */
+        gen_set_label(label_continue);
+    }
 }
 
 /* C3.5.6 Conditional select
@@ -2846,11 +2896,7 @@ static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
             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);
-            }
+            disas_cc(s, insn); /* both imm and reg forms */
             break;
         case 0x4: /* Conditional select */
             disas_cond_select(s, insn);
-- 
1.8.5

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +static void disas_cc(DisasContext *s, uint32_t insn)
>  {
> +    unsigned int sf, op, y, cond, rn, nzcv, is_imm;
> +    int label_continue;
> +    TCGv_i64 tcg_tmp, tcg_y, tcg_rn;

Don't you get a may be used uninitialized warning for label_continue?


> +    tcg_tmp = tcg_temp_new_i64();
> +
> +    if (cond < 0x0e) { /* not always */
> +        int label_match = gen_new_label();
> +        label_continue = gen_new_label();
> +        arm_gen_test_cc(cond, label_match);
> +        /* nomatch: */
> +        tcg_gen_movi_i64(tcg_tmp, nzcv << 28);
> +        gen_set_nzcv(tcg_tmp);
> +        tcg_gen_br(label_continue);
> +        gen_set_label(label_match);
> +    }

You can't re-use tcg_tmp across basic blocks like this.


r~

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Peter Maydell]

On 19 December 2013 21:04, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>
>> +    tcg_tmp = tcg_temp_new_i64();
>> +
>> +    if (cond < 0x0e) { /* not always */
>> +        int label_match = gen_new_label();
>> +        label_continue = gen_new_label();
>> +        arm_gen_test_cc(cond, label_match);
>> +        /* nomatch: */
>> +        tcg_gen_movi_i64(tcg_tmp, nzcv << 28);
>> +        gen_set_nzcv(tcg_tmp);
>> +        tcg_gen_br(label_continue);
>> +        gen_set_label(label_match);
>> +    }
>
> You can't re-use tcg_tmp across basic blocks like this.

Hmm. I clearly don't entirely understand the rules here.
The TCG README says "temporaries are only live
in a basic block" and "After the end of a basic block, the
content of temporaries is destroyed", which I took to
mean that the value in the temp was dead after the
branch, but that I could freely reuse it for something
else afterwards as long as I wrote a new value to
it (as we're doing here). I guess that's wrong?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Richard Henderson]

On 12/19/2013 01:23 PM, Peter Maydell wrote:
> Hmm. I clearly don't entirely understand the rules here.
> The TCG README says "temporaries are only live
> in a basic block" and "After the end of a basic block, the
> content of temporaries is destroyed", which I took to
> mean that the value in the temp was dead after the
> branch, but that I could freely reuse it for something
> else afterwards as long as I wrote a new value to
> it (as we're doing here). I guess that's wrong?

You need to re-allocate it in each basic block.

We don't have enough tcg checking to find errors like this.


r~

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Peter Maydell]

On 19 December 2013 21:26, Richard Henderson <rth@twiddle.net> wrote:
> On 12/19/2013 01:23 PM, Peter Maydell wrote:
>> Hmm. I clearly don't entirely understand the rules here.
>> The TCG README says "temporaries are only live
>> in a basic block" and "After the end of a basic block, the
>> content of temporaries is destroyed", which I took to
>> mean that the value in the temp was dead after the
>> branch, but that I could freely reuse it for something
>> else afterwards as long as I wrote a new value to
>> it (as we're doing here). I guess that's wrong?
>
> You need to re-allocate it in each basic block.
>
> We don't have enough tcg checking to find errors like this.

We could probably at least update the README to
not be misleading :-)

thanks
-- PMM

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Peter Maydell]

On 19 December 2013 21:04, Richard Henderson <rth@twiddle.net> wrote:
> On 12/17/2013 07:12 AM, Peter Maydell wrote:
>> +static void disas_cc(DisasContext *s, uint32_t insn)
>>  {
>> +    unsigned int sf, op, y, cond, rn, nzcv, is_imm;
>> +    int label_continue;
>> +    TCGv_i64 tcg_tmp, tcg_y, tcg_rn;
>
> Don't you get a may be used uninitialized warning for label_continue?

I know some versions of gcc tend to be bad at that,
but at least for mine (gcc-4.6.3) it doesn't warn about this
even with optimization enabled.

Is it worth pulling the assignment
  label_continue = gen_new_label();
out of the if() to protect against possible less smart compilers?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 17/21] target-arm: A64: add support for conditional compare insns

[Richard Henderson]

On 12/20/2013 08:19 AM, Peter Maydell wrote:
> Is it worth pulling the assignment
>   label_continue = gen_new_label();
> out of the if() to protect against possible less smart compilers?

Sure.  Or even initializing to -1.


r~

[Qemu-devel] [PATCH 18/21] target-arm: aarch64: add support for ld lit

[Peter Maydell]

From: Alexander Graf <agraf@suse.de>

Adds support for Load Register (literal), both normal
and SIMD/FP forms.

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 | 47 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 3c18d6f..e4bca31 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1117,10 +1117,53 @@ static void disas_ldst_excl(DisasContext *s, uint32_t insn)
     unsupported_encoding(s, insn);
 }
 
-/* Load register (literal) */
+/*
+ * C3.3.5 Load register (literal)
+ *
+ *  31 30 29   27  26 25 24 23                5 4     0
+ * +-----+-------+---+-----+-------------------+-------+
+ * | opc | 0 1 1 | V | 0 0 |     imm19         |  Rt   |
+ * +-----+-------+---+-----+-------------------+-------+
+ *
+ * V: 1 -> vector (simd/fp)
+ * opc (non-vector): 00 -> 32 bit, 01 -> 64 bit,
+ *                   10-> 32 bit signed, 11 -> prefetch
+ * opc (vector): 00 -> 32 bit, 01 -> 64 bit, 10 -> 128 bit (11 unallocated)
+ */
 static void disas_ld_lit(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rt = extract32(insn, 0, 5);
+    int64_t imm = sextract32(insn, 5, 19) << 2;
+    bool is_vector = extract32(insn, 26, 1);
+    int opc = extract32(insn, 30, 2);
+    bool is_signed = false;
+    int size = 2;
+    TCGv_i64 tcg_rt, tcg_addr;
+
+    if (is_vector) {
+        if (opc == 3) {
+            unallocated_encoding(s);
+            return;
+        }
+        size = 2 + opc;
+    } else {
+        if (opc == 3) {
+            /* PRFM (literal) : prefetch */
+            return;
+        }
+        size = 2 + extract32(opc, 0, 1);
+        is_signed = extract32(opc, 1, 1);
+    }
+
+    tcg_rt = cpu_reg(s, rt);
+
+    tcg_addr = tcg_const_i64((s->pc - 4) + imm);
+    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_temp_free_i64(tcg_addr);
 }
 
 /*
-- 
1.8.5

Re: [Qemu-devel] [PATCH 18/21] target-arm: aarch64: add support for ld lit

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> From: Alexander Graf <agraf@suse.de>
> 
> Adds support for Load Register (literal), both normal
> and SIMD/FP forms.
> 
> 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 | 47 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)

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


r~

[Qemu-devel] [PATCH 19/21] target-arm: Widen exclusive-access support struct fields to 64 bits

[Peter Maydell]

In preparation for adding support for A64 load/store exclusive instructions,
widen the fields in the CPU state struct that deal with address and data values
for exclusives from 32 to 64 bits. Although in practice AArch64 and AArch32
exclusive accesses will be generally separate there are some odd theoretical
corner cases (eg you should be able to do the exclusive load in AArch32, take
an exception to AArch64 and successfully do the store exclusive there), and it's
also easier to reason about.

The changes in semantics for the variables are:
 exclusive_addr  -> extended to 64 bits; -1ULL for "monitor lost",
   otherwise always < 2^32 for AArch32
 exclusive_val   -> extended to 64 bits. 64 bit exclusives in AArch32 now
   use the high half of exclusive_val instead of a separate exclusive_high
 exclusive_high  -> is no longer used in AArch32; extended to 64 bits as
   it will be needed for AArch64's pair-of-64-bit-values exclusives.
 exclusive_test  -> extended to 64 bits, as it is an address. Since this is
   a linux-user-only field, in arm-linux-user it will always have the top
   32 bits zero.
 exclusive_info  -> stays 32 bits, as it is neither data nor address, but
   simply holds register indexes etc. AArch64 will be able to fit all its
   information into 32 bits as well.

Note that the refactoring of gen_store_exclusive() coincidentally fixes
a minor bug where ldrexd would incorrectly update the first CPU register
even if the load for the second register faulted.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/main.c      | 25 +++++++++++--------
 target-arm/cpu.h       |  8 +++----
 target-arm/machine.c   | 12 +++++-----
 target-arm/translate.c | 65 ++++++++++++++++++++++++++++++--------------------
 4 files changed, 64 insertions(+), 46 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index c0df8b5..20f9832 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -589,16 +589,21 @@ do_kernel_trap(CPUARMState *env)
 
 static int do_strex(CPUARMState *env)
 {
-    uint32_t val;
+    uint64_t val;
     int size;
     int rc = 1;
     int segv = 0;
     uint32_t addr;
     start_exclusive();
-    addr = env->exclusive_addr;
-    if (addr != env->exclusive_test) {
+    if (env->exclusive_addr != env->exclusive_test) {
         goto fail;
     }
+    /* We know we're always AArch32 so the address is in uint32_t range
+     * unless it was the -1 exclusive-monitor-lost value (which won't
+     * match exclusive_test above).
+     */
+    assert(extract64(env->exclusive_addr, 32, 32) == 0);
+    addr = env->exclusive_addr;
     size = env->exclusive_info & 0xf;
     switch (size) {
     case 0:
@@ -618,19 +623,19 @@ static int do_strex(CPUARMState *env)
         env->cp15.c6_data = addr;
         goto done;
     }
-    if (val != env->exclusive_val) {
-        goto fail;
-    }
     if (size == 3) {
-        segv = get_user_u32(val, addr + 4);
+        uint32_t valhi;
+        segv = get_user_u32(valhi, addr + 4);
         if (segv) {
             env->cp15.c6_data = addr + 4;
             goto done;
         }
-        if (val != env->exclusive_high) {
-            goto fail;
-        }
+        val = deposit64(val, 32, 32, valhi);
+    }
+    if (val != env->exclusive_val) {
+        goto fail;
     }
+
     val = env->regs[(env->exclusive_info >> 8) & 0xf];
     switch (size) {
     case 0:
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 81c0b1c..744d1dd 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -278,11 +278,11 @@ typedef struct CPUARMState {
         float_status fp_status;
         float_status standard_fp_status;
     } vfp;
-    uint32_t exclusive_addr;
-    uint32_t exclusive_val;
-    uint32_t exclusive_high;
+    uint64_t exclusive_addr;
+    uint64_t exclusive_val;
+    uint64_t exclusive_high;
 #if defined(CONFIG_USER_ONLY)
-    uint32_t exclusive_test;
+    uint64_t exclusive_test;
     uint32_t exclusive_info;
 #endif
 
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 74f010f..8f9e7d4 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -222,9 +222,9 @@ static int cpu_post_load(void *opaque, int version_id)
 
 const VMStateDescription vmstate_arm_cpu = {
     .name = "cpu",
-    .version_id = 13,
-    .minimum_version_id = 13,
-    .minimum_version_id_old = 13,
+    .version_id = 14,
+    .minimum_version_id = 14,
+    .minimum_version_id_old = 14,
     .pre_save = cpu_pre_save,
     .post_load = cpu_post_load,
     .fields = (VMStateField[]) {
@@ -253,9 +253,9 @@ const VMStateDescription vmstate_arm_cpu = {
         VMSTATE_VARRAY_INT32(cpreg_vmstate_values, ARMCPU,
                              cpreg_vmstate_array_len,
                              0, vmstate_info_uint64, uint64_t),
-        VMSTATE_UINT32(env.exclusive_addr, ARMCPU),
-        VMSTATE_UINT32(env.exclusive_val, ARMCPU),
-        VMSTATE_UINT32(env.exclusive_high, ARMCPU),
+        VMSTATE_UINT64(env.exclusive_addr, ARMCPU),
+        VMSTATE_UINT64(env.exclusive_val, ARMCPU),
+        VMSTATE_UINT64(env.exclusive_high, ARMCPU),
         VMSTATE_UINT64(env.features, ARMCPU),
         VMSTATE_TIMER(gt_timer[GTIMER_PHYS], ARMCPU),
         VMSTATE_TIMER(gt_timer[GTIMER_VIRT], ARMCPU),
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 8bfe950..4387547 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -61,11 +61,10 @@ TCGv_ptr cpu_env;
 static TCGv_i64 cpu_V0, cpu_V1, cpu_M0;
 static TCGv_i32 cpu_R[16];
 static TCGv_i32 cpu_CF, cpu_NF, cpu_VF, cpu_ZF;
-static TCGv_i32 cpu_exclusive_addr;
-static TCGv_i32 cpu_exclusive_val;
-static TCGv_i32 cpu_exclusive_high;
+static TCGv_i64 cpu_exclusive_addr;
+static TCGv_i64 cpu_exclusive_val;
 #ifdef CONFIG_USER_ONLY
-static TCGv_i32 cpu_exclusive_test;
+static TCGv_i64 cpu_exclusive_test;
 static TCGv_i32 cpu_exclusive_info;
 #endif
 
@@ -96,14 +95,12 @@ void arm_translate_init(void)
     cpu_VF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, VF), "VF");
     cpu_ZF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, ZF), "ZF");
 
-    cpu_exclusive_addr = tcg_global_mem_new_i32(TCG_AREG0,
+    cpu_exclusive_addr = tcg_global_mem_new_i64(TCG_AREG0,
         offsetof(CPUARMState, exclusive_addr), "exclusive_addr");
-    cpu_exclusive_val = tcg_global_mem_new_i32(TCG_AREG0,
+    cpu_exclusive_val = tcg_global_mem_new_i64(TCG_AREG0,
         offsetof(CPUARMState, exclusive_val), "exclusive_val");
-    cpu_exclusive_high = tcg_global_mem_new_i32(TCG_AREG0,
-        offsetof(CPUARMState, exclusive_high), "exclusive_high");
 #ifdef CONFIG_USER_ONLY
-    cpu_exclusive_test = tcg_global_mem_new_i32(TCG_AREG0,
+    cpu_exclusive_test = tcg_global_mem_new_i64(TCG_AREG0,
         offsetof(CPUARMState, exclusive_test), "exclusive_test");
     cpu_exclusive_info = tcg_global_mem_new_i32(TCG_AREG0,
         offsetof(CPUARMState, exclusive_info), "exclusive_info");
@@ -6758,30 +6755,34 @@ static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
     default:
         abort();
     }
-    tcg_gen_mov_i32(cpu_exclusive_val, tmp);
-    store_reg(s, rt, tmp);
+
     if (size == 3) {
         TCGv_i32 tmp2 = tcg_temp_new_i32();
+        TCGv_i32 tmp3 = tcg_temp_new_i32();
+
         tcg_gen_addi_i32(tmp2, addr, 4);
-        tmp = tcg_temp_new_i32();
-        gen_aa32_ld32u(tmp, tmp2, IS_USER(s));
+        gen_aa32_ld32u(tmp3, tmp2, IS_USER(s));
         tcg_temp_free_i32(tmp2);
-        tcg_gen_mov_i32(cpu_exclusive_high, tmp);
-        store_reg(s, rt2, tmp);
+        tcg_gen_concat_i32_i64(cpu_exclusive_val, tmp, tmp3);
+        store_reg(s, rt2, tmp3);
+    } else {
+        tcg_gen_extu_i32_i64(cpu_exclusive_val, tmp);
     }
-    tcg_gen_mov_i32(cpu_exclusive_addr, addr);
+
+    store_reg(s, rt, tmp);
+    tcg_gen_extu_i32_i64(cpu_exclusive_addr, addr);
 }
 
 static void gen_clrex(DisasContext *s)
 {
-    tcg_gen_movi_i32(cpu_exclusive_addr, -1);
+    tcg_gen_movi_i64(cpu_exclusive_addr, -1);
 }
 
 #ifdef CONFIG_USER_ONLY
 static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
                                 TCGv_i32 addr, int size)
 {
-    tcg_gen_mov_i32(cpu_exclusive_test, addr);
+    tcg_gen_extu_i32_i64(cpu_exclusive_test, addr);
     tcg_gen_movi_i32(cpu_exclusive_info,
                      size | (rd << 4) | (rt << 8) | (rt2 << 12));
     gen_exception_insn(s, 4, EXCP_STREX);
@@ -6791,6 +6792,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
                                 TCGv_i32 addr, int size)
 {
     TCGv_i32 tmp;
+    TCGv_i64 val64, extaddr;
     int done_label;
     int fail_label;
 
@@ -6802,7 +6804,11 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
        } */
     fail_label = gen_new_label();
     done_label = gen_new_label();
-    tcg_gen_brcond_i32(TCG_COND_NE, addr, cpu_exclusive_addr, fail_label);
+    extaddr = tcg_temp_new_i64();
+    tcg_gen_extu_i32_i64(extaddr, addr);
+    tcg_gen_brcond_i64(TCG_COND_NE, extaddr, cpu_exclusive_addr, fail_label);
+    tcg_temp_free_i64(extaddr);
+
     tmp = tcg_temp_new_i32();
     switch (size) {
     case 0:
@@ -6818,17 +6824,24 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
     default:
         abort();
     }
-    tcg_gen_brcond_i32(TCG_COND_NE, tmp, cpu_exclusive_val, fail_label);
-    tcg_temp_free_i32(tmp);
+
+    val64 = tcg_temp_new_i64();
     if (size == 3) {
         TCGv_i32 tmp2 = tcg_temp_new_i32();
+        TCGv_i32 tmp3 = tcg_temp_new_i32();
         tcg_gen_addi_i32(tmp2, addr, 4);
-        tmp = tcg_temp_new_i32();
-        gen_aa32_ld32u(tmp, tmp2, IS_USER(s));
+        gen_aa32_ld32u(tmp3, tmp2, IS_USER(s));
         tcg_temp_free_i32(tmp2);
-        tcg_gen_brcond_i32(TCG_COND_NE, tmp, cpu_exclusive_high, fail_label);
-        tcg_temp_free_i32(tmp);
+        tcg_gen_concat_i32_i64(val64, tmp, tmp3);
+        tcg_temp_free_i32(tmp3);
+    } else {
+        tcg_gen_extu_i32_i64(val64, tmp);
     }
+    tcg_temp_free_i32(tmp);
+
+    tcg_gen_brcond_i64(TCG_COND_NE, val64, cpu_exclusive_val, fail_label);
+    tcg_temp_free_i64(val64);
+
     tmp = load_reg(s, rt);
     switch (size) {
     case 0:
@@ -6856,7 +6869,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
     gen_set_label(fail_label);
     tcg_gen_movi_i32(cpu_R[rd], 1);
     gen_set_label(done_label);
-    tcg_gen_movi_i32(cpu_exclusive_addr, -1);
+    tcg_gen_movi_i64(cpu_exclusive_addr, -1);
 }
 #endif
 
-- 
1.8.5

[Qemu-devel] [PATCH 20/21] target-arm: A64: support for ld/st/cl exclusive

[Peter Maydell]

From: Michael Matz <matz@suse.de>

This implement exclusive loads/stores for aarch64 along the lines of
arm32 and ppc implementations. The exclusive load remembers the address
and loaded value. The exclusive store throws an an exception which uses
those values to check for equality in a proper exclusive region.

This is not actually the architecture mandated semantics (for either
AArch32 or AArch64) but it is close enough for typical guest code
sequences to work correctly, and saves us from having to monitor all
guest stores. It's fairly easy to come up with test cases where we
don't behave like hardware - we don't for example model cache line
behaviour. However in the common patterns this works, and the existing
32 bit ARM exclusive access implementation has the same limitations.

AArch64 also implements new acquire/release loads/stores (which may be
either exclusive or non-exclusive). These imposes extra ordering
constraints on memory operations (ie they act as if they have an implicit
barrier built into them). As TCG is single-threaded all our barriers
are no-ops, so these just behave like normal loads and stores.

Signed-off-by: Michael Matz <matz@suse.de>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---

v3 -> v4 (ajb):
   - use new gpr_ld API

v4 -> v5 (ajb):
   - merge in later fixes from Matz
   - fix bug with using ZR for status
   - add sp alignment check
   - fixup bogus whitespace

v5 -> v6 (ajb):
   - better naming of goto fields
   - implement clrex
   - clear exclusive lock after strex
   - clear exclusive lock after exceptions
   - fix up checkpatch violations

PMM updates:
 * rebase on top of separate 'widen the exclusive fields' patch
 * rearrange code in main.c so we don't pointlessly jump from
   do_strex() to do_strex_a64()
 * use extract32 in do_strex_a64
 * put clearing of excl. monitor at bottom of cpu_loop which makes
   more conceptual sense since in hw it happens on exception return
 * use read_cpu_reg_sp() rather than explicit temp
 * minor tweaking
 * store the fault address in do_strex_a64 rather than forgetting it
---
 linux-user/main.c          | 127 ++++++++++++++++++++++++++++++++-
 target-arm/translate-a64.c | 171 ++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 292 insertions(+), 6 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 20f9832..cabc9e1 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -585,8 +585,8 @@ do_kernel_trap(CPUARMState *env)
 
     return 0;
 }
-#endif
 
+/* Store exclusive handling for AArch32 */
 static int do_strex(CPUARMState *env)
 {
     uint64_t val;
@@ -670,7 +670,6 @@ done:
     return segv;
 }
 
-#ifdef TARGET_ABI32
 void cpu_loop(CPUARMState *env)
 {
     CPUState *cs = CPU(arm_env_get_cpu(env));
@@ -885,6 +884,122 @@ void cpu_loop(CPUARMState *env)
 
 #else
 
+/*
+ * Handle AArch64 store-release exclusive
+ *
+ * rs = gets the status result of store exclusive
+ * rt = is the register that is stored
+ * rt2 = is the second register store (in STP)
+ *
+ */
+static int do_strex_a64(CPUARMState *env)
+{
+    uint64_t val;
+    int size;
+    bool is_pair;
+    int rc = 1;
+    int segv = 0;
+    uint64_t addr;
+    int rs, rt, rt2;
+
+    start_exclusive();
+    /* size | is_pair << 2 | (rs << 4) | (rt << 9) | (rt2 << 14)); */
+    size = extract32(env->exclusive_info, 0, 2);
+    is_pair = extract32(env->exclusive_info, 2, 1);
+    rs = extract32(env->exclusive_info, 4, 5);
+    rt = extract32(env->exclusive_info, 9, 5);
+    rt2 = extract32(env->exclusive_info, 14, 5);
+
+    addr = env->exclusive_addr;
+
+    if (addr != env->exclusive_test) {
+        goto finish;
+    }
+
+    switch (size) {
+    case 0:
+        segv = get_user_u8(val, addr);
+        break;
+    case 1:
+        segv = get_user_u16(val, addr);
+        break;
+    case 2:
+        segv = get_user_u32(val, addr);
+        break;
+    case 3:
+        segv = get_user_u64(val, addr);
+        break;
+    default:
+        abort();
+    }
+    if (segv) {
+        env->cp15.c6_data = addr;
+        goto error;
+    }
+    if (val != env->exclusive_val) {
+        goto finish;
+    }
+    if (is_pair) {
+        if (size == 2) {
+            segv = get_user_u32(val, addr + 4);
+        } else {
+            segv = get_user_u64(val, addr + 8);
+        }
+        if (segv) {
+            env->cp15.c6_data = addr + (size == 2 ? 4 : 8);
+            goto error;
+        }
+        if (val != env->exclusive_high) {
+            goto finish;
+        }
+    }
+    val = env->xregs[rt];
+    switch (size) {
+    case 0:
+        segv = put_user_u8(val, addr);
+        break;
+    case 1:
+        segv = put_user_u16(val, addr);
+        break;
+    case 2:
+        segv = put_user_u32(val, addr);
+        break;
+    case 3:
+        segv = put_user_u64(val, addr);
+        break;
+    }
+    if (segv) {
+        goto error;
+    }
+    if (is_pair) {
+        val = env->xregs[rt2];
+        if (size == 2) {
+            segv = put_user_u32(val, addr + 4);
+        } else {
+            segv = put_user_u64(val, addr + 8);
+        }
+        if (segv) {
+            env->cp15.c6_data = addr + (size == 2 ? 4 : 8);
+            goto error;
+        }
+    }
+    rc = 0;
+finish:
+    env->pc += 4;
+    /* rs == 31 encodes a write to the ZR, thus throwing away
+     * the status return. This is rather silly but valid.
+     */
+    if (rs < 31) {
+        env->xregs[rs] = rc;
+    }
+error:
+    /* instruction faulted, PC does not advance */
+    /* either way a strex releases any exclusive lock we have */
+    env->exclusive_addr = -1;
+    end_exclusive();
+    return segv;
+}
+
 /* AArch64 main loop */
 void cpu_loop(CPUARMState *env)
 {
@@ -944,7 +1059,7 @@ void cpu_loop(CPUARMState *env)
             }
             break;
         case EXCP_STREX:
-            if (do_strex(env)) {
+            if (do_strex_a64(env)) {
                 addr = env->cp15.c6_data;
                 goto do_segv;
             }
@@ -956,6 +1071,12 @@ void cpu_loop(CPUARMState *env)
             abort();
         }
         process_pending_signals(env);
+        /* Exception return on AArch64 always clears the exclusive monitor,
+         * so any return to running guest code implies this.
+         * A strex (successful or otherwise) also clears the monitor, so
+         * we don't need to specialcase EXCP_STREX.
+         */
+        env->exclusive_addr = -1;
     }
 }
 #endif /* ndef TARGET_ABI32 */
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index e4bca31..8344c5f 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -38,6 +38,15 @@ static TCGv_i64 cpu_X[32];
 static TCGv_i64 cpu_pc;
 static TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
 
+/* Load/store exclusive handling */
+static TCGv_i64 cpu_exclusive_addr;
+static TCGv_i64 cpu_exclusive_val;
+static TCGv_i64 cpu_exclusive_high;
+#ifdef CONFIG_USER_ONLY
+static TCGv_i64 cpu_exclusive_test;
+static TCGv_i32 cpu_exclusive_info;
+#endif
+
 static const char *regnames[] = {
     "x0", "x1", "x2", "x3", "x4", "x5", "x6", "x7",
     "x8", "x9", "x10", "x11", "x12", "x13", "x14", "x15",
@@ -70,6 +79,19 @@ void a64_translate_init(void)
     cpu_ZF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, ZF), "ZF");
     cpu_CF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, CF), "CF");
     cpu_VF = tcg_global_mem_new_i32(TCG_AREG0, offsetof(CPUARMState, VF), "VF");
+
+    cpu_exclusive_addr = tcg_global_mem_new_i64(TCG_AREG0,
+        offsetof(CPUARMState, exclusive_addr), "exclusive_addr");
+    cpu_exclusive_val = tcg_global_mem_new_i64(TCG_AREG0,
+        offsetof(CPUARMState, exclusive_val), "exclusive_val");
+    cpu_exclusive_high = tcg_global_mem_new_i64(TCG_AREG0,
+        offsetof(CPUARMState, exclusive_high), "exclusive_high");
+#ifdef CONFIG_USER_ONLY
+    cpu_exclusive_test = tcg_global_mem_new_i64(TCG_AREG0,
+        offsetof(CPUARMState, exclusive_test), "exclusive_test");
+    cpu_exclusive_info = tcg_global_mem_new_i32(TCG_AREG0,
+        offsetof(CPUARMState, exclusive_info), "exclusive_info");
+#endif
 }
 
 void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
@@ -772,6 +794,11 @@ static void handle_hint(DisasContext *s, uint32_t insn,
     }
 }
 
+static void gen_clrex(DisasContext *s, uint32_t insn)
+{
+    tcg_gen_movi_i64(cpu_exclusive_addr, -1);
+}
+
 /* CLREX, DSB, DMB, ISB */
 static void handle_sync(DisasContext *s, uint32_t insn,
                         unsigned int op1, unsigned int op2, unsigned int crm)
@@ -783,7 +810,7 @@ static void handle_sync(DisasContext *s, uint32_t insn,
 
     switch (op2) {
     case 2: /* CLREX */
-        unsupported_encoding(s, insn);
+        gen_clrex(s, insn);
         return;
     case 4: /* DSB */
     case 5: /* DMB */
@@ -1111,10 +1138,148 @@ static void disas_b_exc_sys(DisasContext *s, uint32_t insn)
     }
 }
 
-/* Load/store exclusive */
+/*
+ * Load/Store exclusive instructions are implemented by remembering
+ * the value/address loaded, and seeing if these are the same
+ * when the store is performed. This is not actually the architecturally
+ * mandated semantics, but it works for typical guest code sequences
+ * and avoids having to monitor regular stores.
+ *
+ * In system emulation mode only one CPU will be running at once, so
+ * this sequence is effectively atomic.  In user emulation mode we
+ * throw an exception and handle the atomic operation elsewhere.
+ */
+static void gen_load_exclusive(DisasContext *s, int rt, int rt2,
+                               TCGv_i64 addr, int size, bool is_pair)
+{
+    TCGv_i64 tmp = tcg_temp_new_i64();
+
+    switch (size) {
+    case 0:
+        tcg_gen_qemu_ld8u(tmp, addr, get_mem_index(s));
+        break;
+    case 1:
+        tcg_gen_qemu_ld16u(tmp, addr, get_mem_index(s));
+        break;
+    case 2:
+        tcg_gen_qemu_ld32u(tmp, addr, get_mem_index(s));
+        break;
+    case 3:
+        tcg_gen_qemu_ld64(tmp, addr, get_mem_index(s));
+        break;
+    default:
+        abort();
+    }
+    if (is_pair) {
+        TCGv_i64 addr2 = tcg_temp_new_i64();
+        TCGv_i64 hitmp = tcg_temp_new_i64();
+
+        tcg_gen_addi_i64(addr2, addr, 1 << size);
+        if (size == 2) {
+            tcg_gen_qemu_ld32u(hitmp, addr2, get_mem_index(s));
+        } else {
+            tcg_gen_qemu_ld64(hitmp, addr2, get_mem_index(s));
+        }
+        tcg_temp_free_i64(addr2);
+        tcg_gen_mov_i64(cpu_exclusive_high, hitmp);
+        tcg_gen_mov_i64(cpu_reg(s, rt2), hitmp);
+        tcg_temp_free_i64(hitmp);
+    }
+
+    tcg_gen_mov_i64(cpu_exclusive_val, tmp);
+    tcg_gen_mov_i64(cpu_reg(s, rt), tmp);
+
+    tcg_temp_free_i64(tmp);
+    tcg_gen_mov_i64(cpu_exclusive_addr, addr);
+}
+
+#ifdef CONFIG_USER_ONLY
+static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
+                                TCGv_i64 addr, int size, int is_pair)
+{
+    tcg_gen_mov_i64(cpu_exclusive_test, addr);
+    tcg_gen_movi_i32(cpu_exclusive_info,
+                     size | is_pair << 2 | (rd << 4) | (rt << 9) | (rt2 << 14));
+    gen_exception_insn(s, 4, EXCP_STREX);
+}
+#else
+static void gen_store_exclusive(DisasContext *s, int rd, int rt, int rt2,
+                                TCGv_i64 addr, int size, int is_pair)
+{
+    qemu_log_mask(LOG_UNIMP,
+                  "%s:%d: system mode store_exclusive unsupported "
+                  "at pc=%016" PRIx64 "\n",
+                  __FILE__, __LINE__, s->pc - 4);
+}
+#endif
+
+/* C3.3.6 Load/store exclusive
+ *
+ *  31 30 29         24  23  22   21  20  16  15  14   10 9    5 4    0
+ * +-----+-------------+----+---+----+------+----+-------+------+------+
+ * | sz  | 0 0 1 0 0 0 | o2 | L | o1 |  Rs  | o0 |  Rt2  |  Rn  | Rt   |
+ * +-----+-------------+----+---+----+------+----+-------+------+------+
+ *
+ *  sz: 00 -> 8 bit, 01 -> 16 bit, 10 -> 32 bit, 11 -> 64 bit
+ *   L: 0 -> store, 1 -> load
+ *  o2: 0 -> exclusive, 1 -> not
+ *  o1: 0 -> single register, 1 -> register pair
+ *  o0: 1 -> load-acquire/store-release, 0 -> not
+ *
+ *  o0 == 0 AND o2 == 1 is un-allocated
+ *  o1 == 1 is un-allocated except for 32 and 64 bit sizes
+ */
 static void disas_ldst_excl(DisasContext *s, uint32_t insn)
 {
-    unsupported_encoding(s, insn);
+    int rt = extract32(insn, 0, 5);
+    int rn = extract32(insn, 5, 5);
+    int rt2 = extract32(insn, 10, 5);
+    int is_lasr = extract32(insn, 15, 1);
+    int rs = extract32(insn, 16, 5);
+    int is_pair = extract32(insn, 21, 1);
+    int is_store = !extract32(insn, 22, 1);
+    int is_excl = !extract32(insn, 23, 1);
+    int size = extract32(insn, 30, 2);
+    TCGv_i64 tcg_addr;
+
+    if ((!is_excl && !is_lasr) ||
+        (is_pair && size < 2)) {
+        unallocated_encoding(s);
+        return;
+    }
+
+    if (rn == 31) {
+        gen_check_sp_alignment(s);
+    }
+    tcg_addr = read_cpu_reg_sp(s, rn, 1);
+
+    /* Note that since TCG is single threaded load-acquire/store-release
+     * semantics require no extra if (is_lasr) { ... } handling.
+     */
+
+    if (is_excl) {
+        if (!is_store) {
+            gen_load_exclusive(s, rt, rt2, tcg_addr, size, is_pair);
+        } else {
+            gen_store_exclusive(s, rs, rt, rt2, tcg_addr, size, is_pair);
+        }
+    } 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, false, false);
+        }
+        if (is_pair) {
+            TCGv_i64 tcg_rt2 = cpu_reg(s, rt);
+            tcg_gen_addi_i64(tcg_addr, tcg_addr, 1 << size);
+            if (is_store) {
+                do_gpr_st(s, tcg_rt2, tcg_addr, size);
+            } else {
+                do_gpr_ld(s, tcg_rt2, tcg_addr, size, false, false);
+            }
+        }
+    }
 }
 
 /*
-- 
1.8.5

Re: [Qemu-devel] [PATCH 20/21] target-arm: A64: support for ld/st/cl exclusive

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> +    switch (size) {
> +    case 0:
> +        tcg_gen_qemu_ld8u(tmp, addr, get_mem_index(s));
> +        break;
> +    case 1:
> +        tcg_gen_qemu_ld16u(tmp, addr, get_mem_index(s));
> +        break;
> +    case 2:
> +        tcg_gen_qemu_ld32u(tmp, addr, get_mem_index(s));
> +        break;
> +    case 3:
> +        tcg_gen_qemu_ld64(tmp, addr, get_mem_index(s));
> +        break;
> +    default:
> +        abort();
> +    }

tcg_gen_qemu_ld_i64.

> +        if (size == 2) {
> +            tcg_gen_qemu_ld32u(hitmp, addr2, get_mem_index(s));
> +        } else {
> +            tcg_gen_qemu_ld64(hitmp, addr2, get_mem_index(s));
> +        }

Likewise.


r~

[Qemu-devel] [PATCH 21/21] default-configs: Add config for aarch64-linux-user

[Peter Maydell]

Add a config for aarch64-linux-user, thereby enabling it as
a valid target.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 default-configs/aarch64-linux-user.mak | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 default-configs/aarch64-linux-user.mak

diff --git a/default-configs/aarch64-linux-user.mak b/default-configs/aarch64-linux-user.mak
new file mode 100644
index 0000000..3df7de5
--- /dev/null
+++ b/default-configs/aarch64-linux-user.mak
@@ -0,0 +1,3 @@
+# Default configuration for aarch64-linux-user
+
+CONFIG_GDBSTUB_XML=y
-- 
1.8.5

Re: [Qemu-devel] [PATCH 21/21] default-configs: Add config for aarch64-linux-user

[Richard Henderson]

On 12/17/2013 07:12 AM, Peter Maydell wrote:
> Add a config for aarch64-linux-user, thereby enabling it as
> a valid target.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  default-configs/aarch64-linux-user.mak | 3 +++
>  1 file changed, 3 insertions(+)
>  create mode 100644 default-configs/aarch64-linux-user.mak

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


r~