Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

[Igor Mammedov]

On Wed, 19 Jan 2022 15:48:20 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Wed, 19 Jan 2022 at 14:44, Godmar Back <gback@cs.vt.edu> wrote:
> > after upgrading to 6.2.0, I observed that code such as MIT's xv6 (see
> > [1]) is no longer able to detect multiple CPUs.  Their code works in
> > 6.1.1, however.  
> 
> Hi; this isn't a great place for reporting QEMU bugs, because
> it's more of a user-to-user discussion list. Not all QEMU
> developers read it. I've cc'd the ACPI maintainers, who
> hopefully may have an idea about what's happening here.
> You could also file a bug at
> https://gitlab.com/qemu-project/qemu/-/issues
> 
> > I built 6.1.1 from source and 6.2.0 from source and I have also tested
> > with CentOS stream's 6.1.1 qemu-kvm and was able to pinpoint this
> > change to these 2 versions of qemu. I am using qemu-system-i386
> > specifically.
> >
> > I tried to go through the ChangeLog to see if the `-smp` option was
> > deprecated or changed.  I found this note [2] about invalid topologies
> > having been removed in 5.2. Here's what I found after long
> > experimentation:
> >
> > The legacy MP tables appear to work only if you specify the longform
> > `-smp cpus=4,cores=1,threads=1,sockets=4` in 6.2.0.  If you specify
> > `-smp 4` or `-smp cpus=4` it will not work in 6.2.0 (it worked in
> > 6.1.1). I am guessing that perhaps the MP tables add entries for each
> > socket, but that perhaps the behavior of the shortcuts `-smp n` and
> > `-smp cpus=n` was changed to influence the number of cores rather than
> > sockets.
> >
> > In other words, `-smp cpus=n` now means `-smp
> > cpus=n,cores=n,threads=1,sockets=1` whereas in 6.1.1 and before it
> > meant `-smp cpus=n,cores=1,threads=1,sockets=n`. I note that
> > specifying `-smp cpus=4,cores=4,threads=1,sockets=1` in 6.1.1 also
> > does not create 4 entries in the legacy MP tables.



Thanks for reporting issue in such a detailed way.

QEMU doesn't generate legacy MP tables and as reported
the above issue is still present in earlier versions when
cores are used.
Well seabios has a comment:
         /* Only populate the MPS tables with the first logical CPU in            
           each package */
So I'd guess it has never worked for anything but sockets.
With QEMU starting to prefer cores over sockets by default
I'd suggest to either
  * explicitly provide desired topology (i.e. sockets)
  * use older machine type which still preffers sockets by default
    (ex: up to 6.1 machine types)

If anybody cares about legacy tables + cores/threads usecase,
I suggest to investigate what can be done on SeaBIOS side which
generates MP tables (assuming if anything could be done at all).
CCing SeaBIOS mail-list.

> > Can someone confirm/deny this?  If so, it's a breaking change that
> > perhaps could be mentioned in the Changelog to save others the time
> > when they upgrade. Affected educational OS include MIT's xv6 and
> > Stanford's pintos OS.

Legacy MP table is not actively maintained part of the code,
hence it's configuration which is not tested.
However if someone is interested in maintaining this, one
should contribute at least a testcase that will warn developers
early if usecase is broken. We can't promise not breaking it
ever but at least we would be able to document any breaking
changes in release notes.

> > Thanks for all the work you do on qemu!
> >
> >  - Godmar
> >
> > [1] https://github.com/mit-pdos/xv6-public/blob/eeb7b415dbcb12cc362d0783e41c3d1f44066b17/mp.c
> > [2] https://qemu-project.gitlab.io/qemu/about/removed-features.html#smp-invalid-topologies-removed-in-5-2
> >
> > (I'm typing this email in gmail using the plaintext setting, hopefully
> > it'll preserve line breaks.)  
> 
> thanks
> -- PMM
> 

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

[Ani Sinha]

[-- Attachment #1: Type: text/plain, Size: 4550 bytes --]

+qemu-devel

On Thu, 20 Jan 2022, Ani Sinha wrote:

>
>
> On Wed, 19 Jan 2022, Peter Maydell wrote:
>
> > On Wed, 19 Jan 2022 at 14:44, Godmar Back <gback@cs.vt.edu> wrote:
> > > after upgrading to 6.2.0, I observed that code such as MIT's xv6 (see
> > > [1]) is no longer able to detect multiple CPUs.  Their code works in
> > > 6.1.1, however.
> >
> > Hi; this isn't a great place for reporting QEMU bugs, because
> > it's more of a user-to-user discussion list. Not all QEMU
> > developers read it. I've cc'd the ACPI maintainers, who
> > hopefully may have an idea about what's happening here.
> > You could also file a bug at
> > https://gitlab.com/qemu-project/qemu/-/issues
> >
> > > I built 6.1.1 from source and 6.2.0 from source and I have also tested
> > > with CentOS stream's 6.1.1 qemu-kvm and was able to pinpoint this
> > > change to these 2 versions of qemu. I am using qemu-system-i386
> > > specifically.
> > >
> > > I tried to go through the ChangeLog to see if the `-smp` option was
> > > deprecated or changed.  I found this note [2] about invalid topologies
> > > having been removed in 5.2. Here's what I found after long
> > > experimentation:
> > >
> > > The legacy MP tables appear to work only if you specify the longform
> > > `-smp cpus=4,cores=1,threads=1,sockets=4` in 6.2.0.  If you specify
> > > `-smp 4` or `-smp cpus=4` it will not work in 6.2.0 (it worked in
> > > 6.1.1). I am guessing that perhaps the MP tables add entries for each
> > > socket, but that perhaps the behavior of the shortcuts `-smp n` and
> > > `-smp cpus=n` was changed to influence the number of cores rather than
> > > sockets.
> > >
> > > In other words, `-smp cpus=n` now means `-smp
> > > cpus=n,cores=n,threads=1,sockets=1` whereas in 6.1.1 and before it
> > > meant `-smp cpus=n,cores=1,threads=1,sockets=n`. I note that
> > > specifying `-smp cpus=4,cores=4,threads=1,sockets=1` in 6.1.1 also
> > > does not create 4 entries in the legacy MP tables.
> > >
>
> My suspicion is that the following commit might have introduced a
> regression:
>
> commit 86ce2d28fa09d15547b5cabdc264cadfb53a848c
> Author: Yanan Wang <wangyanan55@huawei.com>
> Date:   Tue Oct 26 11:46:58 2021 +0800
>
>     hw/core/machine: Split out the smp parsing code
>
>     We are going to introduce an unit test for the parser smp_parse()
>     in hw/core/machine.c, but now machine.c is only built in softmmu.
>
>     In order to solve the build dependency on the smp parsing code and
>     avoid building unrelated stuff for the unit tests, move the tested
>     code from machine.c into a separate file, i.e., machine-smp.c and
>     build it in common field.
>
>     Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>     Reviewed-by: Andrew Jones <drjones@redhat.com>
>     Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>     Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>     Message-Id: <20211026034659.22040-2-wangyanan55@huawei.com>
>     Acked-by: Eduardo Habkost <ehabkost@redhat.com>
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> I am still checking.

Yes this patch introduced the behavior of preferring cores over sockets
post 6.2. I think this is intentional. See the following hunk:

+        if (mc->smp_props.prefer_sockets) {
+            /* prefer sockets over cores before 6.2 */
+            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 {
+            /* prefer cores over sockets since 6.2 */
+            if (cores == 0) {
+                sockets = sockets > 0 ? sockets : 1;
+                threads = threads > 0 ? threads : 1;
+                cores = maxcpus / (sockets * dies * threads);
+            } else if (sockets == 0) {
+                threads = threads > 0 ? threads : 1;
+                sockets = maxcpus / (dies * cores * threads);
+            }
+        }


prefer_sockets is set to true for 6.1 machines:

static void pc_q35_6_1_machine_options(MachineClass *m)
{
    pc_q35_6_2_machine_options(m);
    m->alias = NULL;
    compat_props_add(m->compat_props, hw_compat_6_1, hw_compat_6_1_len);
    compat_props_add(m->compat_props, pc_compat_6_1, pc_compat_6_1_len);
    m->smp_props.prefer_sockets = true;
}

So this behavior change is intended.

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

[Ani Sinha]

[-- Attachment #1: Type: text/plain, Size: 6175 bytes --]



On Thu, 20 Jan 2022, Ani Sinha wrote:

> +qemu-devel
>
> On Thu, 20 Jan 2022, Ani Sinha wrote:
>
> >
> >
> > On Wed, 19 Jan 2022, Peter Maydell wrote:
> >
> > > On Wed, 19 Jan 2022 at 14:44, Godmar Back <gback@cs.vt.edu> wrote:
> > > > after upgrading to 6.2.0, I observed that code such as MIT's xv6 (see
> > > > [1]) is no longer able to detect multiple CPUs.  Their code works in
> > > > 6.1.1, however.
> > >
> > > Hi; this isn't a great place for reporting QEMU bugs, because
> > > it's more of a user-to-user discussion list. Not all QEMU
> > > developers read it. I've cc'd the ACPI maintainers, who
> > > hopefully may have an idea about what's happening here.
> > > You could also file a bug at
> > > https://gitlab.com/qemu-project/qemu/-/issues
> > >
> > > > I built 6.1.1 from source and 6.2.0 from source and I have also tested
> > > > with CentOS stream's 6.1.1 qemu-kvm and was able to pinpoint this
> > > > change to these 2 versions of qemu. I am using qemu-system-i386
> > > > specifically.
> > > >
> > > > I tried to go through the ChangeLog to see if the `-smp` option was
> > > > deprecated or changed.  I found this note [2] about invalid topologies
> > > > having been removed in 5.2. Here's what I found after long
> > > > experimentation:
> > > >
> > > > The legacy MP tables appear to work only if you specify the longform
> > > > `-smp cpus=4,cores=1,threads=1,sockets=4` in 6.2.0.  If you specify
> > > > `-smp 4` or `-smp cpus=4` it will not work in 6.2.0 (it worked in
> > > > 6.1.1). I am guessing that perhaps the MP tables add entries for each
> > > > socket, but that perhaps the behavior of the shortcuts `-smp n` and
> > > > `-smp cpus=n` was changed to influence the number of cores rather than
> > > > sockets.
> > > >
> > > > In other words, `-smp cpus=n` now means `-smp
> > > > cpus=n,cores=n,threads=1,sockets=1` whereas in 6.1.1 and before it
> > > > meant `-smp cpus=n,cores=1,threads=1,sockets=n`. I note that
> > > > specifying `-smp cpus=4,cores=4,threads=1,sockets=1` in 6.1.1 also
> > > > does not create 4 entries in the legacy MP tables.
> > > >
> >
> > My suspicion is that the following commit might have introduced a
> > regression:
> >
> > commit 86ce2d28fa09d15547b5cabdc264cadfb53a848c
> > Author: Yanan Wang <wangyanan55@huawei.com>
> > Date:   Tue Oct 26 11:46:58 2021 +0800
> >
> >     hw/core/machine: Split out the smp parsing code
> >
> >     We are going to introduce an unit test for the parser smp_parse()
> >     in hw/core/machine.c, but now machine.c is only built in softmmu.
> >
> >     In order to solve the build dependency on the smp parsing code and
> >     avoid building unrelated stuff for the unit tests, move the tested
> >     code from machine.c into a separate file, i.e., machine-smp.c and
> >     build it in common field.
> >
> >     Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> >     Reviewed-by: Andrew Jones <drjones@redhat.com>
> >     Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >     Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >     Message-Id: <20211026034659.22040-2-wangyanan55@huawei.com>
> >     Acked-by: Eduardo Habkost <ehabkost@redhat.com>
> >     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >
> > I am still checking.
>
> Yes this patch introduced the behavior of preferring cores over sockets
> post 6.2. I think this is intentional. See the following hunk:
>
> +        if (mc->smp_props.prefer_sockets) {
> +            /* prefer sockets over cores before 6.2 */
> +            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 {
> +            /* prefer cores over sockets since 6.2 */
> +            if (cores == 0) {
> +                sockets = sockets > 0 ? sockets : 1;
> +                threads = threads > 0 ? threads : 1;
> +                cores = maxcpus / (sockets * dies * threads);
> +            } else if (sockets == 0) {
> +                threads = threads > 0 ? threads : 1;
> +                sockets = maxcpus / (dies * cores * threads);
> +            }
> +        }

Actually I am not quite right. This is the real change which changed the
preference. The previous change was a code re-org that preserved the
behavior:

commit 4a0af2930a4e4f64ce551152fdb4b9e7be106408
Author: Yanan Wang <wangyanan55@huawei.com>
Date:   Wed Sep 29 10:58:09 2021 +0800

    machine: Prefer cores over sockets in smp parsing since 6.2

    In the real SMP hardware topology world, it's much more likely that
    we have high cores-per-socket counts and few sockets totally. While
    the current preference of sockets over cores in smp parsing results
    in a virtual cpu topology with low cores-per-sockets counts and a
    large number of sockets, which is just contrary to the real world.

    Given that it is better to make the virtual cpu topology be more
    reflective of the real world and also for the sake of compatibility,
    we start to prefer cores over sockets over threads in smp parsing
    since machine type 6.2 for different arches.

    In this patch, a boolean "smp_prefer_sockets" is added, and we only
    enable the old preference on older machines and enable the new one
    since type 6.2 for all arches by using the machine compat mechanism.

    Suggested-by: Daniel P. Berrange <berrange@redhat.com>
    Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
    Acked-by: David Gibson <david@gibson.dropbear.id.au>
    Acked-by: Cornelia Huck <cohuck@redhat.com>
    Reviewed-by: Andrew Jones <drjones@redhat.com>
    Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
    Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
    Message-Id: <20210929025816.21076-10-wangyanan55@huawei.com>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

In any case, the behavior change is intended because of the reasons the
above commit outlines.

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

[Daniel P. Berrangé]

On Thu, Jan 20, 2022 at 03:38:26PM +0530, Ani Sinha wrote:
> Actually I am not quite right. This is the real change which changed the
> preference. The previous change was a code re-org that preserved the
> behavior:
> 
> commit 4a0af2930a4e4f64ce551152fdb4b9e7be106408
> Author: Yanan Wang <wangyanan55@huawei.com>
> Date:   Wed Sep 29 10:58:09 2021 +0800
> 
>     machine: Prefer cores over sockets in smp parsing since 6.2
> 
>     In the real SMP hardware topology world, it's much more likely that
>     we have high cores-per-socket counts and few sockets totally. While
>     the current preference of sockets over cores in smp parsing results
>     in a virtual cpu topology with low cores-per-sockets counts and a
>     large number of sockets, which is just contrary to the real world.
> 
>     Given that it is better to make the virtual cpu topology be more
>     reflective of the real world and also for the sake of compatibility,
>     we start to prefer cores over sockets over threads in smp parsing
>     since machine type 6.2 for different arches.
> 
>     In this patch, a boolean "smp_prefer_sockets" is added, and we only
>     enable the old preference on older machines and enable the new one
>     since type 6.2 for all arches by using the machine compat mechanism.
> 
>     Suggested-by: Daniel P. Berrange <berrange@redhat.com>
>     Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>     Acked-by: David Gibson <david@gibson.dropbear.id.au>
>     Acked-by: Cornelia Huck <cohuck@redhat.com>
>     Reviewed-by: Andrew Jones <drjones@redhat.com>
>     Reviewed-by: Pankaj Gupta <pankaj.gupta@ionos.com>
>     Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>     Message-Id: <20210929025816.21076-10-wangyanan55@huawei.com>
>     Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> In any case, the behavior change is intended because of the reasons the
> above commit outlines.

Further compelling reason not mentioned there is that some OS will
artifically restrict how many sockets they are willing to use, while
happily using as many cores as they get. This is usually a licensing
or billing restriction rather than some technical reason, and kinda
silly since cores/sockets are basically interchangable, but that's
life.

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 :|

Re: MP tables do not report multiple CPUs in Qemu 6.2.0 on x86 when given -smp cpus=n flag

[Godmar Back]

Thank you for the replies. I will note that I suspected SeaBIOS as
well. However, testing 6.2.0 with SeaBIOS 14 (which is the version
that shipped with 6.1.1) did not change the behavior, so I concluded
it was a change in Qemu, despite the fact that SeaBIOS is setting up
the tables.

I was about to write a note that this information is hard to find, but
I feel I need to apologize. When I go to the user manual [1] it
explains the change in behavior:

"Historically preference was given to the coarsest topology parameters
when computing missing values (ie sockets preferred over cores, which
were preferred over threads), however, this behaviour is considered
liable to change. Prior to 6.2 the preference was sockets over cores
over threads. Since 6.2 the preference is cores over sockets over
threads.

For example, the following option defines a machine board with 2
sockets of 1 core before 6.2 and 1 socket of 2 cores after 6.2:

-smp 2"

I should have checked first.

[1] https://www.qemu.org/docs/master/system/invocation.html