Re: [PATCH 2/7] target/riscv: Add new CSR fields for S{sn,mn,m}pm extensions as part of Zjpm v1.0

[Richard Henderson]

On 11/7/24 22:01, baturo.ale...@gmail.com wrote:

@@ -2424,6 +2433,12 @@ static RISCVException write_senvcfg(CPURISCVState *env, 
int csrno,
                                      target_ulong val)
  {
      uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | 
SENVCFG_CBZE;
+    /* Update PMM field only if the value is valid according to Zjpm v1.0 */
+    if (env_archcpu(env)->cfg.ext_ssnpm &&
+        riscv_cpu_mxl(env) == MXL_RV64 &&
+        (get_field(val, SENVCFG_PMM) != PMM_FIELD_RESERVED)) {
+        mask |= SENVCFG_PMM;
+    }
      RISCVException ret;

ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);

Do not insert statements into the declarations at the start of the block.
Drop the unnecessary () around != (multiple instances).

@@ -3525,6 +3546,15 @@ static RISCVException read_hstatus(CPURISCVState *env, 
int csrno,
  static RISCVException write_hstatus(CPURISCVState *env, int csrno,
                                      target_ulong val)
  {
+    uint64_t mask = (target_ulong)-1;
+    /* Update PMM field only if the value is valid according to Zjpm v1.0 */
+    if (env_archcpu(env)->cfg.ext_ssnpm &&
+        (riscv_cpu_mxl(env) == MXL_RV64) &&
+        (get_field(val, HSTATUS_HUPMM) == PMM_FIELD_RESERVED)) {
+        mask &= ~HSTATUS_HUPMM;
+    }

Surely you'd not install HUPMM if ext_ssnpm is false?
Is this missing other extension checks?

+    env->hstatus = (env->hstatus & ~mask) | (val & mask);
+
      env->hstatus = val;

Failure to remove previous assignment.


r~