Re: [edk2-devel] [PATCH v4] IntelFsp2Pkg: LoadMicrocodeDefault() causing unnecessary delay.

Re: [edk2-devel] [PATCH v4] IntelFsp2Pkg: LoadMicrocodeDefault() causing unnecessary delay.

[Chiu, Chasel]

Patch merged:
https://github.com/tianocore/edk2/commit/7df447930c42addaf2cc0d32916141d95ded677e

Thanks,
Chasel


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
> Sent: Monday, April 3, 2023 11:34 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kuo, Ted <ted.kuo@intel.com>
> Subject: [edk2-devel] [PATCH v4] IntelFsp2Pkg: LoadMicrocodeDefault() causing
> unnecessary delay.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4391
> 
> FSP should support the scenario that CPU microcode already loaded before
> calling LoadMicrocodeDefault(), in this case it should return directly without
> spending more time.
> Also the LoadMicrocodeDefault() should only attempt to load one version of the
> microcode for current CPU and return directly without parsing rest of the
> microcode in FV.
> 
> This patch also removed unnecessary LoadCheck code after supporting CPU
> microcode already loaded scenario.
> 
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> Reviewed-by: Ted Kuo <ted.kuo@intel.com>
> ---
>  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm | 46
> ++++++++++++++++++++++++----------------------
>  IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm  | 45
> ++++++++++++++++++++++++---------------------
>  2 files changed, 48 insertions(+), 43 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 2cff8b3643..900126b93b 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -245,6 +245,22 @@ ASM_PFX(LoadMicrocodeDefault):
>     cmp    esp, 0    jz     ParamError +   ;+   ; If microcode already loaded before this
> function, exit this function with SUCCESS.+   ;+   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID+   xor   eax, eax               ; Clear EAX+   xor   edx,
> edx               ; Clear EDX+   wrmsr                        ; Load 0 to MSR at 8Bh++   mov
> eax, 1+   cpuid+   mov   ecx, MSR_IA32_BIOS_SIGN_ID+   rdmsr                         ;
> Get current microcode signature+   xor   eax, eax+   test  edx, edx+   jnz   Exit2+    ;
> skip loading Microcode if the MicrocodeCodeSize is zero    ; and report error if
> size is less than 2k    ; first check UPD header revision@@ -330,7 +346,7 @@
> CheckMainHeader:
>     cmp   ebx, dword [esi + MicrocodeHdr.MicrocodeHdrProcessor]    jne
> LoadMicrocodeDefault1    test  edx, dword [esi +
> MicrocodeHdr.MicrocodeHdrFlags ]-   jnz   LoadCheck  ; Jif signature and
> platform ID match+   jnz   LoadMicrocode  ; Jif signature and platform ID match
> LoadMicrocodeDefault1:    ; Check if extended header exists@@ -363,7 +379,7
> @@ CheckExtSig:
>     cmp   dword [edi + ExtSig.ExtSigProcessor], ebx    jne
> LoadMicrocodeDefault2    test  dword [edi + ExtSig.ExtSigFlags], edx-   jnz
> LoadCheck      ; Jif signature and platform ID match+   jnz   LoadMicrocode      ; Jif
> signature and platform ID match LoadMicrocodeDefault2:    ; Check if any more
> extended signatures exist    add   edi, ExtSig.size@@ -435,23 +451,7 @@
> LoadMicrocodeDefault4:
>     ; Is valid Microcode start point ?    cmp   dword [esi +
> MicrocodeHdr.MicrocodeHdrVersion], 0ffffffffh    jz    Done--LoadCheck:-   ; Get
> the revision of the current microcode update loaded-   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID-   xor   eax, eax               ; Clear EAX-   xor   edx,
> edx               ; Clear EDX-   wrmsr                        ; Load 0 to MSR at 8Bh--   mov   eax,
> 1-   cpuid-   mov   ecx, MSR_IA32_BIOS_SIGN_ID-   rdmsr                        ; Get
> current microcode signature--   ; Verify this microcode update is not already
> loaded-   cmp   dword [esi + MicrocodeHdr.MicrocodeHdrRevision], edx-   je
> Continue-+   jmp   CheckMainHeader LoadMicrocode:    ; EAX contains the linear
> address of the start of the Update Data    ; EDX contains zero@@ -465,10
> +465,12 @@ LoadMicrocode:
>     mov   eax, 1    cpuid -Continue:-   jmp   NextMicrocode- Done:+   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID+   xor   eax, eax               ; Clear EAX+   xor   edx,
> edx               ; Clear EDX+   wrmsr                        ; Load 0 to MSR at 8Bh+    mov
> eax, 1    cpuid    mov   ecx, MSR_IA32_BIOS_SIGN_IDdiff --git
> a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> index b32fa32a89..698bb063a7 100644
> --- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> @@ -141,6 +141,22 @@ ASM_PFX(LoadMicrocodeDefault):
>     jz     ParamError    mov    rsp, rcx +   ;+   ; If microcode already loaded before
> this function, exit this function with SUCCESS.+   ;+   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID+   xor   eax, eax               ; Clear EAX+   xor   edx,
> edx               ; Clear EDX+   wrmsr                        ; Load 0 to MSR at 8Bh++   mov
> eax, 1+   cpuid+   mov   ecx, MSR_IA32_BIOS_SIGN_ID+   rdmsr                         ;
> Get current microcode signature+   xor   rax, rax+   test  edx, edx+   jnz   Exit2+    ;
> skip loading Microcode if the MicrocodeCodeSize is zero    ; and report error if
> size is less than 2k    ; first check UPD header revision@@ -198,7 +214,7 @@
> CheckMainHeader:
>     cmp   ebx, dword [esi + MicrocodeHdr.MicrocodeHdrProcessor]    jne
> LoadMicrocodeDefault1    test  edx, dword [esi +
> MicrocodeHdr.MicrocodeHdrFlags ]-   jnz   LoadCheck  ; Jif signature and
> platform ID match+   jnz   LoadMicrocode  ; Jif signature and platform ID match
> LoadMicrocodeDefault1:    ; Check if extended header exists@@ -231,7 +247,7
> @@ CheckExtSig:
>     cmp   dword [edi + ExtSig.ExtSigProcessor], ebx    jne
> LoadMicrocodeDefault2    test  dword [edi + ExtSig.ExtSigFlags], edx-   jnz
> LoadCheck      ; Jif signature and platform ID match+   jnz   LoadMicrocode      ; Jif
> signature and platform ID match LoadMicrocodeDefault2:    ; Check if any more
> extended signatures exist    add   edi, ExtSig.size@@ -276,22 +292,7 @@
> LoadMicrocodeDefault4:
>     ; Is valid Microcode start point ?    cmp   dword [esi +
> MicrocodeHdr.MicrocodeHdrVersion], 0ffffffffh    jz    Done--LoadCheck:-   ; Get
> the revision of the current microcode update loaded-   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID-   xor   eax, eax               ; Clear EAX-   xor   edx,
> edx               ; Clear EDX-   wrmsr                        ; Load 0 to MSR at 8Bh--   mov   eax,
> 1-   cpuid-   mov   ecx, MSR_IA32_BIOS_SIGN_ID-   rdmsr                        ; Get
> current microcode signature--   ; Verify this microcode update is not already
> loaded-   cmp   dword [esi + MicrocodeHdr.MicrocodeHdrRevision], edx-   je
> Continue+   jmp   CheckMainHeader  LoadMicrocode:    ; EAX contains the linear
> address of the start of the Update Data@@ -306,10 +307,12 @@
> LoadMicrocode:
>     mov   eax, 1    cpuid -Continue:-   jmp   NextMicrocode- Done:+   mov   ecx,
> MSR_IA32_BIOS_SIGN_ID+   xor   eax, eax               ; Clear EAX+   xor   edx,
> edx               ; Clear EDX+   wrmsr                        ; Load 0 to MSR at 8Bh+    mov
> eax, 1    cpuid    mov   ecx, MSR_IA32_BIOS_SIGN_ID--
> 2.35.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#102436): https://edk2.groups.io/g/devel/message/102436
> Mute This Topic: https://groups.io/mt/98042607/1777047
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com] -=-
> =-=-=-=-=
>