On 2021/7/12 23:27, Andrew Jones wrote:
On Mon, Jul 12, 2021 at 05:05:43PM +0200, Andrew Jones wrote:
On Mon, Jul 12, 2021 at 05:04:04PM +0200, Andrew Jones wrote:
On Fri, Jul 02, 2021 at 06:07:36PM +0800, Yanan Wang wrote:
It's possible that dies parameter is explicitly specified as "dies=0"
in the cmdline, if so we will wrongly calculate the other ommited
parameters such as "sockets = maxcpus / (dies * cores * threads);"
with a zeroed dies value.
So perform zero-check (default the value to 1 if zeroed) for -smp dies
before using it to calculate other parameters.
OK, dies=0 may make some sense for a user that doesn't want to describe
dies.
Reviewed-by: Andrew Jones <drjo...@redhat.com>
But... this is the pc smp parse function, not the general one, so maybe my
comment from patch 1 should apply here as well, which is, that dies=0
should be an error rather than silently changed to dies=1.
Also, after reading Daniel's comment on a later patch, I think anything=0
should just be an error, even for the general parser.
Ok, after reading comments in patch #1, I also agree with this solution.
We can reject the "anything = 0" configuration in advance in function
machine_smp_parse(). I will do it in v2.
BTW, I have converted smp_parse() into a generic helper allowing future
extension for new arch-specific parameters, as you have suggested.
So that we don't need to maintain duplicated parsing code any more.
Thanks,
Yanan
.
Fixes: 1b45842203540 (vl.c: Add -smp, dies=* command line support and update
doc)
Signed-off-by: Yanan Wang <wangyana...@huawei.com>
---
hw/i386/pc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a44511c937..93d1f12a49 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -714,12 +714,14 @@ 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;
/* compute missing values, prefer sockets over cores over threads */
+ dies = dies > 0 ? dies : 1;
+
if (cpus == 0 || sockets == 0) {
cores = cores > 0 ? cores : 1;
threads = threads > 0 ? threads : 1;
--
2.19.1
.
