On Thu, 13 Jul 2023 15:45:00 +1000
Gavin Shan <gs...@redhat.com> wrote:
> The CPU type invalidation logic in machine_run_board_init() is
> independent enough. Lets factor it out into helper validate_cpu_type().
> Since we're here, the relevant comments are improved a bit.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gs...@redhat.com>
> ---
> hw/core/machine.c | 81 +++++++++++++++++++++++++----------------------
> 1 file changed, 43 insertions(+), 38 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f0d35c6401..68b866c762 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -1349,12 +1349,52 @@ out:
> return r;
> }
>
> +static void validate_cpu_type(MachineState *machine)
s/validate_cpu_type/is_cpu_type_valid or better is_cpu_type_supported
Is it going to be reused elsewhere (otherwise I don't see much reason to move
code around)?
> +{
> + MachineClass *machine_class = MACHINE_GET_CLASS(machine);
> + ObjectClass *oc = object_class_by_name(machine->cpu_type);
> + CPUClass *cc = CPU_CLASS(oc);
> + int i;
> +
> + /*
> + * Check if the user-specified CPU type is supported when the valid
> + * CPU types have been determined. Note that the user-specified CPU
> + * type is given by '-cpu' option.
> + */
> + if (!machine->cpu_type || !machine_class->valid_cpu_types) {
> + goto out_no_check;
no goto-s please
> + }
> +
> + for (i = 0; machine_class->valid_cpu_types[i]; i++) {
> + if (object_class_dynamic_cast(oc,
> machine_class->valid_cpu_types[i])) {
> + break;
> + }
> + }
> +
> + if (!machine_class->valid_cpu_types[i]) {
> + /* The user-specified CPU type is invalid */
> + error_report("Invalid CPU type: %s", machine->cpu_type);
> + error_printf("The valid types are: %s",
> + machine_class->valid_cpu_types[0]);
> + for (i = 1; machine_class->valid_cpu_types[i]; i++) {
> + error_printf(", %s", machine_class->valid_cpu_types[i]);
> + }
> + error_printf("\n");
> +
> + exit(1);
since you are touching that,
turn it in errp handling, in separate patch 1st
and only then introduce your helper.
> + }
> +
> + /* Check if CPU type is deprecated and warn if so */
> +out_no_check:
> + if (cc && cc->deprecation_note) {
> + warn_report("CPU model %s is deprecated -- %s",
> + machine->cpu_type, cc->deprecation_note);
> + }
> +}
>
> void machine_run_board_init(MachineState *machine, const char *mem_path,
> Error **errp)
> {
> MachineClass *machine_class = MACHINE_GET_CLASS(machine);
> - ObjectClass *oc = object_class_by_name(machine->cpu_type);
> - CPUClass *cc;
>
> /* This checkpoint is required by replay to separate prior clock
> reading from the other reads, because timer polling functions query
> @@ -1405,42 +1445,7 @@ void machine_run_board_init(MachineState *machine,
> const char *mem_path, Error *
> machine->ram = machine_consume_memdev(machine, machine->memdev);
> }
>
> - /* If the machine supports the valid_cpu_types check and the user
> - * specified a CPU with -cpu check here that the user CPU is supported.
> - */
> - if (machine_class->valid_cpu_types && machine->cpu_type) {
> - int i;
> -
> - for (i = 0; machine_class->valid_cpu_types[i]; i++) {
> - if (object_class_dynamic_cast(oc,
> -
> machine_class->valid_cpu_types[i])) {
> - /* The user specificed CPU is in the valid field, we are
> - * good to go.
> - */
> - break;
> - }
> - }
> -
> - if (!machine_class->valid_cpu_types[i]) {
> - /* The user specified CPU is not valid */
> - error_report("Invalid CPU type: %s", machine->cpu_type);
> - error_printf("The valid types are: %s",
> - machine_class->valid_cpu_types[0]);
> - for (i = 1; machine_class->valid_cpu_types[i]; i++) {
> - error_printf(", %s", machine_class->valid_cpu_types[i]);
> - }
> - error_printf("\n");
> -
> - exit(1);
> - }
> - }
> -
> - /* Check if CPU type is deprecated and warn if so */
> - cc = CPU_CLASS(oc);
> - if (cc && cc->deprecation_note) {
> - warn_report("CPU model %s is deprecated -- %s", machine->cpu_type,
> - cc->deprecation_note);
> - }
> + validate_cpu_type(machine);
>
> if (machine->cgs) {
> /*
