[Qemu-devel] [PATCH v1 1/1] RISC-V: Workaround for critical mstatus.FS MTTCG bug

[Michael Clark <mjc@sifive.com>]

This change is a workaround for a bug where mstatus.FS
is not correctly reporting dirty when MTTCG and SMP are
enabled which results in the floating point register file
not being saved during context switches. This a critical
bug for RISC-V in QEMU as it results in floating point
register file corruption when running SMP Linux in the
RISC-V 'virt' machine.

This workaround will return dirty if mstatus.FS is
switched from off to initial or clean. We have checked
the specification and it is legal for an implementation
to return either off, or dirty, if set to initial or clean.

This workaround will result in unnecessary floating point
save restore. When mstatus.FS is off, floating point
instruction trap to indicate the process is using the FPU.
The OS can then save floating-point state of the previous
process using the FPU and set mstatus.FS to initial or
clean. With this workaround, mstatus.FS will always return
dirty if set to a non-zero value, indicating floating point
save restore is necessary, versus misreporting mstatus.FS
resulting in floating point register file corruption.

Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Michael Clark <mjc@sifive.com>
---
 target/riscv/op_helper.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index e34715d..7281b98 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -144,8 +144,23 @@ void csr_write_helper(CPURISCVState *env, target_ulong val_to_write,
         }
 
         mstatus = (mstatus & ~mask) | (val_to_write & mask);
-        int dirty = (mstatus & MSTATUS_FS) == MSTATUS_FS;
-        dirty |= (mstatus & MSTATUS_XS) == MSTATUS_XS;
+
+        /* Note: this is a workaround for an issue where mstatus.FS
+           does not report dirty when SMP and MTTCG is enabled. This
+           workaround is technically compliant with the RISC-V Privileged
+           specification as it is legal to return only off, or dirty,
+           however this may cause unnecessary saves of floating point state.
+           Without this workaround, floating point state is not saved and
+           restored correctly when SMP and MTTCG is enabled, */
+        if (qemu_tcg_mttcg_enabled()) {
+            /* FP is always dirty or off */
+            if (mstatus & MSTATUS_FS) {
+                mstatus |= MSTATUS_FS;
+            }
+        }
+
+        int dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
+                    ((mstatus & MSTATUS_XS) == MSTATUS_XS);
         mstatus = set_field(mstatus, MSTATUS_SD, dirty);
         env->mstatus = mstatus;
         break;
-- 
2.7.0