Re: [RFC PATCH 01/13] target/riscv: Add UXL to tb flags

[Richard Henderson]

On 8/4/21 4:53 PM, LIU Zhiwei wrote:
> For 32-bit applications run on 64-bit cpu, it may share some code
> with other 64-bit applictions. Thus we should distinguish the translated
> cache of the share code with a tb flag.
>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>   target/riscv/cpu.h       | 15 +++++++++++++++
>   target/riscv/translate.c |  3 +++
>   2 files changed, 18 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index bf1c899c00..2b3ba21a78 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -394,9 +394,20 @@ FIELD(TB_FLAGS, SEW, 5, 3)
>   FIELD(TB_FLAGS, VILL, 8, 1)
>   /* Is a Hypervisor instruction load/store allowed? */
>   FIELD(TB_FLAGS, HLSX, 9, 1)
> +FIELD(TB_FLAGS, UXL, 10, 2)

Are you intending to reserve space for RV128 here?
Otherwise this could be a single bit.

Also, you probably don't want to name it "UXL", since it should indicate the current 
operating XLEN, taking MXL, SXL and UXL into account.

Perhaps just name the field XLEN32, and make it a single bit?

> +static inline bool riscv_cpu_is_uxl32(CPURISCVState *env)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    return (get_field(env->mstatus, MSTATUS64_UXL) == 1) &&
> +           !riscv_cpu_is_32bit(env) &&
> +           (env->priv == PRV_U);
> +#endif
> +    return false;
> +}

Again, naming could be better?
It seems trivial to handle all of the fields here.  Perhaps


static inline bool riscv_cpu_is_xlen32(env)
{
#if defined(TARGET_RISCV32)
    return true;
#elif defined(CONFIG_USER_ONLY)
    return false;
#else
    /* When emulating a 32-bit-only cpu, use RV32. */
    if (riscv_cpu_is_32bit(env)) {
        return true;
    }
    /*
     * If MXL has been reduced to RV32, MSTATUSH doesn't have UXL/SXL,
     * therefore, XLEN cannot be widened back to RV64 for lower privs.
     */
    if (get_field(env->misa, MISA64_MXL) == 1) {
        return true;
    }
    switch (env->priv) {
    case PRV_M:
        return false;
    case PRV_U:
        return get_field(env->mstatus, MSTATUS64_UXL) == 1;
    default: /* PRV_S & PRV_H */
        return get_field(env->mstatus, MSTATUS64_SXL) == 1;
    }
#endif
}


> @@ -451,6 +462,10 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
> *env, target_ulong *pc,
>               flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
>           }
>       }
> +    if (riscv_cpu_is_uxl32(env)) {
> +        flags = FIELD_DP32(flags, TB_FLAGS, UXL,
> +                           get_field(env->mstatus, MSTATUS64_UXL));

  flags = FIELD_DP32(flags, TB_FLAGS, XLEN32,
                     riscv_cpu_is_xlen32(env));


r~