Re: [RFC PATCH v2 2/4] target/riscv: Reuse TB_FLAGS.MSTATUS_HFS_FS

[Mayuresh Chitale]

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)
>