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