On 2023/2/14 22:27, Bin Meng wrote:
At present the envcfg CSRs predicate() routines are generic one like
smode(), hmode. The configuration check is done in the read / write
routine. Create a new predicate routine to cover such check, so that
gdbstub can correctly report its existence.

Signed-off-by: Bin Meng <bm...@tinylab.org>

---

  target/riscv/csr.c | 98 +++++++++++++++++++++++++++++-----------------
  1 file changed, 61 insertions(+), 37 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 37350b8a6d..284ccc09dd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -41,40 +41,6 @@ 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);
-    RISCVCPU *cpu = env_archcpu(env);
-
-    if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
-        return RISCV_EXCP_NONE;
-    }
-
-    if (!(env->mstateen[index] & bit)) {
-        return RISCV_EXCP_ILLEGAL_INST;
-    }
-
-    if (virt) {
-        if (!(env->hstateen[index] & bit)) {
-            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-        }
-
-        if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
-            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-        }
-    }
-
-    if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
-        if (!(env->sstateen[index] & bit)) {
-            return RISCV_EXCP_ILLEGAL_INST;
-        }
-    }
-
-    return RISCV_EXCP_NONE;
-}
-#endif
static RISCVException fs(CPURISCVState *env, int csrno)
  {
@@ -318,6 +284,32 @@ static RISCVException umode32(CPURISCVState *env, int 
csrno)
      return umode(env, csrno);
  }
+static RISCVException envcfg(CPURISCVState *env, int csrno)
+{
+    RISCVCPU *cpu = env_archcpu(env);
+    riscv_csr_predicate_fn predicate;
+
+    if (cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }

This check seems not right here.  Why  ILLEGAL_INST is directly triggered if smstateen is enabled?

It seems that smstateen related check will be done  for senvcfg/henvcfg{h} when smstateen is enabled.

Regards,

Weiwei Li

+
+    switch (csrno) {
+    case CSR_SENVCFG:
+        predicate = smode;
+        break;
+    case CSR_HENVCFG:
+        predicate = hmode;
+        break;
+    case CSR_HENVCFGH:
+        predicate = hmode32;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return predicate(env, csrno);
+}
+
  static RISCVException mstateen(CPURISCVState *env, int csrno)
  {
      RISCVCPU *cpu = env_archcpu(env);
@@ -1946,6 +1938,38 @@ static RISCVException write_menvcfgh(CPURISCVState *env, 
int csrno,
      return RISCV_EXCP_NONE;
  }
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
+                                       uint64_t bit)
+{
+    bool virt = riscv_cpu_virt_enabled(env);
+
+    if (env->priv == PRV_M) {
+        return RISCV_EXCP_NONE;
+    }
+
+    if (!(env->mstateen[index] & bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    if (virt) {
+        if (!(env->hstateen[index] & bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+
+        if (env->priv == PRV_U && !(env->sstateen[index] & bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+    }
+
+    if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
+        if (!(env->sstateen[index] & bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+
+    return RISCV_EXCP_NONE;
+}
+
  static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
                                     target_ulong *val)
  {
@@ -4087,11 +4111,11 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
                         .min_priv_ver = PRIV_VERSION_1_12_0              },
      [CSR_MENVCFGH] = { "menvcfgh", umode32, read_menvcfgh, write_menvcfgh,
                         .min_priv_ver = PRIV_VERSION_1_12_0              },
-    [CSR_SENVCFG]  = { "senvcfg",  smode, read_senvcfg,  write_senvcfg,
+    [CSR_SENVCFG]  = { "senvcfg",  envcfg, read_senvcfg,  write_senvcfg,
                         .min_priv_ver = PRIV_VERSION_1_12_0              },
-    [CSR_HENVCFG]  = { "henvcfg",  hmode, read_henvcfg, write_henvcfg,
+    [CSR_HENVCFG]  = { "henvcfg",  envcfg, read_henvcfg,  write_henvcfg,
                         .min_priv_ver = PRIV_VERSION_1_12_0              },
-    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh,
+    [CSR_HENVCFGH] = { "henvcfgh", envcfg, read_henvcfgh, write_henvcfgh,
                         .min_priv_ver = PRIV_VERSION_1_12_0              },
/* Smstateen extension CSRs */


Reply via email to