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

[LIU Zhiwei <zhiwei_liu@c-sky.com>]

[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]


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~

[-- Attachment #2: Type: text/html, Size: 6122 bytes --]