public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: devel@edk2.groups.io
Cc: Hao A Wu <hao.a.wu@intel.com>, Eric Dong <eric.dong@intel.com>,
	Ray Ni <ray.ni@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Star Zeng <star.zeng@intel.com>, Siyuan Fu <siyuan.fu@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: [PATCH v2 2/6] UefiCpuPkg/MpInitLib: Reduce the size when loading microcode patches
Date: Wed, 25 Dec 2019 15:58:10 +0800	[thread overview]
Message-ID: <20191225075814.8372-3-hao.a.wu@intel.com> (raw)
In-Reply-To: <20191225075814.8372-1-hao.a.wu@intel.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2429

This commit will attempt to reduce the copy size when loading the
microcode patches data from flash into memory.

Such optimization is done by a pre-process of the microcode patch headers
(on flash). A microcode patch will be loaded into memory only when the
below 2 criteria are met:

A. With a microcode patch header (which means the data is not padding data
   between microcode patches);
B. The 'ProcessorSignature' & 'ProcessorFlags' fields in the header match
   at least one processor within system.

Criterion B will require all the processors to be woken up once to collect
their CPUID and Platform ID information. Hence, this commit will move the
copy, detect and apply of microcode patch on BSP and APs after all the
processors have been woken up.

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     |  24 ++
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 229 ++++++++++++++++++++
 UefiCpuPkg/Library/MpInitLib/MpLib.c     |  82 ++-----
 3 files changed, 277 insertions(+), 58 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 4440dc2701..56b0df664a 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -44,6 +44,20 @@
 #define CPU_SWITCH_STATE_LOADED 2
 
 //
+// Default maximum number of entries to store the microcode patches information
+//
+#define DEFAULT_MAX_MICROCODE_PATCH_NUM 8
+
+//
+// Data structure for microcode patch information
+//
+typedef struct {
+  UINTN    Address;
+  UINTN    Size;
+  UINTN    AlignedSize;
+} MICROCODE_PATCH_INFO;
+
+//
 // CPU exchange information for switch BSP
 //
 typedef struct {
@@ -576,6 +590,16 @@ MicrocodeDetect (
   );
 
 /**
+  Load the required microcode patches data into memory.
+
+  @param[in, out]  CpuMpData    The pointer to CPU MP Data structure.
+**/
+VOID
+LoadMicrocodePatch (
+  IN OUT CPU_MP_DATA             *CpuMpData
+  );
+
+/**
   Detect whether Mwait-monitor feature is supported.
 
   @retval TRUE    Mwait-monitor feature is supported.
diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 199b1f23ce..2dfbcc13ab 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -331,3 +331,232 @@ Done:
        MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, (UINTN) MicrocodeData, LatestRevision));
   }
 }
+
+/**
+  Actual worker function that loads the required microcode patches into memory.
+
+  @param[in, out]  CpuMpData        The pointer to CPU MP Data structure.
+  @param[in]       Patches          The pointer to an array of information on
+                                    the microcode patches that will be loaded
+                                    into memory.
+  @param[in]       PatchCount       The number of microcode patches that will
+                                    be loaded into memory.
+  @param[in]       TotalLoadSize    The total size of all the microcode patches
+                                    to be loaded.
+**/
+VOID
+LoadMicrocodePatchWorker (
+  IN OUT CPU_MP_DATA             *CpuMpData,
+  IN     MICROCODE_PATCH_INFO    *Patches,
+  IN     UINTN                   PatchCount,
+  IN     UINTN                   TotalLoadSize
+  )
+{
+  UINTN    Index;
+  VOID     *MicrocodePatchInRam;
+  UINT8    *Walker;
+
+  ASSERT ((Patches != NULL) && (PatchCount != 0));
+
+  MicrocodePatchInRam = AllocatePages (EFI_SIZE_TO_PAGES (TotalLoadSize));
+  if (MicrocodePatchInRam == NULL) {
+    return;
+  }
+
+  //
+  // Load all the required microcode patches into memory
+  //
+  for (Walker = MicrocodePatchInRam, Index = 0; Index < PatchCount; Index++) {
+    CopyMem (
+      Walker,
+      (VOID *) Patches[Index].Address,
+      Patches[Index].Size
+      );
+
+    //
+    // Zero-fill the padding area
+    // Please note that AlignedSize will be no less than Size
+    //
+    ZeroMem (
+      Walker + Patches[Index].Size,
+      Patches[Index].AlignedSize - Patches[Index].Size
+      );
+
+    Walker += Patches[Index].AlignedSize;
+  }
+
+  //
+  // Update the microcode patch related fields in CpuMpData
+  //
+  CpuMpData->MicrocodePatchAddress    = (UINTN) MicrocodePatchInRam;
+  CpuMpData->MicrocodePatchRegionSize = TotalLoadSize;
+
+  DEBUG ((
+    DEBUG_INFO,
+    "%a: Required microcode patches have been loaded at 0x%lx, with size 0x%lx.\n",
+    __FUNCTION__, CpuMpData->MicrocodePatchAddress, CpuMpData->MicrocodePatchRegionSize
+    ));
+
+  return;
+}
+
+/**
+  Load the required microcode patches data into memory.
+
+  @param[in, out]  CpuMpData    The pointer to CPU MP Data structure.
+**/
+VOID
+LoadMicrocodePatch (
+  IN OUT CPU_MP_DATA             *CpuMpData
+  )
+{
+  CPU_MICROCODE_HEADER    *MicrocodeEntryPoint;
+  UINTN                   MicrocodeEnd;
+  UINTN                   DataSize;
+  UINTN                   TotalSize;
+  MICROCODE_PATCH_INFO    *PatchInfoBuffer;
+  UINTN                   MaxPatchNumber;
+  UINTN                   PatchCount;
+  UINTN                   TotalLoadSize;
+  UINT32                  ProcessorSignature;
+  UINT32                  ProcessorFlags;
+  UINTN                   Index;
+  CPU_AP_DATA             *CpuData;
+  BOOLEAN                 NeedLoad;
+
+  //
+  // Initialize the microcode patch related fields in CpuMpData as the values
+  // specified by the PCD pair. If the microcode patches are loaded into memory,
+  // these fields will be updated.
+  //
+  CpuMpData->MicrocodePatchAddress    = PcdGet64 (PcdCpuMicrocodePatchAddress);
+  CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize);
+
+  MicrocodeEntryPoint    = (CPU_MICROCODE_HEADER *) (UINTN) CpuMpData->MicrocodePatchAddress;
+  MicrocodeEnd           = (UINTN) MicrocodeEntryPoint +
+                           (UINTN) CpuMpData->MicrocodePatchRegionSize;
+  if ((MicrocodeEntryPoint == NULL) || ((UINTN) MicrocodeEntryPoint == MicrocodeEnd)) {
+    //
+    // There is no microcode patches
+    //
+    return;
+  }
+
+  PatchCount      = 0;
+  MaxPatchNumber  = DEFAULT_MAX_MICROCODE_PATCH_NUM;
+  TotalLoadSize   = 0;
+  PatchInfoBuffer = AllocatePool (MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO));
+  if (PatchInfoBuffer == NULL) {
+    return;
+  }
+
+  //
+  // Process the header of each microcode patch within the region.
+  // The purpose is to decide which microcode patch(es) will be loaded into memory.
+  //
+  do {
+    if (MicrocodeEntryPoint->HeaderVersion != 0x1) {
+      //
+      // Padding data between the microcode patches, skip 1KB to check next entry.
+      //
+      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
+      continue;
+    }
+
+    DataSize = MicrocodeEntryPoint->DataSize;
+    if (DataSize == 0) {
+      TotalSize = sizeof (CPU_MICROCODE_HEADER) + 2000;
+    } else {
+      TotalSize = sizeof (CPU_MICROCODE_HEADER) + DataSize;
+    }
+
+    if ( (UINTN)MicrocodeEntryPoint > (MAX_ADDRESS - TotalSize) ||
+         ((UINTN)MicrocodeEntryPoint + TotalSize) > MicrocodeEnd ||
+         (TotalSize & 0x3) != 0
+       ) {
+      //
+      // Not a valid microcode header, skip 1KB to check next entry.
+      //
+      MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + SIZE_1KB);
+      continue;
+    }
+
+    TotalSize = (DataSize == 0) ? 2048 : MicrocodeEntryPoint->TotalSize;
+
+    //
+    // Check the 'ProcessorSignature' & 'ProcessorFlags' of this microcode patch
+    // with the processors' CPUID & PlatformID to decide if it will be copied
+    // into memory
+    //
+    ProcessorSignature = MicrocodeEntryPoint->ProcessorSignature.Uint32;
+    ProcessorFlags     = MicrocodeEntryPoint->ProcessorFlags;
+    NeedLoad           = FALSE;
+    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+      CpuData = &CpuMpData->CpuData[Index];
+      if ((ProcessorSignature == CpuData->ProcessorSignature) &&
+          (ProcessorFlags & (1 << CpuData->PlatformId)) != 0) {
+        NeedLoad = TRUE;
+        break;
+      }
+    }
+
+    if (NeedLoad) {
+      PatchCount++;
+      if (PatchCount >= MaxPatchNumber) {
+        //
+        // Current 'PatchInfoBuffer' cannot hold the information, double the size
+        // and allocate a new buffer.
+        //
+        if (MaxPatchNumber > MAX_UINTN / 2 / sizeof (MICROCODE_PATCH_INFO)) {
+          //
+          // Overflow check for MaxPatchNumber
+          //
+          goto OnExit;
+        }
+
+        PatchInfoBuffer = ReallocatePool (
+                            MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),
+                            2 * MaxPatchNumber * sizeof (MICROCODE_PATCH_INFO),
+                            PatchInfoBuffer
+                            );
+        if (PatchInfoBuffer == NULL) {
+          goto OnExit;
+        }
+        MaxPatchNumber = MaxPatchNumber * 2;
+      }
+
+      //
+      // Store the information of this microcode patch
+      //
+      if (TotalSize > ALIGN_VALUE (TotalSize, SIZE_1KB) ||
+          ALIGN_VALUE (TotalSize, SIZE_1KB) > MAX_UINTN - TotalLoadSize) {
+        goto OnExit;
+      }
+      PatchInfoBuffer[PatchCount - 1].Address     = (UINTN) MicrocodeEntryPoint;
+      PatchInfoBuffer[PatchCount - 1].Size        = TotalSize;
+      PatchInfoBuffer[PatchCount - 1].AlignedSize = ALIGN_VALUE (TotalSize, SIZE_1KB);
+      TotalLoadSize += PatchInfoBuffer[PatchCount - 1].AlignedSize;
+    }
+
+    //
+    // Process the next microcode patch
+    //
+    MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) MicrocodeEntryPoint) + TotalSize);
+  } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
+
+  if (PatchCount != 0) {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: 0x%x microcode patches will be loaded into memory, with size 0x%x.\n",
+      __FUNCTION__, PatchCount, TotalLoadSize
+      ));
+
+    LoadMicrocodePatchWorker (CpuMpData, PatchInfoBuffer, PatchCount, TotalLoadSize);
+  }
+
+OnExit:
+  if (PatchInfoBuffer != NULL) {
+    FreePool (PatchInfoBuffer);
+  }
+  return;
+}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index d5077e080e..199468156b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -628,10 +628,6 @@ ApWakeupFunction (
       ApTopOfStack  = CpuMpData->Buffer + (ProcessorNumber + 1) * CpuMpData->CpuApStackSize;
       BistData = *(UINT32 *) ((UINTN) ApTopOfStack - sizeof (UINTN));
       //
-      // Do some AP initialize sync
-      //
-      ApInitializeSync (CpuMpData);
-      //
       // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment,
       //   to initialize AP in InitConfig path.
       // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters points to a different IDT shared by all APs.
@@ -1615,7 +1611,6 @@ MpInitLibInitialize (
   UINTN                    ApResetVectorSize;
   UINTN                    BackupBufferAddr;
   UINTN                    ApIdtBase;
-  VOID                     *MicrocodePatchInRam;
 
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
@@ -1683,39 +1678,7 @@ MpInitLibInitialize (
   CpuMpData->SwitchBspFlag    = FALSE;
   CpuMpData->CpuData          = (CPU_AP_DATA *) (CpuMpData + 1);
   CpuMpData->CpuInfoInHob     = (UINT64) (UINTN) (CpuMpData->CpuData + MaxLogicalProcessorNumber);
-  if (OldCpuMpData == NULL) {
-    CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize);
-    //
-    // If platform has more than one CPU, relocate microcode to memory to reduce
-    // loading microcode time.
-    //
-    MicrocodePatchInRam = NULL;
-    if (MaxLogicalProcessorNumber > 1) {
-      MicrocodePatchInRam = AllocatePages (
-                              EFI_SIZE_TO_PAGES (
-                                (UINTN)CpuMpData->MicrocodePatchRegionSize
-                                )
-                              );
-    }
-    if (MicrocodePatchInRam == NULL) {
-      //
-      // there is only one processor, or no microcode patch is available, or
-      // memory allocation failed
-      //
-      CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress);
-    } else {
-      //
-      // there are multiple processors, and a microcode patch is available, and
-      // memory allocation succeeded
-      //
-      CopyMem (
-        MicrocodePatchInRam,
-        (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
-        (UINTN)CpuMpData->MicrocodePatchRegionSize
-        );
-      CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
-    }
-  }else {
+  if (OldCpuMpData != NULL) {
     CpuMpData->MicrocodePatchRegionSize = OldCpuMpData->MicrocodePatchRegionSize;
     CpuMpData->MicrocodePatchAddress    = OldCpuMpData->MicrocodePatchAddress;
   }
@@ -1762,10 +1725,6 @@ MpInitLibInitialize (
       (UINT32 *)(MonitorBuffer + MonitorFilterSize * Index);
   }
   //
-  // Load Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, TRUE);
-  //
   // Store BSP's MTRR setting
   //
   MtrrGetAllMtrrs (&CpuMpData->MtrrTable);
@@ -1781,6 +1740,11 @@ MpInitLibInitialize (
       //
       CollectProcessorCount (CpuMpData);
     }
+
+    //
+    // Load required microcode patches data into memory
+    //
+    LoadMicrocodePatch (CpuMpData);
   } else {
     //
     // APs have been wakeup before, just get the CPU Information
@@ -1788,7 +1752,6 @@ MpInitLibInitialize (
     //
     CpuMpData->CpuCount  = OldCpuMpData->CpuCount;
     CpuMpData->BspNumber = OldCpuMpData->BspNumber;
-    CpuMpData->InitFlag  = ApInitReconfig;
     CpuMpData->CpuInfoInHob = OldCpuMpData->CpuInfoInHob;
     CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
     for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
@@ -1797,21 +1760,24 @@ MpInitLibInitialize (
       CpuMpData->CpuData[Index].ApFunction = 0;
       CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS));
     }
-    if (MaxLogicalProcessorNumber > 1) {
-      //
-      // Wakeup APs to do some AP initialize sync
-      //
-      WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
-      //
-      // Wait for all APs finished initialization
-      //
-      while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
-        CpuPause ();
-      }
-      CpuMpData->InitFlag = ApInitDone;
-      for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
-        SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
-      }
+  }
+
+  //
+  // Detect and apply Microcode on BSP
+  //
+  MicrocodeDetect (CpuMpData, TRUE);
+
+  //
+  // Wakeup APs to do some AP initialize sync (Microcode & MTRR)
+  //
+  if (CpuMpData->CpuCount > 1) {
+    WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
+    while (CpuMpData->FinishedCount < (CpuMpData->CpuCount - 1)) {
+      CpuPause ();
+    }
+
+    for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
+      SetApState (&CpuMpData->CpuData[Index], CpuStateIdle);
     }
   }
 
-- 
2.12.0.windows.1


  parent reply	other threads:[~2019-12-25  7:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-25  7:58 [PATCH v2 0/6] Microcode related optimizations Wu, Hao A
2019-12-25  7:58 ` [PATCH v2 1/6] UefiCpuPkg/MpInitLib: Collect processors' CPUID & Platform ID info Wu, Hao A
2019-12-25  7:58 ` Wu, Hao A [this message]
2019-12-25  7:58 ` [PATCH v2 3/6] UefiCpuPkg: Add definitions for EDKII microcode patch HOB Wu, Hao A
2019-12-25  8:50   ` Ni, Ray
2019-12-25  7:58 ` [PATCH v2 4/6] UefiCpuPkg/MpInitLib: Produce " Wu, Hao A
2019-12-25  7:58 ` [PATCH v2 5/6] UefiCpuPkg/MpInitLib: Relocate microcode patch fields in CPU_MP_DATA Wu, Hao A
2019-12-25  8:52   ` Ni, Ray
2019-12-25  7:58 ` [PATCH v2 6/6] UefiCpuPkg/MpInitLib: Remove redundant microcode " Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191225075814.8372-3-hao.a.wu@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox