Re: [PATCH 2/4] target/riscv: Move misa_mxl_max to class
On 10/12/23 02:42, Akihiko Odaki wrote:
misa_mxl_max is common for all instances of a RISC-V CPU class so they
are better put into class.
Signed-off-by: Akihiko Odaki
---
I see where you're coming from but I'm not sure about the end result. Most of
the
code turned up to be the same thing but with an extra cast to fetch misa_mxl_max
from the CPUClass. And you ended up having to add extra data to the class init
because a TARGET_RISCV64 CPU can have MXL64 or MXL128.
IIUC you don't need this patch for what you want to accomplish (init
gdb_core_xml_file
in init() time instead of realize() time). We have a case in x86_64 where
gdb_core is set
by checking a TARGET ifdef in its class init:
(target/i386/cpu.c, x86_cpu_common_class_init())
#ifdef TARGET_X86_64
cc->gdb_core_xml_file = "i386-64bit.xml";
cc->gdb_num_core_regs = 66;
#else
cc->gdb_core_xml_file = "i386-32bit.xml";
cc->gdb_num_core_regs = 50;
#endif
You can just do the same with riscv_cpu_class_init() by just copy/pasting
what's left
of riscv_cpu_validate_misa_mxl():
switch (env->misa_mxl_max) {
#ifdef TARGET_RISCV64
case MXL_RV64:
case MXL_RV128:
cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
break;
#endif
case MXL_RV32:
cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
break;
default:
g_assert_not_reached();
}
Note that env->misa_mxl_max doesn't matter - the gdb core file only cares about
the target
type, and we do not have any cases where we have env->misa_mxl_max = MXL_RV32
with TARGET_RISCV64
or env->misa_mxl_max = MXL_RV64 with TARGET_RISCV32, so this can be shortened
to:
#ifdef TARGET_RISCV64
cc->gdb_core_xml_file = "riscv-64bit-cpu.xml";
#else
cc->gdb_core_xml_file = "riscv-32bit-cpu.xml";
#endif
You can then remove riscv_cpu_validate_misa_mxl().
Thanks,
Daniel
target/riscv/cpu-qom.h | 1 +
target/riscv/cpu.h | 1 -
hw/riscv/boot.c | 2 +-
target/riscv/cpu.c | 127 +++
target/riscv/gdbstub.c | 12 ++--
target/riscv/machine.c | 7 +--
target/riscv/translate.c | 3 +-
7 files changed, 76 insertions(+), 77 deletions(-)
diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
index 04af50983e..fac36fa15b 100644
--- a/target/riscv/cpu-qom.h
+++ b/target/riscv/cpu-qom.h
@@ -67,5 +67,6 @@ struct RISCVCPUClass {
/*< public >*/
DeviceRealize parent_realize;
ResettablePhases parent_phases;
+uint32_t misa_mxl_max; /* max mxl for this cpu */
};
#endif /* RISCV_CPU_QOM_H */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ef9cf21c0c..83f48f9e7e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -155,7 +155,6 @@ 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 */
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[0])->misa_mxl_max == MXL_RV32;
}
/*
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 550b357fb7..48983e8467 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -271,9 +271,8 @@ const char *riscv_cpu_get_trap_name(target_ulong cause,
bool async)
}
}
-static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
+static void set_misa_ext(CPURISCVState *env, uint32_t ext)
{
-env->misa_mxl_max = env->misa_mxl = mxl;
env->misa_ext_mask = env->misa_ext = ext;
}
@@ -375,11 +374,7 @@ static void riscv_any_cpu_init(Object *obj)
{
RISCVCPU *cpu = RISCV_CPU(obj);
CPURISCVState *env = >env;
-#if defined(TARGET_RISCV32)
-set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#elif defined(TARGET_RISCV64)
-set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#endif
+set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(RISCV_CPU(obj),
@@ -400,8 +395,6 @@ static void riscv_any_cpu_init(Object *obj)
static void rv64_base_cpu_init(Object *obj)
{
CPURISCVState *env = _CPU(obj)->env;
-/* We set this in the realise function */
-set_misa(env, MXL_RV64, 0);
riscv_cpu_add_user_properties(obj);
/* Set latest version of privileged specification */
env->priv_ver = PRIV_VERSION_LATEST;
@@ -414,7 +407,7 @@ static void rv64_sifive_u_cpu_init(Object *obj)
{
RISCVCPU *cpu = RISCV_CPU(obj);
CPURISCVState *env = >env;
-set_misa(env, MXL_RV64, RVI | RVM
[PATCH 2/4] target/riscv: Move misa_mxl_max to class
misa_mxl_max is common for all instances of a RISC-V CPU class so they
are better put into class.
Signed-off-by: Akihiko Odaki
---
target/riscv/cpu-qom.h | 1 +
target/riscv/cpu.h | 1 -
hw/riscv/boot.c | 2 +-
target/riscv/cpu.c | 127 +++
target/riscv/gdbstub.c | 12 ++--
target/riscv/machine.c | 7 +--
target/riscv/translate.c | 3 +-
7 files changed, 76 insertions(+), 77 deletions(-)
diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h
index 04af50983e..fac36fa15b 100644
--- a/target/riscv/cpu-qom.h
+++ b/target/riscv/cpu-qom.h
@@ -67,5 +67,6 @@ struct RISCVCPUClass {
/*< public >*/
DeviceRealize parent_realize;
ResettablePhases parent_phases;
+uint32_t misa_mxl_max; /* max mxl for this cpu */
};
#endif /* RISCV_CPU_QOM_H */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ef9cf21c0c..83f48f9e7e 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -155,7 +155,6 @@ 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 */
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[0])->misa_mxl_max == MXL_RV32;
}
/*
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 550b357fb7..48983e8467 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -271,9 +271,8 @@ const char *riscv_cpu_get_trap_name(target_ulong cause,
bool async)
}
}
-static void set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext)
+static void set_misa_ext(CPURISCVState *env, uint32_t ext)
{
-env->misa_mxl_max = env->misa_mxl = mxl;
env->misa_ext_mask = env->misa_ext = ext;
}
@@ -375,11 +374,7 @@ static void riscv_any_cpu_init(Object *obj)
{
RISCVCPU *cpu = RISCV_CPU(obj);
CPURISCVState *env = >env;
-#if defined(TARGET_RISCV32)
-set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#elif defined(TARGET_RISCV64)
-set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
-#endif
+set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU);
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(RISCV_CPU(obj),
@@ -400,8 +395,6 @@ static void riscv_any_cpu_init(Object *obj)
static void rv64_base_cpu_init(Object *obj)
{
CPURISCVState *env = _CPU(obj)->env;
-/* We set this in the realise function */
-set_misa(env, MXL_RV64, 0);
riscv_cpu_add_user_properties(obj);
/* Set latest version of privileged specification */
env->priv_ver = PRIV_VERSION_LATEST;
@@ -414,7 +407,7 @@ static void rv64_sifive_u_cpu_init(Object *obj)
{
RISCVCPU *cpu = RISCV_CPU(obj);
CPURISCVState *env = >env;
-set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
env->priv_ver = PRIV_VERSION_1_10_0;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(RISCV_CPU(obj), VM_1_10_SV39);
@@ -432,7 +425,7 @@ static void rv64_sifive_e_cpu_init(Object *obj)
CPURISCVState *env = _CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
-set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU);
+set_misa_ext(env, RVI | RVM | RVA | RVC | RVU);
env->priv_ver = PRIV_VERSION_1_10_0;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -449,7 +442,7 @@ static void rv64_thead_c906_cpu_init(Object *obj)
CPURISCVState *env = _CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
-set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU);
+set_misa_ext(env, RVG | RVC | RVS | RVU);
env->priv_ver = PRIV_VERSION_1_11_0;
cpu->cfg.ext_zfa = true;
@@ -480,7 +473,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
CPURISCVState *env = _CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
-set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH);
+set_misa_ext(env, RVG | RVC | RVS | RVU | RVH);
env->priv_ver = PRIV_VERSION_1_12_0;
/* Enable ISA extensions */
@@ -524,8 +517,6 @@ static void rv128_base_cpu_init(Object *obj)
exit(EXIT_FAILURE);
}
CPURISCVState *env = _CPU(obj)->env;
-/* We set this in the realise function */
-set_misa(env, MXL_RV128, 0);
riscv_cpu_add_user_properties(obj);
/* Set latest version of privileged specification */
env->priv_ver = PRIV_VERSION_LATEST;
@@ -537,8 +528,6 @@ static void rv128_base_cpu_init(Object *obj)
static void rv32_base_cpu_init(Object *obj)
{
CPURISCVState *env =
