MaxCpuCountInitialization() currently handles the following options: (1) QEMU does not report the boot CPU count.
In this case, PlatformPei makes MpInitLib enumerate APs up to the default PcdCpuMaxLogicalProcessorNumber value (64) minus 1, or until the default PcdCpuApInitTimeOutInMicroSeconds (50,000) elapses. (Whichever is reached first.) Time-limited AP enumeration had never been reliable on QEMU/KVM, which is why commit 45a70db3c3a5 strated handling case (2) below, in OVMF. (2) QEMU reports the boot CPU count. In this case, PlatformPei sets PcdCpuMaxLogicalProcessorNumber to the reported boot CPU count, and PcdCpuApInitTimeOutInMicroSeconds to practically "infinity" (MAX_UINT32, ~71 minutes). That causes MpInitLib to enumerate exactly the available (boot) APs. With CPU hotplug in mind, this method is not good enough. While UefiCpuPkg expects PcdCpuMaxLogicalProcessorNumber to include both boot (i.e., cold-plugged), and all *potentially* hot-plugged, logical processors, the boot CPU count reported by QEMU does not cover the second category. Rewrite MaxCpuCountInitialization() for handling the following cases: (1) The behavior of case (1) does not change. (No UefiCpuPkg PCDs are set to values different from the defaults.) (2) QEMU reports the boot CPU count, but not the potential maximum CPU count. In this case, the behavior remains unchanged. The way MpInitLib is instructed to do the same differs however: we now set the new PcdCpuBootLogicalProcessorNumber to the boot CPU count (together with PcdCpuMaxLogicalProcessorNumber). PcdCpuApInitTimeOutInMicroSeconds is irrelevant. (3) QEMU reports both the boot CPU count, and the potential maximum CPU count. We tell UefiCpuPkg about the potential maximum through PcdCpuMaxLogicalProcessorNumber. We also tell MpInitLib the boot CPU count for precise and quick AP enumeration, via PcdCpuBootLogicalProcessorNumber. PcdCpuApInitTimeOutInMicroSeconds is irrelevant. This patch is a pre-requisite for enabling CPU hotplug with SMM_REQUIRE. As a side effect, it also enables S3 to work with CPU hotplug at once, *without* SMM_REQUIRE. Cc: Anthony Perard <anthony.per...@citrix.com> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> Cc: Eric Dong <eric.d...@intel.com> Cc: Igor Mammedov <imamm...@redhat.com> Cc: Jordan Justen <jordan.l.jus...@intel.com> Cc: Julien Grall <julien.gr...@arm.com> Cc: Ray Ni <ray...@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 Signed-off-by: Laszlo Ersek <ler...@redhat.com> --- OvmfPkg/OvmfPkgIa32.dsc | 2 +- OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- OvmfPkg/OvmfPkgX64.dsc | 2 +- OvmfPkg/PlatformPei/PlatformPei.inf | 2 +- OvmfPkg/PlatformPei/Platform.c | 83 +++++++++++++------- 5 files changed, 60 insertions(+), 31 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 66e944436a69..89b1fc41458d 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -554,7 +554,7 @@ [PcdsDynamicDefault] # UefiCpuPkg PCDs related to initial AP bringup and general AP management. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 # Set memory encryption mask gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0 diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 51c2bfb44f14..f978d27a0b55 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -566,7 +566,7 @@ [PcdsDynamicDefault] # UefiCpuPkg PCDs related to initial AP bringup and general AP management. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 # Set memory encryption mask gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0 diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index ba7a75884490..04604ed6b35b 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -565,7 +565,7 @@ [PcdsDynamicDefault] # UefiCpuPkg PCDs related to initial AP bringup and general AP management. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 # Set memory encryption mask gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0 diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index d9fd9c8f05b3..30eaebdfae63 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -98,7 +98,7 @@ [Pcd] gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize [FixedPcd] diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 3ba2459872d9..12cec1d60ec1 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -564,43 +564,72 @@ S3Verification ( /** - Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg modules. - Set the mMaxCpuCount variable. + Fetch the boot CPU and max CPU numbers from QEMU, and expose them to + UefiCpuPkg modules. Set the mMaxCpuCount variable. **/ VOID MaxCpuCountInitialization ( VOID ) { - UINT16 ProcessorCount; - RETURN_STATUS PcdStatus; + UINT16 BootCpuCount; + RETURN_STATUS Status; + // + // Try to fetch the boot CPU count. + // QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount); - ProcessorCount = QemuFwCfgRead16 (); - // - // If the fw_cfg key or fw_cfg entirely is unavailable, load mMaxCpuCount - // from the PCD default. No change to PCDs. - // - if (ProcessorCount == 0) { + BootCpuCount = QemuFwCfgRead16 (); + if (BootCpuCount == 0) { + // + // QEMU doesn't report the boot CPU count. Let MpInitLib count APs up to + // (PcdCpuMaxLogicalProcessorNumber-1), or until + // PcdCpuApInitTimeOutInMicroSeconds elapses (whichever is reached first). + // + DEBUG ((DEBUG_WARN, "%a: boot CPU count unavailable\n", __FUNCTION__)); mMaxCpuCount = PcdGet32 (PcdCpuMaxLogicalProcessorNumber); - return; + } else { + // + // We will expose BootCpuCount to MpInitLib. MpInitLib will count APs up to + // (BootCpuCount-1) precisely, regardless of timeout. + // + // Now try to fetch topology info. + // + FIRMWARE_CONFIG_ITEM FwCfgItem; + UINTN FwCfgSize; + FW_CFG_X86_TOPOLOGY Topology; + + Status = QemuFwCfgFindFile ("etc/x86-smp-topology", &FwCfgItem, + &FwCfgSize); + if (RETURN_ERROR (Status) || FwCfgSize != sizeof Topology) { + // + // QEMU doesn't report the topology. Assume that the maximum CPU count + // equals the boot CPU count (implying no hotplug). + // + DEBUG ((DEBUG_WARN, "%a: topology unavailable\n", __FUNCTION__)); + mMaxCpuCount = BootCpuCount; + } else { + // + // Grab the maximum CPU count from the topology info. + // + QemuFwCfgSelectItem (FwCfgItem); + QemuFwCfgReadBytes (FwCfgSize, &Topology); + DEBUG ((DEBUG_VERBOSE, + "%a: DiesPerSocket=%u CoresPerDie=%u ThreadsPerCore=%u\n", + __FUNCTION__, Topology.DiesPerSocket, Topology.CoresPerDie, + Topology.ThreadsPerCore)); + mMaxCpuCount = Topology.MaxLogicalProcessors; + } } - // - // Otherwise, set mMaxCpuCount to the value reported by QEMU. - // - mMaxCpuCount = ProcessorCount; - // - // Additionally, tell UefiCpuPkg modules (a) the exact number of VCPUs, (b) - // to wait, in the initial AP bringup, exactly as long as it takes for all of - // the APs to report in. For this, we set the longest representable timeout - // (approx. 71 minutes). - // - PcdStatus = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, ProcessorCount); - ASSERT_RETURN_ERROR (PcdStatus); - PcdStatus = PcdSet32S (PcdCpuApInitTimeOutInMicroSeconds, MAX_UINT32); - ASSERT_RETURN_ERROR (PcdStatus); - DEBUG ((DEBUG_INFO, "%a: QEMU reports %d processor(s)\n", __FUNCTION__, - ProcessorCount)); + + DEBUG ((DEBUG_INFO, "%a: BootCpuCount=%d mMaxCpuCount=%u\n", __FUNCTION__, + BootCpuCount, mMaxCpuCount)); + ASSERT (BootCpuCount <= mMaxCpuCount); + + Status = PcdSet32S (PcdCpuBootLogicalProcessorNumber, BootCpuCount); + ASSERT_RETURN_ERROR (Status); + Status = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, mMaxCpuCount); + ASSERT_RETURN_ERROR (Status); } -- 2.19.1.3.g30247aa5d201 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48566): https://edk2.groups.io/g/devel/message/48566 Mute This Topic: https://groups.io/mt/34441232/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-