| Message ID | 20250418172908.25147-12-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: Make hw/arm/ common | expand |
On 4/18/25 10:29, Philippe Mathieu-Daudé wrote: > Add MachineClass::valid_cpu_types_list, a dynamic list of strings. > > CPU types can be registered with machine_class_add_valid_cpu_type(). > > Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > include/hw/boards.h | 8 ++++++++ > hw/core/machine.c | 30 ++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 02f43ac5d4d..647a29ff04d 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -56,6 +56,13 @@ void machine_set_cache_topo_level(MachineState *ms, CacheLevelAndType cache, > bool machine_check_smp_cache(const MachineState *ms, Error **errp); > void machine_memory_devices_init(MachineState *ms, hwaddr base, uint64_t size); > > +/** > + * machine_class_add_valid_cpu_type: Add type to list of valid CPUs > + * @mc: Machine class > + * @type: CPU type to allow (should be a subtype of TYPE_CPU) > + */ > +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type); > + > /** > * machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices > * @mc: Machine class > @@ -306,6 +313,7 @@ struct MachineClass { > bool ignore_memory_transaction_failures; > int numa_mem_align_shift; > const char * const *valid_cpu_types; > + GList *valid_cpu_types_list; > strList *allowed_dynamic_sysbus_devices; > bool auto_enable_numa_with_memhp; > bool auto_enable_numa_with_memdev; > diff --git a/hw/core/machine.c b/hw/core/machine.c > index f52a4f2273b..ff27d533b5c 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -1538,6 +1538,12 @@ const char *machine_class_default_cpu_type(MachineClass *mc) > return mc->default_cpu_type; > } > > +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type) > +{ > + mc->valid_cpu_types_list = g_list_prepend(mc->valid_cpu_types_list, > + g_strdup(type)); > +} > + > static bool is_cpu_type_supported(const MachineState *machine, Error **errp) > { > MachineClass *mc = MACHINE_GET_CLASS(machine); > @@ -1581,6 +1587,30 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) > return false; > } > } > + if (mc->valid_cpu_types_list) { > + bool valid = false; > + unsigned count = 0; > + GList *l; > + > + for (l = mc->valid_cpu_types_list; !valid && l != NULL; l = l->next) { > + valid |= !!object_class_dynamic_cast(oc, l->data); > + count++; > + } > + > + if (!valid) { > + g_autofree char *requested = cpu_model_from_type(machine->cpu_type); > + mc->valid_cpu_types_list = g_list_reverse(mc->valid_cpu_types_list); > + error_setg(errp, "Invalid CPU model: %s", requested); > + error_append_hint(errp, "The valid models are: "); > + for (l = mc->valid_cpu_types_list; l != NULL; l = l->next) { > + g_autofree char *model = cpu_model_from_type(l->data); > + error_append_hint(errp, "%s%s", model, --count ? ", " : ""); > + } > + error_append_hint(errp, "\n"); > + > + return false; > + } > + } > > /* Check if CPU type is deprecated and warn if so */ > cc = CPU_CLASS(oc); How about simply changing valid_cpu_types to become a function instead of a static list? This way, it's simple to get list of cpus, and this change is not needed. The actual list will be decided at runtime, instead of statically, which solves the original problem. As well, we'll need it to be able to handle accelerator specific cpus, so it will be changed eventually.
On 4/18/25 18:16, Pierrick Bouvier wrote: > On 4/18/25 10:29, Philippe Mathieu-Daudé wrote: >> Add MachineClass::valid_cpu_types_list, a dynamic list of strings. >> >> CPU types can be registered with machine_class_add_valid_cpu_type(). >> >> Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/boards.h | 8 ++++++++ >> hw/core/machine.c | 30 ++++++++++++++++++++++++++++++ >> 2 files changed, 38 insertions(+) >> >> diff --git a/include/hw/boards.h b/include/hw/boards.h >> index 02f43ac5d4d..647a29ff04d 100644 >> --- a/include/hw/boards.h >> +++ b/include/hw/boards.h >> @@ -56,6 +56,13 @@ void machine_set_cache_topo_level(MachineState *ms, CacheLevelAndType cache, >> bool machine_check_smp_cache(const MachineState *ms, Error **errp); >> void machine_memory_devices_init(MachineState *ms, hwaddr base, uint64_t size); >> >> +/** >> + * machine_class_add_valid_cpu_type: Add type to list of valid CPUs >> + * @mc: Machine class >> + * @type: CPU type to allow (should be a subtype of TYPE_CPU) >> + */ >> +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type); >> + >> /** >> * machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices >> * @mc: Machine class >> @@ -306,6 +313,7 @@ struct MachineClass { >> bool ignore_memory_transaction_failures; >> int numa_mem_align_shift; >> const char * const *valid_cpu_types; >> + GList *valid_cpu_types_list; >> strList *allowed_dynamic_sysbus_devices; >> bool auto_enable_numa_with_memhp; >> bool auto_enable_numa_with_memdev; >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index f52a4f2273b..ff27d533b5c 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -1538,6 +1538,12 @@ const char *machine_class_default_cpu_type(MachineClass *mc) >> return mc->default_cpu_type; >> } >> >> +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type) >> +{ >> + mc->valid_cpu_types_list = g_list_prepend(mc->valid_cpu_types_list, >> + g_strdup(type)); >> +} >> + >> static bool is_cpu_type_supported(const MachineState *machine, Error **errp) >> { >> MachineClass *mc = MACHINE_GET_CLASS(machine); >> @@ -1581,6 +1587,30 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) >> return false; >> } >> } >> + if (mc->valid_cpu_types_list) { >> + bool valid = false; >> + unsigned count = 0; >> + GList *l; >> + >> + for (l = mc->valid_cpu_types_list; !valid && l != NULL; l = l->next) { >> + valid |= !!object_class_dynamic_cast(oc, l->data); >> + count++; >> + } >> + >> + if (!valid) { >> + g_autofree char *requested = cpu_model_from_type(machine->cpu_type); >> + mc->valid_cpu_types_list = g_list_reverse(mc->valid_cpu_types_list); >> + error_setg(errp, "Invalid CPU model: %s", requested); >> + error_append_hint(errp, "The valid models are: "); >> + for (l = mc->valid_cpu_types_list; l != NULL; l = l->next) { >> + g_autofree char *model = cpu_model_from_type(l->data); >> + error_append_hint(errp, "%s%s", model, --count ? ", " : ""); >> + } >> + error_append_hint(errp, "\n"); >> + >> + return false; >> + } >> + } >> >> /* Check if CPU type is deprecated and warn if so */ >> cc = CPU_CLASS(oc); > > How about simply changing valid_cpu_types to become a function instead > of a static list? > > This way, it's simple to get list of cpus, and this change is not > needed. The actual list will be decided at runtime, instead of > statically, which solves the original problem. > As well, we'll need it to be able to handle accelerator specific cpus, > so it will be changed eventually. This would give something like this patch. We can then replace all valid_cpu_types with this, and remove valid_cpu_types field, now it's replaced by get_valid_cpu_types. This way, we still have static lists where possible, and we can conveniently build a dynamic list if needed for concerned boards, which will be evaluated *after* machine is instantiated, so we can poke any accelerator or target setting defined only at runtime. diff --git a/hw/arm/npcm8xx_boards.c b/hw/arm/npcm8xx_boards.c index 3fb8478e72e..75f92dcfe50 100644 --- a/hw/arm/npcm8xx_boards.c +++ b/hw/arm/npcm8xx_boards.c @@ -209,19 +209,22 @@ static void npcm8xx_set_soc_type(NPCM8xxMachineClass *nmc, const char *type) mc->default_cpus = mc->min_cpus = mc->max_cpus = sc->num_cpus; } -static void npcm8xx_machine_class_init(ObjectClass *oc, void *data) -{ - MachineClass *mc = MACHINE_CLASS(oc); - static const char * const valid_cpu_types[] = { +static const char * const *get_valid_cpu_types(void) { + static const char *cpus[] = { ARM_CPU_TYPE_NAME("cortex-a9"), NULL }; + return cpus; +} +static void npcm8xx_machine_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); mc->no_floppy = 1; mc->no_cdrom = 1; mc->no_parallel = 1; mc->default_ram_id = "ram"; - mc->valid_cpu_types = valid_cpu_types; + mc->get_valid_cpu_types = get_valid_cpu_types; }
diff --git a/include/hw/boards.h b/include/hw/boards.h index 02f43ac5d4d..647a29ff04d 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -56,6 +56,13 @@ void machine_set_cache_topo_level(MachineState *ms, CacheLevelAndType cache, bool machine_check_smp_cache(const MachineState *ms, Error **errp); void machine_memory_devices_init(MachineState *ms, hwaddr base, uint64_t size); +/** + * machine_class_add_valid_cpu_type: Add type to list of valid CPUs + * @mc: Machine class + * @type: CPU type to allow (should be a subtype of TYPE_CPU) + */ +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type); + /** * machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices * @mc: Machine class @@ -306,6 +313,7 @@ struct MachineClass { bool ignore_memory_transaction_failures; int numa_mem_align_shift; const char * const *valid_cpu_types; + GList *valid_cpu_types_list; strList *allowed_dynamic_sysbus_devices; bool auto_enable_numa_with_memhp; bool auto_enable_numa_with_memdev; diff --git a/hw/core/machine.c b/hw/core/machine.c index f52a4f2273b..ff27d533b5c 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1538,6 +1538,12 @@ const char *machine_class_default_cpu_type(MachineClass *mc) return mc->default_cpu_type; } +void machine_class_add_valid_cpu_type(MachineClass *mc, const char *type) +{ + mc->valid_cpu_types_list = g_list_prepend(mc->valid_cpu_types_list, + g_strdup(type)); +} + static bool is_cpu_type_supported(const MachineState *machine, Error **errp) { MachineClass *mc = MACHINE_GET_CLASS(machine); @@ -1581,6 +1587,30 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) return false; } } + if (mc->valid_cpu_types_list) { + bool valid = false; + unsigned count = 0; + GList *l; + + for (l = mc->valid_cpu_types_list; !valid && l != NULL; l = l->next) { + valid |= !!object_class_dynamic_cast(oc, l->data); + count++; + } + + if (!valid) { + g_autofree char *requested = cpu_model_from_type(machine->cpu_type); + mc->valid_cpu_types_list = g_list_reverse(mc->valid_cpu_types_list); + error_setg(errp, "Invalid CPU model: %s", requested); + error_append_hint(errp, "The valid models are: "); + for (l = mc->valid_cpu_types_list; l != NULL; l = l->next) { + g_autofree char *model = cpu_model_from_type(l->data); + error_append_hint(errp, "%s%s", model, --count ? ", " : ""); + } + error_append_hint(errp, "\n"); + + return false; + } + } /* Check if CPU type is deprecated and warn if so */ cc = CPU_CLASS(oc);
Add MachineClass::valid_cpu_types_list, a dynamic list of strings. CPU types can be registered with machine_class_add_valid_cpu_type(). Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/boards.h | 8 ++++++++ hw/core/machine.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+)