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

[wangyanan (Y)]

在 2023/2/15 10:53, Zhao Liu 写道:

On Tue, Feb 14, 2023 at 09:46:50AM +0800, wangyanan (Y) wrote:

Date: Tue, 14 Feb 2023 09:46:50 +0800
From: "wangyanan (Y)" <wangyana...@huawei.com>
Subject: Re: [RFC 42/52] hw/machine: Add hybrid_supported in generic topo
  properties

Hi Zhao,

在 2023/2/13 17:50, Zhao Liu 写道:

From: Zhao Liu <zhao1....@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....@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.

Thanks! Will fix.

+ *                   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.

Do you think TopoCompatProps/SMPCompatProps should also be moved
into cpu-topology.h? It seems that SMPCompatProps is a collection
of properties of MachineClass.

TopoCompatProps holds properties all about CPU topology, I think we
can do this, cpu-topology.h will be included in boards.h any way. But it's
ups to you whether to do this.😉

Thanks,
Yanan

And put the "hybrid_supported"
extension into another patch. Would this make it easier to review?

Yes, I agree. Thanks!

Zhao

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,