Re: [edk2-devel][PATCH v2 8/8] IntelFsp2WrapperPkg: SecFspWrapperPlatformSecLibSample support for X64

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

HI Ted,

Please see my comments below inline.

Thanks,
Chasel


> -----Original Message-----
> From: Kuo, Ted <ted.kuo@intel.com>
> Sent: Monday, April 4, 2022 2:23 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: [edk2-devel][PATCH v2 8/8] IntelFsp2WrapperPkg:
> SecFspWrapperPlatformSecLibSample support for X64
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3893
> 1.Added SecFspWrapperPlatformSecLibSample support for X64.
> 2.Adopted FSPT_ARCH2_UPD in SecFspWrapperPlatformSecLibSample.
> 3.Moved Fsp.h up one level to be shared across IA32 and X64.
> 
> 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: Ted Kuo <ted.kuo@intel.com>
> ---
>  .../{Ia32 => }/Fsp.h                               |   0
>  .../Ia32/Stack.nasm                                |   6 +-
>  .../SecFspWrapperPlatformSecLibSample.inf          |   7 +-
>  .../SecRamInitData.c                               |  32 ++--
>  .../X64/PeiCoreEntry.nasm                          | 149 ++++++++++++++++++
>  .../X64/SecEntry.nasm                              | 171 +++++++++++++++++++++
>  .../X64/Stack.nasm                                 |  73 +++++++++
>  7 files changed, 415 insertions(+), 23 deletions(-)  rename
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/{Ia32
> => }/Fsp.h (100%)  create mode 100644
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/P
> eiCoreEntry.nasm
>  create mode 100644
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/S
> ecEntry.nasm
>  create mode 100644
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64/S
> tack.nasm
> 
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32
> /Fsp.h
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Fsp.
> h
> similarity index 100%
> rename from
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32/F
> sp.h
> rename to
> IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Fsp.h
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32
> /Stack.nasm
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32
> /Stack.nasm
> index d7394cf286..65e9c2e895 100644
> ---
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32
> /Stack.nasm
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Ia32
> +++ /Stack.nasm
> @@ -22,7 +22,7 @@
>  global ASM_PFX(SecSwitchStack)
>  ASM_PFX(SecSwitchStack):
>      ;
> -    ; Save three register: eax, ebx, ecx
> +    ; Save four register: eax, ebx, ecx, edx
>      ;
>      push  eax
>      push  ebx
> @@ -55,7 +55,7 @@ ASM_PFX(SecSwitchStack):
>      mov   dword [eax + 12], edx
>      mov   edx, dword [esp + 16]    ; Update this function's return address
> into permanent memory
>      mov   dword [eax + 16], edx
> -    mov   esp, eax                     ; From now, esp is pointed to permanent
> memory
> +    mov   esp, eax                 ; From now, esp is pointed to permanent
> memory
> 
>      ;
>      ; Fixup the ebp point to permanent memory @@ -63,7 +63,7 @@
> ASM_PFX(SecSwitchStack):
>      mov   eax, ebp
>      sub   eax, ebx
>      add   eax, ecx
> -    mov   ebp, eax                ; From now, ebp is pointed to permanent
> memory
> +    mov   ebp, eax                 ; From now, ebp is pointed to permanent
> memory
> 
>      pop   edx
>      pop   ecx
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecF
> spWrapperPlatformSecLibSample.inf
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecF
> spWrapperPlatformSecLibSample.inf
> index 027b127724..7aa4297bcc 100644
> ---
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecF
> spWrapperPlatformSecLibSample.inf
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecF
> +++ spWrapperPlatformSecLibSample.inf
> @@ -39,13 +39,18 @@
>    SecGetPerformance.c
>    SecTempRamDone.c
>    PlatformInit.c
> +  Fsp.h
> 
>  [Sources.IA32]
> -  Ia32/Fsp.h
>    Ia32/SecEntry.nasm
>    Ia32/PeiCoreEntry.nasm
>    Ia32/Stack.nasm
> 
> +[Sources.X64]
> +  X64/SecEntry.nasm
> +  X64/PeiCoreEntry.nasm
> +  X64/Stack.nasm
> +
> 
> ##########################################################
> ######################
>  #
>  # Package Dependency Section - list of Package files that are required for
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecR
> amInitData.c
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Sec
> RamInitData.c
> index 03616cb418..4464abaca0 100644
> ---
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecR
> amInitData.c
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/Sec
> R
> +++ amInitData.c
> @@ -9,20 +9,14 @@
>  #include <Library/PcdLib.h>
>  #include <FspEas.h>
> 
> -typedef struct {
> -  UINT32    MicrocodeRegionBase;
> -  UINT32    MicrocodeRegionSize;
> -  UINT32    CodeRegionBase;
> -  UINT32    CodeRegionSize;
> -} FSPT_CORE_UPD;



Please add back CORE_UPD structure but the elements should be 64bits for new version.



> -
>  typedef struct {
>    FSP_UPD_HEADER    FspUpdHeader;
>    //
> -  // If platform does not support FSP spec 2.2 remove FSPT_ARCH_UPD
> structure.
> +  // If FSP spec version < 2.2, remove FSPT_ARCH_UPD structure.
> +  // Else If FSP spec version >= 2.2 and FSP spec version < 2.4, use
> FSPT_ARCH_UPD structure.
> +  // Else, use FSPT_ARCH2_UPD structure.
>    //
> -  FSPT_ARCH_UPD     FsptArchUpd;
> -  FSPT_CORE_UPD     FsptCoreUpd;
> +  FSPT_ARCH2_UPD    FsptArchUpd;
>  } FSPT_UPD_CORE_DATA;
> 
>  GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA
> FsptUpdDataPtr = { @@ -36,24 +30,24 @@
> GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA
> FsptUpdDataPtr = {
>        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }
>    },
>    //
> -  // If platform does not support FSP spec 2.2 remove FSPT_ARCH_UPD
> structure.
> +  // If FSP spec version < 2.2, remove FSPT_ARCH_UPD structure.
> +  // Else If FSP spec version >= 2.2 and FSP spec version < 2.4, use
> FSPT_ARCH_UPD structure.
> +  // Else, use FSPT_ARCH2_UPD structure.
>    //
>    {
> -    0x01,
> +    0x02,
>      {
>        0x00, 0x00, 0x00
>      },
> -    0x00000020,
> +    0x00000040,
>      0x00000000,
> -    {
> -      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> -    }
> -  },
> -  {
>      FixedPcdGet32 (PcdCpuMicrocodePatchAddress),
>      FixedPcdGet32 (PcdCpuMicrocodePatchRegionSize),
>      FixedPcdGet32 (PcdFlashCodeCacheAddress),
>      FixedPcdGet32 (PcdFlashCodeCacheSize),
> +    {
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +    }
>    }
>  };
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /PeiCoreEntry.nasm
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /PeiCoreEntry.nasm
> new file mode 100644
> index 0000000000..0c0766acb8
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /
> +++ PeiCoreEntry.nasm
> @@ -0,0 +1,149 @@
> +;----------------------------------------------------------------------
> +--------
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> ;
> +SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module Name:
> +;
> +;  PeiCoreEntry.nasm
> +;
> +; Abstract:
> +;
> +;   Find and call SecStartup
> +;
> +;----------------------------------------------------------------------
> +--------
> +
> +SECTION .text
> +
> +%include    "PushPopRegsNasm.inc"
> +
> +extern ASM_PFX(SecStartup)
> +extern ASM_PFX(PlatformInit)
> +
> +;
> +; args 1:XMM, 2:REG, 3:IDX
> +;
> +%macro LXMMN        3
> +            pextrq  %2, %1, (%3 & 3)
> +            %endmacro
> +
> +;
> +; args 1:YMM, 2:XMM, 3:IDX (0 - lower 128bits, 1 - upper 128bits) ;
> +%macro LYMMN        3
> +            vextractf128  %2, %1, %3
> +            %endmacro
> +
> +%macro LOAD_TS      1
> +            LYMMN   ymm6, xmm5, 1
> +            LXMMN   xmm5, %1, 1
> +            %endmacro
> +
> +global ASM_PFX(CallPeiCoreEntryPoint)
> +ASM_PFX(CallPeiCoreEntryPoint):
> +  ;
> +  ; Per X64 calling convention, make sure RSP is 16-byte aligned.
> +  ;
> +  mov     rax, rsp
> +  and     rax, 0fh
> +  sub     rsp, rax
> +
> +  ;
> +  ; Platform init
> +  ;
> +  PUSHA_64
> +  sub     rsp, 20h
> +  call    ASM_PFX(PlatformInit)
> +  add     rsp, 20h
> +  POPA_64
> +
> +  ;
> +  ; Set stack top pointer
> +  ;
> +  mov     rsp, r8
> +
> +  ;
> +  ; Push the hob list pointer
> +  ;
> +  push    rcx
> +
> +  ;
> +  ; RBP holds start of BFV passed from Vtf0. Save it to r10.
> +  ;
> +  mov     r10, rbp
> +
> +  ;
> +  ; Save the value
> +  ;   RDX: start of range
> +  ;   r8: end of range
> +  ;
> +  mov     rbp, rsp
> +  push    rdx
> +  push    r8
> +  mov     r14, rdx
> +  mov     r15, r8
> +
> +  ;
> +  ; Push processor count to stack first, then BIST status (AP then BSP)
> + ;
> +  mov     eax, 1
> +  cpuid
> +  shr     ebx, 16
> +  and     ebx, 0000000FFh
> +  cmp     bl, 1
> +  jae     PushProcessorCount
> +
> +  ;
> +  ; Some processors report 0 logical processors.  Effectively 0 = 1.
> +  ; So we fix up the processor count
> +  ;
> +  inc     ebx
> +
> +PushProcessorCount:
> +  sub     rsp, 4
> +  mov     rdi, rsp
> +  mov     DWORD [rdi], ebx
> +
> +  ;
> +  ; We need to implement a long-term solution for BIST capture.  For
> +now, we just copy BSP BIST
> +  ; for all processor threads
> +  ;
> +  xor     ecx, ecx
> +  mov     cl, bl
> +PushBist:
> +  sub     rsp, 4
> +  mov     rdi, rsp
> +  movd    eax, mm0
> +  mov     DWORD [rdi], eax
> +  loop    PushBist
> +
> +  ; Save Time-Stamp Counter
> +  LOAD_TS rax
> +  push    rax
> +
> +  ;
> +  ; Pass entry point of the PEI core
> +  ;
> +  mov     rdi, 0FFFFFFE0h
> +  mov     edi, DWORD [rdi]
> +  mov     r9, rdi
> +
> +  ;
> +  ; Pass BFV into the PEI Core
> +  ;
> +  mov     r8, r10
> +
> +  ;
> +  ; Pass stack size into the PEI Core
> +  ;
> +  mov     rcx, r15  ; Start of TempRam
> +  mov     rdx, r14  ; End of TempRam
> +
> +  sub     rcx, rdx  ; Size of TempRam
> +
> +  ;
> +  ; Pass Control into the PEI Core
> +  ;
> +  sub     rsp, 20h
> +  call ASM_PFX(SecStartup)
> +
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /SecEntry.nasm
> new file mode 100644
> index 0000000000..dbbf63336e
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /
> +++ SecEntry.nasm
> @@ -0,0 +1,171 @@
> +;----------------------------------------------------------------------
> +--------
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> ;
> +SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module Name:
> +;
> +;  SecEntry.asm
> +;
> +; Abstract:
> +;
> +;  This is the code that calls TempRamInit API from FSP binary and
> +passes ;  control into PEI core.
> +;
> +;----------------------------------------------------------------------
> +--------
> +
> +#include "Fsp.h"
> +
> +IA32_CR4_OSFXSR           equ        200h
> +IA32_CR4_OSXMMEXCPT       equ        400h
> +IA32_CR0_MP               equ        2h
> +
> +IA32_CPUID_SSE2           equ        02000000h
> +IA32_CPUID_SSE2_B         equ        26
> +
> +SECTION .text
> +
> +extern   ASM_PFX(CallPeiCoreEntryPoint)
> +extern   ASM_PFX(FsptUpdDataPtr)
> +
> +; Pcds
> +extern   ASM_PFX(PcdGet32 (PcdFsptBaseAddress))
> +
> +;----------------------------------------------------------------------
> +------
> +;
> +; Procedure:    _ModuleEntryPoint
> +;
> +; Input:        None
> +;
> +; Output:       None
> +;
> +; Destroys:     Assume all registers
> +;
> +; Description:
> +;
> +;  Call TempRamInit API from FSP binary. After TempRamInit done, pass ;
> +control into PEI core.
> +;
> +; Return:       None
> +;
> +;  MMX Usage:
> +;              MM0 = BIST State
> +;
> +;----------------------------------------------------------------------
> +------
> +
> +BITS 64
> +align 16
> +global ASM_PFX(ModuleEntryPoint)
> +ASM_PFX(ModuleEntryPoint):
> +  fninit                                ; clear any pending Floating point exceptions
> +  ;
> +  ; Store the BIST value in mm0
> +  ;
> +  movd    mm0, eax
> +
> +  ; Find the fsp info header
> +  mov     rax, ASM_PFX(PcdGet32 (PcdFsptBaseAddress))
> +  mov     edi, [eax]
> +
> +  mov     eax, dword [edi + FVH_SIGINATURE_OFFSET]
> +  cmp     eax, FVH_SIGINATURE_VALID_VALUE
> +  jnz     FspHeaderNotFound
> +
> +  xor     eax, eax
> +  mov     ax, word [edi + FVH_EXTHEADER_OFFSET_OFFSET]
> +  cmp     ax, 0
> +  jnz     FspFvExtHeaderExist
> +
> +  xor     eax, eax
> +  mov     ax, word [edi + FVH_HEADER_LENGTH_OFFSET]     ; Bypass Fv
> Header
> +  add     edi, eax
> +  jmp     FspCheckFfsHeader
> +
> +FspFvExtHeaderExist:
> +  add     edi, eax
> +  mov     eax, dword [edi + FVH_EXTHEADER_SIZE_OFFSET]  ; Bypass Ext Fv
> Header
> +  add     edi, eax
> +
> +  ; Round up to 8 byte alignment
> +  mov     eax, edi
> +  and     al,  07h
> +  jz      FspCheckFfsHeader
> +
> +  and     edi, 0FFFFFFF8h
> +  add     edi, 08h
> +
> +FspCheckFfsHeader:
> +  ; Check the ffs guid
> +  mov     eax, dword [edi]
> +  cmp     eax, FSP_HEADER_GUID_DWORD1
> +  jnz     FspHeaderNotFound
> +
> +  mov     eax, dword [edi + 4]
> +  cmp     eax, FSP_HEADER_GUID_DWORD2
> +  jnz     FspHeaderNotFound
> +
> +  mov     eax, dword [edi + 8]
> +  cmp     eax, FSP_HEADER_GUID_DWORD3
> +  jnz     FspHeaderNotFound
> +
> +  mov     eax, dword [edi + 0Ch]
> +  cmp     eax, FSP_HEADER_GUID_DWORD4
> +  jnz     FspHeaderNotFound
> +
> +  add     edi, FFS_HEADER_SIZE_VALUE         ; Bypass the ffs header
> +
> +  ; Check the section type as raw section
> +  mov     al, byte [edi + SECTION_HEADER_TYPE_OFFSET]
> +  cmp     al, 019h
> +  jnz FspHeaderNotFound
> +
> +  add     edi, RAW_SECTION_HEADER_SIZE_VALUE ; Bypass the section
> header
> +  jmp     FspHeaderFound
> +
> +FspHeaderNotFound:
> +  jmp     $
> +
> +FspHeaderFound:
> +  ; Get the fsp TempRamInit Api address
> +  mov     eax, dword [edi + FSP_HEADER_IMAGEBASE_OFFSET]
> +  add     eax, dword [edi + FSP_HEADER_TEMPRAMINIT_OFFSET]
> +
> +  ; Setup the hardcode stack
> +  mov     rsp, TempRamInitStack
> +
> +  ; Call the fsp TempRamInit Api
> +  jmp     rax
> +
> +TempRamInitDone:
> +  cmp     rax, 0800000000000000Eh ; Check if EFI_NOT_FOUND returned.
> Error code for Microcode Update not found.
> +  je      CallSecFspInit          ; If microcode not found, don't hang, but
> continue.
> +
> +  cmp     rax, 0                  ; Check if EFI_SUCCESS returned.
> +  jnz     FspApiFailed
> +
> +  ; RDX: start of range
> +  ; R8: end of range
> +CallSecFspInit:
> +
> +  mov     r8,  rdx
> +  mov     rdx, rcx
> +  xor     ecx, ecx ; zero - no Hob List Yet
> +  mov     rsp, r8
> +
> +  ;
> +  ; Per X64 calling convention, make sure RSP is 16-byte aligned.
> +  ;
> +  mov     rax, rsp
> +  and     rax, 0fh
> +  sub     rsp, rax
> +
> +  call    ASM_PFX(CallPeiCoreEntryPoint)
> +
> +FspApiFailed:
> +  jmp     $
> +
> +align 10h
> +TempRamInitStack:
> +    DQ  TempRamInitDone
> +    DQ  ASM_PFX(FsptUpdDataPtr)         ; TempRamInitParams
> +
> diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /Stack.nasm
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /Stack.nasm
> new file mode 100644
> index 0000000000..64e46ce953
> --- /dev/null
> +++
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/X64
> /
> +++ Stack.nasm
> @@ -0,0 +1,73 @@
> +;----------------------------------------------------------------------
> +--------
> +;
> +; Copyright (c) 2022, Intel Corporation. All rights reserved.<BR> ;
> +SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Abstract:
> +;
> +;   Switch the stack from temporary memory to permanent memory.
> +;
> +;----------------------------------------------------------------------
> +--------
> +
> +    SECTION .text
> +
> +;----------------------------------------------------------------------
> +--------
> +; VOID
> +; EFIAPI
> +; SecSwitchStack (
> +;   UINT32   TemporaryMemoryBase,
> +;   UINT32   PermanentMemoryBase
> +;   );
> +;----------------------------------------------------------------------
> +--------
> +global ASM_PFX(SecSwitchStack)
> +ASM_PFX(SecSwitchStack):
> +    ;
> +    ; Save four register: rax, rbx, rcx, rdx
> +    ;
> +    push  rax
> +    push  rbx
> +    push  rcx
> +    push  rdx
> +
> +    ;
> +    ; !!CAUTION!! this function address's is pushed into stack after
> +    ; migration of whole temporary memory, so need save it to permanent
> +    ; memory at first!
> +    ;
> +
> +    mov   rbx, rcx                 ; Save the first parameter
> +    mov   rcx, rdx                 ; Save the second parameter
> +
> +    ;
> +    ; Save this function's return address into permanent memory at first.
> +    ; Then, Fixup the esp point to permanent memory
> +    ;
> +    mov   rax, rsp
> +    sub   rax, rbx
> +    add   rax, rcx
> +    mov   rdx, qword [rsp]         ; copy pushed register's value to permanent
> memory
> +    mov   qword [rax], rdx
> +    mov   rdx, qword [rsp + 8]
> +    mov   qword [rax + 8], rdx
> +    mov   rdx, qword [rsp + 16]
> +    mov   qword [rax + 16], rdx
> +    mov   rdx, qword [rsp + 24]
> +    mov   qword [rax + 24], rdx
> +    mov   rdx, qword [rsp + 32]    ; Update this function's return address
> into permanent memory
> +    mov   qword [rax + 32], rdx
> +    mov   rsp, rax                 ; From now, rsp is pointed to permanent
> memory
> +
> +    ;
> +    ; Fixup the rbp point to permanent memory
> +    ;
> +    mov   rax, rbp
> +    sub   rax, rbx
> +    add   rax, rcx
> +    mov   rbp, rax                 ; From now, rbp is pointed to permanent
> memory
> +
> +    pop   rdx
> +    pop   rcx
> +    pop   rbx
> +    pop   rax
> +    ret
> +
> --
> 2.16.2.windows.1