[v4] Microcode related optimizations

[edk2-devel] [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA

Posted by Wu, Hao A 6 years, 1 month ago

Previous commits have introduced below fields in structure CPU_AP_DATA:

  UINT32                         ProcessorSignature;
  UINT8                          PlatformId;
  UINT64                         MicrocodeEntryAddr;

which store the information of:

A. CPUID
B. Platform ID
C. Detected microcode patch entry address (including the microcode patch
   header)

for each processor within system.

Therefore, the below fields in structure CPU_MP_DATA:

  UINT32                         ProcessorSignature;
  UINT32                         ProcessorFlags;
  UINT64                         MicrocodeDataAddress;
  UINT32                         MicrocodeRevision;

which store the BSP's information of:

A. CPUID
B. Platform ID
C. The address and revision of detected microcode patch

are redundant and can be removed.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/MpLib.h     |  5 --
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 51 ++++++--------------
 2 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 5f50e79744..6609c958ce 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -263,11 +263,6 @@ struct _CPU_MP_DATA {
   BOOLEAN                        PeriodicMode;
   BOOLEAN                        TimerInterruptState;
 
-  UINT32                         ProcessorSignature;
-  UINT32                         ProcessorFlags;
-  UINT64                         MicrocodeDataAddress;
-  UINT32                         MicrocodeRevision;
-
   //
   // Whether need to use Init-Sipi-Sipi to wake up the APs.
   // Two cases need to set this value to TRUE. One is in HLT
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 74a34c48fa..4ec54b6220 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -85,6 +85,7 @@ MicrocodeDetect (
   UINTN                                   Index;
   UINT8                                   PlatformId;
   CPUID_VERSION_INFO_EAX                  Eax;
+  CPU_AP_DATA                             *CpuData;
   UINT32                                  CurrentRevision;
   UINT32                                  LatestRevision;
   UINTN                                   TotalSize;
@@ -92,16 +93,9 @@ MicrocodeDetect (
   UINT32                                  InCompleteCheckSum32;
   BOOLEAN                                 CorrectMicrocode;
   VOID                                    *MicrocodeData;
-  MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;
-  UINT32                                  ProcessorFlags;
   UINT32                                  ThreadId;
   BOOLEAN                                 IsBspCallIn;
 
-  //
-  // set ProcessorFlags to suppress incorrect compiler/analyzer warnings
-  //
-  ProcessorFlags = 0;
-
   if (CpuMpData->MicrocodePatchRegionSize == 0) {
     //
     // There is no microcode patches
@@ -127,28 +121,25 @@ MicrocodeDetect (
   }
 
   ExtendedTableLength = 0;
-  //
-  // Here data of CPUID leafs have not been collected into context buffer, so
-  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
-  //
-  AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);
-
-  //
-  // The index of platform information resides in bits 50:52 of MSR IA32_PLATFORM_ID
-  //
-  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
-  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
+  Eax.Uint32 = CpuMpData->CpuData[ProcessorNumber].ProcessorSignature;
+  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
 
   //
   // Check whether AP has same processor with BSP.
   // If yes, direct use microcode info saved by BSP.
   //
   if (!IsBspCallIn) {
-    if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
-        (CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
-        MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
-        LatestRevision = CpuMpData->MicrocodeRevision;
-        goto Done;
+    //
+    // Get the CPU data for BSP
+    //
+    CpuData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);
+    if ((CpuData->ProcessorSignature == Eax.Uint32) &&
+        (CpuData->PlatformId == PlatformId) &&
+        (CpuData->MicrocodeEntryAddr != 0)) {
+      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *)(UINTN) CpuData->MicrocodeEntryAddr;
+      MicrocodeData       = (VOID *) (MicrocodeEntryPoint + 1);
+      LatestRevision      = MicrocodeEntryPoint->UpdateRevision;
+      goto Done;
     }
   }
 
@@ -216,7 +207,6 @@ MicrocodeDetect (
         CheckSum32 += MicrocodeEntryPoint->Checksum;
         if (CheckSum32 == 0) {
           CorrectMicrocode = TRUE;
-          ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
         }
       } else if ((MicrocodeEntryPoint->DataSize != 0) &&
                  (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
@@ -260,7 +250,6 @@ MicrocodeDetect (
                     // Find one
                     //
                     CorrectMicrocode = TRUE;
-                    ProcessorFlags = ExtendedTable->ProcessorFlag;
                     break;
                   }
                 }
@@ -332,18 +321,6 @@ Done:
     CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =
       (UINTN) MicrocodeData -  sizeof (CPU_MICROCODE_HEADER);
   }
-
-  if (IsBspCallIn && (LatestRevision != 0)) {
-    //
-    // Save BSP processor info and microcode info for later AP use.
-    //
-    CpuMpData->ProcessorSignature   = Eax.Uint32;
-    CpuMpData->ProcessorFlags       = ProcessorFlags;
-    CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
-    CpuMpData->MicrocodeRevision    = LatestRevision;
-    DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags [0x%08x], \
-       MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, (UINTN) MicrocodeData, LatestRevision));
-  }
 }
 
 /**
-- 
2.12.0.windows.1


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

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

Re: [edk2-devel] [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA

Posted by Dong, Eric 6 years, 1 month ago

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Wu, Hao A
> Sent: Friday, December 27, 2019 3:32 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni,
> Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zeng, Star
> <star.zeng@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>
> Subject: [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant
> microcode fields in CPU_MP_DATA
> 
> Previous commits have introduced below fields in structure CPU_AP_DATA:
> 
>   UINT32                         ProcessorSignature;
>   UINT8                          PlatformId;
>   UINT64                         MicrocodeEntryAddr;
> 
> which store the information of:
> 
> A. CPUID
> B. Platform ID
> C. Detected microcode patch entry address (including the microcode patch
>    header)
> 
> for each processor within system.
> 
> Therefore, the below fields in structure CPU_MP_DATA:
> 
>   UINT32                         ProcessorSignature;
>   UINT32                         ProcessorFlags;
>   UINT64                         MicrocodeDataAddress;
>   UINT32                         MicrocodeRevision;
> 
> which store the BSP's information of:
> 
> A. CPUID
> B. Platform ID
> C. The address and revision of detected microcode patch
> 
> are redundant and can be removed.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Hao A Wu <hao.a.wu@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h     |  5 --
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 51 ++++++--------------
>  2 files changed, 14 insertions(+), 42 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 5f50e79744..6609c958ce 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -263,11 +263,6 @@ struct _CPU_MP_DATA {
>    BOOLEAN                        PeriodicMode;
>    BOOLEAN                        TimerInterruptState;
> 
> -  UINT32                         ProcessorSignature;
> -  UINT32                         ProcessorFlags;
> -  UINT64                         MicrocodeDataAddress;
> -  UINT32                         MicrocodeRevision;
> -
>    //
>    // Whether need to use Init-Sipi-Sipi to wake up the APs.
>    // Two cases need to set this value to TRUE. One is in HLT diff --git
> a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 74a34c48fa..4ec54b6220 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -85,6 +85,7 @@ MicrocodeDetect (
>    UINTN                                   Index;
>    UINT8                                   PlatformId;
>    CPUID_VERSION_INFO_EAX                  Eax;
> +  CPU_AP_DATA                             *CpuData;
>    UINT32                                  CurrentRevision;
>    UINT32                                  LatestRevision;
>    UINTN                                   TotalSize;
> @@ -92,16 +93,9 @@ MicrocodeDetect (
>    UINT32                                  InCompleteCheckSum32;
>    BOOLEAN                                 CorrectMicrocode;
>    VOID                                    *MicrocodeData;
> -  MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;
> -  UINT32                                  ProcessorFlags;
>    UINT32                                  ThreadId;
>    BOOLEAN                                 IsBspCallIn;
> 
> -  //
> -  // set ProcessorFlags to suppress incorrect compiler/analyzer warnings
> -  //
> -  ProcessorFlags = 0;
> -
>    if (CpuMpData->MicrocodePatchRegionSize == 0) {
>      //
>      // There is no microcode patches
> @@ -127,28 +121,25 @@ MicrocodeDetect (
>    }
> 
>    ExtendedTableLength = 0;
> -  //
> -  // Here data of CPUID leafs have not been collected into context buffer, so
> -  // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> -  //
> -  AsmCpuid (CPUID_VERSION_INFO, &Eax.Uint32, NULL, NULL, NULL);
> -
> -  //
> -  // The index of platform information resides in bits 50:52 of MSR
> IA32_PLATFORM_ID
> -  //
> -  PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> -  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
> +  Eax.Uint32 = CpuMpData-
> >CpuData[ProcessorNumber].ProcessorSignature;
> +  PlatformId = CpuMpData->CpuData[ProcessorNumber].PlatformId;
> 
>    //
>    // Check whether AP has same processor with BSP.
>    // If yes, direct use microcode info saved by BSP.
>    //
>    if (!IsBspCallIn) {
> -    if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
> -        (CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
> -        MicrocodeData = (VOID *)(UINTN) CpuMpData-
> >MicrocodeDataAddress;
> -        LatestRevision = CpuMpData->MicrocodeRevision;
> -        goto Done;
> +    //
> +    // Get the CPU data for BSP
> +    //
> +    CpuData = &(CpuMpData->CpuData[CpuMpData->BspNumber]);
> +    if ((CpuData->ProcessorSignature == Eax.Uint32) &&
> +        (CpuData->PlatformId == PlatformId) &&
> +        (CpuData->MicrocodeEntryAddr != 0)) {
> +      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *)(UINTN) CpuData-
> >MicrocodeEntryAddr;
> +      MicrocodeData       = (VOID *) (MicrocodeEntryPoint + 1);
> +      LatestRevision      = MicrocodeEntryPoint->UpdateRevision;
> +      goto Done;
>      }
>    }
> 
> @@ -216,7 +207,6 @@ MicrocodeDetect (
>          CheckSum32 += MicrocodeEntryPoint->Checksum;
>          if (CheckSum32 == 0) {
>            CorrectMicrocode = TRUE;
> -          ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
>          }
>        } else if ((MicrocodeEntryPoint->DataSize != 0) &&
>                   (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) { @@ -
> 260,7 +250,6 @@ MicrocodeDetect (
>                      // Find one
>                      //
>                      CorrectMicrocode = TRUE;
> -                    ProcessorFlags = ExtendedTable->ProcessorFlag;
>                      break;
>                    }
>                  }
> @@ -332,18 +321,6 @@ Done:
>      CpuMpData->CpuData[ProcessorNumber].MicrocodeEntryAddr =
>        (UINTN) MicrocodeData -  sizeof (CPU_MICROCODE_HEADER);
>    }
> -
> -  if (IsBspCallIn && (LatestRevision != 0)) {
> -    //
> -    // Save BSP processor info and microcode info for later AP use.
> -    //
> -    CpuMpData->ProcessorSignature   = Eax.Uint32;
> -    CpuMpData->ProcessorFlags       = ProcessorFlags;
> -    CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
> -    CpuMpData->MicrocodeRevision    = LatestRevision;
> -    DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x],
> ProcessorFlags [0x%08x], \
> -       MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags,
> (UINTN) MicrocodeData, LatestRevision));
> -  }
>  }
> 
>  /**
> --
> 2.12.0.windows.1


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

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

[edk2-devel] [PATCH v4 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info
[edk2-devel] [PATCH v4 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
[edk2-devel] [PATCH v4 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB
[edk2-devel] [PATCH v4 4/6] UefiCpuPkg/MpInitLib: Produce EDKII microcode patch HOB
[edk2-devel] [PATCH v4 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA
[edk2-devel] [PATCH v4 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode fields in CPU_MP_DATA