SDM requires one core only needs to load uCode once.
Also load uCode once can save some time.

This change enables this solution.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 351975e2a2..4586e037d2 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -61,6 +61,7 @@ MicrocodeDetect (
   VOID                                    *MicrocodeData;
   MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;
   UINT32                                  ProcessorFlags;
+  UINT32                                  ThreadId;
 
   if (CpuMpData->MicrocodePatchRegionSize == 0) {
     //
@@ -77,6 +78,14 @@ MicrocodeDetect (
     return;
   }
 
+  GetProcessorLocationByApicId (GetInitialApicId(), NULL, NULL, &ThreadId);
+  if (ThreadId != 0) {
+    //
+    // Skip loading microcode if it is not the first thread in one core.
+    //
+    return;
+  }
+
   ExtendedTableLength = 0;
   //
   // Here data of CPUID leafs have not been collected into context buffer, so
-- 
2.15.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

I've got two comments:

On 07/11/18 13:07, Eric Dong wrote:
> SDM requires one core only needs to load uCode once.

(1) This is a very confusing typo ("one core only").

I totally missed the point until I re-read the cover letter of the
patch. In the cover letter, you say:

> 3. Only apply uCode in one thread of a core when hyper threading is
> enabled.

So please fix the commit message to say, "The SDM requires only one
*thread per core* to load the microcode", or something similar.

(You don't need to add the emphasis, I'm only adding it here to make
myself clear.)

> Also load uCode once can save some time.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 351975e2a2..4586e037d2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -61,6 +61,7 @@ MicrocodeDetect (
>    VOID                                    *MicrocodeData;
>    MSR_IA32_PLATFORM_ID_REGISTER           PlatformIdMsr;
>    UINT32                                  ProcessorFlags;
> +  UINT32                                  ThreadId;
>  
>    if (CpuMpData->MicrocodePatchRegionSize == 0) {
>      //
> @@ -77,6 +78,14 @@ MicrocodeDetect (
>      return;
>    }
>  
> +  GetProcessorLocationByApicId (GetInitialApicId(), NULL, NULL, &ThreadId);

(2) A space character should be inserted after "GetInitialApicId".

With those fixed:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> +  if (ThreadId != 0) {
> +    //
> +    // Skip loading microcode if it is not the first thread in one core.
> +    //
> +    return;
> +  }
> +
>    ExtendedTableLength = 0;
>    //
>    // Here data of CPUID leafs have not been collected into context buffer, so
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel