Re: [RFC 06/52] hw/cpu: Introduce hybrid CPU topology

[wangyanan (Y)]

Hi Zhao,

在 2023/2/13 17:49, Zhao Liu 写道:
> From: Zhao Liu <zhao1.liu@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.liu@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？
>   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.liu@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".
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:
>   #