[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v10 4/5] target/riscv: smstateen check for fcsr
|
From: |
mchitale |
|
Subject: |
Re: [PATCH v10 4/5] target/riscv: smstateen check for fcsr |
|
Date: |
Tue, 04 Oct 2022 12:21:51 +0530 |
|
User-agent: |
Evolution 3.36.5-0ubuntu1 |
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)
[PATCH v10 5/5] target/riscv: smstateen knobs, Mayuresh Chitale, 2022/10/03