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 invalid cpu topology check.
Since we require for the valid config that the sum of "sockets * cores * dies * 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 splited into two branches of "cpus == 0" and "sockets == 0" so that we can clearly read that we are parsing -smp cmdlines with a preference of cpus over sockets over cores over threads. Note: change in this patch won't affect any existing working cmdlines but improves consistency and allow more incomplete configs to be valid. Signed-off-by: Yanan Wang <wangyana...@huawei.com> --- hw/core/machine.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index ed6712e964..c9f15b15a5 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -768,24 +768,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 = sockets * dies * cores * threads; - } 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 / (sockets * dies * cores); - threads = threads > 0 ? threads : 1; - } else if (sockets * dies * cores * threads < cpus) { + threads = maxcpus / (sockets * dies * cores); + } + + if (sockets * dies * cores * threads < cpus) { g_autofree char *dies_msg = g_strdup_printf( mc->smp_dies_supported ? " * dies (%u)" : "", dies); error_setg(errp, "cpu topology: " @@ -795,8 +797,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; -- 2.19.1