[Patch 0/2] Move FIT based microcode shadow to platform PEIM.

[Patch 0/2] Move FIT based microcode shadow to platform PEIM.

[Siyuan, Fu]

Commit c7c964b and dd01704 add header file for FIT table and update
MpInitLib to support FIT based microcode shadow operation. There are
comments that FIT is Intel specific specification instead of industry
standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
So this patch adds a platform PPI for the microcode shadow logic, and
remove the FIT related code from EDK2.
The FIT based microcode shadow support will be implemented as a new
platform PEIM in IntelSiliconPkg in edk2-platforms.

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

[Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from
 MpInitLib.
[Patch 2/2] MdePkg: Remove FIT table industry standard header file.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>

Siyuan Fu (2):
  UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.
  MdePkg: Remove FIT table industry standard header file.

 .../IndustryStandard/FirmwareInterfaceTable.h |  76 -------------
 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h      |  66 +++++++++++
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 -
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +++-
 UefiCpuPkg/Library/MpInitLib/Microcode.c      | 105 +-----------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  19 +++-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  70 +++++++++++-
 UefiCpuPkg/UefiCpuPkg.dec                     |  11 +-
 9 files changed, 179 insertions(+), 194 deletions(-)
 delete mode 100644 MdePkg/Include/IndustryStandard/FirmwareInterfaceTable.h
 create mode 100644 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h

-- 
2.19.1.windows.1

[Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

[Siyuan, Fu]

Commit c7c964b and dd01704 add header file for FIT table and update
MpInitLib to support FIT based microcode shadow operation. There are
comments that FIT is Intel specific specification instead of industry
standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
So this patch adds a platform PPI for the microcode shadow logic, and
remove the FIT related code from EDK2.
The FIT based microcode shadow support will be implemented as a new
platform PEIM in IntelSiliconPkg in edk2-platforms.

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: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
---
 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h      |  66 +++++++++++
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 -
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +++-
 UefiCpuPkg/Library/MpInitLib/Microcode.c      | 105 +-----------------
 UefiCpuPkg/Library/MpInitLib/MpLib.h          |  19 +++-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  70 +++++++++++-
 UefiCpuPkg/UefiCpuPkg.dec                     |  11 +-
 8 files changed, 179 insertions(+), 118 deletions(-)
 create mode 100644 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h

diff --git a/UefiCpuPkg/Include/Ppi/ShadowMicrocode.h b/UefiCpuPkg/Include/Ppi/ShadowMicrocode.h
new file mode 100644
index 0000000000..17c19d6307
--- /dev/null
+++ b/UefiCpuPkg/Include/Ppi/ShadowMicrocode.h
@@ -0,0 +1,66 @@
+/** @file
+  This file declares EDKII Shadow Microcode PPI.
+
+  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __PPI_SHADOW_MICROCODE_H__
+#define __PPI_SHADOW_MICROCODE_H__
+
+#define EDKII_PEI_SHADOW_MICROCODE_PPI_GUID \
+  { \
+    0x430f6965, 0x9a69, 0x41c5, { 0x93, 0xed, 0x8b, 0xf0, 0x64, 0x35, 0xc1, 0xc6 } \
+  }
+
+typedef struct _EDKII_PEI_SHADOW_MICROCODE_PPI  EDKII_PEI_SHADOW_MICROCODE_PPI;
+
+typedef struct {
+  UINT32         ProcessorSignature;
+  UINT8          PlatformId;
+} EDKII_PEI_CPU_MICROCODE_ID;
+
+/**
+  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_CPU_MICROCODE_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.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_SHADOW_MICROCODE) (
+  IN  EDKII_PEI_SHADOW_MICROCODE_PPI        *This,
+  IN  UINTN                                 CpuIdCount,
+  IN  EDKII_PEI_CPU_MICROCODE_ID            *MicrocodeCpuId,
+  OUT UINTN                                 *BufferSize,
+  OUT VOID                                  **Buffer
+  );
+
+///
+/// This PPI is installed by some platform or chipset-specific PEIM that
+/// abstracts handling microcode shadow support.
+///
+struct _EDKII_PEI_SHADOW_MICROCODE_PPI {
+  EDKII_PEI_SHADOW_MICROCODE          ShadowMicrocode;
+};
+
+extern EFI_GUID gEdkiiPeiShadowMicrocodePpiGuid;
+
+#endif
+
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index bf5d18d521..cd912ab0c5 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -68,6 +68,5 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize         ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode                       ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                   ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit             ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard                  ## CONSUMES
 
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index b17e287bbf..ca52be943e 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -1,7 +1,7 @@
 /** @file
   MP initialize support functions for DXE phase.
 
-  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -816,3 +816,22 @@ MpInitLibEnableDisableAP (
 
   return Status;
 }
+
+/**
+  This funtion will try to invoke platform specific microcode shadow logic to
+  relocate microcode update patches into memory.
+
+  @param[in] CpuMpData  The pointer to CPU MP Data structure.
+
+  @retval EFI_SUCCESS              Shadow microcode success.
+  @retval EFI_OUT_OF_RESOURCES     No enough resource to complete the operation.
+  @retval EFI_NOT_FOUND            Can't find platform specific microcode shadow
+                                   PPI/Protocol.
+**/
+EFI_STATUS
+PlatformShadowMicrocode (
+  IN OUT CPU_MP_DATA             *CpuMpData
+  )
+{
+  return EFI_NOT_FOUND;
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 247f835b09..0c468cd480 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -619,109 +619,6 @@ OnExit:
   return;
 }
 
-/**
-  Shadow the required microcode patches data into memory according to FIT microcode entry.
-
-  @param[in, out]  CpuMpData    The pointer to CPU MP Data structure.
-
-  @return EFI_SUCCESS           Microcode patch is shadowed into memory.
-  @return EFI_UNSUPPORTED       FIT based microcode shadowing is not supported.
-  @return EFI_OUT_OF_RESOURCES  No enough memory resource.
-  @return EFI_NOT_FOUND         There is something wrong in FIT microcode entry.
-
-**/
-EFI_STATUS
-ShadowMicrocodePatchByFit (
-  IN OUT CPU_MP_DATA             *CpuMpData
-  )
-{
-  UINT64                            FitPointer;
-  FIRMWARE_INTERFACE_TABLE_ENTRY    *FitEntry;
-  UINT32                            EntryNum;
-  UINT32                            Index;
-  MICROCODE_PATCH_INFO              *PatchInfoBuffer;
-  UINTN                             MaxPatchNumber;
-  CPU_MICROCODE_HEADER              *MicrocodeEntryPoint;
-  UINTN                             PatchCount;
-  UINTN                             TotalSize;
-  UINTN                             TotalLoadSize;
-
-  if (!FeaturePcdGet (PcdCpuShadowMicrocodeByFit)) {
-    return EFI_UNSUPPORTED;
-  }
-
-  FitPointer = *(UINT64 *) (UINTN) FIT_POINTER_ADDRESS;
-  if ((FitPointer == 0) ||
-      (FitPointer == 0xFFFFFFFFFFFFFFFF) ||
-      (FitPointer == 0xEEEEEEEEEEEEEEEE)) {
-    //
-    // No FIT table.
-    //
-    ASSERT (FALSE);
-    return EFI_NOT_FOUND;
-  }
-  FitEntry = (FIRMWARE_INTERFACE_TABLE_ENTRY *) (UINTN) FitPointer;
-  if ((FitEntry[0].Type != FIT_TYPE_00_HEADER) ||
-      (FitEntry[0].Address != FIT_TYPE_00_SIGNATURE)) {
-    //
-    // Invalid FIT table, treat it as no FIT table.
-    //
-    ASSERT (FALSE);
-    return EFI_NOT_FOUND;
-  }
-
-  EntryNum = *(UINT32 *)(&FitEntry[0].Size[0]) & 0xFFFFFF;
-
-  //
-  // Calculate microcode entry number
-  //
-  MaxPatchNumber = 0;
-  for (Index = 0; Index < EntryNum; Index++) {
-    if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
-      MaxPatchNumber++;
-    }
-  }
-  if (MaxPatchNumber == 0) {
-    return EFI_NOT_FOUND;
-  }
-
-  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
-  if (PatchInfoBuffer == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  //
-  // Fill up microcode patch info buffer according to FIT table.
-  //
-  PatchCount = 0;
-  TotalLoadSize = 0;
-  for (Index = 0; Index < EntryNum; Index++) {
-    if (FitEntry[Index].Type == FIT_TYPE_01_MICROCODE) {
-      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (UINTN) FitEntry[Index].Address;
-      TotalSize = (MicrocodeEntryPoint->DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
-      if (IsMicrocodePatchNeedLoad (CpuMpData, MicrocodeEntryPoint)) {
-        PatchInfoBuffer[PatchCount].Address     = (UINTN) MicrocodeEntryPoint;
-        PatchInfoBuffer[PatchCount].Size        = TotalSize;
-        TotalLoadSize += TotalSize;
-        PatchCount++;
-      }
-    }
-  }
-
-  if (PatchCount != 0) {
-    DEBUG ((
-      DEBUG_INFO,
-      "%a: 0x%x microcode patches will be loaded into memory, with size 0x%x.\n",
-      __FUNCTION__, PatchCount, TotalLoadSize
-      ));
-
-    ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
-  }
-
-  FreePool (PatchInfoBuffer);
-  return EFI_SUCCESS;
-}
-
 /**
   Shadow the required microcode patches data into memory.
 
@@ -734,7 +631,7 @@ ShadowMicrocodeUpdatePatch (
 {
   EFI_STATUS     Status;
 
-  Status = ShadowMicrocodePatchByFit (CpuMpData);
+  Status = PlatformShadowMicrocode (CpuMpData);
   if (EFI_ERROR (Status)) {
     ShadowMicrocodePatchByPcd (CpuMpData);
   }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 7c62d75acc..7d505e5621 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -29,9 +29,6 @@
 #include <Library/MtrrLib.h>
 #include <Library/HobLib.h>
 
-#include <IndustryStandard/FirmwareInterfaceTable.h>
-
-
 #define WAKEUP_AP_SIGNAL SIGNATURE_32 ('S', 'T', 'A', 'P')
 
 #define CPU_INIT_MP_LIB_HOB_GUID \
@@ -634,5 +631,21 @@ GetProcessorNumber (
   OUT UINTN                    *ProcessorNumber
   );
 
+/**
+  This funtion will try to invoke platform specific microcode shadow logic to
+  relocate microcode update patches into memory.
+
+  @param[in] CpuMpData  The pointer to CPU MP Data structure.
+
+  @retval EFI_SUCCESS              Shadow microcode success.
+  @retval EFI_OUT_OF_RESOURCES     No enough resource to complete the operation.
+  @retval EFI_NOT_FOUND            Can't find platform specific microcode shadow
+                                   PPI/Protocol.
+**/
+EFI_STATUS
+PlatformShadowMicrocode (
+  IN OUT CPU_MP_DATA             *CpuMpData
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 555125a7c5..d78d328b42 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -60,7 +60,9 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize         ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode                       ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate                   ## SOMETIMES_CONSUMES
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit             ## CONSUMES
+
+[Ppis]
+  gEdkiiPeiShadowMicrocodePpiGuid        ## SOMETIMES_CONSUMES
 
 [Guids]
   gEdkiiS3SmmInitDoneGuid
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 06e3f5d0d3..d25cdcc879 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -1,7 +1,7 @@
 /** @file
   MP initialize support functions for PEI phase.
 
-  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -10,6 +10,7 @@
 #include <Library/PeiServicesLib.h>
 #include <Guid/S3SmmInitDone.h>
 #include <Guid/MicrocodePatchHob.h>
+#include <Ppi/ShadowMicrocode.h>
 
 /**
   S3 SMM Init Done notification function.
@@ -640,4 +641,71 @@ MpInitLibEnableDisableAP (
   return EnableDisableApWorker (ProcessorNumber, EnableAP, HealthFlag);
 }
 
+/**
+  This funtion will try to invoke platform specific microcode shadow logic to
+  relocate microcode update patches into memory.
+
+  @param[in] CpuMpData  The pointer to CPU MP Data structure.
+
+  @retval EFI_SUCCESS              Shadow microcode success.
+  @retval EFI_OUT_OF_RESOURCES     No enough resource to complete the operation.
+  @retval EFI_NOT_FOUND            Can't find platform specific microcode shadow
+                                   PPI/Protocol.
+**/
+EFI_STATUS
+PlatformShadowMicrocode (
+  IN OUT CPU_MP_DATA             *CpuMpData
+  )
+{
+  EFI_STATUS                         Status;
+  EDKII_PEI_SHADOW_MICROCODE_PPI     *ShadowMicrocodePpi;
+  UINTN                              CpuCount;
+  EDKII_PEI_CPU_MICROCODE_ID         *MicrocodeCpuId;
+  UINTN                              Index;
+  UINTN                              BufferSize;
+  VOID                               *Buffer;
+
+  Status = PeiServicesLocatePpi (
+             &gEdkiiPeiShadowMicrocodePpiGuid,
+             0,
+             NULL,
+             (VOID **) &ShadowMicrocodePpi
+             );
+  if (EFI_ERROR (Status)) {
+    return EFI_NOT_FOUND;
+  }
+
+  CpuCount = CpuMpData->CpuCount;
+  MicrocodeCpuId = (EDKII_PEI_CPU_MICROCODE_ID *) AllocateZeroPool (sizeof (EDKII_PEI_CPU_MICROCODE_ID) * CpuCount);
+  if (MicrocodeCpuId == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+    MicrocodeCpuId[Index].ProcessorSignature = CpuMpData->CpuData[Index].ProcessorSignature;
+    MicrocodeCpuId[Index].PlatformId         = CpuMpData->CpuData[Index].PlatformId;
+  }
+
+  Status = ShadowMicrocodePpi->ShadowMicrocode (
+             ShadowMicrocodePpi,
+             CpuCount,
+             MicrocodeCpuId,
+             &BufferSize,
+             &Buffer
+             );
+  FreePool (MicrocodeCpuId);
+  if (EFI_ERROR (Status)) {
+    return EFI_NOT_FOUND;
+  }
+
+  CpuMpData->MicrocodePatchAddress    = (UINTN) Buffer;
+  CpuMpData->MicrocodePatchRegionSize = BufferSize;
+
+  DEBUG ((
+    DEBUG_INFO,
+    "%a: Required microcode patches have been loaded at 0x%lx, with size 0x%lx.\n",
+    __FUNCTION__, CpuMpData->MicrocodePatchAddress, CpuMpData->MicrocodePatchRegionSize
+    ));
 
+  return EFI_SUCCESS;
+}
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index a6ebdde1cf..e91dc68cbe 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -1,7 +1,7 @@
 ## @file  UefiCpuPkg.dec
 # This Package provides UEFI compatible CPU modules and libraries.
 #
-# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -81,6 +81,9 @@
 [Ppis]
   gEdkiiPeiMpServices2PpiGuid =    { 0x5cb9cb3d, 0x31a4, 0x480c, { 0x94, 0x98, 0x29, 0xd2, 0x69, 0xba, 0xcf, 0xba}}
 
+  ## Include/Ppi/ShadowMicrocode.h
+  gEdkiiPeiShadowMicrocodePpiGuid = { 0x430f6965, 0x9a69, 0x41c5, { 0x93, 0xed, 0x8b, 0xf0, 0x64, 0x35, 0xc1, 0xc6 }}
+
 [PcdsFeatureFlag]
   ## Indicates if SMM Profile will be enabled.
   #  If enabled, instruction executions in and data accesses to memory outside of SMRAM will be logged.
@@ -139,12 +142,6 @@
   # @Prompt Lock SMM Feature Control MSR.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock|TRUE|BOOLEAN|0x3213210B
 
-  ## Indicates if FIT based microcode shadowing will be enabled.<BR><BR>
-  #   TRUE  - FIT base microcode shadowing will be enabled.<BR>
-  #   FALSE - FIT base microcode shadowing will be disabled.<BR>
-  # @Prompt FIT based microcode shadowing.
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuShadowMicrocodeByFit|FALSE|BOOLEAN|0x3213210D
-
 [PcdsFixedAtBuild]
   ## List of exception vectors which need switching stack.
   #  This PCD will only take into effect if PcdCpuStackGuard is enabled.
-- 
2.19.1.windows.1

[Patch 2/2] MdePkg: Remove FIT table industry standard header file.

[Siyuan, Fu]

Commit c7c964b and dd01704 add header file for FIT table and update
MpInitLib to support FIT based microcode shadow operation. There are
comments that FIT is Intel specific specification instead of industry
standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
This patch removes the header file added by c7c964b.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
---
 .../IndustryStandard/FirmwareInterfaceTable.h | 76 -------------------
 1 file changed, 76 deletions(-)
 delete mode 100644 MdePkg/Include/IndustryStandard/FirmwareInterfaceTable.h

diff --git a/MdePkg/Include/IndustryStandard/FirmwareInterfaceTable.h b/MdePkg/Include/IndustryStandard/FirmwareInterfaceTable.h
deleted file mode 100644
index be3e34ae1b..0000000000
--- a/MdePkg/Include/IndustryStandard/FirmwareInterfaceTable.h
+++ /dev/null
@@ -1,76 +0,0 @@
-/** @file
-  Industry Standard Definitions of Firmware Interface Table BIOS Specification 1.0.
-
-  Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef __FIRMWARE_INTERFACE_TABLE_H__
-#define __FIRMWARE_INTERFACE_TABLE_H__
-
-//
-// FIT Entry type definitions
-//
-#define FIT_TYPE_00_HEADER                  0x00
-#define FIT_TYPE_01_MICROCODE               0x01
-#define FIT_TYPE_02_STARTUP_ACM             0x02
-#define FIT_TYPE_07_BIOS_STARTUP_MODULE     0x07
-#define FIT_TYPE_08_TPM_POLICY              0x08
-#define FIT_TYPE_09_BIOS_POLICY             0x09
-#define FIT_TYPE_0A_TXT_POLICY              0x0A
-#define FIT_TYPE_0B_KEY_MANIFEST            0x0B
-#define FIT_TYPE_0C_BOOT_POLICY_MANIFEST    0x0C
-#define FIT_TYPE_10_CSE_SECURE_BOOT         0x10
-#define FIT_TYPE_2D_TXTSX_POLICY            0x2D
-#define FIT_TYPE_2F_JMP_DEBUG_POLICY        0x2F
-#define FIT_TYPE_7F_SKIP                    0x7F
-
-#define FIT_POINTER_ADDRESS                 0xFFFFFFC0 ///< Fixed address at 4G - 40h
-
-#define FIT_TYPE_VERSION                    0x0100
-
-#define FIT_TYPE_00_SIGNATURE  SIGNATURE_64 ('_', 'F', 'I', 'T', '_', ' ', ' ', ' ')
-
-#pragma pack(1)
-
-typedef struct {
-  //
-  // Address is the base address of the firmware component
-  // must be aligned on 16 byte boundary
-  //
-  UINT64 Address;
-  //
-  // Size is the span of the component in multiple of 16 bytes
-  //
-  UINT8  Size[3];
-  //
-  // Reserved must be set to 0
-  //
-  UINT8  Reserved;
-  //
-  // Component's version number in binary coded decimal (BCD) format.
-  // For the FIT header entry, the value in this field will indicate the revision
-  // number of the FIT data structure. The upper byte of the revision field
-  // indicates the major revision and the lower byte indicates the minor revision.
-  //
-  UINT16 Version;
-  //
-  // FIT types 0x00 to 0x7F
-  //
-  UINT8  Type : 7;
-  //
-  // Checksum Valid indicates whether component has valid checksum.
-  //
-  UINT8  C_V  : 1;
-  //
-  // Component's checksum. The modulo sum of all the bytes in the component and
-  // the value in this field (Chksum) must add up to zero. This field is only
-  // valid if the C_V flag is non-zero.
-  //
-  UINT8  Chksum;
-} FIRMWARE_INTERFACE_TABLE_ENTRY;
-
-#pragma pack()
-
-#endif
-- 
2.19.1.windows.1

Re: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

[Laszlo Ersek]

On 02/10/20 11:58, Siyuan Fu wrote:
> Commit c7c964b and dd01704 add header file for FIT table and update
> MpInitLib to support FIT based microcode shadow operation. There are
> comments that FIT is Intel specific specification instead of industry
> standard, which should not be placed in EDK2 MdePkg and UefiCpuPkg.
> So this patch adds a platform PPI for the microcode shadow logic, and
> remove the FIT related code from EDK2.
> The FIT based microcode shadow support will be implemented as a new
> platform PEIM in IntelSiliconPkg in edk2-platforms.
> 
> 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: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  UefiCpuPkg/Include/Ppi/ShadowMicrocode.h      |  66 +++++++++++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 -
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c       |  21 +++-
>  UefiCpuPkg/Library/MpInitLib/Microcode.c      | 105 +-----------------
>  UefiCpuPkg/Library/MpInitLib/MpLib.h          |  19 +++-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c       |  70 +++++++++++-
>  UefiCpuPkg/UefiCpuPkg.dec                     |  11 +-
>  8 files changed, 179 insertions(+), 118 deletions(-)
>  create mode 100644 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h

(1) Please don't forget to update "UefiCpuPkg/UefiCpuPkg.uni" as well.

With that addressed:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

[Ni, Ray]

> +typedef
> +EFI_STATUS
> +(EFIAPI *EDKII_PEI_SHADOW_MICROCODE) (
> +  IN  EDKII_PEI_SHADOW_MICROCODE_PPI        *This,
> +  IN  UINTN                                 CpuIdCount,
> +  IN  EDKII_PEI_CPU_MICROCODE_ID            *MicrocodeCpuId,

1. How about CpuMicrocodeId or EDKII_PEI_MICROCODE_CPU_ID?
I'd like to keep the name and type be matched.


> +  OUT UINTN                                 *BufferSize,
> +  OUT VOID                                  **Buffer
2. I remember that we offline discussed that Buffer/BufferSize are not needed
to be part of the parameters. It can provide better flexibility that doesn't require
the microcode in memory is in continuous memory.
Why are they still in the parameters?

OK. I see now. Because EDKII_MICROCODE_PATCH_HOB contains below fields:
typedef struct {
  UINT64    MicrocodePatchAddress;
  UINT64    MicrocodePatchRegionSize; 
  ...
} EDKII_MICROCODE_PATCH_HOB;
which already restricts that the microcode in memory is in continuous memory.

I'm ok with this.

> +EFI_STATUS
> +PlatformShadowMicrocode (
> +  IN OUT CPU_MP_DATA             *CpuMpData
> +  )
> +{
> +  return EFI_NOT_FOUND;

3. Can you add comments to say that microcode shadow
in DXE only supports the location identified by PCD?
4. How about returning EFI_UNSUPPORTED?

Re: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

[Ni, Ray]

By the way, please rebase to the latest code when sending out
the V2 patch.

> -----Original Message-----
> From: Ni, Ray
> Sent: Tuesday, February 11, 2020 7:21 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.
> 
> 
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_SHADOW_MICROCODE) (
> > +  IN  EDKII_PEI_SHADOW_MICROCODE_PPI        *This,
> > +  IN  UINTN                                 CpuIdCount,
> > +  IN  EDKII_PEI_CPU_MICROCODE_ID            *MicrocodeCpuId,
> 
> 1. How about CpuMicrocodeId or EDKII_PEI_MICROCODE_CPU_ID?
> I'd like to keep the name and type be matched.
> 
> 
> > +  OUT UINTN                                 *BufferSize,
> > +  OUT VOID                                  **Buffer
> 2. I remember that we offline discussed that Buffer/BufferSize are not needed
> to be part of the parameters. It can provide better flexibility that doesn't require
> the microcode in memory is in continuous memory.
> Why are they still in the parameters?
> 
> OK. I see now. Because EDKII_MICROCODE_PATCH_HOB contains below fields:
> typedef struct {
>   UINT64    MicrocodePatchAddress;
>   UINT64    MicrocodePatchRegionSize;
>   ...
> } EDKII_MICROCODE_PATCH_HOB;
> which already restricts that the microcode in memory is in continuous memory.
> 
> I'm ok with this.
> 
> > +EFI_STATUS
> > +PlatformShadowMicrocode (
> > +  IN OUT CPU_MP_DATA             *CpuMpData
> > +  )
> > +{
> > +  return EFI_NOT_FOUND;
> 
> 3. Can you add comments to say that microcode shadow
> in DXE only supports the location identified by PCD?
> 4. How about returning EFI_UNSUPPORTED?

Re: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

[Siyuan, Fu]

Thank you Ray and Laszlo for your comments, I have sent the V2 patch set for this change.

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: 2020年2月11日 19:24
> To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow
> logic from MpInitLib.
> 
> By the way, please rebase to the latest code when sending out
> the V2 patch.
> 
> > -----Original Message-----
> > From: Ni, Ray
> > Sent: Tuesday, February 11, 2020 7:21 PM
> > To: Fu, Siyuan <siyuan.fu@intel.com>; devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: RE: [Patch 1/2] UefiCpuPkg: Remove FIT based microcode shadow
> logic from MpInitLib.
> >
> >
> > > +typedef
> > > +EFI_STATUS
> > > +(EFIAPI *EDKII_PEI_SHADOW_MICROCODE) (
> > > +  IN  EDKII_PEI_SHADOW_MICROCODE_PPI        *This,
> > > +  IN  UINTN                                 CpuIdCount,
> > > +  IN  EDKII_PEI_CPU_MICROCODE_ID            *MicrocodeCpuId,
> >
> > 1. How about CpuMicrocodeId or EDKII_PEI_MICROCODE_CPU_ID?
> > I'd like to keep the name and type be matched.
> >
> >
> > > +  OUT UINTN                                 *BufferSize,
> > > +  OUT VOID                                  **Buffer
> > 2. I remember that we offline discussed that Buffer/BufferSize are not
> needed
> > to be part of the parameters. It can provide better flexibility that doesn't
> require
> > the microcode in memory is in continuous memory.
> > Why are they still in the parameters?
> >
> > OK. I see now. Because EDKII_MICROCODE_PATCH_HOB contains below
> fields:
> > typedef struct {
> >   UINT64    MicrocodePatchAddress;
> >   UINT64    MicrocodePatchRegionSize;
> >   ...
> > } EDKII_MICROCODE_PATCH_HOB;
> > which already restricts that the microcode in memory is in continuous
> memory.
> >
> > I'm ok with this.
> >
> > > +EFI_STATUS
> > > +PlatformShadowMicrocode (
> > > +  IN OUT CPU_MP_DATA             *CpuMpData
> > > +  )
> > > +{
> > > +  return EFI_NOT_FOUND;
> >
> > 3. Can you add comments to say that microcode shadow
> > in DXE only supports the location identified by PCD?
> > 4. How about returning EFI_UNSUPPORTED?