diff mbox

target-arm: Add debug check for mismatched cpreg resets

Message ID 1436797559-20835-1-git-send-email-peter.maydell@linaro.org
State Superseded
Headers show

Commit Message

Peter Maydell July 13, 2015, 2:25 p.m. UTC
It's easy to accidentally define two cpregs which both try
to reset the same underlying state field (for instance a
clash between an AArch64 EL3 definition and an AArch32
banked register definition). if the two definitions disagree
about the reset value then the result is dependent on which
one happened to be reached last in the hashtable enumeration.

Add a consistency check to detect and assert in these cases:
after reset, we run a second pass where we check that the
reset operation doesn't change the value of the register.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This does correctly flag up the SCTLR_EL3 mismatch I've just posted
a patch for, and doesn't seem to complain about anything else.
However it seems prudent to not put this into 2.4...

 target-arm/cpu.c    | 23 +++++++++++++++++++++++
 target-arm/cpu.h    |  3 +++
 target-arm/helper.c |  2 +-
 3 files changed, 27 insertions(+), 1 deletion(-)
diff mbox

Patch

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 8b4323d..9fb08ab 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -79,6 +79,27 @@  static void cp_reg_reset(gpointer key, gpointer value, gpointer opaque)
     }
 }
 
+static void cp_reg_check_reset(gpointer key, gpointer value,  gpointer opaque)
+{
+    /* Purely an assertion check: we've already done reset once,
+     * so now check that running the reset for the cpreg doesn't
+     * change its value. This traps bugs where two different cpregs
+     * both try to reset the same state field but to different values.
+     */
+    ARMCPRegInfo *ri = value;
+    ARMCPU *cpu = opaque;
+    uint64_t oldvalue, newvalue;
+
+    if (ri->type & (ARM_CP_SPECIAL | ARM_CP_ALIAS | ARM_CP_NO_RAW)) {
+        return;
+    }
+
+    oldvalue = read_raw_cp_reg(&cpu->env, ri);
+    cp_reg_reset(key, value, opaque);
+    newvalue = read_raw_cp_reg(&cpu->env, ri);
+    assert(oldvalue == newvalue);
+}
+
 /* CPUClass::reset() */
 static void arm_cpu_reset(CPUState *s)
 {
@@ -90,6 +111,8 @@  static void arm_cpu_reset(CPUState *s)
 
     memset(env, 0, offsetof(CPUARMState, features));
     g_hash_table_foreach(cpu->cp_regs, cp_reg_reset, cpu);
+    g_hash_table_foreach(cpu->cp_regs, cp_reg_check_reset, cpu);
+
     env->vfp.xregs[ARM_VFP_FPSID] = cpu->reset_fpsid;
     env->vfp.xregs[ARM_VFP_MVFR0] = cpu->mvfr0;
     env->vfp.xregs[ARM_VFP_MVFR1] = cpu->mvfr1;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7e89152..76a0a97 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1445,6 +1445,9 @@  static inline bool cp_access_ok(int current_el,
     return (ri->access >> ((current_el * 2) + isread)) & 1;
 }
 
+/* Raw read of a coprocessor register (as needed for migration, etc) */
+uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri);
+
 /**
  * write_list_to_cpustate
  * @cpu: ARMCPU
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 01f0d0d..fc2f61a 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -144,7 +144,7 @@  static void *raw_ptr(CPUARMState *env, const ARMCPRegInfo *ri)
     return (char *)env + ri->fieldoffset;
 }
 
-static uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
+uint64_t read_raw_cp_reg(CPUARMState *env, const ARMCPRegInfo *ri)
 {
     /* Raw read of a coprocessor register (as needed for migration, etc). */
     if (ri->type & ARM_CP_CONST) {