On Mon, Jul 19, 2021 at 11:20:42AM +0800, Yanan Wang wrote:
> We are going to introduce an unit test for the parser smp_parse()
> in hw/core/machine.c, but now machine.c is only built in softmmu.
>
> In order to solve the build dependency on the smp parsing code and
> avoid building unrelated stuff for the unit tests, move the related
> code from machine.c into a new common file, i.e., machine-smp.c.
>
> Signed-off-by: Yanan Wang <wangyana...@huawei.com>
> ---
> MAINTAINERS | 1 +
> hw/core/machine-smp.c | 124 ++++++++++++++++++++++++++++++++++++++++++
> hw/core/machine.c | 109 -------------------------------------
> hw/core/meson.build | 1 +
> include/hw/boards.h | 1 +
> 5 files changed, 127 insertions(+), 109 deletions(-)
> create mode 100644 hw/core/machine-smp.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9100f9a043..70633e3bf4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1626,6 +1626,7 @@ F: cpu.c
> F: hw/core/cpu.c
> F: hw/core/machine-qmp-cmds.c
> F: hw/core/machine.c
> +F: hw/core/machine-smp.c
> F: hw/core/null-machine.c
> F: hw/core/numa.c
> F: hw/cpu/cluster.c
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> new file mode 100644
> index 0000000000..6a00cfe44a
> --- /dev/null
> +++ b/hw/core/machine-smp.c
> @@ -0,0 +1,124 @@
> +/*
> + * QEMU Machine (related to SMP configuration)
> + *
> + * Copyright (C) 2014 Red Hat Inc
> + *
> + * Authors:
> + * Marcel Apfelbaum <marce...@redhat.com>
This header was obviously copy+pasted without being updated.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/boards.h"
> +#include "qapi/error.h"
> +
> +/*
> + * smp_parse - Generic function used to parse the given SMP configuration
> + *
> + * The topology parameters must be specified equal to or great than one
> + * or just omitted, explicit configuration like "cpus=0" is not allowed.
> + * The omitted parameters will be calculated based on the provided ones.
> + *
> + * maxcpus will default to the value of cpus if omitted and will be used
> + * to compute the missing sockets/cores/threads. cpus will be calculated
> + * from the computed parametrs if omitted.
> + *
> + * In calculation of omitted arch-netural sockets/cores/threads, we prefer
> + * sockets over cores over threads before 6.2, while prefer cores over
> + * sockets over threads since 6.2 on. The arch-specific dies will directly
> + * default to 1 if omitted.
> + */
> +void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> + unsigned cpus = config->has_cpus ? config->cpus : 0;
> + unsigned sockets = config->has_sockets ? config->sockets : 0;
> + unsigned dies = config->has_dies ? config->dies : 1;
> + unsigned cores = config->has_cores ? config->cores : 0;
> + unsigned threads = config->has_threads ? config->threads : 0;
> + unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
> +
> + if ((config->has_cpus && config->cpus == 0) ||
> + (config->has_sockets && config->sockets == 0) ||
> + (config->has_dies && config->dies == 0) ||
> + (config->has_cores && config->cores == 0) ||
> + (config->has_threads && config->threads == 0) ||
> + (config->has_maxcpus && config->maxcpus == 0)) {
> + error_setg(errp, "parameters must be equal to or greater than one"
> + "if provided");
> + return;
> + }
> +
> + if (!mc->smp_dies_supported && dies > 1) {
> + error_setg(errp, "dies not supported by this machine's CPU
> topology");
> + return;
> + }
> +
> + maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +
> + /* prefer sockets over cores over threads before 6.2 */
> + if (mc->smp_prefer_sockets) {
> + if (sockets == 0) {
> + cores = cores > 0 ? cores : 1;
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (dies * cores * threads);
> + sockets = sockets > 0 ? sockets : 1;
> + } else if (cores == 0) {
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * dies * threads);
> + cores = cores > 0 ? cores : 1;
> + } else if (threads == 0) {
> + threads = maxcpus / (sockets * dies * cores);
> + threads = threads > 0 ? threads : 1;
> + }
> + /* prefer cores over sockets over threads since 6.2 */
> + } else {
> + if (cores == 0) {
> + sockets = sockets > 0 ? sockets : 1;
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * dies * threads);
> + cores = cores > 0 ? cores : 1;
> + } else if (sockets == 0) {
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (dies * cores * threads);
> + sockets = sockets > 0 ? sockets : 1;
> + } else if (threads == 0) {
> + threads = maxcpus / (sockets * dies * cores);
> + threads = threads > 0 ? threads : 1;
> + }
> + }
> +
> + /* use the computed parameters to calculate the omitted cpus */
> + cpus = cpus > 0 ? cpus : sockets * dies * cores * threads;
> + maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +
> + if (sockets * dies * cores * threads != maxcpus) {
> + g_autofree char *dies_msg = g_strdup_printf(
> + mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> + error_setg(errp, "Invalid CPU topology: "
> + "sockets (%u)%s * cores (%u) * threads (%u) "
> + "!= maxcpus (%u)",
> + sockets, dies_msg, cores, threads,
> + maxcpus);
> + return;
> + }
> +
> + if (sockets * dies * cores * threads < cpus) {
> + g_autofree char *dies_msg = g_strdup_printf(
> + mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> + error_setg(errp, "Invalid CPU topology: "
> + "sockets (%u)%s * cores (%u) * threads (%u) < "
> + "smp_cpus (%u)",
> + sockets, dies_msg, cores, threads, cpus);
> + return;
> + }
> +
> + ms->smp.cpus = cpus;
> + ms->smp.sockets = sockets;
> + ms->smp.dies = dies;
> + ms->smp.cores = cores;
> + ms->smp.threads = threads;
> + ms->smp.max_cpus = maxcpus;
> +}
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 9d24b67ef3..61be266b6c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -744,115 +744,6 @@ void machine_set_cpu_numa_node(MachineState *machine,
> }
> }
>
> -/*
> - * smp_parse - Generic function used to parse the given SMP configuration
> - *
> - * The topology parameters must be specified equal to or great than one
> - * or just omitted, explicit configuration like "cpus=0" is not allowed.
> - * The omitted parameters will be calculated based on the provided ones.
> - *
> - * maxcpus will default to the value of cpus if omitted and will be used
> - * to compute the missing sockets/cores/threads. cpus will be calculated
> - * from the computed parametrs if omitted.
> - *
> - * In calculation of omitted arch-netural sockets/cores/threads, we prefer
> - * sockets over cores over threads before 6.2, while prefer cores over
> - * sockets over threads since 6.2 on. The arch-specific dies will directly
> - * default to 1 if omitted.
> - */
> -static void smp_parse(MachineState *ms, SMPConfiguration *config, Error
> **errp)
> -{
> - MachineClass *mc = MACHINE_GET_CLASS(ms);
> - unsigned cpus = config->has_cpus ? config->cpus : 0;
> - unsigned sockets = config->has_sockets ? config->sockets : 0;
> - unsigned dies = config->has_dies ? config->dies : 1;
> - unsigned cores = config->has_cores ? config->cores : 0;
> - unsigned threads = config->has_threads ? config->threads : 0;
> - unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
> -
> - if ((config->has_cpus && config->cpus == 0) ||
> - (config->has_sockets && config->sockets == 0) ||
> - (config->has_dies && config->dies == 0) ||
> - (config->has_cores && config->cores == 0) ||
> - (config->has_threads && config->threads == 0) ||
> - (config->has_maxcpus && config->maxcpus == 0)) {
> - error_setg(errp, "parameters must be equal to or greater than one"
> - "if provided");
> - return;
> - }
> -
> - if (!mc->smp_dies_supported && dies > 1) {
> - error_setg(errp, "dies not supported by this machine's CPU
> topology");
> - return;
> - }
> -
> - maxcpus = maxcpus > 0 ? maxcpus : cpus;
> -
> - /* prefer sockets over cores over threads before 6.2 */
> - if (mc->smp_prefer_sockets) {
> - if (sockets == 0) {
> - cores = cores > 0 ? cores : 1;
> - threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (dies * cores * threads);
> - sockets = sockets > 0 ? sockets : 1;
> - } else if (cores == 0) {
> - threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (sockets * dies * threads);
> - cores = cores > 0 ? cores : 1;
> - } else if (threads == 0) {
> - threads = maxcpus / (sockets * dies * cores);
> - threads = threads > 0 ? threads : 1;
> - }
> - /* prefer cores over sockets over threads since 6.2 */
> - } else {
> - if (cores == 0) {
> - sockets = sockets > 0 ? sockets : 1;
> - threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (sockets * dies * threads);
> - cores = cores > 0 ? cores : 1;
> - } else if (sockets == 0) {
> - threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (dies * cores * threads);
> - sockets = sockets > 0 ? sockets : 1;
> - } else if (threads == 0) {
> - threads = maxcpus / (sockets * dies * cores);
> - threads = threads > 0 ? threads : 1;
> - }
> - }
> -
> - /* use the computed parameters to calculate the omitted cpus */
> - cpus = cpus > 0 ? cpus : sockets * dies * cores * threads;
> - maxcpus = maxcpus > 0 ? maxcpus : cpus;
> -
> - if (sockets * dies * cores * threads != maxcpus) {
> - g_autofree char *dies_msg = g_strdup_printf(
> - mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> - error_setg(errp, "Invalid CPU topology: "
> - "sockets (%u)%s * cores (%u) * threads (%u) "
> - "!= maxcpus (%u)",
> - sockets, dies_msg, cores, threads,
> - maxcpus);
> - return;
> - }
> -
> - if (sockets * dies * cores * threads < cpus) {
> - g_autofree char *dies_msg = g_strdup_printf(
> - mc->smp_dies_supported ? " * dies (%u)" : "", dies);
> - error_setg(errp, "Invalid CPU topology: "
> - "sockets (%u)%s * cores (%u) * threads (%u) < "
> - "smp_cpus (%u)",
> - sockets, dies_msg, cores, threads, cpus);
> - return;
> - }
> -
> - ms->smp.cpus = cpus;
> - ms->smp.sockets = sockets;
> - ms->smp.dies = dies;
> - ms->smp.cores = cores;
> - ms->smp.threads = threads;
> - ms->smp.max_cpus = maxcpus;
> -}
> -
> static void machine_get_smp(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> diff --git a/hw/core/meson.build b/hw/core/meson.build
> index 18f44fb7c2..6d727c7742 100644
> --- a/hw/core/meson.build
> +++ b/hw/core/meson.build
> @@ -14,6 +14,7 @@ hwcore_files = files(
> )
>
> common_ss.add(files('cpu-common.c'))
> +common_ss.add(files('machine-smp.c'))
> common_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
> common_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true:
> files('generic-loader.c'))
> common_ss.add(when: ['CONFIG_GUEST_LOADER', fdt], if_true:
> files('guest-loader.c'))
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 12ab0f5968..071eec1e74 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -34,6 +34,7 @@ HotpluggableCPUList
> *machine_query_hotpluggable_cpus(MachineState *machine);
> void machine_set_cpu_numa_node(MachineState *machine,
> const CpuInstanceProperties *props,
> Error **errp);
> +void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp);
>
> /**
> * machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices
> --
> 2.19.1
>
Otherwise
Reviewed-by: Andrew Jones <drjo...@redhat.com>
