| Message ID | 20230414160202.1298242-3-mchitale@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | Smstateen FCSR | expand |
On 2023/4/15 00:02, Mayuresh Chitale wrote: > When misa.F is clear, TB_FLAGS.MSTATUS_HS_FS field is unused and can > be used to save the current state of smstateen0.FCSR check which is > needed by the floating point translation routines. > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > --- > target/riscv/cpu_helper.c | 12 ++++++++++++ > target/riscv/translate.c | 7 +++++++ > 2 files changed, 19 insertions(+) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 433ea529b0..fd1731cc39 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS, > get_field(env->mstatus_hs, MSTATUS_VS)); > } > + /* > + * If misa.F is 0 then the MSTATUS_HS_FS field of the tb->flags > + * can be used to pass the current state of the smstateen.FCSR bit > + * which must be checked for in the floating point translation routines > + */ > + if (!riscv_has_ext(env, RVF)) { > + if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) { > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1); > + } else { > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0); > + } > + } > if (cpu->cfg.debug && !icount_enabled()) { > flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled); > } > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index d0094922b6..e29bbb8b70 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -79,6 +79,7 @@ typedef struct DisasContext { > int frm; > RISCVMXL ol; > bool virt_inst_excp; > + bool smstateen_fcsr_ok; > bool virt_enabled; > const RISCVCPUConfig *cfg_ptr; > bool hlsx; > @@ -1202,6 +1203,12 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > ctx->zero = tcg_constant_tl(0); > ctx->virt_inst_excp = false; > + if (has_ext(ctx, RVF)) { > + ctx->smstateen_fcsr_ok = 1; > + } else { > + ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, > + MSTATUS_HS_FS); Seems unaligned with tb_flags. Otherwise, LGTM. Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn> Weiwei Li > + } > } > > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
On 2023/4/15 00:02, Mayuresh Chitale wrote: > When misa.F is clear, TB_FLAGS.MSTATUS_HS_FS field is unused and can > be used to save the current state of smstateen0.FCSR check which is > needed by the floating point translation routines. > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > --- > target/riscv/cpu_helper.c | 12 ++++++++++++ > target/riscv/translate.c | 7 +++++++ > 2 files changed, 19 insertions(+) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 433ea529b0..fd1731cc39 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS, > get_field(env->mstatus_hs, MSTATUS_VS)); > } > + /* > + * If misa.F is 0 then the MSTATUS_HS_FS field of the tb->flags > + * can be used to pass the current state of the smstateen.FCSR bit > + * which must be checked for in the floating point translation routines > + */ > + if (!riscv_has_ext(env, RVF)) { > + if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) { > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1); > + } else { > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0); > + } > + } > if (cpu->cfg.debug && !icount_enabled()) { > flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled); > } > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index d0094922b6..e29bbb8b70 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -79,6 +79,7 @@ typedef struct DisasContext { > int frm; > RISCVMXL ol; > bool virt_inst_excp; > + bool smstateen_fcsr_ok; > bool virt_enabled; > const RISCVCPUConfig *cfg_ptr; > bool hlsx; > @@ -1202,6 +1203,12 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > ctx->zero = tcg_constant_tl(0); > ctx->virt_inst_excp = false; > + if (has_ext(ctx, RVF)) { > + ctx->smstateen_fcsr_ok = 1; > + } else { > + ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, > + MSTATUS_HS_FS); By the way, it may introduce new question when MSTATUS_FS and MSTATUS_HS_FS is merged to save bits in tb_flag by Richerd's patchset: 20230412114333.118895-5-richard.henderson@linaro.org such as: the check "s->mstatus_fs == 0" in require_rvf() will be false if smstateen_fcsr_ok is true. However, this should be true in this case to indicate F is diabled. So we may need to set ctx->mstatus_fs = 0 here once merged with Richerd's patchset. Regards, Weiwei Li > + } > } > > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
On Sat, Apr 15, 2023 at 7:15 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote: > > > On 2023/4/15 00:02, Mayuresh Chitale wrote: > > When misa.F is clear, TB_FLAGS.MSTATUS_HS_FS field is unused and can > > be used to save the current state of smstateen0.FCSR check which is > > needed by the floating point translation routines. > > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> > > --- > > target/riscv/cpu_helper.c | 12 ++++++++++++ > > target/riscv/translate.c | 7 +++++++ > > 2 files changed, 19 insertions(+) > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > index 433ea529b0..fd1731cc39 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > > flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS, > > get_field(env->mstatus_hs, MSTATUS_VS)); > > } > > + /* > > + * If misa.F is 0 then the MSTATUS_HS_FS field of the tb->flags > > + * can be used to pass the current state of the smstateen.FCSR bit > > + * which must be checked for in the floating point translation routines > > + */ > > + if (!riscv_has_ext(env, RVF)) { > > + if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) { > > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1); > > + } else { > > + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0); > > + } > > + } > > if (cpu->cfg.debug && !icount_enabled()) { > > flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled); > > } > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > > index d0094922b6..e29bbb8b70 100644 > > --- a/target/riscv/translate.c > > +++ b/target/riscv/translate.c > > @@ -79,6 +79,7 @@ typedef struct DisasContext { > > int frm; > > RISCVMXL ol; > > bool virt_inst_excp; > > + bool smstateen_fcsr_ok; > > bool virt_enabled; > > const RISCVCPUConfig *cfg_ptr; > > bool hlsx; > > @@ -1202,6 +1203,12 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > > ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > > ctx->zero = tcg_constant_tl(0); > > ctx->virt_inst_excp = false; > > + if (has_ext(ctx, RVF)) { > > + ctx->smstateen_fcsr_ok = 1; > > + } else { > > + ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, > > + MSTATUS_HS_FS); > > By the way, it may introduce new question when MSTATUS_FS and > MSTATUS_HS_FS is merged to save bits in tb_flag > > by Richerd's patchset: 20230412114333.118895-5-richard.henderson@linaro.org > > such as: the check "s->mstatus_fs == 0" in require_rvf() will be false > if smstateen_fcsr_ok is true. > > However, this should be true in this case to indicate F is diabled. > > So we may need to set ctx->mstatus_fs = 0 here once merged with > Richerd's patchset. Yes, that is correct. > > Regards, > > Weiwei Li > > > + } > > } > > > > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) >
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 433ea529b0..fd1731cc39 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -105,6 +105,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS, get_field(env->mstatus_hs, MSTATUS_VS)); } + /* + * If misa.F is 0 then the MSTATUS_HS_FS field of the tb->flags + * can be used to pass the current state of the smstateen.FCSR bit + * which must be checked for in the floating point translation routines + */ + if (!riscv_has_ext(env, RVF)) { + if (smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR) == RISCV_EXCP_NONE) { + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 1); + } else { + flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS, 0); + } + } if (cpu->cfg.debug && !icount_enabled()) { flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled); } diff --git a/target/riscv/translate.c b/target/riscv/translate.c index d0094922b6..e29bbb8b70 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -79,6 +79,7 @@ typedef struct DisasContext { int frm; RISCVMXL ol; bool virt_inst_excp; + bool smstateen_fcsr_ok; bool virt_enabled; const RISCVCPUConfig *cfg_ptr; bool hlsx; @@ -1202,6 +1203,12 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); ctx->zero = tcg_constant_tl(0); ctx->virt_inst_excp = false; + if (has_ext(ctx, RVF)) { + ctx->smstateen_fcsr_ok = 1; + } else { + ctx->smstateen_fcsr_ok = FIELD_EX32(tb_flags, TB_FLAGS, + MSTATUS_HS_FS); + } } static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu)
When misa.F is clear, TB_FLAGS.MSTATUS_HS_FS field is unused and can be used to save the current state of smstateen0.FCSR check which is needed by the floating point translation routines. Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com> --- target/riscv/cpu_helper.c | 12 ++++++++++++ target/riscv/translate.c | 7 +++++++ 2 files changed, 19 insertions(+)