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

[LIU Zhiwei]

On 2021/8/6 上午3:01, Richard Henderson wrote:

> 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.
>       
>       
>     
    Yes.
> 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.
>       
>       
>

Hi Richard,

I don't have the ambitious to define a XLEN32 at least for this patch set. I think it is much more difficult.

The only purpose of this patch set is that we can run 32-bit application on  a 64 bit Linux from qemu-system-riscv64.

So  I didn't change the default behavior of every instruction except when

1. Current CPU is 64 bit CPU, i.s. TARGET_LONG_BITS is 64.
   
2. Current privileged is U-mode.
3. UXL is 1.

I know that Alistair has done a lot to support 32-bit CPU on qemu-system-riscv64. But We are doing different things,
and it maybe a little confusing.

I still do not find a good to combine them. In my opinion, some code in this patch set can be reused for SXL32.
If you have any advice, please let me know.

Best Regards,
Zhiwei

> 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~
>       
>