public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/6] Fixed bugs in MpInitLib
@ 2016-08-24 14:45 Jeff Fan
  2016-08-24 14:45 ` [Patch 1/6] UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp() Jeff Fan
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel

Jeff Fan (6):
  UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp()
  UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code
  UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag
  UefiCpuPkg/MpInitLib: Fix function header comments typo
  UefiCpuPkg/MpInitLib: Move two functions location
  UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service

 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c  | 96 ++++++++++++++++++--------------
 UefiCpuPkg/Library/MpInitLib/Microcode.c |  2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c     | 52 +++++++++++++++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h     | 24 +++++++-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 45 +--------------
 5 files changed, 127 insertions(+), 92 deletions(-)

-- 
2.7.4.windows.1



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

* [Patch 1/6] UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp()
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-24 14:45 ` [Patch 2/6] UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code Jeff Fan
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

After sending the 1st broadcast INIT-SIPI-SIPI, BSP will collect APs count after
one specified timeout delay. However, WakupAp() will restore reset vector
immediately after sending 1st broadcast INIT-SIPI-SIPI. Some processors may not
complete executing reset vector code.

This fix is to move MicroSecondDelay() from CollectProcessorCount() to the place
that is after sending 1st broadcast INIT-SIPI-SIPI and before FreeResetVector()
in WakupAp().

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index fbe2e8b..47a971b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -513,10 +513,6 @@ CollectProcessorCount (
   CpuMpData->InitFlag     = ApInitConfig;
   CpuMpData->X2ApicEnable = FALSE;
   WakeUpAP (CpuMpData, TRUE, 0, NULL, NULL);
-  //
-  // Wait for AP task to complete and then exit.
-  //
-  MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
   CpuMpData->InitFlag = ApInitDone;
   ASSERT (CpuMpData->CpuCount <= PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
   //
@@ -863,7 +859,12 @@ WakeUpAP (
       //
       SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart);
     }
-    if (CpuMpData->InitFlag != ApInitConfig) {
+    if (CpuMpData->InitFlag == ApInitConfig) {
+      //
+      // Wait for all potential APs waken up in one specified period
+      //
+      MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds));
+    } else {
       //
       // Wait all APs waken up if this is not the 1st broadcast of SIPI
       //
-- 
2.7.4.windows.1



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

* [Patch 2/6] UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
  2016-08-24 14:45 ` [Patch 1/6] UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp() Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-24 14:45 ` [Patch 3/6] UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag Jeff Fan
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

In Exit Boot Services callback function, we cannot use allocate memory services
because it may change the memory map that has been gotten by OS.

This fix is to move allocating reserved memory for AP loop code to
InitMpGlobalData() and save the memory address in one global variable.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 42d320f..383eec9 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -23,7 +23,7 @@ CPU_MP_DATA      *mCpuMpData = NULL;
 EFI_EVENT        mCheckAllApsEvent = NULL;
 EFI_EVENT        mMpInitExitBootServicesEvent = NULL;
 volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
-
+VOID             *mReservedApLoopFunc = NULL;
 
 /**
   Get the pointer to CPU MP Data structure.
@@ -258,19 +258,11 @@ MpInitExitBootServicesCallback (
   )
 {
   CPU_MP_DATA               *CpuMpData;
-  VOID                      *ReservedApLoopFunc;
-  //
-  // Avoid APs access invalid buff data which allocated by BootServices,
-  // so we will allocate reserved data for AP loop code.
-  //
+
   CpuMpData = GetCpuMpData ();
   CpuMpData->PmCodeSegment = GetProtectedModeCS ();
   CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
-  ReservedApLoopFunc = AllocateReservedCopyPool (
-                         CpuMpData->AddressMap.RelocateApLoopFuncSize,
-                         CpuMpData->AddressMap.RelocateApLoopFuncAddress
-                         );
-  WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, ReservedApLoopFunc);
+  WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, mReservedApLoopFunc);
   DEBUG ((DEBUG_INFO, "MpInitExitBootServicesCallback() done!\n"));
 }
 
@@ -288,6 +280,18 @@ InitMpGlobalData (
 
   SaveCpuMpData (CpuMpData);
 
+  //
+  // Avoid APs access invalid buff data which allocated by BootServices,
+  // so we will allocate reserved data for AP loop code.
+  // Allocating it in advance since memory services are not available in
+  // Exit Boot Services callback function.
+  //
+  mReservedApLoopFunc = AllocateReservedCopyPool (
+                          CpuMpData->AddressMap.RelocateApLoopFuncSize,
+                          CpuMpData->AddressMap.RelocateApLoopFuncAddress
+                          );
+  ASSERT (mReservedApLoopFunc != NULL);
+
   Status = gBS->CreateEvent (
                   EVT_TIMER | EVT_NOTIFY_SIGNAL,
                   TPL_NOTIFY,
-- 
2.7.4.windows.1



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

* [Patch 3/6] UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
  2016-08-24 14:45 ` [Patch 1/6] UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp() Jeff Fan
  2016-08-24 14:45 ` [Patch 2/6] UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-24 14:45 ` [Patch 4/6] UefiCpuPkg/MpInitLib: Fix function header comments typo Jeff Fan
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

It will be used by DxePeiLib also.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h    | 2 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 47a971b..f0e9e46 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1292,7 +1292,7 @@ MpInitLibInitialize (
   CpuMpData->CpuApStackSize   = ApStackSize;
   CpuMpData->BackupBuffer     = BackupBufferAddr;
   CpuMpData->BackupBufferSize = ApResetVectorSize;
-  CpuMpData->EndOfPeiFlag     = FALSE;
+  CpuMpData->SaveRestoreFlag  = FALSE;
   CpuMpData->WakeupBuffer     = (UINTN) -1;
   CpuMpData->CpuCount         = 1;
   CpuMpData->BspNumber        = 0;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index e53deb4..9b0366e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -198,7 +198,7 @@ struct _CPU_MP_DATA {
   UINTN                          WakeupBuffer;
   UINTN                          BackupBuffer;
   UINTN                          BackupBufferSize;
-  BOOLEAN                        EndOfPeiFlag;
+  BOOLEAN                        SaveRestoreFlag;
 
   volatile UINT32                StartCount;
   volatile UINT32                FinishedCount;
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 5e714ea..dd0f1da 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -157,7 +157,7 @@ CpuMpEndOfPeiCallback (
       Hob.Raw = GET_NEXT_HOB (Hob);
     }
   } else {
-    CpuMpData->EndOfPeiFlag = TRUE;
+    CpuMpData->SaveRestoreFlag = TRUE;
     RestoreWakeupBuffer (CpuMpData);
   }
   return EFI_SUCCESS;
@@ -316,7 +316,7 @@ AllocateResetVector (
     BackupAndPrepareWakeupBuffer (CpuMpData);
   }
 
-  if (CpuMpData->EndOfPeiFlag) {
+  if (CpuMpData->SaveRestoreFlag) {
     BackupAndPrepareWakeupBuffer (CpuMpData);
   }
 }
@@ -331,7 +331,7 @@ FreeResetVector (
   IN CPU_MP_DATA              *CpuMpData
   )
 {
-  if (CpuMpData->EndOfPeiFlag) {
+  if (CpuMpData->SaveRestoreFlag) {
     RestoreWakeupBuffer (CpuMpData);
   }
 }
-- 
2.7.4.windows.1



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

* [Patch 4/6] UefiCpuPkg/MpInitLib: Fix function header comments typo
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
                   ` (2 preceding siblings ...)
  2016-08-24 14:45 ` [Patch 3/6] UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-24 14:45 ` [Patch 5/6] UefiCpuPkg/MpInitLib: Move two functions location Jeff Fan
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c  | 2 +-
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h     | 2 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 383eec9..e459ebc 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -189,7 +189,7 @@ CheckApsStatus (
 /**
   Get Protected mode code segment from current GDT table.
 
-  @returen  Protected mode code segment value.
+  @return  Protected mode code segment value.
 **/
 UINT16
 GetProtectedModeCS (
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 5bb0145..982995b 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -35,7 +35,7 @@ GetCurrentMicrocodeSignature (
 /**
   Detect whether specified processor can find matching microcode patch and load it.
 
-  @param[in] PeiCpuMpData        Pointer to PEI CPU MP Data
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
 **/
 VOID
 MicrocodeDetect (
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 9b0366e..165853d 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -512,7 +512,7 @@ CheckAndUpdateApsStatus (
 /**
   Detect whether specified processor can find matching microcode patch and load it.
 
-  @param[in] PeiCpuMpData        Pointer to PEI CPU MP Data
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
 **/
 VOID
 MicrocodeDetect (
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index dd0f1da..590f963 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -67,7 +67,7 @@ SaveCpuMpData (
 /**
   Get available system memory below 1MB by specified size.
 
-  @param[in] PeiCpuMpData        Pointer to PEI CPU MP Data
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
 **/
 VOID
 BackupAndPrepareWakeupBuffer(
@@ -89,7 +89,7 @@ BackupAndPrepareWakeupBuffer(
 /**
   Restore wakeup buffer data.
 
-  @param[in] PeiCpuMpData        Pointer to PEI CPU MP Data
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
 **/
 VOID
 RestoreWakeupBuffer(
-- 
2.7.4.windows.1



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

* [Patch 5/6] UefiCpuPkg/MpInitLib: Move two functions location
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
                   ` (3 preceding siblings ...)
  2016-08-24 14:45 ` [Patch 4/6] UefiCpuPkg/MpInitLib: Fix function header comments typo Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-24 14:45 ` [Patch 6/6] UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service Jeff Fan
  2016-08-25  5:21 ` [Patch 0/6] Fixed bugs in MpInitLib Tian, Feng
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

Just move BackupAndPrepareWakeupBuffer() and RestoreWakeupBuffer() from
PeiMpLib.c to MpLib.c.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 39 +++++++++++++++++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.h    | 20 +++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 39 ---------------------------------
 3 files changed, 59 insertions(+), 39 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index f0e9e46..c3fe721 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -2012,3 +2012,42 @@ GetCpuMpDataFromGuidedHob (
   }
   return CpuMpData;
 }
+
+/**
+  Get available system memory below 1MB by specified size.
+
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
+**/
+VOID
+BackupAndPrepareWakeupBuffer(
+  IN CPU_MP_DATA              *CpuMpData
+  )
+{
+  CopyMem (
+    (VOID *) CpuMpData->BackupBuffer,
+    (VOID *) CpuMpData->WakeupBuffer,
+    CpuMpData->BackupBufferSize
+    );
+  CopyMem (
+    (VOID *) CpuMpData->WakeupBuffer,
+    (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress,
+    CpuMpData->AddressMap.RendezvousFunnelSize
+    );
+}
+
+/**
+  Restore wakeup buffer data.
+
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
+**/
+VOID
+RestoreWakeupBuffer(
+  IN CPU_MP_DATA              *CpuMpData
+  )
+{
+  CopyMem (
+    (VOID *) CpuMpData->WakeupBuffer,
+    (VOID *) CpuMpData->BackupBuffer,
+    CpuMpData->BackupBufferSize
+    );
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 165853d..ae78c59 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -550,5 +550,25 @@ CpuMpEndOfPeiCallback (
   IN VOID                         *Ppi
   );
 
+/**
+  Get available system memory below 1MB by specified size.
+
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
+**/
+VOID
+BackupAndPrepareWakeupBuffer(
+  IN CPU_MP_DATA              *CpuMpData
+  );
+
+/**
+  Restore wakeup buffer data.
+
+  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
+**/
+VOID
+RestoreWakeupBuffer(
+  IN CPU_MP_DATA              *CpuMpData
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 590f963..e242d37 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -65,45 +65,6 @@ SaveCpuMpData (
 }
 
 /**
-  Get available system memory below 1MB by specified size.
-
-  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
-**/
-VOID
-BackupAndPrepareWakeupBuffer(
-  IN CPU_MP_DATA              *CpuMpData
-  )
-{
-  CopyMem (
-    (VOID *) CpuMpData->BackupBuffer,
-    (VOID *) CpuMpData->WakeupBuffer,
-    CpuMpData->BackupBufferSize
-    );
-  CopyMem (
-    (VOID *) CpuMpData->WakeupBuffer,
-    (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress,
-    CpuMpData->AddressMap.RendezvousFunnelSize
-    );
-}
-
-/**
-  Restore wakeup buffer data.
-
-  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
-**/
-VOID
-RestoreWakeupBuffer(
-  IN CPU_MP_DATA              *CpuMpData
-  )
-{
-  CopyMem (
-    (VOID *) CpuMpData->WakeupBuffer,
-    (VOID *) CpuMpData->BackupBuffer,
-    CpuMpData->BackupBufferSize
-    );
-}
-
-/**
   Notify function on End Of PEI PPI.
 
   On S3 boot, this function will restore wakeup buffer data.
-- 
2.7.4.windows.1



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

* [Patch 6/6] UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
                   ` (4 preceding siblings ...)
  2016-08-24 14:45 ` [Patch 5/6] UefiCpuPkg/MpInitLib: Move two functions location Jeff Fan
@ 2016-08-24 14:45 ` Jeff Fan
  2016-08-25  5:21 ` [Patch 0/6] Fixed bugs in MpInitLib Tian, Feng
  6 siblings, 0 replies; 8+ messages in thread
From: Jeff Fan @ 2016-08-24 14:45 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Feng Tian

In Exit Boot Services callback function, we cannot use allocate memory services
because it may change the memory map that has been gotten by OS.

This fix is not to allocate reset vector buffer after SaveRestoreFlag is set to
TRUE in MpInitExitBootServicesCallback(). Instead AllocateResetVector() will use
the previous allocated buffer address and save the contents before copying reset
vector code. At the same time, FreeResetVector() will restore original contents
after if SaveRestoreFlag is TRUE.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 68 +++++++++++++++++++--------------
 1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index e459ebc..50b5b27 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -66,29 +66,33 @@ AllocateResetVector (
   UINTN                 ApResetVectorSize;
   EFI_PHYSICAL_ADDRESS  StartAddress;
 
-  ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
-                      sizeof (MP_CPU_EXCHANGE_INFO);
-
-  StartAddress = BASE_1MB;
-  Status = gBS->AllocatePages (
-                  AllocateMaxAddress,
-                  EfiACPIMemoryNVS,
-                  EFI_SIZE_TO_PAGES (ApResetVectorSize),
-                  &StartAddress
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  CpuMpData->WakeupBuffer      = (UINTN) StartAddress;
-  CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *) (UINTN)
+  if (CpuMpData->SaveRestoreFlag) {
+    BackupAndPrepareWakeupBuffer (CpuMpData);
+  } else {
+    ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
+                        sizeof (MP_CPU_EXCHANGE_INFO);
+
+    StartAddress = BASE_1MB;
+    Status = gBS->AllocatePages (
+                    AllocateMaxAddress,
+                    EfiACPIMemoryNVS,
+                    EFI_SIZE_TO_PAGES (ApResetVectorSize),
+                    &StartAddress
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    CpuMpData->WakeupBuffer      = (UINTN) StartAddress;
+    CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *) (UINTN)
                   (CpuMpData->WakeupBuffer + CpuMpData->AddressMap.RendezvousFunnelSize);
-  //
-  // copy AP reset code in it
-  //
-  CopyMem (
-    (VOID *) CpuMpData->WakeupBuffer,
-    (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress,
-    CpuMpData->AddressMap.RendezvousFunnelSize
-    );
+    //
+    // copy AP reset code in it
+    //
+    CopyMem (
+      (VOID *) CpuMpData->WakeupBuffer,
+      (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress,
+      CpuMpData->AddressMap.RendezvousFunnelSize
+      );
+  }
 }
 
 /**
@@ -103,13 +107,18 @@ FreeResetVector (
 {
   EFI_STATUS            Status;
   UINTN                 ApResetVectorSize;
-  ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
-                      sizeof (MP_CPU_EXCHANGE_INFO);
-  Status = gBS->FreePages(
-             (EFI_PHYSICAL_ADDRESS)CpuMpData->WakeupBuffer,
-             EFI_SIZE_TO_PAGES (ApResetVectorSize)
-             );
-  ASSERT_EFI_ERROR (Status);
+
+  if (CpuMpData->SaveRestoreFlag) {
+    RestoreWakeupBuffer (CpuMpData);
+  } else {
+    ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
+                        sizeof (MP_CPU_EXCHANGE_INFO);
+    Status = gBS->FreePages(
+               (EFI_PHYSICAL_ADDRESS)CpuMpData->WakeupBuffer,
+               EFI_SIZE_TO_PAGES (ApResetVectorSize)
+               );
+    ASSERT_EFI_ERROR (Status);
+  }
 }
 
 /**
@@ -260,6 +269,7 @@ MpInitExitBootServicesCallback (
   CPU_MP_DATA               *CpuMpData;
 
   CpuMpData = GetCpuMpData ();
+  CpuMpData->SaveRestoreFlag = TRUE;
   CpuMpData->PmCodeSegment = GetProtectedModeCS ();
   CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
   WakeUpAP (CpuMpData, TRUE, 0, RelocateApLoop, mReservedApLoopFunc);
-- 
2.7.4.windows.1



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

* Re: [Patch 0/6] Fixed bugs in MpInitLib
  2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
                   ` (5 preceding siblings ...)
  2016-08-24 14:45 ` [Patch 6/6] UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service Jeff Fan
@ 2016-08-25  5:21 ` Tian, Feng
  6 siblings, 0 replies; 8+ messages in thread
From: Tian, Feng @ 2016-08-25  5:21 UTC (permalink / raw)
  To: Fan, Jeff, edk2-devel@lists.01.org; +Cc: Tian, Feng

Looks good to me

Reviewed-by: Feng Tian <feng.tian@Intel.com>

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeff Fan
Sent: Wednesday, August 24, 2016 10:45 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/6] Fixed bugs in MpInitLib

Jeff Fan (6):
  UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp()
  UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code
  UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag
  UefiCpuPkg/MpInitLib: Fix function header comments typo
  UefiCpuPkg/MpInitLib: Move two functions location
  UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service

 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c  | 96 ++++++++++++++++++--------------  UefiCpuPkg/Library/MpInitLib/Microcode.c |  2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.c     | 52 +++++++++++++++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h     | 24 +++++++-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c  | 45 +--------------
 5 files changed, 127 insertions(+), 92 deletions(-)

--
2.7.4.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


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

end of thread, other threads:[~2016-08-25  5:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-24 14:45 [Patch 0/6] Fixed bugs in MpInitLib Jeff Fan
2016-08-24 14:45 ` [Patch 1/6] UefiCpuPkg/MpInitLib: Move timeout delay to WakupAp() Jeff Fan
2016-08-24 14:45 ` [Patch 2/6] UefiCpuPkg/MpInitLib: Move allocating reserved memory for AP loop code Jeff Fan
2016-08-24 14:45 ` [Patch 3/6] UefiCpuPkg/MpInitLib: Rename EndOfPeiFlag to SaveRestoreFlag Jeff Fan
2016-08-24 14:45 ` [Patch 4/6] UefiCpuPkg/MpInitLib: Fix function header comments typo Jeff Fan
2016-08-24 14:45 ` [Patch 5/6] UefiCpuPkg/MpInitLib: Move two functions location Jeff Fan
2016-08-24 14:45 ` [Patch 6/6] UefiCpuPkg/MpInitLib: Don't allocate reset vector in Exit Boot Service Jeff Fan
2016-08-25  5:21 ` [Patch 0/6] Fixed bugs in MpInitLib Tian, Feng

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