[PATCH v2] FSP_TEMP_RAM_INIT API call must follow X64 Calling Convention


cbduggap
 

This API accept one parameter using RCX and this is consumed
in mutiple sub functions.
Signed-off-by: cbduggap <chinni.b.duggapu@...>
---
IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 21 ++++++++------
.../Include/SaveRestoreSseAvxNasm.inc | 28 +++++++++++++++++++
2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm b/IntelFsp2Pkg/F=
spSecCore/X64/FspApiEntryT.nasm
index a9f5f28ed7..cddc41125e 100644
--- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
@@ -130,10 +130,9 @@ ASM_PFX(LoadMicrocodeDefault):
=0D
cmp rsp, 0=0D
jz ParamError=0D
- mov eax, dword [rsp + 8] ; Parameter pointer=0D
- cmp eax, 0=0D
+ cmp ecx, 0=0D
jz ParamError=0D
- mov esp, eax=0D
+ mov esp, ecx=0D
=0D
; skip loading Microcode if the MicrocodeCodeSize is zero=0D
; and report error if size is less than 2k=0D
@@ -321,8 +320,7 @@ ASM_PFX(EstablishStackFsp):
;=0D
; Save parameter pointer in rdx=0D
;=0D
- mov rdx, qword [rsp + 8]=0D
-=0D
+ mov rdx, rcx=0D
;=0D
; Enable FSP STACK=0D
;=0D
@@ -420,7 +418,10 @@ ASM_PFX(TempRamInitApi):
;=0D
ENABLE_SSE=0D
ENABLE_AVX=0D
-=0D
+ ;=0D
+ ; Save Input Parameter in YMM10=0D
+ ;=0D
+ SAVE_RCX=0D
;=0D
; Save RBP, RBX, RSI, RDI and RSP in YMM7, YMM8 and YMM6=0D
;=0D
@@ -442,9 +443,8 @@ ASM_PFX(TempRamInitApi):
;=0D
; Check Parameter=0D
;=0D
- mov rax, qword [rsp + 8]=0D
- cmp rax, 0=0D
- mov rax, 08000000000000002h=0D
+ cmp rcx, 0=0D
+ mov rcx, 08000000000000002h=0D
jz TempRamInitExit=0D
=0D
;=0D
@@ -456,17 +456,20 @@ ASM_PFX(TempRamInitApi):
=0D
; Load microcode=0D
LOAD_RSP=0D
+ LOAD_RCX=0D
CALL_YMM ASM_PFX(LoadMicrocodeDefault)=0D
SAVE_UCODE_STATUS rax ; Save microcode return status in SLOT=
0 in YMM9 (upper 128bits).=0D
; @note If return value rax is not 0, microcode did not load, but contin=
ue and attempt to boot.=0D
=0D
; Call Sec CAR Init=0D
LOAD_RSP=0D
+ LOAD_RCX=0D
CALL_YMM ASM_PFX(SecCarInit)=0D
cmp rax, 0=0D
jnz TempRamInitExit=0D
=0D
LOAD_RSP=0D
+ LOAD_RCX=0D
CALL_YMM ASM_PFX(EstablishStackFsp)=0D
cmp rax, 0=0D
jnz TempRamInitExit=0D
diff --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=0D
%endmacro=0D
=0D
+;=0D
+; Upper half of YMM10 to save/restore RCX=0D
+;=0D
+;=0D
+; Save RCX to YMM10[128:191]=0D
+; Modified: XMM5 and YMM10=0D
+;=0D
+=0D
+%macro SAVE_RCX 0=0D
+ LYMMN ymm10, xmm5, 1=0D
+ SXMMN xmm5, 0, rcx=0D
+ SYMMN ymm10, 1, xmm5=0D
+ %endmacro=0D
+=0D
+;=0D
+; Restore RCX from YMM10[128:191]=0D
+; Modified: XMM5 and RCX=0D
+;=0D
+=0D
+%macro LOAD_RCX 0=0D
+ LYMMN ymm10, xmm5, 1=0D
+ movq rcx, xmm5=0D
+ %endmacro=0D
+=0D
;=0D
; YMM7[128:191] for calling stack=0D
; arg 1:Entry=0D
@@ -231,6 +255,7 @@ NextAddress:
; Use CpuId instruction (CPUID.01H:EDX.SSE[bit 25] =3D 1) to t=
est=0D
; whether the processor supports SSE instruction.=0D
;=0D
+ mov r10, rcx=0D
mov rax, 1=0D
cpuid=0D
bt rdx, 25=0D
@@ -241,6 +266,7 @@ NextAddress:
;=0D
bt ecx, 19=0D
jnc SseError=0D
+ mov rcx, r10=0D
=0D
;=0D
; Set OSFXSR bit (bit #9) & OSXMMEXCPT bit (bit #10)=0D
@@ -258,6 +284,7 @@ NextAddress:
%endmacro=0D
=0D
%macro ENABLE_AVX 0=0D
+ mov r10, rcx=0D
mov eax, 1=0D
cpuid=0D
and ecx, 10000000h=0D
@@ -280,5 +307,6 @@ EnableAvx:
xgetbv ; result in edx:eax=0D
or eax, 00000006h ; Set XCR0 bit #1 and bit #2 to enable =
SSE state and AVX state=0D
xsetbv=0D
+ mov rcx, r10=0D
%endmacro=0D
=0D
--=20
2.36.0.windows.1


Chiu, Chasel
 

Thanks Chinni! Just few comments below inline, please help to check.
Would you also help to update commit message format and include Bugzilla link for reference?

Thanks,
Chasel

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
cbduggap
Sent: Thursday, May 12, 2022 5:13 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v2] FSP_TEMP_RAM_INIT API call must follow
X64 Calling Convention

This API accept one parameter using RCX and this is consumed in mutiple
sub functions.
Signed-off-by: cbduggap <chinni.b.duggapu@...>
---
IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 21 ++++++++------
.../Include/SaveRestoreSseAvxNasm.inc | 28 +++++++++++++++++++
2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
index a9f5f28ed7..cddc41125e 100644
--- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
@@ -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

Do you have corresponding change for IntelFsp2WrapperPkg\Library\SecFspWrapperPlatformSecLibSample\X64\SecEntry.nasm to pass parameters by RCX?
Please help to update comments for LoadMicrocodeDefault as it still mentioning below old implementation:
; Inputs:
; rsp -> LoadMicrocodeParams pointer



esp, eax+ mov esp, ecx ; 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 ;@@ -456,17 +456,20
@@ ASM_PFX(TempRamInitApi):
; Load microcode LOAD_RSP+ LOAD_RCX CALL_YMM


Since we have following X64 calling convention to pass parameter by RCX, I think we do not need LOAD_RSP anymore.
Please help to check if we still need it.


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 (#89694):
https://edk2.groups.io/g/devel/message/89694
Mute This Topic: https://groups.io/mt/91054270/1777047
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@...]
-=-=-=-=-=-=