Re: [RFC PATCH] target/riscv: Restrict access to RV128 registers on RV32

[Daniel Henrique Barboza]

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é <phi...@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.barb...@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
      }
  }