UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService.

V3:
   Define union to specify the ppi or protocol.

V2:
1. Initialize CpuFeaturesData->MpService in CpuInitDataInitialize
   and make this function been called at the begin of the
   initialization.
2. let all other functions use CpuFeaturesData->MpService install
   of locate the protocol itself.

V1:
GetProcessorIndex function calls GetMpPpi to get the MP Ppi.
Ap will calls GetProcessorIndex function which final let AP calls
PeiService.

This patch avoid GetProcessorIndex call PeiService.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1411

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>

4 files changed, 99 insertions, 78 deletions

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 624ddee..269af05 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -134,11 +134,10 @@ FillProcessorInfo (
 /**

   Prepares for private data used for CPU features.

 

-  @param[in]  NumberOfCpus  Number of processor in system

 **/

 VOID

 CpuInitDataInitialize (

-  IN UINTN                             NumberOfCpus

+  VOID

   )

 {

   EFI_STATUS                           Status;

@@ -157,12 +156,22 @@ CpuInitDataInitialize (
   ACPI_CPU_DATA                        *AcpiCpuData;

   CPU_STATUS_INFORMATION               *CpuStatus;

   UINT32                               *ValidCoreCountPerPackage;

+  UINTN                                NumberOfCpus;

+  UINTN                                NumberOfEnabledProcessors;

 

   Core    = 0;

   Package = 0;

   Thread  = 0;

 

   CpuFeaturesData = GetCpuFeaturesData ();

+

+  //

+  // Initialize CpuFeaturesData->MpService as early as possile, so later function can use it.

+  //

+  CpuFeaturesData->MpService = GetMpService ();

+

+  GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);

+

   CpuFeaturesData->InitOrder = AllocateZeroPool (sizeof (CPU_FEATURES_INIT_ORDER) * NumberOfCpus);

   ASSERT (CpuFeaturesData->InitOrder != NULL);

 

@@ -409,7 +418,7 @@ CollectProcessorData (
   CPU_FEATURES_DATA                    *CpuFeaturesData;

 

   CpuFeaturesData = (CPU_FEATURES_DATA *)Buffer;

-  ProcessorNumber = GetProcessorIndex ();

+  ProcessorNumber = GetProcessorIndex (CpuFeaturesData);

   CpuInfo = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo;

   //

   // collect processor information

@@ -1105,15 +1114,11 @@ CpuFeaturesDetect (
   VOID

   )

 {

-  UINTN                  NumberOfCpus;

-  UINTN                  NumberOfEnabledProcessors;

   CPU_FEATURES_DATA      *CpuFeaturesData;

 

   CpuFeaturesData = GetCpuFeaturesData();

 

-  GetNumberOfProcessor (&NumberOfCpus, &NumberOfEnabledProcessors);

-

-  CpuInitDataInitialize (NumberOfCpus);

+  CpuInitDataInitialize ();

 

   //

   // Wakeup all APs for data collection.

@@ -1125,6 +1130,6 @@ CpuFeaturesDetect (
   //

   CollectProcessorData (CpuFeaturesData);

 

-  AnalysisProcessorFeatures (NumberOfCpus);

+  AnalysisProcessorFeatures (CpuFeaturesData->NumberOfCpus);

 }

 

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
index 926698d..3654c10 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
@@ -20,7 +20,6 @@
 #include "RegisterCpuFeatures.h"

 

 CPU_FEATURES_DATA          mCpuFeaturesData = {0};

-EFI_MP_SERVICES_PROTOCOL   *mCpuFeaturesMpServices = NULL;

 

 /**

   Worker function to get CPU_FEATURES_DATA pointer.

@@ -38,46 +37,46 @@ GetCpuFeaturesData (
 /**

   Worker function to get EFI_MP_SERVICES_PROTOCOL pointer.

 

-  @return Pointer to EFI_MP_SERVICES_PROTOCOL.

+  @return MP_SERVICES variable.

 **/

-EFI_MP_SERVICES_PROTOCOL *

-GetMpProtocol (

+MP_SERVICES

+GetMpService (

   VOID

   )

 {

-  EFI_STATUS             Status;

-

-  if (mCpuFeaturesMpServices == NULL) {

-    //

-    // Get MP Services Protocol

-    //

-    Status = gBS->LocateProtocol (

-                  &gEfiMpServiceProtocolGuid,

-                  NULL,

-                  (VOID **)&mCpuFeaturesMpServices

-                  );

-    ASSERT_EFI_ERROR (Status);

-  }

+  EFI_STATUS                Status;

+  MP_SERVICES               MpService;

 

-  ASSERT (mCpuFeaturesMpServices != NULL);

-  return mCpuFeaturesMpServices;

+  //

+  // Get MP Services Protocol

+  //

+  Status = gBS->LocateProtocol (

+                &gEfiMpServiceProtocolGuid,

+                NULL,

+                (VOID **)&MpService.Protocol

+                );

+  ASSERT_EFI_ERROR (Status);

+

+  return MpService;

 }

 

 /**

   Worker function to return processor index.

 

+  @param  CpuFeaturesData    Cpu Feature Data structure.

+

   @return  The processor index.

 **/

 UINTN

 GetProcessorIndex (

-  VOID

+  IN CPU_FEATURES_DATA        *CpuFeaturesData

   )

 {

   EFI_STATUS                           Status;

   UINTN                                ProcessorIndex;

   EFI_MP_SERVICES_PROTOCOL             *MpServices;

 

-  MpServices = GetMpProtocol ();

+  MpServices = CpuFeaturesData->MpService.Protocol;

   Status = MpServices->WhoAmI(MpServices, &ProcessorIndex);

   ASSERT_EFI_ERROR (Status);

   return ProcessorIndex;

@@ -101,8 +100,11 @@ GetProcessorInformation (
 {

   EFI_STATUS                           Status;

   EFI_MP_SERVICES_PROTOCOL             *MpServices;

+  CPU_FEATURES_DATA                    *CpuFeaturesData;

+

+  CpuFeaturesData = GetCpuFeaturesData ();

+  MpServices = CpuFeaturesData->MpService.Protocol;

 

-  MpServices = GetMpProtocol ();

   Status = MpServices->GetProcessorInfo (

                MpServices,

                ProcessorNumber,

@@ -130,8 +132,8 @@ StartupAPsWorker (
   CPU_FEATURES_DATA                    *CpuFeaturesData;

 

   CpuFeaturesData = GetCpuFeaturesData ();

+  MpServices = CpuFeaturesData->MpService.Protocol;

 

-  MpServices = GetMpProtocol ();

   //

   // Wakeup all APs

   //

@@ -159,8 +161,11 @@ SwitchNewBsp (
 {

   EFI_STATUS                           Status;

   EFI_MP_SERVICES_PROTOCOL             *MpServices;

+  CPU_FEATURES_DATA                    *CpuFeaturesData;

+

+  CpuFeaturesData = GetCpuFeaturesData ();

+  MpServices = CpuFeaturesData->MpService.Protocol;

 

-  MpServices = GetMpProtocol ();

   //

   // Wakeup all APs

   //

@@ -190,8 +195,10 @@ GetNumberOfProcessor (
 {

   EFI_STATUS                           Status;

   EFI_MP_SERVICES_PROTOCOL             *MpServices;

+  CPU_FEATURES_DATA                    *CpuFeaturesData;

 

-  MpServices = GetMpProtocol ();

+  CpuFeaturesData = GetCpuFeaturesData ();

+  MpServices = CpuFeaturesData->MpService.Protocol;

 

   //

   // Get the number of CPUs

@@ -225,7 +232,7 @@ CpuFeaturesInitialize (
 

   CpuFeaturesData = GetCpuFeaturesData ();

 

-  OldBspNumber = GetProcessorIndex();

+  OldBspNumber = GetProcessorIndex (CpuFeaturesData);

   CpuFeaturesData->BspNumber = OldBspNumber;

 

   Status = gBS->CreateEvent (

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
index 4bab283..03d852e 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c
@@ -68,15 +68,15 @@ GetCpuFeaturesData (
 /**

   Worker function to get MP PPI service pointer.

 

-  @return PEI PPI service pointer.

+  @return MP_SERVICES variable.

 **/

-EFI_PEI_MP_SERVICES_PPI *

-GetMpPpi (

+MP_SERVICES

+GetMpService (

   VOID

   )

 {

   EFI_STATUS                 Status;

-  EFI_PEI_MP_SERVICES_PPI    *CpuMpPpi;

+  MP_SERVICES                MpService;

 

   //

   // Get MP Services Protocol

@@ -85,29 +85,36 @@ GetMpPpi (
              &gEfiPeiMpServicesPpiGuid,

              0,

              NULL,

-             (VOID **)&CpuMpPpi

+             (VOID **)&MpService.Ppi

              );

   ASSERT_EFI_ERROR (Status);

-  return CpuMpPpi;

+  return MpService;

 }

 

 /**

   Worker function to return processor index.

 

+  @param  CpuFeaturesData    Cpu Feature Data structure.

+

   @return  The processor index.

 **/

 UINTN

 GetProcessorIndex (

-  VOID

+  IN CPU_FEATURES_DATA        *CpuFeaturesData

   )

 {

   EFI_STATUS                 Status;

   EFI_PEI_MP_SERVICES_PPI    *CpuMpPpi;

   UINTN                      ProcessorIndex;

 

-  CpuMpPpi = GetMpPpi ();

+  CpuMpPpi = CpuFeaturesData->MpService.Ppi;

 

-  Status = CpuMpPpi->WhoAmI(GetPeiServicesTablePointer (), CpuMpPpi, &ProcessorIndex);

+  //

+  // For two reasons which use NULL for WhoAmI:

+  // 1. This function will be called by APs and AP should not use PeiServices Table

+  // 2. Check WhoAmI implementation, this parameter will not be used.

+  //

+  Status = CpuMpPpi->WhoAmI(NULL, CpuMpPpi, &ProcessorIndex);

   ASSERT_EFI_ERROR (Status);

   return ProcessorIndex;

 }

@@ -130,8 +137,11 @@ GetProcessorInformation (
 {

   EFI_PEI_MP_SERVICES_PPI    *CpuMpPpi;

   EFI_STATUS                 Status;

+  CPU_FEATURES_DATA          *CpuFeaturesData;

+

+  CpuFeaturesData = GetCpuFeaturesData ();

+  CpuMpPpi = CpuFeaturesData->MpService.Ppi;

 

-  CpuMpPpi = GetMpPpi ();

   Status = CpuMpPpi->GetProcessorInfo (

                GetPeiServicesTablePointer(),

                CpuMpPpi,

@@ -160,17 +170,7 @@ StartupAPsWorker (
   CPU_FEATURES_DATA                    *CpuFeaturesData;

 

   CpuFeaturesData = GetCpuFeaturesData ();

-

-  //

-  // Get MP Services Protocol

-  //

-  Status = PeiServicesLocatePpi (

-             &gEfiPeiMpServicesPpiGuid,

-             0,

-             NULL,

-             (VOID **)&CpuMpPpi

-             );

-  ASSERT_EFI_ERROR (Status);

+  CpuMpPpi = CpuFeaturesData->MpService.Ppi;

 

   //

   // Wakeup all APs for data collection.

@@ -198,17 +198,10 @@ SwitchNewBsp (
 {

   EFI_STATUS                           Status;

   EFI_PEI_MP_SERVICES_PPI              *CpuMpPpi;

+  CPU_FEATURES_DATA                    *CpuFeaturesData;

 

-  //

-  // Get MP Services Protocol

-  //

-  Status = PeiServicesLocatePpi (

-             &gEfiPeiMpServicesPpiGuid,

-             0,

-             NULL,

-             (VOID **)&CpuMpPpi

-             );

-  ASSERT_EFI_ERROR (Status);

+  CpuFeaturesData = GetCpuFeaturesData ();

+  CpuMpPpi = CpuFeaturesData->MpService.Ppi;

 

   //

   // Wakeup all APs for data collection.

@@ -240,17 +233,10 @@ GetNumberOfProcessor (
 {

   EFI_STATUS                 Status;

   EFI_PEI_MP_SERVICES_PPI    *CpuMpPpi;

+  CPU_FEATURES_DATA          *CpuFeaturesData;

 

-  //

-  // Get MP Services Protocol

-  //

-  Status = PeiServicesLocatePpi (

-             &gEfiPeiMpServicesPpiGuid,

-             0,

-             NULL,

-             (VOID **)&CpuMpPpi

-             );

-  ASSERT_EFI_ERROR (Status);

+  CpuFeaturesData = GetCpuFeaturesData ();

+  CpuMpPpi = CpuFeaturesData->MpService.Ppi;

 

   //

   // Get the number of CPUs

@@ -283,7 +269,7 @@ CpuFeaturesInitialize (
 

   CpuFeaturesData = GetCpuFeaturesData ();

 

-  OldBspNumber = GetProcessorIndex();

+  OldBspNumber = GetProcessorIndex (CpuFeaturesData);

   CpuFeaturesData->BspNumber = OldBspNumber;

 

   //

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index cf3da84..21dd577 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -14,6 +14,10 @@
 

 #ifndef _REGISTER_CPU_FEATURES_H_

 #define _REGISTER_CPU_FEATURES_H_

+#include <PiPei.h>

+#include <PiDxe.h>

+#include <Ppi/MpServices.h>

+#include <Protocol/MpService.h>

 

 #include <Library/BaseLib.h>

 #include <Library/DebugLib.h>

@@ -66,6 +70,11 @@ typedef struct {
   volatile UINT32          *PackageSemaphoreCount;  // Semaphore containers used to program Package semaphore.

 } PROGRAM_CPU_REGISTER_FLAGS;

 

+typedef union {

+  EFI_MP_SERVICES_PROTOCOL  *Protocol;

+  EFI_PEI_MP_SERVICES_PPI   *Ppi;

+} MP_SERVICES;

+

 typedef struct {

   UINTN                    FeaturesCount;

   UINT32                   BitMaskSize;

@@ -85,6 +94,8 @@ typedef struct {
   UINTN                    BspNumber;

 

   PROGRAM_CPU_REGISTER_FLAGS  CpuFlags;

+

+  MP_SERVICES              MpService;

 } CPU_FEATURES_DATA;

 

 #define CPU_FEATURE_ENTRY_FROM_LINK(a) \

@@ -108,11 +119,13 @@ GetCpuFeaturesData (
 /**

   Worker function to return processor index.

 

+  @param  CpuFeaturesData    Cpu Feature Data structure.

+

   @return  The processor index.

 **/

 UINTN

 GetProcessorIndex (

-  VOID

+  IN CPU_FEATURES_DATA        *CpuFeaturesData

   );

 

 /**

@@ -245,4 +258,14 @@ GetAcpiCpuData (
   VOID

   );

 

+/**

+  Worker function to get MP service pointer.

+

+  @return MP_SERVICES variable.

+**/

+MP_SERVICES

+GetMpService (

+  VOID

+  );

+

 #endif