[edk2-devel] [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.

Siyuan, Fu posted 1 patch 1 week ago
Failed in applying to current master (apply log)
UefiCpuPkg/Include/Guid/MicrocodePatchHob.h | 12 ++++-
UefiCpuPkg/Library/MpInitLib/Microcode.c    | 24 +++++----
UefiCpuPkg/Library/MpInitLib/MpLib.h        |  9 ++++
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c     | 54 +++++++++++++++++++--
4 files changed, 84 insertions(+), 15 deletions(-)

[edk2-devel] [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.

Posted by Siyuan, Fu 1 week ago
This patch adds the original microcode flash address to EDKII microcode
patch HOB after the microcode loaded by processor. This information can
be used to check if a microcode slot has been used by existing processor
or not.

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2454

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/Guid/MicrocodePatchHob.h | 12 ++++-
 UefiCpuPkg/Library/MpInitLib/Microcode.c    | 24 +++++----
 UefiCpuPkg/Library/MpInitLib/MpLib.h        |  9 ++++
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c     | 54 +++++++++++++++++++--
 4 files changed, 84 insertions(+), 15 deletions(-)

diff --git a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
index 2d307fbffb..bbf6f2c66b 100644
--- a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
+++ b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
@@ -3,7 +3,7 @@
     A. Base address and size of the loaded microcode patches data;
     B. Detected microcode patch for each processor within system.
 
-  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -38,7 +38,15 @@ typedef struct {
   // If no microcode patch is detected for certain processor, the relating
   // element will be set to MAX_UINT64.
   //
-  UINT64    ProcessorSpecificPatchOffset[0];
+  //UINT64    ProcessorSpecificPatchOffset[];
+  //
+  // An array with 'ProcessorCount' elements that stores the original
+  // microcode patch address in flash if the patch has been shadowed to
+  // memory. This address will be the same one as specified by
+  // "MicrocodePatchAddress" with "PatchOffset" if the patch wasn't
+  // shadowed to memory.
+  //
+  //UINT64    ProcessorSpecificPatchAddrInRom[];
 } EDKII_MICROCODE_PATCH_HOB;
 
 #endif
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 9389e52ae5..3af5b8495f 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -460,6 +460,7 @@ ShadowMicrocodePatchWorker (
       (VOID *) Patches[Index].Address,
       Patches[Index].Size
       );
+    Patches[Index].AddressInRam = (UINTN) Walker;
     Walker += Patches[Index].Size;
   }
 
@@ -519,7 +520,7 @@ ShadowMicrocodePatchByPcd (
   PatchCount      = 0;
   MaxPatchNumber  = DEFAULT_MAX_MICROCODE_PATCH_NUM;
   TotalLoadSize   = 0;
-  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
+  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
   if (PatchInfoBuffer == NULL) {
     return;
   }
@@ -563,7 +564,7 @@ ShadowMicrocodePatchByPcd (
           //
           // Overflow check for MaxPatchNumber
           //
-          goto OnExit;
+          goto OnError;
         }
 
         PatchInfoBuffer = ReallocatePool (
@@ -572,7 +573,7 @@ ShadowMicrocodePatchByPcd (
                             PatchInfoBuffer
                             );
         if (PatchInfoBuffer == NULL) {
-          goto OnExit;
+          goto OnError;
         }
         MaxPatchNumber = MaxPatchNumber * 2;
       }
@@ -581,7 +582,7 @@ ShadowMicrocodePatchByPcd (
       // Store the information of this microcode patch
       //
       PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;
-      PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;
+      PatchInfoBuffer[PatchCount - 1].Size            = TotalSize;
       TotalLoadSize += TotalSize;
     }
 
@@ -601,7 +602,11 @@ ShadowMicrocodePatchByPcd (
     ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
   }
 
-OnExit:
+  CpuMpData->PatchCount = PatchCount;
+  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
+  return;
+
+OnError:
   if (PatchInfoBuffer != NULL) {
     FreePool (PatchInfoBuffer);
   }
@@ -674,7 +679,7 @@ ShadowMicrocodePatchByFit (
     return EFI_NOT_FOUND;
   }
 
-  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
+  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
   if (PatchInfoBuffer == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
@@ -689,8 +694,8 @@ ShadowMicrocodePatchByFit (
       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;
+        PatchInfoBuffer[PatchCount].Address = (UINTN) MicrocodeEntryPoint;
+        PatchInfoBuffer[PatchCount].Size            = TotalSize;
         TotalLoadSize += TotalSize;
         PatchCount++;
       }
@@ -707,7 +712,8 @@ ShadowMicrocodePatchByFit (
     ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
   }
 
-  FreePool (PatchInfoBuffer);
+  CpuMpData->PatchCount = PatchCount;
+  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
   return EFI_SUCCESS;
 }
 
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 7c62d75acc..51c71ad38e 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -29,6 +29,8 @@
 #include <Library/MtrrLib.h>
 #include <Library/HobLib.h>
 
+#include <Guid/MicrocodePatchHob.h>
+
 #include <IndustryStandard/FirmwareInterfaceTable.h>
 
 
@@ -56,6 +58,7 @@
 //
 typedef struct {
   UINTN    Address;
+  UINTN    AddressInRam;
   UINTN    Size;
 } MICROCODE_PATCH_INFO;
 
@@ -273,6 +276,12 @@ struct _CPU_MP_DATA {
   // driver.
   //
   BOOLEAN                        WakeUpByInitSipiSipi;
+
+  //
+  // Shadow infomation of the microcode patch data.
+  //
+  UINTN                          PatchCount;
+  MICROCODE_PATCH_INFO           *PatchInfoBuffer;
 };
 
 extern EFI_GUID mCpuInitMpLibHobGuid;
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 06e3f5d0d3..07fc05124c 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
 
 **/
@@ -291,6 +291,40 @@ CheckAndUpdateApsStatus (
 {
 }
 
+/**
+  Find the corresponding microcode patch address in ROM before the shadow
+  operation.
+
+  @param[in]  CpuMpData             Pointer to the CPU_MP_DATA structure.
+  @param[in]  MicrocodeEntryAddr    Loaded microcode address to be checked.
+
+  @return    Microcode address in ROM, or MAX_UINT64 if this microcode was
+             not been shadowed.
+**/
+UINT64
+RomAddrOfShadowedMicrocode (
+  IN CPU_MP_DATA    *CpuMpData,
+  IN UINT64         MicrocodeEntryAddr
+  )
+{
+  UINT32                       Index;
+
+  if (CpuMpData->PatchInfoBuffer == NULL) {
+    //
+    // No shadow.
+    //
+    return MicrocodeEntryAddr;
+  }
+
+  for (Index = 0; Index < CpuMpData->PatchCount; Index++) {
+    if (CpuMpData->PatchInfoBuffer[Index].AddressInRam == MicrocodeEntryAddr) {
+      return CpuMpData->PatchInfoBuffer[Index].Address;
+    }
+  }
+
+  return MicrocodeEntryAddr;
+}
+
 /**
   Build the microcode patch HOB that contains the base address and size of the
   microcode patch stored in the memory.
@@ -306,8 +340,11 @@ BuildMicrocodeCacheHob (
   EDKII_MICROCODE_PATCH_HOB    *MicrocodeHob;
   UINTN                        HobDataLength;
   UINT32                       Index;
+  UINT64                       *ProcessorSpecificPatchOffset;
+  UINT64                       *ProcessorSpecificPatchAddrInRom;
 
   HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
+                  sizeof (UINT64) * CpuMpData->CpuCount +
                   sizeof (UINT64) * CpuMpData->CpuCount;
 
   MicrocodeHob  = AllocatePool (HobDataLength);
@@ -317,10 +354,14 @@ BuildMicrocodeCacheHob (
   }
 
   //
-  // Store the information of the memory region that holds the microcode patches.
+  // Store information of the memory region that holds the microcode patches.
   //
   MicrocodeHob->MicrocodePatchAddress    = CpuMpData->MicrocodePatchAddress;
   MicrocodeHob->MicrocodePatchRegionSize = CpuMpData->MicrocodePatchRegionSize;
+  ProcessorSpecificPatchOffset =
+    (UINT64*) ((UINTN )MicrocodeHob + sizeof (EDKII_MICROCODE_PATCH_HOB));
+  ProcessorSpecificPatchAddrInRom =
+    (UINT64*) ((UINTN )ProcessorSpecificPatchOffset + sizeof (UINT64) * CpuMpData->CpuCount);
 
   //
   // Store the detected microcode patch for each processor as well.
@@ -328,10 +369,15 @@ BuildMicrocodeCacheHob (
   MicrocodeHob->ProcessorCount = CpuMpData->CpuCount;
   for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
     if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
-      MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
+      ProcessorSpecificPatchOffset[Index] =
         CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData->MicrocodePatchAddress;
+      ProcessorSpecificPatchAddrInRom[Index] = RomAddrOfShadowedMicrocode (
+                                                 CpuMpData,
+                                                 CpuMpData->CpuData[Index].MicrocodeEntryAddr
+                                                 );
     } else {
-      MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
+      ProcessorSpecificPatchOffset[Index]    = MAX_UINT64;
+      ProcessorSpecificPatchAddrInRom[Index] = MAX_UINT64;
     }
   }
 
-- 
2.19.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53140): https://edk2.groups.io/g/devel/message/53140
Mute This Topic: https://groups.io/mt/69596254/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.

Posted by Ni, Ray 1 week ago
I will review next Monday.

> -----Original Message-----
> From: Fu, Siyuan <siyuan.fu@intel.com>
> Sent: Friday, January 10, 2020 3:08 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>
> Subject: [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode
> patch HOB.
> 
> This patch adds the original microcode flash address to EDKII microcode
> patch HOB after the microcode loaded by processor. This information can
> be used to check if a microcode slot has been used by existing processor
> or not.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2454
> 
> 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/Guid/MicrocodePatchHob.h | 12 ++++-
>  UefiCpuPkg/Library/MpInitLib/Microcode.c    | 24 +++++----
>  UefiCpuPkg/Library/MpInitLib/MpLib.h        |  9 ++++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c     | 54 +++++++++++++++++++--
>  4 files changed, 84 insertions(+), 15 deletions(-)
> 
> diff --git a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> index 2d307fbffb..bbf6f2c66b 100644
> --- a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> +++ b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> @@ -3,7 +3,7 @@
>      A. Base address and size of the loaded microcode patches data;
>      B. Detected microcode patch for each processor within system.
> 
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -38,7 +38,15 @@ typedef struct {
>    // If no microcode patch is detected for certain processor, the relating
>    // element will be set to MAX_UINT64.
>    //
> -  UINT64    ProcessorSpecificPatchOffset[0];
> +  //UINT64    ProcessorSpecificPatchOffset[];
> +  //
> +  // An array with 'ProcessorCount' elements that stores the original
> +  // microcode patch address in flash if the patch has been shadowed to
> +  // memory. This address will be the same one as specified by
> +  // "MicrocodePatchAddress" with "PatchOffset" if the patch wasn't
> +  // shadowed to memory.
> +  //
> +  //UINT64    ProcessorSpecificPatchAddrInRom[];
>  } EDKII_MICROCODE_PATCH_HOB;
> 
>  #endif
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 9389e52ae5..3af5b8495f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -460,6 +460,7 @@ ShadowMicrocodePatchWorker (
>        (VOID *) Patches[Index].Address,
>        Patches[Index].Size
>        );
> +    Patches[Index].AddressInRam = (UINTN) Walker;
>      Walker += Patches[Index].Size;
>    }
> 
> @@ -519,7 +520,7 @@ ShadowMicrocodePatchByPcd (
>    PatchCount      = 0;
>    MaxPatchNumber  = DEFAULT_MAX_MICROCODE_PATCH_NUM;
>    TotalLoadSize   = 0;
> -  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof
> (MICROCODE_PATCH_INFO));
> +  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof
> (MICROCODE_PATCH_INFO));
>    if (PatchInfoBuffer == NULL) {
>      return;
>    }
> @@ -563,7 +564,7 @@ ShadowMicrocodePatchByPcd (
>            //
>            // Overflow check for MaxPatchNumber
>            //
> -          goto OnExit;
> +          goto OnError;
>          }
> 
>          PatchInfoBuffer = ReallocatePool (
> @@ -572,7 +573,7 @@ ShadowMicrocodePatchByPcd (
>                              PatchInfoBuffer
>                              );
>          if (PatchInfoBuffer == NULL) {
> -          goto OnExit;
> +          goto OnError;
>          }
>          MaxPatchNumber = MaxPatchNumber * 2;
>        }
> @@ -581,7 +582,7 @@ ShadowMicrocodePatchByPcd (
>        // Store the information of this microcode patch
>        //
>        PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;
> -      PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;
> +      PatchInfoBuffer[PatchCount - 1].Size            = TotalSize;
>        TotalLoadSize += TotalSize;
>      }
> 
> @@ -601,7 +602,11 @@ ShadowMicrocodePatchByPcd (
>      ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount,
> TotalLoadSize);
>    }
> 
> -OnExit:
> +  CpuMpData->PatchCount = PatchCount;
> +  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
> +  return;
> +
> +OnError:
>    if (PatchInfoBuffer != NULL) {
>      FreePool (PatchInfoBuffer);
>    }
> @@ -674,7 +679,7 @@ ShadowMicrocodePatchByFit (
>      return EFI_NOT_FOUND;
>    }
> 
> -  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof
> (MICROCODE_PATCH_INFO));
> +  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof
> (MICROCODE_PATCH_INFO));
>    if (PatchInfoBuffer == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> @@ -689,8 +694,8 @@ ShadowMicrocodePatchByFit (
>        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;
> +        PatchInfoBuffer[PatchCount].Address = (UINTN) MicrocodeEntryPoint;
> +        PatchInfoBuffer[PatchCount].Size            = TotalSize;
>          TotalLoadSize += TotalSize;
>          PatchCount++;
>        }
> @@ -707,7 +712,8 @@ ShadowMicrocodePatchByFit (
>      ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount,
> TotalLoadSize);
>    }
> 
> -  FreePool (PatchInfoBuffer);
> +  CpuMpData->PatchCount = PatchCount;
> +  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
>    return EFI_SUCCESS;
>  }
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 7c62d75acc..51c71ad38e 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -29,6 +29,8 @@
>  #include <Library/MtrrLib.h>
>  #include <Library/HobLib.h>
> 
> +#include <Guid/MicrocodePatchHob.h>
> +
>  #include <IndustryStandard/FirmwareInterfaceTable.h>
> 
> 
> @@ -56,6 +58,7 @@
>  //
>  typedef struct {
>    UINTN    Address;
> +  UINTN    AddressInRam;
>    UINTN    Size;
>  } MICROCODE_PATCH_INFO;
> 
> @@ -273,6 +276,12 @@ struct _CPU_MP_DATA {
>    // driver.
>    //
>    BOOLEAN                        WakeUpByInitSipiSipi;
> +
> +  //
> +  // Shadow infomation of the microcode patch data.
> +  //
> +  UINTN                          PatchCount;
> +  MICROCODE_PATCH_INFO           *PatchInfoBuffer;
>  };
> 
>  extern EFI_GUID mCpuInitMpLibHobGuid;
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 06e3f5d0d3..07fc05124c 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
> 
>  **/
> @@ -291,6 +291,40 @@ CheckAndUpdateApsStatus (
>  {
>  }
> 
> +/**
> +  Find the corresponding microcode patch address in ROM before the shadow
> +  operation.
> +
> +  @param[in]  CpuMpData             Pointer to the CPU_MP_DATA structure.
> +  @param[in]  MicrocodeEntryAddr    Loaded microcode address to be checked.
> +
> +  @return    Microcode address in ROM, or MAX_UINT64 if this microcode was
> +             not been shadowed.
> +**/
> +UINT64
> +RomAddrOfShadowedMicrocode (
> +  IN CPU_MP_DATA    *CpuMpData,
> +  IN UINT64         MicrocodeEntryAddr
> +  )
> +{
> +  UINT32                       Index;
> +
> +  if (CpuMpData->PatchInfoBuffer == NULL) {
> +    //
> +    // No shadow.
> +    //
> +    return MicrocodeEntryAddr;
> +  }
> +
> +  for (Index = 0; Index < CpuMpData->PatchCount; Index++) {
> +    if (CpuMpData->PatchInfoBuffer[Index].AddressInRam ==
> MicrocodeEntryAddr) {
> +      return CpuMpData->PatchInfoBuffer[Index].Address;
> +    }
> +  }
> +
> +  return MicrocodeEntryAddr;
> +}
> +
>  /**
>    Build the microcode patch HOB that contains the base address and size of the
>    microcode patch stored in the memory.
> @@ -306,8 +340,11 @@ BuildMicrocodeCacheHob (
>    EDKII_MICROCODE_PATCH_HOB    *MicrocodeHob;
>    UINTN                        HobDataLength;
>    UINT32                       Index;
> +  UINT64                       *ProcessorSpecificPatchOffset;
> +  UINT64                       *ProcessorSpecificPatchAddrInRom;
> 
>    HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
> +                  sizeof (UINT64) * CpuMpData->CpuCount +
>                    sizeof (UINT64) * CpuMpData->CpuCount;
> 
>    MicrocodeHob  = AllocatePool (HobDataLength);
> @@ -317,10 +354,14 @@ BuildMicrocodeCacheHob (
>    }
> 
>    //
> -  // Store the information of the memory region that holds the microcode
> patches.
> +  // Store information of the memory region that holds the microcode patches.
>    //
>    MicrocodeHob->MicrocodePatchAddress    = CpuMpData-
> >MicrocodePatchAddress;
>    MicrocodeHob->MicrocodePatchRegionSize = CpuMpData-
> >MicrocodePatchRegionSize;
> +  ProcessorSpecificPatchOffset =
> +    (UINT64*) ((UINTN )MicrocodeHob + sizeof
> (EDKII_MICROCODE_PATCH_HOB));
> +  ProcessorSpecificPatchAddrInRom =
> +    (UINT64*) ((UINTN )ProcessorSpecificPatchOffset + sizeof (UINT64) *
> CpuMpData->CpuCount);
> 
>    //
>    // Store the detected microcode patch for each processor as well.
> @@ -328,10 +369,15 @@ BuildMicrocodeCacheHob (
>    MicrocodeHob->ProcessorCount = CpuMpData->CpuCount;
>    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
>      if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
> -      MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
> +      ProcessorSpecificPatchOffset[Index] =
>          CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData-
> >MicrocodePatchAddress;
> +      ProcessorSpecificPatchAddrInRom[Index] =
> RomAddrOfShadowedMicrocode (
> +                                                 CpuMpData,
> +                                                 CpuMpData->CpuData[Index].MicrocodeEntryAddr
> +                                                 );
>      } else {
> -      MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
> +      ProcessorSpecificPatchOffset[Index]    = MAX_UINT64;
> +      ProcessorSpecificPatchAddrInRom[Index] = MAX_UINT64;
>      }
>    }
> 
> --
> 2.19.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53160): https://edk2.groups.io/g/devel/message/53160
Mute This Topic: https://groups.io/mt/69596254/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.

Posted by Laszlo Ersek 1 week ago
On 01/10/20 08:08, Siyuan Fu wrote:
> This patch adds the original microcode flash address to EDKII microcode
> patch HOB after the microcode loaded by processor. This information can
> be used to check if a microcode slot has been used by existing processor
> or not.
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2454
> 
> 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/Guid/MicrocodePatchHob.h | 12 ++++-
>  UefiCpuPkg/Library/MpInitLib/Microcode.c    | 24 +++++----
>  UefiCpuPkg/Library/MpInitLib/MpLib.h        |  9 ++++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c     | 54 +++++++++++++++++++--
>  4 files changed, 84 insertions(+), 15 deletions(-)

I'll let Eric and Ray review this.

Thanks!
Laszlo

> diff --git a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> index 2d307fbffb..bbf6f2c66b 100644
> --- a/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> +++ b/UefiCpuPkg/Include/Guid/MicrocodePatchHob.h
> @@ -3,7 +3,7 @@
>      A. Base address and size of the loaded microcode patches data;
>      B. Detected microcode patch for each processor within system.
>  
> -  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -38,7 +38,15 @@ typedef struct {
>    // If no microcode patch is detected for certain processor, the relating
>    // element will be set to MAX_UINT64.
>    //
> -  UINT64    ProcessorSpecificPatchOffset[0];
> +  //UINT64    ProcessorSpecificPatchOffset[];
> +  //
> +  // An array with 'ProcessorCount' elements that stores the original
> +  // microcode patch address in flash if the patch has been shadowed to
> +  // memory. This address will be the same one as specified by
> +  // "MicrocodePatchAddress" with "PatchOffset" if the patch wasn't
> +  // shadowed to memory.
> +  //
> +  //UINT64    ProcessorSpecificPatchAddrInRom[];
>  } EDKII_MICROCODE_PATCH_HOB;
>  
>  #endif
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 9389e52ae5..3af5b8495f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -460,6 +460,7 @@ ShadowMicrocodePatchWorker (
>        (VOID *) Patches[Index].Address,
>        Patches[Index].Size
>        );
> +    Patches[Index].AddressInRam = (UINTN) Walker;
>      Walker += Patches[Index].Size;
>    }
>  
> @@ -519,7 +520,7 @@ ShadowMicrocodePatchByPcd (
>    PatchCount      = 0;
>    MaxPatchNumber  = DEFAULT_MAX_MICROCODE_PATCH_NUM;
>    TotalLoadSize   = 0;
> -  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
> +  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
>    if (PatchInfoBuffer == NULL) {
>      return;
>    }
> @@ -563,7 +564,7 @@ ShadowMicrocodePatchByPcd (
>            //
>            // Overflow check for MaxPatchNumber
>            //
> -          goto OnExit;
> +          goto OnError;
>          }
>  
>          PatchInfoBuffer = ReallocatePool (
> @@ -572,7 +573,7 @@ ShadowMicrocodePatchByPcd (
>                              PatchInfoBuffer
>                              );
>          if (PatchInfoBuffer == NULL) {
> -          goto OnExit;
> +          goto OnError;
>          }
>          MaxPatchNumber = MaxPatchNumber * 2;
>        }
> @@ -581,7 +582,7 @@ ShadowMicrocodePatchByPcd (
>        // Store the information of this microcode patch
>        //
>        PatchInfoBuffer[PatchCount - 1].Address = (UINTN) MicrocodeEntryPoint;
> -      PatchInfoBuffer[PatchCount - 1].Size    = TotalSize;
> +      PatchInfoBuffer[PatchCount - 1].Size            = TotalSize;
>        TotalLoadSize += TotalSize;
>      }
>  
> @@ -601,7 +602,11 @@ ShadowMicrocodePatchByPcd (
>      ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
>    }
>  
> -OnExit:
> +  CpuMpData->PatchCount = PatchCount;
> +  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
> +  return;
> +
> +OnError:
>    if (PatchInfoBuffer != NULL) {
>      FreePool (PatchInfoBuffer);
>    }
> @@ -674,7 +679,7 @@ ShadowMicrocodePatchByFit (
>      return EFI_NOT_FOUND;
>    }
>  
> -  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
> +  PatchInfoBuffer = AllocateZeroPool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
>    if (PatchInfoBuffer == NULL) {
>      return EFI_OUT_OF_RESOURCES;
>    }
> @@ -689,8 +694,8 @@ ShadowMicrocodePatchByFit (
>        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;
> +        PatchInfoBuffer[PatchCount].Address = (UINTN) MicrocodeEntryPoint;
> +        PatchInfoBuffer[PatchCount].Size            = TotalSize;
>          TotalLoadSize += TotalSize;
>          PatchCount++;
>        }
> @@ -707,7 +712,8 @@ ShadowMicrocodePatchByFit (
>      ShadowMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
>    }
>  
> -  FreePool (PatchInfoBuffer);
> +  CpuMpData->PatchCount = PatchCount;
> +  CpuMpData->PatchInfoBuffer = PatchInfoBuffer;
>    return EFI_SUCCESS;
>  }
>  
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 7c62d75acc..51c71ad38e 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -29,6 +29,8 @@
>  #include <Library/MtrrLib.h>
>  #include <Library/HobLib.h>
>  
> +#include <Guid/MicrocodePatchHob.h>
> +
>  #include <IndustryStandard/FirmwareInterfaceTable.h>
>  
>  
> @@ -56,6 +58,7 @@
>  //
>  typedef struct {
>    UINTN    Address;
> +  UINTN    AddressInRam;
>    UINTN    Size;
>  } MICROCODE_PATCH_INFO;
>  
> @@ -273,6 +276,12 @@ struct _CPU_MP_DATA {
>    // driver.
>    //
>    BOOLEAN                        WakeUpByInitSipiSipi;
> +
> +  //
> +  // Shadow infomation of the microcode patch data.
> +  //
> +  UINTN                          PatchCount;
> +  MICROCODE_PATCH_INFO           *PatchInfoBuffer;
>  };
>  
>  extern EFI_GUID mCpuInitMpLibHobGuid;
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 06e3f5d0d3..07fc05124c 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
>  
>  **/
> @@ -291,6 +291,40 @@ CheckAndUpdateApsStatus (
>  {
>  }
>  
> +/**
> +  Find the corresponding microcode patch address in ROM before the shadow
> +  operation.
> +
> +  @param[in]  CpuMpData             Pointer to the CPU_MP_DATA structure.
> +  @param[in]  MicrocodeEntryAddr    Loaded microcode address to be checked.
> +
> +  @return    Microcode address in ROM, or MAX_UINT64 if this microcode was
> +             not been shadowed.
> +**/
> +UINT64
> +RomAddrOfShadowedMicrocode (
> +  IN CPU_MP_DATA    *CpuMpData,
> +  IN UINT64         MicrocodeEntryAddr
> +  )
> +{
> +  UINT32                       Index;
> +
> +  if (CpuMpData->PatchInfoBuffer == NULL) {
> +    //
> +    // No shadow.
> +    //
> +    return MicrocodeEntryAddr;
> +  }
> +
> +  for (Index = 0; Index < CpuMpData->PatchCount; Index++) {
> +    if (CpuMpData->PatchInfoBuffer[Index].AddressInRam == MicrocodeEntryAddr) {
> +      return CpuMpData->PatchInfoBuffer[Index].Address;
> +    }
> +  }
> +
> +  return MicrocodeEntryAddr;
> +}
> +
>  /**
>    Build the microcode patch HOB that contains the base address and size of the
>    microcode patch stored in the memory.
> @@ -306,8 +340,11 @@ BuildMicrocodeCacheHob (
>    EDKII_MICROCODE_PATCH_HOB    *MicrocodeHob;
>    UINTN                        HobDataLength;
>    UINT32                       Index;
> +  UINT64                       *ProcessorSpecificPatchOffset;
> +  UINT64                       *ProcessorSpecificPatchAddrInRom;
>  
>    HobDataLength = sizeof (EDKII_MICROCODE_PATCH_HOB) +
> +                  sizeof (UINT64) * CpuMpData->CpuCount +
>                    sizeof (UINT64) * CpuMpData->CpuCount;
>  
>    MicrocodeHob  = AllocatePool (HobDataLength);
> @@ -317,10 +354,14 @@ BuildMicrocodeCacheHob (
>    }
>  
>    //
> -  // Store the information of the memory region that holds the microcode patches.
> +  // Store information of the memory region that holds the microcode patches.
>    //
>    MicrocodeHob->MicrocodePatchAddress    = CpuMpData->MicrocodePatchAddress;
>    MicrocodeHob->MicrocodePatchRegionSize = CpuMpData->MicrocodePatchRegionSize;
> +  ProcessorSpecificPatchOffset =
> +    (UINT64*) ((UINTN )MicrocodeHob + sizeof (EDKII_MICROCODE_PATCH_HOB));
> +  ProcessorSpecificPatchAddrInRom =
> +    (UINT64*) ((UINTN )ProcessorSpecificPatchOffset + sizeof (UINT64) * CpuMpData->CpuCount);
>  
>    //
>    // Store the detected microcode patch for each processor as well.
> @@ -328,10 +369,15 @@ BuildMicrocodeCacheHob (
>    MicrocodeHob->ProcessorCount = CpuMpData->CpuCount;
>    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
>      if (CpuMpData->CpuData[Index].MicrocodeEntryAddr != 0) {
> -      MicrocodeHob->ProcessorSpecificPatchOffset[Index] =
> +      ProcessorSpecificPatchOffset[Index] =
>          CpuMpData->CpuData[Index].MicrocodeEntryAddr - CpuMpData->MicrocodePatchAddress;
> +      ProcessorSpecificPatchAddrInRom[Index] = RomAddrOfShadowedMicrocode (
> +                                                 CpuMpData,
> +                                                 CpuMpData->CpuData[Index].MicrocodeEntryAddr
> +                                                 );
>      } else {
> -      MicrocodeHob->ProcessorSpecificPatchOffset[Index] = MAX_UINT64;
> +      ProcessorSpecificPatchOffset[Index]    = MAX_UINT64;
> +      ProcessorSpecificPatchAddrInRom[Index] = MAX_UINT64;
>      }
>    }
>  
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#53159): https://edk2.groups.io/g/devel/message/53159
Mute This Topic: https://groups.io/mt/69596254/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-