Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-29 Thread Mayuresh Chitale 
On Thu, 2022-07-28 at 09:09 +0100, Ben Dooks wrote:
> On 28/07/2022 07:15, Mayuresh Chitale wrote:
> > On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:
> > > 在 2022/7/24 下午11:49, Mayuresh Chitale 写道:
> > > > On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:
> > > > > 在 2022/7/21 下午11:31, Mayuresh Chitale 写道:
> > > > > > If smstateen is implemented and sstateen0.fcsr is clear
> > > > > > then
> > > > > > the
> > > > > > floating point operations must return illegal instruction
> > > > > > exception.
> > > > > > 
> > > > > > Signed-off-by: Mayuresh Chitale 
> > > > > > ---
> > > > > >target/riscv/csr.c| 23
> > > > > > ++
> > > > > >target/riscv/insn_trans/trans_rvf.c.inc   | 38
> > > > > > +--
> > > > > >target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
> > > > > >3 files changed, 63 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > > > > index ab06b117f9..a597b6cbc7 100644
> > > > > > --- a/target/riscv/csr.c
> > > > > > +++ b/target/riscv/csr.c
> > > > > > @@ -96,6 +96,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;
> > > > > >}
> > > > > > @@ -1876,6 +1880,9 @@ static RISCVException
> > > > > > write_mstateen0(CPURISCVState *env, int csrno,
> > > > > >  target_ulong
> > > > > > new_val)
> > > > > >{
> > > > > >uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > > > SMSTATEEN0_HSENVCFG;
> > > > > > +if (!riscv_has_ext(env, RVF)) {
> > > > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > > > +}
> > > > > >
> > > > > >return write_mstateen(env, csrno, wr_mask, new_val);
> > > > > >}
> > > > > > @@ -1924,6 +1931,10 @@ static RISCVException
> > > > > > write_mstateen0h(CPURISCVState *env, int csrno,
> > > > > >{
> > > > > >uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > > > SMSTATEEN0_HSENVCFG;
> > > > > >
> > > > > > +if (!riscv_has_ext(env, RVF)) {
> > > > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > > > +}
> > > > > > +
> > > > > >return write_mstateenh(env, csrno, wr_mask,
> > > > > > new_val);
> > > > > >}
> > > > > >
> > > > > > @@ -1973,6 +1984,10 @@ static RISCVException
> > > > > > write_hstateen0(CPURISCVState *env, int csrno,
> > > > > >{
> > > > > >uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > > > SMSTATEEN0_HSENVCFG;
> > > > > >
> > > > > > +if (!riscv_has_ext(env, RVF)) {
> > > > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > > > +}
> > > > > > +
> > > > > >return write_hstateen(env, csrno, wr_mask, new_val);
> > > > > >}
> > > > > >
> > > > > > @@ -2024,6 +2039,10 @@ static RISCVException
> > > > > > write_hstateen0h(CPURISCVState *env, int csrno,
> > > > > >{
> > > > > >uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > > > SMSTATEEN0_HSENVCFG;
> > > > > >
> > > > > > +if (!riscv_has_ext(env, RVF)) {
> > > > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > > > +}
> > > > > > +
> > > > > >return write_hstateenh(env, csrno, wr_mask,
> > > > > > new_val);
> > > > > >}
> > > > > >
> > > > > > @@ -2083,6 +2102,10 @@ static RISCVException
> > > > > > write_sstateen0(CPURISCVState *env, int csrno,
> > > > > >{
> > > > > >uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > > > 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..c43c48336b 100644
> > > > > > --- a/target/riscv/insn_trans/trans_rvf.c.inc
> > > > > > +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> > > > > > @@ -24,9 +24,43 @@
> > > > > >return false; \
> > > > > >} while (0)
> > > > > >
> > > > > > +#ifndef CONFIG_USER_ONLY
> > > > > > +#define SMSTATEEN_CHECK(ctx) do {\
> > > > > > +CPUState *cpu = ctx->cs; \
> > > > > > +CPURISCVState *env = cpu->env_ptr; \
> > > > > > +if (ctx->cfg_ptr->ext_smstateen && \
> > > > > > +(env->priv < PRV_M)) { \
> > > > > > +uint64_t stateen = env->mstateen[0]; \
> > > > > > +uint64_t hstateen = env->hstateen[0]; \
> > > > > > +uint64_t sstateen = env->sstateen[0]; \
> > > > > > +if (!(stateen &

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-28 Thread Ben Dooks 

On 28/07/2022 07:15, Mayuresh Chitale wrote:

On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:


在 2022/7/24 下午11:49, Mayuresh Chitale 写道:

On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:

在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

If smstateen is implemented and sstateen0.fcsr is clear then
the
floating point operations must return illegal instruction
exception.

Signed-off-by: Mayuresh Chitale 
---
   target/riscv/csr.c| 23 ++
   target/riscv/insn_trans/trans_rvf.c.inc   | 38
+--
   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
   3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,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;
   }
@@ -1876,6 +1880,9 @@ static RISCVException
write_mstateen0(CPURISCVState *env, int csrno,
 target_ulong new_val)
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
   
   return write_mstateen(env, csrno, wr_mask, new_val);

   }
@@ -1924,6 +1931,10 @@ static RISCVException
write_mstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_mstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -1973,6 +1984,10 @@ static RISCVException

write_hstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateen(env, csrno, wr_mask, new_val);
   }
   
@@ -2024,6 +2039,10 @@ static RISCVException

write_hstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -2083,6 +2102,10 @@ static RISCVException

write_sstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
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..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
   return false; \
   } while (0)
   
+#ifndef CONFIG_USER_ONLY

+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS))
{\eventually
meaning
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \


given the size of that I would have thought an "static inline"
function would be easier to write and maintain for SMSTATEEN_CHECK


--
Ben Dooks   http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-28 Thread Weiwei Li 



在 2022/7/28 下午2:15, Mayuresh Chitale 写道:

On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:

在 2022/7/24 下午11:49, Mayuresh Chitale 写道:

On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:

在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

If smstateen is implemented and sstateen0.fcsr is clear then
the
floating point operations must return illegal instruction
exception.

Signed-off-by: Mayuresh Chitale 
---
   target/riscv/csr.c| 23 ++
   target/riscv/insn_trans/trans_rvf.c.inc   | 38
+--
   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
   3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,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;
   }
@@ -1876,6 +1880,9 @@ static RISCVException
write_mstateen0(CPURISCVState *env, int csrno,
 target_ulong new_val)
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
   
   return write_mstateen(env, csrno, wr_mask, new_val);

   }
@@ -1924,6 +1931,10 @@ static RISCVException
write_mstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_mstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -1973,6 +1984,10 @@ static RISCVException

write_hstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateen(env, csrno, wr_mask, new_val);
   }
   
@@ -2024,6 +2039,10 @@ static RISCVException

write_hstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -2083,6 +2102,10 @@ static RISCVException

write_sstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN |
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..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
   return false; \
   } while (0)
   
+#ifndef CONFIG_USER_ONLY

+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS))
{\eventually
meaning
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \
+} while (0)

It's better to add a space before '\'.

ok. will modify in the next version.

+#else
+#define SMSTATEEN_CHECK(ctx)
+#endif
+
   #define REQUIRE_ZFINX_OR_F(ctx) do {\
-if (!ctx->cfg_ptr->ext_zfinx) { \
-REQUIRE_EXT(ctx, RVF); \
+if (!has_ext(ctx, RVF)) { \
+SMSTATEEN_CHECK(ctx); \
+if (!ctx->cfg_ptr->ext_zfinx) { \
+return false; \
+} \
   } \
   } while (0)

SMSTATEEN_CHECK is for CSR. and REQUIRE_ZFINX_OR_F is for
Extension.
I think It's better to separate them. By the way, if we want the
smallest modification
for current code, adding it to REQUIRE_FPU seems better.

Actually REQUIRE_FPU is checking for mstatus.fs but as per
smstateen
spec we need to check for misa.f which is done in
REQUIRE_ZFINX_OR_F.

OK. It's acceptable to me  even though I prefer separating them.

However, I find another question in SMSTATEEN_CHECK: when access is
disallowed by Xstateen.FCSR,

it's always return false  which will trigger illegal

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-28 Thread Mayuresh Chitale 
On Mon, 2022-07-25 at 15:23 +0800, Weiwei Li wrote:
> 
> 在 2022/7/24 下午11:49, Mayuresh Chitale 写道:
> > On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:
> > > 在 2022/7/21 下午11:31, Mayuresh Chitale 写道:
> > > > If smstateen is implemented and sstateen0.fcsr is clear then
> > > > the
> > > > floating point operations must return illegal instruction
> > > > exception.
> > > > 
> > > > Signed-off-by: Mayuresh Chitale 
> > > > ---
> > > >   target/riscv/csr.c| 23 ++
> > > >   target/riscv/insn_trans/trans_rvf.c.inc   | 38
> > > > +--
> > > >   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
> > > >   3 files changed, 63 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > > > index ab06b117f9..a597b6cbc7 100644
> > > > --- a/target/riscv/csr.c
> > > > +++ b/target/riscv/csr.c
> > > > @@ -96,6 +96,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;
> > > >   }
> > > > @@ -1876,6 +1880,9 @@ static RISCVException
> > > > write_mstateen0(CPURISCVState *env, int csrno,
> > > > target_ulong new_val)
> > > >   {
> > > >   uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > SMSTATEEN0_HSENVCFG;
> > > > +if (!riscv_has_ext(env, RVF)) {
> > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > +}
> > > >   
> > > >   return write_mstateen(env, csrno, wr_mask, new_val);
> > > >   }
> > > > @@ -1924,6 +1931,10 @@ static RISCVException
> > > > write_mstateen0h(CPURISCVState *env, int csrno,
> > > >   {
> > > >   uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > SMSTATEEN0_HSENVCFG;
> > > >   
> > > > +if (!riscv_has_ext(env, RVF)) {
> > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > +}
> > > > +
> > > >   return write_mstateenh(env, csrno, wr_mask, new_val);
> > > >   }
> > > >   
> > > > @@ -1973,6 +1984,10 @@ static RISCVException
> > > > write_hstateen0(CPURISCVState *env, int csrno,
> > > >   {
> > > >   uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > SMSTATEEN0_HSENVCFG;
> > > >   
> > > > +if (!riscv_has_ext(env, RVF)) {
> > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > +}
> > > > +
> > > >   return write_hstateen(env, csrno, wr_mask, new_val);
> > > >   }
> > > >   
> > > > @@ -2024,6 +2039,10 @@ static RISCVException
> > > > write_hstateen0h(CPURISCVState *env, int csrno,
> > > >   {
> > > >   uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > SMSTATEEN0_HSENVCFG;
> > > >   
> > > > +if (!riscv_has_ext(env, RVF)) {
> > > > +wr_mask |= SMSTATEEN0_FCSR;
> > > > +}
> > > > +
> > > >   return write_hstateenh(env, csrno, wr_mask, new_val);
> > > >   }
> > > >   
> > > > @@ -2083,6 +2102,10 @@ static RISCVException
> > > > write_sstateen0(CPURISCVState *env, int csrno,
> > > >   {
> > > >   uint64_t wr_mask = SMSTATEEN_STATEN |
> > > > 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..c43c48336b 100644
> > > > --- a/target/riscv/insn_trans/trans_rvf.c.inc
> > > > +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> > > > @@ -24,9 +24,43 @@
> > > >   return false; \
> > > >   } while (0)
> > > >   
> > > > +#ifndef CONFIG_USER_ONLY
> > > > +#define SMSTATEEN_CHECK(ctx) do {\
> > > > +CPUState *cpu = ctx->cs; \
> > > > +CPURISCVState *env = cpu->env_ptr; \
> > > > +if (ctx->cfg_ptr->ext_smstateen && \
> > > > +(env->priv < PRV_M)) { \
> > > > +uint64_t stateen = env->mstateen[0]; \
> > > > +uint64_t hstateen = env->hstateen[0]; \
> > > > +uint64_t sstateen = env->sstateen[0]; \
> > > > +if (!(stateen & SMSTATEEN_STATEN)) {\
> > > > +hstateen = 0; \
> > > > +sstateen = 0; \
> > > > +} \
> > > > +if (ctx->virt_enabled) { \
> > > > +stateen &= hstateen; \
> > > > +if (!(hstateen & SMSTATEEN_STATEN)) {\
> > > > +sstateen = 0; \
> > > > +} \
> > > > +} \
> > > > +if (env->priv == PRV_U && has_ext(ctx, RVS))
> > > > {\eventually
> > > > meaning
> > > > +stateen &= sstateen; \
> > > > +} \
> > > > +if (!(stateen & SMSTATEEN0_FCSR)) { \
> > > > +return false; \
> > > > +} \
> > > > +} \
> > > > +} while (0)
> > > 
> > > It's better to

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-25 Thread Weiwei Li 


在 2022/7/24 下午11:49, Mayuresh Chitale 写道:

On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:

在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

If smstateen is implemented and sstateen0.fcsr is clear then the
floating point operations must return illegal instruction
exception.

Signed-off-by: Mayuresh Chitale 
---
   target/riscv/csr.c| 23 ++
   target/riscv/insn_trans/trans_rvf.c.inc   | 38
+--
   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
   3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,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;
   }
@@ -1876,6 +1880,9 @@ static RISCVException
write_mstateen0(CPURISCVState *env, int csrno,
 target_ulong new_val)
   {
   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
   
   return write_mstateen(env, csrno, wr_mask, new_val);

   }
@@ -1924,6 +1931,10 @@ static RISCVException
write_mstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_mstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -1973,6 +1984,10 @@ static RISCVException

write_hstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateen(env, csrno, wr_mask, new_val);
   }
   
@@ -2024,6 +2039,10 @@ static RISCVException

write_hstateen0h(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
   
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
   return write_hstateenh(env, csrno, wr_mask, new_val);
   }
   
@@ -2083,6 +2102,10 @@ static RISCVException

write_sstateen0(CPURISCVState *env, int csrno,
   {
   uint64_t wr_mask = SMSTATEEN_STATEN | 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..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
   return false; \
   } while (0)
   
+#ifndef CONFIG_USER_ONLY

+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS)) {\eventually
meaning
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \
+} while (0)

It's better to add a space before '\'.

ok. will modify in the next version.

+#else
+#define SMSTATEEN_CHECK(ctx)
+#endif
+
   #define REQUIRE_ZFINX_OR_F(ctx) do {\
-if (!ctx->cfg_ptr->ext_zfinx) { \
-REQUIRE_EXT(ctx, RVF); \
+if (!has_ext(ctx, RVF)) { \
+SMSTATEEN_CHECK(ctx); \
+if (!ctx->cfg_ptr->ext_zfinx) { \
+return false; \
+} \
   } \
   } while (0)

SMSTATEEN_CHECK is for CSR. and REQUIRE_ZFINX_OR_F is for Extension.
I think It's better to separate them. By the way, if we want the
smallest modification
for current code, adding it to REQUIRE_FPU seems better.

Actually REQUIRE_FPU is checking for mstatus.fs but as per smstateen
spec we need to check for misa.f which is done in REQUIRE_ZFINX_OR_F.


OK. It's acceptable to me  even though I prefer separating them.

However, I find another question in SMSTATEEN_CHECK: when access is 
disallowed by Xstateen.FCSR,


it's always return false  which will trigger illegal instruction 
exception finally.


However, this exception is triggered by accessing fcsr CSR which

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-24 Thread Mayuresh Chitale 
On Fri, 2022-07-22 at 09:42 +0800, Weiwei Li wrote:
> 在 2022/7/21 下午11:31, Mayuresh Chitale 写道:
> > If smstateen is implemented and sstateen0.fcsr is clear then the
> > floating point operations must return illegal instruction
> > exception.
> > 
> > Signed-off-by: Mayuresh Chitale 
> > ---
> >   target/riscv/csr.c| 23 ++
> >   target/riscv/insn_trans/trans_rvf.c.inc   | 38
> > +--
> >   target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
> >   3 files changed, 63 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index ab06b117f9..a597b6cbc7 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -96,6 +96,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;
> >   }
> > @@ -1876,6 +1880,9 @@ static RISCVException
> > write_mstateen0(CPURISCVState *env, int csrno,
> > target_ulong new_val)
> >   {
> >   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> > +if (!riscv_has_ext(env, RVF)) {
> > +wr_mask |= SMSTATEEN0_FCSR;
> > +}
> >   
> >   return write_mstateen(env, csrno, wr_mask, new_val);
> >   }
> > @@ -1924,6 +1931,10 @@ static RISCVException
> > write_mstateen0h(CPURISCVState *env, int csrno,
> >   {
> >   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >   
> > +if (!riscv_has_ext(env, RVF)) {
> > +wr_mask |= SMSTATEEN0_FCSR;
> > +}
> > +
> >   return write_mstateenh(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -1973,6 +1984,10 @@ static RISCVException
> > write_hstateen0(CPURISCVState *env, int csrno,
> >   {
> >   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >   
> > +if (!riscv_has_ext(env, RVF)) {
> > +wr_mask |= SMSTATEEN0_FCSR;
> > +}
> > +
> >   return write_hstateen(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -2024,6 +2039,10 @@ static RISCVException
> > write_hstateen0h(CPURISCVState *env, int csrno,
> >   {
> >   uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
> >   
> > +if (!riscv_has_ext(env, RVF)) {
> > +wr_mask |= SMSTATEEN0_FCSR;
> > +}
> > +
> >   return write_hstateenh(env, csrno, wr_mask, new_val);
> >   }
> >   
> > @@ -2083,6 +2102,10 @@ static RISCVException
> > write_sstateen0(CPURISCVState *env, int csrno,
> >   {
> >   uint64_t wr_mask = SMSTATEEN_STATEN | 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..c43c48336b 100644
> > --- a/target/riscv/insn_trans/trans_rvf.c.inc
> > +++ b/target/riscv/insn_trans/trans_rvf.c.inc
> > @@ -24,9 +24,43 @@
> >   return false; \
> >   } while (0)
> >   
> > +#ifndef CONFIG_USER_ONLY
> > +#define SMSTATEEN_CHECK(ctx) do {\
> > +CPUState *cpu = ctx->cs; \
> > +CPURISCVState *env = cpu->env_ptr; \
> > +if (ctx->cfg_ptr->ext_smstateen && \
> > +(env->priv < PRV_M)) { \
> > +uint64_t stateen = env->mstateen[0]; \
> > +uint64_t hstateen = env->hstateen[0]; \
> > +uint64_t sstateen = env->sstateen[0]; \
> > +if (!(stateen & SMSTATEEN_STATEN)) {\
> > +hstateen = 0; \
> > +sstateen = 0; \
> > +} \
> > +if (ctx->virt_enabled) { \
> > +stateen &= hstateen; \
> > +if (!(hstateen & SMSTATEEN_STATEN)) {\
> > +sstateen = 0; \
> > +} \
> > +} \
> > +if (env->priv == PRV_U && has_ext(ctx, RVS)) {\eventually
> > meaning
> > +stateen &= sstateen; \
> > +} \
> > +if (!(stateen & SMSTATEEN0_FCSR)) { \
> > +return false; \
> > +} \
> > +} \
> > +} while (0)
> 
> It's better to add a space before '\'.
ok. will modify in the next version.
> 
> > +#else
> > +#define SMSTATEEN_CHECK(ctx)
> > +#endif
> > +
> >   #define REQUIRE_ZFINX_OR_F(ctx) do {\
> > -if (!ctx->cfg_ptr->ext_zfinx) { \
> > -REQUIRE_EXT(ctx, RVF); \
> > +if (!has_ext(ctx, RVF)) { \
> > +SMSTATEEN_CHECK(ctx); \
> > +if (!ctx->cfg_ptr->ext_zfinx) { \
> > +return false; \
> > +} \
> >   } \
> >   } while (0)
> 
> SMSTATEEN_CHECK is for CSR. and REQUIRE_ZFINX_OR_F is for Extension.
> I think It's better to separate them. By the way, if we want the
> smallest modification
> for current code, adding it to REQUIRE_FPU seems

Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-21 Thread Weiwei Li 



在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

If smstateen is implemented and sstateen0.fcsr is clear then the
floating point operations must return illegal instruction exception.

Signed-off-by: Mayuresh Chitale 
---
  target/riscv/csr.c| 23 ++
  target/riscv/insn_trans/trans_rvf.c.inc   | 38 +--
  target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
  3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,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;
  }
@@ -1876,6 +1880,9 @@ static RISCVException write_mstateen0(CPURISCVState *env, 
int csrno,
target_ulong new_val)
  {
  uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
  
  return write_mstateen(env, csrno, wr_mask, new_val);

  }
@@ -1924,6 +1931,10 @@ static RISCVException write_mstateen0h(CPURISCVState 
*env, int csrno,
  {
  uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
  
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
  return write_mstateenh(env, csrno, wr_mask, new_val);
  }
  
@@ -1973,6 +1984,10 @@ static RISCVException write_hstateen0(CPURISCVState *env, int csrno,

  {
  uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
  
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
  return write_hstateen(env, csrno, wr_mask, new_val);
  }
  
@@ -2024,6 +2039,10 @@ static RISCVException write_hstateen0h(CPURISCVState *env, int csrno,

  {
  uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
  
+if (!riscv_has_ext(env, RVF)) {

+wr_mask |= SMSTATEEN0_FCSR;
+}
+
  return write_hstateenh(env, csrno, wr_mask, new_val);
  }
  
@@ -2083,6 +2102,10 @@ static RISCVException write_sstateen0(CPURISCVState *env, int csrno,

  {
  uint64_t wr_mask = SMSTATEEN_STATEN | 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..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
  return false; \
  } while (0)
  
+#ifndef CONFIG_USER_ONLY

+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS)) {\
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \
+} while (0)


It's better to add a space before '\'.


+#else
+#define SMSTATEEN_CHECK(ctx)
+#endif
+
  #define REQUIRE_ZFINX_OR_F(ctx) do {\
-if (!ctx->cfg_ptr->ext_zfinx) { \
-REQUIRE_EXT(ctx, RVF); \
+if (!has_ext(ctx, RVF)) { \
+SMSTATEEN_CHECK(ctx); \
+if (!ctx->cfg_ptr->ext_zfinx) { \
+return false; \
+} \
  } \
  } while (0)


SMSTATEEN_CHECK is for CSR. and REQUIRE_ZFINX_OR_F is for Extension.
I think It's better to separate them. By the way, if we want the smallest 
modification
for current code, adding it to REQUIRE_FPU seems better.
Regards,
Weiwei Li

  
diff --git a/target/riscv/insn_trans/trans_rvzfh.c.inc b/target/riscv/insn_trans/trans_rvzfh.c.inc

index 5d07150cd0..b165ea9d58 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -17,24 +17,28 @@
   */
  
  #define REQUIRE_ZFH(ctx) do { \

+SMSTATEEN_CHECK(ctx); \
  if (!ctx->cfg_ptr->ext_zfh) {  \
  return false; \
  } \
  } while (0)
  
  #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \

+SMSTATEEN_CHECK(ctx); \
  if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
  return false;

[PATCH v6 3/5] target/riscv: smstateen check for fcsr

2022-07-21 Thread Mayuresh Chitale 
If smstateen is implemented and sstateen0.fcsr is clear then the
floating point operations must return illegal instruction exception.

Signed-off-by: Mayuresh Chitale 
---
 target/riscv/csr.c| 23 ++
 target/riscv/insn_trans/trans_rvf.c.inc   | 38 +--
 target/riscv/insn_trans/trans_rvzfh.c.inc |  4 +++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ab06b117f9..a597b6cbc7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -96,6 +96,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;
 }
@@ -1876,6 +1880,9 @@ static RISCVException write_mstateen0(CPURISCVState *env, 
int csrno,
   target_ulong new_val)
 {
 uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
 
 return write_mstateen(env, csrno, wr_mask, new_val);
 }
@@ -1924,6 +1931,10 @@ static RISCVException write_mstateen0h(CPURISCVState 
*env, int csrno,
 {
 uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
 
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
 return write_mstateenh(env, csrno, wr_mask, new_val);
 }
 
@@ -1973,6 +1984,10 @@ static RISCVException write_hstateen0(CPURISCVState 
*env, int csrno,
 {
 uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
 
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
 return write_hstateen(env, csrno, wr_mask, new_val);
 }
 
@@ -2024,6 +2039,10 @@ static RISCVException write_hstateen0h(CPURISCVState 
*env, int csrno,
 {
 uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;
 
+if (!riscv_has_ext(env, RVF)) {
+wr_mask |= SMSTATEEN0_FCSR;
+}
+
 return write_hstateenh(env, csrno, wr_mask, new_val);
 }
 
@@ -2083,6 +2102,10 @@ static RISCVException write_sstateen0(CPURISCVState 
*env, int csrno,
 {
 uint64_t wr_mask = SMSTATEEN_STATEN | 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..c43c48336b 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -24,9 +24,43 @@
 return false; \
 } while (0)
 
+#ifndef CONFIG_USER_ONLY
+#define SMSTATEEN_CHECK(ctx) do {\
+CPUState *cpu = ctx->cs; \
+CPURISCVState *env = cpu->env_ptr; \
+if (ctx->cfg_ptr->ext_smstateen && \
+(env->priv < PRV_M)) { \
+uint64_t stateen = env->mstateen[0]; \
+uint64_t hstateen = env->hstateen[0]; \
+uint64_t sstateen = env->sstateen[0]; \
+if (!(stateen & SMSTATEEN_STATEN)) {\
+hstateen = 0; \
+sstateen = 0; \
+} \
+if (ctx->virt_enabled) { \
+stateen &= hstateen; \
+if (!(hstateen & SMSTATEEN_STATEN)) {\
+sstateen = 0; \
+} \
+} \
+if (env->priv == PRV_U && has_ext(ctx, RVS)) {\
+stateen &= sstateen; \
+} \
+if (!(stateen & SMSTATEEN0_FCSR)) { \
+return false; \
+} \
+} \
+} while (0)
+#else
+#define SMSTATEEN_CHECK(ctx)
+#endif
+
 #define REQUIRE_ZFINX_OR_F(ctx) do {\
-if (!ctx->cfg_ptr->ext_zfinx) { \
-REQUIRE_EXT(ctx, RVF); \
+if (!has_ext(ctx, RVF)) { \
+SMSTATEEN_CHECK(ctx); \
+if (!ctx->cfg_ptr->ext_zfinx) { \
+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..b165ea9d58 100644
--- a/target/riscv/insn_trans/trans_rvzfh.c.inc
+++ b/target/riscv/insn_trans/trans_rvzfh.c.inc
@@ -17,24 +17,28 @@
  */
 
 #define REQUIRE_ZFH(ctx) do { \
+SMSTATEEN_CHECK(ctx); \
 if (!ctx->cfg_ptr->ext_zfh) {  \
 return false; \
 } \
 } while (0)
 
 #define REQUIRE_ZHINX_OR_ZFH(ctx) do { \
+SMSTATEEN_CHECK(ctx); \
 if (!ctx->cfg_ptr->ext_zhinx && !ctx->cfg_ptr->ext_zfh) { \
 return false;  \
 }  \
 } while (0)
 
 #define REQUIRE_ZFH_OR_ZFHMIN(ctx) do {   \
+SMSTATEEN_CHECK(ctx); \
 if (!(ctx->cfg_ptr->ext_zfh || ctx->cfg_ptr->ext_zfhmin)) { \
 return false; \
 } \
 } while (0)
 
 #define REQUIRE_ZFH_OR_ZFHMIN_OR_ZHINX_OR_ZHINXMIN(ctx) do { \
+