mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

hw/core: allow parameter=1 for SMP topology on any machine

Browse Source 
This effectively reverts

  commit 54c4ea8f3a
  Author: Zhao Liu <zhao1.liu@intel.com>
  Date:   Sat Mar 9 00:01:37 2024 +0800

    hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations

but is not done as a 'git revert' since the part of the changes to the
file hw/core/machine-smp.c which add 'has_XXX' checks remain desirable.
Furthermore, we have to tweak the subsequently added unit test to
account for differing warning message.

The rationale for the original deprecation was:

  "Currently, it was allowed for users to specify the unsupported
   topology parameter as "1". For example, x86 PC machine doesn't
   support drawer/book/cluster topology levels, but user could specify
   "-smp drawers=1,books=1,clusters=1".

   This is meaningless and confusing, so that the support for this kind
   of configurations is marked deprecated since 9.0."

There are varying POVs on the topic of 'unsupported' topology levels.

It is common to say that on a system without hyperthreading, that there
is always 1 thread. Likewise when new CPUs introduced a concept of
multiple "dies', it was reasonable to say that all historical CPUs
before that implicitly had 1 'die'. Likewise for the more recently
introduced 'modules' and 'clusters' parameter'. From this POV, it is
valid to set 'parameter=1' on the -smp command line for any machine,
only a value > 1 is strictly an error condition.

It doesn't cause any functional difficulty for QEMU, because internally
the QEMU code is itself assuming that all "unsupported" parameters
implicitly have a value of '1'.

At the libvirt level, we've allowed applications to set 'parameter=1'
when configuring a guest, and pass that through to QEMU.

Deprecating this creates extra difficulty for because there's no info
exposed from QEMU about which machine types "support" which parameters.
Thus, libvirt can't know whether it is valid to pass 'parameter=1' for
a given machine type, or whether it will trigger deprecation messages.

Since there's no apparent functional benefit to deleting this deprecated
behaviour from QEMU, and it creates problems for consumers of QEMU,
remove this deprecation.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Message-ID: <20240513123358.612355-2-berrange@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
This commit is contained in:
Daniel P. Berrangé 2024-05-13 13:33:57 +01:00 committed by Philippe Mathieu-Daudé
parent 2563be6317
commit 9d7950edb0
2 changed files with 31 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
hw/core/machine-smp.c
View File

@ -118,76 +118,46 @@ void machine_parse_smp_config(MachineState *ms,
}
/*
* If not supported by the machine, a topology parameter must be
* omitted.
* If not supported by the machine, a topology parameter must
* not be set to a value greater than 1.
*/
if (!mc->smp_props.modules_supported && config->has_modules) {
if (config->modules > 1) {
error_setg(errp, "modules not supported by this "
"machine's CPU topology");
return;
} else {
/* Here modules only equals 1 since we've checked zero case. */
warn_report("Deprecated CPU topology (considered invalid): "
"Unsupported modules parameter mustn't be "
"specified as 1");
}
if (!mc->smp_props.modules_supported &&
config->has_modules && config->modules > 1) {
error_setg(errp,
"modules > 1 not supported by this machine's CPU topology");
return;
}
modules = modules > 0 ? modules : 1;
if (!mc->smp_props.clusters_supported && config->has_clusters) {
if (config->clusters > 1) {
error_setg(errp, "clusters not supported by this "
"machine's CPU topology");
return;
} else {
/* Here clusters only equals 1 since we've checked zero case. */
warn_report("Deprecated CPU topology (considered invalid): "
"Unsupported clusters parameter mustn't be "
"specified as 1");
}
if (!mc->smp_props.clusters_supported &&
config->has_clusters && config->clusters > 1) {
error_setg(errp,
"clusters > 1 not supported by this machine's CPU topology");
return;
}
clusters = clusters > 0 ? clusters : 1;
if (!mc->smp_props.dies_supported && config->has_dies) {
if (config->dies > 1) {
error_setg(errp, "dies not supported by this "
"machine's CPU topology");
return;
} else {
/* Here dies only equals 1 since we've checked zero case. */
warn_report("Deprecated CPU topology (considered invalid): "
"Unsupported dies parameter mustn't be "
"specified as 1");
}
if (!mc->smp_props.dies_supported &&
config->has_dies && config->dies > 1) {
error_setg(errp,
"dies > 1 not supported by this machine's CPU topology");
return;
}
dies = dies > 0 ? dies : 1;
if (!mc->smp_props.books_supported && config->has_books) {
if (config->books > 1) {
error_setg(errp, "books not supported by this "
"machine's CPU topology");
return;
} else {
/* Here books only equals 1 since we've checked zero case. */
warn_report("Deprecated CPU topology (considered invalid): "
"Unsupported books parameter mustn't be "
"specified as 1");
}
if (!mc->smp_props.books_supported &&
config->has_books && config->books > 1) {
error_setg(errp,
"books > 1 not supported by this machine's CPU topology");
return;
}
books = books > 0 ? books : 1;
if (!mc->smp_props.drawers_supported && config->has_drawers) {
if (config->drawers > 1) {
error_setg(errp, "drawers not supported by this "
"machine's CPU topology");
return;
} else {
/* Here drawers only equals 1 since we've checked zero case. */
warn_report("Deprecated CPU topology (considered invalid): "
"Unsupported drawers parameter mustn't be "
"specified as 1");
}
if (!mc->smp_props.drawers_supported &&
config->has_drawers && config->drawers > 1) {
error_setg(errp,
"drawers > 1 not supported by this machine's CPU topology");
return;
}
drawers = drawers > 0 ? drawers : 1;

8
tests/unit/test-smp-parse.c
View File

@ -337,21 +337,21 @@ static const struct SMPTestData data_generic_invalid[] = {
{
/* config: -smp 2,dies=2 */
.config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
.expect_error = "dies not supported by this machine's CPU topology",
.expect_error = "dies > 1 not supported by this machine's CPU topology",
}, {
/* config: -smp 2,clusters=2 */
.config = SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
.expect_error = "clusters not supported by this machine's CPU topology",
.expect_error = "clusters > 1 not supported by this machine's CPU topology",
}, {
/* config: -smp 2,books=2 */
.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F,
0, F, 0, F, 0, F, 0),
.expect_error = "books not supported by this machine's CPU topology",
.expect_error = "books > 1 not supported by this machine's CPU topology",
}, {
/* config: -smp 2,drawers=2 */
.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F,
0, F, 0, F, 0, F, 0),
.expect_error = "drawers not supported by this machine's CPU topology",
.expect_error = "drawers > 1 not supported by this machine's CPU topology",
}, {
/* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
.config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),