public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically
@ 2016-11-28 13:08 Laszlo Ersek
  2016-11-28 13:08 ` [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup Laszlo Ersek
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Laszlo Ersek @ 2016-11-28 13:08 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Igor Mammedov, Jeff Fan, Jordan Justen, Michael Kinney

This is the v3 update on
<https://lists.01.org/pipermail/edk2-devel/2016-November/005207.html>.

Changes in this version [Jeff]:
- preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
- use MAX_UINT32 (~71 minutes) as "infinite wait"

Repo:   https://github.com/lersek/edk2/
Branch: mpinit_v3

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>

Thanks
Laszlo

Laszlo Ersek (2):
  UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
    startup
  OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib

 OvmfPkg/OvmfPkgIa32.dsc              |  4 ++
 OvmfPkg/OvmfPkgIa32X64.dsc           |  4 ++
 OvmfPkg/OvmfPkgX64.dsc               |  4 ++
 OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
 OvmfPkg/PlatformPei/Platform.h       |  2 +
 OvmfPkg/PlatformPei/MemDetect.c      |  2 +-
 OvmfPkg/PlatformPei/Platform.c       | 43 ++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
 8 files changed, 131 insertions(+), 2 deletions(-)

-- 
2.9.2



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup
  2016-11-28 13:08 [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Laszlo Ersek
@ 2016-11-28 13:08 ` Laszlo Ersek
  2016-11-29  0:42   ` Fan, Jeff
  2016-11-28 13:08 ` [PATCH v3 2/2] OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib Laszlo Ersek
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2016-11-28 13:08 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Igor Mammedov, Jeff Fan, Jordan Justen, Michael Kinney

Sometimes a platform knows exactly how many CPUs it has at boot. It should
be able to
- set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
- set PcdCpuApInitTimeOutInMicroSeconds to a very long time (for example
  MAX_UINT32, approx. 71 minutes),
- and expect that MpInitLib wait exactly as long as necessary for all APs
  to report in.

Other platforms should be able to continue setting a reasonably large
upper bound on supported CPUs, and waiting for a reasonable, fixed amount
of time for all APs to report in.

Add this functionality. The TimedWaitForApFinish() function will return
when all APs have reported in, or the timeout has expired -- whichever
happens first.

(Accessing these PCDs dynamically is safe. The PEI and DXE phase instances
of this library are restricted to PEIM and DXE_DRIVER client modules, thus
the PCD accesses cannot be linked into runtime code.)

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
      [Jeff]

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 15dbfa1e7d6c..ed22ce67782e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -684,6 +684,21 @@ FillExchangeInfoData (
 }
 
 /**
+  Helper function that waits until the finished AP count reaches the specified
+  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpData        Pointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimit        The number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA               *CpuMpData,
+  IN UINT32                    FinishedApLimit,
+  IN UINT32                    TimeLimit
+  );
+
+/**
   This function will be called by BSP to wakeup AP.
 
   @param[in] CpuMpData          Pointer to CPU MP Data
@@ -748,7 +763,11 @@ WakeUpAP (
       //
       // Wait for all potential APs waken up in one specified period
       //
-      MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
+      TimedWaitForApFinish (
+        CpuMpData,
+        PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
+        PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
+        );
     } else {
       //
       // Wait all APs waken up if this is not the 1st broadcast of SIPI
@@ -895,6 +914,58 @@ CheckTimeout (
 }
 
 /**
+  Helper function that waits until the finished AP count reaches the specified
+  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpData        Pointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimit        The number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA               *CpuMpData,
+  IN UINT32                    FinishedApLimit,
+  IN UINT32                    TimeLimit
+  )
+{
+  //
+  // CalculateTimeout() and CheckTimeout() consider a TimeLimit of 0
+  // "infinity", so check for (TimeLimit == 0) explicitly.
+  //
+  if (TimeLimit == 0) {
+    return;
+  }
+
+  CpuMpData->TotalTime = 0;
+  CpuMpData->ExpectedTime = CalculateTimeout (
+                              TimeLimit,
+                              &CpuMpData->CurrentTime
+                              );
+  while (CpuMpData->FinishedCount < FinishedApLimit &&
+         !CheckTimeout (
+            &CpuMpData->CurrentTime,
+            &CpuMpData->TotalTime,
+            CpuMpData->ExpectedTime
+            )) {
+    CpuPause ();
+  }
+
+  if (CpuMpData->FinishedCount >= FinishedApLimit) {
+    DEBUG ((
+      DEBUG_VERBOSE,
+      "%a: reached FinishedApLimit=%u in %Lu microseconds\n",
+      __FUNCTION__,
+      FinishedApLimit,
+      DivU64x64Remainder (
+        MultU64x32 (CpuMpData->TotalTime, 1000000),
+        GetPerformanceCounterProperties (NULL, NULL),
+        NULL
+        )
+      ));
+  }
+}
+
+/**
   Reset an AP to Idle state.
 
   Any task being executed by the AP will be aborted and the AP
-- 
2.9.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
  2016-11-28 13:08 [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Laszlo Ersek
  2016-11-28 13:08 ` [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup Laszlo Ersek
@ 2016-11-28 13:08 ` Laszlo Ersek
  2016-11-28 19:24 ` [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Jordan Justen
  2016-11-29  9:11 ` Laszlo Ersek
  3 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2016-11-28 13:08 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Igor Mammedov, Jeff Fan, Jordan Justen, Michael Kinney

These settings will allow CpuMpPei and CpuDxe to wait for the initial AP
check-ins exactly as long as necessary.

It is safe to set PcdCpuMaxLogicalProcessorNumber and
PcdCpuApInitTimeOutInMicroSeconds in OvmfPkg/PlatformPei.
OvmfPkg/PlatformPei installs the permanent PEI RAM, producing
gEfiPeiMemoryDiscoveredPpiGuid, and UefiCpuPkg/CpuMpPei has a depex on
gEfiPeiMemoryDiscoveredPpiGuid.

It is safe to read the fw_cfg item QemuFwCfgItemSmpCpuCount (0x0005). It
was added to QEMU in 2008 as key FW_CFG_NB_CPUS, in commit 905fdcb5264c
("Add common keys to firmware configuration"). Even if the key is
unavailable (or if fw_cfg is entirely unavailable, for example on Xen),
QemuFwCfgRead16() will return 0, and then we stick with the current
behavior.

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - We now preserve the original behavior for
      PcdCpuApInitTimeOutInMicroSeconds==0 in UefiCpuPkg/MpInitLib, in the
      previous patch, so represent the "infinite" timeout with the longest
      representable timeout (MAX_UINT32 microseconds, cca. 71 minutes)
      [Jeff]

 OvmfPkg/OvmfPkgIa32.dsc             |  4 ++
 OvmfPkg/OvmfPkgIa32X64.dsc          |  4 ++
 OvmfPkg/OvmfPkgX64.dsc              |  4 ++
 OvmfPkg/PlatformPei/PlatformPei.inf |  1 +
 OvmfPkg/PlatformPei/Platform.h      |  2 +
 OvmfPkg/PlatformPei/MemDetect.c     |  2 +-
 OvmfPkg/PlatformPei/Platform.c      | 43 ++++++++++++++++++++
 7 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index ed43c4514491..d91303052263 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -488,6 +488,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index ccd156d9231a..8143ea9c2837 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -496,6 +496,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 012ce85462c5..d48d603d33d4 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -495,6 +495,10 @@ [PcdsDynamicDefault]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE
 
+  # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 776a4ab11f79..fbaed3182dcf 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -95,6 +95,7 @@ [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize
 
 [FixedPcd]
diff --git a/OvmfPkg/PlatformPei/Platform.h b/OvmfPkg/PlatformPei/Platform.h
index eda765be30de..18f42c3f0ea8 100644
--- a/OvmfPkg/PlatformPei/Platform.h
+++ b/OvmfPkg/PlatformPei/Platform.h
@@ -101,4 +101,6 @@ extern BOOLEAN mS3Supported;
 
 extern UINT8 mPhysMemAddressWidth;
 
+extern UINT32 mMaxCpuCount;
+
 #endif // _PLATFORM_PEI_H_INCLUDED_
diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index d00a570d4381..af96a04d194a 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -358,7 +358,7 @@ PublishPeiMemory (
   //
   if (mS3Supported) {
     mS3AcpiReservedMemorySize = SIZE_512KB +
-      PcdGet32 (PcdCpuMaxLogicalProcessorNumber) *
+      mMaxCpuCount *
       PcdGet32 (PcdCpuApStackSize);
     mS3AcpiReservedMemoryBase = LowerMemorySize - mS3AcpiReservedMemorySize;
     LowerMemorySize = mS3AcpiReservedMemoryBase;
diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index c6e1106c9ed0..0be86722e548 100644
--- a/OvmfPkg/PlatformPei/Platform.c
+++ b/OvmfPkg/PlatformPei/Platform.c
@@ -68,6 +68,7 @@ EFI_BOOT_MODE mBootMode = BOOT_WITH_FULL_CONFIGURATION;
 
 BOOLEAN mS3Supported = FALSE;
 
+UINT32 mMaxCpuCount;
 
 VOID
 AddIoMemoryBaseSizeHob (
@@ -568,6 +569,47 @@ S3Verification (
 
 
 /**
+  Fetch the number of boot CPUs from QEMU and expose it to UefiCpuPkg modules.
+  Set the mMaxCpuCount variable.
+**/
+VOID
+MaxCpuCountInitialization (
+  VOID
+  )
+{
+  UINT16        ProcessorCount;
+  RETURN_STATUS PcdStatus;
+
+  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) {
+    mMaxCpuCount = PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+    return;
+  }
+  //
+  // 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));
+}
+
+
+/**
   Perform Platform PEI initialization.
 
   @param  FileHandle      Handle of the file being invoked.
@@ -601,6 +643,7 @@ InitializePlatform (
   S3Verification ();
   BootModeInitialization ();
   AddressWidthInitialization ();
+  MaxCpuCountInitialization ();
 
   PublishPeiMemory ();
 
-- 
2.9.2



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically
  2016-11-28 13:08 [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Laszlo Ersek
  2016-11-28 13:08 ` [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup Laszlo Ersek
  2016-11-28 13:08 ` [PATCH v3 2/2] OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib Laszlo Ersek
@ 2016-11-28 19:24 ` Jordan Justen
  2016-11-28 19:58   ` Laszlo Ersek
  2016-11-29  9:11 ` Laszlo Ersek
  3 siblings, 1 reply; 7+ messages in thread
From: Jordan Justen @ 2016-11-28 19:24 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01; +Cc: Igor Mammedov, Jeff Fan, Michael Kinney

Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On 2016-11-28 05:08:37, Laszlo Ersek wrote:
> This is the v3 update on
> <https://lists.01.org/pipermail/edk2-devel/2016-November/005207.html>.
> 
> Changes in this version [Jeff]:
> - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
> - use MAX_UINT32 (~71 minutes) as "infinite wait"
> 
> Repo:   https://github.com/lersek/edk2/
> Branch: mpinit_v3
> 
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
>     startup
>   OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
> 
>  OvmfPkg/OvmfPkgIa32.dsc              |  4 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc           |  4 ++
>  OvmfPkg/OvmfPkgX64.dsc               |  4 ++
>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>  OvmfPkg/PlatformPei/Platform.h       |  2 +
>  OvmfPkg/PlatformPei/MemDetect.c      |  2 +-
>  OvmfPkg/PlatformPei/Platform.c       | 43 ++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
>  8 files changed, 131 insertions(+), 2 deletions(-)
> 
> -- 
> 2.9.2
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically
  2016-11-28 19:24 ` [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Jordan Justen
@ 2016-11-28 19:58   ` Laszlo Ersek
  0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2016-11-28 19:58 UTC (permalink / raw)
  To: Jordan Justen, edk2-devel-01; +Cc: Igor Mammedov, Jeff Fan, Michael Kinney

On 11/28/16 20:24, Jordan Justen wrote:
> Series Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

Thanks Jordan! :) I hope Jeff too will accept this version.

Cheers!
Laszlo

> On 2016-11-28 05:08:37, Laszlo Ersek wrote:
>> This is the v3 update on
>> <https://lists.01.org/pipermail/edk2-devel/2016-November/005207.html>.
>>
>> Changes in this version [Jeff]:
>> - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
>> - use MAX_UINT32 (~71 minutes) as "infinite wait"
>>
>> Repo:   https://github.com/lersek/edk2/
>> Branch: mpinit_v3
>>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Jeff Fan <jeff.fan@intel.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Michael Kinney <michael.d.kinney@intel.com>
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (2):
>>   UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
>>     startup
>>   OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
>>
>>  OvmfPkg/OvmfPkgIa32.dsc              |  4 ++
>>  OvmfPkg/OvmfPkgIa32X64.dsc           |  4 ++
>>  OvmfPkg/OvmfPkgX64.dsc               |  4 ++
>>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>>  OvmfPkg/PlatformPei/Platform.h       |  2 +
>>  OvmfPkg/PlatformPei/MemDetect.c      |  2 +-
>>  OvmfPkg/PlatformPei/Platform.c       | 43 ++++++++++++
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
>>  8 files changed, 131 insertions(+), 2 deletions(-)
>>
>> -- 
>> 2.9.2
>>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup
  2016-11-28 13:08 ` [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup Laszlo Ersek
@ 2016-11-29  0:42   ` Fan, Jeff
  0 siblings, 0 replies; 7+ messages in thread
From: Fan, Jeff @ 2016-11-29  0:42 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-01
  Cc: Igor Mammedov, Justen, Jordan L, Kinney, Michael D

Reviewed-by: Jeff Fan <jeff.fan@intel.com>

Thanks!

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Monday, November 28, 2016 9:09 PM
To: edk2-devel-01
Cc: Igor Mammedov; Fan, Jeff; Justen, Jordan L; Kinney, Michael D
Subject: [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup

Sometimes a platform knows exactly how many CPUs it has at boot. It should be able to
- set PcdCpuMaxLogicalProcessorNumber dynamically to this number,
- set PcdCpuApInitTimeOutInMicroSeconds to a very long time (for example
  MAX_UINT32, approx. 71 minutes),
- and expect that MpInitLib wait exactly as long as necessary for all APs
  to report in.

Other platforms should be able to continue setting a reasonably large upper bound on supported CPUs, and waiting for a reasonable, fixed amount of time for all APs to report in.

Add this functionality. The TimedWaitForApFinish() function will return when all APs have reported in, or the timeout has expired -- whichever happens first.

(Accessing these PCDs dynamically is safe. The PEI and DXE phase instances of this library are restricted to PEIM and DXE_DRIVER client modules, thus the PCD accesses cannot be linked into runtime code.)

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Jeff Fan <jeff.fan@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=116
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v3:
    - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
      [Jeff]

 UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 15dbfa1e7d6c..ed22ce67782e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -684,6 +684,21 @@ FillExchangeInfoData (  }
 
 /**
+  Helper function that waits until the finished AP count reaches the 
+ specified  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpData        Pointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimit        The number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA               *CpuMpData,
+  IN UINT32                    FinishedApLimit,
+  IN UINT32                    TimeLimit
+  );
+
+/**
   This function will be called by BSP to wakeup AP.
 
   @param[in] CpuMpData          Pointer to CPU MP Data
@@ -748,7 +763,11 @@ WakeUpAP (
       //
       // Wait for all potential APs waken up in one specified period
       //
-      MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
+      TimedWaitForApFinish (
+        CpuMpData,
+        PcdGet32 (PcdCpuMaxLogicalProcessorNumber) - 1,
+        PcdGet32 (PcdCpuApInitTimeOutInMicroSeconds)
+        );
     } else {
       //
       // Wait all APs waken up if this is not the 1st broadcast of SIPI @@ -895,6 +914,58 @@ CheckTimeout (  }
 
 /**
+  Helper function that waits until the finished AP count reaches the 
+ specified  limit, or the specified timeout elapses (whichever comes first).
+
+  @param[in] CpuMpData        Pointer to CPU MP Data.
+  @param[in] FinishedApLimit  The number of finished APs to wait for.
+  @param[in] TimeLimit        The number of microseconds to wait for.
+**/
+VOID
+TimedWaitForApFinish (
+  IN CPU_MP_DATA               *CpuMpData,
+  IN UINT32                    FinishedApLimit,
+  IN UINT32                    TimeLimit
+  )
+{
+  //
+  // CalculateTimeout() and CheckTimeout() consider a TimeLimit of 0
+  // "infinity", so check for (TimeLimit == 0) explicitly.
+  //
+  if (TimeLimit == 0) {
+    return;
+  }
+
+  CpuMpData->TotalTime = 0;
+  CpuMpData->ExpectedTime = CalculateTimeout (
+                              TimeLimit,
+                              &CpuMpData->CurrentTime
+                              );
+  while (CpuMpData->FinishedCount < FinishedApLimit &&
+         !CheckTimeout (
+            &CpuMpData->CurrentTime,
+            &CpuMpData->TotalTime,
+            CpuMpData->ExpectedTime
+            )) {
+    CpuPause ();
+  }
+
+  if (CpuMpData->FinishedCount >= FinishedApLimit) {
+    DEBUG ((
+      DEBUG_VERBOSE,
+      "%a: reached FinishedApLimit=%u in %Lu microseconds\n",
+      __FUNCTION__,
+      FinishedApLimit,
+      DivU64x64Remainder (
+        MultU64x32 (CpuMpData->TotalTime, 1000000),
+        GetPerformanceCounterProperties (NULL, NULL),
+        NULL
+        )
+      ));
+  }
+}
+
+/**
   Reset an AP to Idle state.
 
   Any task being executed by the AP will be aborted and the AP
--
2.9.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically
  2016-11-28 13:08 [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Laszlo Ersek
                   ` (2 preceding siblings ...)
  2016-11-28 19:24 ` [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Jordan Justen
@ 2016-11-29  9:11 ` Laszlo Ersek
  3 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2016-11-29  9:11 UTC (permalink / raw)
  To: edk2-devel-01; +Cc: Michael Kinney, Jeff Fan, Jordan Justen

On 11/28/16 14:08, Laszlo Ersek wrote:
> This is the v3 update on
> <https://lists.01.org/pipermail/edk2-devel/2016-November/005207.html>.
> 
> Changes in this version [Jeff]:
> - preserve original behavior for PcdCpuApInitTimeOutInMicroSeconds==0
> - use MAX_UINT32 (~71 minutes) as "infinite wait"
> 
> Repo:   https://github.com/lersek/edk2/
> Branch: mpinit_v3
> 
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (2):
>   UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP
>     startup
>   OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib
> 
>  OvmfPkg/OvmfPkgIa32.dsc              |  4 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc           |  4 ++
>  OvmfPkg/OvmfPkgX64.dsc               |  4 ++
>  OvmfPkg/PlatformPei/PlatformPei.inf  |  1 +
>  OvmfPkg/PlatformPei/Platform.h       |  2 +
>  OvmfPkg/PlatformPei/MemDetect.c      |  2 +-
>  OvmfPkg/PlatformPei/Platform.c       | 43 ++++++++++++
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 73 +++++++++++++++++++-
>  8 files changed, 131 insertions(+), 2 deletions(-)
> 

Thanks all for the reviews, series pushed as 2b2efe33eace..45a70db3c3a5.

Laszlo


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-11-29  9:11 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-28 13:08 [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Laszlo Ersek
2016-11-28 13:08 ` [PATCH v3 1/2] UefiCpuPkg/MpInitLib: wait no longer than necessary for initial AP startup Laszlo Ersek
2016-11-29  0:42   ` Fan, Jeff
2016-11-28 13:08 ` [PATCH v3 2/2] OvmfPkg/PlatformPei: take VCPU count from QEMU and configure MpInitLib Laszlo Ersek
2016-11-28 19:24 ` [PATCH v3 0/2] UefiCpuPkg, OvmfPkg: set maximum (V)CPU count dynamically Jordan Justen
2016-11-28 19:58   ` Laszlo Ersek
2016-11-29  9:11 ` Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox