Hi, Mike Thanks for your comments, I will update patch accordingly and send a V4 for this.
Best Regards Siyuan > -----Original Message----- > From: Kinney, Michael D <michael.d.kin...@intel.com> > Sent: 2020年2月13日 8:58 > To: devel@edk2.groups.io; Fu, Siyuan <siyuan...@intel.com>; Kinney, Michael > D <michael.d.kin...@intel.com> > Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V > <rangasai.v.chaga...@intel.com> > Subject: RE: [edk2-devel] [PATCH v3] IntelSiliconPkg: FIT based shadow > microcode PPI support. > > Siyuan, > > IntelSiliconPkg/Feature/ShadowMicrocode: > > For simple modules that only have a single .c file, there > Is not need to split out a .h file. Please merge the .h > File content into the .c file and delete the .h file. > > More comments inline below. > > Mike > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On > > Behalf Of Siyuan, Fu > > Sent: Tuesday, February 11, 2020 4:48 PM > > To: devel@edk2.groups.io > > Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V > > <rangasai.v.chaga...@intel.com> > > Subject: [edk2-devel] [PATCH v3] IntelSiliconPkg: FIT > > based shadow microcode PPI support. > > > > V3 Changes: > > Remove the feature PCD PcdCpuShadowMicrocodeByFit > > because the whole FIT microcode shadow code is moved to > > this PEIM so platform could disable this feature by not > > include PEIM now. > > > > V2 Changes: > > Rename EDKII_PEI_CPU_MICROCODE_ID to > > EDKII_PEI_MICROCODE_CPU_ID. > > > > This patch adds a platform PEIM for FIT based shadow > > microcode PPI support. A detailed design doc can be > > found here: > > https://edk2.groups.io/g/devel/files/Designs/2020/0214/ > > Support%20 > > the%202nd%20Microcode%20FV%20Flash%20Region.pdf > > > > TEST: Tested on FIT enabled platform. > > BZ: > > https://tianocore.acgmultimedia.com/show_bug.cgi?id=244 > > 9 > > > > Cc: Ray Ni <ray...@intel.com> > > Cc: Rangasai V Chaganty <rangasai.v.chaga...@intel.com> > > Signed-off-by: Siyuan Fu <siyuan...@intel.com> > > --- > > .../ShadowMicrocode/ShadowMicrocodePei.c | 387 > > ++++++++++++++++++ > > .../ShadowMicrocode/ShadowMicrocodePei.h | 62 > > +++ > > .../ShadowMicrocode/ShadowMicrocodePei.inf | 44 ++ > > .../Include/Guid/MicrocodeShadowInfoHob.h | 57 > > +++ > > .../Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 6 + > > .../Intel/IntelSiliconPkg/IntelSiliconPkg.dsc | 3 +- > > 6 files changed, 558 insertions(+), 1 deletion(-) > > create mode 100644 > > Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/S > > hadowMicrocodePei.c > > create mode 100644 > > Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/S > > hadowMicrocodePei.h > > create mode 100644 > > Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/S > > hadowMicrocodePei.inf > > create mode 100644 > > Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeSha > > dowInfoHob.h > > > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.c > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.c > > new file mode 100644 > > index 0000000000..c754524f41 > > --- /dev/null > > +++ > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicroc > > +++ odePei.c > > @@ -0,0 +1,387 @@ > > +/** @file > > + Source code file for Platform Init PEI module > > This description does not match the content > > > + > > +Copyright (c) 2017 - 2019, Intel Corporation. All > > rights reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#include "ShadowMicrocodePei.h" > > + > > +EDKII_PEI_SHADOW_MICROCODE_PPI > > mPeiShadowMicrocodePpi = { > > + ShadowMicrocode > > +}; > > + > > + > > +EFI_PEI_PPI_DESCRIPTOR > > mPeiShadowMicrocodePpiList[] = { > > + { > > + EFI_PEI_PPI_DESCRIPTOR_PPI | > > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, > > + &gEdkiiPeiShadowMicrocodePpiGuid, > > + &mPeiShadowMicrocodePpi > > + } > > +}; > > + > > +/** > > + Determine if a microcode patch matchs the specific > > processor signature and flag. > > + > > + @param[in] CpuIdCount Number of elements > > in MicrocodeCpuId array. > > + @param[in] MicrocodeCpuId A pointer to an > > array of EDKII_PEI_MICROCODE_CPU_ID > > + structures. > > + @param[in] ProcessorSignature The processor > > signature field value > > + supported by a > > microcode patch. > > + @param[in] ProcessorFlags The prcessor flags > > field value supported by > > + a microcode patch. > > + > > + @retval TRUE The specified microcode patch will > > be loaded. > > + @retval FALSE The specified microcode patch will > > not be loaded. > > +**/ > > +BOOLEAN > > +IsProcessorMatchedMicrocodePatch ( > > + IN UINTN CpuIdCount, > > + IN EDKII_PEI_MICROCODE_CPU_ID *MicrocodeCpuId, > > + IN UINT32 > > ProcessorSignature, > > + IN UINT32 ProcessorFlags > > + ) > > +{ > > + UINTN Index; > > + > > + for (Index = 0; Index < CpuIdCount; Index++) { > > + if ((ProcessorSignature == > > MicrocodeCpuId[Index].ProcessorSignature) && > > + (ProcessorFlags & (1 << > > MicrocodeCpuId[Index].PlatformId)) != 0) { > > + return TRUE; > > + } > > + } > > + > > + return FALSE; > > +} > > + > > +/** > > + 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] CpuIdCount Number of elements > > in MicrocodeCpuId array. > > + @param[in] MicrocodeCpuId A pointer to an > > array of EDKII_PEI_MICROCODE_CPU_ID > > + structures. > > + @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 UINTN CpuIdCount, > > + IN EDKII_PEI_MICROCODE_CPU_ID *MicrocodeCpuId, > > + 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 ( > > + CpuIdCount, > > + MicrocodeCpuId, > > + 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 ( > > + CpuIdCount, > > + MicrocodeCpuId, > > + ExtendedTable- > > >ProcessorSignature.Uint32, > > + ExtendedTable->ProcessorFlag > > + ); > > + if (NeedLoad) { > > + break; > > + } > > + ExtendedTable ++; > > + } > > + } > > + > > + return NeedLoad; > > +} > > + > > +/** > > + Actual worker function that shadows the required > > microcode patches into memory. > > + > > + @param[in] Patches The pointer to an > > array of information on > > + the microcode > > patches that will be loaded > > + into memory. > > + @param[in] PatchCount The number of > > microcode patches that will > > + be loaded into > > memory. > > + @param[in] TotalLoadSize The total size of > > all the microcode patches > > + to be loaded. > > + @param[out] BufferSize Pointer to receive > > the total size of Buffer. > > + @param[out] Buffer Pointer to receive > > address of allocated memory > > + with microcode > > patches data in it. > > +**/ > > +VOID > > +ShadowMicrocodePatchWorker ( > > + IN MICROCODE_PATCH_INFO *Patches, > > + IN UINTN PatchCount, > > + IN UINTN TotalLoadSize, > > + OUT UINTN *BufferSize, > > + OUT VOID **Buffer > > + ) > > +{ > > + UINTN Index; > > + VOID > > *MicrocodePatchInRam; > > + UINT8 *Walker; > > + EDKII_MICROCODE_SHADOW_INFO_HOB > > *MicrocodeShadowHob; > > + UINTN HobDataLength; > > + UINT64 > > *MicrocodeAddrInMemory; > > Do not abbreviation "Addr". Expand to "Address". > Same comment applies to entire patch. > > > + UINT64 > > *MicrocodeAddrInFlash; > > + > > + ASSERT ((Patches != NULL) && (PatchCount != 0)); > > + > > + // > > + // Init microcode shadow info HOB content. > > + // > > + HobDataLength = sizeof > > (EDKII_MICROCODE_SHADOW_INFO_HOB) + > > + sizeof (UINT64) * PatchCount * 2; > > MicrocodeShadowHob > > + = AllocatePool (HobDataLength); if > > (MicrocodeShadowHob == NULL) { > > + ASSERT (FALSE); > > + return; > > + } > > + MicrocodeShadowHob->MicrocodeCount = PatchCount; > > CopyGuid ( > > + &MicrocodeShadowHob->StorageType, > > + &gEdkiiMicrocodeStorageTypeFlashGuid > > + ); > > + MicrocodeAddrInMemory = (UINT64 *) > > (MicrocodeShadowHob + 1); > > + MicrocodeAddrInFlash = MicrocodeAddrInMemory + > > PatchCount; > > + > > + // > > + // Allocate memory for microcode shadow operation. > > + // > > + MicrocodePatchInRam = AllocatePages > > (EFI_SIZE_TO_PAGES > > + (TotalLoadSize)); if (MicrocodePatchInRam == NULL) { > > + ASSERT (FALSE); > > + return; > > + } > > + > > + // > > + // Shadow all the required microcode patches into > > memory // for > > + (Walker = MicrocodePatchInRam, Index = 0; Index < > > PatchCount; Index++) { > > + CopyMem ( > > + Walker, > > + (VOID *) Patches[Index].Address, > > + Patches[Index].Size > > + ); > > + MicrocodeAddrInMemory[Index] = (UINT64) Walker; > > + MicrocodeAddrInFlash[Index] = (UINT64) > > Patches[Index].Address; > > + Walker += Patches[Index].Size; > > + } > > + > > + // > > + // Update the microcode patch related fields in > > CpuMpData // > > + *Buffer = (VOID *) (UINTN) MicrocodePatchInRam; > > + *BufferSize = TotalLoadSize; > > + > > + BuildGuidDataHob ( > > + &gEdkiiMicrocodeShadowInfoHobGuid, > > + MicrocodeShadowHob, > > + HobDataLength > > + ); > > + > > + DEBUG (( > > + DEBUG_INFO, > > + "%a: Required microcode patches have been loaded > > at 0x%lx, with size 0x%lx.\n", > > + __FUNCTION__, *Buffer, *BufferSize > > + )); > > + > > + return; > > +} > > + > > +/** > > + Shadow the required microcode patches data into > > memory according > > + to FIT microcode entry. > > + > > +**/ > > +EFI_STATUS > > +ShadowMicrocodePatchByFit ( > > + IN UINTN > > CpuIdCount, > > + IN EDKII_PEI_MICROCODE_CPU_ID > > *MicrocodeCpuId, > > + OUT UINTN > > *BufferSize, > > + OUT VOID **Buffer > > + ) > > +{ > > + 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; > > + > > + FitPointer = *(UINT64 *) (UINTN) > > FIT_POINTER_ADDRESS; if > > + ((FitPointer == 0) || > > + (FitPointer == 0xFFFFFFFFFFFFFFFF) || > > + (FitPointer == 0xEEEEEEEEEEEEEEEE)) { > > Are these constants defined in the FIT include file? > Would be better if they are #defines from FIT include > file or in this module. > > > + // > > + // No FIT table. > > + // > > + ASSERT (FALSE); > > Is it appropriate to ASSERT() here? Can this be removed? > Would a DEBUG_ERROR message be better? > > > + 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); > > Is it appropriate to ASSERT() here? Can this be removed? > Would a DEBUG_ERROR message be better? > > > + 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 (CpuIdCount, > > MicrocodeCpuId, 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 (PatchInfoBuffer, > > PatchCount, > > + TotalLoadSize, BufferSize, Buffer); } > > + > > + FreePool (PatchInfoBuffer); > > + return EFI_SUCCESS; > > +} > > + > > + > > +/** > > + Shadow microcode update patches to memory. > > + > > + The function is used for shadowing microcode update > > patches to a continuous memory. > > + It shall allocate memory buffer and only shadow the > > microcode patches > > + for those processors specified by MicrocodeCpuId > > array. The checksum > > + verification may be skiped in this function so the > > caller must > > + perform checksum verification before using the > > microcode patches in returned memory buffer. > > + > > + @param[in] This The PPI instance > > pointer. > > + @param[in] CpuIdCount Number of elements > > in MicrocodeCpuId array. > > + @param[in] MicrocodeCpuId A pointer to an > > array of EDKII_PEI_MICROCODE_CPU_ID > > + structures. > > + @param[out] BufferSize Pointer to receive > > the total size of Buffer. > > + @param[out] Buffer Pointer to receive > > address of allocated memory > > + with microcode > > patches data in it. > > + > > + @retval EFI_SUCCESS The microcode has > > been shadowed to memory. > > + @retval EFI_OUT_OF_RESOURCES The operation fails > > due to lack of resources. > > + > > +**/ > > +EFI_STATUS > > +ShadowMicrocode ( > > + IN EDKII_PEI_SHADOW_MICROCODE_PPI *This, > > + IN UINTN > > CpuIdCount, > > + IN EDKII_PEI_MICROCODE_CPU_ID > > *MicrocodeCpuId, > > + OUT UINTN > > *BufferSize, > > + OUT VOID **Buffer > > + ) > > +{ > > + if (BufferSize == NULL || Buffer == NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + return ShadowMicrocodePatchByFit (CpuIdCount, > > MicrocodeCpuId, > > +BufferSize, Buffer); } > > + > > + > > +/** > > + Platform Init PEI module entry point > > + > > + @param[in] FileHandle Not used. > > + @param[in] PeiServices General purpose > > services available to every PEIM. > > + > > + @retval EFI_SUCCESS The function > > completes successfully > > + @retval EFI_OUT_OF_RESOURCES Insufficient > > resources to create database > > +**/ > > +EFI_STATUS > > +EFIAPI > > +ShadowMicrocodePeimInit ( > > + IN EFI_PEI_FILE_HANDLE FileHandle, > > + IN CONST EFI_PEI_SERVICES **PeiServices > > + ) > > +{ > > + EFI_STATUS Status; > > + > > + // > > + // Install EDKII Shadow Microcode PPI // Status = > > + PeiServicesInstallPpi(mPeiShadowMicrocodePpiList); > > + ASSERT_EFI_ERROR (Status); > > + > > + return Status; > > +} > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.h > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.h > > new file mode 100644 > > index 0000000000..04fe7cbfd3 > > --- /dev/null > > +++ > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicroc > > +++ odePei.h > > @@ -0,0 +1,62 @@ > > +/** @file > > + Source code file for Platform Init PEI module > > This description does not match the content > > > + > > +Copyright (c) 2020, Intel Corporation. All rights > > reserved.<BR> > > +SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef __SHADOW_MICROCODE_PEI_H__ > > +#define __SHADOW_MICROCODE_PEI_H__ > > + > > + > > +#include <PiPei.h> > > +#include <Ppi/ShadowMicrocode.h> > > +#include <Library/PeiServicesLib.h> > > +#include <Library/HobLib.h> > > +#include <Library/DebugLib.h> > > +#include <Library/BaseMemoryLib.h> > > +#include <Library/MemoryAllocationLib.h> #include > > +<IndustryStandard/FirmwareInterfaceTable.h> > > +#include <Register/Intel/Microcode.h> > > +#include <Register/Intel/Cpuid.h> > > +#include <Guid/MicrocodeShadowInfoHob.h> // // Data > > structure for > > +microcode patch information // typedef struct { > > + UINTN Address; > > + UINTN Size; > > +} MICROCODE_PATCH_INFO; > > + > > +/** > > + Shadow microcode update patches to memory. > > + > > + The function is used for shadowing microcode update > > patches to a continuous memory. > > + It shall allocate memory buffer and only shadow the > > microcode patches > > + for those processors specified by MicrocodeCpuId > > array. The checksum > > + verification may be skiped in this function so the > > caller must > > + perform checksum verification before using the > > microcode patches in returned memory buffer. > > + > > + @param[in] This The PPI instance > > pointer. > > + @param[in] CpuIdCount Number of elements > > in MicrocodeCpuId array. > > + @param[in] MicrocodeCpuId A pointer to an > > array of EDKII_PEI_MICROCODE_CPU_ID > > + structures. > > + @param[out] BufferSize Pointer to receive > > the total size of Buffer. > > + @param[out] Buffer Pointer to receive > > address of allocated memory > > + with microcode > > patches data in it. > > + > > + @retval EFI_SUCCESS The microcode has > > been shadowed to memory. > > + @retval EFI_OUT_OF_RESOURCES The operation fails > > due to lack of resources. > > + > > +**/ > > +EFI_STATUS > > +ShadowMicrocode ( > > + IN EDKII_PEI_SHADOW_MICROCODE_PPI *This, > > + IN UINTN > > CpuIdCount, > > + IN EDKII_PEI_MICROCODE_CPU_ID > > *MicrocodeCpuId, > > + OUT UINTN > > *BufferSize, > > + OUT VOID **Buffer > > + ); > > + > > +#endif > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.inf > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicrocodePei.inf > > new file mode 100644 > > index 0000000000..b2773998ce > > --- /dev/null > > +++ > > b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode > > /ShadowMicroc > > +++ odePei.inf > > @@ -0,0 +1,44 @@ > > +### @file > > +# Component information file for the Platform Init PEI > > module. > > This description does not match the file contents. > > > +# > > +# Copyright (c) 2020, Intel Corporation. All rights > > reserved.<BR> # # > > +SPDX-License-Identifier: BSD-2-Clause-Patent # ### > > + > > +[Defines] > > + INF_VERSION = 0x00010017 > > + BASE_NAME = ShadowMicrocodePei > > + FILE_GUID = 8af4cf68-ebe4-4b21- > > a008-0cb3da277be5 > > + VERSION_STRING = 1.0 > > + MODULE_TYPE = PEIM > > + ENTRY_POINT = > > ShadowMicrocodePeimInit > > + > > +[Sources] > > + ShadowMicrocodePei.c > > + ShadowMicrocodePei.h > > + > > +[LibraryClasses] > > + PeimEntryPoint > > + DebugLib > > + MemoryAllocationLib > > + BaseMemoryLib > > + HobLib > > + PeiServicesLib > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + MdeModulePkg/MdeModulePkg.dec > > + UefiCpuPkg/UefiCpuPkg.dec > > + IntelSiliconPkg/IntelSiliconPkg.dec > > + > > +[Ppis] > > + gEdkiiPeiShadowMicrocodePpiGuid > > ## PRODUCES > > + > > +[Guids] > > + gEdkiiMicrocodeShadowInfoHobGuid > > + gEdkiiMicrocodeStorageTypeFlashGuid > > + > > +[Depex] > > + TRUE > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS > > hadowInfoHob.h > > b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS > > hadowInfoHob.h > > new file mode 100644 > > index 0000000000..59a38cee74 > > --- /dev/null > > +++ > > b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS > > hadowInfoHob. > > +++ h > > @@ -0,0 +1,57 @@ > > +/** @file > > + The definition for VTD PMR Regions Information Hob. > > This description does not match the content > > > + > > + Copyright (c) 2019, Intel Corporation. All rights > > reserved.<BR> > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > + > > + > > +#ifndef _MICROCODE_SHADOW_INFO_HOB_H_ > > +#define _MICROCODE_SHADOW_INFO_HOB_H_ > > + > > +/// > > +/// The Global ID of a GUIDed HOB used to pass > > microcode shadow info to DXE Driver. > > +/// > > +#define EDKII_MICROCODE_SHADOW_INFO_HOB_GUID \ > > + { \ > > + 0x658903f9, 0xda66, 0x460d, { 0x8b, 0xb0, 0x9d, > > 0x2d, 0xdf, 0x65, > > +0x44, 0x59 } \ > > + } > > + > > +extern EFI_GUID gEdkiiMicrocodeShadowInfoHobGuid; > > + > > +typedef struct { > > + // > > + // Number of the microcode patches which have been > > + // relocated to memory. > > + // > > + UINT64 MicrocodeCount; > > + // > > + // An EFI_GUID that defines the contents of > > StorageContext. > > + // > > + GUID StorageType; > > + // > > + // An array with MicrocodeCount elements that stores > > + // the shadowed microcode patch address in memory. > > + // > > + UINT64 MicrocodeAddrInMemory[0]; > > Since this is the last field in the structure visible to the > C compiler, you can use [] instead of [0] so it is interpreted > by the compiler as a flexible array member. This can make > code that uses this structure easier to read. > > https://en.wikipedia.org/wiki/Flexible_array_member > > > > + // > > + // A buffer which contains details about the storage > > information > > + // specific to StorageType. > > + // > > + // UINT8 StorageContext[]; > > +} EDKII_MICROCODE_SHADOW_INFO_HOB; > > + > > +// > > +// An EDKII_MICROCODE_SHADOW_INFO_HOB with StorageType > > set to below > > +GUID will // have the StorageContext of an array with > > MicrocodeCount of > > +UINT64 elements // that stores the original microcode > > patch address on > > +flash. This address is // placed in same order as the > > microcode patches in MicrocodeAddrInMemory. > > +// > > +#define EFI_MICROCODE_STORAGE_TPYE_FLASH_GUID \ > > Typo. "TPYE" should be "TYPE". > > > Should a data structure be added that is associated with > EFI_MICROCODE_STORAGE_TYPE_FLASH_GUID so it can be used > to interpret StorageContext field when StorageType matches > this GUID value? This way, the text in the comments is > not required to know the layout of StorageContext. > > typedef struct { > UINT64 MicrocodeAddressInFlash[]; > } EFI_MICROCODE_STORAGE_TYPE_FLASH_CONTEXT; > > In order to make everything aligned better should the > StorageGuid be listed before MicrocodeCount, so the order > is from largest to smallest. This also guarantees that > StorageContext is aligned on an 8-byte boundary which is > compatible with a typecast to a StorageType specific structure. > > > + { \ > > + 0x2cba01b3, 0xd391, 0x4598, { 0x8d, 0x89, 0xb7, > > 0xfc, 0x39, 0x22, > > +0xfd, 0x71 } \ > > + } > > + > > +extern EFI_GUID gEdkiiMicrocodeStorageTypeFlashGuid; > > + > > +#endif > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > > b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > > index 22ebf19c4e..2d8e40f0b8 100644 > > --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec > > +++ b/Silicon/Intel/IntelSiliconPkg/ IntelSiliconPkg.dec > > @@ -48,6 +48,12 @@ > > Header of IntelSiliconPkg.dec needs to have copyright updated > to 2020. > > > ## HOB GUID to get memory information after MRC is > > done. The hob data will be used to set the PMR ranges > > gVtdPmrInfoDataHobGuid = {0x6fb61645, 0xf168, > > 0x46be, { 0x80, 0xec, 0xb5, 0x02, 0x38, 0x5e, 0xe7, > > 0xe7 } } > > > > + ## Include/Guid/MicrocodeShadowInfoHob.h > > + gEdkiiMicrocodeShadowInfoHobGuid = { 0x658903f9, > > 0xda66, 0x460d, { > > + 0x8b, 0xb0, 0x9d, 0x2d, 0xdf, 0x65, 0x44, 0x59 } } > > + > > + ## Include/Guid/MicrocodeShadowInfoHob.h > > + gEdkiiMicrocodeStorageTypeFlashGuid = { 0x2cba01b3, > > 0xd391, 0x4598, { > > + 0x8d, 0x89, 0xb7, 0xfc, 0x39, 0x22, 0xfd, 0x71 } } > > + > > [Ppis] > > gEdkiiVTdInfoPpiGuid = { 0x8a59fcb3, 0xf191, 0x400c, > > { 0x97, 0x67, 0x67, 0xaf, 0x2b, 0x25, 0x68, 0x4a } } > > > > diff --git > > a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc > > b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc > > index 0a6509d8b3..f995883691 100644 > > --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc > > +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dsc > > @@ -1,7 +1,7 @@ > > ## @file > > # This package provides common open source Intel > > silicon modules. > > # > > -# Copyright (c) 2017 - 2019, Intel Corporation. All > > rights reserved.<BR> > > +# Copyright (c) 2017 - 2020, Intel Corporation. All > > rights > > +reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -84,6 +84,7 @@ > > > > IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/Pl > > atformVTdInfoSamplePei.inf > > > > IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/Micr > > ocodeUpdateDxe.inf > > > > IntelSiliconPkg/Feature/Capsule/Library/MicrocodeFlashA > > ccessLibNull/MicrocodeFlashAccessLibNull.inf > > + > > IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocode > > Pei.inf > > > > IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwa > > reBootMediaLib.inf > > > > IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFir > > mwareBootMediaLib.inf > > > > -- > > 2.19.1.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54393): https://edk2.groups.io/g/devel/message/54393 Mute This Topic: https://groups.io/mt/71200783/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
