Re: [RFC PATCH 00/34] single-binary: Make riscv cpu.h target independent

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

Hi Anton,

thanks for joining the single-binary effort.
You didn't start with the easiest part :).

On 9/24/25 12:20 AM, Anton Johansson wrote:
> Hi,
> 
> this is a first patchset moving towards single-binary support for riscv.
> Additional patchsets for hw/ and target/ are based on this one so it's
> best to make sure the approach taken is ok.  Most patches in this set
> concern fields in CPUArchState which are either widened (usually to
> uint64_t) or fixed to a smaller size which handles all use cases.
> 
> General purpose registers and fields mapped to TCG are dealt with by
> widening the type and applying an offset to tcg_global_mem_new() to
> correctly handle 32-bit targets on big endian hosts.
> 
> Quick question to correct my understanding. AFAICT riscv64-softmmu is a
> superset of riscv32-softmmu which handles 32-, 64, and 128-bit ISAs, so
> concerning single-binary do we for the time being only need to support
> riscv64-softmmu?
>

Changes to riscv64 will probably impact riscv32 anyway, as they have 
files in common (that's the tricky part of single binary porting effort).

A possible solution to this could be to duplicate (manually) compilation 
units between 32 and 64 bits variants, but I don't think it's worth the 
effort. It's better to do the change for all variants for a given 
architecture so all the code is cleaned for a given base architecture.

> Let me know what you think of the direction taken here and if you would
> prefer something else.
> 

Overall, most of the changes you do are correct, widening types, and 
combining low/high part in single 64 bits integer. For this last part, 
I'll let Riscv maintainers decide if they agree with the approach.

However, the main issue is that changing size impacts migration 
(VMSTATE_UINT.*), which would suddenly create a breaking change when 
importing 32 bits machines.

We have two ways to deal with it:
1. call it a day and make a breaking change.
On Arm side, we were lucky to not have cpu structure defined with any 
target type, so the problem was not found yet. But we observed that 
other architectures would need a solution.
If Riscv maintainers are ok for a breaking change, this is the 
easiest/fastest solution.

2. introduce a backward compatibility change, to selectively import size 
if we import from a past QEMU version. In this case, we would keep the 
VMSTATE_UINTTL but adapt it to do the right thing when restoring, either 
writing 32 or 64 bits.
I didn't dive deep enough in migration restore code to see if it's 
easily doable, or not.

Let's see what people think of 1. first. On arm, this is very 
conservative and migration backward compatibility has to work anyway. 
Maybe it's more relaxed for riscv.

> Anton Johansson (34):
>    target/riscv: Use 32 bits for misa extensions
>    target/riscv: Fix size of trivial CPUArchState fields
>    target/riscv: Fix size of mcause
>    target/riscv: Fix size of mhartid
>    target/riscv: Bugfix riscv_pmu_ctr_get_fixed_counters_val()
>    target/riscv: Combine mhpmevent and mhpmeventh
>    target/riscv: Combine mcyclecfg and mcyclecfgh
>    target/riscv: Combine minstretcfg and minstretcfgh
>    target/riscv: Combine mhpmcounter and mhpmcounterh
>    target/riscv: Fix size of gpr and gprh
>    target/riscv: Fix size of vector CSRs
>    target/riscv: Fix size of pc, load_[val|res]
>    target/riscv: Fix size of frm and fflags
>    target/riscv: Fix size of badaddr and bins
>    target/riscv: Fix size of guest_phys_fault_addr
>    target/riscv: Fix size of priv_ver and vext_ver
>    target/riscv: Fix size of retxh
>    target/riscv: Fix size of ssp
>    target/riscv: Fix size of excp_uw2
>    target/riscv: Fix size of sw_check_code
>    target/riscv: Fix size of priv
>    target/riscv: Fix size of gei fields
>    target/riscv: Fix size of [m|s|vs]iselect fields
>    target/riscv: Fix arguments to board IMSIC emulation callbacks
>    target/riscv: Fix size of irq_overflow_left
>    target/riscv: Indent PMUFixedCtrState correctly
>    target/riscv: Replace target_ulong in riscv_cpu_get_trap_name()
>    target/riscv: Replace target_ulong in riscv_ctr_add_entry()
>    target/riscv: Fix size of trigger data
>    target/riscv: Fix size of mseccfg
>    target/riscv: Move debug.h include away from cpu.h
>    target/riscv: Move CSR declarations to separate csr.h header
>    target/riscv: Introduce externally facing CSR access functions
>    target/riscv: Make pmp.h target_ulong agnostic
> 
>   target/riscv/cpu.h                            | 341 +++++++-----------
>   target/riscv/csr.h                            |  93 +++++
>   target/riscv/debug.h                          |   2 -
>   target/riscv/pmp.h                            |  20 +-
>   hw/intc/riscv_imsic.c                         |  34 +-
>   hw/riscv/riscv_hart.c                         |   7 +-
>   linux-user/riscv/signal.c                     |   5 +-
>   target/riscv/cpu.c                            |  11 +-
>   target/riscv/cpu_helper.c                     |  37 +-
>   target/riscv/csr.c                            | 284 ++++++++-------
>   target/riscv/debug.c                          |   1 +
>   target/riscv/fpu_helper.c                     |   6 +-
>   target/riscv/gdbstub.c                        |   1 +
>   target/riscv/kvm/kvm-cpu.c                    |   1 +
>   target/riscv/machine.c                        | 133 ++++---
>   target/riscv/op_helper.c                      |   1 +
>   target/riscv/pmp.c                            |  13 +-
>   target/riscv/pmu.c                            | 150 ++------
>   target/riscv/tcg/tcg-cpu.c                    |   3 +-
>   target/riscv/th_csr.c                         |   1 +
>   target/riscv/translate.c                      |  53 ++-
>   target/riscv/vector_helper.c                  |  22 +-
>   .../riscv/insn_trans/trans_privileged.c.inc   |   2 +-
>   target/riscv/insn_trans/trans_rvi.c.inc       |  16 +-
>   target/riscv/insn_trans/trans_rvm.c.inc       |  16 +-
>   target/riscv/insn_trans/trans_rvv.c.inc       |  22 +-
>   target/riscv/insn_trans/trans_rvzicfiss.c.inc |  22 +-
>   27 files changed, 644 insertions(+), 653 deletions(-)
>   create mode 100644 target/riscv/csr.h
> 

Thanks,
Pierrick