Re: [Qemu-devel] [PATCH v2 3/5] vl.c: convert cpu_model to cpu type and set of global properties before machine_init()

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 09/13/2017 01:04 PM, Igor Mammedov wrote:
> All machines that support user specified cpu_model either call
> cpu_generic_init() or cpu_class_by_name()/CPUClass::parse_features
> to parse feature string and to get CPU type to create.
> 
> Which leads to code duplication and hard-codding default CPU model
> within machine_foo_init() code. Which makes it impossible to
> get CPU type before machine_init() is run.
> 
> So instead of setting default CPUs models and doing parsing in
> target specific machine_foo_init() in various ways, provide
> a generic data driven cpu_model parsing before machine_init()
> is called.
> 
> in follow up per target patches, it will allow to:
>    * define default CPU type in consistent/generic manner
>      per machine type and drop custom code that fallbacks
>      to default if cpu_model is NULL
>    * drop custom features parsing in targets and do it
>      in centralized way.
>    * for cases of
>        cpu_generic_init(TYPE_BASE/DEFAULT_CPU, "some_cpu")
>      replace it with
>        cpu_create(machine->cpu_type) || cpu_create(TYPE_FOO)
>      depending if CPU type is user settable or not.
>      not doing useless parsing and clearly documenting where
>      CPU model is user settable or fixed one.
> 
> Patch allows machine subclasses to define default CPU type
> per machine class at class_init() time and if that is set
> generic code will parse cpu_model into a MachineState::cpu_type
> which will be used to create CPUs for that machine instance
> and allows gradual per board conversion.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
> Target specific changes will split into separate per target/machine
> patches to make changes reviewable.
> ---
>   include/hw/boards.h |  6 ++++++
>   vl.c                | 10 ++++++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 7f044d1..6b67ada 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -125,6 +125,10 @@ typedef struct {
>    *    Caller is responsible for freeing returned list.
>    * @has_hotpluggable_cpus:
>    *    If true, board supports CPUs creation with -device/device_add.
> + * @default_cpu_type:
> + *    specifies default CPU_TYPE, which will be used for parsing target
> + *    specific features and for creating CPUs if CPU name wasn't provided
> + *    explicitly at CLI
>    * @minimum_page_bits:
>    *    If non-zero, the board promises never to create a CPU with a page size
>    *    smaller than this, so QEMU can use a more efficient larger page
> @@ -177,6 +181,7 @@ struct MachineClass {
>       GArray *compat_props;
>       const char *hw_version;
>       ram_addr_t default_ram_size;
> +    const char *default_cpu_type;
>       bool option_rom_has_mr;
>       bool rom_file_has_mr;
>       int minimum_page_bits;
> @@ -231,6 +236,7 @@ struct MachineState {
>       char *kernel_cmdline;
>       char *initrd_filename;
>       const char *cpu_model;
> +    const char *cpu_type;
>       AccelState *accelerator;
>       CPUArchIdList *possible_cpus;
>   };
> diff --git a/vl.c b/vl.c
> index fb1f05b..034180f 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4636,6 +4636,16 @@ int main(int argc, char **argv, char **envp)
>       current_machine->boot_order = boot_order;
>       current_machine->cpu_model = cpu_model;
>   
> +
> +    /* parse features once if machine provides default cpu_type */
> +    if (machine_class->default_cpu_type) {
> +        current_machine->cpu_type = machine_class->default_cpu_type;
> +        if (cpu_model) {
> +            current_machine->cpu_type =
> +                cpu_parse_cpu_model(machine_class->default_cpu_type, cpu_model);
> +        }
> +    }
> +
>       machine_run_board_init(current_machine);
>   
>       realtime_init();
>