Re: [edk2-devel][PATCH v1] IntelFsp2Pkg: Improvement of supporting null UPD pointer in FSP-T

["Chiu, Chasel" <chasel.chiu@intel.com>]

Thanks for cleaner implementation Ted!

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



> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kuo,
> Ted
> Sent: Wednesday, November 9, 2022 3:31 AM
> 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>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>
> Subject: [edk2-devel][PATCH v1] IntelFsp2Pkg: Improvement of supporting
> null UPD pointer in FSP-T
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=4114
> 
> 1.Use xmm5 slot 1 and xmm6 slot 3 to save ucode status and UPD pointer
>   respectively in TempRamInitApi in IA32 FspSecCoreT.
> 2.Correct inappropriate description in the return value of
>   AsmGetFspInfoHeader.
> 3.Replace hardcoded offset value 0x1C with
> FSP_HEADER_IMGBASE_OFFSET in
>   FspHeler.nasm.
> 
> 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>
> Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
> Signed-off-by: Ted Kuo <ted.kuo@intel.com>
> ---
>  .../FspSecCore/Ia32/FspApiEntryT.nasm         | 17 +++--
>  IntelFsp2Pkg/FspSecCore/Ia32/FspHelper.nasm   |  4 +-
>  .../FspSecCore/Ia32/SaveRestoreSseNasm.inc    | 74 ++++++++++---------
>  IntelFsp2Pkg/FspSecCore/SecFsp.h              |  2 +-
>  IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm    |  4 +-
>  5 files changed, 55 insertions(+), 46 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 73821ad22a..2cff8b3643 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -594,37 +594,38 @@ ASM_PFX(TempRamInitApi):
>    SAVE_EAX   SAVE_EDX +  CALL_EBP  ASM_PFX(LoadUpdPointerToECX) ;
> ECX for UPD param+  SAVE_ECX                               ; save UPD param to slot
> 3 in xmm6+   ;   ; Sec Platform Init   ;-  CALL_EBP
> ASM_PFX(LoadUpdPointerToECX) ; ECX for UPD param   CALL_MMX
> ASM_PFX(SecPlatformInit)   cmp       eax, 0   jnz       TempRamInitExit    ;
> Load microcode   LOAD_ESP-  CALL_EBP  ASM_PFX(LoadUpdPointerToECX) ;
> ECX for UPD param+  LOAD_ECX   CALL_MMX
> ASM_PFX(LoadMicrocodeDefault)-  SXMMN     xmm6, 3, eax            ;Save
> microcode return status in ECX-SLOT 3 in xmm6.+
> SAVE_UCODE_STATUS                 ; Save microcode return status in slot 1 in
> xmm5.   ;@note If return value eax is not 0, microcode did not load, but
> continue and attempt to boot.    ; Call Sec CAR Init   LOAD_ESP-  CALL_EBP
> ASM_PFX(LoadUpdPointerToECX) ; ECX for UPD param+  LOAD_ECX
> CALL_MMX  ASM_PFX(SecCarInit)   cmp       eax, 0   jnz       TempRamInitExit
> LOAD_ESP-  CALL_EBP  ASM_PFX(LoadUpdPointerToECX) ; ECX for UPD
> param-  mov       edi, ecx                     ; Save UPD param to EDI for later code
> use+  LOAD_ECX+  mov       edi, ecx                ; Save UPD param to EDI for
> later code use   CALL_MMX  ASM_PFX(EstablishStackFsp)   cmp       eax, 0
> jnz       TempRamInitExit -  LXMMN     xmm6, eax, 3  ;Restore microcode
> status if no CAR init error from ECX-SLOT 3 in xmm6.-  SXMMN     xmm6, 3,
> edi  ;Save FSP-T UPD parameter pointer in ECX-SLOT 3 in xmm6.+
> LOAD_UCODE_STATUS                 ; Restore microcode status if no CAR init
> error from slot 1 in xmm5.  TempRamInitExit:   mov       bl, al                  ;
> save al data in bldiff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspHelper.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspHelper.nasm
> index e3e1945473..3c63f6eea5 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspHelper.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspHelper.nasm
> @@ -7,6 +7,8 @@
>       SECTION .text +FSP_HEADER_IMGBASE_OFFSET    EQU   1Ch+ global
> ASM_PFX(FspInfoHeaderRelativeOff) ASM_PFX(FspInfoHeaderRelativeOff):
> DD    0x12345678               ; This value must be patched by the build
> script@@ -14,7 +16,7 @@ ASM_PFX(FspInfoHeaderRelativeOff):
>  global ASM_PFX(AsmGetFspBaseAddress)
> ASM_PFX(AsmGetFspBaseAddress):    call
> ASM_PFX(AsmGetFspInfoHeader)-   add   eax, 0x1C+   add   eax,
> FSP_HEADER_IMGBASE_OFFSET    mov   eax, dword [eax]    ret diff --git
> a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> index 4c321cbece..a222f2e376 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------ ;-; Copyright (c)
> 2015 - 2019, Intel Corporation. All rights reserved.<BR>+; Copyright (c)
> 2015 - 2022, Intel Corporation. All rights reserved.<BR> ; SPDX-License-
> Identifier: BSD-2-Clause-Patent ; ; Abstract:@@ -16,21 +16,21 @@
>  ; ; Define SSE macros using SSE 4.1 instructions ; args 1:XMM, 2:IDX,
> 3:REG-%macro SXMMN           3+%macro SXMMN    3              pinsrd  %1, %3,
> (%2 & 3)              %endmacro  ; ;args 1:XMM, 2:REG, 3:IDX ;-%macro
> LXMMN           3+%macro LXMMN    3              pextrd  %2, %1, (%3 & 3)
> %endmacro %else ; ; Define SSE macros using SSE 2 instructions ; args
> 1:XMM, 2:IDX, 3:REG-%macro SXMMN       3+%macro SXMMN    3
> pinsrw  %1, %3, (%2 & 3) * 2              ror     %3, 16              pinsrw  %1, %3,
> (%2 & 3) * 2 + 1@@ -38,19 +38,19 @@
>               %endmacro  ;-;args 1:XMM, 2:REG,  3:IDX+;args 1:XMM, 2:REG,
> 3:IDX ; %macro LXMMN    3-             pshufd  %1, %1,  ((0E4E4E4h >> (%3 * 2))
> & 0FFh)+             pshufd  %1, %1, ((0E4E4E4h >> (%3 * 2))  & 0FFh)
> movd    %2, %1-             pshufd  %1, %1,  ((0E4E4E4h >> (%3 * 2 + (%3 & 1) *
> 4)) & 0FFh)+             pshufd  %1, %1, ((0E4E4E4h >> (%3 * 2 + (%3 & 1) * 4))
> & 0FFh)              %endmacro %endif  ;-; XMM7 to save/restore EBP, EBX, ESI,
> EDI+; XMM7 to save/restore EBP - slot 0, EBX - slot 1, ESI - slot 2, EDI - slot
> 3 ;-%macro SAVE_REGS   0+%macro SAVE_REGS    0   SXMMN      xmm7, 0,
> ebp   SXMMN      xmm7, 1, ebx   SXMMN      xmm7, 2, esi@@ -67,63 +67,67
> @@
>               %endmacro  ;-; XMM6 to save/restore EAX, EDX, ECX, ESP+; XMM6
> to save/restore ESP - slot 0, EAX - slot 1, EDX - slot 2, ECX - slot 3 ;-%macro
> LOAD_EAX     0+%macro LOAD_ESP    0+  movd       esp,  xmm6+
> %endmacro++%macro SAVE_ESP    0+  SXMMN      xmm6, 0, esp+
> %endmacro++%macro LOAD_EAX    0   LXMMN      xmm6, eax, 1
> %endmacro -%macro SAVE_EAX     0+%macro SAVE_EAX    0   SXMMN
> xmm6, 1, eax              %endmacro -%macro LOAD_EDX     0+%macro
> LOAD_EDX    0   LXMMN      xmm6, edx, 2              %endmacro -%macro
> SAVE_EDX     0+%macro SAVE_EDX    0   SXMMN      xmm6, 2, edx
> %endmacro -%macro SAVE_ECX     0-  SXMMN      xmm6, 3, ecx-
> %endmacro--%macro LOAD_ECX     0+%macro LOAD_ECX    0   LXMMN
> xmm6, ecx, 3              %endmacro -%macro SAVE_ESP     0-  SXMMN
> xmm6, 0, esp+%macro SAVE_ECX    0+  SXMMN      xmm6, 3, ecx
> %endmacro -%macro LOAD_ESP     0-  movd       esp,  xmm6-
> %endmacro ;-; XMM5 for calling stack+; XMM5 slot 0 for calling stack ; arg
> 1:Entry %macro CALL_XMM       1              mov     esi, %%ReturnAddress-
> pslldq  xmm5, 4-%ifdef USE_SSE41_FLAG-             pinsrd  xmm5, esi, 0-
> %else-             pinsrw  xmm5, esi, 0-             ror     esi,  16-             pinsrw
> xmm5, esi, 1-%endif+             SXMMN   xmm5, 0, esi              mov     esi,  %1
> jmp     esi %%ReturnAddress:              %endmacro  %macro RET_XMM       0-
> movd    esi, xmm5-             psrldq  xmm5, 4+             LXMMN   xmm5, esi, 0
> jmp     esi              %endmacro +;+; XMM5 slot 1 for uCode status+;+%macro
> LOAD_UCODE_STATUS    0+  LXMMN      xmm5, eax, 1+
> %endmacro++%macro SAVE_UCODE_STATUS    0+  SXMMN      xmm5, 1,
> eax+             %endmacro+ %macro ENABLE_SSE   0             ;             ; Initialize
> floating point unitsdiff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.h
> b/IntelFsp2Pkg/FspSecCore/SecFsp.h
> index d7a5976c12..693af29f20 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFsp.h
> +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.h
> @@ -79,7 +79,7 @@ AsmGetFspBaseAddress (
>  /**   This interface gets FspInfoHeader pointer -  @return   FSP binary base
> address.+  @return   FSP info header.  **/ UINTNdiff --git
> a/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm
> index 122fa1d174..71624a3aad 100644
> --- a/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm
> @@ -7,10 +7,12 @@
>      DEFAULT  REL     SECTION .text +FSP_HEADER_IMGBASE_OFFSET    EQU
> 1Ch+ global ASM_PFX(AsmGetFspBaseAddress)
> ASM_PFX(AsmGetFspBaseAddress):    call
> ASM_PFX(AsmGetFspInfoHeader)-   add   rax, 0x1C+   add   rax,
> FSP_HEADER_IMGBASE_OFFSET    mov   eax, [rax]    ret --
> 2.35.3.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#96131):
> https://edk2.groups.io/g/devel/message/96131
> Mute This Topic: https://groups.io/mt/94910671/1777047
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com]
> -=-=-=-=-=-=
>