diff mbox series

[v6,13/25] target/riscv: Introduce mmuidx_priv

Message ID 20230325105429.1142530-14-richard.henderson@linaro.org
State New
Headers show
Series target/riscv: MSTATUS_SUM + cleanups | expand

Commit Message

Richard Henderson March 25, 2023, 10:54 a.m. UTC 
Use the priv level encoded into the mmu_idx, rather than
starting from env->priv.  We have already checked MPRV+MPP
in riscv_cpu_mmu_index -- no need to repeat that.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/internals.h  | 9 +++++++++
 target/riscv/cpu_helper.c | 6 +-----
 2 files changed, 10 insertions(+), 5 deletions(-)

Comments

LIU Zhiwei March 27, 2023, 2:07 a.m. UTC | #1 
On 2023/3/25 18:54, Richard Henderson wrote:
> Use the priv level encoded into the mmu_idx, rather than
> starting from env->priv.  We have already checked MPRV+MPP
> in riscv_cpu_mmu_index -- no need to repeat that.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/riscv/internals.h  | 9 +++++++++
>   target/riscv/cpu_helper.c | 6 +-----
>   2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 0b61f337dd..4aa1cb409f 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -37,6 +37,15 @@
>   #define MMUIdx_M            3
>   #define MMU_2STAGE_BIT      (1 << 2)
>   
> +static inline int mmuidx_priv(int mmu_idx)
> +{
> +    int ret = mmu_idx & 3;
> +    if (ret == MMUIdx_S_SUM) {
> +        ret = PRV_S;
> +    }
> +    return ret;
> +}
> +

Can we remove the PRIV from the tb flags after we have this function?

Best Regards,
Zhiwei

>   static inline bool mmuidx_sum(int mmu_idx)
>   {
>       return (mmu_idx & 3) == MMUIdx_S_SUM;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 7e6cd8e0fd..cb260b88ea 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -771,7 +771,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>        * (riscv_cpu_do_interrupt) is correct */
>       MemTxResult res;
>       MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> -    int mode = env->priv;
> +    int mode = mmuidx_priv(mmu_idx);
>       bool use_background = false;
>       hwaddr ppn;
>       RISCVCPU *cpu = env_archcpu(env);
> @@ -793,10 +793,6 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>          instructions, HLV, HLVX, and HSV. */
>       if (riscv_cpu_two_stage_lookup(mmu_idx)) {
>           mode = get_field(env->hstatus, HSTATUS_SPVP);
> -    } else if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> -        if (get_field(env->mstatus, MSTATUS_MPRV)) {
> -            mode = get_field(env->mstatus, MSTATUS_MPP);
> -        }
>       }
>   
>       if (first_stage == false) {
Richard Henderson March 27, 2023, 4:29 p.m. UTC | #2 
On 3/26/23 19:07, LIU Zhiwei wrote:
>> +static inline int mmuidx_priv(int mmu_idx)
>> +{
>> +    int ret = mmu_idx & 3;
>> +    if (ret == MMUIdx_S_SUM) {
>> +        ret = PRV_S;
>> +    }
>> +    return ret;
>> +}
>> +
> 
> Can we remove the PRIV from the tb flags after we have this function?

No, because this is the priv of the memory operation as modified by e.g. MPRV, not the 
true cpu priv.


r~
LIU Zhiwei March 28, 2023, 1:33 a.m. UTC | #3 
On 2023/3/28 0:29, Richard Henderson wrote:
> On 3/26/23 19:07, LIU Zhiwei wrote:
>>> +static inline int mmuidx_priv(int mmu_idx)
>>> +{
>>> +    int ret = mmu_idx & 3;
>>> +    if (ret == MMUIdx_S_SUM) {
>>> +        ret = PRV_S;
>>> +    }
>>> +    return ret;
>>> +}
>>> +
>>
>> Can we remove the PRIV from the tb flags after we have this function?
>
> No, because this is the priv of the memory operation as modified by 
> e.g. MPRV, not the true cpu priv.

For this implementation, we explicitly use the tb flags for mmu index. I 
think it is the reason why we have to maintain the redundant privilege 
in tb flags.
It may be better to only store machine states into tb flags. Can we just 
pass everything that we need, for example, the priv and sum, and then 
implicitly
calculate the ctx->mem_idx in disas_init_fn?

I remember that you give the similar suggestion in the comment process

https://mail.gnu.org/archive/html/qemu-riscv/2023-03/msg00566.html

Best Regards,
Zhiwei

>
>
> r~
LIU Zhiwei March 28, 2023, 1:54 a.m. UTC | #4 
On 2023/3/28 9:33, LIU Zhiwei wrote:
>
> On 2023/3/28 0:29, Richard Henderson wrote:
>> On 3/26/23 19:07, LIU Zhiwei wrote:
>>>> +static inline int mmuidx_priv(int mmu_idx)
>>>> +{
>>>> +    int ret = mmu_idx & 3;
>>>> +    if (ret == MMUIdx_S_SUM) {
>>>> +        ret = PRV_S;
>>>> +    }
>>>> +    return ret;
>>>> +}
>>>> +
>>>
>>> Can we remove the PRIV from the tb flags after we have this function?
>>
>> No, because this is the priv of the memory operation as modified by 
>> e.g. MPRV, not the true cpu priv.
>
> For this implementation, we explicitly use the tb flags for mmu index. 
> I think it is the reason why we have to maintain the redundant 
> privilege in tb flags.
> It may be better to only store machine states into tb flags. Can we 
> just pass everything that we need, for example, the priv and sum, and 
> then implicitly
> calculate the ctx->mem_idx in disas_init_fn?
>
> I remember that you give the similar suggestion in the comment process
>
> https://mail.gnu.org/archive/html/qemu-riscv/2023-03/msg00566.html
>
> Best Regards,
> Zhiwei
>
To make this comment clear, I paste a simple implementatioin here. But 
it is just for discussing, not a normal patch for merging.

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 623288e6f9..d4506be5be 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -632,12 +632,10 @@ G_NORETURN void 
riscv_raise_exception(CPURISCVState *env,
  target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
  void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);

-#define TB_FLAGS_PRIV_MMU_MASK                3
-#define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
-
  #include "exec/cpu-all.h"

-FIELD(TB_FLAGS, MEM_IDX, 0, 3)
+FIELD(TB_FLAGS, PRIV, 0, 2)
+FIELD(TB_FLAGS, SUM, 2, 1)
  FIELD(TB_FLAGS, FS, 3, 2)
  /* Vector flags */
  FIELD(TB_FLAGS, VS, 5, 2)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f80d069884..b11f583643 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -30,6 +30,7 @@
  #include "sysemu/cpu-timers.h"
  #include "cpu_bits.h"
  #include "debug.h"
+#include "internals.h"

  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
  {
@@ -40,6 +41,20 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
  #endif
  }

+#ifndef CONFIG_USER_ONLY
+static bool riscv_cpu_sum_enabled(CPURISCVState *env)
+{
+    int mode = env->priv;
+    if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
+        mode = get_field(env->mstatus, MSTATUS_MPP);
+    }
+    if (mode == PRV_S && get_field(env->mstatus, MSTATUS_SUM)) {
+        return true;
+    }
+    return false;
+}
+#endif
+
  void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
                            target_ulong *cs_base, uint32_t *pflags)
  {
@@ -80,10 +95,11 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, 
target_ulong *pc,
      }

  #ifdef CONFIG_USER_ONLY
     flags =  FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
     flags =  FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
  #else
-    flags |= cpu_mmu_index(env, 0);
+    flags = FIELD_DP32(flags, TB_FLAGS, PRIV, env->priv);
+    flags = FIELD_DP32(flags, TB_FLAGS, SUM, riscv_cpu_sum_enabled(env));
      if (riscv_cpu_fp_enabled(env)) {
          flags =  FIELD_DP32(flags, TB_FLAGS, FS,
                              get_field(env->mstatus, MSTATUS_FS));
@@ -600,7 +616,7 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, 
bool enable)

  bool riscv_cpu_two_stage_lookup(int mmu_idx)
  {
-    return mmu_idx & TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+    return mmu_idx & MMU_HYP_ACCESS_BIT;
  }

  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
@@ -766,7 +782,7 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
       * (riscv_cpu_do_interrupt) is correct */
      MemTxResult res;
      MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
-    int mode = mmu_idx & TB_FLAGS_PRIV_MMU_MASK;
+    int mode = mmu_idx;
      bool use_background = false;
      hwaddr ppn;
      RISCVCPU *cpu = env_archcpu(env);
@@ -788,10 +804,8 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
         instructions, HLV, HLVX, and HSV. */
      if (riscv_cpu_two_stage_lookup(mmu_idx)) {
          mode = get_field(env->hstatus, HSTATUS_SPVP);
-    } else if (mode == PRV_M && access_type != MMU_INST_FETCH) {
-        if (get_field(env->mstatus, MSTATUS_MPRV)) {
-            mode = get_field(env->mstatus, MSTATUS_MPP);
-        }
+    } else if (mmu_idx == MMUIdx_S_SUM) {
+        mode = PRV_S;
      }
      if (first_stage == false) {
@@ -847,7 +861,7 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
          widened = 2;
      }
      /* status.SUM will be ignored if execute on background */
-    sum = get_field(env->mstatus, MSTATUS_SUM) || use_background || 
is_debug;
+    sum = (mmu_idx == MMUIdx_S_SUM) || use_background || is_debug;
      switch (vm) {
      case VM_1_10_SV32:
        levels = 2; ptidxbits = 10; ptesize = 4; break;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index abea7b749e..dd5f774b2f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1247,7 +1247,7 @@ static RISCVException write_mstatus(CPURISCVState 
*env, int csrno,

      /* flush tlb on mstatus fields that affect VM */
      if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
-            MSTATUS_MPRV | MSTATUS_SUM)) {
+            MSTATUS_MPRV)) {
          tlb_flush(env_cpu(env));
      }
      mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc 
b/target/riscv/insn_trans/trans_privileged.c.inc
index 59501b2780..9305b18299 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -52,7 +52,7 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
       * that no exception will be raised when fetching them.
       */

-    if (semihosting_enabled(ctx->mem_idx < PRV_S) &&
+    if (semihosting_enabled(ctx->priv < PRV_S) &&
          (pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
          pre    = opcode_at(&ctx->base, pre_addr);
          ebreak = opcode_at(&ctx->base, ebreak_addr);
diff --git a/target/riscv/insn_trans/trans_rvh.c.inc 
b/target/riscv/insn_trans/trans_rvh.c.inc
index 9248b48c36..15842f4282 100644
--- a/target/riscv/insn_trans/trans_rvh.c.inc
+++ b/target/riscv/insn_trans/trans_rvh.c.inc
@@ -40,7 +40,7 @@ static bool do_hlv(DisasContext *ctx, arg_r2 *a, MemOp 
mop)
      if (check_access(ctx)) {
          TCGv dest = dest_gpr(ctx, a->rd);
          TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
-        int mem_idx = ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+        int mem_idx = ctx->mem_idx | MMU_HYP_ACCESS_BIT;
          tcg_gen_qemu_ld_tl(dest, addr, mem_idx, mop);
          gen_set_gpr(ctx, a->rd, dest);
      }
@@ -87,7 +87,7 @@ static bool do_hsv(DisasContext *ctx, arg_r2_s *a, 
MemOp mop)
      if (check_access(ctx)) {
          TCGv addr = get_gpr(ctx, a->rs1, EXT_NONE);
          TCGv data = get_gpr(ctx, a->rs2, EXT_NONE);
-        int mem_idx = ctx->mem_idx | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+        int mem_idx = ctx->mem_idx | MMU_HYP_ACCESS_BIT;
          tcg_gen_qemu_st_tl(data, addr, mem_idx, mop);
      }
      return true;
diff --git a/target/riscv/insn_trans/trans_xthead.c.inc 
b/target/riscv/insn_trans/trans_xthead.c.inc
index df504c3f2c..a6d823b382 100644
--- a/target/riscv/insn_trans/trans_xthead.c.inc
+++ b/target/riscv/insn_trans/trans_xthead.c.inc
@@ -263,23 +263,13 @@ static bool trans_th_tst(DisasContext *ctx, 
arg_th_tst *a)

  /* XTheadCmo */

-static inline int priv_level(DisasContext *ctx)
-{
-#ifdef CONFIG_USER_ONLY
-    return PRV_U;
-#else
-     /* Priv level is part of mem_idx. */
-    return ctx->mem_idx & TB_FLAGS_PRIV_MMU_MASK;
-#endif
-}
-
  /* Test if priv level is M, S, or U (cannot fail). */
  #define REQUIRE_PRIV_MSU(ctx)

  /* Test if priv level is M or S. */
  #define REQUIRE_PRIV_MS(ctx)                                    \
  do {                                                            \
-    int priv = priv_level(ctx);                                 \
+    int priv = ctx->priv;                                       \
      if (!(priv == PRV_M ||                                      \
            priv == PRV_S)) {                                     \
          return false;                                           \
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 5620fbffb6..b55152a7dc 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -21,6 +21,20 @@

  #include "hw/registerfields.h"

+/*
+ * The current MMU Modes are:
+ *  - U                 0b000
+ *  - S                 0b001
+ *  - S+SUM             0b010
+ *  - M                 0b011
+ *  - HLV/HLVX/HSV adds 0b100
+ */
+#define MMUIdx_U            0
+#define MMUIdx_S            1
+#define MMUIdx_S_SUM        2
+#define MMUIdx_M            3
+#define MMU_HYP_ACCESS_BIT  (1 << 2)
+
  /* share data between vector helpers and decode code */
  FIELD(VDATA, VM, 0, 1)
  FIELD(VDATA, LMUL, 1, 3)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 84ee018f7d..8a0ddb91b5 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -23,6 +23,7 @@
  #include "qemu/main-loop.h"
  #include "exec/exec-all.h"
  #include "exec/helper-proto.h"
+#include "internals.h"

  /* Exceptions processing helpers */
  G_NORETURN void riscv_raise_exception(CPURISCVState *env,
@@ -428,14 +429,14 @@ void helper_hyp_gvma_tlb_flush(CPURISCVState *env)

  target_ulong helper_hyp_hlvx_hu(CPURISCVState *env, target_ulong address)
  {
-    int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+    int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;

      return cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
  }

  target_ulong helper_hyp_hlvx_wu(CPURISCVState *env, target_ulong address)
  {
-    int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+    int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;

      return cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
  }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index e8bac1b470..f32cd7143a 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -69,6 +69,7 @@ typedef struct DisasContext {
      uint32_t mstatus_hs_fs;
      uint32_t mstatus_hs_vs;
      uint32_t mem_idx;
+    uint32_t priv;
      /* Remember the rounding mode encoded in the previous fp instruction,
         which we have already installed into env->fp_status. Or -1 for
         no previous fp instruction.  Note that we exit the TB when writing
@@ -1152,7 +1153,12 @@ static void 
riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
      uint32_t tb_flags = ctx->base.tb->flags;

      ctx->pc_succ_insn = ctx->base.pc_first;
-    ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
+    ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV);
+    if (FIELD_EX32(tb_flags, TB_FLAGS, SUM)) {
+        ctx->mem_idx = MMUIdx_S_SUM;
+    } else {
+        ctx->mem_idx = ctx->priv;
+    }
      ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
      ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
      ctx->priv_ver = env->priv_ver;

Best Regards,
Zhiwei

>>
>>
>> r~
Richard Henderson March 28, 2023, 2:27 p.m. UTC | #5 
On 3/27/23 18:54, LIU Zhiwei wrote:
>>>> Can we remove the PRIV from the tb flags after we have this function?
>>>
>>> No, because this is the priv of the memory operation as modified by e.g. MPRV, not the 
>>> true cpu priv.
>>
>> For this implementation, we explicitly use the tb flags for mmu index. I think it is the 
>> reason why we have to maintain the redundant privilege in tb flags.
>> It may be better to only store machine states into tb flags. Can we just pass everything 
>> that we need, for example, the priv and sum, and then implicitly
>> calculate the ctx->mem_idx in disas_init_fn?
>>
>> I remember that you give the similar suggestion in the comment process
>>
>> https://mail.gnu.org/archive/html/qemu-riscv/2023-03/msg00566.html
>>
>> Best Regards,
>> Zhiwei
>>
> To make this comment clear, I paste a simple implementatioin here. But it is just for 
> discussing, not a normal patch for merging.
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 623288e6f9..d4506be5be 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -632,12 +632,10 @@ G_NORETURN void riscv_raise_exception(CPURISCVState *env,
>   target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
>   void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
> 
> -#define TB_FLAGS_PRIV_MMU_MASK                3
> -#define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
> -
>   #include "exec/cpu-all.h"
> 
> -FIELD(TB_FLAGS, MEM_IDX, 0, 3)
> +FIELD(TB_FLAGS, PRIV, 0, 2)
> +FIELD(TB_FLAGS, SUM, 2, 1)

We would need to include MPRV, MPP, MPV, VIRT_ENABLED as well.  With SUM, that would be 6 
bits in tb_flags instead of 3 bits for MEM_IDX.


r~
Alistair Francis April 11, 2023, 3:53 a.m. UTC | #6 
On Sat, Mar 25, 2023 at 9:53 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Use the priv level encoded into the mmu_idx, rather than
> starting from env->priv.  We have already checked MPRV+MPP
> in riscv_cpu_mmu_index -- no need to repeat that.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/internals.h  | 9 +++++++++
>  target/riscv/cpu_helper.c | 6 +-----
>  2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 0b61f337dd..4aa1cb409f 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -37,6 +37,15 @@
>  #define MMUIdx_M            3
>  #define MMU_2STAGE_BIT      (1 << 2)
>
> +static inline int mmuidx_priv(int mmu_idx)
> +{
> +    int ret = mmu_idx & 3;
> +    if (ret == MMUIdx_S_SUM) {
> +        ret = PRV_S;
> +    }
> +    return ret;
> +}
> +
>  static inline bool mmuidx_sum(int mmu_idx)
>  {
>      return (mmu_idx & 3) == MMUIdx_S_SUM;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 7e6cd8e0fd..cb260b88ea 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -771,7 +771,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>       * (riscv_cpu_do_interrupt) is correct */
>      MemTxResult res;
>      MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> -    int mode = env->priv;
> +    int mode = mmuidx_priv(mmu_idx);
>      bool use_background = false;
>      hwaddr ppn;
>      RISCVCPU *cpu = env_archcpu(env);
> @@ -793,10 +793,6 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>         instructions, HLV, HLVX, and HSV. */
>      if (riscv_cpu_two_stage_lookup(mmu_idx)) {
>          mode = get_field(env->hstatus, HSTATUS_SPVP);
> -    } else if (mode == PRV_M && access_type != MMU_INST_FETCH) {
> -        if (get_field(env->mstatus, MSTATUS_MPRV)) {
> -            mode = get_field(env->mstatus, MSTATUS_MPP);
> -        }
>      }
>
>      if (first_stage == false) {
> --
> 2.34.1
>
>
diff mbox series

Patch

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 0b61f337dd..4aa1cb409f 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -37,6 +37,15 @@ 
 #define MMUIdx_M            3
 #define MMU_2STAGE_BIT      (1 << 2)
 
+static inline int mmuidx_priv(int mmu_idx)
+{
+    int ret = mmu_idx & 3;
+    if (ret == MMUIdx_S_SUM) {
+        ret = PRV_S;
+    }
+    return ret;
+}
+
 static inline bool mmuidx_sum(int mmu_idx)
 {
     return (mmu_idx & 3) == MMUIdx_S_SUM;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 7e6cd8e0fd..cb260b88ea 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -771,7 +771,7 @@  static int get_physical_address(CPURISCVState *env, hwaddr *physical,
      * (riscv_cpu_do_interrupt) is correct */
     MemTxResult res;
     MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
-    int mode = env->priv;
+    int mode = mmuidx_priv(mmu_idx);
     bool use_background = false;
     hwaddr ppn;
     RISCVCPU *cpu = env_archcpu(env);
@@ -793,10 +793,6 @@  static int get_physical_address(CPURISCVState *env, hwaddr *physical,
        instructions, HLV, HLVX, and HSV. */
     if (riscv_cpu_two_stage_lookup(mmu_idx)) {
         mode = get_field(env->hstatus, HSTATUS_SPVP);
-    } else if (mode == PRV_M && access_type != MMU_INST_FETCH) {
-        if (get_field(env->mstatus, MSTATUS_MPRV)) {
-            mode = get_field(env->mstatus, MSTATUS_MPP);
-        }
     }
 
     if (first_stage == false) {