qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Clark <mjc@sifive.com>
To: qemu-devel@nongnu.org
Cc: patches@groups.riscv.org, Michael Clark <mjc@sifive.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	Sagar Karandikar <sagark@eecs.berkeley.edu>,
	Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
	"Richard W . M . Jones" <rjones@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>
Subject: [Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug
Date: Sat, 24 Mar 2018 11:13:40 -0700	[thread overview]
Message-ID: <1521915220-65389-15-git-send-email-mjc@sifive.com> (raw)
In-Reply-To: <1521915220-65389-1-git-send-email-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: Richard W.M. Jones <rjones@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
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 1fdde90..d345688 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 coorectly 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

  parent reply	other threads:[~2018-03-24 21:11 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-24 18:13 [Qemu-devel] [PATCH v6 00/26] RISC-V: Fixes and cleanups for QEMU 2.12 Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 06/26] RISC-V: Mark ROM read-only after copying in code Michael Clark
2018-03-24 19:45   ` Michael Clark
2018-03-24 20:19     ` Michael Clark
2018-03-24 21:23   ` Peter Maydell
2018-03-25  0:23     ` Michael Clark
2018-03-25 12:47       ` Peter Maydell
2018-03-26 22:22         ` Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 08/26] RISC-V: Make sure rom has space for fdt Michael Clark
2018-03-24 21:25   ` Peter Maydell
2018-03-24 22:35     ` Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 10/26] RISC-V: Improve page table walker spec compliance Michael Clark
2018-03-24 19:39   ` Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 11/26] RISC-V: Update E order and I extension order Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 14/26] RISC-V: Use memory_region_is_ram in pte update Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 16/26] RISC-V: Hardwire satp to 0 for no-mmu case Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 18/26] RISC-V: riscv-qemu port supports sv39 and sv48 Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 19/26] RISC-V: vectored traps are optional Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 20/26] RISC-V: No traps on writes to misa, minstret, mcycle Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 21/26] RISC-V: Remove support for adhoc X_COP interrupt Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 23/26] RISC-V: Clear mtval/stval on exceptions without info Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 24/26] RISC-V: Remove erroneous comment from translate.c Michael Clark
2018-03-24 18:13 ` [Qemu-devel] [PATCH v6 25/26] RISC-V: Fix incorrect disassembly for addiw Michael Clark
2018-03-24 18:13 ` Michael Clark [this message]
2018-03-24 19:17   ` [Qemu-devel] [PATCH v6 26/26] RISC-V: Workaround for critical mstatus.FS MTTCG bug Michael Clark
2018-03-24 19:46   ` Richard W.M. Jones
2018-03-25 15:03 ` [Qemu-devel] [PATCH v6 00/26] RISC-V: Fixes and cleanups for QEMU 2.12 Peter Maydell
2018-03-26 18:07   ` [Qemu-devel] [patches] " Michael Clark
2018-03-26 23:14     ` Michael Clark
2018-03-26 23:45       ` Michael Clark
2018-03-27 10:21         ` Daniel P. Berrangé
2018-03-27  9:42     ` Peter Maydell
2018-03-27 18:39       ` Michael Clark
2018-03-27 19:00         ` Michael Clark

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1521915220-65389-15-git-send-email-mjc@sifive.com \
    --to=mjc@sifive.com \
    --cc=kbastian@mail.uni-paderborn.de \
    --cc=palmer@sifive.com \
    --cc=patches@groups.riscv.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=sagark@eecs.berkeley.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).