At 2022-06-04 00:04:23, "Mayuresh Chitale" <mchitale@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 <mchitale@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.ccounteren
>@@ -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;
>+ }
>+I think mstateen only control access in priv mode under M mode.
So we should take the current priv node into consideration here.
>+ 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;
>+ }
I think there are two problem here:
The first is that we should also take virt mode into consideration here too. As the spec said:
"if executing in VS or VU mode and the circumstances for a virtual instruction
exception apply, raises a virtual instruction exception instead of an illegal instruction exception"
So it will generate RISCV_EXCP_VIRT_INSTRUCTION_FAULT in VU mode.
The second is that sstateen csr works only when 'S' extension is enabled
Regards,
Weiwei Li
>+ }
>+#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);
>+ 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
>