mstatus.MPP field is a WARL field since priv version 1.11, so we
remain it unchanged if an invalid value is written into it. And
after this, RVH shouldn't be passed to riscv_cpu_set_mode().

Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
---
 target/riscv/cpu_helper.c |  8 ++------
 target/riscv/csr.c        | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2310c7905f..433ea529b0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -647,12 +647,8 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
 
 void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
 {
-    if (newpriv > PRV_M) {
-        g_assert_not_reached();
-    }
-    if (newpriv == PRV_H) {
-        newpriv = PRV_U;
-    }
+    g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
+
     if (icount_enabled() && newpriv != env->priv) {
         riscv_itrigger_update_priv(env);
     }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e0b871f6dc..f4d2dcfdc8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1230,6 +1230,32 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
            satp_mode_max_from_map(riscv_cpu_cfg(env)->satp_mode.map);
 }
 
+static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
+                                 target_ulong val)
+{
+    bool valid = false;
+    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
+
+    switch (new_mpp) {
+    case PRV_M:
+        valid = true;
+        break;
+    case PRV_S:
+        valid = riscv_has_ext(env, RVS);
+        break;
+    case PRV_U:
+        valid = riscv_has_ext(env, RVU);
+        break;
+    }
+
+    /* Remain field unchanged if new_mpp value is invalid */
+    if (!valid) {
+        val = set_field(val, MSTATUS_MPP, old_mpp);
+    }
+
+    return val;
+}
+
 static RISCVException write_mstatus(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
@@ -1237,6 +1263,12 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
     uint64_t mask = 0;
     RISCVMXL xl = riscv_cpu_mxl(env);
 
+    /*
+     * MPP field have been made WARL since priv version 1.11. However,
+     * legalization for it will not break any software running on 1.10.
+     */
+    val = legalize_mpp(env, get_field(mstatus, MSTATUS_MPP), val);
+
     /* flush tlb on mstatus fields that affect VM */
     if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
             MSTATUS_MPRV | MSTATUS_SUM)) {
-- 
2.25.1

On Fri, Apr 7, 2023 at 11:49 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> mstatus.MPP field is a WARL field since priv version 1.11, so we
> remain it unchanged if an invalid value is written into it. And
> after this, RVH shouldn't be passed to riscv_cpu_set_mode().
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu_helper.c |  8 ++------
>  target/riscv/csr.c        | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2310c7905f..433ea529b0 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -647,12 +647,8 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, uint32_t priv,
>
>  void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
>  {
> -    if (newpriv > PRV_M) {
> -        g_assert_not_reached();
> -    }
> -    if (newpriv == PRV_H) {
> -        newpriv = PRV_U;
> -    }
> +    g_assert(newpriv <= PRV_M && newpriv != PRV_RESERVED);
> +
>      if (icount_enabled() && newpriv != env->priv) {
>          riscv_itrigger_update_priv(env);
>      }
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e0b871f6dc..f4d2dcfdc8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1230,6 +1230,32 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
>             satp_mode_max_from_map(riscv_cpu_cfg(env)->satp_mode.map);
>  }
>
> +static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> +                                 target_ulong val)
> +{
> +    bool valid = false;
> +    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
> +
> +    switch (new_mpp) {
> +    case PRV_M:
> +        valid = true;
> +        break;
> +    case PRV_S:
> +        valid = riscv_has_ext(env, RVS);
> +        break;
> +    case PRV_U:
> +        valid = riscv_has_ext(env, RVU);
> +        break;
> +    }
> +
> +    /* Remain field unchanged if new_mpp value is invalid */
> +    if (!valid) {
> +        val = set_field(val, MSTATUS_MPP, old_mpp);
> +    }
> +
> +    return val;
> +}
> +
>  static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>                                      target_ulong val)
>  {
> @@ -1237,6 +1263,12 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>      uint64_t mask = 0;
>      RISCVMXL xl = riscv_cpu_mxl(env);
>
> +    /*
> +     * MPP field have been made WARL since priv version 1.11. However,
> +     * legalization for it will not break any software running on 1.10.
> +     */
> +    val = legalize_mpp(env, get_field(mstatus, MSTATUS_MPP), val);
> +
>      /* flush tlb on mstatus fields that affect VM */
>      if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
>              MSTATUS_MPRV | MSTATUS_SUM)) {
> --
> 2.25.1
>
>

On 4/6/23 18:47, Weiwei Li wrote:
> mstatus.MPP field is a WARL field since priv version 1.11, so we
> remain it unchanged if an invalid value is written into it. And
> after this, RVH shouldn't be passed to riscv_cpu_set_mode().
> 
> Signed-off-by: Weiwei Li<liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang<wangjunqiang@iscas.ac.cn>
> ---
>   target/riscv/cpu_helper.c |  8 ++------
>   target/riscv/csr.c        | 32 ++++++++++++++++++++++++++++++++
>   2 files changed, 34 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~