On 2023/10/12 0:23, Alex Bennée wrote:
Akihiko Odaki <akihiko.od...@daynix.com> writes:
MISA limits are common for all instances of a RISC-V CPU class so they
are better put into class.
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
target/riscv/cpu-qom.h | 2 +
target/riscv/cpu.h | 2 -
hw/riscv/boot.c | 2 +-
target/riscv/cpu.c | 212 +++++++++++++++++++++++++++------------
target/riscv/csr.c | 3 +-
target/riscv/gdbstub.c | 12 ++-
target/riscv/machine.c | 11 +-
target/riscv/translate.c | 3 +-
8 files changed, 167 insertions(+), 80 deletions(-)
diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
index 04af50983e..266a07f5be 100644
--- a/target/riscv/cpu-qom.h
+++ b/target/riscv/cpu-qom.h
@@ -67,5 +67,7 @@ struct RISCVCPUClass {
/*< public >*/
DeviceRealize parent_realize;
ResettablePhases parent_phases;
+ uint32_t misa_mxl_max; /* max mxl for this cpu */
+ uint32_t misa_ext_mask; /* max ext for this cpu */
};
#endif /* RISCV_CPU_QOM_H */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ef9cf21c0c..9f9cb6cd2a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -155,9 +155,7 @@ struct CPUArchState {
/* RISCVMXL, but uint32_t for vmstate migration */
uint32_t misa_mxl; /* current mxl */
- uint32_t misa_mxl_max; /* max mxl for this cpu */
uint32_t misa_ext; /* current extensions */
- uint32_t misa_ext_mask; /* max ext for this cpu */
uint32_t xl; /* current xlen */
/* 128-bit helpers upper part return value */
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 52bf8e67de..b7cf08f479 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -36,7 +36,7 @@
bool riscv_is_32bit(RISCVHartArrayState *harts)
{
- return harts->harts[0].env.misa_mxl_max == MXL_RV32;
+ return RISCV_CPU_GET_CLASS(&harts->harts[0])->misa_mxl_max ==
MXL_RV32;
I'm going to defer to the RISCV maintainers here. While I agree the
class is a good place for these parameters that are shared across
multiple vCPUS there is a cost to RISCV_CPU_GET_CLASS() casting.
You might notice we have this comment in include/hw/core/cpu.h:
/*
* The class checkers bring in CPU_GET_CLASS() which is potentially
* expensive given the eventual call to
* object_class_dynamic_cast_assert(). Because of this the CPUState
* has a cached value for the class in cs->cc which is set up in
* cpu_exec_realizefn() for use in hot code paths.
*/
typedef struct CPUClass CPUClass;
DECLARE_CLASS_CHECKERS(CPUClass, CPU,
TYPE_CPU)
It is fine since these variables are cached into DisasContext during TCG
translation as far as I can tell.
However I think you need to check the assumption that you will never see
multiple cores with different RISCV properties.
I missed misa_ext_mask can be different so I changed to only convert
misa_mxl_max into a class member in "[PATCH 0/4] gdbstub and TCG plugin
improvements".