From: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: alistair23@gmail.com, Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH 08/27] target/riscv: store RISCVCPUDef struct directly in the class
Date: Thu, 24 Apr 2025 10:52:24 -0300 [thread overview]
Message-ID: <8f3bae37-e1f3-4e55-9dc6-b7876992b47e@ventanamicro.com> (raw)
In-Reply-To: <20250406070254.274797-9-pbonzini@redhat.com>
Hi,
This patch breaks RISC-V KVM build in my env. The issues are down there:
On 4/6/25 4:02 AM, Paolo Bonzini wrote:
> Prepare for adding more fields to RISCVCPUDef and reading them in
> riscv_cpu_init: instead of storing the misa_mxl_max field in
> RISCVCPUClass, ensure that there's always a valid RISCVCPUDef struct
> and go through it.
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/riscv/cpu.h | 2 +-
> hw/riscv/boot.c | 2 +-
> target/riscv/cpu.c | 23 ++++++++++++++++++-----
> target/riscv/gdbstub.c | 6 +++---
> target/riscv/kvm/kvm-cpu.c | 21 +++++++++------------
> target/riscv/machine.c | 2 +-
> target/riscv/tcg/tcg-cpu.c | 10 +++++-----
> target/riscv/translate.c | 2 +-
> 8 files changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 65c8d6855ec..9bbfdcf6758 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -557,7 +557,7 @@ struct RISCVCPUClass {
>
> DeviceRealize parent_realize;
> ResettablePhases parent_phases;
> - RISCVMXL misa_mxl_max; /* max mxl for this cpu */
> + RISCVCPUDef *def;
> };
>
> static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 765b9e2b1ab..828a867be39 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -37,7 +37,7 @@
> bool riscv_is_32bit(RISCVHartArrayState *harts)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(&harts->harts[0]);
> - return mcc->misa_mxl_max == MXL_RV32;
> + return mcc->def->misa_mxl_max == MXL_RV32;
> }
>
> /*
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3bd2bff1328..25132e57380 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -357,7 +357,7 @@ void riscv_cpu_set_misa_ext(CPURISCVState *env, uint32_t ext)
>
> int riscv_cpu_max_xlen(RISCVCPUClass *mcc)
> {
> - return 16 << mcc->misa_mxl_max;
> + return 16 << mcc->def->misa_mxl_max;
> }
>
> #ifndef CONFIG_USER_ONLY
> @@ -1055,7 +1055,7 @@ static void riscv_cpu_reset_hold(Object *obj, ResetType type)
> mcc->parent_phases.hold(obj, type);
> }
> #ifndef CONFIG_USER_ONLY
> - env->misa_mxl = mcc->misa_mxl_max;
> + env->misa_mxl = mcc->def->misa_mxl_max;
> env->priv = PRV_M;
> env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
> if (env->misa_mxl > MXL_RV32) {
> @@ -1457,7 +1457,7 @@ static void riscv_cpu_init(Object *obj)
> RISCVCPU *cpu = RISCV_CPU(obj);
> CPURISCVState *env = &cpu->env;
>
> - env->misa_mxl = mcc->misa_mxl_max;
> + env->misa_mxl = mcc->def->misa_mxl_max;
>
> #ifndef CONFIG_USER_ONLY
> qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
> @@ -1554,7 +1554,7 @@ 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) {
> + switch (mcc->def->misa_mxl_max) {
> #ifdef TARGET_RISCV64
> case MXL_RV64:
> case MXL_RV128:
> @@ -3079,12 +3079,24 @@ static void riscv_cpu_common_class_init(ObjectClass *c, void *data)
> device_class_set_props(dc, riscv_cpu_properties);
> }
>
> +static void riscv_cpu_class_base_init(ObjectClass *c, void *data)
> +{
> + RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> + RISCVCPUClass *pcc = RISCV_CPU_CLASS(object_class_get_parent(c));
> +
> + if (pcc->def) {
> + mcc->def = g_memdup2(pcc->def, sizeof(*pcc->def));
> + } else {
> + mcc->def = g_new0(RISCVCPUDef, 1);
> + }
> +}
> +
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> {
> RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> const RISCVCPUDef *def = data;
>
> - mcc->misa_mxl_max = def->misa_mxl_max;
> + mcc->def->misa_mxl_max = def->misa_mxl_max;
> riscv_cpu_validate_misa_mxl(mcc);
> }
>
> @@ -3235,6 +3247,7 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> .abstract = true,
> .class_size = sizeof(RISCVCPUClass),
> .class_init = riscv_cpu_common_class_init,
> + .class_base_init = riscv_cpu_class_base_init,
> },
> {
> .name = TYPE_RISCV_DYNAMIC_CPU,
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 18e88f416af..1934f919c01 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -62,7 +62,7 @@ int riscv_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
> return 0;
> }
>
> - switch (mcc->misa_mxl_max) {
> + switch (mcc->def->misa_mxl_max) {
> case MXL_RV32:
> return gdb_get_reg32(mem_buf, tmp);
> case MXL_RV64:
> @@ -82,7 +82,7 @@ int riscv_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> int length = 0;
> target_ulong tmp;
>
> - switch (mcc->misa_mxl_max) {
> + switch (mcc->def->misa_mxl_max) {
> case MXL_RV32:
> tmp = (int32_t)ldl_p(mem_buf);
> length = 4;
> @@ -359,7 +359,7 @@ void riscv_cpu_register_gdb_regs_for_features(CPUState *cs)
> ricsv_gen_dynamic_vector_feature(cs, cs->gdb_num_regs),
> 0);
> }
> - switch (mcc->misa_mxl_max) {
> + switch (mcc->def->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 0f4997a9186..d7e6970a670 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -1997,22 +1997,19 @@ static void kvm_cpu_accel_register_types(void)
> }
> type_init(kvm_cpu_accel_register_types);
>
> -static void riscv_host_cpu_class_init(ObjectClass *c, void *data)
> -{
> - RISCVCPUClass *mcc = RISCV_CPU_CLASS(c);
> -
> -#if defined(TARGET_RISCV32)
> - mcc->misa_mxl_max = MXL_RV32;
> -#elif defined(TARGET_RISCV64)
> - mcc->misa_mxl_max = MXL_RV64;
> -#endif
> -}
> -
> static const TypeInfo riscv_kvm_cpu_type_infos[] = {
> {
> .name = TYPE_RISCV_CPU_HOST,
> .parent = TYPE_RISCV_CPU,
> - .class_init = riscv_host_cpu_class_init,
> +#if defined(TARGET_RISCV32)
> + .class_data = &((const RISCVCPUDef) {
> + .misa_mxl_max = MXL_RV32,
> + },
> +#elif defined(TARGET_RISCV64)
> + .class_data = &((const RISCVCPUDef) {
> + .misa_mxl_max = MXL_RV64,
> + },
> +#endif
> }
> };
../target/riscv/kvm/kvm-cpu.c:2013:5: error: expected expression before '}' token
2013 | }
| ^
../target/riscv/kvm/kvm-cpu.c:2011:10: error: value computed is not used [-Werror=unused-value]
2011 | },
| ^
cc1: all warnings being treated as errors
[11/13] Linking target qemu-nbd
We're missing closing parenthesis after the "}".
If we fix that we'll get another error:
../target/riscv/kvm/kvm-cpu.c:2009:23: error: initialization discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
2009 | .class_data = &((const RISCVCPUDef) {
| ^
cc1: all warnings being treated as errors
Removing the 'const' qualifier fixes this other error.
This diff fixes the KVM build with this patch:
diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index d7e6970a67..c94110a726 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -2002,13 +2002,13 @@ static const TypeInfo riscv_kvm_cpu_type_infos[] = {
.name = TYPE_RISCV_CPU_HOST,
.parent = TYPE_RISCV_CPU,
#if defined(TARGET_RISCV32)
- .class_data = &((const RISCVCPUDef) {
+ .class_data = &((RISCVCPUDef) {
.misa_mxl_max = MXL_RV32,
- },
+ }),
#elif defined(TARGET_RISCV64)
- .class_data = &((const RISCVCPUDef) {
+ .class_data = &((RISCVCPUDef) {
.misa_mxl_max = MXL_RV64,
- },
+ }),
#endif
}
};
Thanks,
Daniel
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 889e2b65701..df2d5bad8d6 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -170,7 +170,7 @@ static bool rv128_needed(void *opaque)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(opaque);
>
> - return mcc->misa_mxl_max == MXL_RV128;
> + return mcc->def->misa_mxl_max == MXL_RV128;
> }
>
> static const VMStateDescription vmstate_rv128 = {
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 2f93e2dd285..b43bd3d35b7 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -581,7 +581,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> return;
> }
>
> - if (mcc->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> + if (mcc->def->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
> error_setg(errp, "Zcf extension is only relevant to RV32");
> return;
> }
> @@ -678,7 +678,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> return;
> }
>
> - if (mcc->misa_mxl_max == MXL_RV32 && cpu->cfg.ext_svukte) {
> + if (mcc->def->misa_mxl_max == MXL_RV32 && cpu->cfg.ext_svukte) {
> error_setg(errp, "svukte is not supported for RV32");
> return;
> }
> @@ -916,7 +916,7 @@ static void cpu_enable_zc_implied_rules(RISCVCPU *cpu)
> 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) && mcc->misa_mxl_max == MXL_RV32) {
> + if (riscv_has_ext(env, RVF) && mcc->def->misa_mxl_max == MXL_RV32) {
> cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
> }
> }
> @@ -925,7 +925,7 @@ static void cpu_enable_zc_implied_rules(RISCVCPU *cpu)
> 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) && mcc->misa_mxl_max == MXL_RV32) {
> + if (riscv_has_ext(env, RVF) && mcc->def->misa_mxl_max == MXL_RV32) {
> cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zcf), true);
> }
>
> @@ -1049,7 +1049,7 @@ static bool riscv_tcg_cpu_realize(CPUState *cs, Error **errp)
> return false;
> }
>
> - if (mcc->misa_mxl_max >= MXL_RV128 && qemu_tcg_mttcg_enabled()) {
> + if (mcc->def->misa_mxl_max >= MXL_RV128 && qemu_tcg_mttcg_enabled()) {
> /* Missing 128-bit aligned atomics */
> error_setg(errp,
> "128-bit RISC-V currently does not work with Multi "
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index d6651f244f6..e22ecd11565 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1282,7 +1282,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 = mcc->misa_mxl_max;
> + ctx->misa_mxl_max = mcc->def->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;
next prev parent reply other threads:[~2025-04-24 13:53 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-04-06 7:02 [PATCH 10.1 v3 00/27] target/riscv: SATP mode and CPU definition overhaul Paolo Bonzini
2025-04-06 7:02 ` [PATCH 01/27] hw/riscv: acpi: only create RHCT MMU entry for supported types Paolo Bonzini
2025-04-06 7:02 ` [PATCH 02/27] target/riscv: assert argument to set_satp_mode_max_supported is valid Paolo Bonzini
2025-04-06 7:02 ` [PATCH 03/27] target/riscv: cpu: store max SATP mode as a single integer Paolo Bonzini
2025-04-06 7:02 ` [PATCH 04/27] target/riscv: update max_satp_mode based on QOM properties Paolo Bonzini
2025-04-06 7:02 ` [PATCH 05/27] target/riscv: remove supported from RISCVSATPMap Paolo Bonzini
2025-04-06 7:02 ` [PATCH 06/27] target/riscv: move satp_mode.{map, init} out of CPUConfig Paolo Bonzini via
2025-04-06 7:02 ` [PATCH 07/27] target/riscv: introduce RISCVCPUDef Paolo Bonzini
2025-04-06 23:21 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 08/27] target/riscv: store RISCVCPUDef struct directly in the class Paolo Bonzini
2025-04-24 13:52 ` Daniel Henrique Barboza [this message]
2025-04-24 14:04 ` Philippe Mathieu-Daudé
2025-04-24 14:21 ` Daniel Henrique Barboza
2025-04-06 7:02 ` [PATCH 09/27] target/riscv: merge riscv_cpu_class_init with the class_base function Paolo Bonzini
2025-04-06 7:02 ` [PATCH 10/27] target/riscv: move RISCVCPUConfig fields to a header file Paolo Bonzini
2025-04-06 7:02 ` [PATCH 11/27] target/riscv: include default value in cpu_cfg_fields.h.inc Paolo Bonzini
2025-04-09 4:53 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 12/27] target/riscv: do not make RISCVCPUConfig fields conditional Paolo Bonzini
2025-04-09 5:12 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 13/27] target/riscv: add more RISCVCPUDef fields Paolo Bonzini
2025-04-22 4:47 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 14/27] target/riscv: convert abstract CPU classes to RISCVCPUDef Paolo Bonzini
2025-04-24 0:50 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 15/27] target/riscv: convert profile CPU models " Paolo Bonzini
2025-04-24 0:11 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 16/27] target/riscv: convert bare " Paolo Bonzini
2025-04-24 0:12 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 17/27] target/riscv: convert dynamic " Paolo Bonzini
2025-04-24 0:15 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 18/27] target/riscv: convert SiFive E " Paolo Bonzini
2025-04-24 0:22 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 19/27] target/riscv: convert ibex " Paolo Bonzini
2025-04-24 0:23 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 20/27] target/riscv: convert SiFive U " Paolo Bonzini
2025-04-24 0:25 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 21/27] target/riscv: th: make CSR insertion test a bit more intuitive Paolo Bonzini
2025-04-24 0:32 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 22/27] target/riscv: generalize custom CSR functionality Paolo Bonzini
2025-04-24 0:36 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 23/27] target/riscv: convert TT C906 to RISCVCPUDef Paolo Bonzini
2025-04-24 0:37 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 24/27] target/riscv: convert TT Ascalon " Paolo Bonzini
2025-04-24 0:38 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 25/27] target/riscv: convert Ventana V1 " Paolo Bonzini
2025-04-24 0:45 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 26/27] target/riscv: convert Xiangshan Nanhu " Paolo Bonzini
2025-04-24 0:47 ` Alistair Francis
2025-04-06 7:02 ` [PATCH 27/27] target/riscv: remove .instance_post_init Paolo Bonzini
2025-04-24 0:48 ` Alistair Francis
2025-04-24 1:26 ` [PATCH 10.1 v3 00/27] target/riscv: SATP mode and CPU definition overhaul Alistair Francis
2025-04-24 14:39 ` Paolo Bonzini
2025-04-25 10:55 ` Paolo Bonzini
2025-04-25 11:02 ` Philippe Mathieu-Daudé
2025-04-25 11:03 ` Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8f3bae37-e1f3-4e55-9dc6-b7876992b47e@ventanamicro.com \
--to=dbarboza@ventanamicro.com \
--cc=alistair.francis@wdc.com \
--cc=alistair23@gmail.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.