On Wed, Oct 15, 2025 at 6:36 AM Anton Johansson via
<[email protected]> wrote:
>
> gprh is only needed for TARGET_RISCV64 when modeling 128-bit registers,
> fixing their size to 64 bits makes sense.
>
> gpr is also fixed to 64 bits since all direct uses of env->gpr
> correctly zero extend/truncate to/from target_ulong, meaning
> !TARGET_RISCV64 will behave as expected.
>
> We do however need to be a bit careful when mapping 64-bit fields to
> 32-bit TCGv globals on big endian hosts.
>
> Note, the cpu/rv128 VMSTATE version is bumped, breaking migration from
> older versions.
>
> Signed-off-by: Anton Johansson <[email protected]>

Acked-by: Alistair Francis <[email protected]>

Alistair

> ---
>  target/riscv/cpu.h            |  4 ++--
>  target/riscv/cpu.c            |  2 +-
>  target/riscv/machine.c        |  8 ++++----
>  target/riscv/riscv-qmp-cmds.c |  2 +-
>  target/riscv/translate.c      | 17 +++++++++++++++--
>  5 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 09d9e4c33c..7573d5aa7e 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -212,8 +212,8 @@ typedef struct PMUFixedCtrState {
>  } PMUFixedCtrState;
>
>  struct CPUArchState {
> -    target_ulong gpr[32];
> -    target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
> +    uint64_t gpr[32];
> +    uint64_t gprh[32]; /* 64 top bits of the 128-bit registers */
>
>      /* vector coprocessor state. */
>      uint64_t vreg[32 * RV_VLEN_MAX / 64] QEMU_ALIGNED(16);
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index a877018ab0..b7690ac00f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -583,7 +583,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, 
> int flags)
>  #endif
>
>      for (i = 0; i < 32; i++) {
> -        qemu_fprintf(f, " %-8s " TARGET_FMT_lx,
> +        qemu_fprintf(f, " %-8s %" PRIx64,
>                       riscv_int_regnames[i], env->gpr[i]);
>          if ((i & 3) == 3) {
>              qemu_fprintf(f, "\n");
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 09c032a879..7349383eab 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -177,11 +177,11 @@ static bool rv128_needed(void *opaque)
>
>  static const VMStateDescription vmstate_rv128 = {
>      .name = "cpu/rv128",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = rv128_needed,
>      .fields = (const VMStateField[]) {
> -        VMSTATE_UINTTL_ARRAY(env.gprh, RISCVCPU, 32),
> +        VMSTATE_UINT64_ARRAY(env.gprh, RISCVCPU, 32),
>          VMSTATE_UINT64(env.mscratchh, RISCVCPU),
>          VMSTATE_UINT64(env.sscratchh, RISCVCPU),
>          VMSTATE_END_OF_LIST()
> @@ -429,7 +429,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>      .minimum_version_id = 11,
>      .post_load = riscv_cpu_post_load,
>      .fields = (const VMStateField[]) {
> -        VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
> +        VMSTATE_UINT64_ARRAY(env.gpr, RISCVCPU, 32),
>          VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32),
>          VMSTATE_UINT8_ARRAY(env.miprio, RISCVCPU, 64),
>          VMSTATE_UINT8_ARRAY(env.siprio, RISCVCPU, 64),
> diff --git a/target/riscv/riscv-qmp-cmds.c b/target/riscv/riscv-qmp-cmds.c
> index c499f9b9a7..95fa713c69 100644
> --- a/target/riscv/riscv-qmp-cmds.c
> +++ b/target/riscv/riscv-qmp-cmds.c
> @@ -262,7 +262,7 @@ static bool reg_is_ulong_integer(CPURISCVState *env, 
> const char *name,
>                                   target_ulong *val, bool is_gprh)
>  {
>      const char * const *reg_names;
> -    target_ulong *vals;
> +    uint64_t *vals;
>
>      if (is_gprh) {
>          reg_names = riscv_int_regnamesh;
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 6fc06c71f5..4308b7712e 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -27,6 +27,7 @@
>  #include "accel/tcg/cpu-ldst.h"
>  #include "exec/translation-block.h"
>  #include "exec/log.h"
> +#include "exec/tswap.h"
>  #include "semihosting/semihost.h"
>
>  #include "internals.h"
> @@ -1428,12 +1429,24 @@ void riscv_translate_init(void)
>       */
>      cpu_gpr[0] = NULL;
>      cpu_gprh[0] = NULL;
> +    /*
> +     * Be careful with big endian hosts when mapping 64-bit CPUArchState 
> fields
> +     * to 32-bit TCGv globals.  An offset of 4 bytes is applied so the least
> +     * significant bytes are correctly written to.
> +     */
> +#if HOST_BIG_ENDIAN && !defined(TARGET_RISCV64)
> +    size_t field_offset = 4;
> +#else
> +    size_t field_offset = 0;
> +#endif
>
>      for (i = 1; i < 32; i++) {
>          cpu_gpr[i] = tcg_global_mem_new(tcg_env,
> -            offsetof(CPURISCVState, gpr[i]), riscv_int_regnames[i]);
> +            offsetof(CPURISCVState, gpr[i]) + field_offset,
> +            riscv_int_regnames[i]);
>          cpu_gprh[i] = tcg_global_mem_new(tcg_env,
> -            offsetof(CPURISCVState, gprh[i]), riscv_int_regnamesh[i]);
> +            offsetof(CPURISCVState, gprh[i]) + field_offset,
> +            riscv_int_regnamesh[i]);
>      }
>
>      for (i = 0; i < 32; i++) {
> --
> 2.51.0
>
>

Reply via email to