From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.151; helo=mga17.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) (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 DB882211B1119 for ; Sun, 6 Jan 2019 17:06:00 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Jan 2019 17:06:00 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,449,1539673200"; d="scan'208";a="123802606" Received: from ydong10-win10.ccr.corp.intel.com ([10.239.9.125]) by orsmga002.jf.intel.com with ESMTP; 06 Jan 2019 17:05:59 -0800 From: Eric Dong To: edk2-devel@lists.01.org Cc: Ruiyu Ni , Laszlo Ersek Date: Mon, 7 Jan 2019 09:05:55 +0800 Message-Id: <20190107010555.23264-3-eric.dong@intel.com> X-Mailer: git-send-email 2.15.0.windows.1 In-Reply-To: <20190107010555.23264-1-eric.dong@intel.com> References: <20190107010555.23264-1-eric.dong@intel.com> Subject: [Patch 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: Mon, 07 Jan 2019 01:06:01 -0000 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 Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 22 +++++--- .../DxeRegisterCpuFeaturesLib.c | 57 +++++++++++--------- .../PeiRegisterCpuFeaturesLib.c | 62 +++++++++------------- .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 16 +++++- 4 files changed, 85 insertions(+), 72 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..2d2095e437 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. @@ -40,44 +39,44 @@ GetCpuFeaturesData ( @return Pointer to EFI_MP_SERVICES_PROTOCOL. **/ -EFI_MP_SERVICES_PROTOCOL * -GetMpProtocol ( +VOID * +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; + EFI_MP_SERVICES_PROTOCOL *MpServices; - ASSERT (mCpuFeaturesMpServices != NULL); - return mCpuFeaturesMpServices; + // + // Get MP Services Protocol + // + Status = gBS->LocateProtocol ( + &gEfiMpServiceProtocolGuid, + NULL, + (VOID **)&MpServices + ); + ASSERT_EFI_ERROR (Status); + + return MpServices; } /** 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 = (EFI_MP_SERVICES_PROTOCOL *)CpuFeaturesData->MpService; 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 = (EFI_MP_SERVICES_PROTOCOL *)CpuFeaturesData->MpService; - MpServices = GetMpProtocol (); Status = MpServices->GetProcessorInfo ( MpServices, ProcessorNumber, @@ -130,8 +132,8 @@ StartupAPsWorker ( CPU_FEATURES_DATA *CpuFeaturesData; CpuFeaturesData = GetCpuFeaturesData (); + MpServices = (EFI_MP_SERVICES_PROTOCOL *)CpuFeaturesData->MpService; - 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 = (EFI_MP_SERVICES_PROTOCOL *)CpuFeaturesData->MpService; - 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 = (EFI_MP_SERVICES_PROTOCOL *)CpuFeaturesData->MpService; // // 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..41f4dd54e8 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.c @@ -72,8 +72,8 @@ GetCpuFeaturesData ( @return PEI PPI service pointer. **/ -EFI_PEI_MP_SERVICES_PPI * -GetMpPpi ( +VOID * +GetMpService ( VOID ) { @@ -96,20 +96,27 @@ GetMpPpi ( /** 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 = (EFI_PEI_MP_SERVICES_PPI *)CpuFeaturesData->MpService; - 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 = (EFI_PEI_MP_SERVICES_PPI *)CpuFeaturesData->MpService; - 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 = (EFI_PEI_MP_SERVICES_PPI *)CpuFeaturesData->MpService; // // 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 = (EFI_PEI_MP_SERVICES_PPI *)CpuFeaturesData->MpService; // // 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 = (EFI_PEI_MP_SERVICES_PPI *)CpuFeaturesData->MpService; // // 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..ac0a2f9cf4 100644 --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h @@ -85,6 +85,8 @@ typedef struct { UINTN BspNumber; PROGRAM_CPU_REGISTER_FLAGS CpuFlags; + + VOID *MpService; } CPU_FEATURES_DATA; #define CPU_FEATURE_ENTRY_FROM_LINK(a) \ @@ -108,11 +110,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 +249,14 @@ GetAcpiCpuData ( VOID ); +/** + Worker function to get MP service pointer. + + @return Mp service pointer. +**/ +VOID * +GetMpService ( + VOID + ); + #endif -- 2.15.0.windows.1