Re: [PATCH 2/3] RISC-V: use FIELD macro to define tb flags

[Richard Henderson]

On 1/9/20 10:12 PM, LIU Zhiwei wrote:
>      if (riscv_cpu_fp_enabled(env)) {
> -        *flags |= TB_FLAGS_MSTATUS_FS;
> +        flags = FIELD_DP32(flags, TB_FLAGS, FS, (env->mstatus & MSTATUS_FS));
>      }

This is wrong.  You're inserting the low two bits of

  env->mstatus & MSTATUS_FS

into TB_FLAGS.  Which, knowing that MSTATUS_FS == 0x6000, is of course always 0.

Because of how TB_FLAGS_MSTATUS_FS is defined, overlapping MSTATUS_FS, you
could just have

    *flags |= env->mstatus & MSTATUS_FS;


BTW, the *existing* code is wrong.  This merges all non-disabled states to
dirty.  This means that the code in mark_fs_dirty in translate.c that handles
initial and clean states is unreachable, and that the kernel's dirty state
tracking will not work.

Has the riscv kernel stopped doing lazy fp context switching?  I would imagine
that it has if this code is to be believed...

BTW2, for the purpose of tb_flags, initial and clean states are identical.  We
should probably fold them, to avoid generating two different TBs for the two
states.


r~