Re: [PATCH v2 05/13] target/riscv: Add MXL/SXL/UXL to TB_FLAGS

[LIU Zhiwei]

On 2021/10/14 上午4:50, Richard Henderson wrote:
> Begin adding support for switching XLEN at runtime.  Extract the
> effective XLEN from MISA and MSTATUS and store for use during translation.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v2: Force SXL and UXL to valid values.
> ---
>   target/riscv/cpu.h        |  2 ++
>   target/riscv/cpu.c        |  8 ++++++++
>   target/riscv/cpu_helper.c | 33 +++++++++++++++++++++++++++++++++
>   target/riscv/csr.c        |  3 +++
>   target/riscv/translate.c  |  2 +-
>   5 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 87248b562a..445ba5b395 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -395,6 +395,8 @@ FIELD(TB_FLAGS, VILL, 8, 1)
>   /* Is a Hypervisor instruction load/store allowed? */
>   FIELD(TB_FLAGS, HLSX, 9, 1)
>   FIELD(TB_FLAGS, MSTATUS_HS_FS, 10, 2)
> +/* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
> +FIELD(TB_FLAGS, XL, 12, 2)
>   
>   #ifdef CONFIG_RISCV32
>   #define riscv_cpu_mxl(env)      MXL_RV32
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1857670a69..840edd66f8 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -355,6 +355,14 @@ static void riscv_cpu_reset(DeviceState *dev)
>       env->misa_mxl = env->misa_mxl_max;
>       env->priv = PRV_M;
>       env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
> +    if (env->misa_mxl > MXL_RV32) {
> +        /*
> +         * The reset status of SXL/UXL is officially undefined,
> +         * but invalid settings would result in a tcg assert.
> +         */
> +        env->mstatus = set_field(env->mstatus, MSTATUS64_SXL, env->misa_mxl);
> +        env->mstatus = set_field(env->mstatus, MSTATUS64_UXL, env->misa_mxl);
> +    }

Can you give more explanation about the assert? As the cpu will always 
reset to M mode, I think we can omit the the setting of UXL or SXL.

Thanks,
Zhiwei

>       env->mcause = 0;
>       env->pc = env->resetvec;
>       env->two_stage_lookup = false;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 403f54171d..429afd1f48 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -35,6 +35,37 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>   #endif
>   }
>   
> +static RISCVMXL cpu_get_xl(CPURISCVState *env)
> +{
> +#if defined(TARGET_RISCV32)
> +    return MXL_RV32;
> +#elif defined(CONFIG_USER_ONLY)
> +    return MXL_RV64;
> +#else
> +    RISCVMXL xl = riscv_cpu_mxl(env);
> +
> +    /*
> +     * When emulating a 32-bit-only cpu, use RV32.
> +     * When emulating a 64-bit cpu, and MXL has been reduced to RV32,
> +     * MSTATUSH doesn't have UXL/SXL, therefore XLEN cannot be widened
> +     * back to RV64 for lower privs.
> +     */
> +    if (xl != MXL_RV32) {
> +        switch (env->priv) {
> +        case PRV_M:
> +            break;
> +        case PRV_U:
> +            xl = get_field(env->mstatus, MSTATUS64_UXL);
> +            break;
> +        default: /* PRV_S | PRV_H */
> +            xl = get_field(env->mstatus, MSTATUS64_SXL);
> +            break;
> +        }
> +    }
> +    return xl;
> +#endif
> +}
> +
>   void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
>                             target_ulong *cs_base, uint32_t *pflags)
>   {
> @@ -78,6 +109,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
> *pc,
>       }
>   #endif
>   
> +    flags = FIELD_DP32(flags, TB_FLAGS, XL, cpu_get_xl(env));
> +
>       *pflags = flags;
>   }
>   
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9c0753bc8b..c4a479ddd2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -526,6 +526,9 @@ static RISCVException write_mstatus(CPURISCVState *env, int 
> csrno,
>           mstatus = set_field(mstatus, MSTATUS32_SD, dirty);
>       } else {
>           mstatus = set_field(mstatus, MSTATUS64_SD, dirty);
> +        /* SXL and UXL fields are for now read only */
> +        mstatus = set_field(mstatus, MSTATUS64_SXL, MXL_RV64);
> +        mstatus = set_field(mstatus, MSTATUS64_UXL, MXL_RV64);
>       }
>       env->mstatus = mstatus;
>   
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 422f8ab8d0..7e7bb67d15 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -539,7 +539,6 @@ static void riscv_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>   #else
>       ctx->virt_enabled = false;
>   #endif
> -    ctx->xl = env->misa_mxl;
>       ctx->misa_ext = env->misa_ext;
>       ctx->frm = -1;  /* unknown rounding mode */
>       ctx->ext_ifencei = cpu->cfg.ext_ifencei;
> @@ -551,6 +550,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>       ctx->lmul = FIELD_EX32(tb_flags, TB_FLAGS, LMUL);
>       ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
>       ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
> +    ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
>       ctx->cs = cs;
>       ctx->w = false;
>       ctx->ntemp = 0;