diff mbox series

[2/2] target/riscv: Legalize MPP value in write_mstatus

Message ID 20230330135818.68417-3-liweiwei@iscas.ac.cn (mailing list archive)
State New, archived
Headers show
Series target/riscv: Fix mstatus.MPP related support | expand

Commit Message

Weiwei Li March 30, 2023, 1:58 p.m. UTC 
mstatus.MPP field is a WARL field, 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 |  5 +----
 target/riscv/csr.c        | 14 ++++++++++++++
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Alistair Francis April 6, 2023, 1:26 a.m. UTC | #1 
On Thu, Mar 30, 2023 at 11:59 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> mstatus.MPP field is a WARL field, so we remain it unchanged if an

Only since version 1.11 of the priv spec and we do still support priv 1.10.

I think it's ok to make this change for all priv versions, as it won't
break any software running 1.10, but it's worth adding a comment or at
least a mention in the commit message.

Alistair

> 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 |  5 +----
>  target/riscv/csr.c        | 14 ++++++++++++++
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f88c503cf4..46baf3ab7c 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -659,12 +659,9 @@ 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) {
> +    if (newpriv > PRV_M || newpriv == PRV_H) {
>          g_assert_not_reached();
>      }
> -    if (newpriv == PRV_H) {
> -        newpriv = PRV_U;
> -    }
>      if (icount_enabled() && newpriv != env->priv) {
>          riscv_itrigger_update_priv(env);
>      }
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index d522efc0b6..a99026c3e8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1238,6 +1238,18 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
>      return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>  }
>
> +static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
> +                                 target_ulong val)
> +{
> +    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
> +    bool mpp_invalid = (new_mpp == PRV_S && !riscv_has_ext(env, RVS)) ||
> +                       (new_mpp == PRV_U && !riscv_has_ext(env, RVU)) ||
> +                       (new_mpp == PRV_H);
> +
> +    /* Remain field unchanged if new_mpp value is invalid */
> +    return mpp_invalid ? set_field(val, MSTATUS_MPP, old_mpp) : val;
> +}
> +
>  static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>                                      target_ulong val)
>  {
> @@ -1245,6 +1257,8 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>      uint64_t mask = 0;
>      RISCVMXL xl = riscv_cpu_mxl(env);
>
> +    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
>
>
Weiwei Li April 6, 2023, 1:35 a.m. UTC | #2 
On 2023/4/6 09:26, Alistair Francis wrote:
> On Thu, Mar 30, 2023 at 11:59 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> mstatus.MPP field is a WARL field, so we remain it unchanged if an
> Only since version 1.11 of the priv spec and we do still support priv 1.10.
>
> I think it's ok to make this change for all priv versions, as it won't
> break any software running 1.10, but it's worth adding a comment or at
> least a mention in the commit message.

OK. I'll add it in next version.

Regards,

Weiwei Li

>
> Alistair
>
>> 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 |  5 +----
>>   target/riscv/csr.c        | 14 ++++++++++++++
>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index f88c503cf4..46baf3ab7c 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -659,12 +659,9 @@ 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) {
>> +    if (newpriv > PRV_M || newpriv == PRV_H) {
>>           g_assert_not_reached();
>>       }
>> -    if (newpriv == PRV_H) {
>> -        newpriv = PRV_U;
>> -    }
>>       if (icount_enabled() && newpriv != env->priv) {
>>           riscv_itrigger_update_priv(env);
>>       }
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index d522efc0b6..a99026c3e8 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -1238,6 +1238,18 @@ static bool validate_vm(CPURISCVState *env, target_ulong vm)
>>       return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
>>   }
>>
>> +static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
>> +                                 target_ulong val)
>> +{
>> +    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
>> +    bool mpp_invalid = (new_mpp == PRV_S && !riscv_has_ext(env, RVS)) ||
>> +                       (new_mpp == PRV_U && !riscv_has_ext(env, RVU)) ||
>> +                       (new_mpp == PRV_H);
>> +
>> +    /* Remain field unchanged if new_mpp value is invalid */
>> +    return mpp_invalid ? set_field(val, MSTATUS_MPP, old_mpp) : val;
>> +}
>> +
>>   static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>>                                       target_ulong val)
>>   {
>> @@ -1245,6 +1257,8 @@ static RISCVException write_mstatus(CPURISCVState *env, int csrno,
>>       uint64_t mask = 0;
>>       RISCVMXL xl = riscv_cpu_mxl(env);
>>
>> +    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
>>
>>
diff mbox series

Patch

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..46baf3ab7c 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -659,12 +659,9 @@  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) {
+    if (newpriv > PRV_M || newpriv == PRV_H) {
         g_assert_not_reached();
     }
-    if (newpriv == PRV_H) {
-        newpriv = PRV_U;
-    }
     if (icount_enabled() && newpriv != env->priv) {
         riscv_itrigger_update_priv(env);
     }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..a99026c3e8 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1238,6 +1238,18 @@  static bool validate_vm(CPURISCVState *env, target_ulong vm)
     return (vm & 0xf) <= satp_mode_max_from_map(cpu->cfg.satp_mode.map);
 }
 
+static target_ulong legalize_mpp(CPURISCVState *env, target_ulong old_mpp,
+                                 target_ulong val)
+{
+    target_ulong new_mpp = get_field(val, MSTATUS_MPP);
+    bool mpp_invalid = (new_mpp == PRV_S && !riscv_has_ext(env, RVS)) ||
+                       (new_mpp == PRV_U && !riscv_has_ext(env, RVU)) ||
+                       (new_mpp == PRV_H);
+
+    /* Remain field unchanged if new_mpp value is invalid */
+    return mpp_invalid ? set_field(val, MSTATUS_MPP, old_mpp) : val;
+}
+
 static RISCVException write_mstatus(CPURISCVState *env, int csrno,
                                     target_ulong val)
 {
@@ -1245,6 +1257,8 @@  static RISCVException write_mstatus(CPURISCVState *env, int csrno,
     uint64_t mask = 0;
     RISCVMXL xl = riscv_cpu_mxl(env);
 
+    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)) {