diff mbox series

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

Message ID 20200110081220.891-2-zhiwei_liu@c-sky.com
State New
Headers show
Series [1/3] select gdb fpu xml by single or double float extension | expand

Commit Message

LIU Zhiwei Jan. 10, 2020, 8:12 a.m. UTC 
FIELD is more unified to define tb flags. It is easier to add new
filed to tb flags.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu.h       | 15 +++++++++------
 target/riscv/translate.c |  5 +++--
 2 files changed, 12 insertions(+), 8 deletions(-)

Comments

Richard Henderson Jan. 14, 2020, 2:22 a.m. UTC | #1 
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~
diff mbox series

Patch

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index e59343e13c..8efd4c5904 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -282,22 +282,25 @@  void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
 target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
 void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
 
-#define TB_FLAGS_MMU_MASK   3
-#define TB_FLAGS_MSTATUS_FS MSTATUS_FS
+FIELD(TB_FLAGS, MMU, 0, 2)
+FIELD(TB_FLAGS, FS, 13, 2)
 
 static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
-                                        target_ulong *cs_base, uint32_t *flags)
+                                        target_ulong *cs_base, uint32_t *pflags)
 {
+    uint32_t flags = 0;
     *pc = env->pc;
     *cs_base = 0;
+
 #ifdef CONFIG_USER_ONLY
-    *flags = TB_FLAGS_MSTATUS_FS;
+    flags = FIELD_DP32(flags, TB_FLAGS, FS, MSTATUS_FS);
 #else
-    *flags = cpu_mmu_index(env, 0);
+    flags = FIELD_DP32(flags, TB_FLAGS, MMU, cpu_mmu_index(env, 0));
     if (riscv_cpu_fp_enabled(env)) {
-        *flags |= TB_FLAGS_MSTATUS_FS;
+        flags = FIELD_DP32(flags, TB_FLAGS, FS, (env->mstatus & MSTATUS_FS));
     }
 #endif
+    *pflags = flags;
 }
 
 int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong *ret_value,
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ab6a891dc3..5de2d11d5c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -735,10 +735,11 @@  static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPURISCVState *env = cs->env_ptr;
     RISCVCPU *cpu = RISCV_CPU(cs);
+    uint32_t tb_flags = ctx->base.tb->flags;
 
     ctx->pc_succ_insn = ctx->base.pc_first;
-    ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
-    ctx->mstatus_fs = ctx->base.tb->flags & TB_FLAGS_MSTATUS_FS;
+    ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MMU);
+    ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
     ctx->priv_ver = env->priv_ver;
     ctx->misa = env->misa;
     ctx->frm = -1;  /* unknown rounding mode */