* [PATCH 0/2] hw/core/machine: An an unit test for smp_parse
@ 2021-10-10 10:39 Yanan Wang
2021-10-10 10:39 ` [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API Yanan Wang
2021-10-10 10:39 ` [PATCH 2/2] tests/unit: Add an unit test for smp parsing Yanan Wang
0 siblings, 2 replies; 9+ messages in thread
From: Yanan Wang @ 2021-10-10 10:39 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Yanan Wang, wanghaibin.wang,
Paolo Bonzini, Philippe Mathieu-Daudé
Hi,
This series introduces an unit test for generic smp_parse().
We have had enough discussions about what kind of SMP configurations
by the user should be considered valid and what should be invalid.
Since we have finished optimizing the SMP parsing code, then this
test normatively listed all the possible valid/invalid configurations
that the user can provide. This can be a testing tool when we
introduce new topology members and need to touch the parsing code.
For your reference,
Some related discussion is here [1]:
[1] https://lore.kernel.org/qemu-devel/YOxf+sxzUSJdbY+j@redhat.com/
The originally posted test is here [2]:
[2] https://lore.kernel.org/qemu-devel/20210910073025.16480-17-wangyanan55@huawei.com/
Compared to [2], the test in this series is reworked and more brief.
Yanan Wang (2):
hw/core/machine: Split out smp_parse as an inline API
tests/unit: Add an unit test for smp parsing
MAINTAINERS | 2 +
hw/core/machine.c | 160 +---------
include/hw/core/smp.h | 185 +++++++++++
tests/unit/meson.build | 1 +
tests/unit/test-smp-parse.c | 613 ++++++++++++++++++++++++++++++++++++
5 files changed, 802 insertions(+), 159 deletions(-)
create mode 100644 include/hw/core/smp.h
create mode 100644 tests/unit/test-smp-parse.c
--
2.19.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API
2021-10-10 10:39 [PATCH 0/2] hw/core/machine: An an unit test for smp_parse Yanan Wang
@ 2021-10-10 10:39 ` Yanan Wang
2021-10-11 5:26 ` Markus Armbruster
2021-10-10 10:39 ` [PATCH 2/2] tests/unit: Add an unit test for smp parsing Yanan Wang
1 sibling, 1 reply; 9+ messages in thread
From: Yanan Wang @ 2021-10-10 10:39 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Yanan Wang, wanghaibin.wang,
Paolo Bonzini, Philippe Mathieu-Daudé
Functionally smp_parse() is only called once and in one place
i.e. machine_set_smp, the possible second place where it'll be
called should be some unit tests if any.
Actually we are going to introduce an unit test for the parser.
For necessary isolation of the tested code, split smp_parse out
into a separate header as an inline API.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
MAINTAINERS | 1 +
hw/core/machine.c | 160 +-----------------------------------
include/hw/core/smp.h | 185 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 187 insertions(+), 159 deletions(-)
create mode 100644 include/hw/core/smp.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 50435b8d2f..dc9091c1d7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1629,6 +1629,7 @@ F: qapi/machine.json
F: qapi/machine-target.json
F: include/hw/boards.h
F: include/hw/core/cpu.h
+F: include/hw/core/smp.h
F: include/hw/cpu/cluster.h
F: include/sysemu/numa.h
T: git https://gitlab.com/ehabkost/qemu.git machine-next
diff --git a/hw/core/machine.c b/hw/core/machine.c
index b8d95eec32..3018230033 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -17,6 +17,7 @@
#include "qemu/units.h"
#include "hw/boards.h"
#include "hw/loader.h"
+#include "hw/core/smp.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-common.h"
#include "qapi/qapi-visit-machine.h"
@@ -749,165 +750,6 @@ void machine_set_cpu_numa_node(MachineState *machine,
}
}
-/*
- * Report information of a machine's supported CPU topology hierarchy.
- * Topology members will be ordered from the largest to the smallest
- * in the string.
- */
-static char *cpu_hierarchy_to_string(MachineState *ms)
-{
- MachineClass *mc = MACHINE_GET_CLASS(ms);
- GString *s = g_string_new(NULL);
-
- g_string_append_printf(s, "sockets (%u)", ms->smp.sockets);
-
- if (mc->smp_props.dies_supported) {
- g_string_append_printf(s, " * dies (%u)", ms->smp.dies);
- }
-
- g_string_append_printf(s, " * cores (%u)", ms->smp.cores);
- g_string_append_printf(s, " * threads (%u)", ms->smp.threads);
-
- return g_string_free(s, false);
-}
-
-/*
- * smp_parse - Generic function used to parse the given SMP configuration
- *
- * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
- * automatically computed based on the provided ones.
- *
- * In the calculation of omitted sockets/cores/threads: we prefer sockets
- * over cores over threads before 6.2, while preferring cores over sockets
- * over threads since 6.2.
- *
- * In the calculation of cpus/maxcpus: When both maxcpus and cpus are omitted,
- * maxcpus will be computed from the given parameters and cpus will be set
- * equal to maxcpus. When only one of maxcpus and cpus is given then the
- * omitted one will be set to its given counterpart's value. Both maxcpus and
- * cpus may be specified, but maxcpus must be equal to or greater than cpus.
- *
- * For compatibility, apart from the parameters that will be computed, newly
- * introduced topology members which are likely to be target specific should
- * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
- */
-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 : 0;
- unsigned cores = config->has_cores ? config->cores : 0;
- unsigned threads = config->has_threads ? config->threads : 0;
- unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
-
- /*
- * Specified CPU topology parameters must be greater than zero,
- * explicit configuration like "cpus=0" is not allowed.
- */
- 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)) {
- warn_report("Deprecated CPU topology (considered invalid): "
- "CPU topology parameters must be greater than zero");
- }
-
- /*
- * If not supported by the machine, a topology parameter must be
- * omitted or specified equal to 1.
- */
- if (!mc->smp_props.dies_supported && dies > 1) {
- error_setg(errp, "dies not supported by this machine's CPU topology");
- return;
- }
-
- dies = dies > 0 ? dies : 1;
-
- /* compute missing values based on the provided ones */
- if (cpus == 0 && maxcpus == 0) {
- sockets = sockets > 0 ? sockets : 1;
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- } else {
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
- if (mc->smp_props.prefer_sockets) {
- /* prefer sockets over cores before 6.2 */
- if (sockets == 0) {
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (dies * cores * threads);
- } else if (cores == 0) {
- threads = threads > 0 ? threads : 1;
- cores = maxcpus / (sockets * dies * threads);
- }
- } else {
- /* prefer cores over sockets since 6.2 */
- if (cores == 0) {
- sockets = sockets > 0 ? sockets : 1;
- threads = threads > 0 ? threads : 1;
- cores = maxcpus / (sockets * dies * threads);
- } else if (sockets == 0) {
- threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (dies * cores * threads);
- }
- }
-
- /* try to calculate omitted threads at last */
- if (threads == 0) {
- threads = maxcpus / (sockets * dies * cores);
- }
- }
-
- maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
- cpus = cpus > 0 ? cpus : maxcpus;
-
- 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;
-
- /* sanity-check of the computed topology */
- if (sockets * dies * cores * threads != maxcpus) {
- g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
- error_setg(errp, "Invalid CPU topology: "
- "product of the hierarchy must match maxcpus: "
- "%s != maxcpus (%u)",
- topo_msg, maxcpus);
- return;
- }
-
- if (maxcpus < cpus) {
- g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
- error_setg(errp, "Invalid CPU topology: "
- "maxcpus must be equal to or greater than smp: "
- "%s == maxcpus (%u) < smp_cpus (%u)",
- topo_msg, maxcpus, cpus);
- return;
- }
-
- if (ms->smp.cpus < mc->min_cpus) {
- error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
- "supported by machine '%s' is %d",
- ms->smp.cpus,
- mc->name, mc->min_cpus);
- return;
- }
-
- if (ms->smp.max_cpus > mc->max_cpus) {
- error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
- "supported by machine '%s' is %d",
- ms->smp.max_cpus,
- mc->name, mc->max_cpus);
- return;
- }
-}
-
static void machine_get_smp(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
diff --git a/include/hw/core/smp.h b/include/hw/core/smp.h
new file mode 100644
index 0000000000..fe892a5193
--- /dev/null
+++ b/include/hw/core/smp.h
@@ -0,0 +1,185 @@
+/*
+ * QEMU Machine core (related to SMP)
+ *
+ * Copyright (c) 2021 Huawei Technologies Co., Ltd
+ *
+ * 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 MACHINE_CORE_SMP_H
+#define MACHINE_CORE_SMP_H
+
+#include "hw/boards.h"
+#include "qapi/error.h"
+
+/*
+ * Report information of a machine's supported CPU topology hierarchy.
+ * Topology members will be ordered from the largest to the smallest
+ * in the string.
+ */
+static inline char *cpu_hierarchy_to_string(MachineState *ms)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+ GString *s = g_string_new(NULL);
+
+ g_string_append_printf(s, "sockets (%u)", ms->smp.sockets);
+
+ if (mc->smp_props.dies_supported) {
+ g_string_append_printf(s, " * dies (%u)", ms->smp.dies);
+ }
+
+ g_string_append_printf(s, " * cores (%u)", ms->smp.cores);
+ g_string_append_printf(s, " * threads (%u)", ms->smp.threads);
+
+ return g_string_free(s, false);
+}
+
+/*
+ * smp_parse - Generic function used to parse the given SMP configuration
+ *
+ * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
+ * automatically computed based on the provided ones.
+ *
+ * In the calculation of omitted sockets/cores/threads: we prefer sockets
+ * over cores over threads before 6.2, while preferring cores over sockets
+ * over threads since 6.2.
+ *
+ * In the calculation of cpus/maxcpus: When both maxcpus and cpus are omitted,
+ * maxcpus will be computed from the given parameters and cpus will be set
+ * equal to maxcpus. When only one of maxcpus and cpus is given then the
+ * omitted one will be set to its given counterpart's value. Both maxcpus and
+ * cpus may be specified, but maxcpus must be equal to or greater than cpus.
+ *
+ * For compatibility, apart from the parameters that will be computed, newly
+ * introduced topology members which are likely to be target specific should
+ * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
+ */
+static inline 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 : 0;
+ unsigned cores = config->has_cores ? config->cores : 0;
+ unsigned threads = config->has_threads ? config->threads : 0;
+ unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+
+ /*
+ * Specified CPU topology parameters must be greater than zero,
+ * explicit configuration like "cpus=0" is not allowed.
+ */
+ 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)) {
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "CPU topology parameters must be greater than zero");
+ }
+
+ /*
+ * If not supported by the machine, a topology parameter must be
+ * omitted or specified equal to 1.
+ */
+ if (!mc->smp_props.dies_supported && dies > 1) {
+ error_setg(errp, "dies not supported by this machine's CPU topology");
+ return;
+ }
+
+ dies = dies > 0 ? dies : 1;
+
+ /* compute missing values based on the provided ones */
+ if (cpus == 0 && maxcpus == 0) {
+ sockets = sockets > 0 ? sockets : 1;
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ } else {
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+ if (mc->smp_props.prefer_sockets) {
+ /* prefer sockets over cores before 6.2 */
+ if (sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (dies * cores * threads);
+ } else if (cores == 0) {
+ threads = threads > 0 ? threads : 1;
+ cores = maxcpus / (sockets * dies * threads);
+ }
+ } else {
+ /* prefer cores over sockets since 6.2 */
+ if (cores == 0) {
+ sockets = sockets > 0 ? sockets : 1;
+ threads = threads > 0 ? threads : 1;
+ cores = maxcpus / (sockets * dies * threads);
+ } else if (sockets == 0) {
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (dies * cores * threads);
+ }
+ }
+
+ /* try to calculate omitted threads at last */
+ if (threads == 0) {
+ threads = maxcpus / (sockets * dies * cores);
+ }
+ }
+
+ maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
+ cpus = cpus > 0 ? cpus : maxcpus;
+
+ 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;
+
+ /* sanity-check of the computed topology */
+ if (sockets * dies * cores * threads != maxcpus) {
+ g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
+ error_setg(errp, "Invalid CPU topology: "
+ "product of the hierarchy must match maxcpus: "
+ "%s != maxcpus (%u)",
+ topo_msg, maxcpus);
+ return;
+ }
+
+ if (maxcpus < cpus) {
+ g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
+ error_setg(errp, "Invalid CPU topology: "
+ "maxcpus must be equal to or greater than smp: "
+ "%s == maxcpus (%u) < smp_cpus (%u)",
+ topo_msg, maxcpus, cpus);
+ return;
+ }
+
+ if (ms->smp.cpus < mc->min_cpus) {
+ error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
+ "supported by machine '%s' is %d",
+ ms->smp.cpus,
+ mc->name, mc->min_cpus);
+ return;
+ }
+
+ if (ms->smp.max_cpus > mc->max_cpus) {
+ error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
+ "supported by machine '%s' is %d",
+ ms->smp.max_cpus,
+ mc->name, mc->max_cpus);
+ return;
+ }
+}
+
+#endif
--
2.19.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] tests/unit: Add an unit test for smp parsing
2021-10-10 10:39 [PATCH 0/2] hw/core/machine: An an unit test for smp_parse Yanan Wang
2021-10-10 10:39 ` [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API Yanan Wang
@ 2021-10-10 10:39 ` Yanan Wang
2021-10-12 13:51 ` Andrew Jones
1 sibling, 1 reply; 9+ messages in thread
From: Yanan Wang @ 2021-10-10 10:39 UTC (permalink / raw)
To: qemu-devel
Cc: Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Yanan Wang, wanghaibin.wang,
Paolo Bonzini, Philippe Mathieu-Daudé
Now that we have a generic parser smp_parse(), let's add an unit
test for the code. All possible valid/invalid SMP configurations
that the user can specify are covered.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
MAINTAINERS | 1 +
tests/unit/meson.build | 1 +
tests/unit/test-smp-parse.c | 613 ++++++++++++++++++++++++++++++++++++
3 files changed, 615 insertions(+)
create mode 100644 tests/unit/test-smp-parse.c
diff --git a/MAINTAINERS b/MAINTAINERS
index dc9091c1d7..b5a5b1469b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1632,6 +1632,7 @@ F: include/hw/core/cpu.h
F: include/hw/core/smp.h
F: include/hw/cpu/cluster.h
F: include/sysemu/numa.h
+F: tests/unit/test-smp-parse.c
T: git https://gitlab.com/ehabkost/qemu.git machine-next
Xtensa Machines
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 7c297d7e5c..0382669fcf 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -45,6 +45,7 @@ tests = {
'test-uuid': [],
'ptimer-test': ['ptimer-test-stubs.c', meson.project_source_root() / 'hw/core/ptimer.c'],
'test-qapi-util': [],
+ 'test-smp-parse': [qom],
}
if have_system or have_tools
diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
new file mode 100644
index 0000000000..7be258171e
--- /dev/null
+++ b/tests/unit/test-smp-parse.c
@@ -0,0 +1,613 @@
+/*
+ * SMP parsing unit-tests
+ *
+ * Copyright (c) 2021 Huawei Technologies Co., Ltd
+ *
+ * Authors:
+ * Yanan Wang <wangyanan55@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+
+#include "hw/boards.h"
+#include "hw/core/smp.h"
+
+#define T true
+#define F false
+
+#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */
+#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
+
+/*
+ * Used to define the generic 3-level CPU topology hierarchy
+ * -sockets/cores/threads
+ */
+#define SMP_CONFIG_GENERIC(ha, a, hb, b, hc, c, hd, d, he, e) \
+ { \
+ .has_cpus = ha, .cpus = a, \
+ .has_sockets = hb, .sockets = b, \
+ .has_cores = hc, .cores = c, \
+ .has_threads = hd, .threads = d, \
+ .has_maxcpus = he, .maxcpus = e, \
+ }
+
+#define CPU_TOPOLOGY_GENERIC(a, b, c, d, e) \
+ { \
+ .cpus = a, \
+ .sockets = b, \
+ .cores = c, \
+ .threads = d, \
+ .max_cpus = e, \
+ }
+
+/*
+ * Currently a 4-level topology hierarchy is supported on PC machines
+ * -sockets/dies/cores/threads
+ */
+#define SMP_CONFIG_WITH_DIES(ha, a, hb, b, hc, c, hd, d, he, e, hf, f) \
+ { \
+ .has_cpus = ha, .cpus = a, \
+ .has_sockets = hb, .sockets = b, \
+ .has_dies = hc, .dies = c, \
+ .has_cores = hd, .cores = d, \
+ .has_threads = he, .threads = e, \
+ .has_maxcpus = hf, .maxcpus = f, \
+ }
+
+#define CPU_TOPOLOGY_WITH_DIES(a, b, c, d, e, f) \
+ { \
+ .cpus = a, \
+ .sockets = b, \
+ .dies = c, \
+ .cores = d, \
+ .threads = e, \
+ .max_cpus = f, \
+ }
+
+/**
+ * @config - the given SMP configuration
+ * @expect_prefer_sockets - the expected parsing result for the
+ * valid configuration, when sockets are preferred over cores
+ * @expect_prefer_cores - the expected parsing result for the
+ * valid configuration, when cores are preferred over sockets
+ * @expect_error - the expected error report when the given
+ * configuration is invalid
+ */
+typedef struct SMPTestData {
+ SMPConfiguration config;
+ CpuTopology expect_prefer_sockets;
+ CpuTopology expect_prefer_cores;
+ const char *expect_error;
+} SMPTestData;
+
+/* Type info of the tested machine */
+static const TypeInfo smp_machine_info = {
+ .name = TYPE_MACHINE,
+ .parent = TYPE_OBJECT,
+ .class_size = sizeof(MachineClass),
+ .instance_size = sizeof(MachineState),
+};
+
+/*
+ * List all the possible valid sub-collections of the generic 5
+ * topology parameters (i.e. cpus/maxcpus/sockets/cores/threads),
+ * then test the automatic calculation algorithm of the missing
+ * values in the parser.
+ */
+static struct SMPTestData data_generic_valid[] = {
+ {
+ /* config: no configuration provided
+ * expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
+ }, {
+ /* config: -smp 8
+ * prefer_sockets: cpus=8,sockets=8,cores=1,threads=1,maxcpus=8
+ * prefer_cores: cpus=8,sockets=1,cores=8,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 1, 8),
+ }, {
+ /* config: -smp sockets=2
+ * expect: cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
+ }, {
+ /* config: -smp cores=4
+ * expect: cpus=4,sockets=1,cores=4,threads=1,maxcpus=4 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
+ }, {
+ /* config: -smp threads=2
+ * expect: cpus=2,sockets=1,cores=1,threads=2,maxcpus=2 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
+ }, {
+ /* config: -smp maxcpus=16
+ * prefer_sockets: cpus=16,sockets=16,cores=1,threads=1,maxcpus=16
+ * prefer_cores: cpus=16,sockets=1,cores=16,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 16, 1, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 16, 1, 16),
+ }, {
+ /* config: -smp 8,sockets=2
+ * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ }, {
+ /* config: -smp 8,cores=4
+ * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ }, {
+ /* config: -smp 8,threads=2
+ * prefer_sockets: cpus=8,sockets=4,cores=1,threads=2,maxcpus=8
+ * prefer_cores: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 1, 2, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
+ }, {
+ /* config: -smp 8,maxcpus=16
+ * prefer_sockets: cpus=8,sockets=16,cores=1,threads=1,maxcpus=16
+ * prefer_cores: cpus=8,sockets=1,cores=16,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 16, 1, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 16, 1, 16),
+ }, {
+ /* config: -smp sockets=2,cores=4
+ * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ }, {
+ /* config: -smp sockets=2,threads=2
+ * expect: cpus=4,sockets=2,cores=1,threads=2,maxcpus=4 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
+ }, {
+ /* config: -smp sockets=2,maxcpus=16
+ * expect: cpus=16,sockets=2,cores=8,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
+ }, {
+ /* config: -smp cores=4,threads=2
+ * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
+ }, {
+ /* config: -smp cores=4,maxcpus=16
+ * expect: cpus=16,sockets=4,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
+ }, {
+ /* config: -smp threads=2,maxcpus=16
+ * prefer_sockets: cpus=16,sockets=8,cores=1,threads=2,maxcpus=16
+ * prefer_cores: cpus=16,sockets=1,cores=8,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 8, 1, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 8, 2, 16),
+ }, {
+ /* config: -smp 8,sockets=2,cores=4
+ * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ }, {
+ /* config: -smp 8,sockets=2,threads=2
+ * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
+ }, {
+ /* config: -smp 8,sockets=2,maxcpus=16
+ * expect: cpus=8,sockets=2,cores=8,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
+ }, {
+ /* config: -smp 8,cores=4,threads=2
+ * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
+ }, {
+ /* config: -smp 8,cores=4,maxcpus=16
+ * expect: cpus=8,sockets=4,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
+ }, {
+ /* config: -smp 8,threads=2,maxcpus=16
+ * prefer_sockets: cpus=8,sockets=8,cores=1,threads=2,maxcpus=16
+ * prefer_cores: cpus=8,sockets=1,cores=8,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 2, 16),
+ }, {
+ /* config: -smp sockets=2,cores=4,threads=2
+ * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ }, {
+ /* config: -smp sockets=2,cores=4,maxcpus=16
+ * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ }, {
+ /* config: -smp sockets=2,threads=2,maxcpus=16
+ * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ }, {
+ /* config: -smp cores=4,threads=2,maxcpus=16
+ * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 8,sockets=2,cores=4,threads=1
+ * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 1, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
+ }, {
+ /* config: -smp 8,sockets=2,cores=4,maxcpus=16
+ * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 8,sockets=2,threads=2,maxcpus=16
+ * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 8,cores=4,threads=2,maxcpus=16
+ * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ }, {
+ /* config: -smp sockets=2,cores=4,threads=2,maxcpus=16
+ * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
+ * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 16),
+ .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
+ },
+};
+
+static struct SMPTestData data_generic_invalid[] = {
+ {
+ /* config: -smp 2,dies=2 */
+ .config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
+ .expect_error = "dies not supported by this machine's CPU topology",
+ }, {
+ /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
+ .expect_error = "Invalid CPU topology: "
+ "product of the hierarchy must match maxcpus: "
+ "sockets (2) * cores (4) * threads (2) "
+ "!= maxcpus (8)",
+ }, {
+ /* config: -smp 18,sockets=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_GENERIC(T, 18, T, 2, T, 4, T, 2, T, 16),
+ .expect_error = "Invalid CPU topology: "
+ "maxcpus must be equal to or greater than smp: "
+ "sockets (2) * cores (4) * threads (2) "
+ "== maxcpus (16) < smp_cpus (18)",
+ }, {
+ /* config: -smp 1
+ * should tweak the supported min CPUs to 2 for testing */
+ .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
+ .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
+ "by machine '(null)' is 2",
+ }, {
+ /* config: -smp 512
+ * should tweak the supported max CPUs to 511 for testing */
+ .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
+ .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
+ "by machine '(null)' is 511",
+ },
+};
+
+static struct SMPTestData data_with_dies_invalid[] = {
+ {
+ /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 16),
+ .expect_error = "Invalid CPU topology: "
+ "product of the hierarchy must match maxcpus: "
+ "sockets (2) * dies (2) * cores (4) * threads (2) "
+ "!= maxcpus (16)",
+ }, {
+ /* config: -smp 34,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 34, T, 2, T, 2, T, 4, T, 2, T, 32),
+ .expect_error = "Invalid CPU topology: "
+ "maxcpus must be equal to or greater than smp: "
+ "sockets (2) * dies (2) * cores (4) * threads (2) "
+ "== maxcpus (32) < smp_cpus (34)",
+ },
+};
+
+static char *smp_config_to_string(SMPConfiguration *config)
+{
+ return g_strdup_printf(
+ "(SMPConfiguration) {\n"
+ " .has_cpus = %5s, cpus = %ld,\n"
+ " .has_sockets = %5s, sockets = %ld,\n"
+ " .has_dies = %5s, dies = %ld,\n"
+ " .has_cores = %5s, cores = %ld,\n"
+ " .has_threads = %5s, threads = %ld,\n"
+ " .has_maxcpus = %5s, maxcpus = %ld,\n"
+ "}",
+ config->has_cpus ? "true" : "false", config->cpus,
+ config->has_sockets ? "true" : "false", config->sockets,
+ config->has_dies ? "true" : "false", config->dies,
+ config->has_cores ? "true" : "false", config->cores,
+ config->has_threads ? "true" : "false", config->threads,
+ config->has_maxcpus ? "true" : "false", config->maxcpus);
+}
+
+static char *cpu_topology_to_string(CpuTopology *topo)
+{
+ return g_strdup_printf(
+ "(CpuTopology) {\n"
+ " .cpus = %u,\n"
+ " .sockets = %u,\n"
+ " .dies = %u,\n"
+ " .cores = %u,\n"
+ " .threads = %u,\n"
+ " .max_cpus = %u,\n"
+ "}",
+ topo->cpus, topo->sockets, topo->dies,
+ topo->cores, topo->threads, topo->max_cpus);
+}
+
+static void check_parse(MachineState *ms, SMPConfiguration *config,
+ CpuTopology *expect_topo, const char *expect_err,
+ bool is_valid)
+{
+ g_autofree char *config_str = smp_config_to_string(config);
+ g_autofree char *expect_topo_str = cpu_topology_to_string(expect_topo);
+ g_autofree char *output_topo_str = NULL;
+ Error *err = NULL;
+
+ /* call the generic parser smp_parse() */
+ smp_parse(ms, config, &err);
+
+ output_topo_str = cpu_topology_to_string(&ms->smp);
+
+ /* when the configuration is supposed to be valid */
+ if (is_valid) {
+ if ((err == NULL) &&
+ (ms->smp.cpus == expect_topo->cpus) &&
+ (ms->smp.sockets == expect_topo->sockets) &&
+ (ms->smp.dies == expect_topo->dies) &&
+ (ms->smp.cores == expect_topo->cores) &&
+ (ms->smp.threads == expect_topo->threads) &&
+ (ms->smp.max_cpus == expect_topo->max_cpus)) {
+ return;
+ }
+
+ if (err != NULL) {
+ g_printerr("Test smp_parse failed!\n"
+ "Input configuration: %s\n"
+ "Should be valid: yes\n"
+ "Expected topology: %s\n\n"
+ "Result is valid: no\n"
+ "Result error report: %s\n",
+ config_str, expect_topo_str, error_get_pretty(err));
+ goto end;
+ }
+
+ g_printerr("Test smp_parse failed!\n"
+ "Input configuration: %s\n"
+ "Should be valid: yes\n"
+ "Expected topology: %s\n\n"
+ "Result is valid: yes\n"
+ "Output topology: %s\n",
+ config_str, expect_topo_str, output_topo_str);
+ goto end;
+ }
+
+ /* when the configuration is supposed to be invalid */
+ if (err != NULL) {
+ if (expect_err == NULL ||
+ g_str_equal(expect_err, error_get_pretty(err))) {
+ error_free(err);
+ return;
+ }
+
+ g_printerr("Test smp_parse failed!\n"
+ "Input configuration: %s\n"
+ "Should be valid: no\n"
+ "Expected error report: %s\n\n"
+ "Result is valid: no\n"
+ "Output error report: %s\n",
+ config_str, expect_err, error_get_pretty(err));
+ goto end;
+ }
+
+ g_printerr("Test smp_parse failed!\n"
+ "Input configuration: %s\n"
+ "Should be valid: no\n"
+ "Expected error report: %s\n\n"
+ "Result is valid: yes\n"
+ "Output topology: %s\n",
+ config_str, expect_err, output_topo_str);
+
+end:
+ if (err != NULL) {
+ error_free(err);
+ }
+
+ abort();
+}
+
+static void smp_parse_test(MachineState *ms, SMPTestData *data, bool is_valid)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+
+ mc->smp_props.prefer_sockets = true;
+ check_parse(ms, &data->config, &data->expect_prefer_sockets,
+ data->expect_error, is_valid);
+
+ mc->smp_props.prefer_sockets = false;
+ check_parse(ms, &data->config, &data->expect_prefer_cores,
+ data->expect_error, is_valid);
+}
+
+static void test_data_copy(SMPTestData *targ, SMPTestData *src)
+{
+ targ->config = src->config;
+ targ->expect_prefer_sockets = src->expect_prefer_sockets;
+ targ->expect_prefer_cores = src->expect_prefer_cores;
+ targ->expect_error = src->expect_error;
+}
+
+/* The parsed results of the unsupported parameters should be 1 */
+static void unsupported_params_init(MachineClass *mc, SMPTestData *data)
+{
+ if (!mc->smp_props.dies_supported) {
+ data->expect_prefer_sockets.dies = 1;
+ data->expect_prefer_cores.dies = 1;
+ }
+}
+
+/* Reset the related machine properties before each sub-test */
+static void smp_machine_class_init(MachineClass *mc)
+{
+ mc->min_cpus = MIN_CPUS;
+ mc->max_cpus = MAX_CPUS;
+
+ mc->smp_props.prefer_sockets = true;
+ mc->smp_props.dies_supported = false;
+}
+
+static void test_generic(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData *data = &(SMPTestData){0};
+ int i;
+
+ smp_machine_class_init(mc);
+
+ for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+ test_data_copy(data, &data_generic_valid[i]);
+ unsupported_params_init(mc, data);
+
+ smp_parse_test(ms, data, true);
+
+ /* Unsupported parameters can be provided with their values as 1 */
+ data->config.has_dies = true;
+ data->config.dies = 1;
+ smp_parse_test(ms, data, true);
+ }
+
+ /* Reset the supported min CPUs and max CPUs */
+ mc->min_cpus = 2;
+ mc->max_cpus = 511;
+
+ for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
+ test_data_copy(data, &data_generic_invalid[i]);
+ unsupported_params_init(mc, data);
+
+ smp_parse_test(ms, data, false);
+ }
+
+ object_unref(obj);
+}
+
+static void test_with_dies(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData *data = &(SMPTestData){0};
+ unsigned int num_dies = 2;
+ int i;
+
+ smp_machine_class_init(mc);
+ mc->smp_props.dies_supported = true;
+
+ for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+ test_data_copy(data, &data_generic_valid[i]);
+ unsupported_params_init(mc, data);
+
+ /* when dies parameter is omitted, it will be set as 1 */
+ data->expect_prefer_sockets.dies = 1;
+ data->expect_prefer_cores.dies = 1;
+
+ smp_parse_test(ms, data, true);
+
+ /* when dies parameter is specified */
+ data->config.has_dies = true;
+ data->config.dies = num_dies;
+ if (data->config.has_cpus) {
+ data->config.cpus *= num_dies;
+ }
+ if (data->config.has_maxcpus) {
+ data->config.maxcpus *= num_dies;
+ }
+
+ data->expect_prefer_sockets.dies = num_dies;
+ data->expect_prefer_sockets.cpus *= num_dies;
+ data->expect_prefer_sockets.max_cpus *= num_dies;
+ data->expect_prefer_cores.dies = num_dies;
+ data->expect_prefer_cores.cpus *= num_dies;
+ data->expect_prefer_cores.max_cpus *= num_dies;
+
+ smp_parse_test(ms, data, true);
+ }
+
+ for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) {
+ test_data_copy(data, &data_with_dies_invalid[i]);
+ unsupported_params_init(mc, data);
+
+ smp_parse_test(ms, data, false);
+ }
+
+ object_unref(obj);
+}
+
+int main(int argc, char *argv[])
+{
+ g_test_init(&argc, &argv, NULL);
+
+ module_call_init(MODULE_INIT_QOM);
+ type_register_static(&smp_machine_info);
+
+ g_test_add_func("/test-smp-parse/generic", test_generic);
+ g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
+
+ g_test_run();
+
+ return 0;
+}
--
2.19.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API
2021-10-10 10:39 ` [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API Yanan Wang
@ 2021-10-11 5:26 ` Markus Armbruster
2021-10-11 7:54 ` wangyanan (Y)
0 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2021-10-11 5:26 UTC (permalink / raw)
To: Yanan Wang
Cc: Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, Paolo Bonzini,
wanghaibin.wang, Philippe Mathieu-Daudé
Yanan Wang <wangyanan55@huawei.com> writes:
> Functionally smp_parse() is only called once and in one place
> i.e. machine_set_smp, the possible second place where it'll be
> called should be some unit tests if any.
>
> Actually we are going to introduce an unit test for the parser.
> For necessary isolation of the tested code, split smp_parse out
> into a separate header as an inline API.
Why inline?
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API
2021-10-11 5:26 ` Markus Armbruster
@ 2021-10-11 7:54 ` wangyanan (Y)
2021-10-12 14:36 ` Markus Armbruster
0 siblings, 1 reply; 9+ messages in thread
From: wangyanan (Y) @ 2021-10-11 7:54 UTC (permalink / raw)
To: Markus Armbruster
Cc: Andrew Jones, Daniel P.Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, Paolo Bonzini,
wanghaibin.wang, Philippe Mathieu-Daudé
Hi Markus,
On 2021/10/11 13:26, Markus Armbruster wrote:
> Yanan Wang <wangyanan55@huawei.com> writes:
>
>> Functionally smp_parse() is only called once and in one place
>> i.e. machine_set_smp, the possible second place where it'll be
>> called should be some unit tests if any.
>>
>> Actually we are going to introduce an unit test for the parser.
>> For necessary isolation of the tested code, split smp_parse out
>> into a separate header as an inline API.
> Why inline?
The motivation of the splitting is to isolate the tested smp_parse
from the other unrelated code in machine.c, so that we can solve
the build dependency problem for the unit test.
I once tried to split smp_parse out into a source file in [1] for the
test, but it looks more concise and convenient to make it as an
inline function in a header compared to [1]. Given that we only call
it in one place, it may not be harmful to keep it an inline.
Anyway, I not sure the method in this patch is most appropriate
and compliant. If it's just wrong I can change back to [1]. :)
[1]
https://lore.kernel.org/qemu-devel/20210910073025.16480-16-wangyanan55@huawei.com/#t
Thanks,
Yanan
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> .
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/unit: Add an unit test for smp parsing
2021-10-10 10:39 ` [PATCH 2/2] tests/unit: Add an unit test for smp parsing Yanan Wang
@ 2021-10-12 13:51 ` Andrew Jones
2021-10-13 1:23 ` wangyanan (Y)
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jones @ 2021-10-12 13:51 UTC (permalink / raw)
To: Yanan Wang
Cc: Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, wanghaibin.wang,
Paolo Bonzini, Philippe Mathieu-Daudé
On Sun, Oct 10, 2021 at 06:39:54PM +0800, Yanan Wang wrote:
> Now that we have a generic parser smp_parse(), let's add an unit
> test for the code. All possible valid/invalid SMP configurations
> that the user can specify are covered.
>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
> MAINTAINERS | 1 +
> tests/unit/meson.build | 1 +
> tests/unit/test-smp-parse.c | 613 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 615 insertions(+)
> create mode 100644 tests/unit/test-smp-parse.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dc9091c1d7..b5a5b1469b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1632,6 +1632,7 @@ F: include/hw/core/cpu.h
> F: include/hw/core/smp.h
> F: include/hw/cpu/cluster.h
> F: include/sysemu/numa.h
> +F: tests/unit/test-smp-parse.c
> T: git https://gitlab.com/ehabkost/qemu.git machine-next
>
> Xtensa Machines
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 7c297d7e5c..0382669fcf 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -45,6 +45,7 @@ tests = {
> 'test-uuid': [],
> 'ptimer-test': ['ptimer-test-stubs.c', meson.project_source_root() / 'hw/core/ptimer.c'],
> 'test-qapi-util': [],
> + 'test-smp-parse': [qom],
> }
>
> if have_system or have_tools
> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
> new file mode 100644
> index 0000000000..7be258171e
> --- /dev/null
> +++ b/tests/unit/test-smp-parse.c
> @@ -0,0 +1,613 @@
> +/*
> + * SMP parsing unit-tests
> + *
> + * Copyright (c) 2021 Huawei Technologies Co., Ltd
> + *
> + * Authors:
> + * Yanan Wang <wangyanan55@huawei.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qom/object.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +
> +#include "hw/boards.h"
> +#include "hw/core/smp.h"
> +
> +#define T true
> +#define F false
> +
> +#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */
> +#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
> +
> +/*
> + * Used to define the generic 3-level CPU topology hierarchy
> + * -sockets/cores/threads
> + */
> +#define SMP_CONFIG_GENERIC(ha, a, hb, b, hc, c, hd, d, he, e) \
> + { \
> + .has_cpus = ha, .cpus = a, \
> + .has_sockets = hb, .sockets = b, \
> + .has_cores = hc, .cores = c, \
> + .has_threads = hd, .threads = d, \
> + .has_maxcpus = he, .maxcpus = e, \
> + }
> +
> +#define CPU_TOPOLOGY_GENERIC(a, b, c, d, e) \
> + { \
> + .cpus = a, \
> + .sockets = b, \
> + .cores = c, \
> + .threads = d, \
> + .max_cpus = e, \
> + }
> +
> +/*
> + * Currently a 4-level topology hierarchy is supported on PC machines
> + * -sockets/dies/cores/threads
> + */
> +#define SMP_CONFIG_WITH_DIES(ha, a, hb, b, hc, c, hd, d, he, e, hf, f) \
> + { \
> + .has_cpus = ha, .cpus = a, \
> + .has_sockets = hb, .sockets = b, \
> + .has_dies = hc, .dies = c, \
> + .has_cores = hd, .cores = d, \
> + .has_threads = he, .threads = e, \
> + .has_maxcpus = hf, .maxcpus = f, \
> + }
> +
> +#define CPU_TOPOLOGY_WITH_DIES(a, b, c, d, e, f) \
> + { \
> + .cpus = a, \
> + .sockets = b, \
> + .dies = c, \
> + .cores = d, \
> + .threads = e, \
> + .max_cpus = f, \
> + }
> +
> +/**
> + * @config - the given SMP configuration
> + * @expect_prefer_sockets - the expected parsing result for the
> + * valid configuration, when sockets are preferred over cores
> + * @expect_prefer_cores - the expected parsing result for the
> + * valid configuration, when cores are preferred over sockets
> + * @expect_error - the expected error report when the given
> + * configuration is invalid
> + */
> +typedef struct SMPTestData {
> + SMPConfiguration config;
> + CpuTopology expect_prefer_sockets;
> + CpuTopology expect_prefer_cores;
> + const char *expect_error;
> +} SMPTestData;
> +
> +/* Type info of the tested machine */
> +static const TypeInfo smp_machine_info = {
> + .name = TYPE_MACHINE,
> + .parent = TYPE_OBJECT,
> + .class_size = sizeof(MachineClass),
> + .instance_size = sizeof(MachineState),
> +};
> +
> +/*
> + * List all the possible valid sub-collections of the generic 5
> + * topology parameters (i.e. cpus/maxcpus/sockets/cores/threads),
> + * then test the automatic calculation algorithm of the missing
> + * values in the parser.
> + */
> +static struct SMPTestData data_generic_valid[] = {
> + {
> + /* config: no configuration provided
> + * expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
> + }, {
> + /* config: -smp 8
> + * prefer_sockets: cpus=8,sockets=8,cores=1,threads=1,maxcpus=8
> + * prefer_cores: cpus=8,sockets=1,cores=8,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 1, 8),
> + }, {
> + /* config: -smp sockets=2
> + * expect: cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
> + }, {
> + /* config: -smp cores=4
> + * expect: cpus=4,sockets=1,cores=4,threads=1,maxcpus=4 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
> + }, {
> + /* config: -smp threads=2
> + * expect: cpus=2,sockets=1,cores=1,threads=2,maxcpus=2 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
> + }, {
> + /* config: -smp maxcpus=16
> + * prefer_sockets: cpus=16,sockets=16,cores=1,threads=1,maxcpus=16
> + * prefer_cores: cpus=16,sockets=1,cores=16,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 16, 1, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 16, 1, 16),
> + }, {
> + /* config: -smp 8,sockets=2
> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + }, {
> + /* config: -smp 8,cores=4
> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + }, {
> + /* config: -smp 8,threads=2
> + * prefer_sockets: cpus=8,sockets=4,cores=1,threads=2,maxcpus=8
> + * prefer_cores: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 1, 2, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
> + }, {
> + /* config: -smp 8,maxcpus=16
> + * prefer_sockets: cpus=8,sockets=16,cores=1,threads=1,maxcpus=16
> + * prefer_cores: cpus=8,sockets=1,cores=16,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 16, 1, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 16, 1, 16),
> + }, {
> + /* config: -smp sockets=2,cores=4
> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + }, {
> + /* config: -smp sockets=2,threads=2
> + * expect: cpus=4,sockets=2,cores=1,threads=2,maxcpus=4 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
> + }, {
> + /* config: -smp sockets=2,maxcpus=16
> + * expect: cpus=16,sockets=2,cores=8,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
> + }, {
> + /* config: -smp cores=4,threads=2
> + * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
> + }, {
> + /* config: -smp cores=4,maxcpus=16
> + * expect: cpus=16,sockets=4,cores=4,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
> + }, {
> + /* config: -smp threads=2,maxcpus=16
> + * prefer_sockets: cpus=16,sockets=8,cores=1,threads=2,maxcpus=16
> + * prefer_cores: cpus=16,sockets=1,cores=8,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 8, 1, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 8, 2, 16),
> + }, {
> + /* config: -smp 8,sockets=2,cores=4
> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + }, {
> + /* config: -smp 8,sockets=2,threads=2
> + * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
> + }, {
> + /* config: -smp 8,sockets=2,maxcpus=16
> + * expect: cpus=8,sockets=2,cores=8,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
> + }, {
> + /* config: -smp 8,cores=4,threads=2
> + * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
> + }, {
> + /* config: -smp 8,cores=4,maxcpus=16
> + * expect: cpus=8,sockets=4,cores=4,threads=1,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
> + }, {
> + /* config: -smp 8,threads=2,maxcpus=16
> + * prefer_sockets: cpus=8,sockets=8,cores=1,threads=2,maxcpus=16
> + * prefer_cores: cpus=8,sockets=1,cores=8,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 2, 16),
> + }, {
> + /* config: -smp sockets=2,cores=4,threads=2
> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + }, {
> + /* config: -smp sockets=2,cores=4,maxcpus=16
> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + }, {
> + /* config: -smp sockets=2,threads=2,maxcpus=16
> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + }, {
> + /* config: -smp cores=4,threads=2,maxcpus=16
> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + }, {
> + /* config: -smp 8,sockets=2,cores=4,threads=1
> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 1, F, 0),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
> + }, {
> + /* config: -smp 8,sockets=2,cores=4,maxcpus=16
> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + }, {
> + /* config: -smp 8,sockets=2,threads=2,maxcpus=16
> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + }, {
> + /* config: -smp 8,cores=4,threads=2,maxcpus=16
> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + }, {
> + /* config: -smp sockets=2,cores=4,threads=2,maxcpus=16
> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
> + }, {
> + /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 16),
> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
> + },
> +};
> +
> +static struct SMPTestData data_generic_invalid[] = {
> + {
> + /* config: -smp 2,dies=2 */
> + .config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
> + .expect_error = "dies not supported by this machine's CPU topology",
> + }, {
> + /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
> + .expect_error = "Invalid CPU topology: "
> + "product of the hierarchy must match maxcpus: "
> + "sockets (2) * cores (4) * threads (2) "
> + "!= maxcpus (8)",
> + }, {
> + /* config: -smp 18,sockets=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_GENERIC(T, 18, T, 2, T, 4, T, 2, T, 16),
> + .expect_error = "Invalid CPU topology: "
> + "maxcpus must be equal to or greater than smp: "
> + "sockets (2) * cores (4) * threads (2) "
> + "== maxcpus (16) < smp_cpus (18)",
> + }, {
> + /* config: -smp 1
> + * should tweak the supported min CPUs to 2 for testing */
> + .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
> + .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
> + "by machine '(null)' is 2",
> + }, {
> + /* config: -smp 512
> + * should tweak the supported max CPUs to 511 for testing */
> + .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
> + .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
> + "by machine '(null)' is 511",
> + },
> +};
> +
> +static struct SMPTestData data_with_dies_invalid[] = {
> + {
> + /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
> + .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 16),
> + .expect_error = "Invalid CPU topology: "
> + "product of the hierarchy must match maxcpus: "
> + "sockets (2) * dies (2) * cores (4) * threads (2) "
> + "!= maxcpus (16)",
> + }, {
> + /* config: -smp 34,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
> + .config = SMP_CONFIG_WITH_DIES(T, 34, T, 2, T, 2, T, 4, T, 2, T, 32),
> + .expect_error = "Invalid CPU topology: "
> + "maxcpus must be equal to or greater than smp: "
> + "sockets (2) * dies (2) * cores (4) * threads (2) "
> + "== maxcpus (32) < smp_cpus (34)",
> + },
> +};
> +
> +static char *smp_config_to_string(SMPConfiguration *config)
> +{
> + return g_strdup_printf(
> + "(SMPConfiguration) {\n"
> + " .has_cpus = %5s, cpus = %ld,\n"
> + " .has_sockets = %5s, sockets = %ld,\n"
> + " .has_dies = %5s, dies = %ld,\n"
> + " .has_cores = %5s, cores = %ld,\n"
> + " .has_threads = %5s, threads = %ld,\n"
> + " .has_maxcpus = %5s, maxcpus = %ld,\n"
> + "}",
> + config->has_cpus ? "true" : "false", config->cpus,
> + config->has_sockets ? "true" : "false", config->sockets,
> + config->has_dies ? "true" : "false", config->dies,
> + config->has_cores ? "true" : "false", config->cores,
> + config->has_threads ? "true" : "false", config->threads,
> + config->has_maxcpus ? "true" : "false", config->maxcpus);
> +}
> +
> +static char *cpu_topology_to_string(CpuTopology *topo)
> +{
> + return g_strdup_printf(
> + "(CpuTopology) {\n"
> + " .cpus = %u,\n"
> + " .sockets = %u,\n"
> + " .dies = %u,\n"
> + " .cores = %u,\n"
> + " .threads = %u,\n"
> + " .max_cpus = %u,\n"
> + "}",
> + topo->cpus, topo->sockets, topo->dies,
> + topo->cores, topo->threads, topo->max_cpus);
> +}
> +
> +static void check_parse(MachineState *ms, SMPConfiguration *config,
> + CpuTopology *expect_topo, const char *expect_err,
> + bool is_valid)
> +{
> + g_autofree char *config_str = smp_config_to_string(config);
> + g_autofree char *expect_topo_str = cpu_topology_to_string(expect_topo);
> + g_autofree char *output_topo_str = NULL;
> + Error *err = NULL;
> +
> + /* call the generic parser smp_parse() */
> + smp_parse(ms, config, &err);
> +
> + output_topo_str = cpu_topology_to_string(&ms->smp);
> +
> + /* when the configuration is supposed to be valid */
> + if (is_valid) {
> + if ((err == NULL) &&
> + (ms->smp.cpus == expect_topo->cpus) &&
> + (ms->smp.sockets == expect_topo->sockets) &&
> + (ms->smp.dies == expect_topo->dies) &&
> + (ms->smp.cores == expect_topo->cores) &&
> + (ms->smp.threads == expect_topo->threads) &&
> + (ms->smp.max_cpus == expect_topo->max_cpus)) {
> + return;
> + }
> +
> + if (err != NULL) {
> + g_printerr("Test smp_parse failed!\n"
> + "Input configuration: %s\n"
> + "Should be valid: yes\n"
> + "Expected topology: %s\n\n"
> + "Result is valid: no\n"
> + "Result error report: %s\n",
> + config_str, expect_topo_str, error_get_pretty(err));
> + goto end;
> + }
> +
> + g_printerr("Test smp_parse failed!\n"
> + "Input configuration: %s\n"
> + "Should be valid: yes\n"
> + "Expected topology: %s\n\n"
> + "Result is valid: yes\n"
> + "Output topology: %s\n",
> + config_str, expect_topo_str, output_topo_str);
> + goto end;
> + }
> +
> + /* when the configuration is supposed to be invalid */
> + if (err != NULL) {
> + if (expect_err == NULL ||
> + g_str_equal(expect_err, error_get_pretty(err))) {
> + error_free(err);
> + return;
> + }
> +
> + g_printerr("Test smp_parse failed!\n"
> + "Input configuration: %s\n"
> + "Should be valid: no\n"
> + "Expected error report: %s\n\n"
> + "Result is valid: no\n"
> + "Output error report: %s\n",
> + config_str, expect_err, error_get_pretty(err));
> + goto end;
> + }
> +
> + g_printerr("Test smp_parse failed!\n"
> + "Input configuration: %s\n"
> + "Should be valid: no\n"
> + "Expected error report: %s\n\n"
> + "Result is valid: yes\n"
> + "Output topology: %s\n",
> + config_str, expect_err, output_topo_str);
> +
> +end:
> + if (err != NULL) {
> + error_free(err);
> + }
> +
> + abort();
> +}
> +
> +static void smp_parse_test(MachineState *ms, SMPTestData *data, bool is_valid)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> +
> + mc->smp_props.prefer_sockets = true;
> + check_parse(ms, &data->config, &data->expect_prefer_sockets,
> + data->expect_error, is_valid);
> +
> + mc->smp_props.prefer_sockets = false;
> + check_parse(ms, &data->config, &data->expect_prefer_cores,
> + data->expect_error, is_valid);
> +}
> +
> +static void test_data_copy(SMPTestData *targ, SMPTestData *src)
> +{
> + targ->config = src->config;
> + targ->expect_prefer_sockets = src->expect_prefer_sockets;
> + targ->expect_prefer_cores = src->expect_prefer_cores;
> + targ->expect_error = src->expect_error;
Why do we need this function? targ and src are of the same type. We can
just use '='.
> +}
> +
> +/* The parsed results of the unsupported parameters should be 1 */
> +static void unsupported_params_init(MachineClass *mc, SMPTestData *data)
> +{
> + if (!mc->smp_props.dies_supported) {
> + data->expect_prefer_sockets.dies = 1;
> + data->expect_prefer_cores.dies = 1;
> + }
> +}
> +
> +/* Reset the related machine properties before each sub-test */
> +static void smp_machine_class_init(MachineClass *mc)
> +{
> + mc->min_cpus = MIN_CPUS;
> + mc->max_cpus = MAX_CPUS;
> +
> + mc->smp_props.prefer_sockets = true;
> + mc->smp_props.dies_supported = false;
> +}
> +
> +static void test_generic(void)
> +{
> + Object *obj = object_new(TYPE_MACHINE);
> + MachineState *ms = MACHINE(obj);
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + SMPTestData *data = &(SMPTestData){0};
> + int i;
> +
> + smp_machine_class_init(mc);
> +
> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
> + test_data_copy(data, &data_generic_valid[i]);
> + unsupported_params_init(mc, data);
> +
> + smp_parse_test(ms, data, true);
> +
> + /* Unsupported parameters can be provided with their values as 1 */
> + data->config.has_dies = true;
> + data->config.dies = 1;
> + smp_parse_test(ms, data, true);
> + }
> +
> + /* Reset the supported min CPUs and max CPUs */
> + mc->min_cpus = 2;
> + mc->max_cpus = 511;
> +
> + for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
> + test_data_copy(data, &data_generic_invalid[i]);
> + unsupported_params_init(mc, data);
> +
> + smp_parse_test(ms, data, false);
> + }
> +
> + object_unref(obj);
> +}
> +
> +static void test_with_dies(void)
> +{
> + Object *obj = object_new(TYPE_MACHINE);
> + MachineState *ms = MACHINE(obj);
> + MachineClass *mc = MACHINE_GET_CLASS(obj);
> + SMPTestData *data = &(SMPTestData){0};
> + unsigned int num_dies = 2;
> + int i;
> +
> + smp_machine_class_init(mc);
> + mc->smp_props.dies_supported = true;
> +
> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
> + test_data_copy(data, &data_generic_valid[i]);
> + unsupported_params_init(mc, data);
> +
> + /* when dies parameter is omitted, it will be set as 1 */
> + data->expect_prefer_sockets.dies = 1;
> + data->expect_prefer_cores.dies = 1;
> +
> + smp_parse_test(ms, data, true);
> +
> + /* when dies parameter is specified */
> + data->config.has_dies = true;
> + data->config.dies = num_dies;
> + if (data->config.has_cpus) {
> + data->config.cpus *= num_dies;
> + }
> + if (data->config.has_maxcpus) {
> + data->config.maxcpus *= num_dies;
> + }
> +
> + data->expect_prefer_sockets.dies = num_dies;
> + data->expect_prefer_sockets.cpus *= num_dies;
> + data->expect_prefer_sockets.max_cpus *= num_dies;
> + data->expect_prefer_cores.dies = num_dies;
> + data->expect_prefer_cores.cpus *= num_dies;
> + data->expect_prefer_cores.max_cpus *= num_dies;
> +
> + smp_parse_test(ms, data, true);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) {
> + test_data_copy(data, &data_with_dies_invalid[i]);
> + unsupported_params_init(mc, data);
> +
> + smp_parse_test(ms, data, false);
> + }
> +
> + object_unref(obj);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + module_call_init(MODULE_INIT_QOM);
> + type_register_static(&smp_machine_info);
> +
> + g_test_add_func("/test-smp-parse/generic", test_generic);
> + g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
> +
> + g_test_run();
> +
> + return 0;
> +}
> --
> 2.19.1
>
Otherwise,
Reviewed-by: Andrew Jones <drjones@redhat.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API
2021-10-11 7:54 ` wangyanan (Y)
@ 2021-10-12 14:36 ` Markus Armbruster
2021-10-13 1:37 ` wangyanan (Y)
0 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2021-10-12 14:36 UTC (permalink / raw)
To: wangyanan (Y)
Cc: Andrew Jones, Daniel P.Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, Paolo Bonzini,
wanghaibin.wang, Philippe Mathieu-Daudé
"wangyanan (Y)" <wangyanan55@huawei.com> writes:
> Hi Markus,
>
> On 2021/10/11 13:26, Markus Armbruster wrote:
>> Yanan Wang <wangyanan55@huawei.com> writes:
>>
>>> Functionally smp_parse() is only called once and in one place
>>> i.e. machine_set_smp, the possible second place where it'll be
>>> called should be some unit tests if any.
>>>
>>> Actually we are going to introduce an unit test for the parser.
>>> For necessary isolation of the tested code, split smp_parse out
>>> into a separate header as an inline API.
>> Why inline?
> The motivation of the splitting is to isolate the tested smp_parse
> from the other unrelated code in machine.c, so that we can solve
> the build dependency problem for the unit test.
>
> I once tried to split smp_parse out into a source file in [1] for the
> test, but it looks more concise and convenient to make it as an
> inline function in a header compared to [1]. Given that we only call
> it in one place, it may not be harmful to keep it an inline.
>
> Anyway, I not sure the method in this patch is most appropriate
> and compliant. If it's just wrong I can change back to [1]. :)
>
> [1]
> https://lore.kernel.org/qemu-devel/20210910073025.16480-16-wangyanan55@huawei.com/#t
I'd prefer to keep it in .c, but I'm not the maintainer.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] tests/unit: Add an unit test for smp parsing
2021-10-12 13:51 ` Andrew Jones
@ 2021-10-13 1:23 ` wangyanan (Y)
0 siblings, 0 replies; 9+ messages in thread
From: wangyanan (Y) @ 2021-10-13 1:23 UTC (permalink / raw)
To: Andrew Jones
Cc: Daniel P. Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, wanghaibin.wang,
Paolo Bonzini, Philippe Mathieu-Daudé
On 2021/10/12 21:51, Andrew Jones wrote:
> On Sun, Oct 10, 2021 at 06:39:54PM +0800, Yanan Wang wrote:
>> Now that we have a generic parser smp_parse(), let's add an unit
>> test for the code. All possible valid/invalid SMP configurations
>> that the user can specify are covered.
>>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>> MAINTAINERS | 1 +
>> tests/unit/meson.build | 1 +
>> tests/unit/test-smp-parse.c | 613 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 615 insertions(+)
>> create mode 100644 tests/unit/test-smp-parse.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index dc9091c1d7..b5a5b1469b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1632,6 +1632,7 @@ F: include/hw/core/cpu.h
>> F: include/hw/core/smp.h
>> F: include/hw/cpu/cluster.h
>> F: include/sysemu/numa.h
>> +F: tests/unit/test-smp-parse.c
>> T: git https://gitlab.com/ehabkost/qemu.git machine-next
>>
>> Xtensa Machines
>> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
>> index 7c297d7e5c..0382669fcf 100644
>> --- a/tests/unit/meson.build
>> +++ b/tests/unit/meson.build
>> @@ -45,6 +45,7 @@ tests = {
>> 'test-uuid': [],
>> 'ptimer-test': ['ptimer-test-stubs.c', meson.project_source_root() / 'hw/core/ptimer.c'],
>> 'test-qapi-util': [],
>> + 'test-smp-parse': [qom],
>> }
>>
>> if have_system or have_tools
>> diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
>> new file mode 100644
>> index 0000000000..7be258171e
>> --- /dev/null
>> +++ b/tests/unit/test-smp-parse.c
>> @@ -0,0 +1,613 @@
>> +/*
>> + * SMP parsing unit-tests
>> + *
>> + * Copyright (c) 2021 Huawei Technologies Co., Ltd
>> + *
>> + * Authors:
>> + * Yanan Wang <wangyanan55@huawei.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qom/object.h"
>> +#include "qemu/module.h"
>> +#include "qapi/error.h"
>> +
>> +#include "hw/boards.h"
>> +#include "hw/core/smp.h"
>> +
>> +#define T true
>> +#define F false
>> +
>> +#define MIN_CPUS 1 /* set the min CPUs supported by the machine as 1 */
>> +#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
>> +
>> +/*
>> + * Used to define the generic 3-level CPU topology hierarchy
>> + * -sockets/cores/threads
>> + */
>> +#define SMP_CONFIG_GENERIC(ha, a, hb, b, hc, c, hd, d, he, e) \
>> + { \
>> + .has_cpus = ha, .cpus = a, \
>> + .has_sockets = hb, .sockets = b, \
>> + .has_cores = hc, .cores = c, \
>> + .has_threads = hd, .threads = d, \
>> + .has_maxcpus = he, .maxcpus = e, \
>> + }
>> +
>> +#define CPU_TOPOLOGY_GENERIC(a, b, c, d, e) \
>> + { \
>> + .cpus = a, \
>> + .sockets = b, \
>> + .cores = c, \
>> + .threads = d, \
>> + .max_cpus = e, \
>> + }
>> +
>> +/*
>> + * Currently a 4-level topology hierarchy is supported on PC machines
>> + * -sockets/dies/cores/threads
>> + */
>> +#define SMP_CONFIG_WITH_DIES(ha, a, hb, b, hc, c, hd, d, he, e, hf, f) \
>> + { \
>> + .has_cpus = ha, .cpus = a, \
>> + .has_sockets = hb, .sockets = b, \
>> + .has_dies = hc, .dies = c, \
>> + .has_cores = hd, .cores = d, \
>> + .has_threads = he, .threads = e, \
>> + .has_maxcpus = hf, .maxcpus = f, \
>> + }
>> +
>> +#define CPU_TOPOLOGY_WITH_DIES(a, b, c, d, e, f) \
>> + { \
>> + .cpus = a, \
>> + .sockets = b, \
>> + .dies = c, \
>> + .cores = d, \
>> + .threads = e, \
>> + .max_cpus = f, \
>> + }
>> +
>> +/**
>> + * @config - the given SMP configuration
>> + * @expect_prefer_sockets - the expected parsing result for the
>> + * valid configuration, when sockets are preferred over cores
>> + * @expect_prefer_cores - the expected parsing result for the
>> + * valid configuration, when cores are preferred over sockets
>> + * @expect_error - the expected error report when the given
>> + * configuration is invalid
>> + */
>> +typedef struct SMPTestData {
>> + SMPConfiguration config;
>> + CpuTopology expect_prefer_sockets;
>> + CpuTopology expect_prefer_cores;
>> + const char *expect_error;
>> +} SMPTestData;
>> +
>> +/* Type info of the tested machine */
>> +static const TypeInfo smp_machine_info = {
>> + .name = TYPE_MACHINE,
>> + .parent = TYPE_OBJECT,
>> + .class_size = sizeof(MachineClass),
>> + .instance_size = sizeof(MachineState),
>> +};
>> +
>> +/*
>> + * List all the possible valid sub-collections of the generic 5
>> + * topology parameters (i.e. cpus/maxcpus/sockets/cores/threads),
>> + * then test the automatic calculation algorithm of the missing
>> + * values in the parser.
>> + */
>> +static struct SMPTestData data_generic_valid[] = {
>> + {
>> + /* config: no configuration provided
>> + * expect: cpus=1,sockets=1,cores=1,threads=1,maxcpus=1 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(1, 1, 1, 1, 1),
>> + }, {
>> + /* config: -smp 8
>> + * prefer_sockets: cpus=8,sockets=8,cores=1,threads=1,maxcpus=8
>> + * prefer_cores: cpus=8,sockets=1,cores=8,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 1, 8),
>> + }, {
>> + /* config: -smp sockets=2
>> + * expect: cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 2, 1, 1, 2),
>> + }, {
>> + /* config: -smp cores=4
>> + * expect: cpus=4,sockets=1,cores=4,threads=1,maxcpus=4 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 1, 4, 1, 4),
>> + }, {
>> + /* config: -smp threads=2
>> + * expect: cpus=2,sockets=1,cores=1,threads=2,maxcpus=2 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(2, 1, 1, 2, 2),
>> + }, {
>> + /* config: -smp maxcpus=16
>> + * prefer_sockets: cpus=16,sockets=16,cores=1,threads=1,maxcpus=16
>> + * prefer_cores: cpus=16,sockets=1,cores=16,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 16, 1, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 16, 1, 16),
>> + }, {
>> + /* config: -smp 8,sockets=2
>> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + }, {
>> + /* config: -smp 8,cores=4
>> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + }, {
>> + /* config: -smp 8,threads=2
>> + * prefer_sockets: cpus=8,sockets=4,cores=1,threads=2,maxcpus=8
>> + * prefer_cores: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 1, 2, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
>> + }, {
>> + /* config: -smp 8,maxcpus=16
>> + * prefer_sockets: cpus=8,sockets=16,cores=1,threads=1,maxcpus=16
>> + * prefer_cores: cpus=8,sockets=1,cores=16,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 16, 1, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 16, 1, 16),
>> + }, {
>> + /* config: -smp sockets=2,cores=4
>> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + }, {
>> + /* config: -smp sockets=2,threads=2
>> + * expect: cpus=4,sockets=2,cores=1,threads=2,maxcpus=4 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(4, 2, 1, 2, 4),
>> + }, {
>> + /* config: -smp sockets=2,maxcpus=16
>> + * expect: cpus=16,sockets=2,cores=8,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 8, 1, 16),
>> + }, {
>> + /* config: -smp cores=4,threads=2
>> + * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
>> + }, {
>> + /* config: -smp cores=4,maxcpus=16
>> + * expect: cpus=16,sockets=4,cores=4,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 4, 4, 1, 16),
>> + }, {
>> + /* config: -smp threads=2,maxcpus=16
>> + * prefer_sockets: cpus=16,sockets=8,cores=1,threads=2,maxcpus=16
>> + * prefer_cores: cpus=16,sockets=1,cores=8,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, F, 0, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 8, 1, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 1, 8, 2, 16),
>> + }, {
>> + /* config: -smp 8,sockets=2,cores=4
>> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + }, {
>> + /* config: -smp 8,sockets=2,threads=2
>> + * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
>> + }, {
>> + /* config: -smp 8,sockets=2,maxcpus=16
>> + * expect: cpus=8,sockets=2,cores=8,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 8, 1, 16),
>> + }, {
>> + /* config: -smp 8,cores=4,threads=2
>> + * expect: cpus=8,sockets=1,cores=4,threads=2,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 4, 2, 8),
>> + }, {
>> + /* config: -smp 8,cores=4,maxcpus=16
>> + * expect: cpus=8,sockets=4,cores=4,threads=1,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 4, 4, 1, 16),
>> + }, {
>> + /* config: -smp 8,threads=2,maxcpus=16
>> + * prefer_sockets: cpus=8,sockets=8,cores=1,threads=2,maxcpus=16
>> + * prefer_cores: cpus=8,sockets=1,cores=8,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 8, 1, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 1, 8, 2, 16),
>> + }, {
>> + /* config: -smp sockets=2,cores=4,threads=2
>> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp sockets=2,cores=4,maxcpus=16
>> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp sockets=2,threads=2,maxcpus=16
>> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, F, 0, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp cores=4,threads=2,maxcpus=16
>> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, F, 0, T, 4, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp 8,sockets=2,cores=4,threads=1
>> + * expect: cpus=8,sockets=2,cores=4,threads=1,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 1, F, 0),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 1, 8),
>> + }, {
>> + /* config: -smp 8,sockets=2,cores=4,maxcpus=16
>> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, F, 0, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp 8,sockets=2,threads=2,maxcpus=16
>> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, F, 0, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp 8,cores=4,threads=2,maxcpus=16
>> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 4, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp sockets=2,cores=4,threads=2,maxcpus=16
>> + * expect: cpus=16,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(F, 0, T, 2, T, 4, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(16, 2, 4, 2, 16),
>> + }, {
>> + /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
>> + * expect: cpus=8,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 16),
>> + .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + .expect_prefer_cores = CPU_TOPOLOGY_GENERIC(8, 2, 4, 2, 16),
>> + },
>> +};
>> +
>> +static struct SMPTestData data_generic_invalid[] = {
>> + {
>> + /* config: -smp 2,dies=2 */
>> + .config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
>> + .expect_error = "dies not supported by this machine's CPU topology",
>> + }, {
>> + /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
>> + .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
>> + .expect_error = "Invalid CPU topology: "
>> + "product of the hierarchy must match maxcpus: "
>> + "sockets (2) * cores (4) * threads (2) "
>> + "!= maxcpus (8)",
>> + }, {
>> + /* config: -smp 18,sockets=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_GENERIC(T, 18, T, 2, T, 4, T, 2, T, 16),
>> + .expect_error = "Invalid CPU topology: "
>> + "maxcpus must be equal to or greater than smp: "
>> + "sockets (2) * cores (4) * threads (2) "
>> + "== maxcpus (16) < smp_cpus (18)",
>> + }, {
>> + /* config: -smp 1
>> + * should tweak the supported min CPUs to 2 for testing */
>> + .config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
>> + .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
>> + "by machine '(null)' is 2",
>> + }, {
>> + /* config: -smp 512
>> + * should tweak the supported max CPUs to 511 for testing */
>> + .config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
>> + .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
>> + "by machine '(null)' is 511",
>> + },
>> +};
>> +
>> +static struct SMPTestData data_with_dies_invalid[] = {
>> + {
>> + /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
>> + .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 16),
>> + .expect_error = "Invalid CPU topology: "
>> + "product of the hierarchy must match maxcpus: "
>> + "sockets (2) * dies (2) * cores (4) * threads (2) "
>> + "!= maxcpus (16)",
>> + }, {
>> + /* config: -smp 34,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
>> + .config = SMP_CONFIG_WITH_DIES(T, 34, T, 2, T, 2, T, 4, T, 2, T, 32),
>> + .expect_error = "Invalid CPU topology: "
>> + "maxcpus must be equal to or greater than smp: "
>> + "sockets (2) * dies (2) * cores (4) * threads (2) "
>> + "== maxcpus (32) < smp_cpus (34)",
>> + },
>> +};
>> +
>> +static char *smp_config_to_string(SMPConfiguration *config)
>> +{
>> + return g_strdup_printf(
>> + "(SMPConfiguration) {\n"
>> + " .has_cpus = %5s, cpus = %ld,\n"
>> + " .has_sockets = %5s, sockets = %ld,\n"
>> + " .has_dies = %5s, dies = %ld,\n"
>> + " .has_cores = %5s, cores = %ld,\n"
>> + " .has_threads = %5s, threads = %ld,\n"
>> + " .has_maxcpus = %5s, maxcpus = %ld,\n"
>> + "}",
>> + config->has_cpus ? "true" : "false", config->cpus,
>> + config->has_sockets ? "true" : "false", config->sockets,
>> + config->has_dies ? "true" : "false", config->dies,
>> + config->has_cores ? "true" : "false", config->cores,
>> + config->has_threads ? "true" : "false", config->threads,
>> + config->has_maxcpus ? "true" : "false", config->maxcpus);
>> +}
>> +
>> +static char *cpu_topology_to_string(CpuTopology *topo)
>> +{
>> + return g_strdup_printf(
>> + "(CpuTopology) {\n"
>> + " .cpus = %u,\n"
>> + " .sockets = %u,\n"
>> + " .dies = %u,\n"
>> + " .cores = %u,\n"
>> + " .threads = %u,\n"
>> + " .max_cpus = %u,\n"
>> + "}",
>> + topo->cpus, topo->sockets, topo->dies,
>> + topo->cores, topo->threads, topo->max_cpus);
>> +}
>> +
>> +static void check_parse(MachineState *ms, SMPConfiguration *config,
>> + CpuTopology *expect_topo, const char *expect_err,
>> + bool is_valid)
>> +{
>> + g_autofree char *config_str = smp_config_to_string(config);
>> + g_autofree char *expect_topo_str = cpu_topology_to_string(expect_topo);
>> + g_autofree char *output_topo_str = NULL;
>> + Error *err = NULL;
>> +
>> + /* call the generic parser smp_parse() */
>> + smp_parse(ms, config, &err);
>> +
>> + output_topo_str = cpu_topology_to_string(&ms->smp);
>> +
>> + /* when the configuration is supposed to be valid */
>> + if (is_valid) {
>> + if ((err == NULL) &&
>> + (ms->smp.cpus == expect_topo->cpus) &&
>> + (ms->smp.sockets == expect_topo->sockets) &&
>> + (ms->smp.dies == expect_topo->dies) &&
>> + (ms->smp.cores == expect_topo->cores) &&
>> + (ms->smp.threads == expect_topo->threads) &&
>> + (ms->smp.max_cpus == expect_topo->max_cpus)) {
>> + return;
>> + }
>> +
>> + if (err != NULL) {
>> + g_printerr("Test smp_parse failed!\n"
>> + "Input configuration: %s\n"
>> + "Should be valid: yes\n"
>> + "Expected topology: %s\n\n"
>> + "Result is valid: no\n"
>> + "Result error report: %s\n",
>> + config_str, expect_topo_str, error_get_pretty(err));
>> + goto end;
>> + }
>> +
>> + g_printerr("Test smp_parse failed!\n"
>> + "Input configuration: %s\n"
>> + "Should be valid: yes\n"
>> + "Expected topology: %s\n\n"
>> + "Result is valid: yes\n"
>> + "Output topology: %s\n",
>> + config_str, expect_topo_str, output_topo_str);
>> + goto end;
>> + }
>> +
>> + /* when the configuration is supposed to be invalid */
>> + if (err != NULL) {
>> + if (expect_err == NULL ||
>> + g_str_equal(expect_err, error_get_pretty(err))) {
>> + error_free(err);
>> + return;
>> + }
>> +
>> + g_printerr("Test smp_parse failed!\n"
>> + "Input configuration: %s\n"
>> + "Should be valid: no\n"
>> + "Expected error report: %s\n\n"
>> + "Result is valid: no\n"
>> + "Output error report: %s\n",
>> + config_str, expect_err, error_get_pretty(err));
>> + goto end;
>> + }
>> +
>> + g_printerr("Test smp_parse failed!\n"
>> + "Input configuration: %s\n"
>> + "Should be valid: no\n"
>> + "Expected error report: %s\n\n"
>> + "Result is valid: yes\n"
>> + "Output topology: %s\n",
>> + config_str, expect_err, output_topo_str);
>> +
>> +end:
>> + if (err != NULL) {
>> + error_free(err);
>> + }
>> +
>> + abort();
>> +}
>> +
>> +static void smp_parse_test(MachineState *ms, SMPTestData *data, bool is_valid)
>> +{
>> + MachineClass *mc = MACHINE_GET_CLASS(ms);
>> +
>> + mc->smp_props.prefer_sockets = true;
>> + check_parse(ms, &data->config, &data->expect_prefer_sockets,
>> + data->expect_error, is_valid);
>> +
>> + mc->smp_props.prefer_sockets = false;
>> + check_parse(ms, &data->config, &data->expect_prefer_cores,
>> + data->expect_error, is_valid);
>> +}
>> +
>> +static void test_data_copy(SMPTestData *targ, SMPTestData *src)
>> +{
>> + targ->config = src->config;
>> + targ->expect_prefer_sockets = src->expect_prefer_sockets;
>> + targ->expect_prefer_cores = src->expect_prefer_cores;
>> + targ->expect_error = src->expect_error;
> Why do we need this function? targ and src are of the same type. We can
> just use '='.
Ah, yes, you are right.
Currently we don't necessarily need this function at all.
I forgot to drop this one. I will drop it in v2.
>> +}
>> +
>> +/* The parsed results of the unsupported parameters should be 1 */
>> +static void unsupported_params_init(MachineClass *mc, SMPTestData *data)
>> +{
>> + if (!mc->smp_props.dies_supported) {
>> + data->expect_prefer_sockets.dies = 1;
>> + data->expect_prefer_cores.dies = 1;
>> + }
>> +}
>> +
>> +/* Reset the related machine properties before each sub-test */
>> +static void smp_machine_class_init(MachineClass *mc)
>> +{
>> + mc->min_cpus = MIN_CPUS;
>> + mc->max_cpus = MAX_CPUS;
>> +
>> + mc->smp_props.prefer_sockets = true;
>> + mc->smp_props.dies_supported = false;
>> +}
>> +
>> +static void test_generic(void)
>> +{
>> + Object *obj = object_new(TYPE_MACHINE);
>> + MachineState *ms = MACHINE(obj);
>> + MachineClass *mc = MACHINE_GET_CLASS(obj);
>> + SMPTestData *data = &(SMPTestData){0};
>> + int i;
>> +
>> + smp_machine_class_init(mc);
>> +
>> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
>> + test_data_copy(data, &data_generic_valid[i]);
>> + unsupported_params_init(mc, data);
>> +
>> + smp_parse_test(ms, data, true);
>> +
>> + /* Unsupported parameters can be provided with their values as 1 */
>> + data->config.has_dies = true;
>> + data->config.dies = 1;
>> + smp_parse_test(ms, data, true);
>> + }
>> +
>> + /* Reset the supported min CPUs and max CPUs */
>> + mc->min_cpus = 2;
>> + mc->max_cpus = 511;
>> +
>> + for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
>> + test_data_copy(data, &data_generic_invalid[i]);
>> + unsupported_params_init(mc, data);
>> +
>> + smp_parse_test(ms, data, false);
>> + }
>> +
>> + object_unref(obj);
>> +}
>> +
>> +static void test_with_dies(void)
>> +{
>> + Object *obj = object_new(TYPE_MACHINE);
>> + MachineState *ms = MACHINE(obj);
>> + MachineClass *mc = MACHINE_GET_CLASS(obj);
>> + SMPTestData *data = &(SMPTestData){0};
>> + unsigned int num_dies = 2;
>> + int i;
>> +
>> + smp_machine_class_init(mc);
>> + mc->smp_props.dies_supported = true;
>> +
>> + for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
>> + test_data_copy(data, &data_generic_valid[i]);
>> + unsupported_params_init(mc, data);
>> +
>> + /* when dies parameter is omitted, it will be set as 1 */
>> + data->expect_prefer_sockets.dies = 1;
>> + data->expect_prefer_cores.dies = 1;
>> +
>> + smp_parse_test(ms, data, true);
>> +
>> + /* when dies parameter is specified */
>> + data->config.has_dies = true;
>> + data->config.dies = num_dies;
>> + if (data->config.has_cpus) {
>> + data->config.cpus *= num_dies;
>> + }
>> + if (data->config.has_maxcpus) {
>> + data->config.maxcpus *= num_dies;
>> + }
>> +
>> + data->expect_prefer_sockets.dies = num_dies;
>> + data->expect_prefer_sockets.cpus *= num_dies;
>> + data->expect_prefer_sockets.max_cpus *= num_dies;
>> + data->expect_prefer_cores.dies = num_dies;
>> + data->expect_prefer_cores.cpus *= num_dies;
>> + data->expect_prefer_cores.max_cpus *= num_dies;
>> +
>> + smp_parse_test(ms, data, true);
>> + }
>> +
>> + for (i = 0; i < ARRAY_SIZE(data_with_dies_invalid); i++) {
>> + test_data_copy(data, &data_with_dies_invalid[i]);
>> + unsupported_params_init(mc, data);
>> +
>> + smp_parse_test(ms, data, false);
>> + }
>> +
>> + object_unref(obj);
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + g_test_init(&argc, &argv, NULL);
>> +
>> + module_call_init(MODULE_INIT_QOM);
>> + type_register_static(&smp_machine_info);
>> +
>> + g_test_add_func("/test-smp-parse/generic", test_generic);
>> + g_test_add_func("/test-smp-parse/with_dies", test_with_dies);
>> +
>> + g_test_run();
>> +
>> + return 0;
>> +}
>> --
>> 2.19.1
>>
> Otherwise,
>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> .
Thank you very much for having a second look at this.
Thanks,
Yanan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API
2021-10-12 14:36 ` Markus Armbruster
@ 2021-10-13 1:37 ` wangyanan (Y)
0 siblings, 0 replies; 9+ messages in thread
From: wangyanan (Y) @ 2021-10-13 1:37 UTC (permalink / raw)
To: Markus Armbruster, Paolo Bonzini
Cc: Andrew Jones, Daniel P.Berrangé,
Eduardo Habkost, Pierre Morel, qemu-devel, wanghaibin.wang,
Philippe Mathieu-Daudé
On 2021/10/12 22:36, Markus Armbruster wrote:
> "wangyanan (Y)" <wangyanan55@huawei.com> writes:
>
>> Hi Markus,
>>
>> On 2021/10/11 13:26, Markus Armbruster wrote:
>>> Yanan Wang <wangyanan55@huawei.com> writes:
>>>
>>>> Functionally smp_parse() is only called once and in one place
>>>> i.e. machine_set_smp, the possible second place where it'll be
>>>> called should be some unit tests if any.
>>>>
>>>> Actually we are going to introduce an unit test for the parser.
>>>> For necessary isolation of the tested code, split smp_parse out
>>>> into a separate header as an inline API.
>>> Why inline?
>> The motivation of the splitting is to isolate the tested smp_parse
>> from the other unrelated code in machine.c, so that we can solve
>> the build dependency problem for the unit test.
>>
>> I once tried to split smp_parse out into a source file in [1] for the
>> test, but it looks more concise and convenient to make it as an
>> inline function in a header compared to [1]. Given that we only call
>> it in one place, it may not be harmful to keep it an inline.
>>
>> Anyway, I not sure the method in this patch is most appropriate
>> and compliant. If it's just wrong I can change back to [1]. :)
>>
>> [1]
>> https://lore.kernel.org/qemu-devel/20210910073025.16480-16-wangyanan55@huawei.com/#t
> I'd prefer to keep it in .c, but I'm not the maintainer.
>
Ok, I will move it into a separate .c file in v2, which seems to meet
the standards more.
Thanks,
Yanan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-13 1:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-10 10:39 [PATCH 0/2] hw/core/machine: An an unit test for smp_parse Yanan Wang
2021-10-10 10:39 ` [PATCH 1/2] hw/core/machine: Split out smp_parse as an inline API Yanan Wang
2021-10-11 5:26 ` Markus Armbruster
2021-10-11 7:54 ` wangyanan (Y)
2021-10-12 14:36 ` Markus Armbruster
2021-10-13 1:37 ` wangyanan (Y)
2021-10-10 10:39 ` [PATCH 2/2] tests/unit: Add an unit test for smp parsing Yanan Wang
2021-10-12 13:51 ` Andrew Jones
2021-10-13 1:23 ` wangyanan (Y)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.