On Tue, Feb 14, 2023 at 07:23:37PM +0800, wangyanan (Y) wrote: > Date: Tue, 14 Feb 2023 19:23:37 +0800 > From: "wangyanan (Y)" <wangyana...@huawei.com> > Subject: Re: [RFC 06/52] hw/cpu: Introduce hybrid CPU topology > > 在 2023/2/14 18:16, Zhao Liu 写道: > > On Mon, Feb 13, 2023 at 09:18:05PM +0800, wangyanan (Y) wrote: > > > Date: Mon, 13 Feb 2023 21:18:05 +0800 > > > From: "wangyanan (Y)" <wangyana...@huawei.com> > > > Subject: Re: [RFC 06/52] hw/cpu: Introduce hybrid CPU topology > > > > > > Hi Zhao, > > > > > > 在 2023/2/13 17:49, Zhao Liu 写道: > > > > From: Zhao Liu <zhao1....@intel.com> > > > > > > > > For smp systems, the parts in one topology level are the same. But now > > > > there are more and more systems with hybrid architectures. Different > > > > parts of the same topology level may have differences. For example, > > > > Intel's Alder Lake series CPU has two types of cores, so the CPU > > > > topology is no longer symmetrical. > > > > > > > > The hybrid topology is compatible with the smp topology type, that is, > > > > different parts on the same level of the hybrid topology can set to be > > > > the same, but the hybrid topology will introduce more complexity (need > > > > to allocate more memory, organized with array or linked-list), so the > > > > original smp topology support is retained while introducing the hybrid > > > > topology, and the hybrid topology is only built when the hybrid is > > > > explicitly required. > > > > > > > > Therefore, we introduce the definition support of hybrid cpu topology > > > > here. At the same time, in order to unify with the original smp, we > > > > introduce a new cpu topology structure that can support smp topology > > > > or hybrid topology. This structure will replace the CpuTopology type (in > > > > include/hw/boards.h) used by MachineState.smp. > > > > > > > > As for now, we only support two hybrid topology levels: core and > > > > cluster. > > > > > > > > Signed-off-by: Zhao Liu <zhao1....@intel.com> > > > > --- > > > > MAINTAINERS | 1 + > > > > include/hw/cpu/cpu-topology.h | 117 > > > > ++++++++++++++++++++++++++++++++++ > > > > qapi/machine.json | 12 ++++ > > > > 3 files changed, 130 insertions(+) > > > > create mode 100644 include/hw/cpu/cpu-topology.h > > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > > index 58794885ced3..918a9418d98e 100644 > > > > --- a/MAINTAINERS > > > > +++ b/MAINTAINERS > > > > @@ -1742,6 +1742,7 @@ F: qapi/machine-target.json > > > > F: include/hw/boards.h > > > > F: include/hw/core/cpu.h > > > > F: include/hw/cpu/cluster.h > > > > +F: include/hw/cpu/cpu-topology.h > > > Should't it be in include/hw/core/* directory? > > Yes, I'll move it to the correct place. > > > > > > F: include/sysemu/numa.h > > > > F: tests/unit/test-smp-parse.c > > > > T: git https://gitlab.com/ehabkost/qemu.git machine-next > > > > diff --git a/include/hw/cpu/cpu-topology.h > > > > b/include/hw/cpu/cpu-topology.h > > > > new file mode 100644 > > > > index 000000000000..8268ea3a8569 > > > > --- /dev/null > > > > +++ b/include/hw/cpu/cpu-topology.h > > > > @@ -0,0 +1,117 @@ > > > > +/* > > > > + * CPU topology defination for Machine core > > > > + * > > > > + * Copyright (c) 2023 Intel Corporation > > > > + * Author: Zhao Liu <zhao1....@intel.com> > > > > + * > > > > + * This program is free software; you can redistribute it and/or modify > > > > + * it under the terms of the GNU General Public License as published by > > > > + * the Free Software Foundation; either version 2 of the License, > > > > + * or (at your option) any later version. > > > > + * > > > > + * This program is distributed in the hope that it will be useful, > > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > > + * GNU General Public License for more details. > > > > + * > > > > + * You should have received a copy of the GNU General Public License > > > > + * along with this program; if not, see <http://www.gnu.org/licenses/>. > > > > + */ > > > > + > > > > +#ifndef CPU_TOPOLOGY_H > > > > +#define CPU_TOPOLOGY_H > > > > + > > > > +#include "qemu/queue.h" > > > > + > > > > +/** > > > > + * SmpCpuTopology - smp cpu topology defination. > > > > + * > > > > + * For smp system, the parts in one topology level are the same. > > > > + * > > > > + * @sockets: the number of sockets on the machine > > > > + * @dies: the number of dies in one socket > > > > + * @clusters: the number of clusters in one die > > > > + * @cores: the number of cores in one cluster > > > > + * @threads: the number of threads in one core > > > > + */ > > > > +typedef struct SmpCpuTopology { > > > > + unsigned int sockets; > > > > + unsigned int dies; > > > > + unsigned int clusters; > > > > + unsigned int cores; > > > > + unsigned int threads; > > > > +} SmpCpuTopology; > > > > + > > > > +/** > > > > + * HybridCore - hybrid core topology defination: > > > > + * @threads: the number of threads in one core. > > > > + */ > > > > +typedef struct HybridCore { > > > > + unsigned int threads; > > > > +} HybridCore; > > > > + > > > > +/** > > > > + * HybridCluster - hybrid cluster topology defination: > > > > + * @cores: the number of cores in current cluster. > > > > + * @core_list: the array includes all the cores that belong to current > > > > + * cluster. > > > > + */ > > > > +typedef struct HybridCluster { > > > > + unsigned int cores; > > > > + HybridCore *core_list; > > > > +} HybridCluster; > > > > + > > > > +/** > > > > + * HybridCpuTopology - hybrid cpu topology defination. > > > > + * > > > > + * At present we only support two heterogeneous topology levels: core > > > > + * and cluster. For heterogeneous levels, we need additional structs > > > > + * to define their custom internal topology. So here we defines > > > > + * symmetric topology levels, and use a list to point to heterogeneous > > > > + * levels. > > > > + * > > > > + * @sockets: the number of sockets on the machine. All sockets are the > > > > + * same. > > > > + * @dies: the number of dies in one socket. All dies are the same. > > > > + * @clusters: the number of clusters in one die. Cluster may be > > > > + * different. This field indicates the length of > > > > + * cluster_list. > > > > + * @cluster_list: the array includes all the clusters in one die. > > > > + */ > > > > +typedef struct HybridCpuTopology { > > > > + unsigned int sockets; > > > > + unsigned int dies; > > > > + unsigned int clusters; > > > > + HybridCluster *cluster_list; > > > > +} HybridCpuTopology; > > > > + > > > > +/** > > > > + * GeneralCpuTopology - General cpu topology defination. > > > > + * > > > > + * It supports one of two topologies: smp topology or hybrid topology. > > > > + * > > > > + * @cpus: the number of present logical processors on the machine > > > > + * @max_cpus: the maximum number of logical processors on the machine > > > > + * @topo_type: the topology type of the machine and this decides which > > > > + * member of the union to visit: smp or hybrid. > > > > + * @smp: the smp cpu topology informantion. Only valid when topo_type > > > > is > > > > + * CPU_TOPO_TYPE_SMP. > > > > + * @hybrid: the hybrid cpu topology informantion. Only valid when > > > > + * topo_type is CPU_TOPO_TYPE_HYBRID. > > > > + */ > > > > +typedef struct GeneralCpuTopology { > > > > + unsigned int cpus; > > > > + unsigned int max_cpus; > > > > + CpuTopoType topo_type; > > > > + union { > > > > + SmpCpuTopology smp; > > > > + HybridCpuTopology hybrid; > > > > + }; > > > > +} GeneralCpuTopology; /* > > > > + * TODO: This name is temporary, just to > > > > distinguish it > > > > + * from the CpuTopology in boards.h. When > > > > CpuTopology in > > > > + * boards.h is merged here, it will be uniformly > > > > named as > > > > + * CpuTopology. > > > > + */ > > > > + > > > A suggestion: > > > 1、Move definition of CpuTopology from boards.h to cpu-topology.h > > > and re-structure it to include SmpCpuTopology, being a generic cpu > > > topology structure. > > > 2、Rename "CpuTopology smp" in MachineState to a generic name > > > "CpuTopology topo". > > Here we need to change the access to MachineState.smp to > > MachineState.topo.smp in other modules. > > > > If replacement of MachineState.topo is in a single patch, do we also > > need to include the modification of access to MachineState.topo.smp > > in other modules? Otherwise, it will break the compilation. > > > > In this way, the patch seems be too large. > Oh, I understand. The temporary "GeneralCpuTopology" seems necessary. > With this patch and help of the introduced generic helpers, we can replace > access to MachineState.smp gradually. And that's what you are doing in > this series. > > But is it possible to strip the whole hybrid related code from the re-work > of smp. After MachineState.smp is re-worked/generalized completely, > then we add the hybrid extansion? Now the re-work code of smp and > hybrid code looks coupled all the way.
I agree. This is more clear. Thanks, Zhao > > Thanks, > Yanan > > > > Thanks, > > Zhao > > > > > 3、Adapt all the code in QEMU to above change. > > > > > > If you can pack above into a single patch, and then add the hybird > > > topology extansion in a next patch, we will not need the temporary > > > thing "GeneralCpuTopology" and the TODO comments, which makes > > > code clearer. > > > > > > Thanks, > > > Yanan > > > > +#endif /* CPU_TOPOLOGY_H */ > > > > diff --git a/qapi/machine.json b/qapi/machine.json > > > > index b9228a5e4616..bd7303f34497 100644 > > > > --- a/qapi/machine.json > > > > +++ b/qapi/machine.json > > > > @@ -36,6 +36,18 @@ > > > > 'sh4eb', 'sparc', 'sparc64', 'tricore', > > > > 'x86_64', 'xtensa', 'xtensaeb' ] } > > > > +## > > > > +# @CpuTopoType: > > > > +# > > > > +# An enumeration of cpu topology type > > > > +# TODO: Expose topology type in query-cpus-fast > > > > +# > > > > +# Since: 8.0 > > > > +## > > > > +{ 'enum': 'CpuTopoType', > > > > + 'prefix': 'CPU_TOPO_TYPE', > > > > + 'data': [ 'smp', 'hybrid' ] } > > > > + > > > > ## > > > > # @CpuS390State: > > > > # >