[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Remove the duplicate loading of microcode in DXE.

[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Remove the duplicate loading of microcode in DXE.

[Yuanhao Xie]

The microcode loading during Mp initialization of the DXE stage can be
removed regardless of bit mode.

Cc: Ray Ni ray.ni@intel.com
Cc: Eric Dong eric.dong@intel.com
Cc: Rahul Kumar rahul1.kumar@intel.com
Cc: Tom Lendacky thomas.lendacky@amd.com
Cc: Laszlo Ersek lersek@redhat.com
Signed-off-by: Yuanhao Xie yuanhao.xie@intel.com
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 57 +++++++++++++++++++---------
 1 file changed, 40 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 9a6ec5db5c..1c68c803d9 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -433,6 +433,26 @@ ApFuncEnableX2Apic (
   SetApicMode (LOCAL_APIC_MODE_X2APIC);
 }
 
+/**
+  Sync BSP's MTRR table to AP during waking upAp
+  @param[in, out] Buffer  Pointer to private data buffer.
+**/
+VOID
+EFIAPI
+ApMtrrSync (
+  IN OUT VOID  *Buffer
+  )
+{
+  CPU_MP_DATA  *CpuMpData;
+
+  CpuMpData = (CPU_MP_DATA *)Buffer;
+
+  //
+  // Sync BSP's MTRR table to AP
+  //
+  MtrrSetAllMtrrs (&CpuMpData->MtrrTable);
+}
+
 /**
   Do sync on APs.
 
@@ -2162,6 +2182,23 @@ MpInitLibInitialize (
       //
       CollectProcessorCount (CpuMpData);
     }
+
+    if (!GetMicrocodePatchInfoFromHob (
+           &CpuMpData->MicrocodePatchAddress,
+           &CpuMpData->MicrocodePatchRegionSize
+           ))
+    {
+      //
+      // The microcode patch information cache HOB does not exist, which means
+      // the microcode patches data has not been loaded into memory yet
+      //
+      ShadowMicrocodeUpdatePatch (CpuMpData);
+    }
+
+    //
+    // Detect and apply Microcode on BSP
+    //
+    MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   } else {
     //
     // APs have been wakeup before, just get the CPU Information
@@ -2224,22 +2261,6 @@ MpInitLibInitialize (
     }
   }
 
-  if (!GetMicrocodePatchInfoFromHob (
-         &CpuMpData->MicrocodePatchAddress,
-         &CpuMpData->MicrocodePatchRegionSize
-         ))
-  {
-    //
-    // The microcode patch information cache HOB does not exist, which means
-    // the microcode patches data has not been loaded into memory yet
-    //
-    ShadowMicrocodeUpdatePatch (CpuMpData);
-  }
-
-  //
-  // Detect and apply Microcode on BSP
-  //
-  MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);
   //
   // Store BSP's MTRR setting
   //
@@ -2256,9 +2277,11 @@ MpInitLibInitialize (
       // in DXE.
       //
       CpuMpData->InitFlag = ApInitReconfig;
+      WakeUpAP (CpuMpData, TRUE, 0, ApMtrrSync, CpuMpData, TRUE);
+    } else {
+      WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
     }
 
-    WakeUpAP (CpuMpData, TRUE, 0, ApInitializeSync, CpuMpData, TRUE);
     //
     // Wait for all APs finished initialization
     //
-- 
2.39.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111443): https://edk2.groups.io/g/devel/message/111443
Mute This Topic: https://groups.io/mt/102701619/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Remove the duplicate loading of microcode in DXE.

[Ni, Ray]

> +
> +    if (!GetMicrocodePatchInfoFromHob (
> +           &CpuMpData->MicrocodePatchAddress,
> +           &CpuMpData->MicrocodePatchRegionSize
> +           ))
> +    {
> +      //
> +      // The microcode patch information cache HOB does not exist, which
> means
> +      // the microcode patches data has not been loaded into memory yet
> +      //
> +      ShadowMicrocodeUpdatePatch (CpuMpData);
> +    }

1. The microcode HOB is produced by MpInitLib PEI instance.
So the above code running in PEI phase cannot get the HOB.
We could simply delete it.

The microcode HOB has multiple consumers, such as some code that
measures the microcode binaries.
So, even MpInitLib DXE instance doesn't consume this HOB any more,
the producing is still needed.



> +
> +    //
> +    // Detect and apply Microcode on BSP
> +    //
> +    MicrocodeDetect (CpuMpData, CpuMpData->BspNumber);

2. Loading microcode in BSP is good. Can you move this function call to together with WakeUpAp (ApInitializeSync)?
I think it could make the code more readable.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111498): https://edk2.groups.io/g/devel/message/111498
Mute This Topic: https://groups.io/mt/102701619/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-