Re: [RFC 42/52] hw/machine: Add hybrid_supported in generic topo properties

["wangyanan (Y)" via <qemu-devel@nongnu.org>]

Hi Zhao,

在 2023/2/13 17:50, Zhao Liu 写道:
> From: Zhao Liu <zhao1.liu@intel.com>
>
> Since hybrid cpu topology configuration can benefit not only x86, but
> also other architectures/platforms that have supported (in real
> machines) or will support hybrid CPU topology, "-hybrid" can be generic.
>
> So add the generic topology property to configure if support hybrid
> cpu topology for architectures/platforms in SmpCompatProps.
>
> Also rename SmpCompatProps to TopoCompatProps to make this structure
> more generic for both smp topology and hybrid topology.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   include/hw/boards.h | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 34ec035b5c9f..17be3485e823 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -127,19 +127,26 @@ typedef struct {
>   } CPUArchIdList;
>   
>   /**
> - * SMPCompatProps:
> - * @prefer_sockets - whether sockets are preferred over cores in smp parsing
> + * TopoCompatProps:
> + * @hybrid_support - whether hybrid cpu topology are supported by machine.
inconsistent with the name in the definition below.
> + *                   Note that hybrid cpu topology requires to specify the
> + *                   topology of each core so that there will no longer be
> + *                   a default core topology, thus prefer_sockets won't work
> + *                   when hybrid_support is enabled.
> + * @prefer_sockets - whether sockets are preferred over cores in smp parsing.
> + *                   Not work when hybrid_support is enabled.
>    * @dies_supported - whether dies are supported by the machine
>    * @clusters_supported - whether clusters are supported by the machine
>    * @has_clusters - whether clusters are explicitly specified in the user
>    *                 provided SMP configuration
>    */
>   typedef struct {
> +    bool hybrid_supported;
>       bool prefer_sockets;
>       bool dies_supported;
>       bool clusters_supported;
>       bool has_clusters;
> -} SMPCompatProps;
> +} TopoCompatProps;
>   
Also here. "Rename SMPCompatProps to TopoCompatProps and
move it to cpu-topology.h and adapt the code" should be organized
in one or more separate patches, being pre-patches together with
the conversion of CpuTopology before. And put the "hybrid_supported"
extension into another patch. Would this make it easier to review?

Thanks,
Yanan
>   /**
>    * MachineClass:
> @@ -281,7 +288,7 @@ struct MachineClass {
>       bool nvdimm_supported;
>       bool numa_mem_supported;
>       bool auto_enable_numa;
> -    SMPCompatProps smp_props;
> +    TopoCompatProps smp_props;
>       const char *default_ram_id;
>   
>       HotplugHandler *(*get_hotplug_handler)(MachineState *machine,