| Message ID | 20250206182711.2420505-3-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: declarative CPU definitions | expand |
On 2/6/25 10:26, Paolo Bonzini wrote: > Start putting all the CPU definitions in a struct. Later this will replace > instance_init functions with declarative code, for now just remove the > ugly cast of class_data. ... > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index ed9da692030..29cfae38b75 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -2955,7 +2955,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; > + mcc->misa_mxl_max = ((RISCVCPUDef *)data)->misa_mxl_max; > riscv_cpu_validate_misa_mxl(mcc); > } > > @@ -3051,40 +3051,48 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char *nodename) > } > #endif > > -#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, 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) \ > + .class_data = &((RISCVCPUDef) { \ > + .misa_mxl_max = (misa_mxl_max_), \ > + }), \ Drop the unnecessary (). It would be nice if this were const, i.e. .class_data = (void *) &(const RISCVCPUDef){ ... }, This will in fact create an anonymous object in .rodata. We have other uses that do the extra casting away const, e.g. armsse_variants in hw/arm/armsse.c. Although I suspect *all* usage of .class_init can and should be with const data. An unrelated cleanup, to be sure. r~
On 6/2/25 22:16, Richard Henderson wrote: > It would be nice if this were const, i.e. > > .class_data = (void *) &(const RISCVCPUDef){ > ... > }, > > This will in fact create an anonymous object in .rodata. > > We have other uses that do the extra casting away const, > e.g. armsse_variants in hw/arm/armsse.c. Although I suspect > *all* usage of .class_init can and should be with const data. The only non-const use I noticed is Xtensa: static void xtensa_finalize_config(XtensaConfig *config) { if (config->isa_internal) { init_libisa(config); } if (config->gdb_regmap.num_regs == 0 || config->gdb_regmap.num_core_regs == 0) { unsigned n_regs = 0; unsigned n_core_regs = 0; xtensa_count_regs(config, &n_regs, &n_core_regs); if (config->gdb_regmap.num_regs == 0) { config->gdb_regmap.num_regs = n_regs; } if (config->gdb_regmap.num_core_regs == 0) { config->gdb_regmap.num_core_regs = n_core_regs; } } } static void xtensa_core_class_init(ObjectClass *oc, void *data) { CPUClass *cc = CPU_CLASS(oc); XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc); XtensaConfig *config = data; xtensa_finalize_config(config); ...
On 9/2/25 19:44, Philippe Mathieu-Daudé wrote: > On 6/2/25 22:16, Richard Henderson wrote: > >> It would be nice if this were const, i.e. >> >> .class_data = (void *) &(const RISCVCPUDef){ >> ... >> }, >> >> This will in fact create an anonymous object in .rodata. >> >> We have other uses that do the extra casting away const, >> e.g. armsse_variants in hw/arm/armsse.c. Although I suspect >> *all* usage of .class_init can and should be with const data. > > The only non-const use I noticed is Xtensa: > > static void xtensa_finalize_config(XtensaConfig *config) > { > if (config->isa_internal) { > init_libisa(config); > } > > if (config->gdb_regmap.num_regs == 0 || > config->gdb_regmap.num_core_regs == 0) { > unsigned n_regs = 0; > unsigned n_core_regs = 0; > > xtensa_count_regs(config, &n_regs, &n_core_regs); > if (config->gdb_regmap.num_regs == 0) { > config->gdb_regmap.num_regs = n_regs; > } > if (config->gdb_regmap.num_core_regs == 0) { > config->gdb_regmap.num_core_regs = n_core_regs; > } > } > } > > static void xtensa_core_class_init(ObjectClass *oc, void *data) > { > CPUClass *cc = CPU_CLASS(oc); > XtensaCPUClass *xcc = XTENSA_CPU_CLASS(oc); > XtensaConfig *config = data; > > xtensa_finalize_config(config); > ... Which I suppose can be fixed by calling xtensa_finalize_config() somewhere within the class register: void xtensa_register_core(XtensaConfigList *node) { TypeInfo type = { .parent = TYPE_XTENSA_CPU, .class_init = xtensa_core_class_init, .class_data = (void *)node->config, };
On 9/2/25 19:53, Philippe Mathieu-Daudé wrote: > On 9/2/25 19:44, Philippe Mathieu-Daudé wrote: >> On 6/2/25 22:16, Richard Henderson wrote: >> >>> It would be nice if this were const, i.e. >>> >>> .class_data = (void *) &(const RISCVCPUDef){ >>> ... >>> }, >>> >>> This will in fact create an anonymous object in .rodata. >>> >>> We have other uses that do the extra casting away const, >>> e.g. armsse_variants in hw/arm/armsse.c. Although I suspect >>> *all* usage of .class_init can and should be with const data. >> >> The only non-const use I noticed is Xtensa: Also the object_class_foreach() callbacks update 'data': static void object_class_get_list_tramp(ObjectClass *klass, void *opaque) { GSList **list = opaque; *list = g_slist_prepend(*list, klass); } GSList *object_class_get_list(const char *implements_type, bool include_abstract) { GSList *list = NULL; object_class_foreach(object_class_get_list_tramp, implements_type, include_abstract, &list); return list; }
On 9/2/25 23:20, Philippe Mathieu-Daudé wrote: > On 9/2/25 19:53, Philippe Mathieu-Daudé wrote: >> On 9/2/25 19:44, Philippe Mathieu-Daudé wrote: >>> On 6/2/25 22:16, Richard Henderson wrote: >>> >>>> It would be nice if this were const, i.e. >>>> >>>> .class_data = (void *) &(const RISCVCPUDef){ >>>> ... >>>> }, >>>> >>>> This will in fact create an anonymous object in .rodata. >>>> >>>> We have other uses that do the extra casting away const, >>>> e.g. armsse_variants in hw/arm/armsse.c. Although I suspect >>>> *all* usage of .class_init can and should be with const data. >>> >>> The only non-const use I noticed is Xtensa: > > Also the object_class_foreach() callbacks update 'data': Oops I misread, object_class_foreach() correctly takes non-const data :)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 97713681cbe..b2c9302634d 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -517,6 +517,10 @@ struct ArchCPU { const GPtrArray *decoders; }; +typedef struct RISCVCPUDef { + RISCVMXL misa_mxl_max; /* max mxl for this cpu */ +} RISCVCPUDef; + /** * RISCVCPUClass: * @parent_realize: The parent class' realize handler. diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index ed9da692030..29cfae38b75 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -2955,7 +2955,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; + mcc->misa_mxl_max = ((RISCVCPUDef *)data)->misa_mxl_max; riscv_cpu_validate_misa_mxl(mcc); } @@ -3051,40 +3051,48 @@ void riscv_isa_write_fdt(RISCVCPU *cpu, void *fdt, char *nodename) } #endif -#define DEFINE_DYNAMIC_CPU(type_name, misa_mxl_max, 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) \ + .class_data = &((RISCVCPUDef) { \ + .misa_mxl_max = (misa_mxl_max_), \ + }), \ } -#define DEFINE_VENDOR_CPU(type_name, misa_mxl_max, initfn) \ +#define DEFINE_VENDOR_CPU(type_name, misa_mxl_max_, initfn) \ { \ .name = (type_name), \ .parent = TYPE_RISCV_VENDOR_CPU, \ .instance_init = (initfn), \ .class_init = riscv_cpu_class_init, \ - .class_data = (void *)(misa_mxl_max) \ + .class_data = &((RISCVCPUDef) { \ + .misa_mxl_max = (misa_mxl_max_), \ + }), \ } -#define DEFINE_BARE_CPU(type_name, misa_mxl_max, initfn) \ +#define DEFINE_BARE_CPU(type_name, misa_mxl_max_, initfn) \ { \ .name = (type_name), \ .parent = TYPE_RISCV_BARE_CPU, \ .instance_init = (initfn), \ .class_init = riscv_cpu_class_init, \ - .class_data = (void *)(misa_mxl_max) \ + .class_data = &((RISCVCPUDef) { \ + .misa_mxl_max = (misa_mxl_max_), \ + }), \ } -#define DEFINE_PROFILE_CPU(type_name, misa_mxl_max, initfn) \ +#define DEFINE_PROFILE_CPU(type_name, misa_mxl_max_, initfn) \ { \ .name = (type_name), \ .parent = TYPE_RISCV_BARE_CPU, \ .instance_init = (initfn), \ .class_init = riscv_cpu_class_init, \ - .class_data = (void *)(misa_mxl_max) \ + .class_data = &((RISCVCPUDef) { \ + .misa_mxl_max = (misa_mxl_max_), \ + }), \ } static const TypeInfo riscv_cpu_type_infos[] = {
Start putting all the CPU definitions in a struct. Later this will replace instance_init functions with declarative code, for now just remove the ugly cast of class_data. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- target/riscv/cpu.h | 4 ++++ target/riscv/cpu.c | 26 +++++++++++++++++--------- 2 files changed, 21 insertions(+), 9 deletions(-)