From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web11.5322.1578627506817694152 for ; Thu, 09 Jan 2020 19:38:26 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 09 Jan 2020 19:38:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,415,1571727600"; d="scan'208";a="423459058" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga006.fm.intel.com with ESMTP; 09 Jan 2020 19:38:26 -0800 Received: from fmsmsx124.amr.corp.intel.com (10.18.125.39) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jan 2020 19:38:25 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx124.amr.corp.intel.com (10.18.125.39) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 9 Jan 2020 19:38:25 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.202]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.55]) with mapi id 14.03.0439.000; Fri, 10 Jan 2020 11:38:23 +0800 From: "Dong, Eric" To: "devel@edk2.groups.io" , "Fu, Siyuan" CC: "Kinney, Michael D" , "Gao, Liming" , "Ni, Ray" , "lersek@redhat.com" Subject: Re: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg: Shadow microcode patch according to FIT microcode entry. Thread-Topic: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg: Shadow microcode patch according to FIT microcode entry. Thread-Index: AQHVxpJ/0fCLn+OxoUqk4NI/z6Hv+qfjQZJQ Date: Fri, 10 Jan 2020 03:38:23 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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 [mailto:devel@edk2.groups.io] On Behalf Of > Siyuan, Fu > Sent: Thursday, January 9, 2020 10:14 AM > To: devel@edk2.groups.io > Cc: Kinney, Michael D ; Gao, Liming > ; Dong, Eric ; Ni, Ray > ; lersek@redhat.com > Subject: [edk2-devel] [PATCH v2 2/2] UefiCpuPkg: Shadow microcode patch > according to FIT microcode entry. >=20 > The existing MpInitLib will shadow the microcode update patches from fla= sh > to memory and this is done by searching microcode region specified by PC= D > PcdCpuMicrocodePatchAddress and PcdCpuMicrocodePatchRegionSize. > This brings a limition to platform FW that all the microcode patches mus= t be > placed in one continuous flash space. >=20 > This patch shadows microcode update according to FIT microcode entries i= f > it's present, otherwise it will fallback to original logic (by PCD). >=20 > A new featured PCD > gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit > is added for enabling/disabling this support. >=20 > TEST: Tested on FIT enabled platform. > BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=3D2449 >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Signed-off-by: Siyuan Fu > --- > UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + > UefiCpuPkg/Library/MpInitLib/Microcode.c | 262 +++++++++++++----- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 4 +- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 7 +- > UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +- > UefiCpuPkg/UefiCpuPkg.dec | 6 + > 6 files changed, 216 insertions(+), 68 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > index cd912ab0c5..0fd420ac93 100644 > --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > @@ -69,4 +69,5 @@ > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## > SOMETIMES_CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## > CONSUMES > + gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit ## > CONSUMES >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c > b/UefiCpuPkg/Library/MpInitLib/Microcode.c > index 8f4d4da40c..9389e52ae5 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c > @@ -318,7 +318,7 @@ Done: > } >=20 > /** > - Determine if a microcode patch will be loaded into memory. > + Determine if a microcode patch matchs the specific processor signatur= e > and flag. >=20 > @param[in] CpuMpData The pointer to CPU MP Data structur= e. > @param[in] ProcessorSignature The processor signature field value > @@ -330,7 +330,7 @@ Done: > @retval FALSE The specified microcode patch will not be loaded. > **/ > BOOLEAN > -IsMicrocodePatchNeedLoad ( > +IsProcessorMatchedMicrocodePatch ( > IN CPU_MP_DATA *CpuMpData, > IN UINT32 ProcessorSignature, > IN UINT32 ProcessorFlags > @@ -351,7 +351,77 @@ IsMicrocodePatchNeedLoad ( } >=20 > /** > - Actual worker function that loads the required microcode patches into > memory. > + Check the 'ProcessorSignature' and 'ProcessorFlags' of the microcode > + patch header with the CPUID and PlatformID of the processors within > + system to decide if it will be copied into memory. > + > + @param[in] CpuMpData The pointer to CPU MP Data structur= e. > + @param[in] MicrocodeEntryPoint The pointer to the microcode patch > header. > + > + @retval TRUE The specified microcode patch need to be loaded. > + @retval FALSE The specified microcode patch dosen't need to be loa= ded. > +**/ > +BOOLEAN > +IsMicrocodePatchNeedLoad ( > + IN CPU_MP_DATA *CpuMpData, > + CPU_MICROCODE_HEADER *MicrocodeEntryPoint > + ) > +{ > + BOOLEAN NeedLoad; > + UINTN DataSize; > + UINTN TotalSize; > + CPU_MICROCODE_EXTENDED_TABLE_HEADER *ExtendedTableHeader; > + UINT32 ExtendedTableCount; > + CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable; > + UINTN Index; > + > + // > + // Check the 'ProcessorSignature' and 'ProcessorFlags' in microcode p= atch > header. > + // > + NeedLoad =3D IsProcessorMatchedMicrocodePatch ( > + CpuMpData, > + MicrocodeEntryPoint->ProcessorSignature.Uint32, > + MicrocodeEntryPoint->ProcessorFlags > + ); > + > + // > + // If the Extended Signature Table exists, check if the processor is > + in the // support list // DataSize =3D > + MicrocodeEntryPoint->DataSize; TotalSize =3D (DataSize =3D=3D 0) ? 20= 48 : > + MicrocodeEntryPoint->TotalSize; if ((!NeedLoad) && (DataSize !=3D 0) = && > + (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) + > + sizeof (CPU_MICROCODE_EXTENDED_TABLE_HEAD= ER))) { > + ExtendedTableHeader =3D (CPU_MICROCODE_EXTENDED_TABLE_HEADER > *) ((UINT8 *) (MicrocodeEntryPoint) > + + DataSize + sizeof (CPU_MICROCODE_HEADER))= ; > + ExtendedTableCount =3D ExtendedTableHeader->ExtendedSignatureCount= ; > + ExtendedTable =3D (CPU_MICROCODE_EXTENDED_TABLE *) > (ExtendedTableHeader + 1); > + > + for (Index =3D 0; Index < ExtendedTableCount; Index ++) { > + // > + // Check the 'ProcessorSignature' and 'ProcessorFlag' of the Exte= nded > + // Signature Table entry with the CPUID and PlatformID of the > processors > + // within system to decide if it will be copied into memory > + // > + NeedLoad =3D IsProcessorMatchedMicrocodePatch ( > + CpuMpData, > + ExtendedTable->ProcessorSignature.Uint32, > + ExtendedTable->ProcessorFlag > + ); > + if (NeedLoad) { > + break; > + } > + ExtendedTable ++; > + } > + } > + > + return NeedLoad; > +} > + > + > +/** > + Actual worker function that shadows the required microcode patches in= to > memory. >=20 > @param[in, out] CpuMpData The pointer to CPU MP Data structur= e. > @param[in] Patches The pointer to an array of informat= ion on > @@ -363,7 +433,7 @@ IsMicrocodePatchNeedLoad ( > to be loaded. > **/ > VOID > -LoadMicrocodePatchWorker ( > +ShadowMicrocodePatchWorker ( > IN OUT CPU_MP_DATA *CpuMpData, > IN MICROCODE_PATCH_INFO *Patches, > IN UINTN PatchCount, > @@ -390,7 +460,6 @@ LoadMicrocodePatchWorker ( > (VOID *) Patches[Index].Address, > Patches[Index].Size > ); > - > Walker +=3D Patches[Index].Size; > } >=20 > @@ -410,12 +479,13 @@ LoadMicrocodePatchWorker ( } >=20 > /** > - Load the required microcode patches data into memory. > + Shadow the required microcode patches data into memory according to > + PCD PcdCpuMicrocodePatchAddress and > PcdCpuMicrocodePatchRegionSize. >=20 > @param[in, out] CpuMpData The pointer to CPU MP Data structure. > **/ > VOID > -LoadMicrocodePatch ( > +ShadowMicrocodePatchByPcd ( > IN OUT CPU_MP_DATA *CpuMpData > ) > { > @@ -423,15 +493,10 @@ LoadMicrocodePatch ( > UINTN MicrocodeEnd; > UINTN DataSize; > UINTN TotalSize; > - CPU_MICROCODE_EXTENDED_TABLE_HEADER *ExtendedTableHeader; > - UINT32 ExtendedTableCount; > - CPU_MICROCODE_EXTENDED_TABLE *ExtendedTable; > MICROCODE_PATCH_INFO *PatchInfoBuffer; > UINTN MaxPatchNumber; > UINTN PatchCount; > UINTN TotalLoadSize; > - UINTN Index; > - BOOLEAN NeedLoad; >=20 > // > // Initialize the microcode patch related fields in CpuMpData as the = values > @@ -487,55 +552,7 @@ LoadMicrocodePatch ( > continue; > } >=20 > - // > - // Check the 'ProcessorSignature' and 'ProcessorFlags' of the micro= code > - // patch header with the CPUID and PlatformID of the processors wit= hin > - // system to decide if it will be copied into memory > - // > - NeedLoad =3D IsMicrocodePatchNeedLoad ( > - CpuMpData, > - MicrocodeEntryPoint->ProcessorSignature.Uint32, > - MicrocodeEntryPoint->ProcessorFlags > - ); > - > - // > - // If the Extended Signature Table exists, check if the processor i= s in the > - // support list > - // > - if ((!NeedLoad) && (DataSize !=3D 0) && > - (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) + > - sizeof (CPU_MICROCODE_EXTENDED_TABLE_HE= ADER))) { > - ExtendedTableHeader =3D (CPU_MICROCODE_EXTENDED_TABLE_HEADER > *) ((UINT8 *) (MicrocodeEntryPoint) > - + DataSize + sizeof (CPU_MICROCODE_HEADER= )); > - ExtendedTableCount =3D ExtendedTableHeader- > >ExtendedSignatureCount; > - ExtendedTable =3D (CPU_MICROCODE_EXTENDED_TABLE *) > (ExtendedTableHeader + 1); > - > - for (Index =3D 0; Index < ExtendedTableCount; Index ++) { > - // > - // Avoid access content beyond MicrocodeEnd > - // > - if ((UINTN) ExtendedTable > MicrocodeEnd - sizeof > (CPU_MICROCODE_EXTENDED_TABLE)) { > - break; > - } > - > - // > - // Check the 'ProcessorSignature' and 'ProcessorFlag' of the Ex= tended > - // Signature Table entry with the CPUID and PlatformID of the > processors > - // within system to decide if it will be copied into memory > - // > - NeedLoad =3D IsMicrocodePatchNeedLoad ( > - CpuMpData, > - ExtendedTable->ProcessorSignature.Uint32, > - ExtendedTable->ProcessorFlag > - ); > - if (NeedLoad) { > - break; > - } > - ExtendedTable ++; > - } > - } > - > - if (NeedLoad) { > + if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) { > PatchCount++; > if (PatchCount > MaxPatchNumber) { > // > @@ -581,7 +598,7 @@ LoadMicrocodePatch ( > __FUNCTION__, PatchCount, TotalLoadSize > )); >=20 > - LoadMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, > TotalLoadSize); > + ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, > PatchCount, > + TotalLoadSize); > } >=20 > OnExit: > @@ -590,3 +607,124 @@ OnExit: > } > return; > } > + > +/** > + Shadow the required microcode patches data into memory according to > FIT microcode entry. > + > + @param[in, out] CpuMpData The pointer to CPU MP Data structure. > + > + @return EFI_SUCCESS Microcode patch is shadowed into memory= . > + @return EFI_UNSUPPORTED FIT based microcode shadowing is not > supported. > + @return EFI_OUT_OF_RESOURCES No enough memory resource. > + @return EFI_NOT_FOUND There is something wrong in FIT microco= de > entry. > + > +**/ > +EFI_STATUS > +ShadowMicrocodePatchByFit ( > + IN OUT CPU_MP_DATA *CpuMpData > + ) > +{ > + UINT64 FitPointer; > + FIRMWARE_INTERFACE_TABLE_ENTRY *FitEntry; > + UINT32 EntryNum; > + UINT32 Index; > + MICROCODE_PATCH_INFO *PatchInfoBuffer; > + UINTN MaxPatchNumber; > + CPU_MICROCODE_HEADER *MicrocodeEntryPoint; > + UINTN PatchCount; > + UINTN TotalSize; > + UINTN TotalLoadSize; > + > + if (!FeaturePcdGet (PcdCpuShadowMicrocodeByFit)) { > + return EFI_UNSUPPORTED; > + } > + > + FitPointer =3D *(UINT64 *) (UINTN) FIT_POINTER_ADDRESS; if > + ((FitPointer =3D=3D 0) || > + (FitPointer =3D=3D 0xFFFFFFFFFFFFFFFF) || > + (FitPointer =3D=3D 0xEEEEEEEEEEEEEEEE)) { > + // > + // No FIT table. > + // > + ASSERT (FALSE); > + return EFI_NOT_FOUND; > + } > + FitEntry =3D (FIRMWARE_INTERFACE_TABLE_ENTRY *) (UINTN) FitPointer; = if > + ((FitEntry[0].Type !=3D FIT_TYPE_00_HEADER) || > + (FitEntry[0].Address !=3D FIT_TYPE_00_SIGNATURE)) { > + // > + // Invalid FIT table, treat it as no FIT table. > + // > + ASSERT (FALSE); > + return EFI_NOT_FOUND; > + } > + > + EntryNum =3D *(UINT32 *)(&FitEntry[0].Size[0]) & 0xFFFFFF; > + > + // > + // Calculate microcode entry number > + // > + MaxPatchNumber =3D 0; > + for (Index =3D 0; Index < EntryNum; Index++) { > + if (FitEntry[Index].Type =3D=3D FIT_TYPE_01_MICROCODE) { > + MaxPatchNumber++; > + } > + } > + if (MaxPatchNumber =3D=3D 0) { > + return EFI_NOT_FOUND; > + } > + > + PatchInfoBuffer =3D AllocatePool (MaxPatchNumber * sizeof > + (MICROCODE_PATCH_INFO)); if (PatchInfoBuffer =3D=3D NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + // > + // Fill up microcode patch info buffer according to FIT table. > + // > + PatchCount =3D 0; > + TotalLoadSize =3D 0; > + for (Index =3D 0; Index < EntryNum; Index++) { > + if (FitEntry[Index].Type =3D=3D FIT_TYPE_01_MICROCODE) { > + MicrocodeEntryPoint =3D (CPU_MICROCODE_HEADER *) (UINTN) > FitEntry[Index].Address; > + TotalSize =3D (MicrocodeEntryPoint->DataSize =3D=3D 0) ? 2048 : > MicrocodeEntryPoint->TotalSize; > + if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) { > + PatchInfoBuffer[PatchCount].Address =3D (UINTN) > MicrocodeEntryPoint; > + PatchInfoBuffer[PatchCount].Size =3D TotalSize; > + TotalLoadSize +=3D TotalSize; > + PatchCount++; > + } > + } > + } > + > + if (PatchCount !=3D 0) { > + DEBUG (( > + DEBUG_INFO, > + "%a: 0x%x microcode patches will be loaded into memory, with size > 0x%x.\n", > + __FUNCTION__, PatchCount, TotalLoadSize > + )); > + > + ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, > PatchCount, > + TotalLoadSize); } > + > + FreePool (PatchInfoBuffer); > + return EFI_SUCCESS; > +} > + > +/** > + Shadow the required microcode patches data into memory. > + > + @param[in, out] CpuMpData The pointer to CPU MP Data structure. > +**/ > +VOID > +ShadowMicrocodeUpdatePatch ( > + IN OUT CPU_MP_DATA *CpuMpData > + ) > +{ > + EFI_STATUS Status; > + > + Status =3D ShadowMicrocodePatchByFit (CpuMpData); > + if (EFI_ERROR (Status)) { > + ShadowMicrocodePatchByPcd (CpuMpData); > + } > +} > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index e611a8ca40..6ec9b172b8 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1,7 +1,7 @@ > /** @file > CPU MP Initialize Library common functions. >=20 > - Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved. > + Copyright (c) 2016 - 2020, Intel Corporation. All rights > + reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -1744,7 +1744,7 @@ MpInitLibInitialize ( > // > // Load required microcode patches data into memory > // > - LoadMicrocodePatch (CpuMpData); > + ShadowMicrocodeUpdatePatch (CpuMpData); > } else { > // > // APs have been wakeup before, just get the CPU Information diff -= -git > a/UefiCpuPkg/Library/MpInitLib/MpLib.h > b/UefiCpuPkg/Library/MpInitLib/MpLib.h > index b6e5a1afab..7c62d75acc 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h > @@ -29,6 +29,9 @@ > #include > #include >=20 > +#include > + > + > #define WAKEUP_AP_SIGNAL SIGNATURE_32 ('S', 'T', 'A', 'P') >=20 > #define CPU_INIT_MP_LIB_HOB_GUID \ > @@ -587,12 +590,12 @@ MicrocodeDetect ( > ); >=20 > /** > - Load the required microcode patches data into memory. > + Shadow the required microcode patches data into memory. >=20 > @param[in, out] CpuMpData The pointer to CPU MP Data structure. > **/ > VOID > -LoadMicrocodePatch ( > +ShadowMicrocodeUpdatePatch ( > IN OUT CPU_MP_DATA *CpuMpData > ); >=20 > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > index 326703cc9a..5b4a1f31c8 100644 > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf > @@ -1,7 +1,7 @@ > ## @file > # MP Initialize Library instance for PEI driver. > # > -# Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved. > +# Copyright (c) 2016 - 2020, Intel Corporation. All rights > +reserved.
> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -60,7 +60,7 @= @ > gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## > CONSUMES > gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## > SOMETIMES_CONSUMES > - > + gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit ## > CONSUMES > [Guids] > gEdkiiS3SmmInitDoneGuid > gEdkiiMicrocodePatchHobGuid > diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec > index 45b267ac61..a6ebdde1cf 100644 > --- a/UefiCpuPkg/UefiCpuPkg.dec > +++ b/UefiCpuPkg/UefiCpuPkg.dec > @@ -139,6 +139,12 @@ > # @Prompt Lock SMM Feature Control MSR. >=20 > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|B > OOLEAN|0x3213210B >=20 > + ## Indicates if FIT based microcode shadowing will be enabled.
> + # TRUE - FIT base microcode shadowing will be enabled.
> + # FALSE - FIT base microcode shadowing will be disabled.
> + # @Prompt FIT based microcode shadowing. > + > + > gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit|FALSE|BOOLE > AN|0x3 > + 213210D > + > [PcdsFixedAtBuild] > ## List of exception vectors which need switching stack. > # This PCD will only take into effect if PcdCpuStackGuard is enabled= . > -- > 2.19.1.windows.1 >=20 >=20 >=20