Hi Ray,
Yes, the step 3 will be redundant after adding the check for microcode already loaded scenario in earlier point. I will remove it and sent V4 patch. Thanks, Chasel > -----Original Message----- > From: Ni, Ray <ray...@intel.com> > Sent: Sunday, April 2, 2023 11:31 PM > To: devel@edk2.groups.io; Chiu, Chasel <chasel.c...@intel.com> > Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Zeng, Star > <star.z...@intel.com> > Subject: RE: [edk2-devel] [PATCH v3] IntelFsp2Pkg: LoadMicrocodeDefault() > causing unnecessary delay. > > Chasel, > With your changes, the flow is like: > 1. check revision of loaded microcode, go to Done if it's not zero 2. find > first > matching microcode 3. check revision of loaded microcode, go to Done if it > equals to the matching one 4. load the matching microcode > Done: return fail/success depending on whether the revision of loaded > microcode is zero > > I guess the step #3 is unnecessary because step #1 guarantees that step #3 > would not go to Done. > Can you please confirm? > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, > > Chasel > > Sent: Saturday, April 1, 2023 6:57 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> > > Subject: [edk2-devel] [PATCH v3] 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. > > > > 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> > > --- > > IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm | 26 > > ++++++++++++++++++++++---- > > IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 26 > > ++++++++++++++++++++++---- > > 2 files changed, 44 insertions(+), 8 deletions(-) > > > > diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm > > b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm > > index 2cff8b3643..79f2a20a2c 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 > > > > @@ -450,7 +466,7 @@ LoadCheck: > > > > > > ; Verify this microcode update is not already loaded > > > > cmp dword [esi + MicrocodeHdr.MicrocodeHdrRevision], edx > > > > - je Continue > > > > + je Done ; if already one version microcode loaded, go to done > > > > > > > > LoadMicrocode: > > > > ; EAX contains the linear address of the start of the Update Data > > > > @@ -465,10 +481,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..3e40678f47 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 > > > > @@ -291,7 +307,7 @@ LoadCheck: > > > > > > ; Verify this microcode update is not already loaded > > > > cmp dword [esi + MicrocodeHdr.MicrocodeHdrRevision], edx > > > > - je Continue > > > > + je Done ; if already one version microcode loaded, go to done > > > > > > > > LoadMicrocode: > > > > ; EAX contains the linear address of the start of the Update Data > > > > @@ -306,10 +322,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 (#102334): > > https://edk2.groups.io/g/devel/message/102334 > > Mute This Topic: https://groups.io/mt/97984449/1777047 > > Group Owner: devel+ow...@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub > > [chasel.c...@intel.com] -=-=-=-=-=-= > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102435): https://edk2.groups.io/g/devel/message/102435 Mute This Topic: https://groups.io/mt/97984449/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
