* [PATCH 0/4] gdbstub and TCG plugin improvements
@ 2023-10-12 5:42 Akihiko Odaki
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-12 5:42 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki
This series extracts fixes and refactorings that can be applied
independently from "[PATCH v9 00/23] plugins: Allow to read registers".
The patch "target/riscv: Move MISA limits to class" was replaced with
patch "target/riscv: Move misa_mxl_max to class" since I found instances
may have different misa_ext_mask.
Akihiko Odaki (4):
target/riscv: Remove misa_mxl validation
target/riscv: Move misa_mxl_max to class
target/riscv: Validate misa_mxl_max only once
plugins: Remove an extra parameter
target/riscv/cpu-qom.h | 1 +
target/riscv/cpu.h | 1 -
accel/tcg/plugin-gen.c | 9 +--
hw/riscv/boot.c | 2 +-
target/riscv/cpu.c | 142 +++++++++++++++++----------------------
target/riscv/gdbstub.c | 12 ++--
target/riscv/machine.c | 7 +-
target/riscv/translate.c | 3 +-
8 files changed, 81 insertions(+), 96 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-12 5:42 [PATCH 0/4] gdbstub and TCG plugin improvements Akihiko Odaki
@ 2023-10-12 5:42 ` Akihiko Odaki
2023-10-13 17:12 ` Daniel Henrique Barboza
2023-10-17 2:29 ` LIU Zhiwei
2023-10-12 5:42 ` [PATCH 2/4] target/riscv: Move misa_mxl_max to class Akihiko Odaki
` (2 subsequent siblings)
3 siblings, 2 replies; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-12 5:42 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, open list:RISC-V TCG CPUs
It is initialized with a simple assignment and there is little room for
error. In fact, the validation is even more complex.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
target/riscv/cpu.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f5572704de..550b357fb7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1042,7 +1042,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
}
}
-static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
+static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
{
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
CPUClass *cc = CPU_CLASS(mcc);
@@ -1062,11 +1062,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
default:
g_assert_not_reached();
}
-
- if (env->misa_mxl_max != env->misa_mxl) {
- error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
- return;
- }
}
/*
@@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
return;
}
- riscv_cpu_validate_misa_mxl(cpu, &local_err);
- if (local_err != NULL) {
- error_propagate(errp, local_err);
- return;
- }
+ riscv_cpu_validate_misa_mxl(cpu);
riscv_cpu_validate_priv_spec(cpu, &local_err);
if (local_err != NULL) {
--
2.42.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] target/riscv: Move misa_mxl_max to class
2023-10-12 5:42 [PATCH 0/4] gdbstub and TCG plugin improvements Akihiko Odaki
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
@ 2023-10-12 5:42 ` Akihiko Odaki
2023-10-13 17:47 ` Daniel Henrique Barboza
2023-10-12 5:42 ` [PATCH 3/4] target/riscv: Validate misa_mxl_max only once Akihiko Odaki
2023-10-12 5:42 ` [PATCH 4/4] plugins: Remove an extra parameter Akihiko Odaki
3 siblings, 1 reply; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-12 5:42 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, open list:RISC-V TCG CPUs
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 <akihiko.odaki@daynix.com>
---
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->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 = &cpu->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 = &RISCV_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 = &cpu->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 = &RISCV_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 = &RISCV_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 = &RISCV_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 = &RISCV_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 = &RISCV_CPU(obj)->env;
- /* We set this in the realise function */
- set_misa(env, MXL_RV32, 0);
riscv_cpu_add_user_properties(obj);
/* Set latest version of privileged specification */
env->priv_ver = PRIV_VERSION_LATEST;
@@ -551,7 +540,7 @@ static void rv32_sifive_u_cpu_init(Object *obj)
{
RISCVCPU *cpu = RISCV_CPU(obj);
CPURISCVState *env = &cpu->env;
- set_misa(env, MXL_RV32, 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_SV32);
@@ -569,7 +558,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
- set_misa(env, MXL_RV32, 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);
@@ -586,7 +575,7 @@ static void rv32_ibex_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
- set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
+ set_misa_ext(env, RVI | RVM | RVC | RVU);
env->priv_ver = PRIV_VERSION_1_11_0;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -604,7 +593,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
CPURISCVState *env = &RISCV_CPU(obj)->env;
RISCVCPU *cpu = RISCV_CPU(obj);
- set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
+ set_misa_ext(env, RVI | RVM | RVA | RVF | RVC | RVU);
env->priv_ver = PRIV_VERSION_1_10_0;
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
@@ -617,19 +606,6 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
}
#endif
-#if defined(CONFIG_KVM)
-static void riscv_host_cpu_init(Object *obj)
-{
- CPURISCVState *env = &RISCV_CPU(obj)->env;
-#if defined(TARGET_RISCV32)
- set_misa(env, MXL_RV32, 0);
-#elif defined(TARGET_RISCV64)
- set_misa(env, MXL_RV64, 0);
-#endif
- riscv_cpu_add_user_properties(obj);
-}
-#endif /* CONFIG_KVM */
-
static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
{
ObjectClass *oc;
@@ -868,8 +844,8 @@ static void riscv_cpu_reset_hold(Object *obj)
if (mcc->parent_phases.hold) {
mcc->parent_phases.hold(obj);
}
+ env->misa_mxl = mcc->misa_mxl_max;
#ifndef CONFIG_USER_ONLY
- env->misa_mxl = env->misa_mxl_max;
env->priv = PRV_M;
env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
if (env->misa_mxl > MXL_RV32) {
@@ -1046,10 +1022,9 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
{
RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
CPUClass *cc = CPU_CLASS(mcc);
- CPURISCVState *env = &cpu->env;
/* Validate that MISA_MXL is set properly. */
- switch (env->misa_mxl_max) {
+ switch (mcc->misa_mxl_max) {
#ifdef TARGET_RISCV64
case MXL_RV64:
case MXL_RV128:
@@ -1070,6 +1045,7 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
*/
void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
{
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
CPURISCVState *env = &cpu->env;
Error *local_err = NULL;
@@ -1237,7 +1213,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
cpu->cfg.ext_zcb = true;
cpu->cfg.ext_zcmp = true;
cpu->cfg.ext_zcmt = true;
- if (riscv_has_ext(env, RVF) && env->misa_mxl_max == MXL_RV32) {
+ if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
cpu->cfg.ext_zcf = true;
}
}
@@ -1245,7 +1221,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
/* zca, zcd and zcf has a PRIV 1.12.0 restriction */
if (riscv_has_ext(env, RVC) && env->priv_ver >= PRIV_VERSION_1_12_0) {
cpu->cfg.ext_zca = true;
- if (riscv_has_ext(env, RVF) && env->misa_mxl_max == MXL_RV32) {
+ if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
cpu->cfg.ext_zcf = true;
}
if (riscv_has_ext(env, RVD)) {
@@ -1253,7 +1229,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
}
}
- if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
+ if (mcc->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
error_setg(errp, "Zcf extension is only relevant to RV32");
return;
}
@@ -2174,7 +2150,7 @@ static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
visit_type_bool(v, name, &value, errp);
}
-static void riscv_cpu_class_init(ObjectClass *c, void *data)
+static void riscv_cpu_common_class_init(ObjectClass *c, void *data)
{
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
CPUClass *cc = CPU_CLASS(c);
@@ -2217,6 +2193,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
device_class_set_props(dc, riscv_cpu_properties);
}
+static void riscv_cpu_class_init(ObjectClass *c, void *data)
+{
+ RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
+
+ mcc->misa_mxl_max = (uint32_t)(uintptr_t)data;
+}
+
static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
int max_str_len)
{
@@ -2282,18 +2265,22 @@ void riscv_cpu_list(void)
g_slist_free(list);
}
-#define DEFINE_CPU(type_name, initfn) \
- { \
- .name = type_name, \
- .parent = TYPE_RISCV_CPU, \
- .instance_init = initfn \
+#define DEFINE_CPU(type_name, misa_mxl_max, initfn) \
+ { \
+ .name = (type_name), \
+ .parent = TYPE_RISCV_CPU, \
+ .instance_init = (initfn), \
+ .class_init = riscv_cpu_class_init, \
+ .class_data = (void *)(misa_mxl_max) \
}
-#define DEFINE_DYNAMIC_CPU(type_name, initfn) \
- { \
- .name = type_name, \
- .parent = TYPE_RISCV_DYNAMIC_CPU, \
- .instance_init = initfn \
+#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, initfn) \
+ { \
+ .name = (type_name), \
+ .parent = TYPE_RISCV_DYNAMIC_CPU, \
+ .instance_init = (initfn), \
+ .class_init = riscv_cpu_class_init, \
+ .class_data = (void *)(misa_mxl_max) \
}
static const TypeInfo riscv_cpu_type_infos[] = {
@@ -2305,31 +2292,39 @@ static const TypeInfo riscv_cpu_type_infos[] = {
.instance_init = riscv_cpu_init,
.abstract = true,
.class_size = sizeof(RISCVCPUClass),
- .class_init = riscv_cpu_class_init,
+ .class_init = riscv_cpu_common_class_init,
},
{
.name = TYPE_RISCV_DYNAMIC_CPU,
.parent = TYPE_RISCV_CPU,
.abstract = true,
},
- DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, riscv_any_cpu_init),
+#if defined(TARGET_RISCV32)
+ DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV32, riscv_any_cpu_init),
+#elif defined(TARGET_RISCV64)
+ DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV64, riscv_any_cpu_init),
+#endif
#if defined(CONFIG_KVM)
- DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
+#if defined(TARGET_RISCV32)
+ DEFINE_CPU(TYPE_RISCV_CPU_HOST, MXL_RV32, riscv_cpu_add_user_properties),
+#elif defined(TARGET_RISCV64)
+ DEFINE_CPU(TYPE_RISCV_CPU_HOST, MXL_RV64, riscv_cpu_add_user_properties),
+#endif
#endif
#if defined(TARGET_RISCV32)
- DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32, rv32_base_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_IBEX, rv32_ibex_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32_sifive_e_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32_imafcu_nommu_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
+ DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32, MXL_RV32, rv32_base_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_IBEX, MXL_RV32, rv32_ibex_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, MXL_RV32, rv32_sifive_e_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, MXL_RV32, rv32_imafcu_nommu_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, MXL_RV32, rv32_sifive_u_cpu_init),
#elif defined(TARGET_RISCV64)
- DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, rv64_sifive_u_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, rv64_thead_c906_cpu_init),
- DEFINE_CPU(TYPE_RISCV_CPU_VEYRON_V1, rv64_veyron_v1_cpu_init),
- DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128, rv128_base_cpu_init),
+ DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64, MXL_RV64, rv64_base_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, MXL_RV64, rv64_sifive_e_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, MXL_RV64, rv64_sifive_u_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, MXL_RV64, rv64_sifive_u_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, MXL_RV64, rv64_thead_c906_cpu_init),
+ DEFINE_CPU(TYPE_RISCV_CPU_VEYRON_V1, MXL_RV64, rv64_veyron_v1_cpu_init),
+ DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128, MXL_RV128, rv128_base_cpu_init),
#endif
};
diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 524bede865..b9528cef5b 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -49,6 +49,7 @@ static const struct TypeSize vec_lanes[] = {
int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
{
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
target_ulong tmp;
@@ -61,7 +62,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
return 0;
}
- switch (env->misa_mxl_max) {
+ switch (mcc->misa_mxl_max) {
case MXL_RV32:
return gdb_get_reg32(mem_buf, tmp);
case MXL_RV64:
@@ -75,12 +76,13 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
{
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
int length = 0;
target_ulong tmp;
- switch (env->misa_mxl_max) {
+ switch (mcc->misa_mxl_max) {
case MXL_RV32:
tmp = (int32_t)ldl_p(mem_buf);
length = 4;
@@ -214,11 +216,12 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
{
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
GString *s = g_string_new(NULL);
riscv_csr_predicate_fn predicate;
- int bitsize = 16 << env->misa_mxl_max;
+ int bitsize = 16 << mcc->misa_mxl_max;
int i;
#if !defined(CONFIG_USER_ONLY)
@@ -310,6 +313,7 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
{
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
CPURISCVState *env = &cpu->env;
if (env->misa_ext & RVD) {
@@ -326,7 +330,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
ricsv_gen_dynamic_vector_xml(cs, base_reg),
"riscv-vector.xml", 0);
}
- switch (env->misa_mxl_max) {
+ switch (mcc->misa_mxl_max) {
case MXL_RV32:
gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
riscv_gdb_set_virtual,
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index c7c862cdd3..c7124a068c 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -175,10 +175,9 @@ static const VMStateDescription vmstate_pointermasking = {
static bool rv128_needed(void *opaque)
{
- RISCVCPU *cpu = opaque;
- CPURISCVState *env = &cpu->env;
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
- return env->misa_mxl_max == MXL_RV128;
+ return mcc->misa_mxl_max == MXL_RV128;
}
static const VMStateDescription vmstate_rv128 = {
@@ -369,7 +368,7 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINTTL(env.vext_ver, RISCVCPU),
VMSTATE_UINT32(env.misa_mxl, RISCVCPU),
VMSTATE_UINT32(env.misa_ext, RISCVCPU),
- VMSTATE_UINT32(env.misa_mxl_max, RISCVCPU),
+ VMSTATE_UNUSED(4),
VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
VMSTATE_UINTTL(env.priv, RISCVCPU),
VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index f0be79bb16..7e383c5eeb 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1167,6 +1167,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
{
DisasContext *ctx = container_of(dcbase, DisasContext, base);
CPURISCVState *env = cpu_env(cs);
+ RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
RISCVCPU *cpu = RISCV_CPU(cs);
uint32_t tb_flags = ctx->base.tb->flags;
@@ -1188,7 +1189,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
ctx->vstart_eq_zero = FIELD_EX32(tb_flags, TB_FLAGS, VSTART_EQ_ZERO);
ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
- ctx->misa_mxl_max = env->misa_mxl_max;
+ ctx->misa_mxl_max = mcc->misa_mxl_max;
ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
ctx->cs = cs;
--
2.42.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] target/riscv: Validate misa_mxl_max only once
2023-10-12 5:42 [PATCH 0/4] gdbstub and TCG plugin improvements Akihiko Odaki
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
2023-10-12 5:42 ` [PATCH 2/4] target/riscv: Move misa_mxl_max to class Akihiko Odaki
@ 2023-10-12 5:42 ` Akihiko Odaki
2023-10-12 5:42 ` [PATCH 4/4] plugins: Remove an extra parameter Akihiko Odaki
3 siblings, 0 replies; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-12 5:42 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Palmer Dabbelt,
Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza,
Liu Zhiwei, open list:RISC-V TCG CPUs
misa_mxl_max is now a class member and initialized only once for each
class. This also moves the initialization of gdb_core_xml_file which
will be referenced before realization in the future.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
target/riscv/cpu.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 48983e8467..02c85bf1ac 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1018,9 +1018,8 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
}
}
-static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
+static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
{
- RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
CPUClass *cc = CPU_CLASS(mcc);
/* Validate that MISA_MXL is set properly. */
@@ -1418,8 +1417,6 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
return;
}
- riscv_cpu_validate_misa_mxl(cpu);
-
riscv_cpu_validate_priv_spec(cpu, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
@@ -2198,6 +2195,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
mcc->misa_mxl_max = (uint32_t)(uintptr_t)data;
+ riscv_cpu_validate_misa_mxl(mcc);
}
static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
--
2.42.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] plugins: Remove an extra parameter
2023-10-12 5:42 [PATCH 0/4] gdbstub and TCG plugin improvements Akihiko Odaki
` (2 preceding siblings ...)
2023-10-12 5:42 ` [PATCH 3/4] target/riscv: Validate misa_mxl_max only once Akihiko Odaki
@ 2023-10-12 5:42 ` Akihiko Odaki
3 siblings, 0 replies; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-12 5:42 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Akihiko Odaki, Richard Henderson,
Paolo Bonzini
copy_call() has an unused parameter so remove it.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
accel/tcg/plugin-gen.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 39b3c9351f..78b331b251 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -327,8 +327,7 @@ static TCGOp *copy_st_ptr(TCGOp **begin_op, TCGOp *op)
return op;
}
-static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *empty_func,
- void *func, int *cb_idx)
+static TCGOp *copy_call(TCGOp **begin_op, TCGOp *op, void *func, int *cb_idx)
{
TCGOp *old_op;
int func_idx;
@@ -372,8 +371,7 @@ static TCGOp *append_udata_cb(const struct qemu_plugin_dyn_cb *cb,
}
/* call */
- op = copy_call(&begin_op, op, HELPER(plugin_vcpu_udata_cb),
- cb->f.vcpu_udata, cb_idx);
+ op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
return op;
}
@@ -420,8 +418,7 @@ static TCGOp *append_mem_cb(const struct qemu_plugin_dyn_cb *cb,
if (type == PLUGIN_GEN_CB_MEM) {
/* call */
- op = copy_call(&begin_op, op, HELPER(plugin_vcpu_mem_cb),
- cb->f.vcpu_udata, cb_idx);
+ op = copy_call(&begin_op, op, cb->f.vcpu_udata, cb_idx);
}
return op;
--
2.42.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
@ 2023-10-13 17:12 ` Daniel Henrique Barboza
2023-10-17 2:29 ` LIU Zhiwei
1 sibling, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-10-13 17:12 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Liu Zhiwei, open list:RISC-V TCG CPUs
On 10/12/23 02:42, Akihiko Odaki wrote:
> It is initialized with a simple assignment and there is little room for
> error. In fact, the validation is even more complex.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> target/riscv/cpu.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f5572704de..550b357fb7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1042,7 +1042,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPUClass *cc = CPU_CLASS(mcc);
> @@ -1062,11 +1062,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
> default:
> g_assert_not_reached();
> }
> -
> - if (env->misa_mxl_max != env->misa_mxl) {
> - error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
> - return;
> - }
> }
Note that this patch isn't applicable anymore due to recent changes on master. These changes
are intended to be in target/riscv/tcg/tcg-cpu.c.
The changes LGTM since env->misa_mxl will always be == env->misa_mxl_max at this point. We do
not have any instance in the code where they'll differ.
I'd rename the helper to riscv_cpu_set_gdb_core() or similar given that there's no more
validation happening in the function.
Thanks,
Daniel
>
> /*
> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
> return;
> }
>
> - riscv_cpu_validate_misa_mxl(cpu, &local_err);
> - if (local_err != NULL) {
> - error_propagate(errp, local_err);
> - return;
> - }
> + riscv_cpu_validate_misa_mxl(cpu);
>
> riscv_cpu_validate_priv_spec(cpu, &local_err);
> if (local_err != NULL) {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] target/riscv: Move misa_mxl_max to class
2023-10-12 5:42 ` [PATCH 2/4] target/riscv: Move misa_mxl_max to class Akihiko Odaki
@ 2023-10-13 17:47 ` Daniel Henrique Barboza
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Henrique Barboza @ 2023-10-13 17:47 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Liu Zhiwei, open list:RISC-V TCG CPUs
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 <akihiko.odaki@daynix.com>
> ---
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->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 = &cpu->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 = &RISCV_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 = &cpu->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 = &RISCV_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 = &RISCV_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 = &RISCV_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 = &RISCV_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 = &RISCV_CPU(obj)->env;
> - /* We set this in the realise function */
> - set_misa(env, MXL_RV32, 0);
> riscv_cpu_add_user_properties(obj);
> /* Set latest version of privileged specification */
> env->priv_ver = PRIV_VERSION_LATEST;
> @@ -551,7 +540,7 @@ static void rv32_sifive_u_cpu_init(Object *obj)
> {
> RISCVCPU *cpu = RISCV_CPU(obj);
> CPURISCVState *env = &cpu->env;
> - set_misa(env, MXL_RV32, 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_SV32);
> @@ -569,7 +558,7 @@ static void rv32_sifive_e_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> RISCVCPU *cpu = RISCV_CPU(obj);
>
> - set_misa(env, MXL_RV32, 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);
> @@ -586,7 +575,7 @@ static void rv32_ibex_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> RISCVCPU *cpu = RISCV_CPU(obj);
>
> - set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU);
> + set_misa_ext(env, RVI | RVM | RVC | RVU);
> env->priv_ver = PRIV_VERSION_1_11_0;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> @@ -604,7 +593,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> CPURISCVState *env = &RISCV_CPU(obj)->env;
> RISCVCPU *cpu = RISCV_CPU(obj);
>
> - set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> + set_misa_ext(env, RVI | RVM | RVA | RVF | RVC | RVU);
> env->priv_ver = PRIV_VERSION_1_10_0;
> #ifndef CONFIG_USER_ONLY
> set_satp_mode_max_supported(cpu, VM_1_10_MBARE);
> @@ -617,19 +606,6 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> }
> #endif
>
> -#if defined(CONFIG_KVM)
> -static void riscv_host_cpu_init(Object *obj)
> -{
> - CPURISCVState *env = &RISCV_CPU(obj)->env;
> -#if defined(TARGET_RISCV32)
> - set_misa(env, MXL_RV32, 0);
> -#elif defined(TARGET_RISCV64)
> - set_misa(env, MXL_RV64, 0);
> -#endif
> - riscv_cpu_add_user_properties(obj);
> -}
> -#endif /* CONFIG_KVM */
> -
> static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
> {
> ObjectClass *oc;
> @@ -868,8 +844,8 @@ static void riscv_cpu_reset_hold(Object *obj)
> if (mcc->parent_phases.hold) {
> mcc->parent_phases.hold(obj);
> }
> + env->misa_mxl = mcc->misa_mxl_max;
> #ifndef CONFIG_USER_ONLY
> - env->misa_mxl = env->misa_mxl_max;
> env->priv = PRV_M;
> env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
> if (env->misa_mxl > MXL_RV32) {
> @@ -1046,10 +1022,9 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPUClass *cc = CPU_CLASS(mcc);
> - CPURISCVState *env = &cpu->env;
>
> /* Validate that MISA_MXL is set properly. */
> - switch (env->misa_mxl_max) {
> + switch (mcc->misa_mxl_max) {
> #ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> @@ -1070,6 +1045,7 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
> */
> void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> {
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPURISCVState *env = &cpu->env;
> Error *local_err = NULL;
>
> @@ -1237,7 +1213,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> cpu->cfg.ext_zcb = true;
> cpu->cfg.ext_zcmp = true;
> cpu->cfg.ext_zcmt = true;
> - if (riscv_has_ext(env, RVF) && env->misa_mxl_max == MXL_RV32) {
> + if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
> cpu->cfg.ext_zcf = true;
> }
> }
> @@ -1245,7 +1221,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> /* zca, zcd and zcf has a PRIV 1.12.0 restriction */
> if (riscv_has_ext(env, RVC) && env->priv_ver >= PRIV_VERSION_1_12_0) {
> cpu->cfg.ext_zca = true;
> - if (riscv_has_ext(env, RVF) && env->misa_mxl_max == MXL_RV32) {
> + if (riscv_has_ext(env, RVF) && mcc->misa_mxl_max == MXL_RV32) {
> cpu->cfg.ext_zcf = true;
> }
> if (riscv_has_ext(env, RVD)) {
> @@ -1253,7 +1229,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> }
> }
>
> - if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> + if (mcc->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> error_setg(errp, "Zcf extension is only relevant to RV32");
> return;
> }
> @@ -2174,7 +2150,7 @@ static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
> visit_type_bool(v, name, &value, errp);
> }
>
> -static void riscv_cpu_class_init(ObjectClass *c, void *data)
> +static void riscv_cpu_common_class_init(ObjectClass *c, void *data)
> {
> RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> CPUClass *cc = CPU_CLASS(c);
> @@ -2217,6 +2193,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
> device_class_set_props(dc, riscv_cpu_properties);
> }
>
> +static void riscv_cpu_class_init(ObjectClass *c, void *data)
> +{
> + RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> +
> + mcc->misa_mxl_max = (uint32_t)(uintptr_t)data;
> +}
> +
> static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str,
> int max_str_len)
> {
> @@ -2282,18 +2265,22 @@ void riscv_cpu_list(void)
> g_slist_free(list);
> }
>
> -#define DEFINE_CPU(type_name, initfn) \
> - { \
> - .name = type_name, \
> - .parent = TYPE_RISCV_CPU, \
> - .instance_init = initfn \
> +#define DEFINE_CPU(type_name, misa_mxl_max, initfn) \
> + { \
> + .name = (type_name), \
> + .parent = TYPE_RISCV_CPU, \
> + .instance_init = (initfn), \
> + .class_init = riscv_cpu_class_init, \
> + .class_data = (void *)(misa_mxl_max) \
> }
>
> -#define DEFINE_DYNAMIC_CPU(type_name, initfn) \
> - { \
> - .name = type_name, \
> - .parent = TYPE_RISCV_DYNAMIC_CPU, \
> - .instance_init = initfn \
> +#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, initfn) \
> + { \
> + .name = (type_name), \
> + .parent = TYPE_RISCV_DYNAMIC_CPU, \
> + .instance_init = (initfn), \
> + .class_init = riscv_cpu_class_init, \
> + .class_data = (void *)(misa_mxl_max) \
> }
>
> static const TypeInfo riscv_cpu_type_infos[] = {
> @@ -2305,31 +2292,39 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .instance_init = riscv_cpu_init,
> .abstract = true,
> .class_size = sizeof(RISCVCPUClass),
> - .class_init = riscv_cpu_class_init,
> + .class_init = riscv_cpu_common_class_init,
> },
> {
> .name = TYPE_RISCV_DYNAMIC_CPU,
> .parent = TYPE_RISCV_CPU,
> .abstract = true,
> },
> - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, riscv_any_cpu_init),
> +#if defined(TARGET_RISCV32)
> + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV32, riscv_any_cpu_init),
> +#elif defined(TARGET_RISCV64)
> + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_ANY, MXL_RV64, riscv_any_cpu_init),
> +#endif
> #if defined(CONFIG_KVM)
> - DEFINE_CPU(TYPE_RISCV_CPU_HOST, riscv_host_cpu_init),
> +#if defined(TARGET_RISCV32)
> + DEFINE_CPU(TYPE_RISCV_CPU_HOST, MXL_RV32, riscv_cpu_add_user_properties),
> +#elif defined(TARGET_RISCV64)
> + DEFINE_CPU(TYPE_RISCV_CPU_HOST, MXL_RV64, riscv_cpu_add_user_properties),
> +#endif
> #endif
> #if defined(TARGET_RISCV32)
> - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32, rv32_base_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_IBEX, rv32_ibex_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32_sifive_e_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32_imafcu_nommu_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
> + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE32, MXL_RV32, rv32_base_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_IBEX, MXL_RV32, rv32_ibex_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, MXL_RV32, rv32_sifive_e_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, MXL_RV32, rv32_imafcu_nommu_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, MXL_RV32, rv32_sifive_u_cpu_init),
> #elif defined(TARGET_RISCV64)
> - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, rv64_sifive_u_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, rv64_thead_c906_cpu_init),
> - DEFINE_CPU(TYPE_RISCV_CPU_VEYRON_V1, rv64_veyron_v1_cpu_init),
> - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128, rv128_base_cpu_init),
> + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE64, MXL_RV64, rv64_base_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, MXL_RV64, rv64_sifive_e_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, MXL_RV64, rv64_sifive_u_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_SHAKTI_C, MXL_RV64, rv64_sifive_u_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_THEAD_C906, MXL_RV64, rv64_thead_c906_cpu_init),
> + DEFINE_CPU(TYPE_RISCV_CPU_VEYRON_V1, MXL_RV64, rv64_veyron_v1_cpu_init),
> + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_BASE128, MXL_RV128, rv128_base_cpu_init),
> #endif
> };
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 524bede865..b9528cef5b 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -49,6 +49,7 @@ static const struct TypeSize vec_lanes[] = {
>
> int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> {
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> CPURISCVState *env = &cpu->env;
> target_ulong tmp;
> @@ -61,7 +62,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> return 0;
> }
>
> - switch (env->misa_mxl_max) {
> + switch (mcc->misa_mxl_max) {
> case MXL_RV32:
> return gdb_get_reg32(mem_buf, tmp);
> case MXL_RV64:
> @@ -75,12 +76,13 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>
> int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> {
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> CPURISCVState *env = &cpu->env;
> int length = 0;
> target_ulong tmp;
>
> - switch (env->misa_mxl_max) {
> + switch (mcc->misa_mxl_max) {
> case MXL_RV32:
> tmp = (int32_t)ldl_p(mem_buf);
> length = 4;
> @@ -214,11 +216,12 @@ static int riscv_gdb_set_virtual(CPURISCVState *cs, uint8_t *mem_buf, int n)
>
> static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
> {
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> CPURISCVState *env = &cpu->env;
> GString *s = g_string_new(NULL);
> riscv_csr_predicate_fn predicate;
> - int bitsize = 16 << env->misa_mxl_max;
> + int bitsize = 16 << mcc->misa_mxl_max;
> int i;
>
> #if !defined(CONFIG_USER_ONLY)
> @@ -310,6 +313,7 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
>
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> {
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> CPURISCVState *env = &cpu->env;
> if (env->misa_ext & RVD) {
> @@ -326,7 +330,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> ricsv_gen_dynamic_vector_xml(cs, base_reg),
> "riscv-vector.xml", 0);
> }
> - switch (env->misa_mxl_max) {
> + switch (mcc->misa_mxl_max) {
> case MXL_RV32:
> gdb_register_coprocessor(cs, riscv_gdb_get_virtual,
> riscv_gdb_set_virtual,
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index c7c862cdd3..c7124a068c 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -175,10 +175,9 @@ static const VMStateDescription vmstate_pointermasking = {
>
> static bool rv128_needed(void *opaque)
> {
> - RISCVCPU *cpu = opaque;
> - CPURISCVState *env = &cpu->env;
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
>
> - return env->misa_mxl_max == MXL_RV128;
> + return mcc->misa_mxl_max == MXL_RV128;
> }
>
> static const VMStateDescription vmstate_rv128 = {
> @@ -369,7 +368,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> VMSTATE_UINTTL(env.vext_ver, RISCVCPU),
> VMSTATE_UINT32(env.misa_mxl, RISCVCPU),
> VMSTATE_UINT32(env.misa_ext, RISCVCPU),
> - VMSTATE_UINT32(env.misa_mxl_max, RISCVCPU),
> + VMSTATE_UNUSED(4),
> VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
> VMSTATE_UINTTL(env.priv, RISCVCPU),
> VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index f0be79bb16..7e383c5eeb 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1167,6 +1167,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> {
> DisasContext *ctx = container_of(dcbase, DisasContext, base);
> CPURISCVState *env = cpu_env(cs);
> + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cs);
> RISCVCPU *cpu = RISCV_CPU(cs);
> uint32_t tb_flags = ctx->base.tb->flags;
>
> @@ -1188,7 +1189,7 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
> ctx->vstart_eq_zero = FIELD_EX32(tb_flags, TB_FLAGS, VSTART_EQ_ZERO);
> ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
> - ctx->misa_mxl_max = env->misa_mxl_max;
> + ctx->misa_mxl_max = mcc->misa_mxl_max;
> ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
> ctx->address_xl = FIELD_EX32(tb_flags, TB_FLAGS, AXL);
> ctx->cs = cs;
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
2023-10-13 17:12 ` Daniel Henrique Barboza
@ 2023-10-17 2:29 ` LIU Zhiwei
2023-10-17 3:37 ` Akihiko Odaki
1 sibling, 1 reply; 11+ messages in thread
From: LIU Zhiwei @ 2023-10-17 2:29 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza,
open list:RISC-V TCG CPUs
On 2023/10/12 13:42, Akihiko Odaki wrote:
> It is initialized with a simple assignment and there is little room for
> error. In fact, the validation is even more complex.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> target/riscv/cpu.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f5572704de..550b357fb7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1042,7 +1042,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
> }
> }
>
> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPUClass *cc = CPU_CLASS(mcc);
> @@ -1062,11 +1062,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
> default:
> g_assert_not_reached();
> }
> -
> - if (env->misa_mxl_max != env->misa_mxl) {
> - error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
> - return;
> - }
> }
>
> /*
> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState *dev, Error **errp)
> return;
> }
>
> - riscv_cpu_validate_misa_mxl(cpu, &local_err);
> - if (local_err != NULL) {
> - error_propagate(errp, local_err);
> - return;
> - }
> + riscv_cpu_validate_misa_mxl(cpu);
This it not right. As we are still working on the supporting for MXL32
or SXL32, this validation is needed.
And we can't ensure the all RISC-V cpus have the same misa_mxl_max or
misa_mxl, it is not right to move it to class.
For example, in the future, riscv64-softmmu can run 32-bit cpu and
64-bit cpu. And maybe in heterogeneous SOC,
we have 32-bit cpu and 64-bit cpu together.
I am afraid that we can not accept this patch set.
Thanks,
Zhiwei
>
> riscv_cpu_validate_priv_spec(cpu, &local_err);
> if (local_err != NULL) {
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-17 2:29 ` LIU Zhiwei
@ 2023-10-17 3:37 ` Akihiko Odaki
2023-10-18 5:53 ` LIU Zhiwei
0 siblings, 1 reply; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-17 3:37 UTC (permalink / raw)
To: LIU Zhiwei
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza,
open list:RISC-V TCG CPUs
On 2023/10/17 11:29, LIU Zhiwei wrote:
>
> On 2023/10/12 13:42, Akihiko Odaki wrote:
>> It is initialized with a simple assignment and there is little room for
>> error. In fact, the validation is even more complex.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>> target/riscv/cpu.c | 13 ++-----------
>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index f5572704de..550b357fb7 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1042,7 +1042,7 @@ static void
>> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>> }
>> }
>> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
>> {
>> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
>> CPUClass *cc = CPU_CLASS(mcc);
>> @@ -1062,11 +1062,6 @@ static void
>> riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>> default:
>> g_assert_not_reached();
>> }
>> -
>> - if (env->misa_mxl_max != env->misa_mxl) {
>> - error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
>> - return;
>> - }
>> }
>> /*
>> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState
>> *dev, Error **errp)
>> return;
>> }
>> - riscv_cpu_validate_misa_mxl(cpu, &local_err);
>> - if (local_err != NULL) {
>> - error_propagate(errp, local_err);
>> - return;
>> - }
>> + riscv_cpu_validate_misa_mxl(cpu);
>
> This it not right. As we are still working on the supporting for MXL32
> or SXL32, this validation is needed.
It's not preventing supporting MXL32 or SXL32. It's removing
env->misa_mxl_max != env->misa_mxl just because it's initialized with a
simple statement:
env->misa_mxl_max = env->misa_mxl = mxl;
It makes little sense to have a validation code that is more complex
than the validated code.
>
> And we can't ensure the all RISC-V cpus have the same misa_mxl_max or
> misa_mxl, it is not right to move it to class.
> For example, in the future, riscv64-softmmu can run 32-bit cpu and
> 64-bit cpu. And maybe in heterogeneous SOC,
> we have 32-bit cpu and 64-bit cpu together.
This patch series does not touch misa_mxl. We don't need to ensure that
all CPUs have the same misa_mxl_max, but we just need to ensure that
CPUs in the same class do. Creating a heterogeneous SoC is still
possible by combining e.g. TYPE_RISCV_CPU_SIFIVE_E31 and
TYPE_RISCV_CPU_SIFIVE_E51, for example.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-17 3:37 ` Akihiko Odaki
@ 2023-10-18 5:53 ` LIU Zhiwei
2023-10-18 12:19 ` Akihiko Odaki
0 siblings, 1 reply; 11+ messages in thread
From: LIU Zhiwei @ 2023-10-18 5:53 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza,
open list:RISC-V TCG CPUs, Richard Henderson
+CC Richard
On 2023/10/17 11:37, Akihiko Odaki wrote:
> On 2023/10/17 11:29, LIU Zhiwei wrote:
>>
>> On 2023/10/12 13:42, Akihiko Odaki wrote:
>>> It is initialized with a simple assignment and there is little room for
>>> error. In fact, the validation is even more complex.
>>>
>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>> ---
>>> target/riscv/cpu.c | 13 ++-----------
>>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index f5572704de..550b357fb7 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -1042,7 +1042,7 @@ static void
>>> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>>> }
>>> }
>>> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>>> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
>>> {
>>> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
>>> CPUClass *cc = CPU_CLASS(mcc);
>>> @@ -1062,11 +1062,6 @@ static void
>>> riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>>> default:
>>> g_assert_not_reached();
>>> }
>>> -
>>> - if (env->misa_mxl_max != env->misa_mxl) {
>>> - error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
>>> - return;
>>> - }
>>> }
>>> /*
>>> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState
>>> *dev, Error **errp)
>>> return;
>>> }
>>> - riscv_cpu_validate_misa_mxl(cpu, &local_err);
>>> - if (local_err != NULL) {
>>> - error_propagate(errp, local_err);
>>> - return;
>>> - }
>>> + riscv_cpu_validate_misa_mxl(cpu);
>>
>> This it not right. As we are still working on the supporting for
>> MXL32 or SXL32, this validation is needed.
>
> It's not preventing supporting MXL32 or SXL32. It's removing
> env->misa_mxl_max != env->misa_mxl just because it's initialized with
> a simple statement:
> env->misa_mxl_max = env->misa_mxl = mxl;
>
> It makes little sense to have a validation code that is more complex
> than the validated code.
>
>>
>> And we can't ensure the all RISC-V cpus have the same misa_mxl_max or
>> misa_mxl, it is not right to move it to class.
>> For example, in the future, riscv64-softmmu can run 32-bit cpu and
>> 64-bit cpu. And maybe in heterogeneous SOC,
>> we have 32-bit cpu and 64-bit cpu together.
>
> This patch series does not touch misa_mxl. We don't need to ensure
> that all CPUs have the same misa_mxl_max, but we just need to ensure
> that CPUs in the same class do. Creating a heterogeneous SoC is still
> possible by combining e.g. TYPE_RISCV_CPU_SIFIVE_E31 and
> TYPE_RISCV_CPU_SIFIVE_E51, for example.
I see what you mean. It makes sense to move the misa_mxl_max field from
env to the class struct. The misa_mxl_max is always be set by cpu init
or the migration.
The former is OK. I don't know whether QEMU supports migration from
32-bit CPU to 64-bit CPU. Otherwise,
Acked-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] target/riscv: Remove misa_mxl validation
2023-10-18 5:53 ` LIU Zhiwei
@ 2023-10-18 12:19 ` Akihiko Odaki
0 siblings, 0 replies; 11+ messages in thread
From: Akihiko Odaki @ 2023-10-18 12:19 UTC (permalink / raw)
To: LIU Zhiwei
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Palmer Dabbelt, Alistair Francis,
Bin Meng, Weiwei Li, Daniel Henrique Barboza,
open list:RISC-V TCG CPUs, Richard Henderson
On 2023/10/18 14:53, LIU Zhiwei wrote:
> +CC Richard
>
> On 2023/10/17 11:37, Akihiko Odaki wrote:
>> On 2023/10/17 11:29, LIU Zhiwei wrote:
>>>
>>> On 2023/10/12 13:42, Akihiko Odaki wrote:
>>>> It is initialized with a simple assignment and there is little room for
>>>> error. In fact, the validation is even more complex.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>> target/riscv/cpu.c | 13 ++-----------
>>>> 1 file changed, 2 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index f5572704de..550b357fb7 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -1042,7 +1042,7 @@ static void
>>>> riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU *cpu)
>>>> }
>>>> }
>>>> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>>>> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
>>>> {
>>>> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
>>>> CPUClass *cc = CPU_CLASS(mcc);
>>>> @@ -1062,11 +1062,6 @@ static void
>>>> riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
>>>> default:
>>>> g_assert_not_reached();
>>>> }
>>>> -
>>>> - if (env->misa_mxl_max != env->misa_mxl) {
>>>> - error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
>>>> - return;
>>>> - }
>>>> }
>>>> /*
>>>> @@ -1447,11 +1442,7 @@ static void riscv_cpu_realize_tcg(DeviceState
>>>> *dev, Error **errp)
>>>> return;
>>>> }
>>>> - riscv_cpu_validate_misa_mxl(cpu, &local_err);
>>>> - if (local_err != NULL) {
>>>> - error_propagate(errp, local_err);
>>>> - return;
>>>> - }
>>>> + riscv_cpu_validate_misa_mxl(cpu);
>>>
>>> This it not right. As we are still working on the supporting for
>>> MXL32 or SXL32, this validation is needed.
>>
>> It's not preventing supporting MXL32 or SXL32. It's removing
>> env->misa_mxl_max != env->misa_mxl just because it's initialized with
>> a simple statement:
>> env->misa_mxl_max = env->misa_mxl = mxl;
>>
>> It makes little sense to have a validation code that is more complex
>> than the validated code.
>>
>>>
>>> And we can't ensure the all RISC-V cpus have the same misa_mxl_max or
>>> misa_mxl, it is not right to move it to class.
>>> For example, in the future, riscv64-softmmu can run 32-bit cpu and
>>> 64-bit cpu. And maybe in heterogeneous SOC,
>>> we have 32-bit cpu and 64-bit cpu together.
>>
>> This patch series does not touch misa_mxl. We don't need to ensure
>> that all CPUs have the same misa_mxl_max, but we just need to ensure
>> that CPUs in the same class do. Creating a heterogeneous SoC is still
>> possible by combining e.g. TYPE_RISCV_CPU_SIFIVE_E31 and
>> TYPE_RISCV_CPU_SIFIVE_E51, for example.
>
> I see what you mean. It makes sense to move the misa_mxl_max field from
> env to the class struct. The misa_mxl_max is always be set by cpu init
> or the migration.
>
> The former is OK. I don't know whether QEMU supports migration from
> 32-bit CPU to 64-bit CPU. Otherwise,
>
> Acked-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
It doesn't. docs/devel/migration.rst states:
> For this to work, QEMU has to be launched with the same arguments the
> two times. I.e. it can only restore the state in one guest that has
> the same devices that the one it was saved (this last requirement can
> be relaxed a bit, but for now we can consider that configuration has
> to be exactly the same).
The corresponding CPUs in two QEMU instances launched with the same
arguments will have the same misa_mxl_max values.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-10-18 12:19 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-12 5:42 [PATCH 0/4] gdbstub and TCG plugin improvements Akihiko Odaki
2023-10-12 5:42 ` [PATCH 1/4] target/riscv: Remove misa_mxl validation Akihiko Odaki
2023-10-13 17:12 ` Daniel Henrique Barboza
2023-10-17 2:29 ` LIU Zhiwei
2023-10-17 3:37 ` Akihiko Odaki
2023-10-18 5:53 ` LIU Zhiwei
2023-10-18 12:19 ` Akihiko Odaki
2023-10-12 5:42 ` [PATCH 2/4] target/riscv: Move misa_mxl_max to class Akihiko Odaki
2023-10-13 17:47 ` Daniel Henrique Barboza
2023-10-12 5:42 ` [PATCH 3/4] target/riscv: Validate misa_mxl_max only once Akihiko Odaki
2023-10-12 5:42 ` [PATCH 4/4] plugins: Remove an extra parameter Akihiko Odaki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).