| Message ID | 20250422145502.70770-12-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | single-binary: Make hw/arm/ common | expand |
On 4/22/25 07:54, Philippe Mathieu-Daudé wrote: > index f52a4f2273b..8b40735ef98 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -1581,6 +1581,33 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) > return false; > } > } > + if (mc->get_valid_cpu_types) { > + GSList *vct = mc->get_valid_cpu_types(machine); > + bool valid = false; > + unsigned count = 0; > + GSList *l; > + > + for (l = vct; !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); > + vct = g_slist_reverse(vct); > + error_setg(errp, "Invalid CPU model: %s", requested); > + error_append_hint(errp, "The valid models are: "); > + for (l = vct; 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"); > + } > + g_slist_free_full(vct, g_free); > + if (!valid) { > + return false; > + } > + } Why use GSList instead of GPtrArray? That would provide you the count without manually computing it, and it would avoid the need for any sort of reverse. I think it would also allow you to auto-free the set. r~
On 4/22/25 10:54, Richard Henderson wrote: > On 4/22/25 07:54, Philippe Mathieu-Daudé wrote: >> index f52a4f2273b..8b40735ef98 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -1581,6 +1581,33 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) >> return false; >> } >> } >> + if (mc->get_valid_cpu_types) { >> + GSList *vct = mc->get_valid_cpu_types(machine); >> + bool valid = false; >> + unsigned count = 0; >> + GSList *l; >> + >> + for (l = vct; !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); >> + vct = g_slist_reverse(vct); >> + error_setg(errp, "Invalid CPU model: %s", requested); >> + error_append_hint(errp, "The valid models are: "); >> + for (l = vct; 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"); >> + } >> + g_slist_free_full(vct, g_free); >> + if (!valid) { >> + return false; >> + } >> + } > > Why use GSList instead of GPtrArray? > That would provide you the count without manually computing it, > and it would avoid the need for any sort of reverse. > I think it would also allow you to auto-free the set. > Same remark than Richard, it would remove all the checks needed, as we can simply use this array "blindly". > > r~
diff --git a/include/hw/boards.h b/include/hw/boards.h index 02f43ac5d4d..be0c0f04804 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -259,6 +259,9 @@ typedef struct { * @smbios_memory_device_size: * Default size of memory device, * SMBIOS 3.1.0 "7.18 Memory Device (Type 17)" + * @get_valid_cpu_types: + * Returns a list of valid CPU types for this board. May be NULL + * if not needed. */ struct MachineClass { /*< private >*/ @@ -306,6 +309,7 @@ struct MachineClass { bool ignore_memory_transaction_failures; int numa_mem_align_shift; const char * const *valid_cpu_types; + GSList *(*get_valid_cpu_types)(const MachineState *ms); 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..8b40735ef98 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -1581,6 +1581,33 @@ static bool is_cpu_type_supported(const MachineState *machine, Error **errp) return false; } } + if (mc->get_valid_cpu_types) { + GSList *vct = mc->get_valid_cpu_types(machine); + bool valid = false; + unsigned count = 0; + GSList *l; + + for (l = vct; !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); + vct = g_slist_reverse(vct); + error_setg(errp, "Invalid CPU model: %s", requested); + error_append_hint(errp, "The valid models are: "); + for (l = vct; 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"); + } + g_slist_free_full(vct, g_free); + if (!valid) { + return false; + } + } /* Check if CPU type is deprecated and warn if so */ cc = CPU_CLASS(oc);
Add MachineClass::get_valid_cpu_types(), a helper that returns a dynamic list of CPU types. Since the helper takes a MachineState argument, we know the machine is created by the time we call it. Suggested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/boards.h | 4 ++++ hw/core/machine.c | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+)