From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Tue, 08 Oct 2019 04:27:26 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BC2433B77; Tue, 8 Oct 2019 11:27:25 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-109.rdu2.redhat.com [10.10.120.109]) by smtp.corp.redhat.com (Postfix) with ESMTP id D6333600CE; Tue, 8 Oct 2019 11:27:23 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Anthony Perard , Ard Biesheuvel , Eric Dong , Igor Mammedov , Jordan Justen , Julien Grall , Ray Ni Subject: [PATCH 4/4] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug Date: Tue, 8 Oct 2019 13:27:14 +0200 Message-Id: <20191008112714.6215-5-lersek@redhat.com> In-Reply-To: <20191008112714.6215-1-lersek@redhat.com> References: <20191008112714.6215-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 08 Oct 2019 11:27:25 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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, whic= h 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 no= w 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 i= s 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 Cc: Ard Biesheuvel Cc: Eric Dong Cc: Igor Mammedov Cc: Jordan Justen Cc: Julien Grall Cc: Ray Ni Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1515 Signed-off-by: Laszlo Ersek --- 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] =20 # UefiCpuPkg PCDs related to initial AP bringup and general AP managem= ent. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 =20 # 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] =20 # UefiCpuPkg PCDs related to initial AP bringup and general AP managem= ent. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 =20 # 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] =20 # UefiCpuPkg PCDs related to initial AP bringup and general AP managem= ent. gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 + gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0 =20 # Set memory encryption mask gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask|0x0 diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/Pl= atformPei.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 =20 [FixedPcd] diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platfor= m.c index 3ba2459872d9..12cec1d60ec1 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -564,43 +564,72 @@ S3Verification ( =20 =20 /** - Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg mo= dules. - 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; =20 + // + // Try to fetch the boot CPU count. + // QemuFwCfgSelectItem (QemuFwCfgItemSmpCpuCount); - ProcessorCount =3D QemuFwCfgRead16 (); - // - // If the fw_cfg key or fw_cfg entirely is unavailable, load mMaxCpuCo= unt - // from the PCD default. No change to PCDs. - // - if (ProcessorCount =3D=3D 0) { + BootCpuCount =3D QemuFwCfgRead16 (); + if (BootCpuCount =3D=3D 0) { + // + // QEMU doesn't report the boot CPU count. Let MpInitLib count APs u= p to + // (PcdCpuMaxLogicalProcessorNumber-1), or until + // PcdCpuApInitTimeOutInMicroSeconds elapses (whichever is reached f= irst). + // + DEBUG ((DEBUG_WARN, "%a: boot CPU count unavailable\n", __FUNCTION__= )); mMaxCpuCount =3D PcdGet32 (PcdCpuMaxLogicalProcessorNumber); - return; + } else { + // + // We will expose BootCpuCount to MpInitLib. MpInitLib will count AP= s 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 =3D QemuFwCfgFindFile ("etc/x86-smp-topology", &FwCfgItem, + &FwCfgSize); + if (RETURN_ERROR (Status) || FwCfgSize !=3D sizeof Topology) { + // + // QEMU doesn't report the topology. Assume that the maximum CPU c= ount + // equals the boot CPU count (implying no hotplug). + // + DEBUG ((DEBUG_WARN, "%a: topology unavailable\n", __FUNCTION__)); + mMaxCpuCount =3D BootCpuCount; + } else { + // + // Grab the maximum CPU count from the topology info. + // + QemuFwCfgSelectItem (FwCfgItem); + QemuFwCfgReadBytes (FwCfgSize, &Topology); + DEBUG ((DEBUG_VERBOSE, + "%a: DiesPerSocket=3D%u CoresPerDie=3D%u ThreadsPerCore=3D%u\n", + __FUNCTION__, Topology.DiesPerSocket, Topology.CoresPerDie, + Topology.ThreadsPerCore)); + mMaxCpuCount =3D Topology.MaxLogicalProcessors; + } } - // - // Otherwise, set mMaxCpuCount to the value reported by QEMU. - // - mMaxCpuCount =3D 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 ti= meout - // (approx. 71 minutes). - // - PcdStatus =3D PcdSet32S (PcdCpuMaxLogicalProcessorNumber, ProcessorCou= nt); - ASSERT_RETURN_ERROR (PcdStatus); - PcdStatus =3D 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=3D%d mMaxCpuCount=3D%u\n", __FUN= CTION__, + BootCpuCount, mMaxCpuCount)); + ASSERT (BootCpuCount <=3D mMaxCpuCount); + + Status =3D PcdSet32S (PcdCpuBootLogicalProcessorNumber, BootCpuCount); + ASSERT_RETURN_ERROR (Status); + Status =3D PcdSet32S (PcdCpuMaxLogicalProcessorNumber, mMaxCpuCount); + ASSERT_RETURN_ERROR (Status); } =20 =20 --=20 2.19.1.3.g30247aa5d201