Re: [RFC 41/52] machine: Introduce core_type() hook

[wangyanan (Y)]

Hi Zhao,

在 2023/2/13 17:50, Zhao Liu 写道:
> From: Zhao Liu <zhao1.liu@intel.com>
>
> Since supported core types are architecture specific, we need this hook
> to allow archs define its own parsing or validation method.
>
> As the example, add the x86 core_type() which will be used in "-hybrid"
> parameter parsing.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   hw/core/machine-topo.c | 14 ++++++++++++++
>   hw/core/machine.c      |  1 +
>   hw/i386/x86.c          | 15 +++++++++++++++
>   include/hw/boards.h    |  7 +++++++
>   4 files changed, 37 insertions(+)
>
> diff --git a/hw/core/machine-topo.c b/hw/core/machine-topo.c
> index 12c05510c1b5..f9ab08a1252e 100644
> --- a/hw/core/machine-topo.c
> +++ b/hw/core/machine-topo.c
> @@ -352,3 +352,17 @@ void machine_parse_smp_config(MachineState *ms,
>           return;
>       }
>   }
> +
> +/*
> + * machine_parse_hybrid_core_type: the default hook to parse hybrid core
> + *                                 type corresponding to the coretype
> + *                                 string option.
> + */
> +int machine_parse_hybrid_core_type(MachineState *ms, const char *coretype)
> +{
> +    if (strcmp(coretype, "") == 0 || strcmp(coretype, "none") == 0) {
> +        return 0;
> +    }
> +
> +    return -1;
> +}
Is it possible that coretype can be NULL?
What would *coretype be if the users don't explicitly specify coretype
in the command line?
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index fad990f49b03..acc32b3be5f6 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -926,6 +926,7 @@ static void machine_class_init(ObjectClass *oc, void *data)
>        * On Linux, each node's border has to be 8MB aligned
>        */
>       mc->numa_mem_align_shift = 23;
> +    mc->core_type = machine_parse_hybrid_core_type;
>   
>       object_class_property_add_str(oc, "kernel",
>           machine_get_kernel, machine_set_kernel);
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index f381fdc43180..f58a90359170 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -1569,6 +1569,20 @@ static void machine_set_sgx_epc(Object *obj, Visitor *v, 
> const char *name,
>       qapi_free_SgxEPCList(list);
>   }
>   
> +static int x86_parse_hybrid_core_type(MachineState *ms, const char *coretype)
> +{
> +    X86HybridCoreType type;
> +
> +    if (strcmp(coretype, "atom") == 0) {
> +        type = INTEL_ATOM_TYPE;
> +    } else if (strcmp(coretype, "core") == 0) {
> +        type = INTEL_CORE_TYPE;
> +    } else {
> +        type = INVALID_HYBRID_TYPE;
> +    }
What about:
INTEL_CORE_TYPE_ATOM
INTEL_CORE_TYPE_CORE
X86_CORE_TYPE_UNKNOWN ?
just a suggestion.

Thanks,
Yanan
> +    return type;
> +}
> +
>   static void x86_machine_initfn(Object *obj)
>   {
>       X86MachineState *x86ms = X86_MACHINE(obj);
> @@ -1596,6 +1610,7 @@ static void x86_machine_class_init(ObjectClass *oc, void 
> *data)
>       x86mc->save_tsc_khz = true;
>       x86mc->fwcfg_dma_enabled = true;
>       nc->nmi_monitor_handler = x86_nmi;
> +    mc->core_type = x86_parse_hybrid_core_type;
>   
>       object_class_property_add(oc, X86_MACHINE_SMM, "OnOffAuto",
>           x86_machine_get_smm, x86_machine_set_smm,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 9364c90d5f1a..34ec035b5c9f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -36,6 +36,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>                                  Error **errp);
>   void machine_parse_smp_config(MachineState *ms,
>                                 const SMPConfiguration *config, Error **errp);
> +int machine_parse_hybrid_core_type(MachineState *ms, const char *coretype);
>   
>   /**
>    * machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices
> @@ -199,6 +200,11 @@ typedef struct {
>    *    Return the type of KVM corresponding to the kvm-type string option or
>    *    computed based on other criteria such as the host kernel capabilities.
>    *    kvm-type may be NULL if it is not needed.
> + * @core_type:
> + *    Return the type of hybrid cores corresponding to the coretype string
> + *    option. The default hook only accept "none" or "" since the most generic
> + *    core topology should not specify any specific core type. Each arch can
> + *    define its own core_type() hook to override the default one.
>    * @numa_mem_supported:
>    *    true if '--numa node.mem' option is supported and false otherwise
>    * @hotplug_allowed:
> @@ -237,6 +243,7 @@ struct MachineClass {
>       void (*reset)(MachineState *state, ShutdownCause reason);
>       void (*wakeup)(MachineState *state);
>       int (*kvm_type)(MachineState *machine, const char *arg);
> +    int (*core_type)(MachineState *state, const char *type);
>   
>       BlockInterfaceType block_default_type;
>       int units_per_default_bus;