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]
This commit is contained in:
Laszlo Ersek 2020-02-26 23:11:42 +01:00 committed by mergify[bot]
parent a1ddad9593
commit 90e11edd16
1 changed files with 12 additions and 2 deletions

View File

@ -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.