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

Anton Johansson via posted 34 patches 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250924072124.6493-1-anjo@rev.ng
Maintainers: Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Laurent Vivier <laurent@vivier.eu>, Christoph Muellner <christoph.muellner@vrull.eu>
There is a newer version of this series
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
[RFC PATCH 00/34] single-binary: Make riscv cpu.h target independent
Posted by Anton Johansson via 4 months, 2 weeks ago
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?

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

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

-- 
2.51.0
Re: [RFC PATCH 00/34] single-binary: Make riscv cpu.h target independent
Posted by Pierrick Bouvier 4 months, 1 week ago
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
Re: [RFC PATCH 00/34] single-binary: Make riscv cpu.h target independent
Posted by Philippe Mathieu-Daudé 4 months, 1 week ago
On 30/9/25 00:44, Pierrick Bouvier wrote:

> On 9/24/25 12:20 AM, Anton Johansson wrote:

>> 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?

You are correct in that they are all part of the same architecture,
the register bits accessed being a mode of a CPU.

IMO The "superset" view comes from QEMU historical development, and
I suspect a generic riscv-softmmu could be sufficient here.


> 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.
IMHO we should really avoid (2). The only problematic architectures (wrt
migration) are PPC and X86.
Re: [RFC PATCH 00/34] single-binary: Make riscv cpu.h target independent
Posted by Pierrick Bouvier 4 months, 1 week ago
On 9/29/25 7:03 PM, Philippe Mathieu-Daudé wrote:
> On 30/9/25 00:44, Pierrick Bouvier wrote:
> 
>> On 9/24/25 12:20 AM, Anton Johansson wrote:
> 
>>> 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?
> 
> You are correct in that they are all part of the same architecture,
> the register bits accessed being a mode of a CPU.
> 
> IMO The "superset" view comes from QEMU historical development, and
> I suspect a generic riscv-softmmu could be sufficient here.
> 
> 
>> 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.
> IMHO we should really avoid (2). The only problematic architectures (wrt
> migration) are PPC and X86.

Indeed, it looks like a nightmare and an excellent maintenance burden.
I saw after sending my email that Alistair was open to a breaking 
change, so that's a good news!