[edk2-devel] [PATCH v3] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must follow X64 Calling Convention
Chiu, Chasel
chasel.chiu at intel.com
Tue May 17 00:27:17 UTC 2022
Thanks for clarification!
In this case, please use "mov rsp, rcx" to support 64bit addressing.
Thanks,
Chasel
> -----Original Message-----
> From: Duggapu, Chinni B <chinni.b.duggapu at intel.com>
> Sent: Tuesday, May 17, 2022 12:33 AM
> To: Chiu, Chasel <chasel.chiu at intel.com>; devel at edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Zeng, Star
> <star.zeng at intel.com>; S, Ashraf Ali <ashraf.ali.s at intel.com>
> Subject: RE: [PATCH v3] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must
> follow X64 Calling Convention
>
> HI Chasel,
> Yes, we don't need to modify esp for LoadMicrocodeDefault. However, this
> function does couple of MSR Accesses in b/w that would lead to modify RCX
> anyway.
> So, if not RSP, we need to use different register to save RCX and consume in
> the whole function.
>
> That's why I have not changed the usage of RSP to hold the input parameter.
>
>
>
> Thanks,
> Chinni.
>
> -----Original Message-----
> From: Chiu, Chasel <chasel.chiu at intel.com>
> Sent: Monday, May 16, 2022 5:38 PM
> To: Duggapu, Chinni B <chinni.b.duggapu at intel.com>;
> devel at edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Zeng, Star
> <star.zeng at intel.com>; S, Ashraf Ali <ashraf.ali.s at intel.com>
> Subject: RE: [PATCH v3] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must
> follow X64 Calling Convention
>
>
> Thanks for correcting format and updating patch per feedbacks!
> Just one more comment below inline and please also help to include patch
> of
> IntelFsp2WrapperPkg\Library\SecFspWrapperPlatformSecLibSample\X64\S
> ecEntry.nasm for passing API parameter by RCX.
> You might want to create a patch series:
> [1/2] IntelFsp2Pkg patch
> [2/2] IntelFsp2WrapperPkg patch
>
> Thanks,
> Chasel
>
> > -----Original Message-----
> > From: Duggapu, Chinni B <chinni.b.duggapu at intel.com>
> > Sent: Monday, May 16, 2022 6:54 PM
> > 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>; S,
> > Ashraf Ali <ashraf.ali.s at intel.com>
> > Subject: [PATCH v3] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must follow
> > X64 Calling Convention
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3926
> > This API accept one parameter using RCX and this is consumed in
> > mutiple sub functions.
> >
> > Cc: Chasel Chiu <chasel.chiu at intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> > Cc: Star Zeng <star.zeng at intel.com>
> > Cc: Ashraf Ali S <ashraf.ali.s at intel.com>
> > Signed-off-by: cbduggap <chinni.b.duggapu at intel.com>
> > ---
> > IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 26 ++++++++---------
> > .../Include/SaveRestoreSseAvxNasm.inc | 28
> +++++++++++++++++++
> > 2 files changed, 41 insertions(+), 13 deletions(-)
> >
> > diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> > b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> > index a9f5f28ed7..9504c96b81 100644
> > --- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> > +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> > @@ -114,7 +114,7 @@ endstruc
> > global ASM_PFX(LoadMicrocodeDefault)
> > ASM_PFX(LoadMicrocodeDefault): ; Inputs:- ; rsp ->
> > LoadMicrocodeParams pointer+ ; rcx -> LoadMicrocodeParams
> pointer ;
> > Register Usage: ; rsp Preserved ; All others destroyed@@ -130,10
> > +130,9 @@ ASM_PFX(LoadMicrocodeDefault):
> > cmp rsp, 0 jz ParamError- mov eax, dword [rsp + 8] ;
> > Parameter pointer- cmp eax, 0+ cmp ecx, 0 jz ParamError- mov
> > esp, eax+ mov esp, ecx
>
>
>
> I think we do not need to modify esp because now esp/rsp only containing
> return address initialized by caller.
>
>
>
> ; skip loading Microcode if the
> > MicrocodeCodeSize is zero ; and report error if size is less than 2k@@ -
> > 321,8 +320,7 @@ ASM_PFX(EstablishStackFsp):
> > ; ; Save parameter pointer in rdx ;- mov rdx, qword [rsp + 8]-+
> mov
> > rdx, rcx ; ; Enable FSP STACK ;@@ -420,7 +418,10 @@
> > ASM_PFX(TempRamInitApi):
> > ; ENABLE_SSE ENABLE_AVX-+ ;+ ; Save Input Parameter in
> YMM10+ ;+
> > SAVE_RCX ; ; Save RBP, RBX, RSI, RDI and RSP in YMM7, YMM8 and
> > YMM6 ;@@ -442,9 +443,8 @@ ASM_PFX(TempRamInitApi):
> > ; ; Check Parameter ;- mov rax, qword [rsp + 8]- cmp rax, 0-
> > mov rax, 08000000000000002h+ cmp rcx, 0+ mov rcx,
> > 08000000000000002h jz TempRamInitExit ;@@ -455,18 +455,18
> > @@ ASM_PFX(TempRamInitApi):
> > jnz TempRamInitExit ; Load microcode- LOAD_RSP+ LOAD_RCX
> > CALL_YMM ASM_PFX(LoadMicrocodeDefault) SAVE_UCODE_STATUS
> > rax ; Save microcode return status in SLOT 0 in YMM9 (upper
> > 128bits). ; @note If return value rax is not 0, microcode did not load, but
> > continue and attempt to boot. ; Call Sec CAR Init- LOAD_RSP+
> LOAD_RCX
> > CALL_YMM ASM_PFX(SecCarInit) cmp rax, 0 jnz
> TempRamInitExit
> > - LOAD_RSP+ LOAD_RCX CALL_YMM ASM_PFX(EstablishStackFsp)
> cmp
> > rax, 0 jnz TempRamInitExitdiff --git
> > a/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> > b/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> > index e8bd91669d..38c807a311 100644
> > --- a/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> > +++ b/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> > @@ -177,6 +177,30 @@
> > LXMMN xmm5, %1, 1 %endmacro +;+; Upper half of
> > YMM10 to save/restore RCX+;+;+; Save RCX to YMM10[128:191]+;
> > Modified: XMM5 and YMM10+;++%macro SAVE_RCX 0+ LYMMN
> > ymm10, xmm5, 1+ SXMMN xmm5, 0, rcx+ SYMMN
> ymm10,
> > 1, xmm5+ %endmacro++;+; Restore RCX from YMM10[128:191]+;
> > Modified: XMM5 and RCX+;++%macro LOAD_RCX 0+ LYMMN
> > ymm10, xmm5, 1+ movq rcx, xmm5+ %endmacro+ ; ;
> > YMM7[128:191] for calling stack ; arg 1:Entry@@ -231,6 +255,7 @@
> > NextAddress:
> > ; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] = 1) to
> > test ; whether the processor supports SSE instruction. ;+
> > mov r10, rcx mov rax, 1 cpuid bt rdx, 25@@ -
> > 241,6 +266,7 @@ NextAddress:
> > ; bt ecx, 19 jnc SseError+ mov rcx,
> > r10 ; ; Set OSFXSR bit (bit #9) & OSXMMEXCPT bit (bit
> > #10)@@ -258,6 +284,7 @@ NextAddress:
> > %endmacro %macro ENABLE_AVX 0+ mov r10, rcx
> > mov eax, 1 cpuid and ecx, 10000000h@@ -280,5
> +307,6
> > @@ EnableAvx:
> > xgetbv ; result in edx:eax or eax, 00000006h ;
> Set
> > XCR0 bit #1 and bit #2 to enable SSE state and AVX state xsetbv+
> > mov rcx, r10 %endmacro --
> > 2.36.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89786): https://edk2.groups.io/g/devel/message/89786
Mute This Topic: https://groups.io/mt/91136907/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