* Re: [edk2-devel] [Patch] UefiCpuPkg: Shadow microcode patch according to FIT microcode entry.
[not found] <15E8732525669510.23004@groups.io>
@ 2020-01-10 7:05 ` Siyuan, Fu
0 siblings, 0 replies; only message in thread
From: Siyuan, Fu @ 2020-01-10 7:05 UTC (permalink / raw)
To: devel@edk2.groups.io, Fu, Siyuan; +Cc: Dong, Eric, Ni, Ray, Laszlo Ersek
Send by misoperation。 This patch has already been merged, pls ignore.
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Siyuan,
> Fu
> Sent: 2020年1月10日 14:38
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [edk2-devel] [Patch] UefiCpuPkg: Shadow microcode patch
> according to FIT microcode entry.
>
> The existing MpInitLib will shadow the microcode update patches from
> flash to memory and this is done by searching microcode region specified
> by PCD PcdCpuMicrocodePatchAddress and
> PcdCpuMicrocodePatchRegionSize.
> This brings a limition to platform FW that all the microcode patches must
> be placed in one continuous flash space.
>
> This patch shadows microcode update according to FIT microcode entries if
> it's present, otherwise it will fallback to original logic (by PCD).
>
> A new featured PCD
> gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit
> is added for enabling/disabling this support.
>
> TEST: Tested on FIT enabled platform.
> BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
> Reviewed-by: Eric Dong <eric.dong@intel.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 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 | 3 +-
> UefiCpuPkg/UefiCpuPkg.dec | 6 +
> UefiCpuPkg/UefiCpuPkg.uni | 6 +
> 7 files changed, 222 insertions(+), 67 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index cd912ab0c5..bf5d18d521 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -68,5 +68,6 @@
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ##
> SOMETIMES_CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit ##
> CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ##
> CONSUMES
>
> 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:
> }
>
> /**
> - Determine if a microcode patch will be loaded into memory.
> + Determine if a microcode patch matchs the specific processor signature
> and flag.
>
> @param[in] CpuMpData The pointer to CPU MP Data structure.
> @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 (
> }
>
> /**
> - 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 structure.
> + @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 loaded.
> +**/
> +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 patch
> header.
> + //
> + NeedLoad = IsProcessorMatchedMicrocodePatch (
> + CpuMpData,
> + MicrocodeEntryPoint->ProcessorSignature.Uint32,
> + MicrocodeEntryPoint->ProcessorFlags
> + );
> +
> + //
> + // If the Extended Signature Table exists, check if the processor is in the
> + // support list
> + //
> + DataSize = MicrocodeEntryPoint->DataSize;
> + TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
> + if ((!NeedLoad) && (DataSize != 0) &&
> + (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) +
> + sizeof (CPU_MICROCODE_EXTENDED_TABLE_HEADER))) {
> + ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER *)
> ((UINT8 *) (MicrocodeEntryPoint)
> + + DataSize + sizeof (CPU_MICROCODE_HEADER));
> + ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
> + ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)
> (ExtendedTableHeader + 1);
> +
> + for (Index = 0; Index < ExtendedTableCount; Index ++) {
> + //
> + // Check the 'ProcessorSignature' and 'ProcessorFlag' of the Extended
> + // Signature Table entry with the CPUID and PlatformID of the
> processors
> + // within system to decide if it will be copied into memory
> + //
> + NeedLoad = IsProcessorMatchedMicrocodePatch (
> + CpuMpData,
> + ExtendedTable->ProcessorSignature.Uint32,
> + ExtendedTable->ProcessorFlag
> + );
> + if (NeedLoad) {
> + break;
> + }
> + ExtendedTable ++;
> + }
> + }
> +
> + return NeedLoad;
> +}
> +
> +
> +/**
> + Actual worker function that shadows the required microcode patches into
> memory.
>
> @param[in, out] CpuMpData The pointer to CPU MP Data structure.
> @param[in] Patches The pointer to an array of information 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 += Patches[Index].Size;
> }
>
> @@ -410,12 +479,13 @@ LoadMicrocodePatchWorker (
> }
>
> /**
> - Load the required microcode patches data into memory.
> + Shadow the required microcode patches data into memory according to
> PCD
> + PcdCpuMicrocodePatchAddress and PcdCpuMicrocodePatchRegionSize.
>
> @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;
>
> //
> // Initialize the microcode patch related fields in CpuMpData as the values
> @@ -487,55 +552,7 @@ LoadMicrocodePatch (
> continue;
> }
>
> - //
> - // 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
> - //
> - NeedLoad = IsMicrocodePatchNeedLoad (
> - CpuMpData,
> - MicrocodeEntryPoint->ProcessorSignature.Uint32,
> - MicrocodeEntryPoint->ProcessorFlags
> - );
> -
> - //
> - // If the Extended Signature Table exists, check if the processor is in the
> - // support list
> - //
> - if ((!NeedLoad) && (DataSize != 0) &&
> - (TotalSize - DataSize > sizeof (CPU_MICROCODE_HEADER) +
> - sizeof (CPU_MICROCODE_EXTENDED_TABLE_HEADER))) {
> - ExtendedTableHeader = (CPU_MICROCODE_EXTENDED_TABLE_HEADER
> *) ((UINT8 *) (MicrocodeEntryPoint)
> - + DataSize + sizeof (CPU_MICROCODE_HEADER));
> - ExtendedTableCount = ExtendedTableHeader->ExtendedSignatureCount;
> - ExtendedTable = (CPU_MICROCODE_EXTENDED_TABLE *)
> (ExtendedTableHeader + 1);
> -
> - for (Index = 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 Extended
> - // Signature Table entry with the CPUID and PlatformID of the
> processors
> - // within system to decide if it will be copied into memory
> - //
> - NeedLoad = 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
> ));
>
> - LoadMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount,
> TotalLoadSize);
> + ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer,
> PatchCount, TotalLoadSize);
> }
>
> 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 microcode
> 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 = *(UINT64 *) (UINTN) FIT_POINTER_ADDRESS;
> + if ((FitPointer == 0) ||
> + (FitPointer == 0xFFFFFFFFFFFFFFFF) ||
> + (FitPointer == 0xEEEEEEEEEEEEEEEE)) {
> + //
> + // No FIT table.
> + //
> + ASSERT (FALSE);
> + return EFI_NOT_FOUND;
> + }
> + FitEntry = (FIRMWARE_INTERFACE_TABLE_ENTRY *) (UINTN) FitPointer;
> + if ((FitEntry[0].Type != FIT_TYPE_00_HEADER) ||
> + (FitEntry[0].Address != FIT_TYPE_00_SIGNATURE)) {
> + //
> + // Invalid FIT table, treat it as no FIT table.
> + //
> + ASSERT (FALSE);
> + return EFI_NOT_FOUND;
> + }
> +
> + EntryNum = *(UINT32 *)(&FitEntry[0].Size[0]) & 0xFFFFFF;
> +
> + //
> + // Calculate microcode entry number
> + //
> + MaxPatchNumber = 0;
> + for (Index = 0; Index < EntryNum; Index++) {
> + if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> + MaxPatchNumber++;
> + }
> + }
> + if (MaxPatchNumber == 0) {
> + return EFI_NOT_FOUND;
> + }
> +
> + PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof
> (MICROCODE_PATCH_INFO));
> + if (PatchInfoBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Fill up microcode patch info buffer according to FIT table.
> + //
> + PatchCount = 0;
> + TotalLoadSize = 0;
> + for (Index = 0; Index < EntryNum; Index++) {
> + if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
> + MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN)
> FitEntry[Index].Address;
> + TotalSize = (MicrocodeEntryPoint->DataSize == 0) ? 2048 :
> MicrocodeEntryPoint->TotalSize;
> + if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) {
> + PatchInfoBuffer[PatchCount].Address = (UINTN)
> MicrocodeEntryPoint;
> + PatchInfoBuffer[PatchCount].Size = TotalSize;
> + TotalLoadSize += TotalSize;
> + PatchCount++;
> + }
> + }
> + }
> +
> + if (PatchCount != 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 = 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.
>
> - Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -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 <Library/MtrrLib.h>
> #include <Library/HobLib.h>
>
> +#include <IndustryStandard/FirmwareInterfaceTable.h>
> +
> +
> #define WAKEUP_AP_SIGNAL SIGNATURE_32 ('S', 'T', 'A', 'P')
>
> #define CPU_INIT_MP_LIB_HOB_GUID \
> @@ -587,12 +590,12 @@ MicrocodeDetect (
> );
>
> /**
> - Load the required microcode patches data into memory.
> + Shadow the required microcode patches data into memory.
>
> @param[in, out] CpuMpData The pointer to CPU MP Data structure.
> **/
> VOID
> -LoadMicrocodePatch (
> +ShadowMicrocodeUpdatePatch (
> IN OUT CPU_MP_DATA *CpuMpData
> );
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index 326703cc9a..555125a7c5 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.<BR>
> +# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -60,6 +60,7 @@
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ##
> SOMETIMES_CONSUMES
> + gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit ##
> CONSUMES
>
> [Guids]
> gEdkiiS3SmmInitDoneGuid
> 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.
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|BO
> OLEAN|0x3213210B
>
> + ## Indicates if FIT based microcode shadowing will be enabled.<BR><BR>
> + # TRUE - FIT base microcode shadowing will be enabled.<BR>
> + # FALSE - FIT base microcode shadowing will be disabled.<BR>
> + # @Prompt FIT based microcode shadowing.
> +
> gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit|FALSE|BOOLE
> AN|0x3213210D
> +
> [PcdsFixedAtBuild]
> ## List of exception vectors which need switching stack.
> # This PCD will only take into effect if PcdCpuStackGuard is enabled.
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
> index c0d6ed5136..2db49e841b 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -100,6 +100,12 @@
> "TRUE - locked.<BR>\n"
> "FALSE - unlocked.<BR>"
>
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuShadowMicrocodeByFit_PROMPT
> #language en-US "FIT based microcode shadowing"
> +
> +#string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuShadowMicrocodeByFit_HELP
> #language en-US "Indicates if FIT based microcode shadowing will be
> enabled.<BR><BR>\n"
> + "TRUE - FIT base
> microcode shadowing will be enabled.<BR>\n"
> + "FALSE - FIT base
> microcode shadowing will be disabled.<BR>"
> +
> #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_PROMPT
> #language en-US "Stack size in the temporary RAM"
>
> #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdPeiTemporaryRamStackSize_HELP
> #language en-US "Specifies stack size in the temporary RAM. 0 means half of
> TemporaryRamSize."
> --
> 2.19.1.windows.1
>
>
>
^ permalink raw reply [flat|nested] only message in thread