On Fri, 2022-07-22 at 08:45 +0800, Weiwei Li wrote: > 在 2022/7/21 下午11:31, Mayuresh Chitale 写道: > > Accesses to henvcfg, henvcfgh and senvcfg are allowed only if > > corresponding bit in mstateen0/hstateen0 is enabled. Otherwise an > > illegal instruction trap is generated. > > > > Signed-off-by: Mayuresh Chitale <mchit...@ventanamicro.com> > > --- > > target/riscv/csr.c | 100 > > +++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 93 insertions(+), 7 deletions(-) > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 27032a416c..ab06b117f9 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -40,6 +40,55 @@ void riscv_set_csr_ops(int csrno, > > riscv_csr_operations *ops) > > } > > > > /* Predicates */ > > +#if !defined(CONFIG_USER_ONLY) > > +static RISCVException smstateen_acc_ok(CPURISCVState *env, int > > index, > > + uint64_t bit) > > +{ > > + bool virt = riscv_cpu_virt_enabled(env); > > + CPUState *cs = env_cpu(env); > > + RISCVCPU *cpu = RISCV_CPU(cs); > > + uint64_t hstateen = env->hstateen[index]; > > + uint64_t sstateen = env->sstateen[index]; > > + > > + if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) { > > + return RISCV_EXCP_NONE; > > + } > > + > > + if (!(env->mstateen[index] & bit)) { > > + return RISCV_EXCP_ILLEGAL_INST; > > + } > > + > > + /* > > + * Treat hstateen and sstateen as read-only zero if > > mstateen0.staten > > + * is clear. > > + */ > > + if (!(env->mstateen[index] & SMSTATEEN_STATEN)) { > > + hstateen = 0; > > + sstateen = 0; > > + } > > + > > + if (virt) { > > + if (!(hstateen & bit)) { > > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > + } > > + /* > > + * Treat sstateen as read-only zero if hstateen0.staten is > > clear. > > + */ > > + if (!(hstateen & SMSTATEEN_STATEN)) { > > + sstateen = 0; > > + } > > + } > > + > > + if (env->priv == PRV_U && riscv_has_ext(env, RVS)) { > > + if (!(sstateen & bit)) { > > + return RISCV_EXCP_ILLEGAL_INST; > > + } > > + } > > + > > + return RISCV_EXCP_NONE; > > +} > > +#endif > > + > > static RISCVException fs(CPURISCVState *env, int csrno) > > { > > #if !defined(CONFIG_USER_ONLY) > > @@ -1708,6 +1757,13 @@ static RISCVException > > write_menvcfgh(CPURISCVState *env, int csrno, > > static RISCVException read_senvcfg(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > > + RISCVException ret; > > + > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > I think it's better to add this check into the predicate. > > By the way, sharing the same function for all related csrs is > easily > misunderstood. However, It seems correct.
We use the default global predicates ie hmode/smode etc for the envcfg registers and the global predicates cant be modified to include additional checks for envcfg registers. We could implement new predicates for envcfg but I think the current approach is simpler. > > Regards, > > Weiwei Li > > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > + > > *val = env->senvcfg; > > return RISCV_EXCP_NONE; > > } > > @@ -1716,15 +1772,27 @@ static RISCVException > > write_senvcfg(CPURISCVState *env, int csrno, > > target_ulong val) > > { > > uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | > > SENVCFG_CBZE; > > + RISCVException ret; > > > > - env->senvcfg = (env->senvcfg & ~mask) | (val & mask); > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > > > + env->senvcfg = (env->senvcfg & ~mask) | (val & mask); > > return RISCV_EXCP_NONE; > > } > > > > static RISCVException read_henvcfg(CPURISCVState *env, int csrno, > > target_ulong *val) > > { > > + RISCVException ret; > > + > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > + > > *val = env->henvcfg; > > return RISCV_EXCP_NONE; > > } > > @@ -1733,6 +1801,12 @@ static RISCVException > > write_henvcfg(CPURISCVState *env, int csrno, > > target_ulong val) > > { > > uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE | > > HENVCFG_CBZE; > > + RISCVException ret; > > + > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > > > if (riscv_cpu_mxl(env) == MXL_RV64) { > > mask |= HENVCFG_PBMTE | HENVCFG_STCE; > > @@ -1746,6 +1820,13 @@ static RISCVException > > write_henvcfg(CPURISCVState *env, int csrno, > > static RISCVException read_henvcfgh(CPURISCVState *env, int > > csrno, > > target_ulong *val) > > { > > + RISCVException ret; > > + > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > + > > *val = env->henvcfg >> 32; > > return RISCV_EXCP_NONE; > > } > > @@ -1755,9 +1836,14 @@ static RISCVException > > write_henvcfgh(CPURISCVState *env, int csrno, > > { > > uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE; > > uint64_t valh = (uint64_t)val << 32; > > + RISCVException ret; > > > > - env->henvcfg = (env->henvcfg & ~mask) | (valh & mask); > > + ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG); > > + if (ret != RISCV_EXCP_NONE) { > > + return ret; > > + } > > > > + env->henvcfg = (env->henvcfg & ~mask) | (valh & mask); > > return RISCV_EXCP_NONE; > > } > > > > @@ -1789,7 +1875,7 @@ static RISCVException > > write_mstateen(CPURISCVState *env, int csrno, > > static RISCVException write_mstateen0(CPURISCVState *env, int > > csrno, > > target_ulong new_val) > > { > > - uint64_t wr_mask = SMSTATEEN_STATEN; > > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG; > > > > return write_mstateen(env, csrno, wr_mask, new_val); > > } > > @@ -1836,7 +1922,7 @@ static RISCVException > > write_mstateenh(CPURISCVState *env, int csrno, > > static RISCVException write_mstateen0h(CPURISCVState *env, int > > csrno, > > target_ulong new_val) > > { > > - uint64_t wr_mask = SMSTATEEN_STATEN; > > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG; > > > > return write_mstateenh(env, csrno, wr_mask, new_val); > > } > > @@ -1885,7 +1971,7 @@ static RISCVException > > write_hstateen(CPURISCVState *env, int csrno, > > static RISCVException write_hstateen0(CPURISCVState *env, int > > csrno, > > target_ulong new_val) > > { > > - uint64_t wr_mask = SMSTATEEN_STATEN; > > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG; > > > > return write_hstateen(env, csrno, wr_mask, new_val); > > } > > @@ -1936,7 +2022,7 @@ static RISCVException > > write_hstateenh(CPURISCVState *env, int csrno, > > static RISCVException write_hstateen0h(CPURISCVState *env, int > > csrno, > > target_ulong new_val) > > { > > - uint64_t wr_mask = SMSTATEEN_STATEN; > > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG; > > > > return write_hstateenh(env, csrno, wr_mask, new_val); > > } > > @@ -1995,7 +2081,7 @@ static RISCVException > > write_sstateen(CPURISCVState *env, int csrno, > > static RISCVException write_sstateen0(CPURISCVState *env, int > > csrno, > > target_ulong new_val) > > { > > - uint64_t wr_mask = SMSTATEEN_STATEN; > > + uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG; > > > > return write_sstateen(env, csrno, wr_mask, new_val); > > }