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

[Weiwei Li]

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)