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


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@intel.com>
---
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@intel.com]
-=-=-=-=-=-=

Join devel@edk2.groups.io to automatically receive all group messages.