Skip to contentSkip to conversation

target/riscv: change mstatus bits for read-only if S ou U unsupported

PATCH In review Christian S. Lima 2 participants 2 messages Nov 10 – Nov 11 RISC-V

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 target/riscv: change mstatus bits for read-only if S ou U unsupported

The risc v privileged architecture says that mstatus.SXL, UXL, TSR, TVM and MPVR should be read-only when S-mode or U-mode are not present.

changes in v2:
- Don't change bits in target/riscv/cpu_bits.h
- Add checks to S-Mode in the mstatus.SXL and U-mode mstatus.UXL in the target/riscv/cpu.c file that were suggested by Daniel in the review [1]

Link to v1: https://lore.kernel.org/qemu-riscv/[email protected]/

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4456

[1] https://lore.kernel.org/qemu-riscv/[email protected]/T/#mddbb34a3ea837e3c707131a2e354c63c989ab029

---
 target/riscv/cpu.c     | 12 ++++++++++--
 target/riscv/tcg/csr.c | 17 ++++++++++++-----
 2 files changed, 22 insertions(+), 7 deletions(-)
target/riscv/cpu.c +10 -2
index 15c27f4ce4..50bdfbe025 100644
@@ -997,8 +997,16 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
          * The reset status of SXL/UXL is undefined, but mstatus is WARL
          * and we must ensure that the value after init is valid for read.
          */
-        env->mstatus = set_field(env->mstatus, MSTATUS64_SXL, env->misa_mxl);
-        env->mstatus = set_field(env->mstatus, MSTATUS64_UXL, env->misa_mxl);
+        if (riscv_has_ext(env, RVS)) {
+            env->mstatus = set_field(env->mstatus,
+                                     MSTATUS64_SXL, env->misa_mxl);
+        }
+
+        if (riscv_has_ext(env, RVU)) {
+            env->mstatus = set_field(env->mstatus,
+                                     MSTATUS64_UXL, env->misa_mxl);
+        }
+
         if (riscv_has_ext(env, RVH)) {
             env->vsstatus = set_field(env->vsstatus,
                                       MSTATUS64_SXL, env->misa_mxl);
target/riscv/tcg/csr.c +12 -5
index af444e3e6f..24eda54bac 100644
@@ -2081,18 +2081,25 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
         tlb_flush(env_cpu(env));
     }
     mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-        MSTATUS_SPP | MSTATUS_MPRV | MSTATUS_SUM |
-        MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
-        MSTATUS_TW;
+        MSTATUS_SPP | MSTATUS_SUM | MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TW;
 
     if (riscv_has_ext(env, RVF)) {
         mask |= MSTATUS_FS;
     }
 
+    if (riscv_has_ext(env, RVS) || riscv_has_ext(env, RVU)) {
+        mask |= MSTATUS_MPRV;
+    }
+
     if (riscv_cpu_cfg(env)->ext_zve32x) {
         mask |= MSTATUS_VS;
     }
 
+    if (riscv_has_ext(env, RVS)) {
+        mask |= MSTATUS_TSR;
+        mask |= MSTATUS_TVM;
+    }
+
     if (riscv_env_smode_dbltrp_enabled(env, env->virt_enabled)) {
         mask |= MSTATUS_SDT;
         if ((val & MSTATUS_SDT) != 0) {
@@ -2111,12 +2118,12 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
         if (riscv_has_ext(env, RVH)) {
             mask |= MSTATUS_MPV | MSTATUS_GVA;
         }
-        if ((val & MSTATUS64_SXL) != 0) {
+        if (riscv_has_ext(env, RVS) && (val & MSTATUS64_SXL) != 0) {
             mask |= MSTATUS64_SXL;
             val = riscv_write_uxl(env, val, MSTATUS64_SXL);
         }
 
-        if ((val & MSTATUS64_UXL) != 0) {
+        if (riscv_has_ext(env, RVU) && (val & MSTATUS64_UXL) != 0) {
             mask |= MSTATUS64_UXL;
             val = riscv_write_uxl(env, val, MSTATUS64_UXL);
         }
Signature · 1 line
2.56.0
Reviewed-by on v2 1/1
#1 lore raw
Quoted 12 lines · Christian S. Lima

The risc v privileged architecture says that mstatus.SXL, UXL, TSR, TVM and MPVR should be read-only when S-mode or U-mode are not present.

changes in v2:
- Don't change bits in target/riscv/cpu_bits.h
- Add checks to S-Mode in the mstatus.SXL and U-mode mstatus.UXL in the target/riscv/cpu.c file that were suggested by Daniel in the review [1]

Link to v1: https://lore.kernel.org/qemu-riscv/[email protected]/

Quoted 7 lines

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4456

[1] https://lore.kernel.org/qemu-riscv/[email protected]/T/#mddbb34a3ea837e3c707131a2e354c63c989ab029

---

Quoted 75 lines
  target/riscv/cpu.c     | 12 ++++++++++--
  target/riscv/tcg/csr.c | 17 ++++++++++++-----
  2 files changed, 22 insertions(+), 7 deletions(-)
target/riscv/cpu.c +10 -2
index 15c27f4ce4..50bdfbe025 100644
@@ -997,8 +997,16 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
           * The reset status of SXL/UXL is undefined, but mstatus is WARL
           * and we must ensure that the value after init is valid for read.
           */
-        env->mstatus = set_field(env->mstatus, MSTATUS64_SXL, env->misa_mxl);
-        env->mstatus = set_field(env->mstatus, MSTATUS64_UXL, env->misa_mxl);
+        if (riscv_has_ext(env, RVS)) {
+            env->mstatus = set_field(env->mstatus,
+                                     MSTATUS64_SXL, env->misa_mxl);
+        }
+
+        if (riscv_has_ext(env, RVU)) {
+            env->mstatus = set_field(env->mstatus,
+                                     MSTATUS64_UXL, env->misa_mxl);
+        }
+
          if (riscv_has_ext(env, RVH)) {
              env->vsstatus = set_field(env->vsstatus,
                                        MSTATUS64_SXL, env->misa_mxl);
target/riscv/tcg/csr.c +12 -5
index af444e3e6f..24eda54bac 100644
@@ -2081,18 +2081,25 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
          tlb_flush(env_cpu(env));
      }
      mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-        MSTATUS_SPP | MSTATUS_MPRV | MSTATUS_SUM |
-        MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
-        MSTATUS_TW;
+        MSTATUS_SPP | MSTATUS_SUM | MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TW;
 
      if (riscv_has_ext(env, RVF)) {
          mask |= MSTATUS_FS;
      }
 
+    if (riscv_has_ext(env, RVS) || riscv_has_ext(env, RVU)) {
+        mask |= MSTATUS_MPRV;
+    }
+
      if (riscv_cpu_cfg(env)->ext_zve32x) {
          mask |= MSTATUS_VS;
      }
 
+    if (riscv_has_ext(env, RVS)) {
+        mask |= MSTATUS_TSR;
+        mask |= MSTATUS_TVM;
+    }
+
      if (riscv_env_smode_dbltrp_enabled(env, env->virt_enabled)) {
          mask |= MSTATUS_SDT;
          if ((val & MSTATUS_SDT) != 0) {
@@ -2111,12 +2118,12 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
          if (riscv_has_ext(env, RVH)) {
              mask |= MSTATUS_MPV | MSTATUS_GVA;
          }
-        if ((val & MSTATUS64_SXL) != 0) {
+        if (riscv_has_ext(env, RVS) && (val & MSTATUS64_SXL) != 0) {
              mask |= MSTATUS64_SXL;
              val = riscv_write_uxl(env, val, MSTATUS64_SXL);
          }
 
-        if ((val & MSTATUS64_UXL) != 0) {
+        if (riscv_has_ext(env, RVU) && (val & MSTATUS64_UXL) != 0) {
              mask |= MSTATUS64_UXL;
              val = riscv_write_uxl(env, val, MSTATUS64_UXL);
          }