[Qemu-devel] [PATCH v1 1/1] mips: properly compute hflags and fcr0 on cpu reset

[Meador Inge]

Currently 'cpu_reset' doesn't fully compute all of the needed
HFLAGs and fails to setup fcr0 after clearing the CPU state.
This can cause instruction exceptions.  For example, using
'madd.d' on machines that should support it is kindly greeted
with:

qemu: uncaught target signal 4 (Illegal instruction) - core dumped
Illegal instruction (core dumped)

because fcr0 is bogus and MIPS_HFLAG_COP1X is not correcly set in hflags.

This is fixed by modifying 'cpu_reset' to use 'compute_hflags' and
initializing 'fcr0' from the current CPU model.

Signed-off-by: Maciej W. Rozycki <address@hidden>
Signed-off-by: Nathan Froyd <address@hidden>
Signed-off-by: Meador Inge <address@hidden>
---
 target-mips/cpu.h       |   49 +++++++++++++++++++++++++++++++++++++++++++++++
 target-mips/op_helper.c |   49 -----------------------------------------------
 target-mips/translate.c |   17 +++------------
 3 files changed, 53 insertions(+), 62 deletions(-)

diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 71cb4e8..fc65348 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -737,4 +737,53 @@ static inline void cpu_pc_from_tb(CPUState *env, 
TranslationBlock *tb)
     env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
 }
 
+static inline void compute_hflags(CPUState *env)
+{
+    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
+                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
+                     MIPS_HFLAG_UX);
+    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
+        !(env->CP0_Status & (1 << CP0St_ERL)) &&
+        !(env->hflags & MIPS_HFLAG_DM)) {
+        env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
+    }
+#if defined(TARGET_MIPS64)
+    if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
+        (env->CP0_Status & (1 << CP0St_PX)) ||
+        (env->CP0_Status & (1 << CP0St_UX))) {
+        env->hflags |= MIPS_HFLAG_64;
+    }
+    if (env->CP0_Status & (1 << CP0St_UX)) {
+        env->hflags |= MIPS_HFLAG_UX;
+    }
+#endif
+    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
+        !(env->hflags & MIPS_HFLAG_KSU)) {
+        env->hflags |= MIPS_HFLAG_CP0;
+    }
+    if (env->CP0_Status & (1 << CP0St_CU1)) {
+        env->hflags |= MIPS_HFLAG_FPU;
+    }
+    if (env->CP0_Status & (1 << CP0St_FR)) {
+        env->hflags |= MIPS_HFLAG_F64;
+    }
+    if (env->insn_flags & ISA_MIPS32R2) {
+        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    } else if (env->insn_flags & ISA_MIPS32) {
+        if (env->hflags & MIPS_HFLAG_64) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    } else if (env->insn_flags & ISA_MIPS4) {
+        /* All supported MIPS IV CPUs use the XX (CU3) to enable
+           and disable the MIPS IV extensions to the MIPS III ISA.
+           Some other MIPS IV CPUs ignore the bit, so the check here
+           would be too restrictive for them.  */
+        if (env->CP0_Status & (1 << CP0St_CU3)) {
+            env->hflags |= MIPS_HFLAG_COP1X;
+        }
+    }
+}
+
 #endif /* !defined (__MIPS_CPU_H__) */
diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index c51b9cb..1f1c736 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -32,55 +32,6 @@
 static inline void cpu_mips_tlb_flush (CPUState *env, int flush_global);
 #endif
 
-static inline void compute_hflags(CPUState *env)
-{
-    env->hflags &= ~(MIPS_HFLAG_COP1X | MIPS_HFLAG_64 | MIPS_HFLAG_CP0 |
-                     MIPS_HFLAG_F64 | MIPS_HFLAG_FPU | MIPS_HFLAG_KSU |
-                     MIPS_HFLAG_UX);
-    if (!(env->CP0_Status & (1 << CP0St_EXL)) &&
-        !(env->CP0_Status & (1 << CP0St_ERL)) &&
-        !(env->hflags & MIPS_HFLAG_DM)) {
-        env->hflags |= (env->CP0_Status >> CP0St_KSU) & MIPS_HFLAG_KSU;
-    }
-#if defined(TARGET_MIPS64)
-    if (((env->hflags & MIPS_HFLAG_KSU) != MIPS_HFLAG_UM) ||
-        (env->CP0_Status & (1 << CP0St_PX)) ||
-        (env->CP0_Status & (1 << CP0St_UX))) {
-        env->hflags |= MIPS_HFLAG_64;
-    }
-    if (env->CP0_Status & (1 << CP0St_UX)) {
-        env->hflags |= MIPS_HFLAG_UX;
-    }
-#endif
-    if ((env->CP0_Status & (1 << CP0St_CU0)) ||
-        !(env->hflags & MIPS_HFLAG_KSU)) {
-        env->hflags |= MIPS_HFLAG_CP0;
-    }
-    if (env->CP0_Status & (1 << CP0St_CU1)) {
-        env->hflags |= MIPS_HFLAG_FPU;
-    }
-    if (env->CP0_Status & (1 << CP0St_FR)) {
-        env->hflags |= MIPS_HFLAG_F64;
-    }
-    if (env->insn_flags & ISA_MIPS32R2) {
-        if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    } else if (env->insn_flags & ISA_MIPS32) {
-        if (env->hflags & MIPS_HFLAG_64) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    } else if (env->insn_flags & ISA_MIPS4) {
-        /* All supported MIPS IV CPUs use the XX (CU3) to enable
-           and disable the MIPS IV extensions to the MIPS III ISA.
-           Some other MIPS IV CPUs ignore the bit, so the check here
-           would be too restrictive for them.  */
-        if (env->CP0_Status & (1 << CP0St_CU3)) {
-            env->hflags |= MIPS_HFLAG_COP1X;
-        }
-    }
-}
-
 /*****************************************************************************/
 /* Exceptions processing helpers */
 
diff --git a/target-mips/translate.c b/target-mips/translate.c
index d5b1c76..cace3a3 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -12769,20 +12769,16 @@ void cpu_reset (CPUMIPSState *env)
     env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3;
     env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask;
     env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4;
+    env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
     env->insn_flags = env->cpu_model->insn_flags;
 
 #if defined(CONFIG_USER_ONLY)
-    env->hflags = MIPS_HFLAG_UM;
+    env->CP0_Status = (MIPS_HFLAG_UM << CP0St_KSU);
     /* Enable access to the SYNCI_Step register.  */
     env->CP0_HWREna |= (1 << 1);
     if (env->CP0_Config1 & (1 << CP0C1_FP)) {
-        env->hflags |= MIPS_HFLAG_FPU;
+        env->CP0_Status |= (1 << CP0St_CU1);
     }
-#ifdef TARGET_MIPS64
-    if (env->active_fpu.fcr0 & (1 << FCR0_F64)) {
-        env->hflags |= MIPS_HFLAG_F64;
-    }
-#endif
 #else
     if (env->hflags & MIPS_HFLAG_BMASK) {
         /* If the exception was raised from a delay slot,
@@ -12812,7 +12808,6 @@ void cpu_reset (CPUMIPSState *env)
     }
     /* Count register increments in debug mode, EJTAG version 1 */
     env->CP0_Debug = (1 << CP0DB_CNT) | (0x1 << CP0DB_VER);
-    env->hflags = MIPS_HFLAG_CP0;
 
     if (env->CP0_Config3 & (1 << CP0C3_MT)) {
         int i;
@@ -12840,11 +12835,7 @@ void cpu_reset (CPUMIPSState *env)
         }
     }
 #endif
-#if defined(TARGET_MIPS64)
-    if (env->cpu_model->insn_flags & ISA_MIPS3) {
-        env->hflags |= MIPS_HFLAG_64;
-    }
-#endif
+    compute_hflags(env);
     env->exception_index = EXCP_NONE;
 }
 
-- 
1.7.7.6