[edk2-devel] [PATCH v3] IntelFsp2Pkg: LoadMicrocodeDefault() causing unnecessary delay.
Chiu, Chasel
chasel.chiu at intel.com
Mon Apr 3 18:30:05 UTC 2023
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.ni at intel.com>
> Sent: Sunday, April 2, 2023 11:31 PM
> To: devel at edk2.groups.io; Chiu, Chasel <chasel.chiu at intel.com>
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Zeng, Star
> <star.zeng at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Chiu,
> > Chasel
> > Sent: Saturday, April 1, 2023 6:57 AM
> > To: devel at edk2.groups.io
> > Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L
> > <nathaniel.l.desimone at intel.com>; Zeng, Star <star.zeng at intel.com>;
> > Ni, Ray <ray.ni at 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.desimone at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Cc: Ray Ni <ray.ni at intel.com>
> > Signed-off-by: Chasel Chiu <chasel.chiu at 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+owner at edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [chasel.chiu at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list