Re: [PATCH] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations

[Prasad Pandit]

Hello Zhao,

On Mon, 4 Mar 2024 at 12:19, Zhao Liu <zhao1.liu@linux.intel.com> wrote:
> > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
>
> This indicates the default maxcpus is initialized as 0 if user doesn't
> specifies it.

* 'has_maxcpus' should be set only if maxcpus > 0. If maxcpus == 0,
then setting 'has_maxcpus=1' seems convoluted.

> However, we could initialize maxcpus as other default value, e.g.,
>
>     maxcpus = config->has_maxcpus ? config->maxcpus : 1.
===
hw/core/machine.c
 machine_initfn
    /* default to mc->default_cpus */
    ms->smp.cpus = mc->default_cpus;
    ms->smp.max_cpus = mc->default_cpus;

   static void machine_class_base_init(ObjectClass *oc, void *data)
   {
       MachineClass *mc = MACHINE_CLASS(oc);
       mc->max_cpus = mc->max_cpus ?: 1;
       mc->min_cpus = mc->min_cpus ?: 1;
       mc->default_cpus = mc->default_cpus ?: 1;
   }
===
* Looking at the above bits, it seems smp.cpus & smp.max_cpus are
initialised to 1 via default_cpus in MachineClass object.

>>  if (config->has_maxcpus && config->maxcpus == 0)
> This check only wants to identify the case that user sets the 0.
> If the default maxcpus is initialized as 0, then (maxcpus == 0) will
> fail if user doesn't set maxcpus.
>
> But it is still necessary to distinguish whether maxcpus is user-set or
> auto-initialized.

* If it is set to zero(0) either by user or by auto-initialise, it is
still invalid, right?

> If it is user-set, -smp should fail is there's invalid maxcpus/invalid
> topology.
>
> Otherwise, if it is auto-initialized, its value should be adjusted based
> on other topology components as the above calculation in (*).

* Why have such diverging ways?
* Could we simplify it as
   - If cpus/maxcpus==0, it is invalid, show an error and exit.
   - If cpus/maxcpus > 0, but incorrect for topology, then
re-calculate the correct value based on topology parameters. If the
re-calculated value is still incorrect or unsatisfactory, then show an
error and exit.

* Saying that user setting cpu/maxcpus=0 is invalid and
auto-initialising it to zero(0) is valid, is not consistent.

...wdyt?
---
  - Prasad