.../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/ShadowMicrocodePei.c create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.h create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.inf create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
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=2449
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Signed-off-by: Siyuan Fu <siyuan.fu@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/ShadowMicrocodePei.c
create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.h
create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.inf
create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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/ShadowMicrocodePei.c
@@ -0,0 +1,387 @@
+/** @file
+ Source code file for Platform Init PEI module
+
+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;
+ 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)) {
+ //
+ // 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 (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/ShadowMicrocodePei.h
@@ -0,0 +1,62 @@
+/** @file
+ Source code file for Platform Init PEI module
+
+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/ShadowMicrocodePei.inf
@@ -0,0 +1,44 @@
+### @file
+# Component information file for the Platform Init PEI module.
+#
+# 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/MicrocodeShadowInfoHob.h b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
new file mode 100644
index 0000000000..59a38cee74
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
@@ -0,0 +1,57 @@
+/** @file
+ The definition for VTD PMR Regions Information Hob.
+
+ 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];
+ //
+ // 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 \
+ { \
+ 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 @@
## 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/PlatformVTdInfoSamplePei.inf
IntelSiliconPkg/Feature/Capsule/MicrocodeUpdateDxe/MicrocodeUpdateDxe.inf
IntelSiliconPkg/Feature/Capsule/Library/MicrocodeFlashAccessLibNull/MicrocodeFlashAccessLibNull.inf
+ IntelSiliconPkg/Feature/ShadowMicrocode/ShadowMicrocodePei.inf
IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/PeiFirmwareBootMediaLib.inf
IntelSiliconPkg/Library/PeiDxeSmmBootMediaLib/DxeSmmFirmwareBootMediaLib.inf
--
2.19.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54236): https://edk2.groups.io/g/devel/message/54236
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@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 (#54316): https://edk2.groups.io/g/devel/message/54316
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Mike
See my reply for the ASSERT and magic number around FIT table parsing code.
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: 2020年2月13日 8:58
> To: devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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\MicrocodeUpdateDxe\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 (#54402): https://edk2.groups.io/g/devel/message/54402
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.
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.
Mike
> -----Original Message-----
> From: Fu, Siyuan <siyuan.fu@intel.com>
> Sent: Thursday, February 13, 2020 5:33 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.kinney@intel.com>
> > Sent: 2020年2月13日 8:58
> > To: devel@edk2.groups.io; Fu, Siyuan
> <siyuan.fu@intel.com>; Kinney, Michael
> > D <michael.d.kinney@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.ni@intel.com>; Chaganty, Rangasai
> V
> > > <rangasai.v.chaganty@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 (#54482): https://edk2.groups.io/g/devel/message/54482
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.kinney@intel.com>
> Sent: 2020年2月15日 7:07
> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.fu@intel.com>
> > Sent: Thursday, February 13, 2020 5:33 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.kinney@intel.com>
> > > Sent: 2020年2月13日 8:58
> > > To: devel@edk2.groups.io; Fu, Siyuan
> > <siyuan.fu@intel.com>; Kinney, Michael
> > > D <michael.d.kinney@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > > <rangasai.v.chaganty@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.ni@intel.com>; Chaganty, Rangasai
> > V
> > > > <rangasai.v.chaganty@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 (#54516): https://edk2.groups.io/g/devel/message/54516
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.fu@intel.com>
> Sent: Saturday, February 15, 2020 7:36 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.kinney@intel.com>
> > Sent: 2020年2月15日 7:07
> > To: Fu, Siyuan <siyuan.fu@intel.com>;
> devel@edk2.groups.io; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.fu@intel.com>
> > > Sent: Thursday, February 13, 2020 5:33 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> V
> > > <rangasai.v.chaganty@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.kinney@intel.com>
> > > > Sent: 2020年2月13日 8:58
> > > > To: devel@edk2.groups.io; Fu, Siyuan
> > > <siyuan.fu@intel.com>; Kinney, Michael
> > > > D <michael.d.kinney@intel.com>
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V
> > > > <rangasai.v.chaganty@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.ni@intel.com>; Chaganty,
> Rangasai
> > > V
> > > > > <rangasai.v.chaganty@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 (#54519): https://edk2.groups.io/g/devel/message/54519
Mute This Topic: https://groups.io/mt/71200783/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.kinney@intel.com>
> Sent: 2020年2月17日 4:35
> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.fu@intel.com>
> > Sent: Saturday, February 15, 2020 7:36 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.kinney@intel.com>
> > > Sent: 2020年2月15日 7:07
> > > To: Fu, Siyuan <siyuan.fu@intel.com>;
> > devel@edk2.groups.io; Kinney,
> > > Michael D <michael.d.kinney@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > > <rangasai.v.chaganty@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.fu@intel.com>
> > > > Sent: Thursday, February 13, 2020 5:33 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > devel@edk2.groups.io
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> > V
> > > > <rangasai.v.chaganty@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.kinney@intel.com>
> > > > > Sent: 2020年2月13日 8:58
> > > > > To: devel@edk2.groups.io; Fu, Siyuan
> > > > <siyuan.fu@intel.com>; Kinney, Michael
> > > > > D <michael.d.kinney@intel.com>
> > > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
> > Rangasai V
> > > > > <rangasai.v.chaganty@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.ni@intel.com>; Chaganty,
> > Rangasai
> > > > V
> > > > > > <rangasai.v.chaganty@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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.kinney@intel.com>
> Sent: 2020年2月13日 8:58
> To: devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.ni@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Signed-off-by: Siyuan Fu <siyuan.fu@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.