Hi Alastair,
the subject is a slightly underhanded description, in that what I really
wanted to achieve was removing RISC-V's use of .instance_post_init; that's
because RISC-V operate with an opposite conception of .instance_post_init
compared to everyone else: RISC-V wants to register properties there,
whereas x86 and hw/pci-bridge/pcie_root_port.c want to set them.
While it's possible to move RISC-V's code to instance_init, the others
have to run after global properties have been set by device_post_init().
However, I think the result is an improvement anyway, in that it makes
CPU definitions entirely declarative. Previously, multiple instance_init
functions each override the properties that were set by the superclass,
and the code used a mix of subclassing and direct invocation of other
functions. Now, instead, after .class_init all the settings for each
model are available in a RISCVCPUDef struct, and the result is copied
into the RISCVCPU at .instance_init time. This is done with a single
function that starts from the parent's RISCVCPUDef and applies the delta
stored in the CPU's class_data.
Apart from the small reduction in line count, one advantage is that
more validation of the models can be done unconditionally at startup,
instead of happening dynamically if a CPU model is chosen.
Tested by running query-cpu-model-expansion on all concrete models,
before and after applying the patches, with no change except the bugfix
noted in patch 10. The 64-bit variant of the script is as follows:
for i in \
"max" "max32" "rv32" "rv64" "x-rv128" "rv32i" "rv32e" "rv64i" "rv64e" \
"rva22u64" "rva22s64" "lowrisc-ibex" "shakti-c" "sifive-e31" "sifive-e34" \
"sifive-e51" "sifive-u34" "sifive-u54" "thead-c906" "veyron-v1" \
"tt-ascalon" "xiangshan-nanhu"
do
echo $i
echo "
{'execute': 'qmp_capabilities'}
{'execute': 'query-cpu-model-expansion', 'arguments':{'type': 'full', 'model': {'name': '$i'}}}
{'execute': 'quit'}
" | ./qemu-system-riscv64 -qmp stdio -display none -M none | jq .return.model > list-new/$i
echo "
{'execute': 'qmp_capabilities'}
{'execute': 'query-cpu-model-expansion', 'arguments':{'type': 'full', 'model': {'name': '$i'}}}
{'execute': 'quit'}
" | ../../qemu-rust/+build/qemu-system-riscv64 -qmp stdio -display none -M none | jq .return.model > list-old/$i
done
Do you think this is a good approach?
Paolo
Paolo Bonzini (22):
target/riscv: remove unused macro DEFINE_CPU
target/riscv: introduce RISCVCPUDef
target/riscv: store RISCVCPUDef struct directly in the class
target/riscv: merge riscv_cpu_class_init with the class_base function
target/riscv: move RISCVCPUConfig fields to a header file
target/riscv: add more RISCVCPUDef fields
target/riscv: convert abstract CPU classes to RISCVCPUDef
target/riscv: convert profile CPU models to RISCVCPUDef
target/riscv: convert bare CPU models to RISCVCPUDef
target/riscv: move 128-bit check to TCG realize
target/riscv: convert dynamic CPU models to RISCVCPUDef
target/riscv: convert SiFive E CPU models to RISCVCPUDef
target/riscv: convert ibex CPU models to RISCVCPUDef
target/riscv: convert SiFive U models to RISCVCPUDef
target/riscv: th: make CSR insertion test a bit more intuitive
target/riscv: generalize custom CSR functionality
target/riscv: convert TT C906 to RISCVCPUDef
target/riscv: convert TT Ascalon to RISCVCPUDef
target/riscv: convert Ventana V1 to RISCVCPUDef
target/riscv: convert Xiangshan Nanhu to RISCVCPUDef
target/riscv: remove .instance_post_init
target/riscv: move SATP modes out of CPUConfig
target/riscv/cpu-qom.h | 2 +
target/riscv/cpu.h | 43 +-
target/riscv/cpu_cfg.h | 175 +-----
target/riscv/cpu_cfg_fields.h.inc | 163 +++++
hw/riscv/boot.c | 2 +-
hw/riscv/virt-acpi-build.c | 6 +-
hw/riscv/virt.c | 4 +-
target/riscv/cpu.c | 967 ++++++++++++++----------------
target/riscv/csr.c | 2 +-
target/riscv/gdbstub.c | 6 +-
target/riscv/kvm/kvm-cpu.c | 21 +-
target/riscv/machine.c | 2 +-
target/riscv/tcg/tcg-cpu.c | 19 +-
target/riscv/th_csr.c | 30 +-
target/riscv/translate.c | 2 +-
15 files changed, 689 insertions(+), 755 deletions(-)
create mode 100644 target/riscv/cpu_cfg_fields.h.inc
--
2.48.1