[edk2-devel] [PATCH 4/4] OvmfPkg/PlatformPei: rewrite MaxCpuCountInitialization() for CPU hotplug
Igor Mammedov
imammedo at redhat.com
Tue Oct 8 15:06:55 UTC 2019
On Tue, 8 Oct 2019 13:27:14 +0200
Laszlo Ersek <lersek at redhat.com> wrote:
> 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.
Can you elaborate why it doesn't 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.
What for max cpu count is/will be used?
>
> 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.
Does OVMF reread boot CPU count on resume from QEMU?
> Cc: Anthony Perard <anthony.perard at citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Igor Mammedov <imammedo at redhat.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Julien Grall <julien.grall at arm.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
> Signed-off-by: Laszlo Ersek <lersek at 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);
> }
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48584): https://edk2.groups.io/g/devel/message/48584
Mute This Topic: https://groups.io/mt/34441232/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list