From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EBEE92194D3B8 for ; Mon, 7 Jan 2019 21:25:32 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Jan 2019 21:25:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,453,1539673200"; d="scan'208";a="132545130" Received: from ydong10-win10.ccr.corp.intel.com ([10.239.9.125]) by fmsmga002.fm.intel.com with ESMTP; 07 Jan 2019 21:25:32 -0800 From: Eric Dong To: edk2-devel@lists.01.org Cc: Ruiyu Ni Date: Tue, 8 Jan 2019 13:25:19 +0800 Message-Id: <20190108052519.22976-3-eric.dong@intel.com> X-Mailer: git-send-email 2.15.0.windows.1 In-Reply-To: <20190108052519.22976-1-eric.dong@intel.com> References: <20190108052519.22976-1-eric.dong@intel.com> Subject: [Patch v4 2/2] UefiCpuPkg/RegisterCpuFeaturesLib: Avoid AP calls PeiService. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Jan 2019 05:25:33 -0000 V3: 1. 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 Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 22 ++++--- .../DxeRegisterCpuFeaturesLib.c | 59 ++++++++++-------- .../PeiRegisterCpuFeaturesLib.c | 70 +++++++++------------- .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 25 +++++++- 4 files changed, 99 insertions(+), 77 deletions(-) diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c index 624ddee055..5866e022f0 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c @@ -138,7 +138,7 @@ FillProcessorInfo ( **/ VOID CpuInitDataInitialize ( - IN UINTN NumberOfCpus + VOID ) { EFI_STATUS Status; @@ -157,12 +157,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 +419,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 +1115,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 +1131,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; - - 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 0bb3dee8b6..5ed76afa24 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c @@ -70,15 +70,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 @@ -87,29 +87,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; } @@ -132,8 +139,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, @@ -162,17 +172,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. @@ -244,17 +244,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. @@ -286,17 +279,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 @@ -329,7 +315,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 -- 2.15.0.windows.1