Re: [PATCH 3/6] target/riscv: Correct SXL return value for RV32 in RV64 QEMU

2024-07-01 Thread LIU Zhiwei


On 2024/7/1 23:10, Philippe Mathieu-Daudé wrote:

Hi Tiancheng, Zhiwei,

On 1/7/24 05:37, LIU Zhiwei wrote:

From: TANG Tiancheng 

Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an
RV64 QEMU.

Signed-off-by: TANG Tiancheng 
Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for 
RV64")

Reviewed-by: Liu Zhiwei 
---
  target/riscv/cpu.h | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..36a712044a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -668,8 +668,11 @@ static inline RISCVMXL 
riscv_cpu_sxl(CPURISCVState *env)

  #ifdef CONFIG_USER_ONLY
  return env->misa_mxl;
  #else
-    return get_field(env->mstatus, MSTATUS64_SXL);
+    if (env->misa_mxl != MXL_RV32) {
+    return get_field(env->mstatus, MSTATUS64_SXL);
+    }
  #endif
+    return MXL_RV32;


Can we simplify the previous TARGET_RISCV32 ifdef'ry?


I think you mean the TARGET_RISCV32 macro here:

#ifdef TARGET_RISCV32
#define riscv_cpu_sxl(env)  ((void)(env), MXL_RV32)
#else
static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env)
{
#ifdef CONFIG_USER_ONLY
    return env->misa_mxl;
#else
    return get_field(env->mstatus, MSTATUS64_SXL);
#endif
}
#endif

I think it is better to keep it here for better performance(as a constant).

If you mean whether we  can simplify all the ifdef TARGET_RISCV32 in 
target/riscv or hw/riscv, IMHO, it depends. I git grep the 
TARGET_RISCV32 from target/riscv:


target/riscv/cpu-param.h:#elif defined(TARGET_RISCV32)

target/riscv/cpu.c:#ifdef TARGET_RISCV32

target/riscv/cpu.c:#if defined(TARGET_RISCV32)

target/riscv/cpu.h:#if defined(TARGET_RISCV32)

target/riscv/cpu.h:#ifdef TARGET_RISCV32

target/riscv/cpu.h:#if defined(TARGET_RISCV32)

target/riscv/cpu.h:#if defined(TARGET_RISCV32)

target/riscv/cpu.h:#ifdef TARGET_RISCV32

target/riscv/insn_trans/trans_rvzacas.c.inc:#ifdef TARGET_RISCV32

target/riscv/kvm/kvm-cpu.c:#if defined(TARGET_RISCV32)

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

target/riscv/translate.c:#ifdef TARGET_RISCV32

One case we can remove the TARGET_RISCV32 is in translate.c.

static void gen_set_fpr_hs(DisasContext *ctx, int reg_num, TCGv_i64 t)

{

    if (!ctx->cfg_ptr->ext_zfinx) {

    tcg_gen_mov_i64(cpu_fpr[reg_num], t);

    return;

    }

    if (reg_num != 0) {

    switch (get_xl(ctx)) {

    case MXL_RV32:

#ifdef TARGET_RISCV32

    tcg_gen_extrl_i64_i32(cpu_gpr[reg_num], t);

    break;

#else

    /* fall through */

    case MXL_RV64:

    tcg_gen_mov_i64(cpu_gpr[reg_num], t);

    break;

#endif

    default:

    g_assert_not_reached();

    }

    }

}

We can simplify this code by tcg_gen_trunc_i64_tl.

One case we can't remove the TARGET_RISCV32 is in cpu.c, where we define 
any or max cpu with the width depending on this macro.
I don't analyze all TARGET_RISCV32 using here.  We will upstream a 
standalone patch for validating all its using later.


Thanks,
Zhiwei




  }
  #endif

Re: [PATCH 3/6] target/riscv: Correct SXL return value for RV32 in RV64 QEMU

2024-07-01 Thread Philippe Mathieu-Daudé

Hi Tiancheng, Zhiwei,

On 1/7/24 05:37, LIU Zhiwei wrote:

From: TANG Tiancheng 

Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an
RV64 QEMU.

Signed-off-by: TANG Tiancheng 
Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for RV64")
Reviewed-by: Liu Zhiwei 
---
  target/riscv/cpu.h | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..36a712044a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -668,8 +668,11 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env)
  #ifdef CONFIG_USER_ONLY
  return env->misa_mxl;
  #else
-return get_field(env->mstatus, MSTATUS64_SXL);
+if (env->misa_mxl != MXL_RV32) {
+return get_field(env->mstatus, MSTATUS64_SXL);
+}
  #endif
+return MXL_RV32;


Can we simplify the previous TARGET_RISCV32 ifdef'ry?


  }
  #endif
  





[PATCH 3/6] target/riscv: Correct SXL return value for RV32 in RV64 QEMU

2024-06-30 Thread LIU Zhiwei
From: TANG Tiancheng 

Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an
RV64 QEMU.

Signed-off-by: TANG Tiancheng 
Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for RV64")
Reviewed-by: Liu Zhiwei 
---
 target/riscv/cpu.h | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..36a712044a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -668,8 +668,11 @@ static inline RISCVMXL riscv_cpu_sxl(CPURISCVState *env)
 #ifdef CONFIG_USER_ONLY
 return env->misa_mxl;
 #else
-return get_field(env->mstatus, MSTATUS64_SXL);
+if (env->misa_mxl != MXL_RV32) {
+return get_field(env->mstatus, MSTATUS64_SXL);
+}
 #endif
+return MXL_RV32;
 }
 #endif
 
-- 
2.43.0