| Message ID | 20230324055954.908-1-zhiwei_liu@linux.alibaba.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fix tb flags use | expand |
On 2023/3/24 13:59, LIU Zhiwei wrote: > The pointer masking is the only extension that directly use status. > The vector or float extension uses the status in an indirect way. > > Replace the pointer masking extension special status fields with > the general status. > > Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > --- > target/riscv/cpu.c | 2 +- > target/riscv/cpu.h | 9 +++++++++ > target/riscv/cpu_bits.h | 6 ------ > target/riscv/csr.c | 14 +++++++------- > 4 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 1e97473af2..1135106b3e 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj) > i++; > } > /* mmte is supposed to have pm.current hardwired to 1 */ > - env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT); > + env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT); > #endif > env->xl = riscv_cpu_mxl(env); > riscv_cpu_update_mask(env); > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 12fe8d8546..5049e21518 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -99,6 +99,15 @@ enum { > TRANSLATE_G_STAGE_FAIL > }; > > +/* Extension Context Status */ > +enum { > + EXT_STATUS_DISABLED = 0, > + EXT_STATUS_INITIAL, > + EXT_STATUS_CLEAN, > + EXT_STATUS_DIRTY, > + EXT_STATUS_MASK, I think the right value for EXT_STATUS_MASK should be 3 here. And it can replace the following PM_XS_MASK. Regards, Weiwei Li > +}; > + > #define MMU_USER_IDX 3 > > #define MAX_RISCV_PMPS (16) > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index fca7ef0cef..5280bd41c2 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -736,12 +736,6 @@ typedef enum RISCVException { > #define PM_INSN 0x00000004ULL > #define PM_XS_MASK 0x00000003ULL > > -/* PointerMasking XS bits values */ > -#define PM_EXT_DISABLE 0x00000000ULL > -#define PM_EXT_INITIAL 0x00000001ULL > -#define PM_EXT_CLEAN 0x00000002ULL > -#define PM_EXT_DIRTY 0x00000003ULL > - > /* Execution enviornment configuration bits */ > #define MENVCFG_FIOM BIT(0) > #define MENVCFG_CBIE (3UL << 4) > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index d522efc0b6..abea7b749e 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState *env, int csrno, > > /* hardwiring pm.instruction bit to 0, since it's not supported yet */ > wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN); > - env->mmte = wpri_val | PM_EXT_DIRTY; > + env->mmte = wpri_val | EXT_STATUS_DIRTY; > riscv_cpu_update_mask(env); > > /* Set XS and SD bits, since PM CSRs are dirty */ > @@ -3593,7 +3593,7 @@ static RISCVException write_mpmmask(CPURISCVState *env, int csrno, > if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) { > env->cur_pmmask = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS; > @@ -3621,7 +3621,7 @@ static RISCVException write_spmmask(CPURISCVState *env, int csrno, > if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) { > env->cur_pmmask = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS; > @@ -3649,7 +3649,7 @@ static RISCVException write_upmmask(CPURISCVState *env, int csrno, > if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) { > env->cur_pmmask = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS; > @@ -3673,7 +3673,7 @@ static RISCVException write_mpmbase(CPURISCVState *env, int csrno, > if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) { > env->cur_pmbase = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS; > @@ -3701,7 +3701,7 @@ static RISCVException write_spmbase(CPURISCVState *env, int csrno, > if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) { > env->cur_pmbase = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS; > @@ -3729,7 +3729,7 @@ static RISCVException write_upmbase(CPURISCVState *env, int csrno, > if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) { > env->cur_pmbase = val; > } > - env->mmte |= PM_EXT_DIRTY; > + env->mmte |= EXT_STATUS_DIRTY; > > /* Set XS and SD bits, since PM CSRs are dirty */ > mstatus = env->mstatus | MSTATUS_XS;
On 2023/3/24 13:59, LIU Zhiwei wrote: > Reuse the MSTATUS_FS and MSTATUS_VS for the tb flags positions is not a normal > way. > > It will make us change the tb flags layout difficult. And even worse, if we > want to keep tb flags for a same extension togather without a hole. > > Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> > --- Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> Weiwei Li > target/riscv/cpu.h | 15 +++++++-------- > target/riscv/cpu_helper.c | 11 ++++++----- > target/riscv/insn_trans/trans_rvv.c.inc | 8 ++++---- > target/riscv/translate.c | 20 ++++++++++---------- > 4 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 5049e21518..41f7aef666 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -634,18 +634,17 @@ void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong); > > #define TB_FLAGS_PRIV_MMU_MASK 3 > #define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 2) > -#define TB_FLAGS_MSTATUS_FS MSTATUS_FS > -#define TB_FLAGS_MSTATUS_VS MSTATUS_VS > > #include "exec/cpu-all.h" > > FIELD(TB_FLAGS, MEM_IDX, 0, 3) > -FIELD(TB_FLAGS, LMUL, 3, 3) > -FIELD(TB_FLAGS, SEW, 6, 3) > -/* Skip MSTATUS_VS (0x600) bits */ > -FIELD(TB_FLAGS, VL_EQ_VLMAX, 11, 1) > -FIELD(TB_FLAGS, VILL, 12, 1) > -/* Skip MSTATUS_FS (0x6000) bits */ > +FIELD(TB_FLAGS, FS, 3, 2) > +/* Vector flags */ > +FIELD(TB_FLAGS, VS, 5, 2) > +FIELD(TB_FLAGS, LMUL, 7, 3) > +FIELD(TB_FLAGS, SEW, 10, 3) > +FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1) > +FIELD(TB_FLAGS, VILL, 14, 1) > /* Is a Hypervisor instruction load/store allowed? */ > FIELD(TB_FLAGS, HLSX, 15, 1) > FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2) > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 9d50e7bbb6..87c6effcc2 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -79,16 +79,17 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > } > > #ifdef CONFIG_USER_ONLY > - flags |= TB_FLAGS_MSTATUS_FS; > - flags |= TB_FLAGS_MSTATUS_VS; > + flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY); > + flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY); > #else > flags |= cpu_mmu_index(env, 0); > if (riscv_cpu_fp_enabled(env)) { > - flags |= env->mstatus & MSTATUS_FS; > + flags = FIELD_DP32(flags, TB_FLAGS, FS, > + get_field(env->mstatus, MSTATUS_FS)); > } > - > if (riscv_cpu_vector_enabled(env)) { > - flags |= env->mstatus & MSTATUS_VS; > + flags = FIELD_DP32(flags, TB_FLAGS, VS, > + get_field(env->mstatus, MSTATUS_VS)); > } > > if (riscv_has_ext(env, RVH)) { > diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc > index f2e3d38515..6297c3b50d 100644 > --- a/target/riscv/insn_trans/trans_rvv.c.inc > +++ b/target/riscv/insn_trans/trans_rvv.c.inc > @@ -29,12 +29,12 @@ static inline bool is_overlapped(const int8_t astart, int8_t asize, > > static bool require_rvv(DisasContext *s) > { > - return s->mstatus_vs != 0; > + return s->mstatus_vs != EXT_STATUS_DISABLED; > } > > static bool require_rvf(DisasContext *s) > { > - if (s->mstatus_fs == 0) { > + if (s->mstatus_fs == EXT_STATUS_DISABLED) { > return false; > } > > @@ -52,7 +52,7 @@ static bool require_rvf(DisasContext *s) > > static bool require_scale_rvf(DisasContext *s) > { > - if (s->mstatus_fs == 0) { > + if (s->mstatus_fs == EXT_STATUS_DISABLED) { > return false; > } > > @@ -70,7 +70,7 @@ static bool require_scale_rvf(DisasContext *s) > > static bool require_scale_rvfmin(DisasContext *s) > { > - if (s->mstatus_fs == 0) { > + if (s->mstatus_fs == EXT_STATUS_DISABLED) { > return false; > } > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index 880f6318aa..85ca3ba202 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -611,9 +611,9 @@ static void mark_fs_dirty(DisasContext *ctx) > return; > } > > - if (ctx->mstatus_fs != MSTATUS_FS) { > + if (ctx->mstatus_fs != EXT_STATUS_DIRTY) { > /* Remember the state change for the rest of the TB. */ > - ctx->mstatus_fs = MSTATUS_FS; > + ctx->mstatus_fs = EXT_STATUS_DIRTY; > > tmp = tcg_temp_new(); > tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus)); > @@ -621,9 +621,9 @@ static void mark_fs_dirty(DisasContext *ctx) > tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus)); > } > > - if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) { > + if (ctx->virt_enabled && ctx->mstatus_hs_fs != EXT_STATUS_DIRTY) { > /* Remember the stage change for the rest of the TB. */ > - ctx->mstatus_hs_fs = MSTATUS_FS; > + ctx->mstatus_hs_fs = EXT_STATUS_DIRTY; > > tmp = tcg_temp_new(); > tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs)); > @@ -645,9 +645,9 @@ static void mark_vs_dirty(DisasContext *ctx) > { > TCGv tmp; > > - if (ctx->mstatus_vs != MSTATUS_VS) { > + if (ctx->mstatus_vs != EXT_STATUS_DIRTY) { > /* Remember the state change for the rest of the TB. */ > - ctx->mstatus_vs = MSTATUS_VS; > + ctx->mstatus_vs = EXT_STATUS_DIRTY; > > tmp = tcg_temp_new(); > tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus)); > @@ -655,9 +655,9 @@ static void mark_vs_dirty(DisasContext *ctx) > tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus)); > } > > - if (ctx->virt_enabled && ctx->mstatus_hs_vs != MSTATUS_VS) { > + if (ctx->virt_enabled && ctx->mstatus_hs_vs != EXT_STATUS_DIRTY) { > /* Remember the stage change for the rest of the TB. */ > - ctx->mstatus_hs_vs = MSTATUS_VS; > + ctx->mstatus_hs_vs = EXT_STATUS_DIRTY; > > tmp = tcg_temp_new(); > tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs)); > @@ -1153,8 +1153,8 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > > ctx->pc_succ_insn = ctx->base.pc_first; > ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX); > - ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS; > - ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS; > + ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS); > + ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS); > ctx->priv_ver = env->priv_ver; > ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED); > ctx->misa_ext = env->misa_ext;
On 2023/3/24 20:53, liweiwei wrote: > > On 2023/3/24 13:59, LIU Zhiwei wrote: >> The pointer masking is the only extension that directly use status. >> The vector or float extension uses the status in an indirect way. >> >> Replace the pointer masking extension special status fields with >> the general status. >> >> Signed-off-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> >> --- >> target/riscv/cpu.c | 2 +- >> target/riscv/cpu.h | 9 +++++++++ >> target/riscv/cpu_bits.h | 6 ------ >> target/riscv/csr.c | 14 +++++++------- >> 4 files changed, 17 insertions(+), 14 deletions(-) >> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c >> index 1e97473af2..1135106b3e 100644 >> --- a/target/riscv/cpu.c >> +++ b/target/riscv/cpu.c >> @@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj) >> i++; >> } >> /* mmte is supposed to have pm.current hardwired to 1 */ >> - env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT); >> + env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT); >> #endif >> env->xl = riscv_cpu_mxl(env); >> riscv_cpu_update_mask(env); >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >> index 12fe8d8546..5049e21518 100644 >> --- a/target/riscv/cpu.h >> +++ b/target/riscv/cpu.h >> @@ -99,6 +99,15 @@ enum { >> TRANSLATE_G_STAGE_FAIL >> }; >> +/* Extension Context Status */ >> +enum { >> + EXT_STATUS_DISABLED = 0, >> + EXT_STATUS_INITIAL, >> + EXT_STATUS_CLEAN, >> + EXT_STATUS_DIRTY, >> + EXT_STATUS_MASK, > > I think the right value for EXT_STATUS_MASK should be 3 here. Yes, it is. > > And it can replace the following PM_XS_MASK. Once I wanted to replace PM_XS_MASK with EXT_STATUS_MASK here. But PM_XS_MASK has a ULL type which is needed for a 64-bit register. So I want to drop the definition of EXT_STATUS_MASK from here. And define a EXT_STATUS_MASK macro in cpu_bits.h. It will replace the PM_XS_MASK. Zhiwei > > Regards, > > Weiwei Li > >> +}; >> + >> #define MMU_USER_IDX 3 >> #define MAX_RISCV_PMPS (16) >> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h >> index fca7ef0cef..5280bd41c2 100644 >> --- a/target/riscv/cpu_bits.h >> +++ b/target/riscv/cpu_bits.h >> @@ -736,12 +736,6 @@ typedef enum RISCVException { >> #define PM_INSN 0x00000004ULL >> #define PM_XS_MASK 0x00000003ULL >> -/* PointerMasking XS bits values */ >> -#define PM_EXT_DISABLE 0x00000000ULL >> -#define PM_EXT_INITIAL 0x00000001ULL >> -#define PM_EXT_CLEAN 0x00000002ULL >> -#define PM_EXT_DIRTY 0x00000003ULL >> - >> /* Execution enviornment configuration bits */ >> #define MENVCFG_FIOM BIT(0) >> #define MENVCFG_CBIE (3UL << 4) >> diff --git a/target/riscv/csr.c b/target/riscv/csr.c >> index d522efc0b6..abea7b749e 100644 >> --- a/target/riscv/csr.c >> +++ b/target/riscv/csr.c >> @@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState >> *env, int csrno, >> /* hardwiring pm.instruction bit to 0, since it's not >> supported yet */ >> wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN); >> - env->mmte = wpri_val | PM_EXT_DIRTY; >> + env->mmte = wpri_val | EXT_STATUS_DIRTY; >> riscv_cpu_update_mask(env); >> /* Set XS and SD bits, since PM CSRs are dirty */ >> @@ -3593,7 +3593,7 @@ static RISCVException >> write_mpmmask(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) { >> env->cur_pmmask = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS; >> @@ -3621,7 +3621,7 @@ static RISCVException >> write_spmmask(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) { >> env->cur_pmmask = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS; >> @@ -3649,7 +3649,7 @@ static RISCVException >> write_upmmask(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) { >> env->cur_pmmask = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS; >> @@ -3673,7 +3673,7 @@ static RISCVException >> write_mpmbase(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) { >> env->cur_pmbase = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS; >> @@ -3701,7 +3701,7 @@ static RISCVException >> write_spmbase(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) { >> env->cur_pmbase = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS; >> @@ -3729,7 +3729,7 @@ static RISCVException >> write_upmbase(CPURISCVState *env, int csrno, >> if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) { >> env->cur_pmbase = val; >> } >> - env->mmte |= PM_EXT_DIRTY; >> + env->mmte |= EXT_STATUS_DIRTY; >> /* Set XS and SD bits, since PM CSRs are dirty */ >> mstatus = env->mstatus | MSTATUS_XS;