Re: [PATCH v11 11/14] machine: Make smp_parse generic enough for all arches

[Daniel P . Berrangé]

On Tue, Sep 28, 2021 at 12:57:21PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/28/21 05:57, Yanan Wang wrote:
> > Currently the only difference between smp_parse and pc_smp_parse
> > is the support of dies parameter and the related error reporting.
> > With some arch compat variables like "bool dies_supported", we can
> > make smp_parse generic enough for all arches and the PC specific
> > one can be removed.
> > 
> > Making smp_parse() generic enough can reduce code duplication and
> > ease the code maintenance, and also allows extending the topology
> > with more arch specific members (e.g., clusters) in the future.
> > 
> > Suggested-by: Andrew Jones <drjones@redhat.com>
> > Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> >  hw/core/machine.c   | 91 +++++++++++++++++++++++++++++++++++----------
> >  hw/i386/pc.c        | 84 +----------------------------------------
> >  include/hw/boards.h |  9 +++++
> >  3 files changed, 81 insertions(+), 103 deletions(-)
> 
> > +/*
> > + * smp_parse - Generic function used to parse the given SMP configuration
> > + *
> > + * Any missing parameter in "cpus/maxcpus/sockets/cores/threads" will be
> > + * automatically computed based on the provided ones.
> > + *
> > + * In the calculation of omitted sockets/cores/threads: we prefer sockets
> > + * over cores over threads before 6.2, while preferring cores over sockets
> > + * over threads since 6.2.
> > + *
> > + * In the calculation of cpus/maxcpus: When both maxcpus and cpus are 
> > omitted,
> > + * maxcpus will be computed 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 given counterpart's value. Both maxcpus 
> > and
> > + * cpus may be specified, but maxcpus must be equal to or greater than 
> > cpus.
> > + *
> > + * For compatibility, apart from the parameters that will be computed, 
> > newly
> > + * introduced topology members which are likely to be target specific 
> > should
> > + * be directly set as 1 if they are omitted (e.g. dies for PC since 4.1).
> > + */
> >  static void smp_parse(MachineState *ms, SMPConfiguration *config, Error 
> > **errp)
> 
> Can we have it return a boolean instead?

That's unrelated to this change, so ought to be a separate commit if
done.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|