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 <tangtiancheng....@alibaba-inc.com>
Ensure that riscv_cpu_sxl returns MXL_RV32 when runningRV32 in an
RV64 QEMU.
Signed-off-by: TANG Tiancheng <tangtiancheng....@alibaba-inc.com>
Fixes: 05e6ca5e156 ("target/riscv: Ignore reserved bits in PTE for
RV64")
Reviewed-by: Liu Zhiwei <zhiwei_...@linux.alibaba.com>
---
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