* [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
@ 2026-03-15 16:59 Philippe Mathieu-Daudé
2026-03-15 17:14 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-15 16:59 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Frédéric Pétrot, Daniel Henrique Barboza,
Alistair Francis, Palmer Dabbelt, Philippe Mathieu-Daudé
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-15 16:59 [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32 Philippe Mathieu-Daudé
@ 2026-03-15 17:14 ` Philippe Mathieu-Daudé
2026-03-15 18:55 ` Frédéric Pétrot
2026-03-16 9:31 ` Daniel Henrique Barboza
2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-15 17:14 UTC (permalink / raw)
To: qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Frédéric Pétrot, Daniel Henrique Barboza,
Alistair Francis, Palmer Dabbelt
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();
}
---
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-15 16:59 [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32 Philippe Mathieu-Daudé
2026-03-15 17:14 ` Philippe Mathieu-Daudé
@ 2026-03-15 18:55 ` Frédéric Pétrot
2026-03-17 10:05 ` Philippe Mathieu-Daudé
2026-03-16 9:31 ` Daniel Henrique Barboza
2 siblings, 1 reply; 7+ messages in thread
From: Frédéric Pétrot @ 2026-03-15 18:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Daniel Henrique Barboza, Alistair Francis, Palmer Dabbelt
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 |
+---------------------------------------------------------------------------+
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-15 16:59 [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32 Philippe Mathieu-Daudé
2026-03-15 17:14 ` Philippe Mathieu-Daudé
2026-03-15 18:55 ` Frédéric Pétrot
@ 2026-03-16 9:31 ` Daniel Henrique Barboza
2 siblings, 0 replies; 7+ messages in thread
From: Daniel Henrique Barboza @ 2026-03-16 9:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Frédéric Pétrot, Alistair Francis, Palmer Dabbelt
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
> }
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-15 18:55 ` Frédéric Pétrot
@ 2026-03-17 10:05 ` Philippe Mathieu-Daudé
2026-03-17 16:48 ` Frédéric Pétrot
0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-17 10:05 UTC (permalink / raw)
To: Frédéric Pétrot, qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Daniel Henrique Barboza, Alistair Francis, Palmer Dabbelt
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(-)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-17 10:05 ` Philippe Mathieu-Daudé
@ 2026-03-17 16:48 ` Frédéric Pétrot
2026-03-18 10:09 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 7+ messages in thread
From: Frédéric Pétrot @ 2026-03-17 16:48 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Daniel Henrique Barboza, Alistair Francis, Palmer Dabbelt
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 |
+---------------------------------------------------------------------------+
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32
2026-03-17 16:48 ` Frédéric Pétrot
@ 2026-03-18 10:09 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2026-03-18 10:09 UTC (permalink / raw)
To: Frédéric Pétrot, qemu-devel
Cc: Pierrick Bouvier, qemu-riscv, Weiwei Li, Liu Zhiwei,
Daniel Henrique Barboza, Alistair Francis, Palmer Dabbelt
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(-)
>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-18 10:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-15 16:59 [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32 Philippe Mathieu-Daudé
2026-03-15 17:14 ` Philippe Mathieu-Daudé
2026-03-15 18:55 ` Frédéric Pétrot
2026-03-17 10:05 ` Philippe Mathieu-Daudé
2026-03-17 16:48 ` Frédéric Pétrot
2026-03-18 10:09 ` Philippe Mathieu-Daudé
2026-03-16 9:31 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox