UefiCpuPkg/MpInitLib: honor the platform's boot CPU count in AP detection

- If a platform boots such that the boot CPU count is smaller than
  PcdCpuMaxLogicalProcessorNumber, then the platform cannot use the "fast
  AP detection" logic added in commit 6e1987f19af7. (Which has been
  documented as a subset of use case (2) in the previous patch.)

  Said logic depends on the boot CPU count being equal to
  PcdCpuMaxLogicalProcessorNumber. If the equality does not hold, the
  platform either has to wait too long, or risk missing APs due to an
  early timeout.

- The platform may not be able to use the variant added in commit
  0594ec417c89 either. (Which has been documented as use case (1) in the
  previous patch.)

  See commit 861218740d6d. When OVMF runs on QEMU/KVM, APs may check in
  with the BSP in arbitrary order, plus the individual AP may take
  arbitrarily long to check-in. If "NumApsExecuting" falls to zero
  mid-enumeration, APs will be missed.

Allow platforms to specify the exact boot CPU count, independently of
PcdCpuMaxLogicalProcessorNumber. In this mode, the BSP waits for all APs
to check-in regardless of timeout. If at least one AP fails to check-in,
then the AP enumeration hangs forever. That is the desired behavior when
the exact boot CPU count is known in advance. (A hung boot is better than
an AP checking-in after timeout, and executing code from released
storage.)

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1515
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 37b3f64e578aca946b50c155814beb4ebace5f1b..cd912ab0c5ee483ed5a03cabe6675a1338cf2693 100644 (file)
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -61,6 +61,7 @@
 \r
 [Pcd]\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## CONSUMES\r
 \r
 [Pcd]\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## CONSUMES\r

+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber       ## CONSUMES\r

   gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds      ## SOMETIMES_CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                      ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress            ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds      ## SOMETIMES_CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                      ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress            ## CONSUMES\r

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 594a035d8b927ec031daff272c600d7c81dfd447..622b70ca3c4ecd60c031c3afd2c86edb149356cd 100644 (file)
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1044,46 +1044,67 @@ WakeUpAP (
       SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart);\r
     }\r
     if (CpuMpData->InitFlag == ApInitConfig) {\r
       SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart);\r
     }\r
     if (CpuMpData->InitFlag == ApInitConfig) {\r

-      //\r
-      // The AP enumeration algorithm below is suitable for two use cases.\r
-      //\r
-      // (1) The check-in time for an individual AP is bounded, and APs run\r
-      //     through their initialization routines strongly concurrently. In\r
-      //     particular, the number of concurrently running APs\r
-      //     ("NumApsExecuting") is never expected to fall to zero\r
-      //     *temporarily* -- it is expected to fall to zero only when all\r
-      //     APs have checked-in.\r
-      //\r
-      //     In this case, the platform is supposed to set\r
-      //     PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long\r
-      //     enough for one AP to start initialization). The timeout will be\r
-      //     reached soon, and remaining APs are collected by watching\r
-      //     NumApsExecuting fall to zero. If NumApsExecuting falls to zero\r
-      //     mid-process, while some APs have not completed initialization,\r
-      //     the behavior is undefined.\r
-      //\r
-      // (2) The check-in time for an individual AP is unbounded, and/or APs\r
-      //     may complete their initializations widely spread out. In\r
-      //     particular, some APs may finish initialization before some APs\r
-      //     even start.\r
-      //\r
-      //     In this case, the platform is supposed to set\r
-      //     PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP\r
-      //     enumeration will always take that long (except when the boot CPU\r
-      //     count happens to be maximal, that is,\r
-      //     PcdCpuMaxLogicalProcessorNumber). All APs are expected to\r
-      //     check-in before the timeout, and NumApsExecuting is assumed zero\r
-      //     at timeout. APs that miss the time-out may cause undefined\r
-      //     behavior.\r
-      //\r
-      TimedWaitForApFinish (\r
-        CpuMpData,\r
-        PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,\r
-        PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)\r
-        );\r
+      if (PcdGet32 (PcdCpuBootLogicalProcessorNumber) > 0) {\r
+        //\r
+        // The AP enumeration algorithm below is suitable only when the\r
+        // platform can tell us the *exact* boot CPU count in advance.\r
+        //\r
+        // The wait below finishes only when the detected AP count reaches\r
+        // (PcdCpuBootLogicalProcessorNumber - 1), regardless of how long that\r
+        // takes. If at least one AP fails to check in (meaning a platform\r
+        // hardware bug), the detection hangs forever, by design. If the actual\r
+        // boot CPU count in the system is higher than\r
+        // PcdCpuBootLogicalProcessorNumber (meaning a platform\r
+        // misconfiguration), then some APs may complete initialization after\r
+        // the wait finishes, and cause undefined behavior.\r
+        //\r
+        TimedWaitForApFinish (\r
+          CpuMpData,\r
+          PcdGet32 (PcdCpuBootLogicalProcessorNumber) - 1,\r
+          MAX_UINT32 // approx. 71 minutes\r
+          );\r
+      } else {\r
+        //\r
+        // The AP enumeration algorithm below is suitable for two use cases.\r
+        //\r
+        // (1) The check-in time for an individual AP is bounded, and APs run\r
+        //     through their initialization routines strongly concurrently. In\r
+        //     particular, the number of concurrently running APs\r
+        //     ("NumApsExecuting") is never expected to fall to zero\r
+        //     *temporarily* -- it is expected to fall to zero only when all\r
+        //     APs have checked-in.\r
+        //\r
+        //     In this case, the platform is supposed to set\r
+        //     PcdCpuApInitTimeOutInMicroSeconds to a low-ish value (just long\r
+        //     enough for one AP to start initialization). The timeout will be\r
+        //     reached soon, and remaining APs are collected by watching\r
+        //     NumApsExecuting fall to zero. If NumApsExecuting falls to zero\r
+        //     mid-process, while some APs have not completed initialization,\r
+        //     the behavior is undefined.\r
+        //\r
+        // (2) The check-in time for an individual AP is unbounded, and/or APs\r
+        //     may complete their initializations widely spread out. In\r
+        //     particular, some APs may finish initialization before some APs\r
+        //     even start.\r
+        //\r
+        //     In this case, the platform is supposed to set\r
+        //     PcdCpuApInitTimeOutInMicroSeconds to a high-ish value. The AP\r
+        //     enumeration will always take that long (except when the boot CPU\r
+        //     count happens to be maximal, that is,\r
+        //     PcdCpuMaxLogicalProcessorNumber). All APs are expected to\r
+        //     check-in before the timeout, and NumApsExecuting is assumed zero\r
+        //     at timeout. APs that miss the time-out may cause undefined\r
+        //     behavior.\r
+        //\r
+        TimedWaitForApFinish (\r
+          CpuMpData,\r
+          PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,\r
+          PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)\r
+          );\r

 \r
 \r

-      while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {\r
-        CpuPause();\r
+        while (CpuMpData->MpCpuExchangeInfo->NumApsExecuting != 0) {\r
+          CpuPause();\r
+        }\r

       }\r
     } else {\r
       //\r
       }\r
     } else {\r
       //\r

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 82b77b63ea877011479ad6541f3a99e19b1c56fd..1538185ef99a52d123c03999c87d2d5a76d6e082 100644 (file)
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -53,7 +53,8 @@
 \r
 [Pcd]\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## CONSUMES\r
 \r
 [Pcd]\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## CONSUMES\r

-  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds      ## CONSUMES\r
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber       ## CONSUMES\r
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds      ## SOMETIMES_CONSUMES\r

   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                      ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress            ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize         ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize                      ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress            ## CONSUMES\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize         ## CONSUMES\r

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 031a2ccd680ace8663b9b49e24d23065632ae5d7..12f4413ea5b01ec4441f4c8fa893ec2fa58b1ab0 100644 (file)
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -227,6 +227,19 @@
   ## Specifies timeout value in microseconds for the BSP to detect all APs for the first time.\r
   # @Prompt Timeout for the BSP to detect all APs for the first time.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000|UINT32|0x00000004\r
   ## Specifies timeout value in microseconds for the BSP to detect all APs for the first time.\r
   # @Prompt Timeout for the BSP to detect all APs for the first time.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000|UINT32|0x00000004\r

+  ## Specifies the number of Logical Processors that are available in the\r
+  #  preboot environment after platform reset, including BSP and APs. Possible\r
+  #  values:<BR><BR>\r
+  #  zero (default) - PcdCpuBootLogicalProcessorNumber is ignored, and\r
+  #                   PcdCpuApInitTimeOutInMicroSeconds limits the initial AP\r
+  #                   detection by the BSP.<BR>\r
+  #  nonzero        - PcdCpuApInitTimeOutInMicroSeconds is ignored. The initial\r
+  #                   AP detection finishes only when the detected CPU count\r
+  #                   (BSP plus APs) reaches the value of\r
+  #                   PcdCpuBootLogicalProcessorNumber, regardless of how long\r
+  #                   that takes.<BR>\r
+  # @Prompt Number of Logical Processors available after platform reset.\r
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0|UINT32|0x00000008\r

   ## Specifies the base address of the first microcode Patch in the microcode Region.\r
   # @Prompt Microcode Region base address.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x00000005\r
   ## Specifies the base address of the first microcode Patch in the microcode Region.\r
   # @Prompt Microcode Region base address.\r
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress|0x0|UINT64|0x00000005\r

diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index fbf7680726683171ee0d4c97116218ff19a139d3..a7e279c5cb14eae3e8447cb9fed391a750b0dbca 100644 (file)
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -37,6 +37,10 @@
 \r
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApInitTimeOutInMicroSeconds_HELP  #language en-US "Specifies timeout value in microseconds for the BSP to detect all APs for the first time."\r
 \r
 \r
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuApInitTimeOutInMicroSeconds_HELP  #language en-US "Specifies timeout value in microseconds for the BSP to detect all APs for the first time."\r
 \r

+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_PROMPT  #language en-US "Number of Logical Processors available after platform reset."\r
+\r
+#string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuBootLogicalProcessorNumber_HELP  #language en-US "Specifies the number of Logical Processors that are available in the preboot environment after platform reset, including BSP and APs."\r
+\r

 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_PROMPT  #language en-US "Microcode Region base address."\r
 \r
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_HELP  #language en-US "Specifies the base address of the first microcode Patch in the microcode Region."\r
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_PROMPT  #language en-US "Microcode Region base address."\r
 \r
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuMicrocodePatchAddress_HELP  #language en-US "Specifies the base address of the first microcode Patch in the microcode Region."\r