From 83357313dd6750e5c3c4e290676acee9d391d9e3 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Tue, 8 Oct 2019 09:15:38 +0200 Subject: [PATCH] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug MaxCpuCountInitialization() currently handles the following options: (1) QEMU does not report the boot CPU count (FW_CFG_NB_CPUS is 0) 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 (FW_CFG_NB_CPUS is nonzero) In this case, PlatformPei sets - PcdCpuMaxLogicalProcessorNumber to the reported boot CPU count (FW_CFG_NB_CPUS, which exports "PCMachineState.boot_cpus"), - and PcdCpuApInitTimeOutInMicroSeconds to practically "infinity" (MAX_UINT32, ~71 minutes). That causes MpInitLib to enumerate exactly the present (boot) APs. With CPU hotplug in mind, this method is not good enough. Because, using QEMU terminology, UefiCpuPkg expects PcdCpuMaxLogicalProcessorNumber to provide the "possible CPUs" count ("MachineState.smp.max_cpus"), which includes present and not present CPUs both (with not present CPUs being subject for hot-plugging). FW_CFG_NB_CPUS does not include not present CPUs. 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 ("PCMachineState.boot_cpus", via FW_CFG_NB_CPUS), but not the possible CPUs count ("MachineState.smp.max_cpus"). 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 (while continuing to set PcdCpuMaxLogicalProcessorNumber identically). PcdCpuApInitTimeOutInMicroSeconds becomes irrelevant. (3) QEMU reports both the boot CPU count ("PCMachineState.boot_cpus", via FW_CFG_NB_CPUS), and the possible CPUs count ("MachineState.smp.max_cpus"). We tell UefiCpuPkg about the possible CPUs count through PcdCpuMaxLogicalProcessorNumber. We also tell MpInitLib the boot CPU count for precise and quick AP enumeration, via PcdCpuBootLogicalProcessorNumber. PcdCpuApInitTimeOutInMicroSeconds is irrelevant again. This patch is a pre-requisite for enabling CPU hotplug with SMM_REQUIRE. As a side effect, the patch also enables S3 to work with CPU hotplug at once, *without* SMM_REQUIRE. (Without the patch, S3 resume fails, if a CPU is hot-plugged at OS runtime, prior to suspend: the FW_CFG_NB_CPUS increase seen during resume causes PcdCpuMaxLogicalProcessorNumber to increase as well, which is not permitted. With the patch, PcdCpuMaxLogicalProcessorNumber stays the same, namely "MachineState.smp.max_cpus". Therefore, the CPU structures allocated during normal boot can accommodate the CPUs at S3 resume that have been hotplugged prior to S3 suspend.) Cc: Anthony Perard Cc: Ard Biesheuvel Cc: Igor Mammedov Cc: Jordan Justen Cc: Julien Grall Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515 Signed-off-by: Laszlo Ersek Message-Id: <20191022221554.14963-4-lersek@redhat.com> Acked-by: Anthony PERARD Reviewed-by: Philippe Mathieu-Daude Acked-by: Ard Biesheuvel --- OvmfPkg/OvmfPkgIa32.dsc | 2 +- OvmfPkg/OvmfPkgIa32X64.dsc | 2 +- OvmfPkg/OvmfPkgX64.dsc | 2 +- OvmfPkg/PlatformPei/Platform.c | 170 ++++++++++++++++++++++++---- OvmfPkg/PlatformPei/PlatformPei.inf | 2 +- 5 files changed, 150 insertions(+), 28 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 1da6cc7f23..4568b78cad 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -558,7 +558,7 @@ # 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 213e70345f..152b5d0671 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -570,7 +570,7 @@ # 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 4d0173dfd2..4bfad441bd 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -569,7 +569,7 @@ # 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/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 3ba2459872..e5e8581752 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -30,7 +30,10 @@ #include #include #include +#include #include +#include +#include #include #include "Platform.h" @@ -564,43 +567,161 @@ S3Verification ( /** - Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg modules. - Set the mMaxCpuCount variable. + Fetch the boot CPU count and the possible CPU count from QEMU, and expose + them to UefiCpuPkg modules. Set the mMaxCpuCount variable. **/ VOID MaxCpuCountInitialization ( VOID ) { - UINT16 ProcessorCount; + UINT16 BootCpuCount; RETURN_STATUS PcdStatus; - 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. + // Try to fetch the boot CPU count. // - if (ProcessorCount == 0) { + QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount); + BootCpuCount = QemuFwCfgRead16 (); + if (BootCpuCount == 0) { + // + // QEMU doesn't report the boot CPU count. (BootCpuCount == 0) will 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 the possible CPU count. + // + UINTN CpuHpBase; + UINT32 CmdData2; + + CpuHpBase = ((mHostBridgeDevId == INTEL_Q35_MCH_DEVICE_ID) ? + ICH9_CPU_HOTPLUG_BASE : PIIX4_CPU_HOTPLUG_BASE); + + // + // If only legacy mode is available in the CPU hotplug register block, or + // the register block is completely missing, then the writes below are + // no-ops. + // + // 1. Switch the hotplug register block to modern mode. + // + IoWrite32 (CpuHpBase + QEMU_CPUHP_W_CPU_SEL, 0); + // + // 2. Select a valid CPU for deterministic reading of + // QEMU_CPUHP_R_CMD_DATA2. + // + // CPU#0 is always valid; it is the always present and non-removable + // BSP. + // + IoWrite32 (CpuHpBase + QEMU_CPUHP_W_CPU_SEL, 0); + // + // 3. Send a command after which QEMU_CPUHP_R_CMD_DATA2 is specified to + // read as zero, and which does not invalidate the selector. (The + // selector may change, but it must not become invalid.) + // + // Send QEMU_CPUHP_CMD_GET_PENDING, as it will prove useful later. + // + IoWrite8 (CpuHpBase + QEMU_CPUHP_W_CMD, QEMU_CPUHP_CMD_GET_PENDING); + // + // 4. Read QEMU_CPUHP_R_CMD_DATA2. + // + // If the register block is entirely missing, then this is an unassigned + // IO read, returning all-bits-one. + // + // If only legacy mode is available, then bit#0 stands for CPU#0 in the + // "CPU present bitmap". CPU#0 is always present. + // + // Otherwise, QEMU_CPUHP_R_CMD_DATA2 is either still reserved (returning + // all-bits-zero), or it is specified to read as zero after the above + // steps. Both cases confirm modern mode. + // + CmdData2 = IoRead32 (CpuHpBase + QEMU_CPUHP_R_CMD_DATA2); + DEBUG ((DEBUG_VERBOSE, "%a: CmdData2=0x%x\n", __FUNCTION__, CmdData2)); + if (CmdData2 != 0) { + // + // QEMU doesn't support the modern CPU hotplug interface. Assume that the + // possible CPU count equals the boot CPU count (precluding hotplug). + // + DEBUG ((DEBUG_WARN, "%a: modern CPU hotplug interface unavailable\n", + __FUNCTION__)); + mMaxCpuCount = BootCpuCount; + } else { + // + // Grab the possible CPU count from the modern CPU hotplug interface. + // + UINT32 Present, Possible, Selected; + + Present = 0; + Possible = 0; + + // + // We've sent QEMU_CPUHP_CMD_GET_PENDING last; this ensures + // QEMU_CPUHP_RW_CMD_DATA can now be read usefully. However, + // QEMU_CPUHP_CMD_GET_PENDING may have selected a CPU with actual pending + // hotplug events; therefore, select CPU#0 forcibly. + // + IoWrite32 (CpuHpBase + QEMU_CPUHP_W_CPU_SEL, Possible); + + do { + UINT8 CpuStatus; + + // + // Read the status of the currently selected CPU. This will help with a + // sanity check against "BootCpuCount". + // + CpuStatus = IoRead8 (CpuHpBase + QEMU_CPUHP_R_CPU_STAT); + if ((CpuStatus & QEMU_CPUHP_STAT_ENABLED) != 0) { + ++Present; + } + // + // Attempt to select the next CPU. + // + ++Possible; + IoWrite32 (CpuHpBase + QEMU_CPUHP_W_CPU_SEL, Possible); + // + // If the selection is successful, then the following read will return + // the selector (which we know is positive at this point). Otherwise, + // the read will return 0. + // + Selected = IoRead32 (CpuHpBase + QEMU_CPUHP_RW_CMD_DATA); + ASSERT (Selected == Possible || Selected == 0); + } while (Selected > 0); + + // + // Sanity check: fw_cfg and the modern CPU hotplug interface should + // return the same boot CPU count. + // + if (BootCpuCount != Present) { + DEBUG ((DEBUG_WARN, "%a: QEMU v2.7 reset bug: BootCpuCount=%d " + "Present=%u\n", __FUNCTION__, BootCpuCount, Present)); + // + // The handling of QemuFwCfgItemSmpCpuCount, across CPU hotplug plus + // platform reset (including S3), was corrected in QEMU commit + // e3cadac073a9 ("pc: fix FW_CFG_NB_CPUS to account for -device added + // CPUs", 2016-11-16), part of release v2.8.0. + // + BootCpuCount = (UINT16)Present; + } + + mMaxCpuCount = Possible; + } } - // - // 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); + + DEBUG ((DEBUG_INFO, "%a: BootCpuCount=%d mMaxCpuCount=%u\n", __FUNCTION__, + BootCpuCount, mMaxCpuCount)); + ASSERT (BootCpuCount <= mMaxCpuCount); + + PcdStatus = PcdSet32S (PcdCpuBootLogicalProcessorNumber, BootCpuCount); ASSERT_RETURN_ERROR (PcdStatus); - PcdStatus = PcdSet32S (PcdCpuApInitTimeOutInMicroSeconds, MAX_UINT32); + PcdStatus = PcdSet32S (PcdCpuMaxLogicalProcessorNumber, mMaxCpuCount); ASSERT_RETURN_ERROR (PcdStatus); - DEBUG ((DEBUG_INFO, "%a: QEMU reports %d processor(s)\n", __FUNCTION__, - ProcessorCount)); } @@ -638,13 +759,14 @@ InitializePlatform ( S3Verification (); BootModeInitialization (); AddressWidthInitialization (); - MaxCpuCountInitialization (); // // Query Host Bridge DID // mHostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID); + MaxCpuCountInitialization (); + if (FeaturePcdGet (PcdSmmSmramRequire)) { Q35TsegMbytesInitialization (); } diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index d9fd9c8f05..30eaebdfae 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -98,7 +98,7 @@ gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize [FixedPcd] -- 2.39.2