[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] RISC-V: use FIELD macro to define tb flags
From: |
Richard Henderson |
Subject: |
Re: [PATCH 2/3] RISC-V: use FIELD macro to define tb flags |
Date: |
Mon, 13 Jan 2020 16:22:46 -1000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2 |
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~