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

Sun, 15 Mar 2026 11:57:27 -0700 

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é <[email protected]>
---
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 [email protected] |
+---------------------------------------------------------------------------+

Reply via email to