Re: [PATCH v2] hw/i386: Expand the range of CPU topologies between smp and maxcpus

[Like Xu]

On 2021/4/26 21:30, Daniel P. Berrangé wrote:
> On Mon, Apr 26, 2021 at 10:08:52AM +0800, caodongli@kingsoft.com wrote:
> > Change the criteria for the initial CPU topology and maxcpus, user can
> > have more settings
>
> Can you provide a better explanation of why this is needed. What
> valid usage scenario is blocked by the current check ?
>
> AFAICT, it partially reverts an intentional change done in several
> years ago in :
>
>
>    commit bc1fb850a31468ac4976f3895f01a6d981e06d0a
>    Author: Igor Mammedov <imammedo@redhat.com>
>    Date:   Thu Sep 13 13:06:01 2018 +0200
>
>      vl.c deprecate incorrect CPUs topology
>      
>      -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
>      so that total number of logical CPUs [sockets * cores * threads]
>      would be equal to [maxcpus], however historically we didn't have
>      such check in QEMU and it is possible to start VM with an invalid
>      topology.
>      Deprecate invalid options combination so we can make sure that
>      the topology VM started with is always correct in the future.
>      Users with an invalid sockets/cores/threads/maxcpus values should
>      fix their CLI to make sure that
>         [sockets * cores * threads] == [maxcpus]

Another helpful commit would be:

commit c4332cd1dcf2964c23893ab4c0bf8d774e42a3cf
Author: Igor Mammedov <imammedo@redhat.com>
Date:   Fri Sep 11 09:32:02 2020 -0400

    smp: drop support for deprecated (invalid topologies)

    it's was deprecated since 3.1

    Support for invalid topologies is removed, the user must ensure
    that topologies described with -smp include all possible cpus,
    i.e. (sockets * cores * threads) == maxcpus or QEMU will
    exit with error.


So is the following statement correct:

When we explicitly set the topology, we must ensure that the combination 
(sockets/dies/cores/threads/maxcpus) is always valid. If we need hot plug 
testing, we can only use something like "-smp 1,maxcpus = 4" since 3.1.

?


> > Signed-off-by: Dongli Cao <caodongli@kingsoft.com>
> > ---
> > hw/i386/pc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 8a84b25..ef2e819 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -751,7 +751,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
> >               exit(1);
> >           }
> >
> > -        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> > +        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> >               error_report("Invalid CPU topology deprecated: "
> >                            "sockets (%u) * dies (%u) * cores (%u) * threads (%u) 
> > "
> >                            "!= maxcpus (%u)",
>
> This is
>
> > --
> > 1.8.3.1
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > caodongli@kingsoft.com
>
> Regards,
> Daniel