target/riscv/cpu.h | 2 ++ target/riscv/machine.c | 4 +++ target/riscv/monitor.c | 4 +++ target/riscv/translate.c | 31 +++++++++++++++++++-- target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++ 5 files changed, 41 insertions(+), 2 deletions(-)
One half of 128-bit registers code is not reachable for CPUs
running in 32-bit mode, while the other half is completely
broken (handled as a pair of 32-bit registers).
Better restrict this code on 32-bit builds until RV128 is
properly implemented.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
More than 1y having issue with the 128-bit code path and
the single binary effort. Fortunately while I dunno how to
test 128-bit I could with 32/64 bits.
---
target/riscv/cpu.h | 2 ++
target/riscv/machine.c | 4 +++
target/riscv/monitor.c | 4 +++
target/riscv/translate.c | 31 +++++++++++++++++++--
target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++
5 files changed, 41 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 633d5301f30..94f4daa06e7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -214,7 +214,9 @@ typedef struct PMUFixedCtrState {
struct CPUArchState {
target_ulong gpr[32];
+#if defined(TARGET_RISCV64)
target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
+#endif
/* vector coprocessor state. */
uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 09c032a8791..e220e562ff4 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -168,6 +168,7 @@ static const VMStateDescription vmstate_pointermasking = {
}
};
+#ifdef TARGET_RISCV64
static bool rv128_needed(void *opaque)
{
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
@@ -187,6 +188,7 @@ static const VMStateDescription vmstate_rv128 = {
VMSTATE_END_OF_LIST()
}
};
+#endif
#ifdef CONFIG_KVM
static bool kvmtimer_needed(void *opaque)
@@ -487,7 +489,9 @@ const VMStateDescription vmstate_riscv_cpu = {
&vmstate_hyper,
&vmstate_vector,
&vmstate_pointermasking,
+#ifdef TARGET_RISCV64
&vmstate_rv128,
+#endif
#ifdef CONFIG_KVM
&vmstate_kvmtimer,
#endif
diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index a9d31114442..b99a4e6ec18 100644
--- a/target/riscv/monitor.c
+++ b/target/riscv/monitor.c
@@ -251,8 +251,12 @@ static bool reg_is_ulong_integer(CPURISCVState *env, const char *name,
target_ulong *vals;
if (is_gprh) {
+#ifdef TARGET_RISCV64
reg_names = riscv_int_regnamesh;
vals = env->gprh;
+#else
+ g_assert_not_reached();
+#endif
} else {
reg_names = riscv_int_regnames;
vals = env->gpr;
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 6d0f316ef1e..5b4a9934e83 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -38,7 +38,10 @@
#include "tcg/tcg-cpu.h"
/* global register indices */
-static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
+static TCGv cpu_gpr[32], cpu_pc, cpu_vl, cpu_vstart;
+#ifdef TARGET_RISCV64
+static TCGv cpu_gprh[32];
+#endif
static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
static TCGv load_res;
static TCGv load_val;
@@ -374,11 +377,15 @@ static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
static TCGv get_gprh(DisasContext *ctx, int reg_num)
{
+#ifdef TARGET_RISCV64
assert(get_xl(ctx) == MXL_RV128);
if (reg_num == 0) {
return ctx->zero;
}
return cpu_gprh[reg_num];
+#else
+ g_assert_not_reached();
+#endif
}
static TCGv dest_gpr(DisasContext *ctx, int reg_num)
@@ -391,10 +398,14 @@ static TCGv dest_gpr(DisasContext *ctx, int reg_num)
static TCGv dest_gprh(DisasContext *ctx, int reg_num)
{
+#ifdef TARGET_RISCV64
if (reg_num == 0) {
return tcg_temp_new();
}
return cpu_gprh[reg_num];
+#else
+ g_assert_not_reached();
+#endif
}
static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
@@ -404,17 +415,21 @@ static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
case MXL_RV32:
tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
break;
+#ifdef TARGET_RISCV64
case MXL_RV64:
case MXL_RV128:
tcg_gen_mov_tl(cpu_gpr[reg_num], t);
break;
+#endif
default:
g_assert_not_reached();
}
+#ifdef TARGET_RISCV64
if (get_xl_max(ctx) == MXL_RV128) {
tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
}
+#endif
}
}
@@ -425,27 +440,35 @@ static void gen_set_gpri(DisasContext *ctx, int reg_num, target_long imm)
case MXL_RV32:
tcg_gen_movi_tl(cpu_gpr[reg_num], (int32_t)imm);
break;
+#ifdef TARGET_RISCV64
case MXL_RV64:
case MXL_RV128:
tcg_gen_movi_tl(cpu_gpr[reg_num], imm);
break;
+#endif
default:
g_assert_not_reached();
}
+#ifdef TARGET_RISCV64
if (get_xl_max(ctx) == MXL_RV128) {
tcg_gen_movi_tl(cpu_gprh[reg_num], -(imm < 0));
}
+#endif
}
}
static void gen_set_gpr128(DisasContext *ctx, int reg_num, TCGv rl, TCGv rh)
{
+#ifdef TARGET_RISCV64
assert(get_ol(ctx) == MXL_RV128);
if (reg_num != 0) {
tcg_gen_mov_tl(cpu_gpr[reg_num], rl);
tcg_gen_mov_tl(cpu_gprh[reg_num], rh);
}
+#else
+ g_assert_not_reached();
+#endif
}
static TCGv_i64 get_fpr_hs(DisasContext *ctx, int reg_num)
@@ -1453,14 +1476,18 @@ void riscv_translate_init(void)
* unless you specifically block reads/writes to reg 0.
*/
cpu_gpr[0] = NULL;
- cpu_gprh[0] = NULL;
for (i = 1; i < 32; i++) {
cpu_gpr[i] = tcg_global_mem_new(tcg_env,
offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
+ }
+#ifdef TARGET_RISCV64
+ cpu_gprh[0] = NULL;
+ for (i = 1; i < 32; i++) {
cpu_gprh[i] = tcg_global_mem_new(tcg_env,
offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
}
+#endif
for (i = 0; i < 32; i++) {
cpu_fpr[i] = tcg_global_mem_new_i64(tcg_env,
diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
index 8d94b83ce94..8cf7cbd4599 100644
--- a/target/riscv/insn_trans/trans_rvzacas.c.inc
+++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
@@ -55,10 +55,12 @@ static void gen_set_gpr_pair(DisasContext *ctx, int reg_num, TCGv_i64 t)
tcg_gen_sari_i64(cpu_gpr[reg_num + 1], t, 32);
#endif
+#ifdef TARGET_RISCV64
if (get_xl_max(ctx) == MXL_RV128) {
tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
tcg_gen_sari_tl(cpu_gprh[reg_num + 1], cpu_gpr[reg_num + 1], 63);
}
+#endif
}
}
--
2.53.0
On 3/15/2026 1:59 PM, Philippe Mathieu-Daudé wrote:
> One half of 128-bit registers code is not reachable for CPUs
> running in 32-bit mode, while the other half is completely
> broken (handled as a pair of 32-bit registers).
> Better restrict this code on 32-bit builds until RV128 is
> properly implemented.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
I agree with the premise that if it's not properly implemented for
32 bits we should restrict it.
Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> More than 1y having issue with the 128-bit code path and
> the single binary effort. Fortunately while I dunno how to
> test 128-bit I could with 32/64 bits.
> ---
Not sure how you're tacking this effort in the risc-v front (let me
know if you need help btw) but in a quick glance seems like every
"IF TARGET_RISCV32" could be changed to
get_xl(ctx) == MXL_RV32
And same thing with TARGET_RISCV64 and MXL_RV64. And in theory the same
thing for 128 too - the rv128 CPU will exist in the same binary as the
other ones, ergo it must behave nicely according to get_xl = MXL_128
alone.
Thanks,
Daniel
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 4 +++
> target/riscv/monitor.c | 4 +++
> target/riscv/translate.c | 31 +++++++++++++++++++--
> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++
> 5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 633d5301f30..94f4daa06e7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -214,7 +214,9 @@ typedef struct PMUFixedCtrState {
>
> struct CPUArchState {
> target_ulong gpr[32];
> +#if defined(TARGET_RISCV64)
> target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> +#endif
>
> /* vector coprocessor state. */
> uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 09c032a8791..e220e562ff4 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -168,6 +168,7 @@ static const VMStateDescription vmstate_pointermasking = {
> }
> };
>
> +#ifdef TARGET_RISCV64
> static bool rv128_needed(void *opaque)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
> @@ -187,6 +188,7 @@ static const VMStateDescription vmstate_rv128 = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> #ifdef CONFIG_KVM
> static bool kvmtimer_needed(void *opaque)
> @@ -487,7 +489,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_hyper,
> &vmstate_vector,
> &vmstate_pointermasking,
> +#ifdef TARGET_RISCV64
> &vmstate_rv128,
> +#endif
> #ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> #endif
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index a9d31114442..b99a4e6ec18 100644
> --- a/target/riscv/monitor.c
> +++ b/target/riscv/monitor.c
> @@ -251,8 +251,12 @@ static bool reg_is_ulong_integer(CPURISCVState *env, const char *name,
> target_ulong *vals;
>
> if (is_gprh) {
> +#ifdef TARGET_RISCV64
> reg_names = riscv_int_regnamesh;
> vals = env->gprh;
> +#else
> + g_assert_not_reached();
> +#endif
> } else {
> reg_names = riscv_int_regnames;
> vals = env->gpr;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 6d0f316ef1e..5b4a9934e83 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -38,7 +38,10 @@
> #include "tcg/tcg-cpu.h"
>
> /* global register indices */
> -static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
> +static TCGv cpu_gpr[32], cpu_pc, cpu_vl, cpu_vstart;
> +#ifdef TARGET_RISCV64
> +static TCGv cpu_gprh[32];
> +#endif
> static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> static TCGv load_res;
> static TCGv load_val;
> @@ -374,11 +377,15 @@ static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
>
> static TCGv get_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> assert(get_xl(ctx) == MXL_RV128);
> if (reg_num == 0) {
> return ctx->zero;
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv dest_gpr(DisasContext *ctx, int reg_num)
> @@ -391,10 +398,14 @@ static TCGv dest_gpr(DisasContext *ctx, int reg_num)
>
> static TCGv dest_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> if (reg_num == 0) {
> return tcg_temp_new();
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> @@ -404,17 +415,21 @@ static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> case MXL_RV32:
> tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_mov_tl(cpu_gpr[reg_num], t);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> }
> +#endif
> }
> }
>
> @@ -425,27 +440,35 @@ static void gen_set_gpri(DisasContext *ctx, int reg_num, target_long imm)
> case MXL_RV32:
> tcg_gen_movi_tl(cpu_gpr[reg_num], (int32_t)imm);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_movi_tl(cpu_gpr[reg_num], imm);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_movi_tl(cpu_gprh[reg_num], -(imm < 0));
> }
> +#endif
> }
> }
>
> static void gen_set_gpr128(DisasContext *ctx, int reg_num, TCGv rl, TCGv rh)
> {
> +#ifdef TARGET_RISCV64
> assert(get_ol(ctx) == MXL_RV128);
> if (reg_num != 0) {
> tcg_gen_mov_tl(cpu_gpr[reg_num], rl);
> tcg_gen_mov_tl(cpu_gprh[reg_num], rh);
> }
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv_i64 get_fpr_hs(DisasContext *ctx, int reg_num)
> @@ -1453,14 +1476,18 @@ void riscv_translate_init(void)
> * unless you specifically block reads/writes to reg 0.
> */
> cpu_gpr[0] = NULL;
> - cpu_gprh[0] = NULL;
>
> for (i = 1; i < 32; i++) {
> cpu_gpr[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
> + }
> +#ifdef TARGET_RISCV64
> + cpu_gprh[0] = NULL;
> + for (i = 1; i < 32; i++) {
> cpu_gprh[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
> }
> +#endif
>
> for (i = 0; i < 32; i++) {
> cpu_fpr[i] = tcg_global_mem_new_i64(tcg_env,
> diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
> index 8d94b83ce94..8cf7cbd4599 100644
> --- a/target/riscv/insn_trans/trans_rvzacas.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
> @@ -55,10 +55,12 @@ static void gen_set_gpr_pair(DisasContext *ctx, int reg_num, TCGv_i64 t)
> tcg_gen_sari_i64(cpu_gpr[reg_num + 1], t, 32);
> #endif
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> tcg_gen_sari_tl(cpu_gprh[reg_num + 1], cpu_gpr[reg_num + 1], 63);
> }
> +#endif
> }
> }
>
Hi Phil,
my understanding is that rv128 makes no sense with rv32, and
the user is not expected to run qemu-system-riscv32 with
the -cpu=x-rv128 (it triggers an "unable to find CPU model
'x-rv128'" error).
The rv128 code just relies on XLEN being 128, and assumes
(implicitely I admit) running on a 64-bit host machine, so
with tl = 64.
The code is absolutely erroneous when tl = 32, agreed.
The thing is that, as we want to get rid of the
TARGET_RISCV64/TARGET_RISCV32 defines, the rv128 specific
code still must compile whatever tl, so that qemu can be built.
However, it should never be executed if tl = 32.
Typically, the patches I sent earlier this year to deal
with lq/sq endianness fit into that 'compile only' category
for rv32.
Thanks,
Frédéric
On 3/15/26 17:59, Philippe Mathieu-Daudé wrote:
> One half of 128-bit registers code is not reachable for CPUs
> running in 32-bit mode, while the other half is completely
> broken (handled as a pair of 32-bit registers).
> Better restrict this code on 32-bit builds until RV128 is
> properly implemented.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> More than 1y having issue with the 128-bit code path and
> the single binary effort. Fortunately while I dunno how to
> test 128-bit I could with 32/64 bits.
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 4 +++
> target/riscv/monitor.c | 4 +++
> target/riscv/translate.c | 31 +++++++++++++++++++--
> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++
> 5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 633d5301f30..94f4daa06e7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -214,7 +214,9 @@ typedef struct PMUFixedCtrState {
>
> struct CPUArchState {
> target_ulong gpr[32];
> +#if defined(TARGET_RISCV64)
> target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> +#endif
>
> /* vector coprocessor state. */
> uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 09c032a8791..e220e562ff4 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -168,6 +168,7 @@ static const VMStateDescription vmstate_pointermasking = {
> }
> };
>
> +#ifdef TARGET_RISCV64
> static bool rv128_needed(void *opaque)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
> @@ -187,6 +188,7 @@ static const VMStateDescription vmstate_rv128 = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> #ifdef CONFIG_KVM
> static bool kvmtimer_needed(void *opaque)
> @@ -487,7 +489,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_hyper,
> &vmstate_vector,
> &vmstate_pointermasking,
> +#ifdef TARGET_RISCV64
> &vmstate_rv128,
> +#endif
> #ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> #endif
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index a9d31114442..b99a4e6ec18 100644
> --- a/target/riscv/monitor.c
> +++ b/target/riscv/monitor.c
> @@ -251,8 +251,12 @@ static bool reg_is_ulong_integer(CPURISCVState *env, const char *name,
> target_ulong *vals;
>
> if (is_gprh) {
> +#ifdef TARGET_RISCV64
> reg_names = riscv_int_regnamesh;
> vals = env->gprh;
> +#else
> + g_assert_not_reached();
> +#endif
> } else {
> reg_names = riscv_int_regnames;
> vals = env->gpr;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 6d0f316ef1e..5b4a9934e83 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -38,7 +38,10 @@
> #include "tcg/tcg-cpu.h"
>
> /* global register indices */
> -static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
> +static TCGv cpu_gpr[32], cpu_pc, cpu_vl, cpu_vstart;
> +#ifdef TARGET_RISCV64
> +static TCGv cpu_gprh[32];
> +#endif
> static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> static TCGv load_res;
> static TCGv load_val;
> @@ -374,11 +377,15 @@ static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
>
> static TCGv get_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> assert(get_xl(ctx) == MXL_RV128);
> if (reg_num == 0) {
> return ctx->zero;
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv dest_gpr(DisasContext *ctx, int reg_num)
> @@ -391,10 +398,14 @@ static TCGv dest_gpr(DisasContext *ctx, int reg_num)
>
> static TCGv dest_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> if (reg_num == 0) {
> return tcg_temp_new();
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> @@ -404,17 +415,21 @@ static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> case MXL_RV32:
> tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_mov_tl(cpu_gpr[reg_num], t);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> }
> +#endif
> }
> }
>
> @@ -425,27 +440,35 @@ static void gen_set_gpri(DisasContext *ctx, int reg_num, target_long imm)
> case MXL_RV32:
> tcg_gen_movi_tl(cpu_gpr[reg_num], (int32_t)imm);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_movi_tl(cpu_gpr[reg_num], imm);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_movi_tl(cpu_gprh[reg_num], -(imm < 0));
> }
> +#endif
> }
> }
>
> static void gen_set_gpr128(DisasContext *ctx, int reg_num, TCGv rl, TCGv rh)
> {
> +#ifdef TARGET_RISCV64
> assert(get_ol(ctx) == MXL_RV128);
> if (reg_num != 0) {
> tcg_gen_mov_tl(cpu_gpr[reg_num], rl);
> tcg_gen_mov_tl(cpu_gprh[reg_num], rh);
> }
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv_i64 get_fpr_hs(DisasContext *ctx, int reg_num)
> @@ -1453,14 +1476,18 @@ void riscv_translate_init(void)
> * unless you specifically block reads/writes to reg 0.
> */
> cpu_gpr[0] = NULL;
> - cpu_gprh[0] = NULL;
>
> for (i = 1; i < 32; i++) {
> cpu_gpr[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
> + }
> +#ifdef TARGET_RISCV64
> + cpu_gprh[0] = NULL;
> + for (i = 1; i < 32; i++) {
> cpu_gprh[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
> }
> +#endif
>
> for (i = 0; i < 32; i++) {
> cpu_fpr[i] = tcg_global_mem_new_i64(tcg_env,
> diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
> index 8d94b83ce94..8cf7cbd4599 100644
> --- a/target/riscv/insn_trans/trans_rvzacas.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
> @@ -55,10 +55,12 @@ static void gen_set_gpr_pair(DisasContext *ctx, int reg_num, TCGv_i64 t)
> tcg_gen_sari_i64(cpu_gpr[reg_num + 1], t, 32);
> #endif
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> tcg_gen_sari_tl(cpu_gprh[reg_num + 1], cpu_gpr[reg_num + 1], 63);
> }
> +#endif
> }
> }
>
--
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-UGA@Ensimag/TIMA |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
+---------------------------------------------------------------------------+
Hi Frédéric, On 15/3/26 19:55, Frédéric Pétrot wrote: > Hi Phil, > > my understanding is that rv128 makes no sense with rv32, and > the user is not expected to run qemu-system-riscv32 with > the -cpu=x-rv128 (it triggers an "unable to find CPU model > 'x-rv128'" error). > The rv128 code just relies on XLEN being 128, and assumes > (implicitely I admit) running on a 64-bit host machine, so > with tl = 64. > The code is absolutely erroneous when tl = 32, agreed. > The thing is that, as we want to get rid of the > TARGET_RISCV64/TARGET_RISCV32 defines, the rv128 specific > code still must compile whatever tl, so that qemu can be built. > However, it should never be executed if tl = 32. > Typically, the patches I sent earlier this year to deal > with lq/sq endianness fit into that 'compile only' category > for rv32. I'd really like to include your patch which fixes the endianness issue I reported, but unfortunately as reported it doesn't even build for rv32 builds: https://lore.kernel.org/qemu-devel/ad717d8c-a6b9-4dd8-a4bd-fdd6d1d756a5@linaro.org/ Which is why I'm trying to restrict RV128 on rv32 binary so we can apply your other patch. Anyway, Daniel suggestions to use 'get_xl(ctx) == MXL_RV32' instead of this ugly TARGET_RISCV32 #ifdef'ry is certainly the way to go (which is why I posted this as RFC). > > Thanks, > Frédéric > > On 3/15/26 17:59, Philippe Mathieu-Daudé wrote: >> One half of 128-bit registers code is not reachable for CPUs >> running in 32-bit mode, while the other half is completely >> broken (handled as a pair of 32-bit registers). >> Better restrict this code on 32-bit builds until RV128 is >> properly implemented. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> More than 1y having issue with the 128-bit code path and >> the single binary effort. Fortunately while I dunno how to >> test 128-bit I could with 32/64 bits. >> --- >> target/riscv/cpu.h | 2 ++ >> target/riscv/machine.c | 4 +++ >> target/riscv/monitor.c | 4 +++ >> target/riscv/translate.c | 31 +++++++++++++++++++-- >> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++ >> 5 files changed, 41 insertions(+), 2 deletions(-)
Thanks Phil, On 3/17/26 11:05, Philippe Mathieu-Daudé wrote: > Hi Frédéric, > > On 15/3/26 19:55, Frédéric Pétrot wrote: >> Hi Phil, >> >> my understanding is that rv128 makes no sense with rv32, and >> the user is not expected to run qemu-system-riscv32 with >> the -cpu=x-rv128 (it triggers an "unable to find CPU model >> 'x-rv128'" error). >> The rv128 code just relies on XLEN being 128, and assumes >> (implicitely I admit) running on a 64-bit host machine, so >> with tl = 64. >> The code is absolutely erroneous when tl = 32, agreed. >> The thing is that, as we want to get rid of the >> TARGET_RISCV64/TARGET_RISCV32 defines, the rv128 specific >> code still must compile whatever tl, so that qemu can be built. >> However, it should never be executed if tl = 32. >> Typically, the patches I sent earlier this year to deal >> with lq/sq endianness fit into that 'compile only' category >> for rv32. > > I'd really like to include your patch which fixes the endianness > issue I reported, but unfortunately as reported it doesn't even > build for rv32 builds: > > https://lore.kernel.org/qemu-devel/ad717d8c-a6b9-4dd8-a4bd-fdd6d1d756a5@linaro.org/ Very sorry for that. I took care of the errors in a later patch you might have missed : https://lore.kernel.org/qemu-devel/20260101181442.2489496-1-frederic.petrot@univ-grenoble-alpes.fr/ This compiles fine on Alister current riscv-to-apply.next branch for both 32 and 64 bit versions (known that the 32 bit version does nothing useful, but is never used anyway). > > Which is why I'm trying to restrict RV128 on rv32 binary so > we can apply your other patch. > > Anyway, Daniel suggestions to use 'get_xl(ctx) == MXL_RV32' > instead of this ugly TARGET_RISCV32 #ifdef'ry is certainly > the way to go (which is why I posted this as RFC). It might, indeed. Thanks again for your time, Frédéric > >> >> Thanks, >> Frédéric >> >> On 3/15/26 17:59, Philippe Mathieu-Daudé wrote: >>> One half of 128-bit registers code is not reachable for CPUs >>> running in 32-bit mode, while the other half is completely >>> broken (handled as a pair of 32-bit registers). >>> Better restrict this code on 32-bit builds until RV128 is >>> properly implemented. >>> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> --- >>> More than 1y having issue with the 128-bit code path and >>> the single binary effort. Fortunately while I dunno how to >>> test 128-bit I could with 32/64 bits. >>> --- >>> target/riscv/cpu.h | 2 ++ >>> target/riscv/machine.c | 4 +++ >>> target/riscv/monitor.c | 4 +++ >>> target/riscv/translate.c | 31 +++++++++++++++++++-- >>> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++ >>> 5 files changed, 41 insertions(+), 2 deletions(-) > > > > > -- +---------------------------------------------------------------------------+ | Frédéric Pétrot, Pr. Grenoble INP-UGA@Ensimag/TIMA | | Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70 Ad augusta per angusta | | http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr | +---------------------------------------------------------------------------+
On 17/3/26 17:48, Frédéric Pétrot wrote: > Thanks Phil, > > On 3/17/26 11:05, Philippe Mathieu-Daudé wrote: >> Hi Frédéric, >> >> On 15/3/26 19:55, Frédéric Pétrot wrote: >>> Hi Phil, >>> >>> my understanding is that rv128 makes no sense with rv32, and >>> the user is not expected to run qemu-system-riscv32 with >>> the -cpu=x-rv128 (it triggers an "unable to find CPU model >>> 'x-rv128'" error). >>> The rv128 code just relies on XLEN being 128, and assumes >>> (implicitely I admit) running on a 64-bit host machine, so >>> with tl = 64. >>> The code is absolutely erroneous when tl = 32, agreed. >>> The thing is that, as we want to get rid of the >>> TARGET_RISCV64/TARGET_RISCV32 defines, the rv128 specific >>> code still must compile whatever tl, so that qemu can be built. >>> However, it should never be executed if tl = 32. >>> Typically, the patches I sent earlier this year to deal >>> with lq/sq endianness fit into that 'compile only' category >>> for rv32. >> >> I'd really like to include your patch which fixes the endianness >> issue I reported, but unfortunately as reported it doesn't even >> build for rv32 builds: >> >> https://lore.kernel.org/qemu-devel/ad717d8c-a6b9-4dd8-a4bd- >> fdd6d1d756a5@linaro.org/ > > Very sorry for that. > I took care of the errors in a later patch you might have > missed : > > https://lore.kernel.org/qemu-devel/20260101181442.2489496-1- > frederic.petrot@univ-grenoble-alpes.fr/ Oh indeed this isn't in my mailbox... Fortunately the list got it so I could import it. > > This compiles fine on Alister current riscv-to-apply.next > branch for both 32 and 64 bit versions (known that the > 32 bit version does nothing useful, but is never used > anyway). Yep, this is sufficient to fixes my issues and let me continue! > >> >> Which is why I'm trying to restrict RV128 on rv32 binary so >> we can apply your other patch. >> >> Anyway, Daniel suggestions to use 'get_xl(ctx) == MXL_RV32' >> instead of this ugly TARGET_RISCV32 #ifdef'ry is certainly >> the way to go (which is why I posted this as RFC). > > It might, indeed. > Thanks again for your time, Sorry if I sounded grumpy ;) > Frédéric > >> >>> >>> Thanks, >>> Frédéric >>> >>> On 3/15/26 17:59, Philippe Mathieu-Daudé wrote: >>>> One half of 128-bit registers code is not reachable for CPUs >>>> running in 32-bit mode, while the other half is completely >>>> broken (handled as a pair of 32-bit registers). >>>> Better restrict this code on 32-bit builds until RV128 is >>>> properly implemented. >>>> >>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>>> --- >>>> More than 1y having issue with the 128-bit code path and >>>> the single binary effort. Fortunately while I dunno how to >>>> test 128-bit I could with 32/64 bits. >>>> --- >>>> target/riscv/cpu.h | 2 ++ >>>> target/riscv/machine.c | 4 +++ >>>> target/riscv/monitor.c | 4 +++ >>>> target/riscv/translate.c | 31 +++++++++++++++++ >>>> ++-- >>>> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++ >>>> 5 files changed, 41 insertions(+), 2 deletions(-) >> >> >> >> >> >
On 15/3/26 17:59, Philippe Mathieu-Daudé wrote:
> One half of 128-bit registers code is not reachable for CPUs
> running in 32-bit mode, while the other half is completely
> broken (handled as a pair of 32-bit registers).
> Better restrict this code on 32-bit builds until RV128 is
> properly implemented.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> More than 1y having issue with the 128-bit code path and
> the single binary effort. Fortunately while I dunno how to
> test 128-bit I could with 32/64 bits.
> ---
> target/riscv/cpu.h | 2 ++
> target/riscv/machine.c | 4 +++
> target/riscv/monitor.c | 4 +++
> target/riscv/translate.c | 31 +++++++++++++++++++--
> target/riscv/insn_trans/trans_rvzacas.c.inc | 2 ++
> 5 files changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 633d5301f30..94f4daa06e7 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -214,7 +214,9 @@ typedef struct PMUFixedCtrState {
>
> struct CPUArchState {
> target_ulong gpr[32];
> +#if defined(TARGET_RISCV64)
> target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> +#endif
>
> /* vector coprocessor state. */
> uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 09c032a8791..e220e562ff4 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -168,6 +168,7 @@ static const VMStateDescription vmstate_pointermasking = {
> }
> };
>
> +#ifdef TARGET_RISCV64
> static bool rv128_needed(void *opaque)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
> @@ -187,6 +188,7 @@ static const VMStateDescription vmstate_rv128 = {
> VMSTATE_END_OF_LIST()
> }
> };
> +#endif
>
> #ifdef CONFIG_KVM
> static bool kvmtimer_needed(void *opaque)
> @@ -487,7 +489,9 @@ const VMStateDescription vmstate_riscv_cpu = {
> &vmstate_hyper,
> &vmstate_vector,
> &vmstate_pointermasking,
> +#ifdef TARGET_RISCV64
> &vmstate_rv128,
> +#endif
> #ifdef CONFIG_KVM
> &vmstate_kvmtimer,
> #endif
> diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> index a9d31114442..b99a4e6ec18 100644
> --- a/target/riscv/monitor.c
> +++ b/target/riscv/monitor.c
> @@ -251,8 +251,12 @@ static bool reg_is_ulong_integer(CPURISCVState *env, const char *name,
> target_ulong *vals;
>
> if (is_gprh) {
> +#ifdef TARGET_RISCV64
> reg_names = riscv_int_regnamesh;
> vals = env->gprh;
> +#else
> + g_assert_not_reached();
> +#endif
> } else {
> reg_names = riscv_int_regnames;
> vals = env->gpr;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 6d0f316ef1e..5b4a9934e83 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -38,7 +38,10 @@
> #include "tcg/tcg-cpu.h"
>
> /* global register indices */
> -static TCGv cpu_gpr[32], cpu_gprh[32], cpu_pc, cpu_vl, cpu_vstart;
> +static TCGv cpu_gpr[32], cpu_pc, cpu_vl, cpu_vstart;
> +#ifdef TARGET_RISCV64
> +static TCGv cpu_gprh[32];
> +#endif
> static TCGv_i64 cpu_fpr[32]; /* assume F and D extensions */
> static TCGv load_res;
> static TCGv load_val;
> @@ -374,11 +377,15 @@ static TCGv get_gpr(DisasContext *ctx, int reg_num, DisasExtend ext)
>
> static TCGv get_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> assert(get_xl(ctx) == MXL_RV128);
> if (reg_num == 0) {
> return ctx->zero;
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv dest_gpr(DisasContext *ctx, int reg_num)
> @@ -391,10 +398,14 @@ static TCGv dest_gpr(DisasContext *ctx, int reg_num)
>
> static TCGv dest_gprh(DisasContext *ctx, int reg_num)
> {
> +#ifdef TARGET_RISCV64
> if (reg_num == 0) {
> return tcg_temp_new();
> }
> return cpu_gprh[reg_num];
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> @@ -404,17 +415,21 @@ static void gen_set_gpr(DisasContext *ctx, int reg_num, TCGv t)
> case MXL_RV32:
> tcg_gen_ext32s_tl(cpu_gpr[reg_num], t);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_mov_tl(cpu_gpr[reg_num], t);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> }
> +#endif
> }
> }
>
> @@ -425,27 +440,35 @@ static void gen_set_gpri(DisasContext *ctx, int reg_num, target_long imm)
> case MXL_RV32:
> tcg_gen_movi_tl(cpu_gpr[reg_num], (int32_t)imm);
> break;
> +#ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> tcg_gen_movi_tl(cpu_gpr[reg_num], imm);
> break;
> +#endif
> default:
> g_assert_not_reached();
> }
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_movi_tl(cpu_gprh[reg_num], -(imm < 0));
> }
> +#endif
> }
> }
>
> static void gen_set_gpr128(DisasContext *ctx, int reg_num, TCGv rl, TCGv rh)
> {
> +#ifdef TARGET_RISCV64
> assert(get_ol(ctx) == MXL_RV128);
> if (reg_num != 0) {
> tcg_gen_mov_tl(cpu_gpr[reg_num], rl);
> tcg_gen_mov_tl(cpu_gprh[reg_num], rh);
> }
> +#else
> + g_assert_not_reached();
> +#endif
> }
>
> static TCGv_i64 get_fpr_hs(DisasContext *ctx, int reg_num)
> @@ -1453,14 +1476,18 @@ void riscv_translate_init(void)
> * unless you specifically block reads/writes to reg 0.
> */
> cpu_gpr[0] = NULL;
> - cpu_gprh[0] = NULL;
>
> for (i = 1; i < 32; i++) {
> cpu_gpr[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
> + }
> +#ifdef TARGET_RISCV64
> + cpu_gprh[0] = NULL;
> + for (i = 1; i < 32; i++) {
> cpu_gprh[i] = tcg_global_mem_new(tcg_env,
> offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
> }
> +#endif
>
> for (i = 0; i < 32; i++) {
> cpu_fpr[i] = tcg_global_mem_new_i64(tcg_env,
> diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc b/target/riscv/insn_trans/trans_rvzacas.c.inc
> index 8d94b83ce94..8cf7cbd4599 100644
> --- a/target/riscv/insn_trans/trans_rvzacas.c.inc
> +++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
> @@ -55,10 +55,12 @@ static void gen_set_gpr_pair(DisasContext *ctx, int reg_num, TCGv_i64 t)
> tcg_gen_sari_i64(cpu_gpr[reg_num + 1], t, 32);
> #endif
>
> +#ifdef TARGET_RISCV64
> if (get_xl_max(ctx) == MXL_RV128) {
> tcg_gen_sari_tl(cpu_gprh[reg_num], cpu_gpr[reg_num], 63);
> tcg_gen_sari_tl(cpu_gprh[reg_num + 1], cpu_gpr[reg_num + 1], 63);
> }
> +#endif
> }
> }
Doesn't matter much, but I forgot to commit this hunk:
-- >8 --
diff --git a/target/riscv/insn_trans/trans_rvzacas.c.inc
b/target/riscv/insn_trans/trans_rvzacas.c.inc
index 8cf7cbd4599..dc67a027f86 100644
--- a/target/riscv/insn_trans/trans_rvzacas.c.inc
+++ b/target/riscv/insn_trans/trans_rvzacas.c.inc
@@ -92,9 +92,11 @@ static bool trans_amocas_d(DisasContext *ctx,
arg_amocas_d *a)
switch (get_ol(ctx)) {
case MXL_RV32:
return gen_cmpxchg64(ctx, a, MO_ALIGN | MO_UQ);
+#ifdef TARGET_RISCV64
case MXL_RV64:
case MXL_RV128:
return gen_cmpxchg(ctx, a, MO_ALIGN | MO_UQ);
+#endif
default:
g_assert_not_reached();
}
---
© 2016 - 2026 Red Hat, Inc.