diff options
| author | Laszlo Ersek <lersek@redhat.com> | 2020-02-26 23:11:42 +0100 |
|---|---|---|
| committer | mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> | 2020-03-04 12:22:07 +0000 |
| commit | 90e11edd16c7a97e3d5fd79f67acaab7c2777d79 (patch) | |
| tree | 090616b4e6ad1e3cb80af1218700753ad081c81e /UefiCpuPkg/PiSmmCpuDxeSmm | |
| parent | a1ddad95933eeb54519e1366c3568c7cac8b25b9 (diff) | |
| download | edk2-90e11edd16c7a97e3d5fd79f67acaab7c2777d79.tar.gz edk2-90e11edd16c7a97e3d5fd79f67acaab7c2777d79.tar.bz2 edk2-90e11edd16c7a97e3d5fd79f67acaab7c2777d79.zip | |
UefiCpuPkg/PiSmmCpuDxeSmm: fix S3 Resume for CPU hotplug
The "ACPI_CPU_DATA.NumberOfCpus" field is specified as follows, in
"UefiCpuPkg/Include/AcpiCpuData.h" (rewrapped for this commit message):
//
// The number of CPUs. If a platform does not support hot plug CPUs,
// then this is the number of CPUs detected when the platform is booted,
// regardless of being enabled or disabled. If a platform does support
// hot plug CPUs, then this is the maximum number of CPUs that the
// platform supports.
//
The InitializeCpuBeforeRebase() and InitializeCpuAfterRebase() functions
in "UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c" try to restore CPU configuration on
the S3 Resume path for *all* CPUs accounted for in
"ACPI_CPU_DATA.NumberOfCpus". This is wrong, as with CPU hotplug, not all
of the possible CPUs may be present at the time of S3 Suspend / Resume.
The symptom is an infinite wait.
Instead, the "mNumberOfCpus" variable should be used, which is properly
maintained through the EFI_SMM_CPU_SERVICE_PROTOCOL implementation (see
SmmAddProcessor(), SmmRemoveProcessor(), SmmCpuUpdate() in
"UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c").
When CPU hotplug is disabled, "mNumberOfCpus" is constant, and equals
"ACPI_CPU_DATA.NumberOfCpus" at all times.
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1512
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200226221156.29589-3-lersek@redhat.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Eric Dong <eric.dong@intel.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
[lersek@redhat.com: shut up UINTN->UINT32 warning from Windows VS2019 PR]
Diffstat (limited to 'UefiCpuPkg/PiSmmCpuDxeSmm')
| -rw-r--r-- | UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 14 |
1 files changed, 12 insertions, 2 deletions
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index ba5cc0194c..29e9ba92b4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -616,7 +616,12 @@ InitializeCpuBeforeRebase ( PrepareApStartupVector (mAcpiCpuData.StartupVector);
- mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1;
+ if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
+ ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus);
+ } else {
+ ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);
+ }
+ mNumberToFinish = (UINT32)(mNumberOfCpus - 1);
mExchangeInfo->ApFunction = (VOID *) (UINTN) InitializeAp;
//
@@ -646,7 +651,12 @@ InitializeCpuAfterRebase ( VOID
)
{
- mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1;
+ if (FeaturePcdGet (PcdCpuHotPlugSupport)) {
+ ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus);
+ } else {
+ ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);
+ }
+ mNumberToFinish = (UINT32)(mNumberOfCpus - 1);
//
// Signal that SMM base relocation is complete and to continue initialization for all APs.
|