From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web09.7401.1581042872954345661 for ; Thu, 06 Feb 2020 18:34:33 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 06 Feb 2020 18:34:32 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,411,1574150400"; d="scan'208";a="379264660" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga004.jf.intel.com with ESMTP; 06 Feb 2020 18:34:32 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 6 Feb 2020 18:34:32 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.126]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.222]) with mapi id 14.03.0439.000; Fri, 7 Feb 2020 10:34:29 +0800 From: "Dong, Eric" To: "devel@edk2.groups.io" , "Wu, Hao A" CC: "Kubacki, Michael A" , "Kinney, Michael D" , "Ni, Ray" , Laszlo Ersek Subject: Re: [edk2-devel] [PATCH v1 2/2] UefiCpuPkg/MpInitLib: Not pass microcode info between archs in CPU_MP_DATA Thread-Topic: [edk2-devel] [PATCH v1 2/2] UefiCpuPkg/MpInitLib: Not pass microcode info between archs in CPU_MP_DATA Thread-Index: AQHV3K2q7eIZOKYh30C0wqaeRUNP8qgPBNvw Date: Fri, 7 Feb 2020 02:34:29 +0000 Message-ID: References: <20200206052356.3672-1-hao.a.wu@intel.com> <20200206052356.3672-3-hao.a.wu@intel.com> In-Reply-To: <20200206052356.3672-3-hao.a.wu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjI4MzAyOTUtNzcxNC00YzA3LTk2NTEtZjcwNGU5MjM4Y2UxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiczJQWGVRbk5iMHU3aE53NEJWU2JmUVUyU1lhSlFEM3dydkxWb1FUeHRkOUNQRDBTWlIwdmhMQVwvYU9UdlZLMjkifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong -----Original Message----- From: devel@edk2.groups.io On Behalf Of Wu, Hao A Sent: Thursday, February 6, 2020 1:24 PM To: devel@edk2.groups.io Cc: Wu, Hao A ; Kubacki, Michael A ; Kinney, Michael D ; Dong, Eric ; Ni, Ray ; Laszlo Ersek Subject: [edk2-devel] [PATCH v1 2/2] UefiCpuPkg/MpInitLib: Not pass microc= ode info between archs in CPU_MP_DATA REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D2465 Commit 89164babec: UefiCpuPkg/MpInitLib: don't shadow the microcode patch twice. attempted to use 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' fields to avoid loading the microcode patches data into memory again in th= e DXE phase. However, the CPU_MP_DATA structure has members with type 'UINTN' or pointe= r before the microcode patch related fields. This may cause issues when PEI= and DXE are of different archs (e.g. PEI - IA32, DXE - x64), since the mic= rocode patch related fields will have different offsets in the CPU_MP_DATA = structure. Commit 88bd066166: UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA tried to resolve the above-mentioned issue by relocating the fields 'Micro= codePatchRegionSize' and 'MicrocodePatchAddress' before members with differ= ent size between different archs. But it failed to take the case of pre-bui= lt binaries (e.g. FSP) into consideration. Binaries can be built when the code base had a different version of the CP= U_MP_DATA structure definition. This may cause issues when accessing these = microcode patch related fields, since their offsets are different (between = PEI phase in the binaries and DXE phase in current code implementation). This commit will use the newly introduced EDKII microcode patch HOB instea= d for the DXE phase to get the information of the loaded microcode patches = data done in the PEI phase. And the 'MicrocodePatchRegionSize' and 'Microco= dePatchAddress' fields in CPU_MP_DATA will not be used to pass information = between phases. For pre-built binaries, they can be classified into 3 types with regard to= the time when they are being built: A. Before commit 89164babec (In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' were not being used to skip microcode load in DXE) For this case, the EDKII microcode patch HOB will not be produced. This co= mmit will load the microcode patches data again in DXE. Such behavior is th= e same with the code base back then. B. After commit 89164babec, before commit e1ed55738e (In other words, 'MicrocodePatchRegionSize' and 'MicrocodePatchAddress' being used to skip microcode load in DXE, but failed to work properly between differnt archs.) For this case, the EDKII microcode patch HOB will not be produced as well. This commit will also load the microcode patches data again in DXE. But since commit 89164babec failed to keep the detection and application o= f microcode patches working properly in DXE after skipping the load, we fal= l back to the origin behavior (that is to load the microcode patches data a= gain in DXE). C. After commit e1ed55738e (In other words, EDKII microcode patch HOB will be produced.) For this case, it will have the same behavior with the BIOS built from the= current source codes. Cc: Michael Kubacki Cc: Michael D Kinney Cc: Eric Dong Cc: Ray Ni Cc: Laszlo Ersek Signed-off-by: Hao A Wu --- UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 3 +- UefiCpuPkg/Library/MpInitLib/MpLib.h | 23 +++++++++++ UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 ++++++++++++++++++++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 20 +++++---- UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 3 +- 5 files changed, 80 insertions(+), 12 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Li= brary/MpInitLib/DxeMpInitLib.inf index bf5d18d521..9e6cce0895 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf @@ -1,7 +1,7 @@ ## @file # MP Initialize Library instance for DXE driver. # -# Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 2020, Intel Corporation. All rights=20 +reserved.
# SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -58,6 +58,7 @@ = [Protocols] [Guids] gEfiEventExitBootServicesGuid ## CONSUMES ## Event gEfiEventLegacyBootGuid ## SOMETIMES_CONSUMES ##= Event + gEdkiiMicrocodePatchHobGuid ## SOMETIMES_CONSUMES ##= HOB =20 [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CON= SUMES diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpI= nitLib/MpLib.h index d7e20f0b74..a6eab5f3d7 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -29,6 +29,8 @@ #include #include =20 +#include + #include =20 =20 @@ -600,6 +602,27 @@ ShadowMicrocodeUpdatePatch ( ); =20 /** + Get the cached microcode patch base address and size from the=20 + microcode patch information cache HOB. + + @param[out] Address Base address of the microcode patches data. + It will be updated if the microcode patch + information cache HOB is found. + @param[out] RegionSize Size of the microcode patches data. + It will be updated if the microcode patch + information cache HOB is found. + + @retval TRUE The microcode patch information cache HOB is found. + @retval FALSE The microcode patch information cache HOB is not foun= d. + +**/ +BOOLEAN +GetMicrocodePatchInfoFromHob ( + UINT64 *Address, + UINT64 *RegionSize + ); + +/** Detect whether Mwait-monitor feature is supported. =20 @retval TRUE Mwait-monitor feature is supported. diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library= /MpInitLib/Microcode.c index 247f835b09..67e214d463 100644 --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c @@ -739,3 +739,46 @@ ShadowMicrocodeUpdatePatch ( ShadowMicrocodePatchByPcd (CpuMpData); } } + +/** + Get the cached microcode patch base address and size from the=20 +microcode patch + information cache HOB. + + @param[out] Address Base address of the microcode patches data. + It will be updated if the microcode patch + information cache HOB is found. + @param[out] RegionSize Size of the microcode patches data. + It will be updated if the microcode patch + information cache HOB is found. + + @retval TRUE The microcode patch information cache HOB is found. + @retval FALSE The microcode patch information cache HOB is not foun= d. + +**/ +BOOLEAN +GetMicrocodePatchInfoFromHob ( + UINT64 *Address, + UINT64 *RegionSize + ) +{ + EFI_HOB_GUID_TYPE *GuidHob; + EDKII_MICROCODE_PATCH_HOB *MicrocodePathHob; + + GuidHob =3D GetFirstGuidHob (&gEdkiiMicrocodePatchHobGuid); if=20 + (GuidHob =3D=3D NULL) { + DEBUG((DEBUG_INFO, "%a: Microcode patch cache HOB is not found.\n", _= _FUNCTION__)); + return FALSE; + } + + MicrocodePathHob =3D GET_GUID_HOB_DATA (GuidHob); + + *Address =3D MicrocodePathHob->MicrocodePatchAddress; + *RegionSize =3D MicrocodePathHob->MicrocodePatchRegionSize; + + DEBUG(( + DEBUG_INFO, "%a: MicrocodeBase =3D 0x%lx, MicrocodeSize =3D 0x%lx\n", + __FUNCTION__, *Address, *RegionSize + )); + + return TRUE; +} diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpI= nitLib/MpLib.c index 855d37ba3e..d0fbc17ce5 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1682,10 +1682,6 @@ MpInitLibInitialize ( CpuMpData->SwitchBspFlag =3D FALSE; CpuMpData->CpuData =3D (CPU_AP_DATA *) (CpuMpData + 1); CpuMpData->CpuInfoInHob =3D (UINT64) (UINTN) (CpuMpData->CpuData + = MaxLogicalProcessorNumber); - if (OldCpuMpData !=3D NULL) { - CpuMpData->MicrocodePatchRegionSize =3D OldCpuMpData->MicrocodePatchR= egionSize; - CpuMpData->MicrocodePatchAddress =3D OldCpuMpData->MicrocodePatchA= ddress; - } InitializeSpinLock(&CpuMpData->MpLock); =20 // @@ -1740,11 +1736,6 @@ MpInitLibInitialize ( // CollectProcessorCount (CpuMpData); } - - // - // Load required microcode patches data into memory - // - ShadowMicrocodeUpdatePatch (CpuMpData); } else { // // APs have been wakeup before, just get the CPU Information @@ -1762= ,6 +1753,17 @@ MpInitLibInitialize ( } } =20 + if (!GetMicrocodePatchInfoFromHob ( + &CpuMpData->MicrocodePatchAddress, + &CpuMpData->MicrocodePatchRegionSize + )) { + // + // The microcode patch information cache HOB does not exist, which me= ans + // the microcode patches data has not been loaded into memory yet + // + ShadowMicrocodeUpdatePatch (CpuMpData); } + // // Detect and apply Microcode on BSP // diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/= MpInitLib/PeiMpLib.c index 06e3f5d0d3..6ecbed39ec 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -1,7 +1,7 @@ /** @file MP initialize support functions for PEI phase. =20 - Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.
+ Copyright (c) 2016 - 2020, Intel Corporation. All rights=20 + reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ @@ -9,7 +9,6 @@ #include "MpLib.h" #include #include -#include =20 /** S3 SMM Init Done notification function. -- 2.12.0.windows.1