Reviewed-by: Ray Ni <ray...@intel.com> > -----Original Message----- > From: Chiu, Chasel <chasel.c...@intel.com> > Sent: Tuesday, April 4, 2023 2:34 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Zeng, Star <star.z...@intel.com>; Ni, > Ray <ray...@intel.com>; Kuo, Ted <ted....@intel.com> > Subject: [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.desim...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Ray Ni <ray...@intel.com> > Signed-off-by: Chasel Chiu <chasel.c...@intel.com> > Reviewed-by: Ted Kuo <ted....@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_ID > > diff --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 (#102458): https://edk2.groups.io/g/devel/message/102458 Mute This Topic: https://groups.io/mt/98042607/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
