From 7217b8796d2727db76cd6684ba706c3b643b1d62 Mon Sep 17 00:00:00 2001 From: Eric Dong Date: Fri, 4 Jan 2019 13:37:29 +0800 Subject: [PATCH] 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 Reviewed-by: Ray Ni --- .../CpuFeaturesInitialize.c | 23 +++--- .../DxeRegisterCpuFeaturesLib.c | 57 ++++++++------- .../PeiRegisterCpuFeaturesLib.c | 70 ++++++++----------- .../RegisterCpuFeatures.h | 25 ++++++- 4 files changed, 98 insertions(+), 77 deletions(-) diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c index 624ddee055..269af052b1 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 926698dc95..3654c105ef 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; + EFI_STATUS Status; + MP_SERVICES MpService; - if (mCpuFeaturesMpServices == NULL) { - // - // Get MP Services Protocol - // - Status = gBS->LocateProtocol ( - &gEfiMpServiceProtocolGuid, - NULL, - (VOID **)&mCpuFeaturesMpServices - ); - ASSERT_EFI_ERROR (Status); - } + // + // Get MP Services Protocol + // + Status = gBS->LocateProtocol ( + &gEfiMpServiceProtocolGuid, + NULL, + (VOID **)&MpService.Protocol + ); + ASSERT_EFI_ERROR (Status); - ASSERT (mCpuFeaturesMpServices != NULL); - return mCpuFeaturesMpServices; + 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 4bab2837cc..03d852e618 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 cf3da84837..21dd5773a6 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 +#include +#include +#include #include #include @@ -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