mbox series

[for-2.12,0/2] RISC-V: Mark FP status dirty

Message ID 20180328022233.13400-1-richard.henderson@linaro.org
Headers show
Series RISC-V: Mark FP status dirty | expand

Message

Richard Henderson March 28, 2018, 2:22 a.m. UTC
Since it was my patch that broke FP state tracking in the
first place, I feel obligated to fix it again.

Mark mstatus[fs] as dirty whenever we write to the file.
This can be optimized by only doing so once within a TB
which initially began with a clean file.

I have not yet put together an environment that can test
this, so I'll need someone else to give it a go.


r~


Richard Henderson (2):
  target/riscv: Split out mstatus_fs from tb_flags during translation
  target/riscv: Mark MSTATUS_FS dirty

 target/riscv/cpu.h       |  6 +++---
 target/riscv/op_helper.c | 25 ++++++++++++++++--------
 target/riscv/translate.c | 50 ++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 64 insertions(+), 17 deletions(-)

-- 
2.14.3

Comments

Michael Clark March 28, 2018, 4:54 a.m. UTC | #1
Hi Richard,

Thanks! I'll test this tomorrow morning and we can choose whether to
include your proper fix or the workaround.

I think we have time assuming we send out PRs tomorrow.

Given our important fixes have review including either this fix by tomorrow
or the workaround, and Philippe has reviewed our other important bugs
fixes, then we should be fine.

Then after getting the critical and important fixes out of the way, then I
perhaps make a PR for the other reviewed changes, although these might best
wait until QEMU 2.13 opens.

Thanks again,
Michael.


On Tue, Mar 27, 2018 at 7:22 PM, Richard Henderson <
richard.henderson@linaro.org> wrote:

> Since it was my patch that broke FP state tracking in the

> first place, I feel obligated to fix it again.

>

> Mark mstatus[fs] as dirty whenever we write to the file.

> This can be optimized by only doing so once within a TB

> which initially began with a clean file.

>

> I have not yet put together an environment that can test

> this, so I'll need someone else to give it a go.

>

>

> r~

>

>

> Richard Henderson (2):

>   target/riscv: Split out mstatus_fs from tb_flags during translation

>   target/riscv: Mark MSTATUS_FS dirty

>

>  target/riscv/cpu.h       |  6 +++---

>  target/riscv/op_helper.c | 25 ++++++++++++++++--------

>  target/riscv/translate.c | 50 ++++++++++++++++++++++++++++++

> ++++++++++++------

>  3 files changed, 64 insertions(+), 17 deletions(-)

>

> --

> 2.14.3

>

>
Michael Clark March 28, 2018, 5:58 p.m. UTC | #2
On Tue, Mar 27, 2018 at 7:22 PM, Richard Henderson <
richard.henderson@linaro.org> wrote:

> Since it was my patch that broke FP state tracking in the

> first place, I feel obligated to fix it again.

>

> Mark mstatus[fs] as dirty whenever we write to the file.

> This can be optimized by only doing so once within a TB

> which initially began with a clean file.

>

> I have not yet put together an environment that can test

> this, so I'll need someone else to give it a go.

>


I have tested it with the simple test case running SMP Linux and it appears
okay (Note it must be compiled with -O2):

 http://oirase.annexia.org/tmp/sched.c

It is clearly broken without your two patches.
Richard W.M. Jones April 3, 2018, 7:46 a.m. UTC | #3
On Wed, Mar 28, 2018 at 10:22:31AM +0800, Richard Henderson wrote:
> Since it was my patch that broke FP state tracking in the

> first place, I feel obligated to fix it again.


I missed this patch, thanks Michael Clark for pointing it out to me.

I've just tried it now using my test reproducer of the bug
(http://oirase.annexia.org/tmp/sched.c) and Fedora/RISC-V and it
appears to fix the problem.

> Mark mstatus[fs] as dirty whenever we write to the file.

> This can be optimized by only doing so once within a TB

> which initially began with a clean file.

> 

> I have not yet put together an environment that can test

> this, so I'll need someone else to give it a go.


This is the Fedora stage4 disk image which works fine under qemu from
git (make sure you read the readme.txt file first):

  https://fedorapeople.org/groups/risc-v/disk-images/

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org