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.dugg...@intel.com> > Sent: Tuesday, May 17, 2022 12:33 AM > To: Chiu, Chasel <chasel.c...@intel.com>; devel@edk2.groups.io > Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Zeng, Star > <star.z...@intel.com>; S, Ashraf Ali <ashraf.al...@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.c...@intel.com> > Sent: Monday, May 16, 2022 5:38 PM > To: Duggapu, Chinni B <chinni.b.dugg...@intel.com>; > devel@edk2.groups.io > Cc: Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Zeng, Star > <star.z...@intel.com>; S, Ashraf Ali <ashraf.al...@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.dugg...@intel.com> > > Sent: Monday, May 16, 2022 6:54 PM > > To: devel@edk2.groups.io > > Cc: Chiu, Chasel <chasel.c...@intel.com>; Desimone, Nathaniel L > > <nathaniel.l.desim...@intel.com>; Zeng, Star <star.z...@intel.com>; S, > > Ashraf Ali <ashraf.al...@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.c...@intel.com> > > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > > Cc: Star Zeng <star.z...@intel.com> > > Cc: Ashraf Ali S <ashraf.al...@intel.com> > > Signed-off-by: cbduggap <chinni.b.dugg...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
