Hi, Mike I have sent V5 patch which removes the ASSERT and check the FIT pointer against valid firmware address range (4G-16MB to 4G-0x40). Please help to review it. Thanks.
Best Regards Siyuan > -----Original Message----- > From: Kinney, Michael D <michael.d.kin...@intel.com> > Sent: 2020年2月17日 4:35 > To: Fu, Siyuan <siyuan...@intel.com>; devel@edk2.groups.io; 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. > > Then the checks against 0xFFFFFFFFFFFFFFFF and 0xEEEEEEEEEEEEEEEE > should be removed. > > If those checks are important, then replace with checks against > max physical address. Or if it is guaranteed to be below 4GB, > then check against that value with a clear comment for the > checks being performed. > > Mike > > > -----Original Message----- > > From: Fu, Siyuan <siyuan...@intel.com> > > Sent: Saturday, February 15, 2020 7:36 PM > > To: Kinney, Michael D <michael.d.kin...@intel.com>; > > devel@edk2.groups.io > > 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. > > > > Hi, Mike > > > > The FITPointer points to the FIT table address on flash > > (within top 16MB > > of 4GB). It's not a memory address and normally it's > > always large than > > the MemoryTop address in PHIT. So we couldn't use PHIT > > HOB to check > > the FIT pointer. > > > > > > > > Best Regards > > Siyuan > > > > > -----Original Message----- > > > From: Kinney, Michael D <michael.d.kin...@intel.com> > > > Sent: 2020年2月15日 7:07 > > > To: Fu, Siyuan <siyuan...@intel.com>; > > devel@edk2.groups.io; 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, > > > > > > If the FIT is not valid, then the API should just > > return > > > an error without ASSERT(). Not all system support > > FIT or > > > fill in FIT. The code is more generic if it just > > does > > > the check and returns an error. > > > > This is an optional PEIM and only these systems which > > support FIT should use it. Other platforms should not > > include this PEIM so MpInitLib will still use the PCD > > based microcode shadow as usual. > > But it's ok to me if you think it's necessary to remove > > these ASSERT so any platform can include this PEIM > > without additional concern. I will update patch for > > this. > > > > > > > > The check looks incomplete to me. We know that max > > physical > > > address of the CPU from the PHIT HOB. If the > > physical address > > > value is greater than the max physical address, then > > the > > > pointer is invalid. This would cover the 2 point > > cases of > > > all Fs and all Es. > > > > The FITPointer points to the FIT table address on flash > > (within top > > 16MB of 4GB). It's not a memory address and normally > > it's always > > larger than the MemoryTop address in PHIT. So we > > couldn't use > > PHIT HOB to check the FIT pointer. > > > > Best Regards, > > Siyuan > > > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Fu, Siyuan <siyuan...@intel.com> > > > > Sent: Thursday, February 13, 2020 5:33 PM > > > > To: Kinney, Michael D <michael.d.kin...@intel.com>; > > > > devel@edk2.groups.io > > > > 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. > > > > > > > > Hi Mike > > > > > > > > See my reply for the ASSERT and magic number around > > FIT > > > > table parsing code. > > > > > > > > > -----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 > > > > Trim long patch content. > > > > > > > > > > + > > > > > > +**/ > > > > > > +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. > > > > > > > > These values are not defined in FIT include file or > > FIT > > > > specification. > > > > The only way to identify if FIT table is exist in > > FIT > > > > spec is the _FIT_ > > > > signature, which defined in FIT header file as > > > > FIT_TYPE_00_SIGNATURE and check below. > > > > > > > > This if check is copied from the > > > > InitializeFitMicrocodeInfo() function > > > > in > > > > > > Silicon\Intel\IntelSiliconPkg\Feature\Capsule\Microcode > > > > UpdateDxe\MicrocodeFmp.c. > > > > I think it just assumes the default value of flash > > > > content is 0xFF > > > > or 0xEE and check that. > > > > > > > > This is also why I use ASSERT if the flash content > > > > doesn't seems > > > > like a valid FIT table in below if checks. FIT boot > > is > > > > critical to > > > > processor microcode load and BIOS RTU setup. And > > > > including > > > > this PEIM into the platform means the platform > > owner > > > > want to > > > > use FIT based boot and microcode loading. These > > ASSERTs > > > > would > > > > be helpful to let them if the FIT table content is > > > > invalid in a DEBUG > > > > version BIOS image. > > > > > > > > > > > > > > > + // > > > > > > + // 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 (#54553): https://edk2.groups.io/g/devel/message/54553 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] -=-=-=-=-=-=-=-=-=-=-=-
