On Tue, 2022-10-04 at 21:23 +0800, weiwei wrote:
>     
> 
>     
> 
>     On 2022/10/4 14:51,
>       mchit...@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 <mchit...@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)
> > > >         
> > > 
> > >       
> > 
> >     
> 
>   
> 

Reply via email to