[PATCH v4 0/2] FSP_TEMP_RAM_INIT call must follow X64 Calling

[PATCH v4 0/2] FSP_TEMP_RAM_INIT call must follow X64 Calling

[cbduggap]

*** BLURB HERE ***
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3926
FSP_TEMP_RAM_INIT consume input parameter using RCX.

cbduggap (2):
  IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must follow X64 Calling
    Convention
  IntelFsp2WrapperPkg: FSP_TEMP_RAM_INIT call must follow X64 Calling
    Convention

 IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 39 ++++++++++---------
 .../Include/SaveRestoreSseAvxNasm.inc         | 28 +++++++++++++
 .../X64/SecEntry.nasm                         |  4 +-
 3 files changed, 51 insertions(+), 20 deletions(-)

-- 
2.36.0.windows.1

[PATCH v4 1/2] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must follow X64 Calling Convention

[cbduggap]

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@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Signed-off-by: cbduggap <chinni.b.duggapu@intel.com>
---
 IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 39 ++++++++++---------
 .../Include/SaveRestoreSseAvxNasm.inc         | 28 +++++++++++++
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
index a9f5f28ed7..22dbea1fed 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    rcx, 0
    jz     ParamError
-   mov    esp, eax
+   mov    rsp, rcx
 
    ; skip loading Microcode if the MicrocodeCodeSize is zero
    ; and report error if size is less than 2k
@@ -144,14 +143,14 @@ ASM_PFX(LoadMicrocodeDefault):
    jne    ParamError
 
    ; UPD structure is compliant with FSP spec 2.4
-   mov    eax, dword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeSize]
-   cmp    eax, 0
+   mov    rax, qword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeSize]
+   cmp    rax, 0
    jz     Exit2
-   cmp    eax, 0800h
+   cmp    rax, 0800h
    jl     ParamError
 
-   mov    esi, dword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeAddr]
-   cmp    esi, 0
+   mov    rsi, qword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeAddr]
+   cmp    rsi, 0
    jnz    CheckMainHeader
 
 ParamError:
@@ -256,7 +255,8 @@ CheckAddress:
    ; UPD structure is compliant with FSP spec 2.4
    ; Is automatic size detection ?
    mov   rax, qword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeSize]
-   cmp   rax, 0ffffffffffffffffh
+   cmp   rcx, 0ffffffffffffffffh
+   cmp   rax, rcx
    jz    LoadMicrocodeDefault4
 
    ; Address >= microcode region address + microcode region size?
@@ -321,8 +321,7 @@ ASM_PFX(EstablishStackFsp):
   ;
   ; Save parameter pointer in rdx
   ;
-  mov       rdx, qword [rsp + 8]
-
+  mov       rdx, rcx
   ;
   ; Enable FSP STACK
   ;
@@ -420,7 +419,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 +444,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 +456,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       TempRamInitExit
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
             %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

[PATCH v4 2/2] IntelFsp2WrapperPkg: FSP_TEMP_RAM_INIT call must follow X64 Calling Convention

[cbduggap]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3926
Pass Input parameters using RCX.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Signed-off-by: cbduggap <chinni.b.duggapu@intel.com>
---
 .../SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm
index dbbf63336e..065d80d0e2 100644
--- a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm
+++ b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm
@@ -130,6 +130,9 @@ FspHeaderFound:
   mov     eax, dword [edi + FSP_HEADER_IMAGEBASE_OFFSET]
   add     eax, dword [edi + FSP_HEADER_TEMPRAMINIT_OFFSET]
 
+  ; Pass Fsp T Udp pointer as Input parameter
+  mov     rcx, ASM_PFX(FsptUpdDataPtr)
+
   ; Setup the hardcode stack
   mov     rsp, TempRamInitStack
 
@@ -167,5 +170,4 @@ FspApiFailed:
 align 10h
 TempRamInitStack:
     DQ  TempRamInitDone
-    DQ  ASM_PFX(FsptUpdDataPtr)         ; TempRamInitParams
 
-- 
2.36.0.windows.1

Re: [PATCH v4 1/2] IntelFsp2Pkg: FSP_TEMP_RAM_INIT call must follow X64 Calling Convention

[Chiu, Chasel]

Thanks Chinni!
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>


> -----Original Message-----
> From: Duggapu, Chinni B <chinni.b.duggapu@intel.com>
> Sent: Tuesday, May 17, 2022 1:07 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; S,
> Ashraf Ali <ashraf.ali.s@intel.com>
> Subject: [PATCH v4 1/2] 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@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
> Signed-off-by: cbduggap <chinni.b.duggapu@intel.com>
> ---
>  IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 39 ++++++++++---------
>  .../Include/SaveRestoreSseAvxNasm.inc         | 28 +++++++++++++
>  2 files changed, 48 insertions(+), 19 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> index a9f5f28ed7..22dbea1fed 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    rcx, 0    jz     ParamError-   mov
> esp, eax+   mov    rsp, rcx     ; skip loading Microcode if the
> MicrocodeCodeSize is zero    ; and report error if size is less than 2k@@ -
> 144,14 +143,14 @@ ASM_PFX(LoadMicrocodeDefault):
>     jne    ParamError     ; UPD structure is compliant with FSP spec 2.4-   mov
> eax, dword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeSize]-   cmp
> eax, 0+   mov    rax, qword [rsp +
> LoadMicrocodeParamsFsp24.MicrocodeCodeSize]+   cmp    rax, 0    jz
> Exit2-   cmp    eax, 0800h+   cmp    rax, 0800h    jl     ParamError -   mov    esi,
> dword [rsp + LoadMicrocodeParamsFsp24.MicrocodeCodeAddr]-   cmp    esi,
> 0+   mov    rsi, qword [rsp +
> LoadMicrocodeParamsFsp24.MicrocodeCodeAddr]+   cmp    rsi, 0    jnz
> CheckMainHeader  ParamError:@@ -256,7 +255,8 @@ CheckAddress:
>     ; UPD structure is compliant with FSP spec 2.4    ; Is automatic size
> detection ?    mov   rax, qword [rsp +
> LoadMicrocodeParamsFsp24.MicrocodeCodeSize]-   cmp   rax,
> 0ffffffffffffffffh+   cmp   rcx, 0ffffffffffffffffh+   cmp   rax, rcx    jz
> LoadMicrocodeDefault4     ; Address >= microcode region address +
> microcode region size?@@ -321,8 +321,7 @@
> ASM_PFX(EstablishStackFsp):
>    ;   ; Save parameter pointer in rdx   ;-  mov       rdx, qword [rsp + 8]-+  mov
> rdx, rcx   ;   ; Enable FSP STACK   ;@@ -420,7 +419,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 +444,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 +456,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

Re: [PATCH v4 2/2] IntelFsp2WrapperPkg: FSP_TEMP_RAM_INIT call must follow X64 Calling Convention

[Chiu, Chasel]

Thanks Chinni!
Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>


> -----Original Message-----
> From: Duggapu, Chinni B <chinni.b.duggapu@intel.com>
> Sent: Tuesday, May 17, 2022 1:07 PM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>; S,
> Ashraf Ali <ashraf.ali.s@intel.com>
> Subject: [PATCH v4 2/2] IntelFsp2WrapperPkg: FSP_TEMP_RAM_INIT call
> must follow X64 Calling Convention
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3926
> Pass Input parameters using RCX.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
> Signed-off-by: cbduggap <chinni.b.duggapu@intel.com>
> ---
>  .../SecFspWrapperPlatformSecLibSample/X64/SecEntry.nasm       | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> index dbbf63336e..065d80d0e2 100644
> ---
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> @@ -130,6 +130,9 @@ FspHeaderFound:
>    mov     eax, dword [edi + FSP_HEADER_IMAGEBASE_OFFSET]
> 
>    add     eax, dword [edi + FSP_HEADER_TEMPRAMINIT_OFFSET]
> 
> 
> 
> +  ; Pass Fsp T Udp pointer as Input parameter
> 
> +  mov     rcx, ASM_PFX(FsptUpdDataPtr)
> 
> +
> 
>    ; Setup the hardcode stack
> 
>    mov     rsp, TempRamInitStack
> 
> 
> 
> @@ -167,5 +170,4 @@ FspApiFailed:
>  align 10h
> 
>  TempRamInitStack:
> 
>      DQ  TempRamInitDone
> 
> -    DQ  ASM_PFX(FsptUpdDataPtr)         ; TempRamInitParams
> 
> 
> 
> --
> 2.36.0.windows.1