On Thu, Jun 16, 2022 at 4:54 PM Alistair Francis <alistai...@gmail.com> wrote:
>
> On Sat, Jun 4, 2022 at 2:16 AM Mayuresh Chitale
> <mchit...@ventanamicro.com> wrote:
> >
> > 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 | 84 ++++++++++++++++++++++++++++++++++++++++++----
> > 1 file changed, 78 insertions(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 324fefce59..ae91ae1f7e 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations
> > *ops)
> > }
> >
> > /* Predicates */
> > +static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int
> > bit)
> > +{
> > + CPUState *cs = env_cpu(env);
> > + RISCVCPU *cpu = RISCV_CPU(cs);
> > + bool virt = riscv_cpu_virt_enabled(env);
> > +
> > + if (!cpu->cfg.ext_smstateen) {
> > + return RISCV_EXCP_NONE;
> > + }
> > +
> > +#if !defined(CONFIG_USER_ONLY)
> > + if (!(env->mstateen[0] & 1UL << bit)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + if (virt) {
> > + if (!(env->hstateen[0] & 1UL << bit)) {
> > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> > + }
> > + }
> > +
> > + if (mode == PRV_U) {
> > + if (!(env->sstateen[0] & 1UL << bit)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > + }
> > +#endif
> > +
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > static RISCVException fs(CPURISCVState *env, int csrno)
> > {
> > #if !defined(CONFIG_USER_ONLY)
> > @@ -1557,6 +1588,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, PRV_S, SMSTATEEN0_HSENVCFG);
>
> Couldn't this be part of the original permission check so we don't
> need a second check?
Whoops, misread the function. Ignore that
Alistair
>
> Alistair
>
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->senvcfg;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1565,15 +1603,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, PRV_S, 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, PRV_S, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->henvcfg;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1582,6 +1632,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, PRV_S, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> >
> > if (riscv_cpu_mxl(env) == MXL_RV64) {
> > mask |= HENVCFG_PBMTE | HENVCFG_STCE;
> > @@ -1595,6 +1651,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, PRV_S, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> > +
> > *val = env->henvcfg >> 32;
> > return RISCV_EXCP_NONE;
> > }
> > @@ -1604,9 +1667,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, PRV_S, SMSTATEEN0_HSENVCFG);
> > + if (ret != RISCV_EXCP_NONE) {
> > + return ret;
> > + }
> >
> > + env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> > return RISCV_EXCP_NONE;
> > }
> >
> > @@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState
> > *env, int csrno,
> > target_ulong new_val)
> > {
> > uint64_t *reg;
> > - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> > + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> > + (1UL << SMSTATEEN0_HSENVCFG);
> >
> > reg = &env->mstateen[csrno - CSR_MSTATEEN0];
> > write_smstateen(env, reg, wr_mask, new_val);
> > @@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState
> > *env, int csrno,
> > {
> > uint64_t *reg;
> > uint64_t val;
> > - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> > + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> > + (1UL << SMSTATEEN0_HSENVCFG);
> >
> > reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
> > val = (uint64_t)new_val << 32;
> > @@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState
> > *env, int csrno,
> > target_ulong new_val)
> > {
> > uint64_t *reg;
> > - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> > + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> > + (1UL << SMSTATEEN0_HSENVCFG);
> > int index = csrno - CSR_HSTATEEN0;
> >
> > reg = &env->hstateen[index];
> > @@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState
> > *env, int csrno,
> > {
> > uint64_t *reg;
> > uint64_t val;
> > - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> > int index = csrno - CSR_HSTATEEN0H;
> > + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> > + (1UL << SMSTATEEN0_HSENVCFG);
> >
> > reg = &env->hstateen[index];
> > val = (uint64_t)new_val << 32;
> > --
> > 2.25.1
> >
> >
