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
toggle quoted messageShow quoted text
-----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] -=-=-=-=-=-=
|