From 1e3f7a3782f1928a19bba81d9d0dba28d15fdae5 Mon Sep 17 00:00:00 2001 From: Eric Dong Date: Thu, 19 Oct 2017 10:40:16 +0800 Subject: [PATCH] UefiCpuPkg/MpInitLib: Avoid call PcdGet* in Ap & Bsp. MicrocodeDetect function will run by every threads, and it will use PcdGet to get PcdCpuMicrocodePatchAddress and PcdCpuMicrocodePatchRegionSize, if change both PCD default to dynamic, system will in non-deterministic behavior. By design, UEFI/PI services are single threaded and not re-entrant so Multi processor code should not use UEFI/PI services. Here, Pcd protocol/PPI is used to access dynamic PCDs so it would result in non-deterministic behavior. This code get PCD value in BSP and save them in CPU_MP_DATA for Ap. https://bugzilla.tianocore.org/show_bug.cgi?id=726 Cc: Crystal Lee Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong Reviewed-by: Ruiyu Ni --- UefiCpuPkg/Library/MpInitLib/Microcode.c | 12 ++++-------- UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 ++ UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 ++ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c index 982995be7d..9d61da6c23 100644 --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c @@ -1,7 +1,7 @@ /** @file Implementation of loading microcode on processors. - Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+ Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -42,8 +42,6 @@ MicrocodeDetect ( IN CPU_MP_DATA *CpuMpData ) { - UINT64 MicrocodePatchAddress; - UINT64 MicrocodePatchRegionSize; UINT32 ExtendedTableLength; UINT32 ExtendedTableCount; CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable; @@ -61,9 +59,7 @@ MicrocodeDetect ( VOID *MicrocodeData; MSR_IA32_PLATFORM_ID_REGISTER PlatformIdMsr; - MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress); - MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize); - if (MicrocodePatchRegionSize == 0) { + if (CpuMpData->MicrocodePatchRegionSize == 0) { // // There is no microcode patches // @@ -93,8 +89,8 @@ MicrocodeDetect ( LatestRevision = 0; MicrocodeData = NULL; - MicrocodeEnd = (UINTN) (MicrocodePatchAddress + MicrocodePatchRegionSize); - MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) MicrocodePatchAddress; + MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + CpuMpData->MicrocodePatchRegionSize); + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress; do { // // Check if the microcode is for the Cpu and the version is newer diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 924b9097f2..f3ee6d4436 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1458,6 +1458,8 @@ MpInitLibInitialize ( CpuMpData->SwitchBspFlag = FALSE; CpuMpData->CpuData = (CPU_AP_DATA *) (CpuMpData + 1); CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber); + CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress); + CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize); InitializeSpinLock(&CpuMpData->MpLock); // // Save BSP's Control registers to APs diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 19defdaf4b..84ae24f88d 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -233,6 +233,8 @@ struct _CPU_MP_DATA { UINT8 Vector; BOOLEAN PeriodicMode; BOOLEAN TimerInterruptState; + UINT64 MicrocodePatchAddress; + UINT64 MicrocodePatchRegionSize; }; extern EFI_GUID mCpuInitMpLibHobGuid; -- 2.39.2