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]

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
index ba5cc0194c2d2121e1efdc1a3cb0a424e3349e1e..29e9ba92b453ea7760d220c395d4601cb93f2a0d 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
@@ -616,7 +616,12 @@ InitializeCpuBeforeRebase (
 \r
   PrepareApStartupVector (mAcpiCpuData.StartupVector);\r
 \r
-  mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1;\r
+  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {\r
+    ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus);\r
+  } else {\r
+    ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);\r
+  }\r
+  mNumberToFinish = (UINT32)(mNumberOfCpus - 1);\r
   mExchangeInfo->ApFunction  = (VOID *) (UINTN) InitializeAp;\r
 \r
   //\r
@@ -646,7 +651,12 @@ InitializeCpuAfterRebase (
   VOID\r
   )\r
 {\r
-  mNumberToFinish = mAcpiCpuData.NumberOfCpus - 1;\r
+  if (FeaturePcdGet (PcdCpuHotPlugSupport)) {\r
+    ASSERT (mNumberOfCpus <= mAcpiCpuData.NumberOfCpus);\r
+  } else {\r
+    ASSERT (mNumberOfCpus == mAcpiCpuData.NumberOfCpus);\r
+  }\r
+  mNumberToFinish = (UINT32)(mNumberOfCpus - 1);\r
 \r
   //\r
   // Signal that SMM base relocation is complete and to continue initialization for all APs.\r