diff mbox series

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

Message ID 20210805025312.15720-2-zhiwei_liu@c-sky.com (mailing list archive)
State New, archived
Headers show
Series Support UXL field in mstatus | expand

Commit Message

LIU Zhiwei Aug. 5, 2021, 2:53 a.m. UTC
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(+)

Comments

Alistair Francis Aug. 5, 2021, 6 a.m. UTC | #1
On Thu, Aug 5, 2021 at 12:55 PM LIU Zhiwei <zhiwei_liu@c-sky.com> 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  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)
>
>  bool riscv_cpu_is_32bit(CPURISCVState *env);
>
> +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;
> +}
> +
>  /*
>   * A simplification for VLMAX
>   * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
> @@ -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));
> +    }
>  #endif
>
>      *pflags = flags;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 076f28b9c1..ac4a545da8 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -67,6 +67,8 @@ typedef struct DisasContext {
>      CPUState *cs;
>      TCGv zero;
>      TCGv sink;
> +    /* UXLEN is 32 bit for 64-bit CPU */
> +    bool uxl32;
>  } DisasContext;
>
>  static inline bool has_ext(DisasContext *ctx, uint32_t ext)
> @@ -912,6 +914,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>      ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
>      ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
>      ctx->cs = cs;
> +    ctx->uxl32 = FIELD_EX32(tb_flags, TB_FLAGS, UXL) == 1;
>  }
>
>  static void riscv_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)
> --
> 2.17.1
>
>
Richard Henderson Aug. 5, 2021, 7:01 p.m. UTC | #2
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~
LIU Zhiwei Aug. 6, 2021, 2:49 a.m. UTC | #3
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~
diff mbox series

Patch

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)
 
 bool riscv_cpu_is_32bit(CPURISCVState *env);
 
+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;
+}
+
 /*
  * A simplification for VLMAX
  * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
@@ -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));
+    }
 #endif
 
     *pflags = flags;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 076f28b9c1..ac4a545da8 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -67,6 +67,8 @@  typedef struct DisasContext {
     CPUState *cs;
     TCGv zero;
     TCGv sink;
+    /* UXLEN is 32 bit for 64-bit CPU */
+    bool uxl32;
 } DisasContext;
 
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
@@ -912,6 +914,7 @@  static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->mlen = 1 << (ctx->sew  + 3 - ctx->lmul);
     ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
     ctx->cs = cs;
+    ctx->uxl32 = FIELD_EX32(tb_flags, TB_FLAGS, UXL) == 1;
 }
 
 static void riscv_tr_tb_start(DisasContextBase *dcbase, CPUState *cpu)