public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
@ 2018-07-02  6:01 Ruiyu Ni
  2018-07-02 17:11 ` Laszlo Ersek
  2018-07-04  2:33 ` Dong, Eric
  0 siblings, 2 replies; 3+ messages in thread
From: Ruiyu Ni @ 2018-07-02  6:01 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jeff Fan, Eric Dong, Jiewen Yao, Fish Andrew, Laszlo Ersek

Today's MpInitLib PEI implementation directly calls
PeiServices->GetHobList() from AP which may cause racing issue.

This patch fixes this issue by duplicating IDT for APs.
Because CpuMpData structure is stored just after IDT, the CpuMPData
address equals to IDTR.BASE + IDTR.LIMIT + 1.

v2:
  1. Add ALIGN_VALUE() on BufferSize.
  2. Add ASSERT() to make sure no memory usage outside of the allocated buffer.
  3. Add more comments in InitConfig path when restoring CpuData[0].VolatileRegisters.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jeff Fan <vanjeff_919@hotmail.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Fish Andrew <afish@apple.com>
Cc: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c    | 59 +++++++++++++++++++++++++++------
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 18 +++++++---
 2 files changed, 63 insertions(+), 14 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index eb2765910c..108eea0a6f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -615,7 +615,9 @@ ApWakeupFunction (
       //
       ApInitializeSync (CpuMpData);
       //
-      // Sync BSP's Control registers to APs
+      // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment,
+      //   to initialize AP in InitConfig path.
+      // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters points to a different IDT shared by all APs.
       //
       RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
       InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
@@ -1506,6 +1508,7 @@ MpInitLibInitialize (
   UINT32                   MaxLogicalProcessorNumber;
   UINT32                   ApStackSize;
   MP_ASSEMBLY_ADDRESS_MAP  AddressMap;
+  CPU_VOLATILE_REGISTERS   VolatileRegisters;
   UINTN                    BufferSize;
   UINT32                   MonitorFilterSize;
   VOID                     *MpBuffer;
@@ -1516,6 +1519,7 @@ MpInitLibInitialize (
   UINTN                    Index;
   UINTN                    ApResetVectorSize;
   UINTN                    BackupBufferAddr;
+  UINTN                    ApIdtBase;
 
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
@@ -1530,19 +1534,48 @@ MpInitLibInitialize (
   ApStackSize = PcdGet32(PcdCpuApStackSize);
   ApLoopMode  = GetApLoopMode (&MonitorFilterSize);
 
+  //
+  // Save BSP's Control registers for APs
+  //
+  SaveVolatileRegisters (&VolatileRegisters);
+
   BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
   BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
-  BufferSize += sizeof (CPU_MP_DATA);
   BufferSize += ApResetVectorSize;
+  BufferSize  = ALIGN_VALUE (BufferSize, 8);
+  BufferSize += VolatileRegisters.Idtr.Limit + 1;
+  BufferSize += sizeof (CPU_MP_DATA);
   BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber;
   MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
   ASSERT (MpBuffer != NULL);
   ZeroMem (MpBuffer, BufferSize);
   Buffer = (UINTN) MpBuffer;
 
+  //
+  //  The layout of the Buffer is as below:
+  //
+  //    +--------------------+ <-- Buffer
+  //        AP Stacks (N)
+  //    +--------------------+ <-- MonitorBuffer
+  //    AP Monitor Filters (N)
+  //    +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer)
+  //         Backup Buffer
+  //    +--------------------+
+  //           Padding
+  //    +--------------------+ <-- ApIdtBase (8-byte boundary)
+  //           AP IDT          All APs share one separate IDT. So AP can get address of CPU_MP_DATA from IDT Base.
+  //    +--------------------+ <-- CpuMpData
+  //         CPU_MP_DATA
+  //    +--------------------+ <-- CpuMpData->CpuData
+  //        CPU_AP_DATA (N)
+  //    +--------------------+ <-- CpuMpData->CpuInfoInHob
+  //      CPU_INFO_IN_HOB (N)
+  //    +--------------------+
+  //
   MonitorBuffer    = (UINT8 *) (Buffer + ApStackSize * MaxLogicalProcessorNumber);
   BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize * MaxLogicalProcessorNumber;
-  CpuMpData = (CPU_MP_DATA *) (BackupBufferAddr + ApResetVectorSize);
+  ApIdtBase        = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
+  CpuMpData        = (CPU_MP_DATA *) (ApIdtBase + VolatileRegisters.Idtr.Limit + 1);
   CpuMpData->Buffer           = Buffer;
   CpuMpData->CpuApStackSize   = ApStackSize;
   CpuMpData->BackupBuffer     = BackupBufferAddr;
@@ -1557,10 +1590,20 @@ MpInitLibInitialize (
   CpuMpData->MicrocodePatchAddress    = PcdGet64 (PcdCpuMicrocodePatchAddress);
   CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize);
   InitializeSpinLock(&CpuMpData->MpLock);
+
+  //
+  // Make sure no memory usage outside of the allocated buffer.
   //
-  // Save BSP's Control registers to APs
+  ASSERT ((CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) ==
+          Buffer + BufferSize);
+
+  //
+  // Duplicate BSP's IDT to APs.
+  // All APs share one separate IDT. So AP can get the address of CpuMpData by using IDTR.BASE + IDTR.LIMIT + 1
   //
-  SaveVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters);
+  CopyMem ((VOID *)ApIdtBase, (VOID *)VolatileRegisters.Idtr.Base, VolatileRegisters.Idtr.Limit + 1);
+  VolatileRegisters.Idtr.Base = ApIdtBase;
+  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, sizeof (VolatileRegisters));
   //
   // Set BSP basic information
   //
@@ -1618,11 +1661,7 @@ MpInitLibInitialize (
       }
       CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0)? TRUE:FALSE;
       CpuMpData->CpuData[Index].ApFunction = 0;
-      CopyMem (
-        &CpuMpData->CpuData[Index].VolatileRegisters,
-        &CpuMpData->CpuData[0].VolatileRegisters,
-        sizeof (CPU_VOLATILE_REGISTERS)
-        );
+      CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
     }
     if (MaxLogicalProcessorNumber > 1) {
       //
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 791ae9db6e..92f28681e4 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -27,6 +27,8 @@ EnableDebugAgent (
 
 /**
   Get pointer to CPU MP Data structure.
+  For BSP, the pointer is retrieved from HOB.
+  For AP, the structure is just after IDT.
 
   @return  The pointer to CPU MP Data structure.
 **/
@@ -35,10 +37,18 @@ GetCpuMpData (
   VOID
   )
 {
-  CPU_MP_DATA      *CpuMpData;
-
-  CpuMpData = GetCpuMpDataFromGuidedHob ();
-  ASSERT (CpuMpData != NULL);
+  CPU_MP_DATA                  *CpuMpData;
+  MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
+  IA32_DESCRIPTOR              Idtr;
+
+  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
+  if (ApicBaseMsr.Bits.BSP == 1) {
+    CpuMpData = GetCpuMpDataFromGuidedHob ();
+    ASSERT (CpuMpData != NULL);
+  } else {
+    AsmReadIdtr (&Idtr);
+    CpuMpData = (CPU_MP_DATA *) (Idtr.Base + Idtr.Limit + 1);
+  }
   return CpuMpData;
 }
 
-- 
2.16.1.windows.1



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

* Re: [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
  2018-07-02  6:01 [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP Ruiyu Ni
@ 2018-07-02 17:11 ` Laszlo Ersek
  2018-07-04  2:33 ` Dong, Eric
  1 sibling, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2018-07-02 17:11 UTC (permalink / raw)
  To: Ruiyu Ni, edk2-devel; +Cc: Jiewen Yao, Eric Dong, Fish Andrew

On 07/02/18 08:01, Ruiyu Ni wrote:
> Today's MpInitLib PEI implementation directly calls
> PeiServices->GetHobList() from AP which may cause racing issue.
> 
> This patch fixes this issue by duplicating IDT for APs.
> Because CpuMpData structure is stored just after IDT, the CpuMPData
> address equals to IDTR.BASE + IDTR.LIMIT + 1.
> 
> v2:
>   1. Add ALIGN_VALUE() on BufferSize.
>   2. Add ASSERT() to make sure no memory usage outside of the allocated buffer.
>   3. Add more comments in InitConfig path when restoring CpuData[0].VolatileRegisters.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jeff Fan <vanjeff_919@hotmail.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Fish Andrew <afish@apple.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c    | 59 +++++++++++++++++++++++++++------
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 18 +++++++---
>  2 files changed, 63 insertions(+), 14 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index eb2765910c..108eea0a6f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -615,7 +615,9 @@ ApWakeupFunction (
>        //
>        ApInitializeSync (CpuMpData);
>        //
> -      // Sync BSP's Control registers to APs
> +      // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment,
> +      //   to initialize AP in InitConfig path.
> +      // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters points to a different IDT shared by all APs.
>        //
>        RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE);
>        InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
> @@ -1506,6 +1508,7 @@ MpInitLibInitialize (
>    UINT32                   MaxLogicalProcessorNumber;
>    UINT32                   ApStackSize;
>    MP_ASSEMBLY_ADDRESS_MAP  AddressMap;
> +  CPU_VOLATILE_REGISTERS   VolatileRegisters;
>    UINTN                    BufferSize;
>    UINT32                   MonitorFilterSize;
>    VOID                     *MpBuffer;
> @@ -1516,6 +1519,7 @@ MpInitLibInitialize (
>    UINTN                    Index;
>    UINTN                    ApResetVectorSize;
>    UINTN                    BackupBufferAddr;
> +  UINTN                    ApIdtBase;
>  
>    OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>    if (OldCpuMpData == NULL) {
> @@ -1530,19 +1534,48 @@ MpInitLibInitialize (
>    ApStackSize = PcdGet32(PcdCpuApStackSize);
>    ApLoopMode  = GetApLoopMode (&MonitorFilterSize);
>  
> +  //
> +  // Save BSP's Control registers for APs
> +  //
> +  SaveVolatileRegisters (&VolatileRegisters);
> +
>    BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
>    BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
> -  BufferSize += sizeof (CPU_MP_DATA);
>    BufferSize += ApResetVectorSize;
> +  BufferSize  = ALIGN_VALUE (BufferSize, 8);
> +  BufferSize += VolatileRegisters.Idtr.Limit + 1;
> +  BufferSize += sizeof (CPU_MP_DATA);
>    BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber;
>    MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
>    ASSERT (MpBuffer != NULL);
>    ZeroMem (MpBuffer, BufferSize);
>    Buffer = (UINTN) MpBuffer;
>  
> +  //
> +  //  The layout of the Buffer is as below:
> +  //
> +  //    +--------------------+ <-- Buffer
> +  //        AP Stacks (N)
> +  //    +--------------------+ <-- MonitorBuffer
> +  //    AP Monitor Filters (N)
> +  //    +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer)
> +  //         Backup Buffer
> +  //    +--------------------+
> +  //           Padding
> +  //    +--------------------+ <-- ApIdtBase (8-byte boundary)
> +  //           AP IDT          All APs share one separate IDT. So AP can get address of CPU_MP_DATA from IDT Base.
> +  //    +--------------------+ <-- CpuMpData
> +  //         CPU_MP_DATA
> +  //    +--------------------+ <-- CpuMpData->CpuData
> +  //        CPU_AP_DATA (N)
> +  //    +--------------------+ <-- CpuMpData->CpuInfoInHob
> +  //      CPU_INFO_IN_HOB (N)
> +  //    +--------------------+
> +  //
>    MonitorBuffer    = (UINT8 *) (Buffer + ApStackSize * MaxLogicalProcessorNumber);
>    BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize * MaxLogicalProcessorNumber;
> -  CpuMpData = (CPU_MP_DATA *) (BackupBufferAddr + ApResetVectorSize);
> +  ApIdtBase        = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
> +  CpuMpData        = (CPU_MP_DATA *) (ApIdtBase + VolatileRegisters.Idtr.Limit + 1);
>    CpuMpData->Buffer           = Buffer;
>    CpuMpData->CpuApStackSize   = ApStackSize;
>    CpuMpData->BackupBuffer     = BackupBufferAddr;
> @@ -1557,10 +1590,20 @@ MpInitLibInitialize (
>    CpuMpData->MicrocodePatchAddress    = PcdGet64 (PcdCpuMicrocodePatchAddress);
>    CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize);
>    InitializeSpinLock(&CpuMpData->MpLock);
> +
> +  //
> +  // Make sure no memory usage outside of the allocated buffer.
>    //
> -  // Save BSP's Control registers to APs
> +  ASSERT ((CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) ==
> +          Buffer + BufferSize);
> +
> +  //
> +  // Duplicate BSP's IDT to APs.
> +  // All APs share one separate IDT. So AP can get the address of CpuMpData by using IDTR.BASE + IDTR.LIMIT + 1
>    //
> -  SaveVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters);
> +  CopyMem ((VOID *)ApIdtBase, (VOID *)VolatileRegisters.Idtr.Base, VolatileRegisters.Idtr.Limit + 1);
> +  VolatileRegisters.Idtr.Base = ApIdtBase;
> +  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, sizeof (VolatileRegisters));
>    //
>    // Set BSP basic information
>    //
> @@ -1618,11 +1661,7 @@ MpInitLibInitialize (
>        }
>        CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0)? TRUE:FALSE;
>        CpuMpData->CpuData[Index].ApFunction = 0;
> -      CopyMem (
> -        &CpuMpData->CpuData[Index].VolatileRegisters,
> -        &CpuMpData->CpuData[0].VolatileRegisters,
> -        sizeof (CPU_VOLATILE_REGISTERS)
> -        );
> +      CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
>      }
>      if (MaxLogicalProcessorNumber > 1) {
>        //
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 791ae9db6e..92f28681e4 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -27,6 +27,8 @@ EnableDebugAgent (
>  
>  /**
>    Get pointer to CPU MP Data structure.
> +  For BSP, the pointer is retrieved from HOB.
> +  For AP, the structure is just after IDT.
>  
>    @return  The pointer to CPU MP Data structure.
>  **/
> @@ -35,10 +37,18 @@ GetCpuMpData (
>    VOID
>    )
>  {
> -  CPU_MP_DATA      *CpuMpData;
> -
> -  CpuMpData = GetCpuMpDataFromGuidedHob ();
> -  ASSERT (CpuMpData != NULL);
> +  CPU_MP_DATA                  *CpuMpData;
> +  MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
> +  IA32_DESCRIPTOR              Idtr;
> +
> +  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
> +  if (ApicBaseMsr.Bits.BSP == 1) {
> +    CpuMpData = GetCpuMpDataFromGuidedHob ();
> +    ASSERT (CpuMpData != NULL);
> +  } else {
> +    AsmReadIdtr (&Idtr);
> +    CpuMpData = (CPU_MP_DATA *) (Idtr.Base + Idtr.Limit + 1);
> +  }
>    return CpuMpData;
>  }
>  
> 

Acked-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>

(I performed my usual regression tests, with/without SMM, and S3.)

Thanks!
Laszlo


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

* Re: [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
  2018-07-02  6:01 [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP Ruiyu Ni
  2018-07-02 17:11 ` Laszlo Ersek
@ 2018-07-04  2:33 ` Dong, Eric
  1 sibling, 0 replies; 3+ messages in thread
From: Dong, Eric @ 2018-07-04  2:33 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org
  Cc: Jeff Fan, Yao, Jiewen, Fish Andrew, Laszlo Ersek

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Monday, July 2, 2018 2:02 PM
> To: edk2-devel@lists.01.org
> Cc: Jeff Fan <vanjeff_919@hotmail.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Fish Andrew <afish@apple.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
> 
> Today's MpInitLib PEI implementation directly calls
> PeiServices->GetHobList() from AP which may cause racing issue.
> 
> This patch fixes this issue by duplicating IDT for APs.
> Because CpuMpData structure is stored just after IDT, the CpuMPData
> address equals to IDTR.BASE + IDTR.LIMIT + 1.
> 
> v2:
>   1. Add ALIGN_VALUE() on BufferSize.
>   2. Add ASSERT() to make sure no memory usage outside of the allocated
> buffer.
>   3. Add more comments in InitConfig path when restoring
> CpuData[0].VolatileRegisters.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jeff Fan <vanjeff_919@hotmail.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Fish Andrew <afish@apple.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c    | 59
> +++++++++++++++++++++++++++------
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 18 +++++++---
>  2 files changed, 63 insertions(+), 14 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index eb2765910c..108eea0a6f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -615,7 +615,9 @@ ApWakeupFunction (
>        //
>        ApInitializeSync (CpuMpData);
>        //
> -      // Sync BSP's Control registers to APs
> +      // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP
> environment,
> +      //   to initialize AP in InitConfig path.
> +      // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters
> points to a different IDT shared by all APs.
>        //
>        RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters,
> FALSE);
>        InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack);
> @@ -1506,6 +1508,7 @@ MpInitLibInitialize (
>    UINT32                   MaxLogicalProcessorNumber;
>    UINT32                   ApStackSize;
>    MP_ASSEMBLY_ADDRESS_MAP  AddressMap;
> +  CPU_VOLATILE_REGISTERS   VolatileRegisters;
>    UINTN                    BufferSize;
>    UINT32                   MonitorFilterSize;
>    VOID                     *MpBuffer;
> @@ -1516,6 +1519,7 @@ MpInitLibInitialize (
>    UINTN                    Index;
>    UINTN                    ApResetVectorSize;
>    UINTN                    BackupBufferAddr;
> +  UINTN                    ApIdtBase;
> 
>    OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>    if (OldCpuMpData == NULL) {
> @@ -1530,19 +1534,48 @@ MpInitLibInitialize (
>    ApStackSize = PcdGet32(PcdCpuApStackSize);
>    ApLoopMode  = GetApLoopMode (&MonitorFilterSize);
> 
> +  //
> +  // Save BSP's Control registers for APs
> +  //
> +  SaveVolatileRegisters (&VolatileRegisters);
> +
>    BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
>    BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
> -  BufferSize += sizeof (CPU_MP_DATA);
>    BufferSize += ApResetVectorSize;
> +  BufferSize  = ALIGN_VALUE (BufferSize, 8);
> +  BufferSize += VolatileRegisters.Idtr.Limit + 1;
> +  BufferSize += sizeof (CPU_MP_DATA);
>    BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))*
> MaxLogicalProcessorNumber;
>    MpBuffer    = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
>    ASSERT (MpBuffer != NULL);
>    ZeroMem (MpBuffer, BufferSize);
>    Buffer = (UINTN) MpBuffer;
> 
> +  //
> +  //  The layout of the Buffer is as below:
> +  //
> +  //    +--------------------+ <-- Buffer
> +  //        AP Stacks (N)
> +  //    +--------------------+ <-- MonitorBuffer
> +  //    AP Monitor Filters (N)
> +  //    +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer)
> +  //         Backup Buffer
> +  //    +--------------------+
> +  //           Padding
> +  //    +--------------------+ <-- ApIdtBase (8-byte boundary)
> +  //           AP IDT          All APs share one separate IDT. So AP can get address
> of CPU_MP_DATA from IDT Base.
> +  //    +--------------------+ <-- CpuMpData
> +  //         CPU_MP_DATA
> +  //    +--------------------+ <-- CpuMpData->CpuData
> +  //        CPU_AP_DATA (N)
> +  //    +--------------------+ <-- CpuMpData->CpuInfoInHob
> +  //      CPU_INFO_IN_HOB (N)
> +  //    +--------------------+
> +  //
>    MonitorBuffer    = (UINT8 *) (Buffer + ApStackSize *
> MaxLogicalProcessorNumber);
>    BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize *
> MaxLogicalProcessorNumber;
> -  CpuMpData = (CPU_MP_DATA *) (BackupBufferAddr + ApResetVectorSize);
> +  ApIdtBase        = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
> +  CpuMpData        = (CPU_MP_DATA *) (ApIdtBase +
> VolatileRegisters.Idtr.Limit + 1);
>    CpuMpData->Buffer           = Buffer;
>    CpuMpData->CpuApStackSize   = ApStackSize;
>    CpuMpData->BackupBuffer     = BackupBufferAddr;
> @@ -1557,10 +1590,20 @@ MpInitLibInitialize (
>    CpuMpData->MicrocodePatchAddress    = PcdGet64
> (PcdCpuMicrocodePatchAddress);
>    CpuMpData->MicrocodePatchRegionSize = PcdGet64
> (PcdCpuMicrocodePatchRegionSize);
>    InitializeSpinLock(&CpuMpData->MpLock);
> +
> +  //
> +  // Make sure no memory usage outside of the allocated buffer.
>    //
> -  // Save BSP's Control registers to APs
> +  ASSERT ((CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) *
> MaxLogicalProcessorNumber) ==
> +          Buffer + BufferSize);
> +
> +  //
> +  // Duplicate BSP's IDT to APs.
> +  // All APs share one separate IDT. So AP can get the address of CpuMpData
> by using IDTR.BASE + IDTR.LIMIT + 1
>    //
> -  SaveVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters);
> +  CopyMem ((VOID *)ApIdtBase, (VOID *)VolatileRegisters.Idtr.Base,
> VolatileRegisters.Idtr.Limit + 1);
> +  VolatileRegisters.Idtr.Base = ApIdtBase;
> +  CopyMem (&CpuMpData->CpuData[0].VolatileRegisters,
> &VolatileRegisters, sizeof (VolatileRegisters));
>    //
>    // Set BSP basic information
>    //
> @@ -1618,11 +1661,7 @@ MpInitLibInitialize (
>        }
>        CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health
> == 0)? TRUE:FALSE;
>        CpuMpData->CpuData[Index].ApFunction = 0;
> -      CopyMem (
> -        &CpuMpData->CpuData[Index].VolatileRegisters,
> -        &CpuMpData->CpuData[0].VolatileRegisters,
> -        sizeof (CPU_VOLATILE_REGISTERS)
> -        );
> +      CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters,
> &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
>      }
>      if (MaxLogicalProcessorNumber > 1) {
>        //
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 791ae9db6e..92f28681e4 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -27,6 +27,8 @@ EnableDebugAgent (
> 
>  /**
>    Get pointer to CPU MP Data structure.
> +  For BSP, the pointer is retrieved from HOB.
> +  For AP, the structure is just after IDT.
> 
>    @return  The pointer to CPU MP Data structure.
>  **/
> @@ -35,10 +37,18 @@ GetCpuMpData (
>    VOID
>    )
>  {
> -  CPU_MP_DATA      *CpuMpData;
> -
> -  CpuMpData = GetCpuMpDataFromGuidedHob ();
> -  ASSERT (CpuMpData != NULL);
> +  CPU_MP_DATA                  *CpuMpData;
> +  MSR_IA32_APIC_BASE_REGISTER  ApicBaseMsr;
> +  IA32_DESCRIPTOR              Idtr;
> +
> +  ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE);
> +  if (ApicBaseMsr.Bits.BSP == 1) {
> +    CpuMpData = GetCpuMpDataFromGuidedHob ();
> +    ASSERT (CpuMpData != NULL);
> +  } else {
> +    AsmReadIdtr (&Idtr);
> +    CpuMpData = (CPU_MP_DATA *) (Idtr.Base + Idtr.Limit + 1);
> +  }
>    return CpuMpData;
>  }
> 
> --
> 2.16.1.windows.1



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

end of thread, other threads:[~2018-07-04  2:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-02  6:01 [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP Ruiyu Ni
2018-07-02 17:11 ` Laszlo Ersek
2018-07-04  2:33 ` Dong, Eric

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