On Mon, Apr 28, 2025 at 1:50 PM Daniel Henrique Barboza
<dbarb...@ventanamicro.com> wrote:
>
>
>
> On 4/28/25 4:34 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.fran...@wdc.com>
> > Signed-off-by: Paolo Bonzini <pbonz...@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 62e303f0635..842c9d3f194 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -553,7 +553,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 f30cf1b532b..d8c189d596b 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
> > @@ -1048,7 +1048,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) {
> > @@ -1450,7 +1450,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,
> > @@ -1544,7 +1544,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:
> > @@ -3071,12 +3071,24 @@ static void riscv_cpu_common_class_init(ObjectClass
> > *c, const void *data)
> > device_class_set_props(dc, riscv_cpu_properties);
> > }
> >
> > +static void riscv_cpu_class_base_init(ObjectClass *c, const 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, const 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);
> > }
> >
> > @@ -3227,6 +3239,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 75724b6af4f..41b6f34552a 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, const 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
> > }
> > };
>
> Are we sure this patch compiles?
No, you're right; I was not aware that RISC-V KVM is not covered by
CI. I'm sorry.
https://lore.kernel.org/qemu-devel/20250425152843.69638-1-phi...@linaro.org/T/#t
> As I said in the v3 this except opening 2 parentheses
> and closing just one after RISCVCPUDef:
>
> The error is fixable by closing both parentheses right before the comma, e.g:
>
> > + }),
>
>
> Can you mention the pull requests you based this series on top of? I can
> apply the
> dependencies and this series on top of it to see if it builds.
It's based on master +
https://lore.kernel.org/qemu-devel/20250425152843.69638-1-phi...@linaro.org/.
Paolo
