| Message ID | 1522197746-26020-2-git-send-email-mjc@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 03/28/2018 08:42 AM, Michael Clark wrote: > This change is a workaround for a bug where mstatus.FS > is not correctly reporting dirty after operations that > modify floating point registers. This a critical bug > or RISC-V in QEMU as it results in floating point > register file corruption when running SMP Linux due to > task migration and possibly uniprocessor Linux if > more than one process is using the FPU. > > This workaround will return dirty if mstatus.FS is > switched from off to initial or clean. According to > the specification it is legal for an implementation > to return only off, or dirty. > > 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> > Cc: Alex Bennée <alex.bennee@linaro.org> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> > Tested-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Michael Clark <mjc@sifive.com> > --- > target/riscv/op_helper.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) In case the more extensive fix waits until 2.13, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c index e34715d..7c6068b 100644 --- a/target/riscv/op_helper.c +++ b/target/riscv/op_helper.c @@ -144,8 +144,21 @@ 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 after floating point operations + that modify floating point state. This workaround is + technically compliant with the RISC-V Privileged + specification as it is legal to return only off, or dirty. + at the expense of extra floating point save/restore. */ + + /* 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;