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


Reply via email to