[Qemu-devel] [PATCH v2] mips: Correctly save/restore the FP flush-to-zero state

[Maciej W. Rozycki]

Fix the FP state save/restore operations by saving the `flush_to_zero' 
rather than the `float_detect_tininess' setting.  There is no provision 
for the latter in MIPS hardware, whereas the former is controlled by the 
CP1.FCSR.FS bit.  As a result all the older saved state images are 
invalid as they do not restore the FP state corresponding to the 
CP1.FCSR.FS bit and may execute differently when resumed compared to the 
case where no save/restore operations have ever been made.  Therefore 
reject any such older images too and do not allow them to be loaded.

According to the architecture manual[1][2]:

"Flush to Zero (Flush Subnormals).
[...]
    Encoding                     Meaning
         0        Input subnormal values and tiny non-
                  zero results are not altered.  Unimple-
                  mented Operation Exception may be
                  signaled as needed.
         1        When FS is one, subnormal results are
                  flushed to zero.  The Unimplemented
                  Operation Exception is NOT signalled
                  for this reason.
                     Every tiny non-zero result is
                  replaced with zero of the same sign.
                     Prior to Release 5 it is implementa-
                  tion dependent whether subnormal
                  operand values are flushed to zero
                  before the operation is carried out.
                     As of Release 5 every input subnor-
                  mal value is replaced with zero of the
                  same sign."

References:

[1] "MIPS Architecture For Programmers, Volume I-A: Introduction to the 
    MIPS32 Architecture", MIPS Technologies, Inc., Document Number: 
    MD00082, Revision 5.03, Sept. 9, 2013, Table 5.7 "FCSR Register 
    Field Descriptions", p. 81.

[2] "MIPS Architecture For Programmers, Volume I-A: Introduction to the 
    MIPS64 Architecture", Imagination Technologies, Inc., Document 
    Number: MD00083, Revision 6.00, March 31, 2014, Table 6.7 "FCSR 
    Register Field Descriptions", p. 93.

Signed-off-by: Maciej W. Rozycki <address@hidden>
---
Hi,

 Updates from v1:

- use qemu_put_8s/qemu_get_8s rather than qemu_put_s8s/qemu_get_s8s.

For some reason `float_detect_tininess' is signed while `flush_to_zero' 
is unsigned -- what's the point?

 Background information: I didn't realize the QEMU build process does 
not enable -Werror by default.  I have now noticed it and explicitly 
enabled the flag having spent a few hours debugging an issue that should 
have been caught by the compilation process (an incompatible pointer 
type used in a function call) -- shouldn't QEMU default to -Werror for 
development sources and only disable it for releases?

 Fortunately this is the only change affected from these I have 
submitted so far.

 I haven't updated the subject to have `target-mips:' rather than 
`mips:' as the tag so that automatic patch processing tools pick up this 
change as a replacement for the previous version.  I can resend with 
such an adjustment if that was desired.  Otherwise please apply.

  Maciej

qemu-mips-fp_status.diff
Index: qemu-git-trunk/target-mips/cpu.h
===================================================================
--- qemu-git-trunk.orig/target-mips/cpu.h       2014-11-20 08:07:13.000000000 
+0000
+++ qemu-git-trunk/target-mips/cpu.h    2014-11-20 08:10:28.568927441 +0000
@@ -615,7 +615,7 @@ void mips_cpu_list (FILE *f, fprintf_fun
 extern void cpu_wrdsp(uint32_t rs, uint32_t mask_num, CPUMIPSState *env);
 extern uint32_t cpu_rddsp(uint32_t mask_num, CPUMIPSState *env);
 
-#define CPU_SAVE_VERSION 5
+#define CPU_SAVE_VERSION 6
 
 /* MMU modes definitions. We carefully match the indices with our
    hflags layout. */
Index: qemu-git-trunk/target-mips/machine.c
===================================================================
--- qemu-git-trunk.orig/target-mips/machine.c   2014-11-20 08:07:13.000000000 
+0000
+++ qemu-git-trunk/target-mips/machine.c        2014-11-20 08:29:03.688928596 
+0000
@@ -34,7 +34,7 @@ static void save_fpu(QEMUFile *f, CPUMIP
 
     for(i = 0; i < 32; i++)
         qemu_put_be64s(f, &fpu->fpr[i].d);
-    qemu_put_s8s(f, &fpu->fp_status.float_detect_tininess);
+    qemu_put_8s(f, &fpu->fp_status.flush_to_zero);
     qemu_put_s8s(f, &fpu->fp_status.float_rounding_mode);
     qemu_put_s8s(f, &fpu->fp_status.float_exception_flags);
     qemu_put_be32s(f, &fpu->fcr0);
@@ -162,7 +162,7 @@ void cpu_save(QEMUFile *f, void *opaque)
         save_fpu(f, &env->fpus[i]);
 }
 
-static void load_tc(QEMUFile *f, TCState *tc, int version_id)
+static void load_tc(QEMUFile *f, TCState *tc)
 {
     int i;
 
@@ -184,9 +184,7 @@ static void load_tc(QEMUFile *f, TCState
     qemu_get_betls(f, &tc->CP0_TCSchedule);
     qemu_get_betls(f, &tc->CP0_TCScheFBack);
     qemu_get_sbe32s(f, &tc->CP0_Debug_tcstatus);
-    if (version_id >= 4) {
-        qemu_get_betls(f, &tc->CP0_UserLocal);
-    }
+    qemu_get_betls(f, &tc->CP0_UserLocal);
 }
 
 static void load_fpu(QEMUFile *f, CPUMIPSFPUContext *fpu)
@@ -195,7 +193,7 @@ static void load_fpu(QEMUFile *f, CPUMIP
 
     for(i = 0; i < 32; i++)
         qemu_get_be64s(f, &fpu->fpr[i].d);
-    qemu_get_s8s(f, &fpu->fp_status.float_detect_tininess);
+    qemu_get_8s(f, &fpu->fp_status.flush_to_zero);
     qemu_get_s8s(f, &fpu->fp_status.float_rounding_mode);
     qemu_get_s8s(f, &fpu->fp_status.float_exception_flags);
     qemu_get_be32s(f, &fpu->fcr0);
@@ -208,12 +206,12 @@ int cpu_load(QEMUFile *f, void *opaque, 
     MIPSCPU *cpu = mips_env_get_cpu(env);
     int i;
 
-    if (version_id < 3) {
+    if (version_id != CPU_SAVE_VERSION) {
         return -EINVAL;
     }
 
     /* Load active TC */
-    load_tc(f, &env->active_tc, version_id);
+    load_tc(f, &env->active_tc);
 
     /* Load active FPU */
     load_fpu(f, &env->active_fpu);
@@ -328,7 +326,7 @@ int cpu_load(QEMUFile *f, void *opaque, 
 
     /* Load inactive TC state */
     for (i = 0; i < MIPS_SHADOW_SET_MAX; i++) {
-        load_tc(f, &env->tcs[i], version_id);
+        load_tc(f, &env->tcs[i]);
     }
     for (i = 0; i < MIPS_FPU_MAX; i++)
         load_fpu(f, &env->fpus[i]);