* [PATCH v4 0/5] gdbstub and TCG plugin improvements
@ 2023-10-17 18:53 Akihiko Odaki
2023-10-17 18:53 ` [PATCH v4 1/5] gdbstub: Check if gdb_regs is NULL Akihiko Odaki
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Akihiko Odaki @ 2023-10-17 18:53 UTC (permalink / raw)
Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel,
Philippe Mathieu-Daudé, Fabiano Rosas, 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.
V3 -> V4:
Added patch "gdbstub: Check if gdb_regs is NULL".
V2 -> V3:
Restored patch sets from the previous version.
Rebased to commit 800485762e6564e04e2ab315132d477069562d91.
V1 -> V2:
Added patch "target/riscv: Do not allow MXL_RV32 for TARGET_RISCV64".
Added patch "target/riscv: Initialize gdb_core_xml_file only once".
Dropped patch "target/riscv: Remove misa_mxl validation".
Dropped patch "target/riscv: Move misa_mxl_max to class".
Dropped patch "target/riscv: Validate misa_mxl_max only once".
Akihiko Odaki (5):
gdbstub: Check if gdb_regs is NULL
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 | 3 +-
accel/tcg/plugin-gen.c | 9 +--
gdbstub/gdbstub.c | 34 +++++----
hw/riscv/boot.c | 2 +-
target/riscv/cpu.c | 139 ++++++++++++++++++++++---------------
target/riscv/gdbstub.c | 12 ++--
target/riscv/kvm/kvm-cpu.c | 10 +--
target/riscv/machine.c | 7 +-
target/riscv/tcg/tcg-cpu.c | 42 ++---------
target/riscv/translate.c | 3 +-
11 files changed, 131 insertions(+), 131 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v4 1/5] gdbstub: Check if gdb_regs is NULL 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki @ 2023-10-17 18:53 ` Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 2/5] target/riscv: Remove misa_mxl validation Akihiko Odaki ` (3 subsequent siblings) 4 siblings, 0 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-17 18:53 UTC (permalink / raw) Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Akihiko Odaki cpu->gdb_regs may be NULL if no coprocessor is registered. Fixes: 73c392c26b ("gdbstub: Replace gdb_regs with an array") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> --- gdbstub/gdbstub.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index 1e96a71c0c..29540a0284 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -385,12 +385,14 @@ static const char *get_feature_xml(const char *p, const char **newp, xml, g_markup_printf_escaped("<xi:include href=\"%s\"/>", cc->gdb_core_xml_file)); - for (guint i = 0; i < cpu->gdb_regs->len; i++) { - r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); - g_ptr_array_add( - xml, - g_markup_printf_escaped("<xi:include href=\"%s\"/>", - r->xml)); + if (cpu->gdb_regs) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); + g_ptr_array_add( + xml, + g_markup_printf_escaped("<xi:include href=\"%s\"/>", + r->xml)); + } } g_ptr_array_add(xml, g_strdup("</target>")); g_ptr_array_add(xml, NULL); @@ -430,10 +432,12 @@ static int gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) return cc->gdb_read_register(cpu, buf, reg); } - for (guint i = 0; i < cpu->gdb_regs->len; i++) { - r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); - if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { - return r->get_reg(env, buf, reg - r->base_reg); + if (cpu->gdb_regs) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); + if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { + return r->get_reg(env, buf, reg - r->base_reg); + } } } return 0; @@ -449,10 +453,12 @@ static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg) return cc->gdb_write_register(cpu, mem_buf, reg); } - for (guint i = 0; i < cpu->gdb_regs->len; i++) { - r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); - if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { - return r->set_reg(env, mem_buf, reg - r->base_reg); + if (cpu->gdb_regs) { + for (guint i = 0; i < cpu->gdb_regs->len; i++) { + r = &g_array_index(cpu->gdb_regs, GDBRegisterState, i); + if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) { + return r->set_reg(env, mem_buf, reg - r->base_reg); + } } } return 0; -- 2.42.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 2/5] target/riscv: Remove misa_mxl validation 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 1/5] gdbstub: Check if gdb_regs is NULL Akihiko Odaki @ 2023-10-17 18:53 ` Akihiko Odaki 2023-10-18 5:54 ` LIU Zhiwei 2023-10-18 12:51 ` Daniel Henrique Barboza 2023-10-17 18:53 ` [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class Akihiko Odaki ` (2 subsequent siblings) 4 siblings, 2 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-17 18:53 UTC (permalink / raw) Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Akihiko Odaki, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv 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/tcg/tcg-cpu.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index a28918ab30..7f45e42000 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -148,7 +148,7 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) } } -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); @@ -168,11 +168,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; - } } static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp) @@ -573,11 +568,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp) return false; } - riscv_cpu_validate_misa_mxl(cpu, &local_err); - if (local_err != NULL) { - error_propagate(errp, local_err); - return false; - } + 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] 14+ messages in thread
* Re: [PATCH v4 2/5] target/riscv: Remove misa_mxl validation 2023-10-17 18:53 ` [PATCH v4 2/5] target/riscv: Remove misa_mxl validation Akihiko Odaki @ 2023-10-18 5:54 ` LIU Zhiwei 2023-10-18 12:51 ` Daniel Henrique Barboza 1 sibling, 0 replies; 14+ messages in thread From: LIU Zhiwei @ 2023-10-18 5:54 UTC (permalink / raw) To: Akihiko Odaki Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, qemu-riscv On 2023/10/18 2:53, 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/tcg/tcg-cpu.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index a28918ab30..7f45e42000 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -148,7 +148,7 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) > } > } > > -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); > @@ -168,11 +168,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; > - } > } > > static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp) > @@ -573,11 +568,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp) > return false; > } > > - riscv_cpu_validate_misa_mxl(cpu, &local_err); > - if (local_err != NULL) { > - error_propagate(errp, local_err); > - return false; > - } > + riscv_cpu_validate_misa_mxl(cpu); Acked-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Zhiwei > > riscv_cpu_validate_priv_spec(cpu, &local_err); > if (local_err != NULL) { ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 2/5] target/riscv: Remove misa_mxl validation 2023-10-17 18:53 ` [PATCH v4 2/5] target/riscv: Remove misa_mxl validation Akihiko Odaki 2023-10-18 5:54 ` LIU Zhiwei @ 2023-10-18 12:51 ` Daniel Henrique Barboza 1 sibling, 0 replies; 14+ messages in thread From: Daniel Henrique Barboza @ 2023-10-18 12:51 UTC (permalink / raw) To: Akihiko Odaki Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv On 10/17/23 15:53, 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> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > target/riscv/tcg/tcg-cpu.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index a28918ab30..7f45e42000 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -148,7 +148,7 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) > } > } > > -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); > @@ -168,11 +168,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; > - } > } > > static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp) > @@ -573,11 +568,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp) > return false; > } > > - riscv_cpu_validate_misa_mxl(cpu, &local_err); > - if (local_err != NULL) { > - error_propagate(errp, local_err); > - return false; > - } > + riscv_cpu_validate_misa_mxl(cpu); > > riscv_cpu_validate_priv_spec(cpu, &local_err); > if (local_err != NULL) { ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 1/5] gdbstub: Check if gdb_regs is NULL Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 2/5] target/riscv: Remove misa_mxl validation Akihiko Odaki @ 2023-10-17 18:53 ` Akihiko Odaki 2023-10-18 6:50 ` LIU Zhiwei 2023-10-18 13:01 ` Daniel Henrique Barboza 2023-10-17 18:53 ` [PATCH v4 4/5] target/riscv: Validate misa_mxl_max only once Akihiko Odaki 2023-10-17 18:54 ` [PATCH v4 5/5] plugins: Remove an extra parameter Akihiko Odaki 4 siblings, 2 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-17 18:53 UTC (permalink / raw) Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Akihiko Odaki, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv 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 | 3 +- hw/riscv/boot.c | 2 +- target/riscv/cpu.c | 118 +++++++++++++++++++------------------ target/riscv/gdbstub.c | 12 ++-- target/riscv/kvm/kvm-cpu.c | 10 ++-- target/riscv/machine.c | 7 +-- target/riscv/tcg/tcg-cpu.c | 12 ++-- target/riscv/translate.c | 3 +- 9 files changed, 88 insertions(+), 80 deletions(-) diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h index f3fbe37a2c..33b6d52c90 100644 --- a/target/riscv/cpu-qom.h +++ b/target/riscv/cpu-qom.h @@ -68,5 +68,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 f8ffa5ee38..ef10efd1e7 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -159,7 +159,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 */ @@ -711,7 +710,7 @@ enum riscv_pmu_event_idx { /* used by tcg/tcg-cpu.c*/ void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool en); bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset); -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext); +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext); typedef struct RISCVCPUMultiExtConfig { const char *name; 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 ac4a6c7eec..1fb5747f00 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -263,9 +263,8 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, bool async) } } -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext) +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext) { - env->misa_mxl_max = env->misa_mxl = mxl; env->misa_ext_mask = env->misa_ext = ext; } @@ -367,11 +366,7 @@ static void riscv_any_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); CPURISCVState *env = &cpu->env; -#if defined(TARGET_RISCV32) - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU); -#elif defined(TARGET_RISCV64) - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU); -#endif + riscv_cpu_set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU); #ifndef CONFIG_USER_ONLY set_satp_mode_max_supported(RISCV_CPU(obj), @@ -392,16 +387,14 @@ static void riscv_max_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); CPURISCVState *env = &cpu->env; - RISCVMXL mlx = MXL_RV64; -#ifdef TARGET_RISCV32 - mlx = MXL_RV32; -#endif - riscv_cpu_set_misa(env, mlx, 0); env->priv_ver = PRIV_VERSION_LATEST; #ifndef CONFIG_USER_ONLY - set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ? - VM_1_10_SV32 : VM_1_10_SV57); +#ifdef TARGET_RISCV32 + set_satp_mode_max_supported(cpu, VM_1_10_SV32); +#else + set_satp_mode_max_supported(cpu, VM_1_10_SV57); +#endif #endif } @@ -409,8 +402,6 @@ static void riscv_max_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 */ - riscv_cpu_set_misa(env, MXL_RV64, 0); /* Set latest version of privileged specification */ env->priv_ver = PRIV_VERSION_LATEST; #ifndef CONFIG_USER_ONLY @@ -422,8 +413,7 @@ static void rv64_sifive_u_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); CPURISCVState *env = &cpu->env; - riscv_cpu_set_misa(env, MXL_RV64, - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); + riscv_cpu_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); @@ -441,7 +431,7 @@ static void rv64_sifive_e_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU); + riscv_cpu_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); @@ -458,7 +448,7 @@ static void rv64_thead_c906_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU); + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU); env->priv_ver = PRIV_VERSION_1_11_0; cpu->cfg.ext_zfa = true; @@ -489,7 +479,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH); + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU | RVH); env->priv_ver = PRIV_VERSION_1_12_0; /* Enable ISA extensions */ @@ -533,8 +523,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 */ - riscv_cpu_set_misa(env, MXL_RV128, 0); /* Set latest version of privileged specification */ env->priv_ver = PRIV_VERSION_LATEST; #ifndef CONFIG_USER_ONLY @@ -545,8 +533,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 */ - riscv_cpu_set_misa(env, MXL_RV32, 0); /* Set latest version of privileged specification */ env->priv_ver = PRIV_VERSION_LATEST; #ifndef CONFIG_USER_ONLY @@ -558,8 +544,7 @@ static void rv32_sifive_u_cpu_init(Object *obj) { RISCVCPU *cpu = RISCV_CPU(obj); CPURISCVState *env = &cpu->env; - riscv_cpu_set_misa(env, MXL_RV32, - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); + riscv_cpu_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); @@ -577,7 +562,7 @@ static void rv32_sifive_e_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU); + riscv_cpu_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); @@ -594,7 +579,7 @@ static void rv32_ibex_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU); + riscv_cpu_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); @@ -612,7 +597,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj) CPURISCVState *env = &RISCV_CPU(obj)->env; RISCVCPU *cpu = RISCV_CPU(obj); - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU); + riscv_cpu_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); @@ -834,7 +819,7 @@ static void riscv_cpu_reset_hold(Object *obj) mcc->parent_phases.hold(obj); } #ifndef CONFIG_USER_ONLY - env->misa_mxl = env->misa_mxl_max; + env->misa_mxl = mcc->misa_mxl_max; env->priv = PRV_M; env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV); if (env->misa_mxl > MXL_RV32) { @@ -1169,6 +1154,12 @@ static void riscv_cpu_post_init(Object *obj) static void riscv_cpu_init(Object *obj) { + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj); + RISCVCPU *cpu = RISCV_CPU(obj); + CPURISCVState *env = &cpu->env; + + env->misa_mxl = mcc->misa_mxl_max; + #ifndef CONFIG_USER_ONLY qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq, IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX); @@ -1555,7 +1546,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); @@ -1597,6 +1588,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) { @@ -1662,18 +1660,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[] = { @@ -1686,29 +1688,31 @@ static const TypeInfo riscv_cpu_type_infos[] = { .instance_post_init = riscv_cpu_post_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), - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, riscv_max_cpu_init), #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_ANY, MXL_RV32, riscv_any_cpu_init), + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV32, riscv_max_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_ANY, MXL_RV64, riscv_any_cpu_init), + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV64, riscv_max_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/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c index 090d617627..186ca6e45c 100644 --- a/target/riscv/kvm/kvm-cpu.c +++ b/target/riscv/kvm/kvm-cpu.c @@ -1461,14 +1461,14 @@ static void kvm_cpu_accel_register_types(void) } type_init(kvm_cpu_accel_register_types); -static void riscv_host_cpu_init(Object *obj) +static void riscv_host_cpu_class_init(ObjectClass *c, void *data) { - CPURISCVState *env = &RISCV_CPU(obj)->env; + RISCVCPUClass *mcc = RISCV_CPU_CLASS(c); #if defined(TARGET_RISCV32) - env->misa_mxl_max = env->misa_mxl = MXL_RV32; + mcc->misa_mxl_max = MXL_RV32; #elif defined(TARGET_RISCV64) - env->misa_mxl_max = env->misa_mxl = MXL_RV64; + mcc->misa_mxl_max = MXL_RV64; #endif } @@ -1476,7 +1476,7 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = { { .name = TYPE_RISCV_CPU_HOST, .parent = TYPE_RISCV_CPU, - .instance_init = riscv_host_cpu_init, + .class_init = riscv_host_cpu_class_init, } }; 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/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 7f45e42000..5bf9d31f7c 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -152,10 +152,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: @@ -265,6 +264,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(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; @@ -445,7 +445,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcb), true); cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmp), true); cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); } } @@ -453,7 +453,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_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); } if (riscv_has_ext(env, RVD)) { @@ -461,7 +461,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; } @@ -861,7 +861,7 @@ static void riscv_init_max_cpu_extensions(Object *obj) const RISCVCPUMultiExtConfig *prop; /* Enable RVG, RVJ and RVV that are disabled by default */ - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV); + riscv_cpu_set_misa_ext(env, env->misa_ext | RVG | RVJ | RVV); for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { isa_ext_update_enabled(cpu, prop->offset, true); 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] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-17 18:53 ` [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class Akihiko Odaki @ 2023-10-18 6:50 ` LIU Zhiwei 2023-10-18 12:23 ` Akihiko Odaki 2023-10-18 13:01 ` Daniel Henrique Barboza 1 sibling, 1 reply; 14+ messages in thread From: LIU Zhiwei @ 2023-10-18 6:50 UTC (permalink / raw) To: Akihiko Odaki Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, qemu-riscv On 2023/10/18 2:53, 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> > --- > target/riscv/cpu-qom.h | 1 + > target/riscv/cpu.h | 3 +- > hw/riscv/boot.c | 2 +- > target/riscv/cpu.c | 118 +++++++++++++++++++------------------ > target/riscv/gdbstub.c | 12 ++-- > target/riscv/kvm/kvm-cpu.c | 10 ++-- > target/riscv/machine.c | 7 +-- > target/riscv/tcg/tcg-cpu.c | 12 ++-- > target/riscv/translate.c | 3 +- > 9 files changed, 88 insertions(+), 80 deletions(-) > > diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h > index f3fbe37a2c..33b6d52c90 100644 > --- a/target/riscv/cpu-qom.h > +++ b/target/riscv/cpu-qom.h > @@ -68,5 +68,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 f8ffa5ee38..ef10efd1e7 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -159,7 +159,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 */ > @@ -711,7 +710,7 @@ enum riscv_pmu_event_idx { > /* used by tcg/tcg-cpu.c*/ > void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool en); > bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset); > -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext); > +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext); > > typedef struct RISCVCPUMultiExtConfig { > const char *name; > 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; Hi Akihiko, Can we use the cached CPUClass in CPUState? Like (RISCVCPUClass *)((CPUState *)(&harts->harts[0])->cc) Zhiwei > } > > /* > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ac4a6c7eec..1fb5747f00 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -263,9 +263,8 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, bool async) > } > } > > -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext) > +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext) > { > - env->misa_mxl_max = env->misa_mxl = mxl; > env->misa_ext_mask = env->misa_ext = ext; > } > > @@ -367,11 +366,7 @@ static void riscv_any_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > -#if defined(TARGET_RISCV32) > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > -#elif defined(TARGET_RISCV64) > - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > -#endif > + riscv_cpu_set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > > #ifndef CONFIG_USER_ONLY > set_satp_mode_max_supported(RISCV_CPU(obj), > @@ -392,16 +387,14 @@ static void riscv_max_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - RISCVMXL mlx = MXL_RV64; > > -#ifdef TARGET_RISCV32 > - mlx = MXL_RV32; > -#endif > - riscv_cpu_set_misa(env, mlx, 0); > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > - set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ? > - VM_1_10_SV32 : VM_1_10_SV57); > +#ifdef TARGET_RISCV32 > + set_satp_mode_max_supported(cpu, VM_1_10_SV32); > +#else > + set_satp_mode_max_supported(cpu, VM_1_10_SV57); > +#endif > #endif > } > > @@ -409,8 +402,6 @@ static void riscv_max_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 */ > - riscv_cpu_set_misa(env, MXL_RV64, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -422,8 +413,7 @@ static void rv64_sifive_u_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - riscv_cpu_set_misa(env, MXL_RV64, > - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > + riscv_cpu_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); > @@ -441,7 +431,7 @@ static void rv64_sifive_e_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU); > + riscv_cpu_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); > @@ -458,7 +448,7 @@ static void rv64_thead_c906_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU); > + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU); > env->priv_ver = PRIV_VERSION_1_11_0; > > cpu->cfg.ext_zfa = true; > @@ -489,7 +479,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH); > + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU | RVH); > env->priv_ver = PRIV_VERSION_1_12_0; > > /* Enable ISA extensions */ > @@ -533,8 +523,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 */ > - riscv_cpu_set_misa(env, MXL_RV128, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -545,8 +533,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 */ > - riscv_cpu_set_misa(env, MXL_RV32, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -558,8 +544,7 @@ static void rv32_sifive_u_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - riscv_cpu_set_misa(env, MXL_RV32, > - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > + riscv_cpu_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); > @@ -577,7 +562,7 @@ static void rv32_sifive_e_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU); > + riscv_cpu_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); > @@ -594,7 +579,7 @@ static void rv32_ibex_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU); > + riscv_cpu_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); > @@ -612,7 +597,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU); > + riscv_cpu_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); > @@ -834,7 +819,7 @@ static void riscv_cpu_reset_hold(Object *obj) > mcc->parent_phases.hold(obj); > } > #ifndef CONFIG_USER_ONLY > - env->misa_mxl = env->misa_mxl_max; > + env->misa_mxl = mcc->misa_mxl_max; > env->priv = PRV_M; > env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV); > if (env->misa_mxl > MXL_RV32) { > @@ -1169,6 +1154,12 @@ static void riscv_cpu_post_init(Object *obj) > > static void riscv_cpu_init(Object *obj) > { > + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj); > + RISCVCPU *cpu = RISCV_CPU(obj); > + CPURISCVState *env = &cpu->env; > + > + env->misa_mxl = mcc->misa_mxl_max; > + > #ifndef CONFIG_USER_ONLY > qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq, > IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX); > @@ -1555,7 +1546,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); > @@ -1597,6 +1588,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) > { > @@ -1662,18 +1660,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[] = { > @@ -1686,29 +1688,31 @@ static const TypeInfo riscv_cpu_type_infos[] = { > .instance_post_init = riscv_cpu_post_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), > - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, riscv_max_cpu_init), > #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_ANY, MXL_RV32, riscv_any_cpu_init), > + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV32, riscv_max_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_ANY, MXL_RV64, riscv_any_cpu_init), > + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV64, riscv_max_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/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c > index 090d617627..186ca6e45c 100644 > --- a/target/riscv/kvm/kvm-cpu.c > +++ b/target/riscv/kvm/kvm-cpu.c > @@ -1461,14 +1461,14 @@ static void kvm_cpu_accel_register_types(void) > } > type_init(kvm_cpu_accel_register_types); > > -static void riscv_host_cpu_init(Object *obj) > +static void riscv_host_cpu_class_init(ObjectClass *c, void *data) > { > - CPURISCVState *env = &RISCV_CPU(obj)->env; > + RISCVCPUClass *mcc = RISCV_CPU_CLASS(c); > > #if defined(TARGET_RISCV32) > - env->misa_mxl_max = env->misa_mxl = MXL_RV32; > + mcc->misa_mxl_max = MXL_RV32; > #elif defined(TARGET_RISCV64) > - env->misa_mxl_max = env->misa_mxl = MXL_RV64; > + mcc->misa_mxl_max = MXL_RV64; > #endif > } > > @@ -1476,7 +1476,7 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = { > { > .name = TYPE_RISCV_CPU_HOST, > .parent = TYPE_RISCV_CPU, > - .instance_init = riscv_host_cpu_init, > + .class_init = riscv_host_cpu_class_init, > } > }; > > 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/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 7f45e42000..5bf9d31f7c 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -152,10 +152,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: > @@ -265,6 +264,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(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; > > @@ -445,7 +445,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcb), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmp), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); > } > } > @@ -453,7 +453,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_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); > } > if (riscv_has_ext(env, RVD)) { > @@ -461,7 +461,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; > } > @@ -861,7 +861,7 @@ static void riscv_init_max_cpu_extensions(Object *obj) > const RISCVCPUMultiExtConfig *prop; > > /* Enable RVG, RVJ and RVV that are disabled by default */ > - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV); > + riscv_cpu_set_misa_ext(env, env->misa_ext | RVG | RVJ | RVV); > > for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { > isa_ext_update_enabled(cpu, prop->offset, true); > 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] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-18 6:50 ` LIU Zhiwei @ 2023-10-18 12:23 ` Akihiko Odaki 0 siblings, 0 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-18 12:23 UTC (permalink / raw) To: LIU Zhiwei Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, qemu-riscv On 2023/10/18 15:50, LIU Zhiwei wrote: > > On 2023/10/18 2:53, 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> >> --- >> target/riscv/cpu-qom.h | 1 + >> target/riscv/cpu.h | 3 +- >> hw/riscv/boot.c | 2 +- >> target/riscv/cpu.c | 118 +++++++++++++++++++------------------ >> target/riscv/gdbstub.c | 12 ++-- >> target/riscv/kvm/kvm-cpu.c | 10 ++-- >> target/riscv/machine.c | 7 +-- >> target/riscv/tcg/tcg-cpu.c | 12 ++-- >> target/riscv/translate.c | 3 +- >> 9 files changed, 88 insertions(+), 80 deletions(-) >> >> diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h >> index f3fbe37a2c..33b6d52c90 100644 >> --- a/target/riscv/cpu-qom.h >> +++ b/target/riscv/cpu-qom.h >> @@ -68,5 +68,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 f8ffa5ee38..ef10efd1e7 100644 >> --- a/target/riscv/cpu.h >> +++ b/target/riscv/cpu.h >> @@ -159,7 +159,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 */ >> @@ -711,7 +710,7 @@ enum riscv_pmu_event_idx { >> /* used by tcg/tcg-cpu.c*/ >> void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool >> en); >> bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset); >> -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext); >> +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext); >> typedef struct RISCVCPUMultiExtConfig { >> const char *name; >> 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; > > Hi Akihiko, > > Can we use the cached CPUClass in CPUState? Like > > (RISCVCPUClass *)((CPUState *)(&harts->harts[0])->cc) If just casting, you can do: (RISCVCPUClass *)((Object *)&harts->harts[0])->class But it removes type safety checks RISCV_CPU_GET_CLASS() provides. This is not a hot path so it's better to keep the checks. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-17 18:53 ` [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class Akihiko Odaki 2023-10-18 6:50 ` LIU Zhiwei @ 2023-10-18 13:01 ` Daniel Henrique Barboza 2023-10-18 13:31 ` Akihiko Odaki 1 sibling, 1 reply; 14+ messages in thread From: Daniel Henrique Barboza @ 2023-10-18 13:01 UTC (permalink / raw) To: Akihiko Odaki Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv On 10/17/23 15:53, 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'll repeat what I said in the v1: this patch is adding an extra class parameter, an extra param required to each class_init, and an extra CPUClass cast every time we want to read misa_mxl_max, all of that because we want to assign gdb_core_xml_file earlier. If my previous suggestion of assigning gdb_core directly into riscv_cpu_class_init() doesn't work necause we need misa_mxl_max to do it, a good alternative is setting gdb_core_xml_file in riscv_cpu_post_init(), which is executed after all cpu_init() functions where we already have env->misa_mxl_max set properly. Something like this patch: diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index c64cd726f4..78732fd6cb 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1166,8 +1166,33 @@ static bool riscv_cpu_is_dynamic(Object *cpu_obj) return object_dynamic_cast(cpu_obj, TYPE_RISCV_DYNAMIC_CPU) != NULL; } +static void riscv_cpu_set_gdb_core_file(RISCVCPU *cpu) +{ + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu); + CPUClass *cc = CPU_CLASS(mcc); + CPURISCVState *env = &cpu->env; + + switch (env->misa_mxl_max) { + case MXL_RV64: + case MXL_RV128: + cc->gdb_core_xml_file = "riscv-64bit-cpu.xml"; + break; + case MXL_RV32: + cc->gdb_core_xml_file = "riscv-32bit-cpu.xml"; + break; + default: + g_assert_not_reached(); + } +} + static void riscv_cpu_post_init(Object *obj) { + /* + * Setting gdb_core requires env->misa_mxl_max to be already + * set via cpu_init(), and doing it during realize() is too + * late. + */ + riscv_cpu_set_gdb_core_file(RISCV_CPU(obj)); accel_cpu_instance_init(CPU(obj)); } diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 0680731770..15c17a7c4a 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -148,28 +148,6 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) } } -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) { -#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(); - } -} - static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp) { CPURISCVState *env = &cpu->env; @@ -589,8 +567,6 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp) return false; } - riscv_cpu_validate_misa_mxl(cpu); - riscv_cpu_validate_priv_spec(cpu, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); Something like this, coupled with your first patch, will init gdb_core_xml_file during init() and will remove riscv_cpu_validate_misa_mxl() since it's now doing nothing. Thanks, Daniel > target/riscv/cpu-qom.h | 1 + > target/riscv/cpu.h | 3 +- > hw/riscv/boot.c | 2 +- > target/riscv/cpu.c | 118 +++++++++++++++++++------------------ > target/riscv/gdbstub.c | 12 ++-- > target/riscv/kvm/kvm-cpu.c | 10 ++-- > target/riscv/machine.c | 7 +-- > target/riscv/tcg/tcg-cpu.c | 12 ++-- > target/riscv/translate.c | 3 +- > 9 files changed, 88 insertions(+), 80 deletions(-) > > diff --git a/target/riscv/cpu-qom.h b/target/riscv/cpu-qom.h > index f3fbe37a2c..33b6d52c90 100644 > --- a/target/riscv/cpu-qom.h > +++ b/target/riscv/cpu-qom.h > @@ -68,5 +68,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 f8ffa5ee38..ef10efd1e7 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -159,7 +159,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 */ > @@ -711,7 +710,7 @@ enum riscv_pmu_event_idx { > /* used by tcg/tcg-cpu.c*/ > void isa_ext_update_enabled(RISCVCPU *cpu, uint32_t ext_offset, bool en); > bool isa_ext_is_enabled(RISCVCPU *cpu, uint32_t ext_offset); > -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext); > +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext); > > typedef struct RISCVCPUMultiExtConfig { > const char *name; > 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 ac4a6c7eec..1fb5747f00 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -263,9 +263,8 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, bool async) > } > } > > -void riscv_cpu_set_misa(CPURISCVState *env, RISCVMXL mxl, uint32_t ext) > +void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext) > { > - env->misa_mxl_max = env->misa_mxl = mxl; > env->misa_ext_mask = env->misa_ext = ext; > } > > @@ -367,11 +366,7 @@ static void riscv_any_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > -#if defined(TARGET_RISCV32) > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > -#elif defined(TARGET_RISCV64) > - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > -#endif > + riscv_cpu_set_misa_ext(env, RVI | RVM | RVA | RVF | RVD | RVC | RVU); > > #ifndef CONFIG_USER_ONLY > set_satp_mode_max_supported(RISCV_CPU(obj), > @@ -392,16 +387,14 @@ static void riscv_max_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - RISCVMXL mlx = MXL_RV64; > > -#ifdef TARGET_RISCV32 > - mlx = MXL_RV32; > -#endif > - riscv_cpu_set_misa(env, mlx, 0); > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > - set_satp_mode_max_supported(RISCV_CPU(obj), mlx == MXL_RV32 ? > - VM_1_10_SV32 : VM_1_10_SV57); > +#ifdef TARGET_RISCV32 > + set_satp_mode_max_supported(cpu, VM_1_10_SV32); > +#else > + set_satp_mode_max_supported(cpu, VM_1_10_SV57); > +#endif > #endif > } > > @@ -409,8 +402,6 @@ static void riscv_max_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 */ > - riscv_cpu_set_misa(env, MXL_RV64, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -422,8 +413,7 @@ static void rv64_sifive_u_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - riscv_cpu_set_misa(env, MXL_RV64, > - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > + riscv_cpu_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); > @@ -441,7 +431,7 @@ static void rv64_sifive_e_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVI | RVM | RVA | RVC | RVU); > + riscv_cpu_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); > @@ -458,7 +448,7 @@ static void rv64_thead_c906_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU); > + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU); > env->priv_ver = PRIV_VERSION_1_11_0; > > cpu->cfg.ext_zfa = true; > @@ -489,7 +479,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV64, RVG | RVC | RVS | RVU | RVH); > + riscv_cpu_set_misa_ext(env, RVG | RVC | RVS | RVU | RVH); > env->priv_ver = PRIV_VERSION_1_12_0; > > /* Enable ISA extensions */ > @@ -533,8 +523,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 */ > - riscv_cpu_set_misa(env, MXL_RV128, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -545,8 +533,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 */ > - riscv_cpu_set_misa(env, MXL_RV32, 0); > /* Set latest version of privileged specification */ > env->priv_ver = PRIV_VERSION_LATEST; > #ifndef CONFIG_USER_ONLY > @@ -558,8 +544,7 @@ static void rv32_sifive_u_cpu_init(Object *obj) > { > RISCVCPU *cpu = RISCV_CPU(obj); > CPURISCVState *env = &cpu->env; > - riscv_cpu_set_misa(env, MXL_RV32, > - RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU); > + riscv_cpu_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); > @@ -577,7 +562,7 @@ static void rv32_sifive_e_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVC | RVU); > + riscv_cpu_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); > @@ -594,7 +579,7 @@ static void rv32_ibex_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVC | RVU); > + riscv_cpu_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); > @@ -612,7 +597,7 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj) > CPURISCVState *env = &RISCV_CPU(obj)->env; > RISCVCPU *cpu = RISCV_CPU(obj); > > - riscv_cpu_set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU); > + riscv_cpu_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); > @@ -834,7 +819,7 @@ static void riscv_cpu_reset_hold(Object *obj) > mcc->parent_phases.hold(obj); > } > #ifndef CONFIG_USER_ONLY > - env->misa_mxl = env->misa_mxl_max; > + env->misa_mxl = mcc->misa_mxl_max; > env->priv = PRV_M; > env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV); > if (env->misa_mxl > MXL_RV32) { > @@ -1169,6 +1154,12 @@ static void riscv_cpu_post_init(Object *obj) > > static void riscv_cpu_init(Object *obj) > { > + RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(obj); > + RISCVCPU *cpu = RISCV_CPU(obj); > + CPURISCVState *env = &cpu->env; > + > + env->misa_mxl = mcc->misa_mxl_max; > + > #ifndef CONFIG_USER_ONLY > qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq, > IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX); > @@ -1555,7 +1546,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); > @@ -1597,6 +1588,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) > { > @@ -1662,18 +1660,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[] = { > @@ -1686,29 +1688,31 @@ static const TypeInfo riscv_cpu_type_infos[] = { > .instance_post_init = riscv_cpu_post_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), > - DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, riscv_max_cpu_init), > #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_ANY, MXL_RV32, riscv_any_cpu_init), > + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV32, riscv_max_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_ANY, MXL_RV64, riscv_any_cpu_init), > + DEFINE_DYNAMIC_CPU(TYPE_RISCV_CPU_MAX, MXL_RV64, riscv_max_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/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c > index 090d617627..186ca6e45c 100644 > --- a/target/riscv/kvm/kvm-cpu.c > +++ b/target/riscv/kvm/kvm-cpu.c > @@ -1461,14 +1461,14 @@ static void kvm_cpu_accel_register_types(void) > } > type_init(kvm_cpu_accel_register_types); > > -static void riscv_host_cpu_init(Object *obj) > +static void riscv_host_cpu_class_init(ObjectClass *c, void *data) > { > - CPURISCVState *env = &RISCV_CPU(obj)->env; > + RISCVCPUClass *mcc = RISCV_CPU_CLASS(c); > > #if defined(TARGET_RISCV32) > - env->misa_mxl_max = env->misa_mxl = MXL_RV32; > + mcc->misa_mxl_max = MXL_RV32; > #elif defined(TARGET_RISCV64) > - env->misa_mxl_max = env->misa_mxl = MXL_RV64; > + mcc->misa_mxl_max = MXL_RV64; > #endif > } > > @@ -1476,7 +1476,7 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = { > { > .name = TYPE_RISCV_CPU_HOST, > .parent = TYPE_RISCV_CPU, > - .instance_init = riscv_host_cpu_init, > + .class_init = riscv_host_cpu_class_init, > } > }; > > 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/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 7f45e42000..5bf9d31f7c 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -152,10 +152,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: > @@ -265,6 +264,7 @@ static void riscv_cpu_disable_priv_spec_isa_exts(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; > > @@ -445,7 +445,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcb), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcmp), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); > } > } > @@ -453,7 +453,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_auto_update(cpu, CPU_CFG_OFFSET(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_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true); > } > if (riscv_has_ext(env, RVD)) { > @@ -461,7 +461,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; > } > @@ -861,7 +861,7 @@ static void riscv_init_max_cpu_extensions(Object *obj) > const RISCVCPUMultiExtConfig *prop; > > /* Enable RVG, RVJ and RVV that are disabled by default */ > - riscv_cpu_set_misa(env, env->misa_mxl, env->misa_ext | RVG | RVJ | RVV); > + riscv_cpu_set_misa_ext(env, env->misa_ext | RVG | RVJ | RVV); > > for (prop = riscv_cpu_extensions; prop && prop->name; prop++) { > isa_ext_update_enabled(cpu, prop->offset, true); > 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 related [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-18 13:01 ` Daniel Henrique Barboza @ 2023-10-18 13:31 ` Akihiko Odaki 2023-10-18 14:23 ` Daniel Henrique Barboza 0 siblings, 1 reply; 14+ messages in thread From: Akihiko Odaki @ 2023-10-18 13:31 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv On 2023/10/18 22:01, Daniel Henrique Barboza wrote: > > > On 10/17/23 15:53, 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'll repeat what I said in the v1: this patch is adding an extra class > parameter, > an extra param required to each class_init, and an extra CPUClass cast > every time > we want to read misa_mxl_max, all of that because we want to assign > gdb_core_xml_file > earlier. > > If my previous suggestion of assigning gdb_core directly into > riscv_cpu_class_init() > doesn't work necause we need misa_mxl_max to do it, a good alternative > is setting > gdb_core_xml_file in riscv_cpu_post_init(), which is executed after all > cpu_init() > functions where we already have env->misa_mxl_max set properly. We want to assign gdb_core_xml_file *earlier* so assigning it after cpu_init() is not OK. In general it should be considered unsafe to initialize a class variable after class_init(); otherwise other subsystems cannot know when it becomes available. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-18 13:31 ` Akihiko Odaki @ 2023-10-18 14:23 ` Daniel Henrique Barboza 2023-10-19 10:17 ` Akihiko Odaki 0 siblings, 1 reply; 14+ messages in thread From: Daniel Henrique Barboza @ 2023-10-18 14:23 UTC (permalink / raw) To: Akihiko Odaki Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv On 10/18/23 10:31, Akihiko Odaki wrote: > On 2023/10/18 22:01, Daniel Henrique Barboza wrote: >> >> >> On 10/17/23 15:53, 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'll repeat what I said in the v1: this patch is adding an extra class parameter, >> an extra param required to each class_init, and an extra CPUClass cast every time >> we want to read misa_mxl_max, all of that because we want to assign gdb_core_xml_file >> earlier. >> >> If my previous suggestion of assigning gdb_core directly into riscv_cpu_class_init() >> doesn't work necause we need misa_mxl_max to do it, a good alternative is setting >> gdb_core_xml_file in riscv_cpu_post_init(), which is executed after all cpu_init() >> functions where we already have env->misa_mxl_max set properly. > > We want to assign gdb_core_xml_file *earlier* so assigning it after cpu_init() is not OK. In general it should be considered unsafe to initialize a class variable after class_init(); otherwise other subsystems cannot know when it becomes available. There's no difference in assigning it during .instance_init (cpu_init()) and .instance_post_init. The callbacks are called one after the other during init time. See qom/object.c, object_initialize_with_type(). Now, if we want to be strict with only initializing class variables during class_init(), which is something that I will defer to the maintainers, we want to minimize the use of env->misa_mxl_max in the code before converting it to a class variable. env->misa_mxl is always equal to env->misa_mxl_max, and they are being used interchangeably, but if misa_mxl_max is going to be a class variable then we want to use env->misa_mxl instead to avoid the RISCV_CPU_GET_CLASS() overhead. I suggest a pre-patch to change the uses of env->misa_mxl_max to env->misa_mxl in riscv_is_32bit(), riscv_cpu_gdb_read_register(), riscv_cpu_gdb_write_register() and so on (there are quite a few places). And then apply this patch on top of it. This would at least minimize the amount of casts being done. All this considered, there's still one extra class cast that we will have to deal with in riscv_tr_init_disas_context(). I am not sure how hot this function is but, taking a look at other .init_disas_context implementations from other archs and not finding class casts in them, I'm worried about the overhead it'll add. It seems like we can just do "ctx->misa_mxl_max = env->misa_mxl" to avoid it. Thanks, Daniel ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class 2023-10-18 14:23 ` Daniel Henrique Barboza @ 2023-10-19 10:17 ` Akihiko Odaki 0 siblings, 0 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-19 10:17 UTC (permalink / raw) To: Daniel Henrique Barboza Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, qemu-riscv On 2023/10/18 23:23, Daniel Henrique Barboza wrote: > > > On 10/18/23 10:31, Akihiko Odaki wrote: >> On 2023/10/18 22:01, Daniel Henrique Barboza wrote: >>> >>> >>> On 10/17/23 15:53, 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'll repeat what I said in the v1: this patch is adding an extra >>> class parameter, >>> an extra param required to each class_init, and an extra CPUClass >>> cast every time >>> we want to read misa_mxl_max, all of that because we want to assign >>> gdb_core_xml_file >>> earlier. >>> >>> If my previous suggestion of assigning gdb_core directly into >>> riscv_cpu_class_init() >>> doesn't work necause we need misa_mxl_max to do it, a good >>> alternative is setting >>> gdb_core_xml_file in riscv_cpu_post_init(), which is executed after >>> all cpu_init() >>> functions where we already have env->misa_mxl_max set properly. >> >> We want to assign gdb_core_xml_file *earlier* so assigning it after >> cpu_init() is not OK. In general it should be considered unsafe to >> initialize a class variable after class_init(); otherwise other >> subsystems cannot know when it becomes available. > > There's no difference in assigning it during .instance_init (cpu_init()) > and > .instance_post_init. The callbacks are called one after the other during > init time. See qom/object.c, object_initialize_with_type(). > > Now, if we want to be strict with only initializing class variables > during class_init(), > which is something that I will defer to the maintainers, we want to > minimize the use > of env->misa_mxl_max in the code before converting it to a class > variable. env->misa_mxl > is always equal to env->misa_mxl_max, and they are being used > interchangeably, but if > misa_mxl_max is going to be a class variable then we want to use > env->misa_mxl instead > to avoid the RISCV_CPU_GET_CLASS() overhead. > > I suggest a pre-patch to change the uses of env->misa_mxl_max to > env->misa_mxl in > riscv_is_32bit(), riscv_cpu_gdb_read_register(), > riscv_cpu_gdb_write_register() and > so on (there are quite a few places). And then apply this patch on top > of it. This would > at least minimize the amount of casts being done. misa_mxl and misa_mxl_max may be same for now, but they exist because they will be different in the future; otherwise there is no reason to have misa_mxl_max at the first place. misa_mxl represents the current effective value and misa_mxl_max represents the maximum value the emulated CPU supports. So we need to preserve the existing choice of misa_mxl and misa_mxl_max in general. riscv_cpu_gdb_read_register() and riscv_cpu_gdb_write_register(), for example, must use misa_mxl_max instead of misa_mxl since GDB doesn't support changing register width at runtime, and the register width should match with the feature description provided by gdb_core_xml_file. The use of env->misa_mxl_max in riscv_is_32bit(), however, looks not good to me. If a 64-bit processor is in 32-bit mode, the 32-bit kernel should be loaded. I added a patch to change it to env->misa_mxl in v5. > > All this considered, there's still one extra class cast that we will > have to deal with > in riscv_tr_init_disas_context(). I am not sure how hot this function is > but, taking a > look at other .init_disas_context implementations from other archs and > not finding class > casts in them, I'm worried about the overhead it'll add. It seems like > we can just do > "ctx->misa_mxl_max = env->misa_mxl" to avoid it. The cost of the class type check should be negligible considering that init_disas_context() is followed by the TCG translation, which takes most of execution time. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 4/5] target/riscv: Validate misa_mxl_max only once 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki ` (2 preceding siblings ...) 2023-10-17 18:53 ` [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class Akihiko Odaki @ 2023-10-17 18:53 ` Akihiko Odaki 2023-10-17 18:54 ` [PATCH v4 5/5] plugins: Remove an extra parameter Akihiko Odaki 4 siblings, 0 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-17 18:53 UTC (permalink / raw) Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, Akihiko Odaki, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv 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 | 21 +++++++++++++++++++++ target/riscv/tcg/tcg-cpu.c | 23 ----------------------- 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 1fb5747f00..72124e57fd 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1193,6 +1193,26 @@ static const MISAExtInfo misa_ext_info_arr[] = { MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"), }; +static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc) +{ + CPUClass *cc = CPU_CLASS(mcc); + + /* Validate that MISA_MXL is set properly. */ + switch (mcc->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(); + } +} + static int riscv_validate_misa_info_idx(uint32_t bit) { int idx; @@ -1593,6 +1613,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, diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 5bf9d31f7c..a82c49ef67 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -148,27 +148,6 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState *env, Error **errp) } } -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu) -{ - RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu); - CPUClass *cc = CPU_CLASS(mcc); - - /* Validate that MISA_MXL is set properly. */ - switch (mcc->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(); - } -} - static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp) { CPURISCVState *env = &cpu->env; @@ -568,8 +547,6 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp) return false; } - riscv_cpu_validate_misa_mxl(cpu); - riscv_cpu_validate_priv_spec(cpu, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); -- 2.42.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 5/5] plugins: Remove an extra parameter 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki ` (3 preceding siblings ...) 2023-10-17 18:53 ` [PATCH v4 4/5] target/riscv: Validate misa_mxl_max only once Akihiko Odaki @ 2023-10-17 18:54 ` Akihiko Odaki 4 siblings, 0 replies; 14+ messages in thread From: Akihiko Odaki @ 2023-10-17 18:54 UTC (permalink / raw) Cc: Alex Bennée, Mikhail Tyutin, Aleksandr Anenkov, qemu-devel, Philippe Mathieu-Daudé, Fabiano Rosas, 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] 14+ messages in thread
end of thread, other threads:[~2023-10-19 10:18 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-17 18:53 [PATCH v4 0/5] gdbstub and TCG plugin improvements Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 1/5] gdbstub: Check if gdb_regs is NULL Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 2/5] target/riscv: Remove misa_mxl validation Akihiko Odaki 2023-10-18 5:54 ` LIU Zhiwei 2023-10-18 12:51 ` Daniel Henrique Barboza 2023-10-17 18:53 ` [PATCH v4 3/5] target/riscv: Move misa_mxl_max to class Akihiko Odaki 2023-10-18 6:50 ` LIU Zhiwei 2023-10-18 12:23 ` Akihiko Odaki 2023-10-18 13:01 ` Daniel Henrique Barboza 2023-10-18 13:31 ` Akihiko Odaki 2023-10-18 14:23 ` Daniel Henrique Barboza 2023-10-19 10:17 ` Akihiko Odaki 2023-10-17 18:53 ` [PATCH v4 4/5] target/riscv: Validate misa_mxl_max only once Akihiko Odaki 2023-10-17 18:54 ` [PATCH v4 5/5] 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).