public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.
@ 2020-01-10  7:08 Siyuan, Fu
  2020-01-10 11:58 ` Laszlo Ersek
  2020-01-10 13:51 ` Ni, Ray
  0 siblings, 2 replies; 3+ messages in thread
From: Siyuan, Fu @ 2020-01-10  7:08 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.
  2020-01-10  7:08 [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB Siyuan, Fu
@ 2020-01-10 11:58 ` Laszlo Ersek
  2020-01-10 13:51 ` Ni, Ray
  1 sibling, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2020-01-10 11:58 UTC (permalink / raw)
  To: Siyuan Fu, devel; +Cc: Eric Dong, Ray Ni

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;
>      }
>    }
>  
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB.
  2020-01-10  7:08 [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB Siyuan, Fu
  2020-01-10 11:58 ` Laszlo Ersek
@ 2020-01-10 13:51 ` Ni, Ray
  1 sibling, 0 replies; 3+ messages in thread
From: Ni, Ray @ 2020-01-10 13:51 UTC (permalink / raw)
  To: Fu, Siyuan, devel@edk2.groups.io; +Cc: Dong, Eric, Laszlo Ersek

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-01-10 13:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-10  7:08 [Patch] UefiCpuPkg: Add microcode flash address to EDKII microcode patch HOB Siyuan, Fu
2020-01-10 11:58 ` Laszlo Ersek
2020-01-10 13:51 ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox