[Qemu-devel] [PATCH 0/4] target-arm: Fix decoding of preload and hint insns

[Qemu-devel] [PATCH 0/4] target-arm: Fix decoding of preload and hint insns

[Peter Maydell]

The primary aim of this patch series is to fix the decoding of the
preload and hint instruction space (PLD, PLDW, PLI). Some of these
instructions (PLDW and some unallocated space which should NOP) are
v7MP only, so we introduce a feature flag for cores with the v7MP
extensions.

The flag also lets us clean up the MPIDR coprocessor register
implementation so it isn't A9 specific any more.

Tested as usual with random instruction sequences.

Peter Maydell (4):
  target-arm: Add CPU feature flag for v7MP
  target-arm: Clean up handling of MPIDR
  target-arm: Fix decoding of preload and memory hint space
  target-arm: Fix decoding of Thumb preload and hint space

 target-arm/cpu.h       |    3 +-
 target-arm/helper.c    |   32 +++++++++++++--
 target-arm/translate.c |   98 +++++++++++++++++++++++++++++++++++------------
 3 files changed, 102 insertions(+), 31 deletions(-)

[Qemu-devel] [PATCH 1/4] target-arm: Add CPU feature flag for v7MP

[Peter Maydell]

Add a CPU feature flag for v7MP (the multiprocessing extensions); some
instructions exist only for v7MP and not for the base v7 architecture.

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

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5bcd53a..0d96325 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -362,7 +362,8 @@ enum arm_features {
     ARM_FEATURE_DIV,
     ARM_FEATURE_M, /* Microcontroller profile.  */
     ARM_FEATURE_OMAPCP, /* OMAP specific CP15 ops handling.  */
-    ARM_FEATURE_THUMB2EE
+    ARM_FEATURE_THUMB2EE,
+    ARM_FEATURE_V7MP    /* v7 Multiprocessing Extensions */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index b562767..3cf9181 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -123,6 +123,11 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         set_feature(env, ARM_FEATURE_VFP_FP16);
         set_feature(env, ARM_FEATURE_NEON);
         set_feature(env, ARM_FEATURE_THUMB2EE);
+        /* Note that A9 supports the MP extensions even for
+         * A9UP and single-core A9MP (which are both different
+         * and valid configurations; we don't model A9UP).
+         */
+        set_feature(env, ARM_FEATURE_V7MP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41034000; /* Guess */
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
@@ -152,6 +157,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         set_feature(env, ARM_FEATURE_NEON);
         set_feature(env, ARM_FEATURE_THUMB2EE);
         set_feature(env, ARM_FEATURE_DIV);
+        set_feature(env, ARM_FEATURE_V7MP);
         break;
     case ARM_CPUID_TI915T:
     case ARM_CPUID_TI925T:
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] target-arm: Clean up handling of MPIDR

[Peter Maydell]

The ARM cp15 register 0,c0,c0,5 is standardised in the v7 architecture
as the MPIDR. Clean up its implementation to remove A9 specific handling.

This commit includes fixing an error in the value returned for the
MPIDR on A9, where we were erroneously claiming a cluster ID of 9.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c |   26 +++++++++++++++++++++-----
 1 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3cf9181..d46defc 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -1608,12 +1608,28 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
                     return 0;
                 case 3: /* TLB type register.  */
                     return 0; /* No lockable TLB entries.  */
-                case 5: /* CPU ID */
-                    if (ARM_CPUID(env) == ARM_CPUID_CORTEXA9) {
-                        return env->cpu_index | 0x80000900;
-                    } else {
-                        return env->cpu_index;
+                case 5: /* MPIDR */
+                    /* The MPIDR was standardised in v7; prior to
+                     * this it was implemented only in the 11MPCore.
+                     * For all other pre-v7 cores it does not exist.
+                     */
+                    if (arm_feature(env, ARM_FEATURE_V7) ||
+                        ARM_CPUID(env) == ARM_CPUID_ARM11MPCORE) {
+                        int mpidr = env->cpu_index;
+                        /* We don't support setting cluster ID ([8..11])
+                         * so these bits always RAZ.
+                         */
+                        if (arm_feature(env, ARM_FEATURE_V7MP)) {
+                            mpidr |= (1 << 31);
+                            /* Cores which are uniprocessor (non-coherent)
+                             * but still implement the MP extensions set
+                             * bit 30. (For instance, A9UP.) However we do
+                             * not currently model any of those cores.
+                             */
+                        }
+                        return mpidr;
                     }
+                    /* otherwise fall through to the unimplemented-reg case */
                 default:
                     goto bad_reg;
                 }
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] target-arm: Fix decoding of preload and memory hint space

[Peter Maydell]

Correct the decoding of the ARM preload and memory hint space,
by adding decoding of PLI, PLDW and the v7MP unallocated hint
space. This commit also corrects a slightly overexuberant
decoding of PLD(register) which was not checking that bit 4
was one.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   28 +++++++++++++++++++++++++---
 1 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index d95133f..b742394 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6102,9 +6102,31 @@ static void disas_arm_insn(CPUState * env, DisasContext *s)
                 goto illegal_op;
             return;
         }
-        if ((insn & 0x0d70f000) == 0x0550f000)
-            return; /* PLD */
-        else if ((insn & 0x0ffffdff) == 0x01010000) {
+        if (((insn & 0x0f30f000) == 0x0510f000) ||
+            ((insn & 0x0f30f010) == 0x0710f000)) {
+            if ((insn & (1 << 22)) == 0) {
+                /* PLDW; v7MP */
+                if (!arm_feature(env, ARM_FEATURE_V7MP)) {
+                    goto illegal_op;
+                }
+            }
+            /* Otherwise PLD; v5TE+ */
+            return;
+        }
+        if (((insn & 0x0f70f000) == 0x0450f000) ||
+            ((insn & 0x0f70f010) == 0x0650f000)) {
+            ARCH(7);
+            return; /* PLI; V7 */
+        }
+        if (((insn & 0x0f700000) == 0x04100000) ||
+            ((insn & 0x0f700010) == 0x06100000)) {
+            if (!arm_feature(env, ARM_FEATURE_V7MP)) {
+                goto illegal_op;
+            }
+            return; /* v7MP: Unallocated memory hint: must NOP */
+        }
+
+        if ((insn & 0x0ffffdff) == 0x01010000) {
             ARCH(6);
             /* setend */
             if (insn & (1 << 9)) {
-- 
1.7.1

[Qemu-devel] [PATCH 4/4] target-arm: Fix decoding of Thumb preload and hint space

[Peter Maydell]

Refine the decoding of the Thumb preload and hint space, so we
UNDEF on the patterns that are supposed to UNDEF rather than NOP.
We also move the tests for this space earlier, so we don't emit
harmless but unnecessary address generation code for preload
hints (which by their nature are likely to be in hot code paths).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/translate.c |   70 +++++++++++++++++++++++++++++++++---------------
 1 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index b742394..b445355 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8308,6 +8308,42 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 goto illegal_op;
             break;
         }
+        op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
+        if (rs == 15) {
+            if (!(insn & (1 << 20))) {
+                goto illegal_op;
+            }
+            if (op != 2) {
+                /* Byte or halfword load space with dest == r15 : memory hints.
+                 * Catch them early so we don't emit pointless addressing code.
+                 * This space is a mix of:
+                 *  PLD/PLDW/PLI,  which we implement as NOPs (note that unlike
+                 *     the ARM encodings, PLDW space doesn't UNDEF for non-v7MP
+                 *     cores)
+                 *  unallocated hints, which must be treated as NOPs
+                 *  UNPREDICTABLE space, which we NOP or UNDEF depending on
+                 *     which is easiest for the decoding logic
+                 *  Some space which must UNDEF
+                 */
+                int op1 = (insn >> 23) & 3;
+                int op2 = (insn >> 6) & 0x3f;
+                if (op & 2) {
+                    goto illegal_op;
+                }
+                if (rn == 15) {
+                    /* UNPREDICTABLE or unallocated hint */
+                    return 0;
+                }
+                if (op1 & 1) {
+                    return 0; /* PLD* or unallocated hint */
+                }
+                if ((op2 == 0) || ((op2 & 0x3c) == 0x30)) {
+                    return 0; /* PLD* or unallocated hint */
+                }
+                /* UNDEF space, or an UNPREDICTABLE */
+                return 1;
+            }
+        }
         user = IS_USER(s);
         if (rn == 15) {
             addr = new_tmp();
@@ -8326,9 +8362,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 imm = insn & 0xfff;
                 tcg_gen_addi_i32(addr, addr, imm);
             } else {
-                op = (insn >> 8) & 7;
                 imm = insn & 0xff;
-                switch (op) {
+                switch ((insn >> 8) & 7) {
                 case 0: case 8: /* Shifted Register.  */
                     shift = (insn >> 4) & 0xf;
                     if (shift > 3)
@@ -8365,32 +8400,23 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
                 }
             }
         }
-        op = ((insn >> 21) & 3) | ((insn >> 22) & 4);
         if (insn & (1 << 20)) {
             /* Load.  */
-            if (rs == 15 && op != 2) {
-                if (op & 2)
-                    goto illegal_op;
-                /* Memory hint.  Implemented as NOP.  */
+            switch (op) {
+            case 0: tmp = gen_ld8u(addr, user); break;
+            case 4: tmp = gen_ld8s(addr, user); break;
+            case 1: tmp = gen_ld16u(addr, user); break;
+            case 5: tmp = gen_ld16s(addr, user); break;
+            case 2: tmp = gen_ld32(addr, user); break;
+            default: goto illegal_op;
+            }
+            if (rs == 15) {
+                gen_bx(s, tmp);
             } else {
-                switch (op) {
-                case 0: tmp = gen_ld8u(addr, user); break;
-                case 4: tmp = gen_ld8s(addr, user); break;
-                case 1: tmp = gen_ld16u(addr, user); break;
-                case 5: tmp = gen_ld16s(addr, user); break;
-                case 2: tmp = gen_ld32(addr, user); break;
-                default: goto illegal_op;
-                }
-                if (rs == 15) {
-                    gen_bx(s, tmp);
-                } else {
-                    store_reg(s, rs, tmp);
-                }
+                store_reg(s, rs, tmp);
             }
         } else {
             /* Store.  */
-            if (rs == 15)
-                goto illegal_op;
             tmp = load_reg(s, rs);
             switch (op) {
             case 0: gen_st8(tmp, addr, user); break;
-- 
1.7.1

Re: [Qemu-devel] [PATCH 4/4] target-arm: Fix decoding of Thumb preload and hint space

[Peter Maydell]

On 3 February 2011 19:43, Peter Maydell <peter.maydell@linaro.org> wrote:
> @@ -8326,9 +8362,8 @@ static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
>                 imm = insn & 0xfff;
>                 tcg_gen_addi_i32(addr, addr, imm);
>             } else {
> -                op = (insn >> 8) & 7;
>                 imm = insn & 0xff;
> -                switch (op) {
> +                switch ((insn >> 8) & 7) {
>                 case 0: case 8: /* Shifted Register.  */
>                     shift = (insn >> 4) & 0xf;
>                     if (shift > 3)

It's pretty obvious from the combination of "switch (something & 7)" with
"case 8:" that the load/store address decode logic here is bogus, but
since that's unrelated to the preload/hint space I'm going to address
it in a separate patchset later.

-- PMM

Re: [Qemu-devel] [PATCH 0/4] target-arm: Fix decoding of preload and hint insns

[Aurelien Jarno]

On Thu, Feb 03, 2011 at 07:43:21PM +0000, Peter Maydell wrote:
> The primary aim of this patch series is to fix the decoding of the
> preload and hint instruction space (PLD, PLDW, PLI). Some of these
> instructions (PLDW and some unallocated space which should NOP) are
> v7MP only, so we introduce a feature flag for cores with the v7MP
> extensions.
> 
> The flag also lets us clean up the MPIDR coprocessor register
> implementation so it isn't A9 specific any more.
> 
> Tested as usual with random instruction sequences.
> 
> Peter Maydell (4):
>   target-arm: Add CPU feature flag for v7MP
>   target-arm: Clean up handling of MPIDR
>   target-arm: Fix decoding of preload and memory hint space
>   target-arm: Fix decoding of Thumb preload and hint space
> 
>  target-arm/cpu.h       |    3 +-
>  target-arm/helper.c    |   32 +++++++++++++--
>  target-arm/translate.c |   98 +++++++++++++++++++++++++++++++++++------------
>  3 files changed, 102 insertions(+), 31 deletions(-)
> 

Thanks, all applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net