* [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement
@ 2021-08-13 2:38 Yanan Wang
2021-08-13 2:38 ` [PATCH for-6.2 v5 01/14] machine: Minor refactor/cleanup for the smp parsers Yanan Wang
` (13 more replies)
0 siblings, 14 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:38 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Hi,
This is new version (v5 with a little update in patch #6) of the series [1]
that I have posted to introduce some fixes and improvement for SMP parsing.
[1] https://lore.kernel.org/qemu-devel/20210803080527.156556-1-wangyanan55@huawei.com/
Most of this series is about the SMP parsers:
maxcpus is now uniformly used to calculate the omitted topology members,
calculation of omitted maxcpus/cpus is improved, the error reporting is
improved. It's also suggested that we should start to prefer cores over
sockets over threads on the newer machine types, which will make the
computed virtual topology more reflective of the real hardware.
In order to reduce code duplication and ease the code maintenance, smp_parse
in now converted into a parser generic enough for all arches, so that the PC
specific one can be removed. It's also convenient to introduce more topology
members to the generic parser in the future.
Finally, a QEMU unit test for the parsing of given SMP configuration is added.
Since all the parsing logic is in generic function smp_parse(), this test
passes different SMP configurations to the function and compare the parsing
result with what is expected. In the test, all possible collections of the
topology parameters and the corresponding expected results are listed,
including the valid and invalid ones. The preference of sockets over cores
and the preference of cores over sockets, and the support of multi-dies are
also taken into consideration.
---
Changelogs:
v4->v5:
- refactor out the duplicated "threads == 0" case in patch #6 (Pankaj)
- pick up more R-b tags from v4 (thanks very much for the review!)
- v4: https://lore.kernel.org/qemu-devel/20210803080527.156556-1-wangyanan55@huawei.com/
v3->v4:
- put all the sanity check into the parser
- refine the unit test and add it back to the series
- add the R-b/A-b tags for the reviewed/acked patches
- v3: https://lore.kernel.org/qemu-devel/20210728034848.75228-1-wangyanan55@huawei.com/
v2->v3:
- apply the calculation improvement to smp_parse and pc_smp_parse
separately and then convert the finally improved parsers into a
generic one, so that patches can be reviewed separately.
- to ease review, drop the unit test part for a while until we have
a good enough generic parser.
- send the patch "machine: Disallow specifying topology parameters as zero"
for 6.1 separately.
- v2: https://lore.kernel.org/qemu-devel/20210719032043.25416-1-wangyanan55@huawei.com/
v1->v2:
- disallow "anything=0" in the smp configuration (Andrew)
- make function smp_parse() a generic helper for all arches
- improve the error reporting in the parser
- start to prefer cores over sockets since 6.2 (Daniel)
- add a unit test for the smp parsing (Daniel)
- v1: https://lore.kernel.org/qemu-devel/20210702100739.13672-1-wangyanan55@huawei.com/
---
Yanan Wang (14):
machine: Minor refactor/cleanup for the smp parsers
machine: Uniformly use maxcpus to calculate the omitted parameters
machine: Set the value of cpus to match maxcpus if it's omitted
machine: Improve the error reporting of smp parsing
hw: Add compat machines for 6.2
machine: Prefer cores over sockets in smp parsing since 6.2
machine: Use ms instead of global current_machine in sanity-check
machine: Tweak the order of topology members in struct CpuTopology
machine: Make smp_parse generic enough for all arches
machine: Remove smp_parse callback from MachineClass
machine: Move smp_prefer_sockets to struct SMPCompatProps
machine: Put all sanity-check in the generic SMP parser
machine: Split out the smp parsing code
tests/unit: Add a unit test for smp parsing
MAINTAINERS | 2 +
hw/arm/virt.c | 10 +-
hw/core/machine-smp.c | 200 ++++++++
hw/core/machine.c | 106 +----
hw/core/meson.build | 1 +
hw/i386/pc.c | 66 +--
hw/i386/pc_piix.c | 15 +-
hw/i386/pc_q35.c | 14 +-
hw/ppc/spapr.c | 16 +-
hw/s390x/s390-virtio-ccw.c | 15 +-
include/hw/boards.h | 27 +-
include/hw/i386/pc.h | 3 +
qemu-options.hx | 14 +-
tests/unit/meson.build | 1 +
tests/unit/test-smp-parse.c | 892 ++++++++++++++++++++++++++++++++++++
15 files changed, 1205 insertions(+), 177 deletions(-)
create mode 100644 hw/core/machine-smp.c
create mode 100644 tests/unit/test-smp-parse.c
--
2.19.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 01/14] machine: Minor refactor/cleanup for the smp parsers
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
@ 2021-08-13 2:38 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 02/14] machine: Uniformly use maxcpus to calculate the omitted parameters Yanan Wang
` (12 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:38 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
To pave the way for the functional improvement in later patches,
make some refactor/cleanup for the smp parsers, including using
local maxcpus instead of ms->smp.max_cpus in the calculation,
defaulting dies to 0 initially like other members, cleanup the
sanity check for dies.
No functional change intended.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/core/machine.c | 19 +++++++++++--------
hw/i386/pc.c | 23 ++++++++++++++---------
2 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 06148fc225..5ad646b3f0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -749,9 +749,11 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
unsigned sockets = config->has_sockets ? config->sockets : 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;
- if (config->has_dies && config->dies != 0 && config->dies != 1) {
+ if (config->has_dies && config->dies > 1) {
error_setg(errp, "dies not supported by this machine's CPU topology");
+ return;
}
/* compute missing values, prefer sockets over cores over threads */
@@ -762,8 +764,8 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
sockets = sockets > 0 ? sockets : 1;
cpus = cores * threads * sockets;
} else {
- ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
- sockets = ms->smp.max_cpus / (cores * threads);
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+ sockets = maxcpus / (cores * threads);
}
} else if (cores == 0) {
threads = threads > 0 ? threads : 1;
@@ -780,26 +782,27 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
return;
}
- ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
- if (ms->smp.max_cpus < cpus) {
+ if (maxcpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
return;
}
- if (sockets * cores * threads != ms->smp.max_cpus) {
+ if (sockets * cores * threads != maxcpus) {
error_setg(errp, "Invalid CPU topology: "
"sockets (%u) * cores (%u) * threads (%u) "
"!= maxcpus (%u)",
sockets, cores, threads,
- ms->smp.max_cpus);
+ maxcpus);
return;
}
ms->smp.cpus = cpus;
+ ms->smp.sockets = sockets;
ms->smp.cores = cores;
ms->smp.threads = threads;
- ms->smp.sockets = sockets;
+ ms->smp.max_cpus = maxcpus;
}
static void machine_get_smp(Object *obj, Visitor *v, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c2b9d62a35..acd31af452 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -716,9 +716,13 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
{
unsigned cpus = config->has_cpus ? config->cpus : 0;
unsigned sockets = config->has_sockets ? config->sockets : 0;
- unsigned dies = config->has_dies ? config->dies : 1;
+ unsigned 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;
+
+ /* directly default dies to 1 if it's omitted */
+ dies = dies > 0 ? dies : 1;
/* compute missing values, prefer sockets over cores over threads */
if (cpus == 0 || sockets == 0) {
@@ -728,8 +732,8 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
sockets = sockets > 0 ? sockets : 1;
cpus = cores * threads * dies * sockets;
} else {
- ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
- sockets = ms->smp.max_cpus / (cores * threads * dies);
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+ sockets = maxcpus / (dies * cores * threads);
}
} else if (cores == 0) {
threads = threads > 0 ? threads : 1;
@@ -746,27 +750,28 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
return;
}
- ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
- if (ms->smp.max_cpus < cpus) {
+ if (maxcpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
return;
}
- if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+ if (sockets * dies * cores * threads != maxcpus) {
error_setg(errp, "Invalid CPU topology deprecated: "
"sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
"!= maxcpus (%u)",
sockets, dies, cores, threads,
- ms->smp.max_cpus);
+ maxcpus);
return;
}
ms->smp.cpus = cpus;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
ms->smp.sockets = sockets;
ms->smp.dies = dies;
+ ms->smp.cores = cores;
+ ms->smp.threads = threads;
+ ms->smp.maxcpus = maxcpus;
}
static
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 02/14] machine: Uniformly use maxcpus to calculate the omitted parameters
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
2021-08-13 2:38 ` [PATCH for-6.2 v5 01/14] machine: Minor refactor/cleanup for the smp parsers Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 03/14] machine: Set the value of cpus to match maxcpus if it's omitted Yanan Wang
` (11 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
We are currently using maxcpus to calculate the omitted sockets
but using cpus to calculate the omitted cores/threads. This makes
cmdlines like:
-smp cpus=8,maxcpus=16
-smp cpus=8,cores=4,maxcpus=16
-smp cpus=8,threads=2,maxcpus=16
work fine but the ones like:
-smp cpus=8,sockets=2,maxcpus=16
-smp cpus=8,sockets=2,cores=4,maxcpus=16
-smp cpus=8,sockets=2,threads=2,maxcpus=16
break the sanity check.
Since we require for a valid config that the product of "sockets * cores
* threads" should equal to the maxcpus, we should uniformly use maxcpus
to calculate their omitted values.
Also the if-branch of "cpus == 0 || sockets == 0" was split into two
branches of "cpus == 0" and "sockets == 0" so that we can clearly read
that we are parsing the configuration with a preference on cpus over
sockets over cores over threads.
Note: change in this patch won't affect any existing working cmdlines
but improves consistency and allows more incomplete configs to be valid.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
---
hw/core/machine.c | 30 +++++++++++++++---------------
hw/i386/pc.c | 30 +++++++++++++++---------------
2 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5ad646b3f0..ff10347ce8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -757,24 +757,26 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
}
/* compute missing values, prefer sockets over cores over threads */
- if (cpus == 0 || sockets == 0) {
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+ if (cpus == 0) {
+ sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
- if (cpus == 0) {
- sockets = sockets > 0 ? sockets : 1;
- cpus = cores * threads * sockets;
- } else {
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
- sockets = maxcpus / (cores * threads);
- }
+ cpus = sockets * cores * threads;
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+ } else if (sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (cores * threads);
} else if (cores == 0) {
threads = threads > 0 ? threads : 1;
- cores = cpus / (sockets * threads);
- cores = cores > 0 ? cores : 1;
+ cores = maxcpus / (sockets * threads);
} else if (threads == 0) {
- threads = cpus / (cores * sockets);
- threads = threads > 0 ? threads : 1;
- } else if (sockets * cores * threads < cpus) {
+ threads = maxcpus / (sockets * cores);
+ }
+
+ if (sockets * cores * threads < cpus) {
error_setg(errp, "cpu topology: "
"sockets (%u) * cores (%u) * threads (%u) < "
"smp_cpus (%u)",
@@ -782,8 +784,6 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
return;
}
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
if (maxcpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
return;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index acd31af452..a9ff9ef52c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -725,24 +725,26 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
dies = dies > 0 ? dies : 1;
/* compute missing values, prefer sockets over cores over threads */
- if (cpus == 0 || sockets == 0) {
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+
+ if (cpus == 0) {
+ sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
- if (cpus == 0) {
- sockets = sockets > 0 ? sockets : 1;
- cpus = cores * threads * dies * sockets;
- } else {
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
- sockets = maxcpus / (dies * cores * threads);
- }
+ cpus = sockets * dies * cores * threads;
+ maxcpus = maxcpus > 0 ? maxcpus : cpus;
+ } else 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 = cpus / (sockets * dies * threads);
- cores = cores > 0 ? cores : 1;
+ cores = maxcpus / (sockets * dies * threads);
} else if (threads == 0) {
- threads = cpus / (cores * dies * sockets);
- threads = threads > 0 ? threads : 1;
- } else if (sockets * dies * cores * threads < cpus) {
+ threads = maxcpus / (sockets * dies * cores);
+ }
+
+ if (sockets * dies * cores * threads < cpus) {
error_setg(errp, "cpu topology: "
"sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
"smp_cpus (%u)",
@@ -750,8 +752,6 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
return;
}
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
if (maxcpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
return;
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 03/14] machine: Set the value of cpus to match maxcpus if it's omitted
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
2021-08-13 2:38 ` [PATCH for-6.2 v5 01/14] machine: Minor refactor/cleanup for the smp parsers Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 02/14] machine: Uniformly use maxcpus to calculate the omitted parameters Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 04/14] machine: Improve the error reporting of smp parsing Yanan Wang
` (10 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Currently we directly calculate the omitted cpus based on the given
incomplete collection of parameters. This makes some cmdlines like:
-smp maxcpus=16
-smp sockets=2,maxcpus=16
-smp sockets=2,dies=2,maxcpus=16
-smp sockets=2,cores=4,maxcpus=16
not work. We should probably set the value of cpus to match maxcpus
if it's omitted, which will make above configs start to work.
So the calculation logic of cpus/maxcpus after this patch will be:
When both maxcpus and cpus are omitted, maxcpus will be calculated
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 counterpart's value. Both maxcpus and cpus may
be specified, but maxcpus must be equal to or greater than cpus.
Note: change in this patch won't affect any existing working cmdlines
but allows more incomplete configs to be valid.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/core/machine.c | 29 ++++++++++++++++-------------
hw/i386/pc.c | 29 ++++++++++++++++-------------
qemu-options.hx | 11 ++++++++---
3 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ff10347ce8..1a6c6cfd0e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -757,25 +757,28 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
}
/* compute missing values, prefer sockets over cores over threads */
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
- if (cpus == 0) {
+ if (cpus == 0 && maxcpus == 0) {
sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
- cpus = sockets * cores * threads;
+ } else {
maxcpus = maxcpus > 0 ? maxcpus : cpus;
- } else if (sockets == 0) {
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (cores * threads);
- } else if (cores == 0) {
- threads = threads > 0 ? threads : 1;
- cores = maxcpus / (sockets * threads);
- } else if (threads == 0) {
- threads = maxcpus / (sockets * cores);
+
+ if (sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (cores * threads);
+ } else if (cores == 0) {
+ threads = threads > 0 ? threads : 1;
+ cores = maxcpus / (sockets * threads);
+ } else if (threads == 0) {
+ threads = maxcpus / (sockets * cores);
+ }
}
+ maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
+ cpus = cpus > 0 ? cpus : maxcpus;
+
if (sockets * cores * threads < cpus) {
error_setg(errp, "cpu topology: "
"sockets (%u) * cores (%u) * threads (%u) < "
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a9ff9ef52c..9ad7ae5254 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -725,25 +725,28 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
dies = dies > 0 ? dies : 1;
/* compute missing values, prefer sockets over cores over threads */
- maxcpus = maxcpus > 0 ? maxcpus : cpus;
-
- if (cpus == 0) {
+ if (cpus == 0 && maxcpus == 0) {
sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
- cpus = sockets * dies * cores * threads;
+ } else {
maxcpus = maxcpus > 0 ? maxcpus : cpus;
- } else 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 if (threads == 0) {
- threads = maxcpus / (sockets * dies * cores);
+
+ 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 if (threads == 0) {
+ threads = maxcpus / (sockets * dies * cores);
+ }
}
+ maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
+ cpus = cpus > 0 ? cpus : maxcpus;
+
if (sockets * dies * cores * threads < cpus) {
error_setg(errp, "cpu topology: "
"sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
diff --git a/qemu-options.hx b/qemu-options.hx
index aee622f577..06f819177e 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -214,9 +214,14 @@ SRST
Simulate a SMP system with '\ ``n``\ ' CPUs initially present on
the machine type board. On boards supporting CPU hotplug, the optional
'\ ``maxcpus``\ ' parameter can be set to enable further CPUs to be
- added at runtime. If omitted the maximum number of CPUs will be
- set to match the initial CPU count. Both parameters are subject to
- an upper limit that is determined by the specific machine type chosen.
+ added at runtime. When both parameters are omitted, the maximum number
+ of CPUs will be calculated from the provided topology members and the
+ initial CPU count will match the maximum number. When only one of them
+ is given then the omitted one will be set to its counterpart's value.
+ Both parameters may be specified, but the maximum number of CPUs must
+ be equal to or greater than the initial CPU count. Both parameters are
+ subject to an upper limit that is determined by the specific machine
+ type chosen.
To control reporting of CPU topology information, the number of sockets,
dies per socket, cores per die, and threads per core can be specified.
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 04/14] machine: Improve the error reporting of smp parsing
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (2 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 03/14] machine: Set the value of cpus to match maxcpus if it's omitted Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 05/14] hw: Add compat machines for 6.2 Yanan Wang
` (9 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
We have two requirements for a valid SMP configuration:
the product of "sockets * cores * threads" must represent all the
possible cpus, i.e., max_cpus, and then must include the initially
present cpus, i.e., smp_cpus.
So we only need to ensure 1) "sockets * cores * threads == maxcpus"
at first and then ensure 2) "maxcpus >= cpus". With a reasonable
order of the sanity check, we can simplify the error reporting code.
When reporting an error message we also report the exact value of
each topology member to make users easily see what's going on.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
---
hw/core/machine.c | 22 +++++++++-------------
hw/i386/pc.c | 24 ++++++++++--------------
2 files changed, 19 insertions(+), 27 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1a6c6cfd0e..c6646bf922 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -779,25 +779,21 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
cpus = cpus > 0 ? cpus : maxcpus;
- if (sockets * cores * threads < cpus) {
- error_setg(errp, "cpu topology: "
- "sockets (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, cores, threads, cpus);
+ if (sockets * cores * threads != maxcpus) {
+ error_setg(errp, "Invalid CPU topology: "
+ "product of the hierarchy must match maxcpus: "
+ "sockets (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, cores, threads, maxcpus);
return;
}
if (maxcpus < cpus) {
- error_setg(errp, "maxcpus must be equal to or greater than smp");
- return;
- }
-
- if (sockets * cores * threads != maxcpus) {
error_setg(errp, "Invalid CPU topology: "
+ "maxcpus must be equal to or greater than smp: "
"sockets (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, cores, threads,
- maxcpus);
+ "== maxcpus (%u) < smp_cpus (%u)",
+ sockets, cores, threads, maxcpus, cpus);
return;
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9ad7ae5254..fcf6905219 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -747,25 +747,21 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
cpus = cpus > 0 ? cpus : maxcpus;
- if (sockets * dies * cores * threads < cpus) {
- error_setg(errp, "cpu topology: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, dies, cores, threads, cpus);
+ if (sockets * dies * cores * threads != maxcpus) {
+ error_setg(errp, "Invalid CPU topology: "
+ "product of the hierarchy must match maxcpus: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, dies, cores, threads, maxcpus);
return;
}
if (maxcpus < cpus) {
- error_setg(errp, "maxcpus must be equal to or greater than smp");
- return;
- }
-
- if (sockets * dies * cores * threads != maxcpus) {
- error_setg(errp, "Invalid CPU topology deprecated: "
+ error_setg(errp, "Invalid CPU topology: "
+ "maxcpus must be equal to or greater than smp: "
"sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, dies, cores, threads,
- maxcpus);
+ "== maxcpus (%u) < smp_cpus (%u)",
+ sockets, dies, cores, threads, maxcpus, cpus);
return;
}
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 05/14] hw: Add compat machines for 6.2
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (3 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 04/14] machine: Improve the error reporting of smp parsing Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2 Yanan Wang
` (8 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Add 6.2 machine types for arm/i440fx/q35/s390x/spapr.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
---
hw/arm/virt.c | 9 ++++++++-
hw/core/machine.c | 3 +++
hw/i386/pc.c | 3 +++
hw/i386/pc_piix.c | 14 +++++++++++++-
hw/i386/pc_q35.c | 13 ++++++++++++-
hw/ppc/spapr.c | 15 +++++++++++++--
hw/s390x/s390-virtio-ccw.c | 14 +++++++++++++-
include/hw/boards.h | 3 +++
include/hw/i386/pc.h | 3 +++
9 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 81eda46b0b..01165f7f53 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2788,10 +2788,17 @@ static void machvirt_machine_init(void)
}
type_init(machvirt_machine_init);
+static void virt_machine_6_2_options(MachineClass *mc)
+{
+}
+DEFINE_VIRT_MACHINE_AS_LATEST(6, 2)
+
static void virt_machine_6_1_options(MachineClass *mc)
{
+ virt_machine_6_2_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
}
-DEFINE_VIRT_MACHINE_AS_LATEST(6, 1)
+DEFINE_VIRT_MACHINE(6, 1)
static void virt_machine_6_0_options(MachineClass *mc)
{
diff --git a/hw/core/machine.c b/hw/core/machine.c
index c6646bf922..bdce80df32 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,6 +37,9 @@
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-pci.h"
+GlobalProperty hw_compat_6_1[] = {};
+const size_t hw_compat_6_1_len = G_N_ELEMENTS(hw_compat_6_1);
+
GlobalProperty hw_compat_6_0[] = {
{ "gpex-pcihost", "allow-unmapped-accesses", "false" },
{ "i8042", "extended-state", "false"},
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index fcf6905219..afd8b9c283 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -94,6 +94,9 @@
#include "trace.h"
#include CONFIG_DEVICES
+GlobalProperty pc_compat_6_1[] = {};
+const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
+
GlobalProperty pc_compat_6_0[] = {
{ "qemu64" "-" TYPE_X86_CPU, "family", "6" },
{ "qemu64" "-" TYPE_X86_CPU, "model", "6" },
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 30b8bd6ea9..fd5c2277f2 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -413,7 +413,7 @@ static void pc_i440fx_machine_options(MachineClass *m)
machine_class_allow_dynamic_sysbus_dev(m, TYPE_VMBUS_BRIDGE);
}
-static void pc_i440fx_6_1_machine_options(MachineClass *m)
+static void pc_i440fx_6_2_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_i440fx_machine_options(m);
@@ -422,6 +422,18 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_I440FX_MACHINE(v6_2, "pc-i440fx-6.2", NULL,
+ pc_i440fx_6_2_machine_options);
+
+static void pc_i440fx_6_1_machine_options(MachineClass *m)
+{
+ pc_i440fx_6_2_machine_options(m);
+ m->alias = NULL;
+ m->is_default = false;
+ compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+ compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+}
+
DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
pc_i440fx_6_1_machine_options);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 04b4a4788d..b45903b15e 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -355,7 +355,7 @@ static void pc_q35_machine_options(MachineClass *m)
m->max_cpus = 288;
}
-static void pc_q35_6_1_machine_options(MachineClass *m)
+static void pc_q35_6_2_machine_options(MachineClass *m)
{
PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_machine_options(m);
@@ -363,6 +363,17 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
pcmc->default_cpu_version = 1;
}
+DEFINE_Q35_MACHINE(v6_2, "pc-q35-6.2", NULL,
+ pc_q35_6_2_machine_options);
+
+static void pc_q35_6_1_machine_options(MachineClass *m)
+{
+ pc_q35_6_2_machine_options(m);
+ m->alias = NULL;
+ compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+ compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+}
+
DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
pc_q35_6_1_machine_options);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 81699d4f8b..d39fd4e644 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4685,15 +4685,26 @@ static void spapr_machine_latest_class_options(MachineClass *mc)
} \
type_init(spapr_machine_register_##suffix)
+/*
+ * pseries-6.2
+ */
+static void spapr_machine_6_2_class_options(MachineClass *mc)
+{
+ /* Defaults for the latest behaviour inherited from the base class */
+}
+
+DEFINE_SPAPR_MACHINE(6_2, "6.2", true);
+
/*
* pseries-6.1
*/
static void spapr_machine_6_1_class_options(MachineClass *mc)
{
- /* Defaults for the latest behaviour inherited from the base class */
+ spapr_machine_6_2_class_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
}
-DEFINE_SPAPR_MACHINE(6_1, "6.1", true);
+DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
/*
* pseries-6.0
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e4b18aef49..4d25278cf2 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -791,14 +791,26 @@ bool css_migration_enabled(void)
} \
type_init(ccw_machine_register_##suffix)
+static void ccw_machine_6_2_instance_options(MachineState *machine)
+{
+}
+
+static void ccw_machine_6_2_class_options(MachineClass *mc)
+{
+}
+DEFINE_CCW_MACHINE(6_2, "6.2", true);
+
static void ccw_machine_6_1_instance_options(MachineState *machine)
{
+ ccw_machine_6_2_instance_options(machine);
}
static void ccw_machine_6_1_class_options(MachineClass *mc)
{
+ ccw_machine_6_2_class_options(mc);
+ compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
}
-DEFINE_CCW_MACHINE(6_1, "6.1", true);
+DEFINE_CCW_MACHINE(6_1, "6.1", false);
static void ccw_machine_6_0_instance_options(MachineState *machine)
{
diff --git a/include/hw/boards.h b/include/hw/boards.h
index accd6eff35..463a5514f9 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -353,6 +353,9 @@ struct MachineState {
} \
type_init(machine_initfn##_register_types)
+extern GlobalProperty hw_compat_6_1[];
+extern const size_t hw_compat_6_1_len;
+
extern GlobalProperty hw_compat_6_0[];
extern const size_t hw_compat_6_0_len;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 88dffe7517..97b4ab79b5 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -196,6 +196,9 @@ void pc_system_parse_ovmf_flash(uint8_t *flash_ptr, size_t flash_size);
void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
const CPUArchIdList *apic_ids, GArray *entry);
+extern GlobalProperty pc_compat_6_1[];
+extern const size_t pc_compat_6_1_len;
+
extern GlobalProperty pc_compat_6_0[];
extern const size_t pc_compat_6_0_len;
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (4 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 05/14] hw: Add compat machines for 6.2 Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 16:30 ` Pankaj Gupta
2021-08-16 8:02 ` Pankaj Gupta
2021-08-13 2:39 ` [PATCH for-6.2 v5 07/14] machine: Use ms instead of global current_machine in sanity-check Yanan Wang
` (7 subsequent siblings)
13 siblings, 2 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
In the real SMP hardware topology world, it's much more likely that
we have high cores-per-socket counts and few sockets totally. While
the current preference of sockets over cores in smp parsing results
in a virtual cpu topology with low cores-per-sockets counts and a
large number of sockets, which is just contrary to the real world.
Given that it is better to make the virtual cpu topology be more
reflective of the real world and also for the sake of compatibility,
we start to prefer cores over sockets over threads in smp parsing
since machine type 6.2 for different arches.
In this patch, a boolean "smp_prefer_sockets" is added, and we only
enable the old preference on older machines and enable the new one
since type 6.2 for all arches by using the machine compat mechanism.
Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Acked-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/arm/virt.c | 1 +
hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
hw/i386/pc_piix.c | 1 +
hw/i386/pc_q35.c | 1 +
hw/ppc/spapr.c | 1 +
hw/s390x/s390-virtio-ccw.c | 1 +
include/hw/boards.h | 1 +
qemu-options.hx | 3 ++-
9 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 01165f7f53..7babea40dc 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
{
virt_machine_6_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+ mc->smp_prefer_sockets = true;
}
DEFINE_VIRT_MACHINE(6, 1)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index bdce80df32..15b41c52e8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
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 cores = config->has_cores ? config->cores : 0;
@@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
return;
}
- /* compute missing values, prefer sockets over cores over threads */
+ /* compute missing values based on the provided ones */
if (cpus == 0 && maxcpus == 0) {
sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
@@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
} else {
maxcpus = maxcpus > 0 ? maxcpus : cpus;
- if (sockets == 0) {
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (cores * threads);
- } else if (cores == 0) {
- threads = threads > 0 ? threads : 1;
- cores = maxcpus / (sockets * threads);
- } else if (threads == 0) {
+ if (mc->smp_prefer_sockets) {
+ /* prefer sockets over cores before 6.2 */
+ if (sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (cores * threads);
+ } else if (cores == 0) {
+ threads = threads > 0 ? threads : 1;
+ cores = maxcpus / (sockets * 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 * threads);
+ } else if (sockets == 0) {
+ threads = threads > 0 ? threads : 1;
+ sockets = maxcpus / (cores * threads);
+ }
+ }
+
+ /* try to calculate omitted threads at last */
+ if (threads == 0) {
threads = maxcpus / (sockets * cores);
}
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index afd8b9c283..4b05ff7160 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
*/
static void pc_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;
@@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
/* directly default dies to 1 if it's omitted */
dies = dies > 0 ? dies : 1;
- /* compute missing values, prefer sockets over cores over threads */
+ /* compute missing values based on the provided ones */
if (cpus == 0 && maxcpus == 0) {
sockets = sockets > 0 ? sockets : 1;
cores = cores > 0 ? cores : 1;
@@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
} else {
maxcpus = maxcpus > 0 ? maxcpus : cpus;
- 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 if (threads == 0) {
+ if (mc->smp_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);
}
}
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index fd5c2277f2..9b811fc6ca 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
m->is_default = false;
compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+ m->smp_prefer_sockets = true;
}
DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b45903b15e..88efb7fde4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
m->alias = NULL;
compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
+ m->smp_prefer_sockets = true;
}
DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index d39fd4e644..a481fade51 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
{
spapr_machine_6_2_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+ mc->smp_prefer_sockets = true;
}
DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4d25278cf2..b40e647883 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
{
ccw_machine_6_2_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
+ mc->smp_prefer_sockets = true;
}
DEFINE_CCW_MACHINE(6_1, "6.1", false);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 463a5514f9..2ae039b74f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -247,6 +247,7 @@ struct MachineClass {
bool nvdimm_supported;
bool numa_mem_supported;
bool auto_enable_numa;
+ bool smp_prefer_sockets;
const char *default_ram_id;
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
diff --git a/qemu-options.hx b/qemu-options.hx
index 06f819177e..451d2cd817 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -238,7 +238,8 @@ SRST
Historically preference was given to the coarsest topology parameters
when computing missing values (ie sockets preferred over cores, which
were preferred over threads), however, this behaviour is considered
- liable to change.
+ liable to change. Prior to 6.2 the preference was sockets over cores
+ over threads. Since 6.2 the preference is cores over sockets over threads.
ERST
DEF("numa", HAS_ARG, QEMU_OPTION_numa,
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 07/14] machine: Use ms instead of global current_machine in sanity-check
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (5 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2 Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 08/14] machine: Tweak the order of topology members in struct CpuTopology Yanan Wang
` (6 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
In the sanity-check of smp_cpus and max_cpus against mc in function
machine_set_smp(), we are now using ms->smp.max_cpus for the check
but using current_machine->smp.max_cpus in the error message.
Tweak this by uniformly using the local ms.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
hw/core/machine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 15b41c52e8..45fa3cd61b 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -881,7 +881,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
} else if (ms->smp.max_cpus > mc->max_cpus) {
error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
"supported by machine '%s' is %d",
- current_machine->smp.max_cpus,
+ ms->smp.max_cpus,
mc->name, mc->max_cpus);
}
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 08/14] machine: Tweak the order of topology members in struct CpuTopology
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (6 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 07/14] machine: Use ms instead of global current_machine in sanity-check Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 09/14] machine: Make smp_parse generic enough for all arches Yanan Wang
` (5 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Now that all the possible topology parameters are integrated in struct
CpuTopology, tweak the order of topology members to be "cpus/sockets/
dies/cores/threads/maxcpus" for readability and consistency. We also
tweak the comment by adding explanation of dies parameter.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
---
hw/core/machine.c | 8 ++++----
include/hw/boards.h | 7 ++++---
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 45fa3cd61b..d271456153 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -829,11 +829,11 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name,
{
MachineState *ms = MACHINE(obj);
SMPConfiguration *config = &(SMPConfiguration){
- .has_cores = true, .cores = ms->smp.cores,
+ .has_cpus = true, .cpus = ms->smp.cpus,
.has_sockets = true, .sockets = ms->smp.sockets,
.has_dies = true, .dies = ms->smp.dies,
+ .has_cores = true, .cores = ms->smp.cores,
.has_threads = true, .threads = ms->smp.threads,
- .has_cpus = true, .cpus = ms->smp.cpus,
.has_maxcpus = true, .maxcpus = ms->smp.max_cpus,
};
if (!visit_type_SMPConfiguration(v, name, &config, &error_abort)) {
@@ -1060,10 +1060,10 @@ static void machine_initfn(Object *obj)
/* default to mc->default_cpus */
ms->smp.cpus = mc->default_cpus;
ms->smp.max_cpus = mc->default_cpus;
- ms->smp.cores = 1;
+ ms->smp.sockets = 1;
ms->smp.dies = 1;
+ ms->smp.cores = 1;
ms->smp.threads = 1;
- ms->smp.sockets = 1;
}
static void machine_finalize(Object *obj)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2ae039b74f..2a1bba86c0 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -275,17 +275,18 @@ typedef struct DeviceMemoryState {
/**
* CpuTopology:
* @cpus: the number of present logical processors on the machine
- * @cores: the number of cores in one package
- * @threads: the number of threads in one core
* @sockets: the number of sockets on the machine
+ * @dies: the number of dies in one socket
+ * @cores: the number of cores in one die
+ * @threads: the number of threads in one core
* @max_cpus: the maximum number of logical processors on the machine
*/
typedef struct CpuTopology {
unsigned int cpus;
+ unsigned int sockets;
unsigned int dies;
unsigned int cores;
unsigned int threads;
- unsigned int sockets;
unsigned int max_cpus;
} CpuTopology;
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 09/14] machine: Make smp_parse generic enough for all arches
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (7 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 08/14] machine: Tweak the order of topology members in struct CpuTopology Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 10/14] machine: Remove smp_parse callback from MachineClass Yanan Wang
` (4 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Currently the only difference between smp_parse and pc_smp_parse
is the support of dies parameter and the related error reporting.
With some arch compat variables like "bool dies_supported", we can
make smp_parse generic enough for all arches and the PC specific
one can be removed.
Making smp_parse() generic enough can reduce code duplication and
ease the code maintenance, and also allows extending the topology
with more arch specific members (e.g., clusters) in the future.
Suggested-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/core/machine.c | 110 ++++++++++++++++++++++++++++++++++++--------
hw/i386/pc.c | 84 +--------------------------------
include/hw/boards.h | 9 ++++
3 files changed, 100 insertions(+), 103 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d271456153..b0f82c8811 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -15,6 +15,7 @@
#include "qapi/qmp/qerror.h"
#include "sysemu/replay.h"
#include "qemu/units.h"
+#include "qemu/cutils.h"
#include "hw/boards.h"
#include "hw/loader.h"
#include "qapi/error.h"
@@ -746,20 +747,87 @@ void machine_set_cpu_numa_node(MachineState *machine,
}
}
+static char *cpu_topology_hierarchy(MachineState *ms)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+ SMPCompatProps *smp_props = &mc->smp_props;
+ char topo_msg[256] = "";
+
+ /*
+ * Topology members should be ordered from the largest to the smallest.
+ * Concept of sockets/cores/threads is supported by default and will be
+ * reported in the hierarchy. Unsupported members will not be reported.
+ */
+ g_autofree char *sockets_msg = g_strdup_printf(
+ " * sockets (%u)", ms->smp.sockets);
+ pstrcat(topo_msg, sizeof(topo_msg), sockets_msg);
+
+ if (smp_props->dies_supported) {
+ g_autofree char *dies_msg = g_strdup_printf(
+ " * dies (%u)", ms->smp.dies);
+ pstrcat(topo_msg, sizeof(topo_msg), dies_msg);
+ }
+
+ g_autofree char *cores_msg = g_strdup_printf(
+ " * cores (%u)", ms->smp.cores);
+ pstrcat(topo_msg, sizeof(topo_msg), cores_msg);
+
+ g_autofree char *threads_msg = g_strdup_printf(
+ " * threads (%u)", ms->smp.threads);
+ pstrcat(topo_msg, sizeof(topo_msg), threads_msg);
+
+ return g_strdup_printf("%s", topo_msg + 3);
+}
+
+/*
+ * smp_parse - Generic function used to parse the given SMP configuration
+ *
+ * If not supported by the machine, a topology parameter must be omitted
+ * or specified equal to 1. Concept of sockets/cores/threads is supported
+ * by default. Unsupported members will not be reported in the topology
+ * hierarchy message.
+ *
+ * For compatibility, omitted arch-specific members (e.g. dies) will not
+ * be computed, but will directly default to 1 instead. This logic should
+ * also apply to future introduced ones.
+ *
+ * Omitted arch-neutral parameters (i.e. cpus/sockets/cores/threads/maxcpus)
+ * will be computed based on the provided ones. 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.
+ *
+ * In 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.
+ */
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;
- if (config->has_dies && config->dies > 1) {
+ /*
+ * 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;
}
+ /*
+ * Omitted arch-specific members will not be computed, but will
+ * directly default to 1 instead.
+ */
+ dies = dies > 0 ? dies : 1;
+
/* compute missing values based on the provided ones */
if (cpus == 0 && maxcpus == 0) {
sockets = sockets > 0 ? sockets : 1;
@@ -773,55 +841,57 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
if (sockets == 0) {
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (cores * threads);
+ sockets = maxcpus / (dies * cores * threads);
} else if (cores == 0) {
threads = threads > 0 ? threads : 1;
- cores = maxcpus / (sockets * threads);
+ 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 * threads);
+ cores = maxcpus / (sockets * dies * threads);
} else if (sockets == 0) {
threads = threads > 0 ? threads : 1;
- sockets = maxcpus / (cores * threads);
+ sockets = maxcpus / (dies * cores * threads);
}
}
/* try to calculate omitted threads at last */
if (threads == 0) {
- threads = maxcpus / (sockets * cores);
+ threads = maxcpus / (sockets * dies * cores);
}
}
- maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
+ maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
cpus = cpus > 0 ? cpus : maxcpus;
- if (sockets * cores * threads != 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_topology_hierarchy(ms);
error_setg(errp, "Invalid CPU topology: "
"product of the hierarchy must match maxcpus: "
- "sockets (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, cores, threads, maxcpus);
+ "%s != maxcpus (%u)",
+ topo_msg, maxcpus);
return;
}
if (maxcpus < cpus) {
+ g_autofree char *topo_msg = cpu_topology_hierarchy(ms);
error_setg(errp, "Invalid CPU topology: "
"maxcpus must be equal to or greater than smp: "
- "sockets (%u) * cores (%u) * threads (%u) "
- "== maxcpus (%u) < smp_cpus (%u)",
- sockets, cores, threads, maxcpus, cpus);
+ "%s == maxcpus (%u) < smp_cpus (%u)",
+ topo_msg, maxcpus, cpus);
return;
}
-
- ms->smp.cpus = cpus;
- ms->smp.sockets = sockets;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.max_cpus = maxcpus;
}
static void machine_get_smp(Object *obj, Visitor *v, const char *name,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4b05ff7160..ce493a9294 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -711,88 +711,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
}
}
-/*
- * This function is very similar to smp_parse()
- * in hw/core/machine.c but includes CPU die support.
- */
-static void pc_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;
-
- /* directly default dies to 1 if it's omitted */
- 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_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;
-
- if (sockets * dies * cores * threads != maxcpus) {
- error_setg(errp, "Invalid CPU topology: "
- "product of the hierarchy must match maxcpus: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, dies, cores, threads, maxcpus);
- return;
- }
-
- if (maxcpus < cpus) {
- error_setg(errp, "Invalid CPU topology: "
- "maxcpus must be equal to or greater than smp: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
- "== maxcpus (%u) < smp_cpus (%u)",
- sockets, dies, cores, threads, maxcpus, cpus);
- return;
- }
-
- ms->smp.cpus = cpus;
- ms->smp.sockets = sockets;
- ms->smp.dies = dies;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.maxcpus = maxcpus;
-}
-
static
void pc_machine_done(Notifier *notifier, void *data)
{
@@ -1746,7 +1664,6 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
mc->auto_enable_numa_with_memdev = true;
mc->has_hotpluggable_cpus = true;
mc->default_boot_order = "cad";
- mc->smp_parse = pc_smp_parse;
mc->block_default_type = IF_IDE;
mc->max_cpus = 255;
mc->reset = pc_machine_reset;
@@ -1757,6 +1674,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data)
hc->unplug = pc_machine_device_unplug_cb;
mc->default_cpu_type = TARGET_DEFAULT_CPU_TYPE;
mc->nvdimm_supported = true;
+ mc->smp_props.dies_supported = true;
mc->default_ram_id = "pc.ram";
object_class_property_add(oc, PC_MACHINE_MAX_RAM_BELOW_4G, "size",
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2a1bba86c0..72a23e4e0f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -108,6 +108,14 @@ typedef struct {
CPUArchId cpus[];
} CPUArchIdList;
+/**
+ * SMPCompatProps:
+ * @dies_supported - whether dies are supported by the machine
+ */
+typedef struct {
+ bool dies_supported;
+} SMPCompatProps;
+
/**
* MachineClass:
* @deprecation_reason: If set, the machine is marked as deprecated. The
@@ -248,6 +256,7 @@ struct MachineClass {
bool numa_mem_supported;
bool auto_enable_numa;
bool smp_prefer_sockets;
+ SMPCompatProps smp_props;
const char *default_ram_id;
HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 10/14] machine: Remove smp_parse callback from MachineClass
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (8 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 09/14] machine: Make smp_parse generic enough for all arches Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 11/14] machine: Move smp_prefer_sockets to struct SMPCompatProps Yanan Wang
` (3 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Now we have a generic smp parser for all arches, and there will
not be any other arch specific ones, so let's remove the callback
from MachineClass and call the parser directly.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/core/machine.c | 3 +--
include/hw/boards.h | 5 -----
2 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index b0f82c8811..3823e609e0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -937,7 +937,7 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
goto out_free;
}
- mc->smp_parse(ms, config, errp);
+ smp_parse(ms, config, errp);
if (errp) {
goto out_free;
}
@@ -966,7 +966,6 @@ static void machine_class_init(ObjectClass *oc, void *data)
/* Default 128 MB as guest ram size */
mc->default_ram_size = 128 * MiB;
mc->rom_file_has_mr = true;
- mc->smp_parse = smp_parse;
/* numa node memory size aligned on 8MB by default.
* On Linux, each node's border has to be 8MB aligned
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 72a23e4e0f..fa284e01e9 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -177,10 +177,6 @@ typedef struct {
* kvm-type may be NULL if it is not needed.
* @numa_mem_supported:
* true if '--numa node.mem' option is supported and false otherwise
- * @smp_parse:
- * The function pointer to hook different machine specific functions for
- * parsing "smp-opts" from QemuOpts to MachineState::CpuTopology and more
- * machine specific topology fields, such as smp_dies for PCMachine.
* @hotplug_allowed:
* If the hook is provided, then it'll be called for each device
* hotplug to check whether the device hotplug is allowed. Return
@@ -217,7 +213,6 @@ struct MachineClass {
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
int (*kvm_type)(MachineState *machine, const char *arg);
- void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp);
BlockInterfaceType block_default_type;
int units_per_default_bus;
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 11/14] machine: Move smp_prefer_sockets to struct SMPCompatProps
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (9 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 10/14] machine: Remove smp_parse callback from MachineClass Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 12/14] machine: Put all sanity-check in the generic SMP parser Yanan Wang
` (2 subsequent siblings)
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Now we have a common structure SMPCompatProps used to store information
about SMP compatibility stuff, so we can also move smp_prefer_sockets
there for cleaner code.
No functional change intended.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
hw/arm/virt.c | 2 +-
hw/core/machine.c | 2 +-
hw/i386/pc_piix.c | 2 +-
hw/i386/pc_q35.c | 2 +-
hw/ppc/spapr.c | 2 +-
hw/s390x/s390-virtio-ccw.c | 2 +-
include/hw/boards.h | 3 ++-
7 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7babea40dc..ae029680da 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2797,7 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
{
virt_machine_6_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
- mc->smp_prefer_sockets = true;
+ mc->smp_props.prefer_sockets = true;
}
DEFINE_VIRT_MACHINE(6, 1)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 3823e609e0..7e1dd623ae 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -836,7 +836,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
} else {
maxcpus = maxcpus > 0 ? maxcpus : cpus;
- if (mc->smp_prefer_sockets) {
+ if (mc->smp_props.prefer_sockets) {
/* prefer sockets over cores before 6.2 */
if (sockets == 0) {
cores = cores > 0 ? cores : 1;
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9b811fc6ca..a60ebfc2c1 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -432,7 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
m->is_default = false;
compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
- m->smp_prefer_sockets = true;
+ m->smp_props.prefer_sockets = true;
}
DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 88efb7fde4..4b622ffb82 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -372,7 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
m->alias = NULL;
compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
- m->smp_prefer_sockets = true;
+ m->smp_props.prefer_sockets = true;
}
DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a481fade51..efdea43c0d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4702,7 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
{
spapr_machine_6_2_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
- mc->smp_prefer_sockets = true;
+ mc->smp_props.prefer_sockets = true;
}
DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index b40e647883..5bdef9b4d7 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -809,7 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
{
ccw_machine_6_2_class_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
- mc->smp_prefer_sockets = true;
+ mc->smp_props.prefer_sockets = true;
}
DEFINE_CCW_MACHINE(6_1, "6.1", false);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index fa284e01e9..5adbcbb99b 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -110,9 +110,11 @@ typedef struct {
/**
* SMPCompatProps:
+ * @prefer_sockets - whether sockets are preferred over cores in smp parsing
* @dies_supported - whether dies are supported by the machine
*/
typedef struct {
+ bool prefer_sockets;
bool dies_supported;
} SMPCompatProps;
@@ -250,7 +252,6 @@ struct MachineClass {
bool nvdimm_supported;
bool numa_mem_supported;
bool auto_enable_numa;
- bool smp_prefer_sockets;
SMPCompatProps smp_props;
const char *default_ram_id;
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 12/14] machine: Put all sanity-check in the generic SMP parser
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (10 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 11/14] machine: Move smp_prefer_sockets to struct SMPCompatProps Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 13/14] machine: Split out the smp parsing code Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 14/14] tests/unit: Add a unit test for smp parsing Yanan Wang
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Put both sanity-check of the input SMP configuration and sanity-check
of the output SMP configuration uniformly in the generic parser. Then
machine_set_smp() will become cleaner, also all the invalid scenarios
can be tested only by calling the parser.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
---
hw/core/machine.c | 63 +++++++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 32 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 7e1dd623ae..653d352cf6 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -813,6 +813,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
unsigned threads = config->has_threads ? config->threads : 0;
unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+ /*
+ * A specified topology parameter 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)) {
+ error_setg(errp, "CPU topology parameters must be greater than zero");
+ return;
+ }
+
/*
* If not supported by the machine, a topology parameter must be
* omitted or specified equal to 1.
@@ -892,6 +906,22 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
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,
@@ -914,7 +944,6 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name,
static void machine_set_smp(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- MachineClass *mc = MACHINE_GET_CLASS(obj);
MachineState *ms = MACHINE(obj);
SMPConfiguration *config;
ERRP_GUARD();
@@ -923,40 +952,10 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name,
return;
}
- /*
- * The CPU topology parameters must be specified greater than zero or
- * just omitted, 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)) {
- error_setg(errp, "CPU topology parameters must be greater than zero");
- goto out_free;
- }
-
smp_parse(ms, config, errp);
if (errp) {
- goto out_free;
+ qapi_free_SMPConfiguration(config);
}
-
- /* sanity-check smp_cpus and max_cpus against mc */
- 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);
- } else 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);
- }
-
-out_free:
- qapi_free_SMPConfiguration(config);
}
static void machine_class_init(ObjectClass *oc, void *data)
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 13/14] machine: Split out the smp parsing code
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (11 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 12/14] machine: Put all sanity-check in the generic SMP parser Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 14/14] tests/unit: Add a unit test for smp parsing Yanan Wang
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
We are going to introduce an unit test for the parser smp_parse()
in hw/core/machine.c, but now machine.c is only built in softmmu.
In order to solve the build dependency on the smp parsing code and
avoid building unrelated stuff for the unit tests, move the related
code from machine.c into a new common file, i.e., machine-smp.c.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
MAINTAINERS | 1 +
hw/core/machine-smp.c | 200 ++++++++++++++++++++++++++++++++++++++++++
hw/core/machine.c | 178 -------------------------------------
hw/core/meson.build | 1 +
include/hw/boards.h | 1 +
5 files changed, 203 insertions(+), 178 deletions(-)
create mode 100644 hw/core/machine-smp.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 37b1a8e442..5510aaddfd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1629,6 +1629,7 @@ F: cpu.c
F: hw/core/cpu.c
F: hw/core/machine-qmp-cmds.c
F: hw/core/machine.c
+F: hw/core/machine-smp.c
F: hw/core/null-machine.c
F: hw/core/numa.c
F: hw/cpu/cluster.c
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
new file mode 100644
index 0000000000..8fa6b71885
--- /dev/null
+++ b/hw/core/machine-smp.c
@@ -0,0 +1,200 @@
+/*
+ * QEMU Machine (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/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "qapi/error.h"
+#include "qemu/cutils.h"
+
+static char *cpu_topology_hierarchy(MachineState *ms)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+ SMPCompatProps *smp_props = &mc->smp_props;
+ char topo_msg[256] = "";
+
+ /*
+ * Topology members should be ordered from the largest to the smallest.
+ * Concept of sockets/cores/threads is supported by default and will be
+ * reported in the hierarchy. Unsupported members will not be reported.
+ */
+ g_autofree char *sockets_msg = g_strdup_printf(
+ " * sockets (%u)", ms->smp.sockets);
+ pstrcat(topo_msg, sizeof(topo_msg), sockets_msg);
+
+ if (smp_props->dies_supported) {
+ g_autofree char *dies_msg = g_strdup_printf(
+ " * dies (%u)", ms->smp.dies);
+ pstrcat(topo_msg, sizeof(topo_msg), dies_msg);
+ }
+
+ g_autofree char *cores_msg = g_strdup_printf(
+ " * cores (%u)", ms->smp.cores);
+ pstrcat(topo_msg, sizeof(topo_msg), cores_msg);
+
+ g_autofree char *threads_msg = g_strdup_printf(
+ " * threads (%u)", ms->smp.threads);
+ pstrcat(topo_msg, sizeof(topo_msg), threads_msg);
+
+ return g_strdup_printf("%s", topo_msg + 3);
+}
+
+/*
+ * smp_parse - Generic function used to parse the given SMP configuration
+ *
+ * If not supported by the machine, a topology parameter must be omitted
+ * or specified equal to 1. Concept of sockets/cores/threads is supported
+ * by default. Unsupported members will not be reported in the topology
+ * hierarchy message.
+ *
+ * For compatibility, omitted arch-specific members (e.g. dies) will not
+ * be computed, but will directly default to 1 instead. This logic should
+ * also apply to future introduced ones.
+ *
+ * Omitted arch-neutral parameters (i.e. cpus/sockets/cores/threads/maxcpus)
+ * will be computed based on the provided ones. 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.
+ *
+ * In 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.
+ */
+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;
+
+ /*
+ * A specified topology parameter 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)) {
+ error_setg(errp, "CPU topology parameters must be greater than zero");
+ return;
+ }
+
+ /*
+ * 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;
+ }
+
+ /*
+ * Omitted arch-specific members will not be computed, but will
+ * directly default to 1 instead.
+ */
+ 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_topology_hierarchy(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_topology_hierarchy(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;
+ }
+}
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 653d352cf6..94a97cdad5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -15,7 +15,6 @@
#include "qapi/qmp/qerror.h"
#include "sysemu/replay.h"
#include "qemu/units.h"
-#include "qemu/cutils.h"
#include "hw/boards.h"
#include "hw/loader.h"
#include "qapi/error.h"
@@ -747,183 +746,6 @@ void machine_set_cpu_numa_node(MachineState *machine,
}
}
-static char *cpu_topology_hierarchy(MachineState *ms)
-{
- MachineClass *mc = MACHINE_GET_CLASS(ms);
- SMPCompatProps *smp_props = &mc->smp_props;
- char topo_msg[256] = "";
-
- /*
- * Topology members should be ordered from the largest to the smallest.
- * Concept of sockets/cores/threads is supported by default and will be
- * reported in the hierarchy. Unsupported members will not be reported.
- */
- g_autofree char *sockets_msg = g_strdup_printf(
- " * sockets (%u)", ms->smp.sockets);
- pstrcat(topo_msg, sizeof(topo_msg), sockets_msg);
-
- if (smp_props->dies_supported) {
- g_autofree char *dies_msg = g_strdup_printf(
- " * dies (%u)", ms->smp.dies);
- pstrcat(topo_msg, sizeof(topo_msg), dies_msg);
- }
-
- g_autofree char *cores_msg = g_strdup_printf(
- " * cores (%u)", ms->smp.cores);
- pstrcat(topo_msg, sizeof(topo_msg), cores_msg);
-
- g_autofree char *threads_msg = g_strdup_printf(
- " * threads (%u)", ms->smp.threads);
- pstrcat(topo_msg, sizeof(topo_msg), threads_msg);
-
- return g_strdup_printf("%s", topo_msg + 3);
-}
-
-/*
- * smp_parse - Generic function used to parse the given SMP configuration
- *
- * If not supported by the machine, a topology parameter must be omitted
- * or specified equal to 1. Concept of sockets/cores/threads is supported
- * by default. Unsupported members will not be reported in the topology
- * hierarchy message.
- *
- * For compatibility, omitted arch-specific members (e.g. dies) will not
- * be computed, but will directly default to 1 instead. This logic should
- * also apply to future introduced ones.
- *
- * Omitted arch-neutral parameters (i.e. cpus/sockets/cores/threads/maxcpus)
- * will be computed based on the provided ones. 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.
- *
- * In 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.
- */
-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;
-
- /*
- * A specified topology parameter 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)) {
- error_setg(errp, "CPU topology parameters must be greater than zero");
- return;
- }
-
- /*
- * 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;
- }
-
- /*
- * Omitted arch-specific members will not be computed, but will
- * directly default to 1 instead.
- */
- 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_topology_hierarchy(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_topology_hierarchy(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/hw/core/meson.build b/hw/core/meson.build
index 18f44fb7c2..6d727c7742 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -14,6 +14,7 @@ hwcore_files = files(
)
common_ss.add(files('cpu-common.c'))
+common_ss.add(files('machine-smp.c'))
common_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
common_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true: files('generic-loader.c'))
common_ss.add(when: ['CONFIG_GUEST_LOADER', fdt], if_true: files('guest-loader.c'))
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5adbcbb99b..e36fc7d861 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -34,6 +34,7 @@ HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine);
void machine_set_cpu_numa_node(MachineState *machine,
const CpuInstanceProperties *props,
Error **errp);
+void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp);
/**
* machine_class_allow_dynamic_sysbus_dev: Add type to list of valid devices
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH for-6.2 v5 14/14] tests/unit: Add a unit test for smp parsing
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
` (12 preceding siblings ...)
2021-08-13 2:39 ` [PATCH for-6.2 v5 13/14] machine: Split out the smp parsing code Yanan Wang
@ 2021-08-13 2:39 ` Yanan Wang
13 siblings, 0 replies; 20+ messages in thread
From: Yanan Wang @ 2021-08-13 2:39 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jones, Cornelia Huck,
Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin, Pankaj Gupta,
Richard Henderson, Greg Kurz, Halil Pasic, wanghaibin.wang,
Thomas Huth, Paolo Bonzini, Yanan Wang, David Gibson
Add a QEMU unit test for the parsing of given SMP configuration.
Since all the parsing logic is in generic function smp_parse(),
this test passes different SMP configurations to the function
and compare the parsing result with what is expected.
In the test, all possible collections of the topology parameters
and the corresponding expected results are listed, including the
valid and invalid ones.
The preference of sockets over cores and the preference of cores
over sockets, and the support of multi-dies are also considered.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
---
MAINTAINERS | 1 +
tests/unit/meson.build | 1 +
tests/unit/test-smp-parse.c | 892 ++++++++++++++++++++++++++++++++++++
3 files changed, 894 insertions(+)
create mode 100644 tests/unit/test-smp-parse.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 5510aaddfd..bc05fa8891 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1639,6 +1639,7 @@ F: include/hw/boards.h
F: include/hw/core/cpu.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 5736d285b2..e208173970 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.source_root() / 'hw/core/ptimer.c'],
'test-qapi-util': [],
+ 'test-smp-parse': [qom, meson.source_root() / 'hw/core/machine-smp.c'],
}
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..291fa3fa60
--- /dev/null
+++ b/tests/unit/test-smp-parse.c
@@ -0,0 +1,892 @@
+/*
+ * 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"
+
+#define T true
+#define F false
+
+#define MIN_CPUS 1
+#define MAX_CPUS 512
+
+/* define a CPU topology hierarchy of 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, \
+ }
+
+/* define a CPU topology hierarchy of 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, \
+ }
+
+/**
+ * SMPTestData:
+ * @config - the given SMP configuration
+ * @expect_prefer_sockets - expected topology result for the valid
+ * configuration, when sockets are preferred over cores in parsing
+ * @expect_prefer_cores - expected topology result for the valid
+ * configuration, when cores are preferred over sockets in parsing
+ * @expect_error - expected error report for the invalid configuration
+ */
+typedef struct SMPTestData {
+ SMPConfiguration config;
+ CpuTopology expect_prefer_sockets;
+ CpuTopology expect_prefer_cores;
+ const char *expect_error;
+} SMPTestData;
+
+/* specific machine type info for testing */
+static const TypeInfo smp_machine_info = {
+ .name = TYPE_MACHINE,
+ .parent = TYPE_OBJECT,
+ .class_size = sizeof(MachineClass),
+ .instance_size = sizeof(MachineState),
+};
+
+/*
+ * all possible valid collections of generic topology parameters
+ * and the corresponding expected outputs are listed.
+ */
+static struct SMPTestData data_generic[] = {
+ {
+ /* 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),
+ },
+};
+
+/*
+ * all possible valid collections of topology parameters (with dies)
+ * and the corresponding expected outputs are listed.
+ */
+static SMPTestData data_with_dies[] = {
+ {
+ /* config: -smp dies=2
+ * expect: cpus=2,sockets=1,dies=2,cores=1,threads=1,maxcpus=2 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(2, 1, 2, 1, 1, 2),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(2, 1, 2, 1, 1, 2),
+ }, {
+ /* config: -smp 16,dies=2
+ * prefer_sockets: cpus=16,sockets=8,dies=2,cores=1,threads=1,maxcpus=16
+ * prefer_cores: cpus=16,sockets=1,dies=2,cores=8,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 8, 2, 1, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 8, 1, 16),
+ }, {
+ /* config: -smp sockets=2,dies=2
+ * expect: cpus=4,sockets=2,dies=2,cores=1,threads=1,maxcpus=4 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(4, 2, 2, 1, 1, 4),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(4, 2, 2, 1, 1, 4),
+ }, {
+ /* config: -smp dies=2,cores=4
+ * expect: cpus=8,sockets=1,dies=2,cores=4,threads=1,maxcpus=8 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(8, 1, 2, 4, 1, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(8, 1, 2, 4, 1, 8),
+ }, {
+ /* config: -smp dies=2,threads=2
+ * expect: cpus=4,sockets=1,dies=2,cores=1,threads=2,maxcpus=4 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(4, 1, 2, 1, 2, 4),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(4, 1, 2, 1, 2, 4),
+ }, {
+ /* config: -smp dies=2,maxcpus=32
+ * prefer_sockets: cpus=32,sockets=16,dies=2,cores=1,threads=1,maxcpus=32
+ * prefer_cores: cpus=32,sockets=1,dies=2,cores=16,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, F, 0, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 16, 2, 1, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 1, 2, 16, 1, 32),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, F, 0, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ }, {
+ /* config: -smp 16,dies=2,cores=4
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ }, {
+ /* config: -smp 16,dies=2,threads=2
+ * prefer_sockets: cpus=16,sockets=4,dies=2,cores=1,threads=2,maxcpus=16
+ * prefer_cores: cpus=16,sockets=1,dies=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 4, 2, 1, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 16,dies=2,maxcpus=32
+ * prefer_sockets: cpus=16,sockets=16,dies=2,cores=1,threads=1,maxcpus=32
+ * prefer_cores: cpus=16,sockets=1,dies=2,cores=16,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, F, 0, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 16, 2, 1, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 16, 1, 32),
+ }, {
+ /* config: -smp sockets=2,dies=2,cores=4
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ }, {
+ /* config: -smp sockets=2,dies=2,threads=2
+ * expect: cpus=8,sockets=2,dies=2,cores=1,threads=2,maxcpus=8 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(8, 2, 2, 1, 2, 8),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(8, 2, 2, 1, 2, 8),
+ }, {
+ /* config: -smp sockets=2,dies=2,maxcpus=32
+ * expect: cpus=32,sockets=2,dies=2,cores=8,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, F, 0, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 8, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 8, 1, 32),
+ }, {
+ /* config: -smp dies=2,cores=4,threads=2
+ * expect: cpus=16,sockets=1,dies=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 4, 2, 16),
+ }, {
+ /* config: -smp dies=2,cores=4,maxcpus=32
+ * expect: cpus=32,sockets=4,dies=2,cores=4,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, T, 4, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 4, 2, 4, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 4, 2, 4, 1, 32),
+ }, {
+ /* config: -smp dies=2,threads=2,maxcpus=32
+ * prefer_sockets: cpus=32,sockets=8,dies=2,cores=1,threads=2,maxcpus=32
+ * prefer_cores: cpus=32,sockets=1,dies=2,cores=8,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, F, 0, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 8, 2, 1, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 1, 2, 8, 2, 32),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,cores=4
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, F, 0, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,threads=2
+ * expect: cpus=16,sockets=2,dies=2,cores=2,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, F, 0, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 2, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 2, 2, 16),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,maxcpus=32
+ * expect: cpus=16,sockets=2,dies=2,cores=8,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, F, 0, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 8, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 8, 1, 32),
+ }, {
+ /* config: -smp 16,dies=2,cores=4,threads=2
+ * expect: cpus=16,sockets=1,dies=2,cores=4,threads=2,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 4, 2, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 4, 2, 16),
+ }, {
+ /* config: -smp 16,dies=2,cores=4,maxcpus=32
+ * expect: cpus=16,sockets=4,dies=2,cores=4,threads=1,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, T, 4, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 4, 2, 4, 1, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 4, 2, 4, 1, 32),
+ }, {
+ /* config: -smp 16,dies=2,threads=2,maxcpus=32
+ * prefer_sockets: cpus=16,sockets=8,dies=2,cores=1,threads=2,maxcpus=32
+ * prefer_cores: cpus=16,sockets=1,dies=2,cores=8,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, F, 0, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 8, 2, 1, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 1, 2, 8, 2, 32),
+ }, {
+ /* config: -smp sockets=2,dies=2,cores=4,threads=2
+ * expect: cpus=32,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, T, 4, T, 2, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp sockets=2,dies=2,cores=4,maxcpus=32
+ * expect: cpus=32,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, T, 4, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp sockets=2,dies=2,threads=2,maxcpus=32
+ * expect: cpus=32,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, F, 0, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp dies=2,cores=4,threads=2,maxcpus=32
+ * expect: cpus=32,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, F, 0, T, 2, T, 4, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,cores=4,threads=1
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=1,maxcpus=16 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 1, F, 0),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 1, 16),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,cores=4,maxcpus=32
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, F, 0, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,threads=2,maxcpus=32
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, F, 0, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp 16,dies=2,cores=4,threads=2,maxcpus=32
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, F, 0, T, 2, T, 4, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp sockets=2,dies=2,cores=4,threads=2,maxcpus=32
+ * expect: cpus=32,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(F, 0, T, 2, T, 2, T, 4, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(32, 2, 2, 4, 2, 32),
+ }, {
+ /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=32
+ * expect: cpus=16,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ .config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 32),
+ .expect_prefer_sockets = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ .expect_prefer_cores = CPU_TOPOLOGY_WITH_DIES(16, 2, 2, 4, 2, 32),
+ },
+};
+
+/*
+ * generic invalid configurations
+ * parameters can't be specified as zero in the configuration.
+ * specified smp CPUs can't be less than supported min CPUs.
+ * specified max CPUs can't be more than supported max CPUs.
+ */
+#define NON_ZERO_ERROR_MSG "CPU topology parameters must be greater than zero"
+
+static SMPTestData data_generic_invalid[] = {
+ {
+ /* config: -smp 0 */
+ .config = SMP_CONFIG_GENERIC(T, 0, F, 0, F, 0, F, 0, F, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp 8,sockets=0 */
+ .config = SMP_CONFIG_GENERIC(T, 8, T, 0, F, 0, F, 0, F, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp 8,cores=0 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, T, 0, F, 0, F, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp 8,threads=0 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, T, 0, F, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp 8,maxcpus=0 */
+ .config = SMP_CONFIG_GENERIC(T, 8, F, 0, F, 0, F, 0, T, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp 8,dies=0 */
+ .config = SMP_CONFIG_WITH_DIES(T, 8, F, 0, T, 0, F, 0, F, 0, F, 0),
+ .expect_error = NON_ZERO_ERROR_MSG,
+ }, {
+ /* config: -smp MIN_CPUS
+ * reset the machine supported min CPUs to "MIN_CPUS + 1" */
+ .config = SMP_CONFIG_GENERIC(T, MIN_CPUS, 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 MAX_CPUS
+ * reset the machine supported max CPUs to "MAX_CPUS - 1" */
+ .config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
+ .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
+ "by machine '(null)' is 511",
+ },
+};
+
+static char *get_config_info(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 *get_topo_info(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_smp_parse(MachineState *ms, SMPConfiguration *config,
+ CpuTopology *expect_topo, const char *expect_err,
+ bool valid)
+{
+ g_autofree char *config_info = NULL;
+ g_autofree char *expect_topo_info = NULL;
+ g_autofree char *result_topo_info = NULL;
+ const char *result_err;
+ Error *err = NULL;
+
+ /* call the generic parser smp_parse() in hw/core/machine-smp.c */
+ smp_parse(ms, config, &err);
+
+ if (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;
+ }
+
+ config_info = get_config_info(config);
+ expect_topo_info = get_topo_info(expect_topo);
+
+ if (err != NULL) {
+ g_printerr("Check smp_parse failed:\n"
+ "config: %s\n"
+ "expect_topo: %s\n"
+ "should_be_valid: yes\n\n"
+ "result_is_valid: no\n"
+ "result_error: %s\n",
+ config_info, expect_topo_info,
+ error_get_pretty(err));
+ error_free(err);
+ } else {
+ result_topo_info = get_topo_info(&ms->smp);
+ g_printerr("Check smp_parse failed:\n"
+ "config: %s\n"
+ "expect_topo: %s\n"
+ "should_be_valid: yes\n\n"
+ "result_is_valid: yes\n"
+ "result_topo: %s\n",
+ config_info, expect_topo_info,
+ result_topo_info);
+ }
+ } else {
+ if (err != NULL) {
+ result_err = error_get_pretty(err);
+
+ if (expect_err == NULL || (expect_err != NULL &&
+ g_str_equal(expect_err, result_err))) {
+ error_free(err);
+ return;
+ }
+
+ config_info = get_config_info(config);
+ g_printerr("Check smp_parse failed:\n"
+ "config: %s\n"
+ "expect_error: %s\n"
+ "should_be_valid: no\n\n"
+ "result_is_valid: no\n"
+ "result_error: %s\n",
+ config_info, expect_err, result_err);
+ error_free(err);
+ } else {
+ config_info = get_config_info(config);
+ result_topo_info = get_topo_info(&ms->smp);
+
+ g_printerr("Check smp_parse failed:\n"
+ "config: %s\n"
+ "should_be_valid: no\n\n"
+ "result_is_valid: yes\n"
+ "result_topo: %s\n",
+ config_info, result_topo_info);
+ }
+ }
+
+ abort();
+}
+
+static void smp_test_data_init(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;
+}
+
+static void smp_machine_class_reinit(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 smp_generic_test(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData data;
+ int i;
+
+ /* make sure that we have created the object */
+ g_assert_nonnull(ms);
+ g_assert_nonnull(mc);
+
+ /* reinitialize related machine properties before each subtest */
+ smp_machine_class_reinit(mc);
+
+ for (i = 0; i < ARRAY_SIZE(data_generic); i++) {
+ smp_test_data_init(&data, &data_generic[i]);
+
+ /* parsed values of unsupported parameters should be 1 */
+ data.expect_prefer_sockets.dies = 1;
+ data.expect_prefer_cores.dies = 1;
+
+ mc->smp_props.prefer_sockets = true;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_sockets, NULL, true);
+
+ mc->smp_props.prefer_sockets = false;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_cores, NULL, true);
+
+ /*
+ * it's now allowed that unsupported dies can be set equal to 1
+ * in the SMP configuration.
+ */
+ data.config.has_dies = true;
+ data.config.dies = 1;
+
+ mc->smp_props.prefer_sockets = true;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_sockets, NULL, true);
+
+ mc->smp_props.prefer_sockets = false;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_cores, NULL, true);
+ }
+
+ object_unref(obj);
+}
+
+static void smp_with_dies_test(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData data;
+ int i;
+
+ /* make sure that we have created the object */
+ g_assert_nonnull(ms);
+ g_assert_nonnull(mc);
+
+ /* reinitialize related machine properties before each subtest */
+ smp_machine_class_reinit(mc);
+
+ mc->smp_props.dies_supported = true;
+
+ for (i = 0; i < ARRAY_SIZE(data_generic); i++) {
+ smp_test_data_init(&data, &data_generic[i]);
+
+ /* omitted arch-specific dies should directly default to 1 */
+ data.expect_prefer_sockets.dies = 1;
+ data.expect_prefer_cores.dies = 1;
+
+ mc->smp_props.prefer_sockets = true;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_sockets, NULL, true);
+
+ mc->smp_props.prefer_sockets = false;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_cores, NULL, true);
+ }
+
+ /* when dies is provided in the configuration */
+ for (i = 0; i < ARRAY_SIZE(data_with_dies); i++) {
+ smp_test_data_init(&data, &data_with_dies[i]);
+
+ mc->smp_props.prefer_sockets = true;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_sockets, NULL, true);
+
+ mc->smp_props.prefer_sockets = false;
+ check_smp_parse(ms, &data.config,
+ &data.expect_prefer_cores, NULL, true);
+ }
+
+ object_unref(obj);
+}
+
+static void acceptance_generic_test(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData *datap;
+ int i;
+
+ /* make sure that we have created the object */
+ g_assert_nonnull(ms);
+ g_assert_nonnull(mc);
+
+ /* reinitialize related machine properties before each subtest */
+ smp_machine_class_reinit(mc);
+
+ /* reset the machine supported min CPUs and max CPUs */
+ mc->min_cpus = MIN_CPUS + 1;
+ mc->max_cpus = MAX_CPUS - 1;
+
+ for (i = 0; i < ARRAY_SIZE(data_generic_invalid); i++) {
+ datap = &data_generic_invalid[i];
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, false);
+ }
+
+ /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
+ datap = &(SMPTestData){
+ .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)",
+ };
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, false);
+
+ /* config: -smp 18,sockets=2,cores=4,threads=2,maxcpus=16 */
+ datap = &(SMPTestData){
+ .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)",
+ };
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, false);
+
+ /* config: -smp 8,dies=2 */
+ datap = &(SMPTestData){
+ .config = SMP_CONFIG_WITH_DIES(T, 8, F, 0, T, 2, F, 0, F, 0, F, 0),
+ .expect_error = "dies not supported by this machine's CPU topology",
+ };
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, false);
+
+ object_unref(obj);
+}
+
+static void acceptance_with_dies_test(void)
+{
+ Object *obj = object_new(TYPE_MACHINE);
+ MachineState *ms = MACHINE(obj);
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ SMPTestData *datap;
+
+ /* make sure that we have created the object */
+ g_assert_nonnull(ms);
+ g_assert_nonnull(mc);
+
+ /* reinitialize related machine properties before each subtest */
+ smp_machine_class_reinit(mc);
+
+ mc->smp_props.dies_supported = true;
+
+ /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
+ datap = &(SMPTestData){
+ .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)",
+ };
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, false);
+
+ /* config: -smp 34,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
+ datap = &(SMPTestData){
+ .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)",
+ };
+ check_smp_parse(ms, &datap->config, NULL, datap->expect_error, 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/smp-generic", smp_generic_test);
+ g_test_add_func("/test-smp-parse/smp-with-dies", smp_with_dies_test);
+ g_test_add_func("/test-smp-parse/acceptance-generic",
+ acceptance_generic_test);
+ g_test_add_func("/test-smp-parse/acceptance-with-dies",
+ acceptance_with_dies_test);
+
+ g_test_run();
+
+ return 0;
+}
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-13 2:39 ` [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2 Yanan Wang
@ 2021-08-13 16:30 ` Pankaj Gupta
2021-08-16 3:02 ` wangyanan (Y)
2021-08-16 8:02 ` Pankaj Gupta
1 sibling, 1 reply; 20+ messages in thread
From: Pankaj Gupta @ 2021-08-13 16:30 UTC (permalink / raw)
To: Yanan Wang
Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin,
Cornelia Huck, Richard Henderson, Qemu Developers, Greg Kurz,
Halil Pasic, wanghaibin.wang, Thomas Huth, Paolo Bonzini,
David Gibson
> In the real SMP hardware topology world, it's much more likely that
> we have high cores-per-socket counts and few sockets totally. While
> the current preference of sockets over cores in smp parsing results
> in a virtual cpu topology with low cores-per-sockets counts and a
> large number of sockets, which is just contrary to the real world.
>
> Given that it is better to make the virtual cpu topology be more
> reflective of the real world and also for the sake of compatibility,
> we start to prefer cores over sockets over threads in smp parsing
> since machine type 6.2 for different arches.
>
> In this patch, a boolean "smp_prefer_sockets" is added, and we only
> enable the old preference on older machines and enable the new one
> since type 6.2 for all arches by using the machine compat mechanism.
>
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> ---
> hw/arm/virt.c | 1 +
> hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
> hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
> hw/i386/pc_piix.c | 1 +
> hw/i386/pc_q35.c | 1 +
> hw/ppc/spapr.c | 1 +
> hw/s390x/s390-virtio-ccw.c | 1 +
> include/hw/boards.h | 1 +
> qemu-options.hx | 3 ++-
> 9 files changed, 60 insertions(+), 19 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 01165f7f53..7babea40dc 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
> {
> virt_machine_6_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
Will this be set for virt_machine_6_2_?
> }
> DEFINE_VIRT_MACHINE(6, 1)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index bdce80df32..15b41c52e8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>
> 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 cores = config->has_cores ? config->cores : 0;
> @@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> return;
> }
>
> - /* compute missing values, prefer sockets over cores over threads */
> + /* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> @@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> } else {
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - if (sockets == 0) {
> - cores = cores > 0 ? cores : 1;
> - threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (cores * threads);
> - } else if (cores == 0) {
> - threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (sockets * threads);
> - } else if (threads == 0) {
> + if (mc->smp_prefer_sockets) {
> + /* prefer sockets over cores before 6.2 */
> + if (sockets == 0) {
> + cores = cores > 0 ? cores : 1;
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (cores * threads);
> + } else if (cores == 0) {
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * 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 * threads);
> + } else if (sockets == 0) {
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (cores * threads);
> + }
> + }
> +
> + /* try to calculate omitted threads at last */
> + if (threads == 0) {
> threads = maxcpus / (sockets * cores);
> }
> }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index afd8b9c283..4b05ff7160 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
> */
> static void pc_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;
> @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> /* directly default dies to 1 if it's omitted */
> dies = dies > 0 ? dies : 1;
>
> - /* compute missing values, prefer sockets over cores over threads */
> + /* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> @@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> } else {
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - 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 if (threads == 0) {
> + if (mc->smp_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);
> }
> }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fd5c2277f2..9b811fc6ca 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
> m->is_default = false;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b45903b15e..88efb7fde4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
> m->alias = NULL;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d39fd4e644..a481fade51 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
> {
> spapr_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
>
> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4d25278cf2..b40e647883 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
> {
> ccw_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
> DEFINE_CCW_MACHINE(6_1, "6.1", false);
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 463a5514f9..2ae039b74f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -247,6 +247,7 @@ struct MachineClass {
> bool nvdimm_supported;
> bool numa_mem_supported;
> bool auto_enable_numa;
> + bool smp_prefer_sockets;
> const char *default_ram_id;
>
> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 06f819177e..451d2cd817 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -238,7 +238,8 @@ SRST
> Historically preference was given to the coarsest topology parameters
> when computing missing values (ie sockets preferred over cores, which
> were preferred over threads), however, this behaviour is considered
> - liable to change.
> + liable to change. Prior to 6.2 the preference was sockets over cores
> + over threads. Since 6.2 the preference is cores over sockets over threads.
> ERST
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-13 16:30 ` Pankaj Gupta
@ 2021-08-16 3:02 ` wangyanan (Y)
2021-08-16 8:00 ` Pankaj Gupta
0 siblings, 1 reply; 20+ messages in thread
From: wangyanan (Y) @ 2021-08-16 3:02 UTC (permalink / raw)
To: Pankaj Gupta
Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin,
Cornelia Huck, Richard Henderson, Qemu Developers, Greg Kurz,
Halil Pasic, wanghaibin.wang, Thomas Huth, Paolo Bonzini,
David Gibson
On 2021/8/14 0:30, Pankaj Gupta wrote:
>> In the real SMP hardware topology world, it's much more likely that
>> we have high cores-per-socket counts and few sockets totally. While
>> the current preference of sockets over cores in smp parsing results
>> in a virtual cpu topology with low cores-per-sockets counts and a
>> large number of sockets, which is just contrary to the real world.
>>
>> Given that it is better to make the virtual cpu topology be more
>> reflective of the real world and also for the sake of compatibility,
>> we start to prefer cores over sockets over threads in smp parsing
>> since machine type 6.2 for different arches.
>>
>> In this patch, a boolean "smp_prefer_sockets" is added, and we only
>> enable the old preference on older machines and enable the new one
>> since type 6.2 for all arches by using the machine compat mechanism.
>>
>> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> ---
>> hw/arm/virt.c | 1 +
>> hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
>> hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
>> hw/i386/pc_piix.c | 1 +
>> hw/i386/pc_q35.c | 1 +
>> hw/ppc/spapr.c | 1 +
>> hw/s390x/s390-virtio-ccw.c | 1 +
>> include/hw/boards.h | 1 +
>> qemu-options.hx | 3 ++-
>> 9 files changed, 60 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 01165f7f53..7babea40dc 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
>> {
>> virt_machine_6_2_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
> Will this be set for virt_machine_6_2_?
>
Hi,
We hope to start to assume cores over sockets on virt_6_2 machines
and the later, and keep the old style of assuming sockets over cores
on virt_6_1 machines and the older. Is there any concern here?
Thanks,
Yanan
>> }
>> DEFINE_VIRT_MACHINE(6, 1)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index bdce80df32..15b41c52e8 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>>
>> 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 cores = config->has_cores ? config->cores : 0;
>> @@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
>> return;
>> }
>>
>> - /* compute missing values, prefer sockets over cores over threads */
>> + /* compute missing values based on the provided ones */
>> if (cpus == 0 && maxcpus == 0) {
>> sockets = sockets > 0 ? sockets : 1;
>> cores = cores > 0 ? cores : 1;
>> @@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
>> } else {
>> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>>
>> - if (sockets == 0) {
>> - cores = cores > 0 ? cores : 1;
>> - threads = threads > 0 ? threads : 1;
>> - sockets = maxcpus / (cores * threads);
>> - } else if (cores == 0) {
>> - threads = threads > 0 ? threads : 1;
>> - cores = maxcpus / (sockets * threads);
>> - } else if (threads == 0) {
>> + if (mc->smp_prefer_sockets) {
>> + /* prefer sockets over cores before 6.2 */
>> + if (sockets == 0) {
>> + cores = cores > 0 ? cores : 1;
>> + threads = threads > 0 ? threads : 1;
>> + sockets = maxcpus / (cores * threads);
>> + } else if (cores == 0) {
>> + threads = threads > 0 ? threads : 1;
>> + cores = maxcpus / (sockets * 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 * threads);
>> + } else if (sockets == 0) {
>> + threads = threads > 0 ? threads : 1;
>> + sockets = maxcpus / (cores * threads);
>> + }
>> + }
>> +
>> + /* try to calculate omitted threads at last */
>> + if (threads == 0) {
>> threads = maxcpus / (sockets * cores);
>> }
>> }
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index afd8b9c283..4b05ff7160 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>> */
>> static void pc_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;
>> @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
>> /* directly default dies to 1 if it's omitted */
>> dies = dies > 0 ? dies : 1;
>>
>> - /* compute missing values, prefer sockets over cores over threads */
>> + /* compute missing values based on the provided ones */
>> if (cpus == 0 && maxcpus == 0) {
>> sockets = sockets > 0 ? sockets : 1;
>> cores = cores > 0 ? cores : 1;
>> @@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
>> } else {
>> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>>
>> - 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 if (threads == 0) {
>> + if (mc->smp_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);
>> }
>> }
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index fd5c2277f2..9b811fc6ca 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
>> m->is_default = false;
>> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
>> + m->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index b45903b15e..88efb7fde4 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
>> m->alias = NULL;
>> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
>> + m->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d39fd4e644..a481fade51 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
>> {
>> spapr_machine_6_2_class_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 4d25278cf2..b40e647883 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
>> {
>> ccw_machine_6_2_class_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
>> }
>> DEFINE_CCW_MACHINE(6_1, "6.1", false);
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 463a5514f9..2ae039b74f 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -247,6 +247,7 @@ struct MachineClass {
>> bool nvdimm_supported;
>> bool numa_mem_supported;
>> bool auto_enable_numa;
>> + bool smp_prefer_sockets;
>> const char *default_ram_id;
>>
>> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 06f819177e..451d2cd817 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -238,7 +238,8 @@ SRST
>> Historically preference was given to the coarsest topology parameters
>> when computing missing values (ie sockets preferred over cores, which
>> were preferred over threads), however, this behaviour is considered
>> - liable to change.
>> + liable to change. Prior to 6.2 the preference was sockets over cores
>> + over threads. Since 6.2 the preference is cores over sockets over threads.
>> ERST
>>
>> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>> --
>> 2.19.1
>>
> .
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-16 3:02 ` wangyanan (Y)
@ 2021-08-16 8:00 ` Pankaj Gupta
0 siblings, 0 replies; 20+ messages in thread
From: Pankaj Gupta @ 2021-08-16 8:00 UTC (permalink / raw)
To: wangyanan (Y)
Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin,
Cornelia Huck, Richard Henderson, Qemu Developers, Greg Kurz,
Halil Pasic, wanghaibin.wang, Thomas Huth, Paolo Bonzini,
David Gibson
> On 2021/8/14 0:30, Pankaj Gupta wrote:
> >> In the real SMP hardware topology world, it's much more likely that
> >> we have high cores-per-socket counts and few sockets totally. While
> >> the current preference of sockets over cores in smp parsing results
> >> in a virtual cpu topology with low cores-per-sockets counts and a
> >> large number of sockets, which is just contrary to the real world.
> >>
> >> Given that it is better to make the virtual cpu topology be more
> >> reflective of the real world and also for the sake of compatibility,
> >> we start to prefer cores over sockets over threads in smp parsing
> >> since machine type 6.2 for different arches.
> >>
> >> In this patch, a boolean "smp_prefer_sockets" is added, and we only
> >> enable the old preference on older machines and enable the new one
> >> since type 6.2 for all arches by using the machine compat mechanism.
> >>
> >> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> >> Acked-by: Cornelia Huck <cohuck@redhat.com>
> >> Reviewed-by: Andrew Jones <drjones@redhat.com>
> >> ---
> >> hw/arm/virt.c | 1 +
> >> hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
> >> hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
> >> hw/i386/pc_piix.c | 1 +
> >> hw/i386/pc_q35.c | 1 +
> >> hw/ppc/spapr.c | 1 +
> >> hw/s390x/s390-virtio-ccw.c | 1 +
> >> include/hw/boards.h | 1 +
> >> qemu-options.hx | 3 ++-
> >> 9 files changed, 60 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index 01165f7f53..7babea40dc 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
> >> {
> >> virt_machine_6_2_options(mc);
> >> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> >> + mc->smp_prefer_sockets = true;
> > Will this be set for virt_machine_6_2_?
> >
> Hi,
>
> We hope to start to assume cores over sockets on virt_6_2 machines
> and the later, and keep the old style of assuming sockets over cores
> on virt_6_1 machines and the older. Is there any concern here?
O.k. Thank you for clarifying.
Best regards,
Pankaj
>
> Thanks,
> Yanan
> >> }
> >> DEFINE_VIRT_MACHINE(6, 1)
> >>
> >> diff --git a/hw/core/machine.c b/hw/core/machine.c
> >> index bdce80df32..15b41c52e8 100644
> >> --- a/hw/core/machine.c
> >> +++ b/hw/core/machine.c
> >> @@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
> >>
> >> 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 cores = config->has_cores ? config->cores : 0;
> >> @@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> >> return;
> >> }
> >>
> >> - /* compute missing values, prefer sockets over cores over threads */
> >> + /* compute missing values based on the provided ones */
> >> if (cpus == 0 && maxcpus == 0) {
> >> sockets = sockets > 0 ? sockets : 1;
> >> cores = cores > 0 ? cores : 1;
> >> @@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> >> } else {
> >> maxcpus = maxcpus > 0 ? maxcpus : cpus;
> >>
> >> - if (sockets == 0) {
> >> - cores = cores > 0 ? cores : 1;
> >> - threads = threads > 0 ? threads : 1;
> >> - sockets = maxcpus / (cores * threads);
> >> - } else if (cores == 0) {
> >> - threads = threads > 0 ? threads : 1;
> >> - cores = maxcpus / (sockets * threads);
> >> - } else if (threads == 0) {
> >> + if (mc->smp_prefer_sockets) {
> >> + /* prefer sockets over cores before 6.2 */
> >> + if (sockets == 0) {
> >> + cores = cores > 0 ? cores : 1;
> >> + threads = threads > 0 ? threads : 1;
> >> + sockets = maxcpus / (cores * threads);
> >> + } else if (cores == 0) {
> >> + threads = threads > 0 ? threads : 1;
> >> + cores = maxcpus / (sockets * 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 * threads);
> >> + } else if (sockets == 0) {
> >> + threads = threads > 0 ? threads : 1;
> >> + sockets = maxcpus / (cores * threads);
> >> + }
> >> + }
> >> +
> >> + /* try to calculate omitted threads at last */
> >> + if (threads == 0) {
> >> threads = maxcpus / (sockets * cores);
> >> }
> >> }
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index afd8b9c283..4b05ff7160 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
> >> */
> >> static void pc_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;
> >> @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> >> /* directly default dies to 1 if it's omitted */
> >> dies = dies > 0 ? dies : 1;
> >>
> >> - /* compute missing values, prefer sockets over cores over threads */
> >> + /* compute missing values based on the provided ones */
> >> if (cpus == 0 && maxcpus == 0) {
> >> sockets = sockets > 0 ? sockets : 1;
> >> cores = cores > 0 ? cores : 1;
> >> @@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> >> } else {
> >> maxcpus = maxcpus > 0 ? maxcpus : cpus;
> >>
> >> - 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 if (threads == 0) {
> >> + if (mc->smp_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);
> >> }
> >> }
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index fd5c2277f2..9b811fc6ca 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
> >> m->is_default = false;
> >> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> >> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> >> + m->smp_prefer_sockets = true;
> >> }
> >>
> >> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index b45903b15e..88efb7fde4 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
> >> m->alias = NULL;
> >> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> >> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> >> + m->smp_prefer_sockets = true;
> >> }
> >>
> >> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> >> index d39fd4e644..a481fade51 100644
> >> --- a/hw/ppc/spapr.c
> >> +++ b/hw/ppc/spapr.c
> >> @@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
> >> {
> >> spapr_machine_6_2_class_options(mc);
> >> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> >> + mc->smp_prefer_sockets = true;
> >> }
> >>
> >> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >> index 4d25278cf2..b40e647883 100644
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
> >> {
> >> ccw_machine_6_2_class_options(mc);
> >> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> >> + mc->smp_prefer_sockets = true;
> >> }
> >> DEFINE_CCW_MACHINE(6_1, "6.1", false);
> >>
> >> diff --git a/include/hw/boards.h b/include/hw/boards.h
> >> index 463a5514f9..2ae039b74f 100644
> >> --- a/include/hw/boards.h
> >> +++ b/include/hw/boards.h
> >> @@ -247,6 +247,7 @@ struct MachineClass {
> >> bool nvdimm_supported;
> >> bool numa_mem_supported;
> >> bool auto_enable_numa;
> >> + bool smp_prefer_sockets;
> >> const char *default_ram_id;
> >>
> >> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> >> diff --git a/qemu-options.hx b/qemu-options.hx
> >> index 06f819177e..451d2cd817 100644
> >> --- a/qemu-options.hx
> >> +++ b/qemu-options.hx
> >> @@ -238,7 +238,8 @@ SRST
> >> Historically preference was given to the coarsest topology parameters
> >> when computing missing values (ie sockets preferred over cores, which
> >> were preferred over threads), however, this behaviour is considered
> >> - liable to change.
> >> + liable to change. Prior to 6.2 the preference was sockets over cores
> >> + over threads. Since 6.2 the preference is cores over sockets over threads.
> >> ERST
> >>
> >> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> >> --
> >> 2.19.1
> >>
> > .
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-13 2:39 ` [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2 Yanan Wang
2021-08-13 16:30 ` Pankaj Gupta
@ 2021-08-16 8:02 ` Pankaj Gupta
2021-08-16 9:15 ` wangyanan (Y)
1 sibling, 1 reply; 20+ messages in thread
From: Pankaj Gupta @ 2021-08-16 8:02 UTC (permalink / raw)
To: Yanan Wang
Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin,
Cornelia Huck, Richard Henderson, Qemu Developers, Greg Kurz,
Halil Pasic, wanghaibin.wang, Thomas Huth, Paolo Bonzini,
David Gibson
> In the real SMP hardware topology world, it's much more likely that
> we have high cores-per-socket counts and few sockets totally. While
> the current preference of sockets over cores in smp parsing results
> in a virtual cpu topology with low cores-per-sockets counts and a
> large number of sockets, which is just contrary to the real world.
>
> Given that it is better to make the virtual cpu topology be more
> reflective of the real world and also for the sake of compatibility,
> we start to prefer cores over sockets over threads in smp parsing
> since machine type 6.2 for different arches.
>
> In this patch, a boolean "smp_prefer_sockets" is added, and we only
> enable the old preference on older machines and enable the new one
> since type 6.2 for all arches by using the machine compat mechanism.
>
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Acked-by: Cornelia Huck <cohuck@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> ---
> hw/arm/virt.c | 1 +
> hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
> hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
> hw/i386/pc_piix.c | 1 +
> hw/i386/pc_q35.c | 1 +
> hw/ppc/spapr.c | 1 +
> hw/s390x/s390-virtio-ccw.c | 1 +
> include/hw/boards.h | 1 +
> qemu-options.hx | 3 ++-
> 9 files changed, 60 insertions(+), 19 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 01165f7f53..7babea40dc 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
> {
> virt_machine_6_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
> DEFINE_VIRT_MACHINE(6, 1)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index bdce80df32..15b41c52e8 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>
> 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 cores = config->has_cores ? config->cores : 0;
> @@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> return;
> }
>
> - /* compute missing values, prefer sockets over cores over threads */
> + /* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> @@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
> } else {
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - if (sockets == 0) {
> - cores = cores > 0 ? cores : 1;
> - threads = threads > 0 ? threads : 1;
> - sockets = maxcpus / (cores * threads);
> - } else if (cores == 0) {
> - threads = threads > 0 ? threads : 1;
> - cores = maxcpus / (sockets * threads);
> - } else if (threads == 0) {
> + if (mc->smp_prefer_sockets) {
> + /* prefer sockets over cores before 6.2 */
> + if (sockets == 0) {
> + cores = cores > 0 ? cores : 1;
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (cores * threads);
> + } else if (cores == 0) {
> + threads = threads > 0 ? threads : 1;
> + cores = maxcpus / (sockets * 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 * threads);
> + } else if (sockets == 0) {
> + threads = threads > 0 ? threads : 1;
> + sockets = maxcpus / (cores * threads);
> + }
> + }
> +
> + /* try to calculate omitted threads at last */
> + if (threads == 0) {
> threads = maxcpus / (sockets * cores);
> }
> }
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index afd8b9c283..4b05ff7160 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
> */
> static void pc_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;
> @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> /* directly default dies to 1 if it's omitted */
> dies = dies > 0 ? dies : 1;
>
> - /* compute missing values, prefer sockets over cores over threads */
> + /* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> @@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
> } else {
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> - 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 if (threads == 0) {
> + if (mc->smp_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);
> }
> }
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index fd5c2277f2..9b811fc6ca 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
> m->is_default = false;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b45903b15e..88efb7fde4 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
> m->alias = NULL;
> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
> + m->smp_prefer_sockets = true;
> }
>
> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d39fd4e644..a481fade51 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
> {
> spapr_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
>
> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 4d25278cf2..b40e647883 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
> {
> ccw_machine_6_2_class_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> + mc->smp_prefer_sockets = true;
> }
> DEFINE_CCW_MACHINE(6_1, "6.1", false);
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 463a5514f9..2ae039b74f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -247,6 +247,7 @@ struct MachineClass {
> bool nvdimm_supported;
> bool numa_mem_supported;
> bool auto_enable_numa;
> + bool smp_prefer_sockets;
> const char *default_ram_id;
>
> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 06f819177e..451d2cd817 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -238,7 +238,8 @@ SRST
> Historically preference was given to the coarsest topology parameters
> when computing missing values (ie sockets preferred over cores, which
> were preferred over threads), however, this behaviour is considered
> - liable to change.
> + liable to change. Prior to 6.2 the preference was sockets over cores
> + over threads. Since 6.2 the preference is cores over sockets over threads.
> ERST
>
> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
> --
Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
2021-08-16 8:02 ` Pankaj Gupta
@ 2021-08-16 9:15 ` wangyanan (Y)
0 siblings, 0 replies; 20+ messages in thread
From: wangyanan (Y) @ 2021-08-16 9:15 UTC (permalink / raw)
To: Pankaj Gupta
Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé,
Eduardo Habkost, Pierre Morel, Michael S . Tsirkin,
Cornelia Huck, Richard Henderson, Qemu Developers, Greg Kurz,
Halil Pasic, wanghaibin.wang, Thomas Huth, Paolo Bonzini,
David Gibson
On 2021/8/16 16:02, Pankaj Gupta wrote:
>> In the real SMP hardware topology world, it's much more likely that
>> we have high cores-per-socket counts and few sockets totally. While
>> the current preference of sockets over cores in smp parsing results
>> in a virtual cpu topology with low cores-per-sockets counts and a
>> large number of sockets, which is just contrary to the real world.
>>
>> Given that it is better to make the virtual cpu topology be more
>> reflective of the real world and also for the sake of compatibility,
>> we start to prefer cores over sockets over threads in smp parsing
>> since machine type 6.2 for different arches.
>>
>> In this patch, a boolean "smp_prefer_sockets" is added, and we only
>> enable the old preference on older machines and enable the new one
>> since type 6.2 for all arches by using the machine compat mechanism.
>>
>> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> Acked-by: David Gibson <david@gibson.dropbear.id.au>
>> Acked-by: Cornelia Huck <cohuck@redhat.com>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> ---
>> hw/arm/virt.c | 1 +
>> hw/core/machine.c | 35 ++++++++++++++++++++++++++---------
>> hw/i386/pc.c | 35 ++++++++++++++++++++++++++---------
>> hw/i386/pc_piix.c | 1 +
>> hw/i386/pc_q35.c | 1 +
>> hw/ppc/spapr.c | 1 +
>> hw/s390x/s390-virtio-ccw.c | 1 +
>> include/hw/boards.h | 1 +
>> qemu-options.hx | 3 ++-
>> 9 files changed, 60 insertions(+), 19 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 01165f7f53..7babea40dc 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
>> {
>> virt_machine_6_2_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
>> }
>> DEFINE_VIRT_MACHINE(6, 1)
>>
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index bdce80df32..15b41c52e8 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -748,6 +748,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>>
>> 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 cores = config->has_cores ? config->cores : 0;
>> @@ -759,7 +760,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
>> return;
>> }
>>
>> - /* compute missing values, prefer sockets over cores over threads */
>> + /* compute missing values based on the provided ones */
>> if (cpus == 0 && maxcpus == 0) {
>> sockets = sockets > 0 ? sockets : 1;
>> cores = cores > 0 ? cores : 1;
>> @@ -767,14 +768,30 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
>> } else {
>> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>>
>> - if (sockets == 0) {
>> - cores = cores > 0 ? cores : 1;
>> - threads = threads > 0 ? threads : 1;
>> - sockets = maxcpus / (cores * threads);
>> - } else if (cores == 0) {
>> - threads = threads > 0 ? threads : 1;
>> - cores = maxcpus / (sockets * threads);
>> - } else if (threads == 0) {
>> + if (mc->smp_prefer_sockets) {
>> + /* prefer sockets over cores before 6.2 */
>> + if (sockets == 0) {
>> + cores = cores > 0 ? cores : 1;
>> + threads = threads > 0 ? threads : 1;
>> + sockets = maxcpus / (cores * threads);
>> + } else if (cores == 0) {
>> + threads = threads > 0 ? threads : 1;
>> + cores = maxcpus / (sockets * 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 * threads);
>> + } else if (sockets == 0) {
>> + threads = threads > 0 ? threads : 1;
>> + sockets = maxcpus / (cores * threads);
>> + }
>> + }
>> +
>> + /* try to calculate omitted threads at last */
>> + if (threads == 0) {
>> threads = maxcpus / (sockets * cores);
>> }
>> }
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index afd8b9c283..4b05ff7160 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>> */
>> static void pc_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;
>> @@ -727,7 +728,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
>> /* directly default dies to 1 if it's omitted */
>> dies = dies > 0 ? dies : 1;
>>
>> - /* compute missing values, prefer sockets over cores over threads */
>> + /* compute missing values based on the provided ones */
>> if (cpus == 0 && maxcpus == 0) {
>> sockets = sockets > 0 ? sockets : 1;
>> cores = cores > 0 ? cores : 1;
>> @@ -735,14 +736,30 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
>> } else {
>> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>>
>> - 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 if (threads == 0) {
>> + if (mc->smp_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);
>> }
>> }
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index fd5c2277f2..9b811fc6ca 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -432,6 +432,7 @@ static void pc_i440fx_6_1_machine_options(MachineClass *m)
>> m->is_default = false;
>> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
>> + m->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_I440FX_MACHINE(v6_1, "pc-i440fx-6.1", NULL,
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index b45903b15e..88efb7fde4 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -372,6 +372,7 @@ static void pc_q35_6_1_machine_options(MachineClass *m)
>> m->alias = NULL;
>> compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
>> + m->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL,
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index d39fd4e644..a481fade51 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4702,6 +4702,7 @@ static void spapr_machine_6_1_class_options(MachineClass *mc)
>> {
>> spapr_machine_6_2_class_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
>> }
>>
>> DEFINE_SPAPR_MACHINE(6_1, "6.1", false);
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 4d25278cf2..b40e647883 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -809,6 +809,7 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
>> {
>> ccw_machine_6_2_class_options(mc);
>> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
>> + mc->smp_prefer_sockets = true;
>> }
>> DEFINE_CCW_MACHINE(6_1, "6.1", false);
>>
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 463a5514f9..2ae039b74f 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -247,6 +247,7 @@ struct MachineClass {
>> bool nvdimm_supported;
>> bool numa_mem_supported;
>> bool auto_enable_numa;
>> + bool smp_prefer_sockets;
>> const char *default_ram_id;
>>
>> HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 06f819177e..451d2cd817 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -238,7 +238,8 @@ SRST
>> Historically preference was given to the coarsest topology parameters
>> when computing missing values (ie sockets preferred over cores, which
>> were preferred over threads), however, this behaviour is considered
>> - liable to change.
>> + liable to change. Prior to 6.2 the preference was sockets over cores
>> + over threads. Since 6.2 the preference is cores over sockets over threads.
>> ERST
>>
>> DEF("numa", HAS_ARG, QEMU_OPTION_numa,
>> --
> Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
> .
Thanks!
Yanan
.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2021-08-16 9:17 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 2:38 [PATCH for-6.2 v5 00/14] machine: smp parsing fixes and improvement Yanan Wang
2021-08-13 2:38 ` [PATCH for-6.2 v5 01/14] machine: Minor refactor/cleanup for the smp parsers Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 02/14] machine: Uniformly use maxcpus to calculate the omitted parameters Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 03/14] machine: Set the value of cpus to match maxcpus if it's omitted Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 04/14] machine: Improve the error reporting of smp parsing Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 05/14] hw: Add compat machines for 6.2 Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 06/14] machine: Prefer cores over sockets in smp parsing since 6.2 Yanan Wang
2021-08-13 16:30 ` Pankaj Gupta
2021-08-16 3:02 ` wangyanan (Y)
2021-08-16 8:00 ` Pankaj Gupta
2021-08-16 8:02 ` Pankaj Gupta
2021-08-16 9:15 ` wangyanan (Y)
2021-08-13 2:39 ` [PATCH for-6.2 v5 07/14] machine: Use ms instead of global current_machine in sanity-check Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 08/14] machine: Tweak the order of topology members in struct CpuTopology Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 09/14] machine: Make smp_parse generic enough for all arches Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 10/14] machine: Remove smp_parse callback from MachineClass Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 11/14] machine: Move smp_prefer_sockets to struct SMPCompatProps Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 12/14] machine: Put all sanity-check in the generic SMP parser Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 13/14] machine: Split out the smp parsing code Yanan Wang
2021-08-13 2:39 ` [PATCH for-6.2 v5 14/14] tests/unit: Add a unit test for smp parsing Yanan Wang
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.