Re: [Qemu-devel] [PATCH v3] RISC-V: Fix riscv_isa_string memory size bug

[Michael Clark]

On Mon, Mar 19, 2018 at 12:42 PM, Richard W.M. Jones <address@hidden>
wrote:

> On Mon, Mar 19, 2018 at 11:35:51AM -0700, Michael Clark wrote:
> > The RISC-V post-merge spec conformance and cleanup series has had a lot
> of
> > testing. I've been using it to compile QEMU inside of QEMU using the
> RISC-V
> > Fedora Image and its native RISC-V GCC toolchain running inside SMP Linux
> > 4.16-rc2. It appears to be pretty rock-solid. The rcu lock fix would
> likely
> > only affect users who are ballooning memory while a guest is under load.
> > The page walker changes have also been tested under load (including
> > performance tests).
>
> Did you see the problem with restoring floating point registers on
> context switch?  The test case is quite simple:
>
>   http://oirase.annexia.org/tmp/sched.c


No I didn't. Thanks.


> Note you must compile it with gcc -O2 to manifest the bug.  (We
> originally thought it was a problem with gcc's optimization, but it
> isn't.)
>

Okay. I must admit most of my testing has been integer heavy workloads.


> In Fedora's qemu tree are carrying the following patch which is just a
> workaround:
>
>   https://github.com/rwmjones/fedora-riscv-bootstrap/blob/
> master/stage1-riscv-qemu/force-float-save.patch


I will take a look...

There were some changes around v4 of our port upstream patch series to add
MSTATUS.FS (TB_FLAGS_FP_ENABLE) to to tb->flags to avoid checking the flags
on all FP ops.

static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong
*pc,
                                        target_ulong *cs_base, uint32_t
*flags)
{
    *pc = env->pc;
    *cs_base = 0;
#ifdef CONFIG_USER_ONLY
    *flags = TB_FLAGS_FP_ENABLE;
#else
    *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
#endif
}

I'll have to get my head around the implication of this or with the
privilege level and how MSTATUS.FS is currently set...