Currently the mptable setup code only considers one core per cpu package for populating the cpu tables. The detection logic goes back to machines where the cpuid would signify the presence of multiple logical cores (HT) and presumably this change was made to prevent creating vcpus for each hyperthread.
In practice this restriction is not required any longer especially since current processors feature many physical cores per socket, and it is preventing QEMU guests that don't enable ACPI from seeing more than one cores, unless those cores are explicitly placed across multiple sockets (one core per socket, specifically).
During v6.2 QEMU changed the default cpu topology arrangement preference from arranging multiple cores spreaded across multiple sockets to placing them within the same socket [1]. In practice this means that without specifing explicitly the cpu topology and assuming QEMU defaults, a guest without ACPI will not be able to instantiate more than one core.
Fix this by lifting the restriction of only populating the mptable with the first logical processor per package.
[1] commit 4a0af2930a4e ("machine: Prefer cores over sockets in smp parsing since 6.2")
Fixes: c0ad0e8febe5 ("Only add the first logical CPU in each physical CPU to the MPS tables.")
Signed-off-by: Anthony Iliopoulos ailiop@suse.com --- src/fw/mptable.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/fw/mptable.c b/src/fw/mptable.c index 47385cc5d32d..a939743691c9 100644 --- a/src/fw/mptable.c +++ b/src/fw/mptable.c @@ -47,19 +47,13 @@ mptable_setup(void) cpuid_signature = 0x600; cpuid_features = 0x201; } - int pkgcpus = 1; - if (cpuid_features & (1 << 28)) { - /* Only populate the MPS tables with the first logical CPU in - each package */ - pkgcpus = (ebx >> 16) & 0xff; - pkgcpus = 1 << (__fls(pkgcpus - 1) + 1); /* round up to power of 2 */ - } + u8 apic_version = readl((u8*)BUILD_APIC_ADDR + 0x30) & 0xff;
// CPU definitions. struct mpt_cpu *cpus = (void*)&config[1], *cpu = cpus; int i; - for (i = 0; i < MaxCountCPUs; i+=pkgcpus) { + for (i = 0; i < MaxCountCPUs; i++) { memset(cpu, 0, sizeof(*cpu)); cpu->type = MPT_TYPE_CPU; cpu->apicid = i;
On Wed, Dec 11, 2024 at 11:36:42AM +0100, Anthony Iliopoulos via SeaBIOS wrote:
Currently the mptable setup code only considers one core per cpu package for populating the cpu tables. The detection logic goes back to machines where the cpuid would signify the presence of multiple logical cores (HT) and presumably this change was made to prevent creating vcpus for each hyperthread.
In practice this restriction is not required any longer especially since current processors feature many physical cores per socket, and it is preventing QEMU guests that don't enable ACPI from seeing more than one cores, unless those cores are explicitly placed across multiple sockets (one core per socket, specifically).
During v6.2 QEMU changed the default cpu topology arrangement preference from arranging multiple cores spreaded across multiple sockets to placing them within the same socket [1]. In practice this means that without specifing explicitly the cpu topology and assuming QEMU defaults, a guest without ACPI will not be able to instantiate more than one core.
Fix this by lifting the restriction of only populating the mptable with the first logical processor per package.
[1] commit 4a0af2930a4e ("machine: Prefer cores over sockets in smp parsing since 6.2")
Fixes: c0ad0e8febe5 ("Only add the first logical CPU in each physical CPU to the MPS tables.")
Signed-off-by: Anthony Iliopoulos ailiop@suse.com
Alas, we found that we can't maintain these files - anytime we make any change (for better or worse) it causes backwards compatibility problems with clients. If you look at the top of mptable.c you'll find:
// DO NOT ADD NEW FEATURES HERE. (See paravirt.c / biostables.c instead.)
So, if a change is needed to the mptable then code will be needed to generate the desired table in qemu and pass it through to SeaBIOS.
Cheers, -Kevin
src/fw/mptable.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/fw/mptable.c b/src/fw/mptable.c index 47385cc5d32d..a939743691c9 100644 --- a/src/fw/mptable.c +++ b/src/fw/mptable.c @@ -47,19 +47,13 @@ mptable_setup(void) cpuid_signature = 0x600; cpuid_features = 0x201; }
- int pkgcpus = 1;
- if (cpuid_features & (1 << 28)) {
/* Only populate the MPS tables with the first logical CPU ineach package */pkgcpus = (ebx >> 16) & 0xff;pkgcpus = 1 << (__fls(pkgcpus - 1) + 1); /* round up to power of 2 */- }
u8 apic_version = readl((u8*)BUILD_APIC_ADDR + 0x30) & 0xff;
// CPU definitions. struct mpt_cpu *cpus = (void*)&config[1], *cpu = cpus; int i;
- for (i = 0; i < MaxCountCPUs; i+=pkgcpus) {
- for (i = 0; i < MaxCountCPUs; i++) { memset(cpu, 0, sizeof(*cpu)); cpu->type = MPT_TYPE_CPU; cpu->apicid = i;
-- 2.47.0
SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Tue, Dec 24, 2024 at 06:55:19PM -0500, Kevin O'Connor wrote:
On Wed, Dec 11, 2024 at 11:36:42AM +0100, Anthony Iliopoulos via SeaBIOS wrote:
Currently the mptable setup code only considers one core per cpu package for populating the cpu tables. The detection logic goes back to machines where the cpuid would signify the presence of multiple logical cores (HT) and presumably this change was made to prevent creating vcpus for each hyperthread.
In practice this restriction is not required any longer especially since current processors feature many physical cores per socket, and it is preventing QEMU guests that don't enable ACPI from seeing more than one cores, unless those cores are explicitly placed across multiple sockets (one core per socket, specifically).
During v6.2 QEMU changed the default cpu topology arrangement preference from arranging multiple cores spreaded across multiple sockets to placing them within the same socket [1]. In practice this means that without specifing explicitly the cpu topology and assuming QEMU defaults, a guest without ACPI will not be able to instantiate more than one core.
Fix this by lifting the restriction of only populating the mptable with the first logical processor per package.
[1] commit 4a0af2930a4e ("machine: Prefer cores over sockets in smp parsing since 6.2")
Fixes: c0ad0e8febe5 ("Only add the first logical CPU in each physical CPU to the MPS tables.")
Signed-off-by: Anthony Iliopoulos ailiop@suse.com
Alas, we found that we can't maintain these files - anytime we make any change (for better or worse) it causes backwards compatibility problems with clients. If you look at the top of mptable.c you'll find:
What are those other clients, out of curiosity? AFAICT other than qemu it is only bochs that may be leveraging it (although not by default).
// DO NOT ADD NEW FEATURES HERE. (See paravirt.c / biostables.c instead.)
So, if a change is needed to the mptable then code will be needed to generate the desired table in qemu and pass it through to SeaBIOS.
I assume you mean via the fw_cfg interface entries, but in paravirt.c there's the other comment:
// List of QEMU fw_cfg entries. DO NOT ADD MORE. (All new content // should be passed via the fw_cfg "file" interface.)
Also since qemu is already passing all the necessary info (CFG_MAX_CPUS), there shouldn't be any need to construct and pass any new entries, what about something simpler along the lines of the following diff:
diff --git a/src/fw/mptable.c b/src/fw/mptable.c index 47385cc5d32d..76d2213d4b30 100644 --- a/src/fw/mptable.c +++ b/src/fw/mptable.c @@ -12,6 +12,7 @@ #include "hw/pci_regs.h" // PCI_INTERRUPT_PIN #include "malloc.h" // free #include "output.h" // dprintf +#include "paravirt.h" // runningOnQEMU #include "romfile.h" // romfile_loadint #include "std/mptable.h" // MPTABLE_SIGNATURE #include "string.h" // memset @@ -48,7 +49,7 @@ mptable_setup(void) cpuid_features = 0x201; } int pkgcpus = 1; - if (cpuid_features & (1 << 28)) { + if (cpuid_features & (1 << 28) && !runningOnQEMU()) { /* Only populate the MPS tables with the first logical CPU in each package */ pkgcpus = (ebx >> 16) & 0xff;
Regards, Anthony
On Thu, Dec 26, 2024 at 12:22:43PM +0100, Anthony Iliopoulos wrote:
On Tue, Dec 24, 2024 at 06:55:19PM -0500, Kevin O'Connor wrote:
On Wed, Dec 11, 2024 at 11:36:42AM +0100, Anthony Iliopoulos via SeaBIOS wrote:
Currently the mptable setup code only considers one core per cpu package for populating the cpu tables. The detection logic goes back to machines where the cpuid would signify the presence of multiple logical cores (HT) and presumably this change was made to prevent creating vcpus for each hyperthread.
In practice this restriction is not required any longer especially since current processors feature many physical cores per socket, and it is preventing QEMU guests that don't enable ACPI from seeing more than one cores, unless those cores are explicitly placed across multiple sockets (one core per socket, specifically).
During v6.2 QEMU changed the default cpu topology arrangement preference from arranging multiple cores spreaded across multiple sockets to placing them within the same socket [1]. In practice this means that without specifing explicitly the cpu topology and assuming QEMU defaults, a guest without ACPI will not be able to instantiate more than one core.
Fix this by lifting the restriction of only populating the mptable with the first logical processor per package.
[1] commit 4a0af2930a4e ("machine: Prefer cores over sockets in smp parsing since 6.2")
Fixes: c0ad0e8febe5 ("Only add the first logical CPU in each physical CPU to the MPS tables.")
Signed-off-by: Anthony Iliopoulos ailiop@suse.com
Alas, we found that we can't maintain these files - anytime we make any change (for better or worse) it causes backwards compatibility problems with clients. If you look at the top of mptable.c you'll find:
What are those other clients, out of curiosity? AFAICT other than qemu it is only bochs that may be leveraging it (although not by default).
By "clients" I meant "guests" - for example various versions of linux, windows, etc. that are running in the guest VM alongside SeaBIOS.
// DO NOT ADD NEW FEATURES HERE. (See paravirt.c / biostables.c instead.)
So, if a change is needed to the mptable then code will be needed to generate the desired table in qemu and pass it through to SeaBIOS.
I assume you mean via the fw_cfg interface entries, but in paravirt.c there's the other comment:
I was referring to how qemu_platform_setup() calls romfile_loader_execute() to populate acpi (and possibly other) tables generated by qemu. Or, similarly, how smbios_build_tables() copies the smbios table generated by qemu.
// List of QEMU fw_cfg entries. DO NOT ADD MORE. (All new content // should be passed via the fw_cfg "file" interface.)
Also since qemu is already passing all the necessary info (CFG_MAX_CPUS), there shouldn't be any need to construct and pass any new entries, what about something simpler along the lines of the following diff:
As before, we've found that any change to these guest visible tables causes unexpected compatibility and rollout issues.
Cheers, -Kevin
-
Anthony Iliopoulos -
Kevin O'Connor