On Thu, Feb 16, 2023 at 08:28:37PM +0800, wangyanan (Y) wrote:
> Date: Thu, 16 Feb 2023 20:28:37 +0800
> From: "wangyanan (Y)" <wangyana...@huawei.com>
> Subject: Re: [RFC 42/52] hw/machine: Add hybrid_supported in generic topo
> properties
>
> 在 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.😉
Yeah, it makes sense to manage all topologically related.
So I will, thanks!
>
> 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,
>
