On Tue, Feb 14, 2023 at 09:14:13AM +0800, Mi, Dapeng1 wrote:
> Date: Tue, 14 Feb 2023 09:14:13 +0800
> From: "Mi, Dapeng1" <dapeng1...@intel.com>
> Subject: RE: [RFC 09/52] hw/machine: Introduce core type for hybrid topology
>
> > From: Zhao Liu <zhao1....@linux.intel.com>
> > Sent: Monday, February 13, 2023 5:50 PM
> > To: Eduardo Habkost <edua...@habkost.net>; Marcel Apfelbaum
> > <marcel.apfelb...@gmail.com>; Philippe Mathieu-Daud? <phi...@linaro.org>;
> > Yanan Wang <wangyana...@huawei.com>; Michael S . Tsirkin
> > <m...@redhat.com>; Richard Henderson <richard.hender...@linaro.org>; Paolo
> > Bonzini <pbonz...@redhat.com>; Eric Blake <ebl...@redhat.com>; Markus
> > Armbruster <arm...@redhat.com>
> > Cc: qemu-devel@nongnu.org; Wang, Zhenyu Z <zhenyu.z.w...@intel.com>; Mi,
> > Dapeng1 <dapeng1...@intel.com>; Ding, Zhuocheng
> > <zhuocheng.d...@intel.com>; Robert Hoo <robert...@linux.intel.com>;
> > Christopherson,, Sean <sea...@google.com>; Like Xu
> > <like.xu.li...@gmail.com>; Liu, Zhao1 <zhao1....@intel.com>
> > Subject: [RFC 09/52] hw/machine: Introduce core type for hybrid topology
> >
> > From: Zhao Liu <zhao1....@intel.com>
> >
> > Under the hybrid cpu topology, some CPUs care about the core type.
> > For example, Intel's Alder Lake series CPU contains two types of cores:
> > Intel Core and Intel Atom. The type information of these two types is
> > exposed in
> > 1A leaf of CPUID.
> >
> > Core type should also be part of the hybrid topology, and
> > MachineState.cpu_type cannot provide different type information for
> > different
> > cpus in the same machine, so add a type field for the core level in the
> > hybrid cpu
> > topology.
> >
> > Additionally, add a helper to get core type information from MachineState.
> > Though core_type is only used in hybrid case, don't use assert since it may
> > be
> > used to initialize some generic fields.
> >
> > Signed-off-by: Zhao Liu <zhao1....@intel.com>
> > ---
> > hw/core/machine-topo.c | 12 ++++++++++++
> > include/hw/boards.h | 3 +++
> > include/hw/cpu/cpu-topology.h | 2 ++
> > 3 files changed, 17 insertions(+)
> >
> > diff --git a/hw/core/machine-topo.c b/hw/core/machine-topo.c index
> > b20160479629..e0ec07b53d42 100644
> > --- a/hw/core/machine-topo.c
> > +++ b/hw/core/machine-topo.c
> > @@ -51,6 +51,18 @@ unsigned int machine_topo_get_smp_threads(const
> > MachineState *ms)
> > return ms->topo.smp.threads;
> > }
> >
> > +unsigned int machine_topo_get_hybrid_core_type(const MachineState *ms,
> > + unsigned int cluster_id,
> > + unsigned int core_id) {
> > + if (!machine_topo_is_smp(ms)) {
> > + return ms->topo.hybrid.cluster_list[cluster_id]
> > + .core_list[core_id].core_type;
> > + } else {
> > + return 0;
> > + }
> > +}
> > +
>
> We'd better not to return the hard-coded '0'. Suggest to define a 'enum'
> data structure to represent the core_type. This makes the code look more
> intuitively.
Yes. I defined a core type 'enum' in x86 code, Here I can use a macro to
avoid hardcoding.
Zhao
>
> > unsigned int machine_topo_get_threads(const MachineState *ms,
> > unsigned int cluster_id,
> > unsigned int core_id) diff --git
> > a/include/hw/boards.h
> > b/include/hw/boards.h index 34b64b012022..78e52af38cb1 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -484,6 +484,9 @@ unsigned int machine_topo_get_dies(const
> > MachineState *ms); unsigned int machine_topo_get_clusters(const
> > MachineState *ms); unsigned int machine_topo_get_smp_cores(const
> > MachineState *ms); unsigned int machine_topo_get_smp_threads(const
> > MachineState *ms);
> > +unsigned int machine_topo_get_hybrid_core_type(const MachineState *ms,
> > + unsigned int cluster_id,
> > + unsigned int core_id);
> > unsigned int machine_topo_get_threads(const MachineState *ms,
> > unsigned int cluster_id,
> > unsigned int core_id); diff --git
> > a/include/hw/cpu/cpu-
> > topology.h b/include/hw/cpu/cpu-topology.h index
> > 8268ea3a8569..87d832556229 100644
> > --- a/include/hw/cpu/cpu-topology.h
> > +++ b/include/hw/cpu/cpu-topology.h
> > @@ -45,9 +45,11 @@ typedef struct SmpCpuTopology {
> > /**
> > * HybridCore - hybrid core topology defination:
> > * @threads: the number of threads in one core.
> > + * @core_type: the type of current core.
> > */
> > typedef struct HybridCore {
> > unsigned int threads;
> > + unsigned int core_type;
> > } HybridCore;
> >
> > /**
> > --
> > 2.34.1
>
