Re: [edk2-devel] [PATCH v3 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

2020-02-13 Thread Michael D Kinney
Reviewed-by: Michael D Kinney 

Mike

> -Original Message-
> From: devel@edk2.groups.io  On
> Behalf Of Siyuan, Fu
> Sent: Wednesday, February 12, 2020 5:57 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric ; Ni, Ray
> ; Laszlo Ersek 
> Subject: [edk2-devel] [PATCH v3 1/2] UefiCpuPkg: Remove
> FIT based microcode shadow logic from MpInitLib.
> 
> V2 Changes:
> Rename EDKII_PEI_CPU_MICROCODE_ID to
> EDKII_PEI_MICROCODE_CPU_ID.
> Return EFI_UNSUPPORTED instead of EFI_NOT_FOUND if no
> platform
> microcode shadow PPI/Protocol is found.
> Remove PcdCpuShadowMicrocodeByFit related tokens from
> UefiCpuPkg.uni
> V3 Changes
> Add comments to DXE version PlatformShadowMicrocode().
> 
> 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.
> This patch doesn't provide a DXE version shadow
> microcode protocol,
> a platform which only uses DxeMpInitLib instance only
> supports PCD
> based microcode shadowing.
> 
> 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: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Signed-off-by: Siyuan Fu 
> ---
>  UefiCpuPkg/Include/Ppi/ShadowMicrocode.h  |  66
> +++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 -
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   |  26
> -
>  UefiCpuPkg/Library/MpInitLib/Microcode.c  | 105 +-
> 
>  UefiCpuPkg/Library/MpInitLib/MpLib.h  |  19
> +++-
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +-
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   |  68
> 
>  UefiCpuPkg/UefiCpuPkg.dec |  11 +-
>  UefiCpuPkg/UefiCpuPkg.uni |   6 -
>  9 files changed, 183 insertions(+), 123 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 00..be48965422
> --- /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.
> +  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_MICROCODE_CPU_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_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.
> +
> +**/
> +typedef
&g

[edk2-devel] [PATCH v3 1/2] UefiCpuPkg: Remove FIT based microcode shadow logic from MpInitLib.

2020-02-12 Thread Siyuan, Fu
V2 Changes:
Rename EDKII_PEI_CPU_MICROCODE_ID to EDKII_PEI_MICROCODE_CPU_ID.
Return EFI_UNSUPPORTED instead of EFI_NOT_FOUND if no platform
microcode shadow PPI/Protocol is found.
Remove PcdCpuShadowMicrocodeByFit related tokens from UefiCpuPkg.uni
V3 Changes
Add comments to DXE version PlatformShadowMicrocode().

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.
This patch doesn't provide a DXE version shadow microcode protocol,
a platform which only uses DxeMpInitLib instance only supports PCD
based microcode shadowing.

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 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Signed-off-by: Siyuan Fu 
---
 UefiCpuPkg/Include/Ppi/ShadowMicrocode.h  |  66 +++
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 -
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   |  26 -
 UefiCpuPkg/Library/MpInitLib/Microcode.c  | 105 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h  |  19 +++-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   4 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   |  68 
 UefiCpuPkg/UefiCpuPkg.dec |  11 +-
 UefiCpuPkg/UefiCpuPkg.uni |   6 -
 9 files changed, 183 insertions(+), 123 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 00..be48965422
--- /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.
+  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_MICROCODE_CPU_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_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.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_SHADOW_MICROCODE) (
+  IN  EDKII_PEI_SHADOW_MICROCODE_PPI*This,
+  IN  UINTN CpuIdCount,
+  IN  EDKII_PEI_MICROCODE_CPU_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 9e6cce0895..45aaa179ff 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -69,6 +69,5 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode