On Wed, Jul 28, 2021 at 11:48:40AM +0800, Yanan Wang wrote: > 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 <wangyana...@huawei.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 69979c93dd..958e6e7107 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -755,25 +755,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 e077aa80bf..0912236b4b 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -210,9 +210,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 > + equal to or greater than the initial CPU count. Both parameters are
be equal to > + 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 > Otherwise Reviewed-by: Andrew Jones <drjo...@redhat.com>