diff mbox series

[v10,4/5] target/riscv: smstateen check for fcsr

Message ID 20221003114718.30659-5-mchitale@ventanamicro.com
State New
Headers show
Series RISC-V Smstateen support | expand

Commit Message

Mayuresh Chitale Oct. 3, 2022, 11:47 a.m. UTC
If smstateen is implemented and sstateen0.fcsr is clear then the floating point
operations must return illegal instruction exception or virtual instruction
trap, if relevant.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 target/riscv/csr.c                        | 23 ++++++++++++
 target/riscv/insn_trans/trans_rvf.c.inc   | 43 +++++++++++++++++++++--
 target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++
 3 files changed, 75 insertions(+), 3 deletions(-)

Comments

Weiwei Li Oct. 3, 2022, 1:02 p.m. UTC | #1
On 2022/10/3 19:47, Mayuresh Chitale wrote:
> If smstateen is implemented and sstateen0.fcsr is clear then the floating point
> operations must return illegal instruction exception or virtual instruction
> trap, if relevant.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
>   target/riscv/csr.c                        | 23 ++++++++++++
>   target/riscv/insn_trans/trans_rvf.c.inc   | 43 +++++++++++++++++++++--
>   target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++
>   3 files changed, 75 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 71236f2b5d..8b25f885ec 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -84,6 +84,10 @@ static RISCVException fs(CPURISCVState *env, int csrno)
>           !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
>           return RISCV_EXCP_ILLEGAL_INST;
>       }
> +
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
> +        return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
> +    }
>   #endif
>       return RISCV_EXCP_NONE;
>   }
> @@ -2023,6 +2027,9 @@ static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
>                                         target_ulong new_val)
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
>   
>       return write_mstateen(env, csrno, wr_mask, new_val);
>   }
> @@ -2059,6 +2066,10 @@ static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>   
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
> +
>       return write_mstateenh(env, csrno, wr_mask, new_val);
>   }
>   
> @@ -2096,6 +2107,10 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>   
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
> +
>       return write_hstateen(env, csrno, wr_mask, new_val);
>   }
>   
> @@ -2135,6 +2150,10 @@ static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>   
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
> +
>       return write_hstateenh(env, csrno, wr_mask, new_val);
>   }
>   
> @@ -2182,6 +2201,10 @@ static RISCVException write_sstateen0(CPURISCVState *env, int csrno,
>   {
>       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>   
> +    if (!riscv_has_ext(env, RVF)) {
> +        wr_mask |= SMSTATEEN0_FCSR;
> +    }
> +
>       return write_sstateen(env, csrno, wr_mask, new_val);
>   }
>   
> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
> index a1d3eb52ad..93657680c6 100644
> --- a/target/riscv/insn_trans/trans_rvf.c.inc
> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> @@ -24,9 +24,46 @@
>               return false; \
>   } while (0)
>   
> -#define REQUIRE_ZFINX_OR_F(ctx) do {\
> -    if (!ctx->cfg_ptr->ext_zfinx) { \
> -        REQUIRE_EXT(ctx, RVF); \
> +#ifndef CONFIG_USER_ONLY
> +static inline bool smstateen_fcsr_check(DisasContext *ctx, int index)
> +{
> +    CPUState *cpu = ctx->cs;
> +    CPURISCVState *env = cpu->env_ptr;
> +    uint64_t stateen = env->mstateen[index];
> +
> +    if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {
> +        return true;
> +    }
> +
> +    if (ctx->virt_enabled) {
> +        stateen &= env->hstateen[index];
> +    }
> +
> +    if (env->priv == PRV_U && has_ext(ctx, RVS)) {
> +        stateen &= env->sstateen[index];
> +    }
> +
> +    if (!(stateen & SMSTATEEN0_FCSR)) {
> +        if (ctx->virt_enabled) {
> +            ctx->virt_inst_excp = true;
> +        }

Are there any considerations for adding  virt_inst_excp instead of directly

"generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)" here?

Regards,

Weiwei Li

> +        return false;
> +    }
> +
> +    return true;
> +}
> +#else
> +#define smstateen_fcsr_check(ctx, index) (true)
> +#endif
> +
> +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> +    if (!has_ext(ctx, RVF)) { \
> +        if (!ctx->cfg_ptr->ext_zfinx) { \
> +            return false; \
> +        } \
> +        if (!smstateen_fcsr_check(ctx, 0)) { \
> +            return false; \
> +        } \
>       } \
>   } while (0)
>   
> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
> index 5d07150cd0..6c2e338c0a 100644
> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> @@ -20,18 +20,27 @@
>       if (!ctx->cfg_ptr->ext_zfh) {      \
>           return false;         \
>       }                         \
> +    if (!smstateen_fcsr_check(ctx, 0)) { \
> +        return false; \
> +    } \
>   } while (0)
>   
>   #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
>       if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>           return false;                  \
>       }                                  \
> +    if (!smstateen_fcsr_check(ctx, 0)) { \
> +        return false; \
> +    } \
>   } while (0)
>   
>   #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \
>       if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
>           return false;                         \
>       }                                         \
> +    if (!smstateen_fcsr_check(ctx, 0)) { \
> +        return false; \
> +    } \
>   } while (0)
>   
>   #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
> @@ -39,6 +48,9 @@
>             ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin)) {     \
>           return false;                                        \
>       }                                                        \
> +    if (!smstateen_fcsr_check(ctx, 0)) { \
> +        return false; \
> +    } \
>   } while (0)
>   
>   static bool trans_flh(DisasContext *ctx, arg_flh *a)
Mayuresh Chitale Oct. 4, 2022, 6:51 a.m. UTC | #2
On Mon, 2022-10-03 at 21:02 +0800, weiwei wrote:
> On 2022/10/3 19:47, Mayuresh Chitale wrote:
> > If smstateen is implemented and sstateen0.fcsr is clear then the
> > floating point
> > operations must return illegal instruction exception or virtual
> > instruction
> > trap, if relevant.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > ---
> >   target/riscv/csr.c                        | 23 ++++++++++++
> >   target/riscv/insn_trans/trans_rvf.c.inc   | 43
> > +++++++++++++++++++++--
> >   target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++
> >   3 files changed, 75 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 71236f2b5d..8b25f885ec 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -84,6 +84,10 @@ static RISCVException fs(CPURISCVState *env, int
> > csrno)
> >           !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
> >           return RISCV_EXCP_ILLEGAL_INST;
> >       }
> > +
> > +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
> > +        return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
> > +    }
> >   #endif
> >       return RISCV_EXCP_NONE;
> >   }
> > @@ -2023,6 +2027,9 @@ static RISCVException
> > write_mstateen0(CPURISCVState *env, int csrno,
> >                                         target_ulong new_val)
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> >   
> >       return write_mstateen(env, csrno, wr_mask, new_val);
> >   }
> > @@ -2059,6 +2066,10 @@ static RISCVException
> > write_mstateen0h(CPURISCVState *env, int csrno,
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> >   
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> > +
> >       return write_mstateenh(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -2096,6 +2107,10 @@ static RISCVException
> > write_hstateen0(CPURISCVState *env, int csrno,
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> >   
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> > +
> >       return write_hstateen(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -2135,6 +2150,10 @@ static RISCVException
> > write_hstateen0h(CPURISCVState *env, int csrno,
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> >   
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> > +
> >       return write_hstateenh(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -2182,6 +2201,10 @@ static RISCVException
> > write_sstateen0(CPURISCVState *env, int csrno,
> >   {
> >       uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
> >   
> > +    if (!riscv_has_ext(env, RVF)) {
> > +        wr_mask |= SMSTATEEN0_FCSR;
> > +    }
> > +
> >       return write_sstateen(env, csrno, wr_mask, new_val);
> >   }
> >   
> > diff --git a/target/riscv/insn_trans/trans_rvf.c.inc
> > b/target/riscv/insn_trans/trans_rvf.c.inc
> > index a1d3eb52ad..93657680c6 100644
> > --- a/target/riscv/insn_trans/trans_rvf.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> > @@ -24,9 +24,46 @@
> >               return false; \
> >   } while (0)
> >   
> > -#define REQUIRE_ZFINX_OR_F(ctx) do {\
> > -    if (!ctx->cfg_ptr->ext_zfinx) { \
> > -        REQUIRE_EXT(ctx, RVF); \
> > +#ifndef CONFIG_USER_ONLY
> > +static inline bool smstateen_fcsr_check(DisasContext *ctx, int
> > index)
> > +{
> > +    CPUState *cpu = ctx->cs;
> > +    CPURISCVState *env = cpu->env_ptr;
> > +    uint64_t stateen = env->mstateen[index];
> > +
> > +    if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {
> > +        return true;
> > +    }
> > +
> > +    if (ctx->virt_enabled) {
> > +        stateen &= env->hstateen[index];
> > +    }
> > +
> > +    if (env->priv == PRV_U && has_ext(ctx, RVS)) {
> > +        stateen &= env->sstateen[index];
> > +    }
> > +
> > +    if (!(stateen & SMSTATEEN0_FCSR)) {
> > +        if (ctx->virt_enabled) {
> > +            ctx->virt_inst_excp = true;
> > +        }
> 
> Are there any considerations for adding  virt_inst_excp instead of
> directly
> 
> "generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)" here?
> 
> Regards,
> 
> Weiwei Li

I had considered it but I think this is a simpler approach as the rest
of the code path stays the same as generating an illegal instruction
exception, for e.g setting the bins value (tval). Also we need to
return true from all the caller trans_* functions even if the smstateen
check has failed.
> 
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +#else
> > +#define smstateen_fcsr_check(ctx, index) (true)
> > +#endif
> > +
> > +#define REQUIRE_ZFINX_OR_F(ctx) do { \
> > +    if (!has_ext(ctx, RVF)) { \
> > +        if (!ctx->cfg_ptr->ext_zfinx) { \
> > +            return false; \
> > +        } \
> > +        if (!smstateen_fcsr_check(ctx, 0)) { \
> > +            return false; \
> > +        } \
> >       } \
> >   } while (0)
> >   
> > diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc
> > b/target/riscv/insn_trans/trans_rvzfh.c.inc
> > index 5d07150cd0..6c2e338c0a 100644
> > --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
> > @@ -20,18 +20,27 @@
> >       if (!ctx->cfg_ptr->ext_zfh) {      \
> >           return false;         \
> >       }                         \
> > +    if (!smstateen_fcsr_check(ctx, 0)) { \
> > +        return false; \
> > +    } \
> >   } while (0)
> >   
> >   #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
> >       if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
> >           return false;                  \
> >       }                                  \
> > +    if (!smstateen_fcsr_check(ctx, 0)) { \
> > +        return false; \
> > +    } \
> >   } while (0)
> >   
> >   #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \
> >       if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
> >           return false;                         \
> >       }                                         \
> > +    if (!smstateen_fcsr_check(ctx, 0)) { \
> > +        return false; \
> > +    } \
> >   } while (0)
> >   
> >   #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
> > @@ -39,6 +48,9 @@
> >             ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin)) 
> > {     \
> >           return false;                                        \
> >       }                                                        \
> > +    if (!smstateen_fcsr_check(ctx, 0)) { \
> > +        return false; \
> > +    } \
> >   } while (0)
> >   
> >   static bool trans_flh(DisasContext *ctx, arg_flh *a)
Weiwei Li Oct. 4, 2022, 1:23 p.m. UTC | #3
On 2022/10/4 14:51, mchitale@ventanamicro.com wrote:
> On Mon, 2022-10-03 at 21:02 +0800, weiwei wrote:
>> On 2022/10/3 19:47, Mayuresh Chitale wrote:
>>> If smstateen is implemented and sstateen0.fcsr is clear then the
>>> floating point
>>> operations must return illegal instruction exception or virtual
>>> instruction
>>> trap, if relevant.
>>>
>>> Signed-off-by: Mayuresh Chitale<mchitale@ventanamicro.com>
>>> ---
>>>    target/riscv/csr.c                        | 23 ++++++++++++
>>>    target/riscv/insn_trans/trans_rvf.c.inc   | 43
>>> +++++++++++++++++++++--
>>>    target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++
>>>    3 files changed, 75 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>> index 71236f2b5d..8b25f885ec 100644
>>> --- a/target/riscv/csr.c
>>> +++ b/target/riscv/csr.c
>>> @@ -84,6 +84,10 @@ static RISCVException fs(CPURISCVState *env, int
>>> csrno)
>>>            !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
>>>            return RISCV_EXCP_ILLEGAL_INST;
>>>        }
>>> +
>>> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>>> +        return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
>>> +    }
>>>    #endif
>>>        return RISCV_EXCP_NONE;
>>>    }
>>> @@ -2023,6 +2027,9 @@ static RISCVException
>>> write_mstateen0(CPURISCVState *env, int csrno,
>>>                                          target_ulong new_val)
>>>    {
>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>> +    if (!riscv_has_ext(env, RVF)) {
>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>> +    }
>>>    
>>>        return write_mstateen(env, csrno, wr_mask, new_val);
>>>    }
>>> @@ -2059,6 +2066,10 @@ static RISCVException
>>> write_mstateen0h(CPURISCVState *env, int csrno,
>>>    {
>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>    
>>> +    if (!riscv_has_ext(env, RVF)) {
>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>> +    }
>>> +
>>>        return write_mstateenh(env, csrno, wr_mask, new_val);
>>>    }
>>>    
>>> @@ -2096,6 +2107,10 @@ static RISCVException
>>> write_hstateen0(CPURISCVState *env, int csrno,
>>>    {
>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>    
>>> +    if (!riscv_has_ext(env, RVF)) {
>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>> +    }
>>> +
>>>        return write_hstateen(env, csrno, wr_mask, new_val);
>>>    }
>>>    
>>> @@ -2135,6 +2150,10 @@ static RISCVException
>>> write_hstateen0h(CPURISCVState *env, int csrno,
>>>    {
>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>    
>>> +    if (!riscv_has_ext(env, RVF)) {
>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>> +    }
>>> +
>>>        return write_hstateenh(env, csrno, wr_mask, new_val);
>>>    }
>>>    
>>> @@ -2182,6 +2201,10 @@ static RISCVException
>>> write_sstateen0(CPURISCVState *env, int csrno,
>>>    {
>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>    
>>> +    if (!riscv_has_ext(env, RVF)) {
>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>> +    }
>>> +
>>>        return write_sstateen(env, csrno, wr_mask, new_val);
>>>    }
>>>    
>>> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc
>>> b/target/riscv/insn_trans/trans_rvf.c.inc
>>> index a1d3eb52ad..93657680c6 100644
>>> --- a/target/riscv/insn_trans/trans_rvf.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
>>> @@ -24,9 +24,46 @@
>>>                return false; \
>>>    } while (0)
>>>    
>>> -#define REQUIRE_ZFINX_OR_F(ctx) do {\
>>> -    if (!ctx->cfg_ptr->ext_zfinx) { \
>>> -        REQUIRE_EXT(ctx, RVF); \
>>> +#ifndef CONFIG_USER_ONLY
>>> +static inline bool smstateen_fcsr_check(DisasContext *ctx, int
>>> index)
>>> +{
>>> +    CPUState *cpu = ctx->cs;
>>> +    CPURISCVState *env = cpu->env_ptr;
>>> +    uint64_t stateen = env->mstateen[index];
>>> +
>>> +    if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {
>>> +        return true;
>>> +    }
>>> +
>>> +    if (ctx->virt_enabled) {
>>> +        stateen &= env->hstateen[index];
>>> +    }
>>> +
>>> +    if (env->priv == PRV_U && has_ext(ctx, RVS)) {
>>> +        stateen &= env->sstateen[index];
>>> +    }
>>> +
>>> +    if (!(stateen & SMSTATEEN0_FCSR)) {
>>> +        if (ctx->virt_enabled) {
>>> +            ctx->virt_inst_excp = true;
>>> +        }
>> Are there any considerations for adding  virt_inst_excp instead of
>> directly
>>
>> "generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)" here?
>>
>> Regards,
>>
>> Weiwei Li
> I had considered it but I think this is a simpler approach as the rest
> of the code path stays the same as generating an illegal instruction
> exception, for e.g setting the bins value (tval).

OK,  we did need to set bins value for virt instruction exception. 
However I prefer directly call a

new gen_exception_virt function(similar togen_exception_illegal) here.

>   Also we need to
> return true from all the caller trans_* functions even if the smstateen
> check has failed.

False is returned when smstateen check fails currently.

Regards,

Weiwei Li

>>> +        return false;
>>> +    }
>>> +
>>> +    return true;
>>> +}
>>> +#else
>>> +#define smstateen_fcsr_check(ctx, index) (true)
>>> +#endif
>>> +
>>> +#define REQUIRE_ZFINX_OR_F(ctx) do { \
>>> +    if (!has_ext(ctx, RVF)) { \
>>> +        if (!ctx->cfg_ptr->ext_zfinx) { \
>>> +            return false; \
>>> +        } \
>>> +        if (!smstateen_fcsr_check(ctx, 0)) { \
>>> +            return false; \
>>> +        } \
>>>        } \
>>>    } while (0)
>>>    
>>> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> b/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> index 5d07150cd0..6c2e338c0a 100644
>>> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
>>> @@ -20,18 +20,27 @@
>>>        if (!ctx->cfg_ptr->ext_zfh) {      \
>>>            return false;         \
>>>        }                         \
>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>> +        return false; \
>>> +    } \
>>>    } while (0)
>>>    
>>>    #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
>>>        if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>>>            return false;                  \
>>>        }                                  \
>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>> +        return false; \
>>> +    } \
>>>    } while (0)
>>>    
>>>    #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \
>>>        if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
>>>            return false;                         \
>>>        }                                         \
>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>> +        return false; \
>>> +    } \
>>>    } while (0)
>>>    
>>>    #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
>>> @@ -39,6 +48,9 @@
>>>              ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin))
>>> {     \
>>>            return false;                                        \
>>>        }                                                        \
>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>> +        return false; \
>>> +    } \
>>>    } while (0)
>>>    
>>>    static bool trans_flh(DisasContext *ctx, arg_flh *a)
Mayuresh Chitale Oct. 6, 2022, 5:06 p.m. UTC | #4
On Tue, 2022-10-04 at 21:23 +0800, weiwei wrote:
>     
> 
>     
> 
>     On 2022/10/4 14:51,
>       mchitale@ventanamicro.com wrote:
> 
>     
> 
>     
> >       On Mon, 2022-10-03 at 21:02 +0800, weiwei wrote:
> >       
> > >         On 2022/10/3 19:47, Mayuresh Chitale wrote:
> > >         
> > > >           If smstateen is implemented and sstateen0.fcsr is
> > > > clear then thefloating pointoperations must return illegal
> > > > instruction exception or virtualinstructiontrap, if relevant.
> > > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > > > ---  target/riscv/csr.c                        | 23
> > > > ++++++++++++  target/riscv/insn_trans/trans_rvf.c.inc   |
> > > > 43+++++++++++++++++++++
> > > > --  target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++  3
> > > > files changed, 75 insertions(+), 3 deletions(-)
> > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.cindex
> > > > 71236f2b5d..8b25f885ec 100644--- a/target/riscv/csr.c+++
> > > > b/target/riscv/csr.c@@ -84,6 +84,10 @@ static RISCVException
> > > > fs(CPURISCVState *env,
> > > > intcsrno)          !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx)
> > > > {          return RISCV_EXCP_ILLEGAL_INST;      }++    if
> > > > (!env->debugger && !riscv_cpu_fp_enabled(env)) {+        return
> > > > smstateen_acc_ok(env, 0,
> > > > SMSTATEEN0_FCSR);+    }  #endif      return
> > > > RISCV_EXCP_NONE;  }@@ -2023,6 +2027,9 @@ static
> > > > RISCVExceptionwrite_mstateen0(CPURISCVState *env, int
> > > > csrno,                                        target_ulong
> > > > new_val)  {      uint64_t wr_mask = SMSTATEEN_STATEEN |
> > > > SMSTATEEN0_HSENVCFG;+    if (!riscv_has_ext(env, RVF))
> > > > {+        wr_mask |= SMSTATEEN0_FCSR;+    }        return
> > > > write_mstateen(env, csrno, wr_mask, new_val);  }@@ -2059,6
> > > > +2066,10 @@ static RISCVExceptionwrite_mstateen0h(CPURISCVState
> > > > *env, int csrno,  {      uint64_t wr_mask = SMSTATEEN_STATEEN |
> > > > SMSTATEEN0_HSENVCFG;  +    if (!riscv_has_ext(env, RVF))
> > > > {+        wr_mask |= SMSTATEEN0_FCSR;+    }+      return
> > > > write_mstateenh(env, csrno, wr_mask, new_val);  }  @@ -2096,6
> > > > +2107,10 @@ static RISCVExceptionwrite_hstateen0(CPURISCVState
> > > > *env, int csrno,  {      uint64_t wr_mask = SMSTATEEN_STATEEN |
> > > > SMSTATEEN0_HSENVCFG;  +    if (!riscv_has_ext(env, RVF))
> > > > {+        wr_mask |= SMSTATEEN0_FCSR;+    }+      return
> > > > write_hstateen(env, csrno, wr_mask, new_val);  }  @@ -2135,6
> > > > +2150,10 @@ static RISCVExceptionwrite_hstateen0h(CPURISCVState
> > > > *env, int csrno,  {      uint64_t wr_mask = SMSTATEEN_STATEEN |
> > > > SMSTATEEN0_HSENVCFG;  +    if (!riscv_has_ext(env, RVF))
> > > > {+        wr_mask |= SMSTATEEN0_FCSR;+    }+      return
> > > > write_hstateenh(env, csrno, wr_mask, new_val);  }  @@ -2182,6
> > > > +2201,10 @@ static RISCVExceptionwrite_sstateen0(CPURISCVState
> > > > *env, int csrno,  {      uint64_t wr_mask = SMSTATEEN_STATEEN |
> > > > SMSTATEEN0_HSENVCFG;  +    if (!riscv_has_ext(env, RVF))
> > > > {+        wr_mask |= SMSTATEEN0_FCSR;+    }+      return
> > > > write_sstateen(env, csrno, wr_mask, new_val);  }  diff --git
> > > > a/target/riscv/insn_trans/trans_rvf.c.incb/target/riscv/insn_tr
> > > > ans/trans_rvf.c.incindex a1d3eb52ad..93657680c6 100644---
> > > > a/target/riscv/insn_trans/trans_rvf.c.inc+++
> > > > b/target/riscv/insn_trans/trans_rvf.c.inc@@ -24,9 +24,46
> > > > @@              return false; \  } while (0)  -#define
> > > > REQUIRE_ZFINX_OR_F(ctx) do {\-    if (!ctx->cfg_ptr->ext_zfinx) 
> > > > { \-        REQUIRE_EXT(ctx, RVF); \+#ifndef
> > > > CONFIG_USER_ONLY+static inline bool
> > > > smstateen_fcsr_check(DisasContext *ctx,
> > > > intindex)+{+    CPUState *cpu = ctx->cs;+    CPURISCVState *env
> > > > = cpu->env_ptr;+    uint64_t stateen = env-
> > > > >mstateen[index];++    if (!ctx->cfg_ptr->ext_smstateen || env-
> > > > >priv == PRV_M) {+        return true;+    }++    if (ctx-
> > > > >virt_enabled) {+        stateen &= env-
> > > > >hstateen[index];+    }++    if (env->priv == PRV_U &&
> > > > has_ext(ctx, RVS)) {+        stateen &= env-
> > > > >sstateen[index];+    }++    if (!(stateen & SMSTATEEN0_FCSR))
> > > > {+        if (ctx->virt_enabled) {+            ctx-
> > > > >virt_inst_excp = true;+        }
> > > >         
> > > 
> > >         Are there any considerations for adding  virt_inst_excp
> > > instead ofdirectly
> > > "generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)"
> > > here?
> > > Regards,
> > > Weiwei Li
> > >       
> > 
> >       I had considered it but I think this is a simpler approach as
> > the restof the code path stays the same as generating an illegal
> > instructionexception, for e.g setting the bins value (tval).
> >     
> 
>     OK,  we did need to set bins value for virt instruction
>       exception. However I prefer directly call a 
> 
>     
> 
>     new gen_exception_virt function(similar to gen_exception_illegal)
> here.
> 
>     
> >        Also we need toreturn true from all the caller trans_*
> > functions even if the smstateencheck has failed.
> >     
> 
>     False is returned when smstateen check fails currently.
Yes, however if we make this change then should return true if the
check fails so that the decode_opc doesnt fall through and generate
another exception. It can be done but it would be contrary to the
general convention.
>     Regards,
>     Weiwei Li
> 
>     
>     
> >       
> >       
> > >         
> > >         
> > > >           +        return false;+    }++    return
> > > > true;+}+#else+#define smstateen_fcsr_check(ctx, index)
> > > > (true)+#endif++#define REQUIRE_ZFINX_OR_F(ctx) do { \+    if
> > > > (!has_ext(ctx, RVF)) { \+        if (!ctx->cfg_ptr->ext_zfinx)
> > > > { \+            return false; \+        } \+        if
> > > > (!smstateen_fcsr_check(ctx, 0)) { \+            return false;
> > > > \+        } \      } \  } while (0)  diff --git
> > > > a/target/riscv/insn_trans/trans_rvzfh.c.incb/target/riscv/insn_
> > > > trans/trans_rvzfh.c.incindex 5d07150cd0..6c2e338c0a 100644---
> > > > a/target/riscv/insn_trans/trans_rvzfh.c.inc+++
> > > > b/target/riscv/insn_trans/trans_rvzfh.c.inc@@ -20,18 +20,27
> > > > @@      if (!ctx->cfg_ptr->ext_zfh) {      \          return
> > > > false;         \      }                         \+    if
> > > > (!smstateen_fcsr_check(ctx, 0)) { \+        return false;
> > > > \+    } \  } while (0)    #define REQUIRE_ZHINX_OR_ZFH(ctx) do
> > > > { \      if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr-
> > > > >ext_zfh) { \          return
> > > > false;                  \      }                               
> > > >    \+    if (!smstateen_fcsr_check(ctx, 0)) { \+        return
> > > > false; \+    } \  } while (0)    #define
> > > > REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \      if (!(ctx-
> > > > >cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) {
> > > > \          return
> > > > false;                         \      }                        
> > > >                  \+    if (!smstateen_fcsr_check(ctx, 0)) {
> > > > \+        return false; \+    } \  } while (0)    #define
> > > > REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \@@ -39,6
> > > > +48,9 @@            ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr-
> > > > >ext_zhinxmin)) {     \          return
> > > > false;                                        \      }         
> > > >                                                \+    if
> > > > (!smstateen_fcsr_check(ctx, 0)) { \+        return false;
> > > > \+    } \  } while (0)    static bool trans_flh(DisasContext
> > > > *ctx, arg_flh *a)
> > > >         
> > > 
> > >       
> > 
> >     
> 
>   
>
Weiwei Li Oct. 10, 2022, 2:41 p.m. UTC | #5
On 2022/10/7 01:06, mchitale@ventanamicro.com wrote:
> On Tue, 2022-10-04 at 21:23 +0800, weiwei wrote:
>>
>>
>> On 2022/10/4 14:51, mchitale@ventanamicro.com wrote:
>>> On Mon, 2022-10-03 at 21:02 +0800, weiwei wrote:
>>>> On 2022/10/3 19:47, Mayuresh Chitale wrote:
>>>>> If smstateen is implemented and sstateen0.fcsr is clear then the
>>>>> floating point
>>>>> operations must return illegal instruction exception or virtual
>>>>> instruction
>>>>> trap, if relevant.
>>>>> Signed-off-by: Mayuresh Chitale
>>>>> <mchitale@ventanamicro.com>
>>>>> ---
>>>>>    target/riscv/csr.c                        | 23 ++++++++++++
>>>>>    target/riscv/insn_trans/trans_rvf.c.inc   | 43
>>>>> +++++++++++++++++++++--
>>>>>    target/riscv/insn_trans/trans_rvzfh.c.inc | 12 +++++++
>>>>>    3 files changed, 75 insertions(+), 3 deletions(-)
>>>>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>>>>> index 71236f2b5d..8b25f885ec 100644
>>>>> --- a/target/riscv/csr.c
>>>>> +++ b/target/riscv/csr.c
>>>>> @@ -84,6 +84,10 @@ static RISCVException fs(CPURISCVState *env, int
>>>>> csrno)
>>>>>            !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
>>>>>            return RISCV_EXCP_ILLEGAL_INST;
>>>>>        }
>>>>> +
>>>>> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>>>>> +        return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
>>>>> +    }
>>>>>    #endif
>>>>>        return RISCV_EXCP_NONE;
>>>>>    }
>>>>> @@ -2023,6 +2027,9 @@ static RISCVException
>>>>> write_mstateen0(CPURISCVState *env, int csrno,
>>>>>                                          target_ulong new_val)
>>>>>    {
>>>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>>> +    if (!riscv_has_ext(env, RVF)) {
>>>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>>>> +    }
>>>>>    
>>>>>        return write_mstateen(env, csrno, wr_mask, new_val);
>>>>>    }
>>>>> @@ -2059,6 +2066,10 @@ static RISCVException
>>>>> write_mstateen0h(CPURISCVState *env, int csrno,
>>>>>    {
>>>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>>>    
>>>>> +    if (!riscv_has_ext(env, RVF)) {
>>>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>>>> +    }
>>>>> +
>>>>>        return write_mstateenh(env, csrno, wr_mask, new_val);
>>>>>    }
>>>>>    
>>>>> @@ -2096,6 +2107,10 @@ static RISCVException
>>>>> write_hstateen0(CPURISCVState *env, int csrno,
>>>>>    {
>>>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>>>    
>>>>> +    if (!riscv_has_ext(env, RVF)) {
>>>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>>>> +    }
>>>>> +
>>>>>        return write_hstateen(env, csrno, wr_mask, new_val);
>>>>>    }
>>>>>    
>>>>> @@ -2135,6 +2150,10 @@ static RISCVException
>>>>> write_hstateen0h(CPURISCVState *env, int csrno,
>>>>>    {
>>>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>>>    
>>>>> +    if (!riscv_has_ext(env, RVF)) {
>>>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>>>> +    }
>>>>> +
>>>>>        return write_hstateenh(env, csrno, wr_mask, new_val);
>>>>>    }
>>>>>    
>>>>> @@ -2182,6 +2201,10 @@ static RISCVException
>>>>> write_sstateen0(CPURISCVState *env, int csrno,
>>>>>    {
>>>>>        uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
>>>>>    
>>>>> +    if (!riscv_has_ext(env, RVF)) {
>>>>> +        wr_mask |= SMSTATEEN0_FCSR;
>>>>> +    }
>>>>> +
>>>>>        return write_sstateen(env, csrno, wr_mask, new_val);
>>>>>    }
>>>>>    
>>>>> diff --git a/target/riscv/insn_trans/trans_rvf.c.inc
>>>>> b/target/riscv/insn_trans/trans_rvf.c.inc
>>>>> index a1d3eb52ad..93657680c6 100644
>>>>> --- a/target/riscv/insn_trans/trans_rvf.c.inc
>>>>> +++ b/target/riscv/insn_trans/trans_rvf.c.inc
>>>>> @@ -24,9 +24,46 @@
>>>>>                return false; \
>>>>>    } while (0)
>>>>>    
>>>>> -#define REQUIRE_ZFINX_OR_F(ctx) do {\
>>>>> -    if (!ctx->cfg_ptr->ext_zfinx) { \
>>>>> -        REQUIRE_EXT(ctx, RVF); \
>>>>> +#ifndef CONFIG_USER_ONLY
>>>>> +static inline bool smstateen_fcsr_check(DisasContext *ctx, int
>>>>> index)
>>>>> +{
>>>>> +    CPUState *cpu = ctx->cs;
>>>>> +    CPURISCVState *env = cpu->env_ptr;
>>>>> +    uint64_t stateen = env->mstateen[index];
>>>>> +
>>>>> +    if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {
>>>>> +        return true;
>>>>> +    }
>>>>> +
>>>>> +    if (ctx->virt_enabled) {
>>>>> +        stateen &= env->hstateen[index];
>>>>> +    }
>>>>> +
>>>>> +    if (env->priv == PRV_U && has_ext(ctx, RVS)) {
>>>>> +        stateen &= env->sstateen[index];
>>>>> +    }
>>>>> +
>>>>> +    if (!(stateen & SMSTATEEN0_FCSR)) {
>>>>> +        if (ctx->virt_enabled) {
>>>>> +            ctx->virt_inst_excp = true;
>>>>> +        }
>>>> Are there any considerations for adding  virt_inst_excp instead of
>>>> directly
>>>> "generate_exception(ctx, RISCV_EXCP_VIRT_INSTRUCTION_FAULT)" here?
>>>> Regards,
>>>> Weiwei Li
>>> I had considered it but I think this is a simpler approach as the rest
>>> of the code path stays the same as generating an illegal instruction
>>> exception, for e.g setting the bins value (tval).
>>
>> OK,  we did need to set bins value for virt instruction exception. 
>> However I prefer directly call a
>>
>> new gen_exception_virt function(similar togen_exception_illegal) here.
>>
>>>   Also we need to
>>> return true from all the caller trans_* functions even if the smstateen
>>> check has failed.
>>
>> False is returned when smstateen check fails currently.
>>
> Yes, however if we make this change then should return true if the 
> check fails so that the decode_opc doesnt fall through and generate 
> another exception. It can be done but it would be contrary to the 
> general convention.

OK.  Acceptable to me.

Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Regards,

Weiwei Li

>> Regards,
>>
>> Weiwei Li
>>
>>>>> +        return false;
>>>>> +    }
>>>>> +
>>>>> +    return true;
>>>>> +}
>>>>> +#else
>>>>> +#define smstateen_fcsr_check(ctx, index) (true)
>>>>> +#endif
>>>>> +
>>>>> +#define REQUIRE_ZFINX_OR_F(ctx) do { \
>>>>> +    if (!has_ext(ctx, RVF)) { \
>>>>> +        if (!ctx->cfg_ptr->ext_zfinx) { \
>>>>> +            return false; \
>>>>> +        } \
>>>>> +        if (!smstateen_fcsr_check(ctx, 0)) { \
>>>>> +            return false; \
>>>>> +        } \
>>>>>        } \
>>>>>    } while (0)
>>>>>    
>>>>> diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc
>>>>> b/target/riscv/insn_trans/trans_rvzfh.c.inc
>>>>> index 5d07150cd0..6c2e338c0a 100644
>>>>> --- a/target/riscv/insn_trans/trans_rvzfh.c.inc
>>>>> +++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
>>>>> @@ -20,18 +20,27 @@
>>>>>        if (!ctx->cfg_ptr->ext_zfh) {      \
>>>>>            return false;         \
>>>>>        }                         \
>>>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>>>> +        return false; \
>>>>> +    } \
>>>>>    } while (0)
>>>>>    
>>>>>    #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
>>>>>        if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
>>>>>            return false;                  \
>>>>>        }                                  \
>>>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>>>> +        return false; \
>>>>> +    } \
>>>>>    } while (0)
>>>>>    
>>>>>    #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \
>>>>>        if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
>>>>>            return false;                         \
>>>>>        }                                         \
>>>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>>>> +        return false; \
>>>>> +    } \
>>>>>    } while (0)
>>>>>    
>>>>>    #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
>>>>> @@ -39,6 +48,9 @@
>>>>>              ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin))
>>>>> {     \
>>>>>            return false;                                        \
>>>>>        }                                                        \
>>>>> +    if (!smstateen_fcsr_check(ctx, 0)) { \
>>>>> +        return false; \
>>>>> +    } \
>>>>>    } while (0)
>>>>>    
>>>>>    static bool trans_flh(DisasContext *ctx, arg_flh *a)
diff mbox series

Patch

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 71236f2b5d..8b25f885ec 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -84,6 +84,10 @@  static RISCVException fs(CPURISCVState *env, int csrno)
         !RISCV_CPU(env_cpu(env))->cfg.ext_zfinx) {
         return RISCV_EXCP_ILLEGAL_INST;
     }
+
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
+        return smstateen_acc_ok(env, 0, SMSTATEEN0_FCSR);
+    }
 #endif
     return RISCV_EXCP_NONE;
 }
@@ -2023,6 +2027,9 @@  static RISCVException write_mstateen0(CPURISCVState *env, int csrno,
                                       target_ulong new_val)
 {
     uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }
 
     return write_mstateen(env, csrno, wr_mask, new_val);
 }
@@ -2059,6 +2066,10 @@  static RISCVException write_mstateen0h(CPURISCVState *env, int csrno,
 {
     uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
 
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
     return write_mstateenh(env, csrno, wr_mask, new_val);
 }
 
@@ -2096,6 +2107,10 @@  static RISCVException write_hstateen0(CPURISCVState *env, int csrno,
 {
     uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
 
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
     return write_hstateen(env, csrno, wr_mask, new_val);
 }
 
@@ -2135,6 +2150,10 @@  static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,
 {
     uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
 
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
     return write_hstateenh(env, csrno, wr_mask, new_val);
 }
 
@@ -2182,6 +2201,10 @@  static RISCVException write_sstateen0(CPURISCVState *env, int csrno,
 {
     uint64_t wr_mask = SMSTATEEN_STATEEN | SMSTATEEN0_HSENVCFG;
 
+    if (!riscv_has_ext(env, RVF)) {
+        wr_mask |= SMSTATEEN0_FCSR;
+    }
+
     return write_sstateen(env, csrno, wr_mask, new_val);
 }
 
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc b/target/riscv/insn_trans/trans_rvf.c.inc
index a1d3eb52ad..93657680c6 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,46 @@ 
             return false; \
 } while (0)
 
-#define REQUIRE_ZFINX_OR_F(ctx) do {\
-    if (!ctx->cfg_ptr->ext_zfinx) { \
-        REQUIRE_EXT(ctx, RVF); \
+#ifndef CONFIG_USER_ONLY
+static inline bool smstateen_fcsr_check(DisasContext *ctx, int index)
+{
+    CPUState *cpu = ctx->cs;
+    CPURISCVState *env = cpu->env_ptr;
+    uint64_t stateen = env->mstateen[index];
+
+    if (!ctx->cfg_ptr->ext_smstateen || env->priv == PRV_M) {
+        return true;
+    }
+
+    if (ctx->virt_enabled) {
+        stateen &= env->hstateen[index];
+    }
+
+    if (env->priv == PRV_U && has_ext(ctx, RVS)) {
+        stateen &= env->sstateen[index];
+    }
+
+    if (!(stateen & SMSTATEEN0_FCSR)) {
+        if (ctx->virt_enabled) {
+            ctx->virt_inst_excp = true;
+        }
+        return false;
+    }
+
+    return true;
+}
+#else
+#define smstateen_fcsr_check(ctx, index) (true)
+#endif
+
+#define REQUIRE_ZFINX_OR_F(ctx) do { \
+    if (!has_ext(ctx, RVF)) { \
+        if (!ctx->cfg_ptr->ext_zfinx) { \
+            return false; \
+        } \
+        if (!smstateen_fcsr_check(ctx, 0)) { \
+            return false; \
+        } \
     } \
 } while (0)
 
diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc
index 5d07150cd0..6c2e338c0a 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -20,18 +20,27 @@ 
     if (!ctx->cfg_ptr->ext_zfh) {      \
         return false;         \
     }                         \
+    if (!smstateen_fcsr_check(ctx, 0)) { \
+        return false; \
+    } \
 } while (0)
 
 #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
     if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
         return false;                  \
     }                                  \
+    if (!smstateen_fcsr_check(ctx, 0)) { \
+        return false; \
+    } \
 } while (0)
 
 #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {       \
     if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
         return false;                         \
     }                                         \
+    if (!smstateen_fcsr_check(ctx, 0)) { \
+        return false; \
+    } \
 } while (0)
 
 #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
@@ -39,6 +48,9 @@ 
           ctx->cfg_ptr->ext_zhinx || ctx->cfg_ptr->ext_zhinxmin)) {     \
         return false;                                        \
     }                                                        \
+    if (!smstateen_fcsr_check(ctx, 0)) { \
+        return false; \
+    } \
 } while (0)
 
 static bool trans_flh(DisasContext *ctx, arg_flh *a)