Re: [PATCH v6 13/25] target/riscv: Introduce mmuidx_priv

[LIU Zhiwei]

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) {