+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 <gb...@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 <wangyana...@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 <wangyana...@huawei.com> > Reviewed-by: Andrew Jones <drjo...@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com> > Message-Id: <20211026034659.22040-2-wangyana...@huawei.com> > Acked-by: Eduardo Habkost <ehabk...@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <phi...@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.