[edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64

Kuo, Ted posted 1 patch 4 months, 1 week ago
Failed in applying to current master (apply log)
There is a newer version of this series
.../SecFspWrapperPlatformSecLib/FsptCoreUpd.h |  25 +-
.../Ia32/SecEntry.nasm                        |   4 +-
.../SecFspWrapperPlatformSecLib.inf           |  12 +-
.../SecGetPerformance.c                       |  11 +-
.../SecPlatformInformation.c                  |   8 +-
.../SecRamInitData.c                          |  56 ++++-
.../X64/PeiCoreEntry.nasm                     | 224 ++++++++++++++++++
.../X64/SecEntry.nasm                         | 214 +++++++++++++++++
.../X64/Stack.nasm                            |  72 ++++++
.../Ia32 => Include}/Fsp.h                    |   4 +-
.../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  21 ++
11 files changed, 629 insertions(+), 22 deletions(-)
create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
rename Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32 => Include}/Fsp.h (86%)
[edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Posted by Kuo, Ted 4 months, 1 week ago
https://bugzilla.tianocore.org/show_bug.cgi?id=4623
1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
2.Made changes in common file to support both IA32 and X64.
3.Added the PCDs below for FSP-T UPD revsions and X64 feature.
 - PcdFspWrapperResetVectorInFsp
 - PcdFspWrapperBfvforResetVectorInFsp
 - PcdFsptUpdHeaderRevision
 - PcdFsptArchUpdHeaderRevision

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
---
 .../SecFspWrapperPlatformSecLib/FsptCoreUpd.h |  25 +-
 .../Ia32/SecEntry.nasm                        |   4 +-
 .../SecFspWrapperPlatformSecLib.inf           |  12 +-
 .../SecGetPerformance.c                       |  11 +-
 .../SecPlatformInformation.c                  |   8 +-
 .../SecRamInitData.c                          |  56 ++++-
 .../X64/PeiCoreEntry.nasm                     | 224 ++++++++++++++++++
 .../X64/SecEntry.nasm                         | 214 +++++++++++++++++
 .../X64/Stack.nasm                            |  72 ++++++
 .../Ia32 => Include}/Fsp.h                    |   4 +-
 .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  21 ++
 11 files changed, 629 insertions(+), 22 deletions(-)
 create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
 create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
 create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
 rename Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32 => Include}/Fsp.h (86%)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
index 7c0f605b92..7c4ddc09a8 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -10,6 +10,28 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #pragma pack(1)
 
+#if defined (MDE_CPU_X64)
+/** Fsp T Core UPD
+**/
+typedef struct {
+
+/** Offset 0x0040
+**/
+  EFI_PHYSICAL_ADDRESS        MicrocodeRegionBase;
+
+/** Offset 0x0048
+**/
+  UINT64                      MicrocodeRegionSize;
+
+/** Offset 0x0050
+**/
+  EFI_PHYSICAL_ADDRESS        CodeRegionBase;
+
+/** Offset 0x0058
+**/
+  UINT64                      CodeRegionSize;
+} FSPT_CORE_UPD;
+#else
 /** Fsp T Core UPD
 **/
 typedef struct {
@@ -34,6 +56,7 @@ typedef struct {
 **/
   UINT8                       Reserved[16];
 } FSPT_CORE_UPD;
+#endif
 
 #pragma pack()
 
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
index 7f6d771e41..de44066a20 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
@@ -1,6 +1,6 @@
 ;------------------------------------------------------------------------------
 ;
-; Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2019 - 2023, Intel Corporation. All rights reserved.<BR>
 ; SPDX-License-Identifier: BSD-2-Clause-Patent
 ; Module Name:
 ;
@@ -13,7 +13,7 @@
 ;
 ;------------------------------------------------------------------------------
 
-#include "Fsp.h"
+#include <Fsp.h>
 
 SECTION .text
 
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
index 2e0d67eae4..99a04cc264 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  Provide FSP wrapper platform sec related function.
 #
-#  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -47,7 +47,11 @@
   Ia32/SecEntry.nasm
   Ia32/PeiCoreEntry.nasm
   Ia32/Stack.nasm
-  Ia32/Fsp.h
+
+[Sources.X64]
+  X64/SecEntry.nasm
+  X64/PeiCoreEntry.nasm
+  X64/Stack.nasm
 
 ################################################################################
 #
@@ -96,3 +100,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                  ## CONSUMES
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection                 ## CONSUMES
   gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain       ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp         ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp   ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision              ## CONSUMES
+  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision          ## CONSUMES
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
index ac2deeabec..471e9e86a2 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
@@ -1,7 +1,7 @@
 /** @file
   Sample to provide SecGetPerformance function.
 
-Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -58,6 +58,7 @@ SecGetPerformance (
   if (EFI_ERROR (Status)) {
     return EFI_NOT_FOUND;
   }
+
   //
   // |--------------| <- TopOfTemporaryRam - BL
   // |   List Ptr   |
@@ -77,12 +78,12 @@ SecGetPerformance (
   // |  TSC[31:00]  |
   // |--------------|
   //
-  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
-  TopOfTemporaryRam -= sizeof (UINT32) * 2;
-  Count             = *(UINT32 *)(TopOfTemporaryRam - sizeof (UINT32));
+  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof(UINTN);
+  TopOfTemporaryRam -= sizeof(UINTN) * 2;
+  Count             = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32));
   Size              = Count * sizeof (UINT32);
 
-  Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);
+  Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);
   Performance->ResetEnd = GetTimeInNanoSecond (Ticker);
 
   return EFI_SUCCESS;
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
index 24d55ed838..68b3fa14c0 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
@@ -1,7 +1,7 @@
 /** @file
   Provide SecPlatformInformation function.
 
-Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -59,9 +59,9 @@ SecPlatformInformation (
   // This routine copies the BIST information to the buffer pointed by
   // PlatformInformationRecord for output.
   //
-  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
-  TopOfTemporaryRam -= sizeof (UINT32) * 2;
-  Count             = *((UINT32 *)(TopOfTemporaryRam - sizeof (UINT32)));
+  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof (UINTN);
+  TopOfTemporaryRam -= sizeof(UINTN) * 2;
+  Count             = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof (UINT32)));
   Size              = Count * sizeof (IA32_HANDOFF_STATUS);
 
   if ((*StructureSize) < (UINT64) Size) {
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
index 355d1e6509..928049f13b 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
 /** @file
   Provide TempRamInitParams data.
 
-Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -12,17 +12,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 typedef struct {
   FSP_UPD_HEADER    FspUpdHeader;
+#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
+  FSPT_ARCH_UPD     FsptArchUpd;
+#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
+  FSPT_ARCH2_UPD    FsptArchUpd;
+#endif
   FSPT_CORE_UPD     FsptCoreUpd;
-} FSPT_UPD_CORE_DATA;
+  UINT16            UpdTerminator;
+} FSPT_UPD_DATA;
 
-GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
+GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_DATA FsptUpdDataPtr = {
   {
     0x4450555F54505346,
-    0x00,
+    FixedPcdGet8 (PcdFsptUpdHeaderRevision),
     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00
     }
   },
+#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
+  {
+    0x01,
+    {
+      0x00, 0x00, 0x00
+    },
+    0x00000020,
+    0,
+    {
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+    }
+  },
+#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
+  {
+    0x02,
+    {
+      0x00, 0x00, 0x00
+    },
+    0x00000020,
+    0,
+    {
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+    }
+  },
+#endif
+#if defined (MDE_CPU_X64)
+  {
+    FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
+    FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),
+    0,          // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.
+    FixedPcdGet32 (PcdFlashCodeCacheSize)
+  },
+#else
   {
     FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
     FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),
@@ -31,6 +73,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
       0x00, 0x00, 0x00, 0x00, 0x00, 0x00
     }
-  }
+  },
+#endif
+  0x55AA
 };
 
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
new file mode 100644
index 0000000000..c69c3b1116
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
@@ -0,0 +1,224 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+;  PeiCoreEntry.nasm
+;
+; Abstract:
+;
+;   Find and call SecStartup
+;
+;------------------------------------------------------------------------------
+
+SECTION .text
+
+extern ASM_PFX(SecStartup)
+extern ASM_PFX(PlatformInit)
+extern ASM_PFX(PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
+
+;-----------------------------------------------------------------------------
+;  Macro:        PUSHA_64
+;
+;  Description:  Saves all registers on stack
+;
+;  Input:        None
+;
+;  Output:       None
+;-----------------------------------------------------------------------------
+%macro PUSHA_64   0
+  push    r8
+  push    r9
+  push    r10
+  push    r11
+  push    r12
+  push    r13
+  push    r14
+  push    r15
+  push    rax
+  push    rcx
+  push    rdx
+  push    rbx
+  push    rsp
+  push    rbp
+  push    rsi
+  push    rdi
+%endmacro
+
+;-----------------------------------------------------------------------------
+;  Macro:        POPA_64
+;
+;  Description:  Restores all registers from stack
+;
+;  Input:        None
+;
+;  Output:       None
+;-----------------------------------------------------------------------------
+%macro POPA_64   0
+  pop    rdi
+  pop    rsi
+  pop    rbp
+  pop    rsp
+  pop    rbx
+  pop    rdx
+  pop    rcx
+  pop    rax
+  pop    r15
+  pop    r14
+  pop    r13
+  pop    r12
+  pop    r11
+  pop    r10
+  pop    r9
+  pop    r8
+%endmacro
+
+;
+; 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
+
+  ;
+  ; Per X64 calling convention, make sure RSP is 16-byte aligned.
+  ;
+  mov     rax, rsp
+  and     rax, 0fh
+  sub     rsp, rax
+
+  ;
+  ; Pass entry point of the PEI core
+  ;
+  mov     rdi, 0FFFFFFE0h
+  mov     edi, DWORD [rdi]
+  mov     r9, rdi
+
+  ;
+  ; Pass BFV into the PEI Core
+  ;
+#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
+  ;
+  ; Reset Vector and initial sec core (to initialize Temp Ram) is part of FSP O
+  ; Default UefiCpuPkg Reset Vector locates FSP O as BFV
+  ; However the Actual Sec Core that launches PEI is part of another Fv.
+  ; We need to Pass that Fv as BFV to PEI Core
+  ;
+  xor     rcx, rcx
+  mov     r8, ASM_PFX (PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
+  mov     ecx, DWORD[r8]
+  mov     r8,  rcx
+#else
+  mov     r8, r10
+#endif
+
+  ;
+  ; 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/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
new file mode 100644
index 0000000000..c4a95fbe10
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
@@ -0,0 +1,214 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+; Module Name:
+;
+;  SecEntry.nasm
+;
+; Abstract:
+;
+;  This is the code that passes control to 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)
+extern   ASM_PFX(BoardBeforeTempRamInit)
+; Pcds
+extern   ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
+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 to PEI core.
+;
+; Return:       None
+;
+;  MMX Usage:
+;              MM0 = BIST State
+;
+;----------------------------------------------------------------------------
+
+BITS 64
+align 16
+global ASM_PFX(_ModuleEntryPoint)
+ASM_PFX(_ModuleEntryPoint):
+#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
+  ;
+  ; When Fsp holds reset vector, TempRamInitApi would be called by FSP itself
+  ; and this will be the first call to FspWrapper. So, Skip calling TempRamInitApi
+  ; and proceed further to launch PeiCore Entry.
+  ;
+  jmp    CallSecFspInit
+#endif
+
+  fninit                                ; clear any pending Floating point exceptions
+  ;
+  ; Store the BIST value in mm0
+  ;
+  movd    mm0, eax
+  cli
+
+  ;
+  ; Check INIT# is asserted by port 0xCF9
+  ;
+  mov     dx, 0CF9h
+  in      al, dx
+  cmp     al, 04h
+  jz      IssueWarmReset
+
+  ; Trigger warm reset if PCIEBAR register is not in reset/default value state
+  ;
+  mov     eax, 80000060h ; PCIEX_BAR_REG B0:D0:F0:R60
+  mov     dx,  0CF8h
+  out     dx,  eax
+  mov     dx,  0CFCh
+  in      eax, dx
+  cmp     eax, 0
+  jz      NotWarmStart
+
+IssueWarmReset:
+  ;
+  ; @note Issue warm reset, since if CPU only reset is issued not all MSRs are restored to their defaults
+  ;
+  mov     dx, 0CF9h
+  mov     al, 06h
+  out     dx, al
+  jmp     $
+
+NotWarmStart:
+  ; Find 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         ; move return address to rsp
+  mov     rcx, ASM_PFX(FsptUpdDataPtr)  ; TempRamInitParams
+
+  ; Call the fsp TempRamInit Api
+  jmp     rax
+
+TempRamInitDone:
+  mov     rbx, 0800000000000000Eh
+  cmp     rax, rbx                ; Check if EFI_NOT_FOUND returned. Error code for Microcode Update not found.
+  je      CallSecFspInit          ; If microcode not found, don't hang, but continue.
+
+  test    rax, rax                ; Check if EFI_SUCCESS returned.
+  jnz     FspApiFailed
+
+  ; RDX: start of range
+  ; R8: end of range
+CallSecFspInit:
+#if FixedPcdGet8(PcdFspModeSelection) == 1
+  push    rax
+  mov     rax, ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
+  sub     edx, dword [rax]        ; TemporaryRam for FSP
+  pop     rax
+#endif
+
+  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
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
new file mode 100644
index 0000000000..d7ae97c5da
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
@@ -0,0 +1,72 @@
+;------------------------------------------------------------------------------
+;
+; Copyright (c) 2023, 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
+
diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
similarity index 86%
rename from Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
rename to Platform/Intel/MinPlatformPkg/Include/Fsp.h
index 9f6cdcf476..1b86912583 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
+++ b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
@@ -36,7 +36,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 //
 // Fsp Header
 //
-#define FSP_HEADER_IMAGEBASE_OFFSET     0x1C
-#define FSP_HEADER_TEMPRAMINIT_OFFSET   0x30
+#define FSP_HEADER_IMAGEBASE_OFFSET   0x1C
+#define FSP_HEADER_TEMPRAMINIT_OFFSET 0x30
 
 #endif
diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index a14c6b2db5..8256e62510 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -393,6 +393,27 @@
   #
   gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain|TRUE|BOOLEAN|0xF00000A8
 
+  ## Reset Vector in FSP
+  # FALSE: Reset Vector is in FSP Wrapper
+  # TRUE:  Reset Vector is in FSP
+  #
+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp|FALSE|BOOLEAN|0xF00000A9
+
+  ## BFV Location for Reset Vector in FSP
+  # The default of BFV Location for Reset Vector in FSP is 0xFFFF0000.
+  #
+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp|0xFFFF0000|UINT32|0xF00000AA
+
+  ## FSP-T UPD Header Revision
+  # The default of FSP-T UPD Header Revision is 0.
+  #
+  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision|0x0|UINT8|0xF00000AC
+
+  ## FSP-T ARCH UPD Header Revision
+  # The default of FSP-T ARCH UPD Header Revision is 0.
+  #
+  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision|0x0|UINT8|0xF00000AD
+
 [PcdsFeatureFlag]
 
   gMinPlatformPkgTokenSpaceGuid.PcdStopAfterDebugInit     |FALSE|BOOLEAN|0xF00000A1
-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112627): https://edk2.groups.io/g/devel/message/112627
Mute This Topic: https://groups.io/mt/103237142/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Posted by Nate DeSimone 4 months, 1 week ago
Hi Ted,

Looking at this code, the X64 version will only work if PcdFspWrapperResetVectorInFsp == TRUE. Moreover, the IA32 version will only work if PcdFspWrapperResetVectorInFsp == FALSE. So... what is the point of having a PCD if the PCD must always be set to one value or the other? Please choose one of these options:

Option 1: Make the PCD work correctly for all the 4 cases:

- IA32 + Bootloader Reset Vector
- IA32 + FSP Reset Vector
- X64 + Bootloader Reset Vector
- X64 + FSP Reset Vector

Option 2: Make a separate instance of PlatformSecLib for the case of FSP-O providing the reset vector.

Additional feedback is below inline.

Thanks,
Nate

> -----Original Message-----
> From: Kuo, Ted <ted.kuo@intel.com>
> Sent: Sunday, December 17, 2023 8:03 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Dong, Eric <eric.dong@intel.com>; S,
> Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B
> <chinni.b.duggapu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support
> SecFspWrapperPlatformSecLib in X64
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4623
> 1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
> 2.Made changes in common file to support both IA32 and X64.
> 3.Added the PCDs below for FSP-T UPD revsions and X64 feature.
>  - PcdFspWrapperResetVectorInFsp
>  - PcdFspWrapperBfvforResetVectorInFsp
>  - PcdFsptUpdHeaderRevision
>  - PcdFsptArchUpdHeaderRevision
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
> Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Ted Kuo <ted.kuo@intel.com>
> ---
>  .../SecFspWrapperPlatformSecLib/FsptCoreUpd.h |  25 +-
>  .../Ia32/SecEntry.nasm                        |   4 +-
>  .../SecFspWrapperPlatformSecLib.inf           |  12 +-
>  .../SecGetPerformance.c                       |  11 +-
>  .../SecPlatformInformation.c                  |   8 +-
>  .../SecRamInitData.c                          |  56 ++++-
>  .../X64/PeiCoreEntry.nasm                     | 224 ++++++++++++++++++
>  .../X64/SecEntry.nasm                         | 214 +++++++++++++++++
>  .../X64/Stack.nasm                            |  72 ++++++
>  .../Ia32 => Include}/Fsp.h                    |   4 +-
>  .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  21 ++
>  11 files changed, 629 insertions(+), 22 deletions(-)
>  create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
>  create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
>  create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
>  rename Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32 => Include}/Fsp.h (86%)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
> index 7c0f605b92..7c4ddc09a8 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -10,6 +10,28 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #pragma pack(1)
>  
> +#if defined (MDE_CPU_X64)
> +/** Fsp T Core UPD
> +**/
> +typedef struct {
> +
> +/** Offset 0x0040
> +**/
> +  EFI_PHYSICAL_ADDRESS        MicrocodeRegionBase;
> +
> +/** Offset 0x0048
> +**/
> +  UINT64                      MicrocodeRegionSize;
> +
> +/** Offset 0x0050
> +**/
> +  EFI_PHYSICAL_ADDRESS        CodeRegionBase;
> +
> +/** Offset 0x0058
> +**/
> +  UINT64                      CodeRegionSize;
> +} FSPT_CORE_UPD;
> +#else
>  /** Fsp T Core UPD
>  **/
>  typedef struct {
> @@ -34,6 +56,7 @@ typedef struct {
>  **/
>    UINT8                       Reserved[16];
>  } FSPT_CORE_UPD;
> +#endif
>  
>  #pragma pack()
>  
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
> index 7f6d771e41..de44066a20 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
> @@ -1,6 +1,6 @@
>  ;------------------------------------------------------------------------------
>  ;
> -; Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2019 - 2023, Intel Corporation. All rights reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent
>  ; Module Name:
>  ;
> @@ -13,7 +13,7 @@
>  ;
>  ;------------------------------------------------------------------------------
>  
> -#include "Fsp.h"
> +#include <Fsp.h>
>  
>  SECTION .text
>  
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
> index 2e0d67eae4..99a04cc264 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Provide FSP wrapper platform sec related function.
>  #
> -#  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -47,7 +47,11 @@
>    Ia32/SecEntry.nasm
>    Ia32/PeiCoreEntry.nasm
>    Ia32/Stack.nasm
> -  Ia32/Fsp.h
> +
> +[Sources.X64]
> +  X64/SecEntry.nasm
> +  X64/PeiCoreEntry.nasm
> +  X64/Stack.nasm
>  
>  ################################################################################
>  #
> @@ -96,3 +100,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                  ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection                 ## CONSUMES
>    gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain       ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp         ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp   ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision              ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision          ## CONSUMES
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
> index ac2deeabec..471e9e86a2 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Sample to provide SecGetPerformance function.
>  
> -Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -58,6 +58,7 @@ SecGetPerformance (
>    if (EFI_ERROR (Status)) {
>      return EFI_NOT_FOUND;
>    }
> +
>    //
>    // |--------------| <- TopOfTemporaryRam - BL
>    // |   List Ptr   |
> @@ -77,12 +78,12 @@ SecGetPerformance (
>    // |  TSC[31:00]  |
>    // |--------------|
>    //
> -  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
> -  TopOfTemporaryRam -= sizeof (UINT32) * 2;
> -  Count             = *(UINT32 *)(TopOfTemporaryRam - sizeof (UINT32));
> +  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof(UINTN);

Why the cast from UINTN to UINT32? This will truncate the value and cause the code to break if we map the reset vector to > 4 GB.

> +  TopOfTemporaryRam -= sizeof(UINTN) * 2;
> +  Count             = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32));
>    Size              = Count * sizeof (UINT32);
>  
> -  Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);
> +  Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);

Any reason for using sizeof (UINT32) * 2 instead of sizeof(UINT64)? To me it seems somewhat obvious that a TSC value would be 64-bit size even though EDX:EAX are used to return it.

Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT64));

>    Performance->ResetEnd = GetTimeInNanoSecond (Ticker);
>  
>    return EFI_SUCCESS;
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
> index 24d55ed838..68b3fa14c0 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Provide SecPlatformInformation function.
>  
> -Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -59,9 +59,9 @@ SecPlatformInformation (
>    // This routine copies the BIST information to the buffer pointed by
>    // PlatformInformationRecord for output.
>    //
> -  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
> -  TopOfTemporaryRam -= sizeof (UINT32) * 2;
> -  Count             = *((UINT32 *)(TopOfTemporaryRam - sizeof (UINT32)));
> +  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof (UINTN);

Why the cast from UINTN to UINT32? This will truncate the value and cause the code to break if we map the reset vector to > 4 GB.

> +  TopOfTemporaryRam -= sizeof(UINTN) * 2;
> +  Count             = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof (UINT32)));
>    Size              = Count * sizeof (IA32_HANDOFF_STATUS);
>  
>    if ((*StructureSize) < (UINT64) Size) {
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
> index 355d1e6509..928049f13b 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Provide TempRamInitParams data.
>  
> -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -12,17 +12,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  typedef struct {
>    FSP_UPD_HEADER    FspUpdHeader;
> +#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
> +  FSPT_ARCH_UPD     FsptArchUpd;
> +#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
> +  FSPT_ARCH2_UPD    FsptArchUpd;
> +#endif
>    FSPT_CORE_UPD     FsptCoreUpd;
> -} FSPT_UPD_CORE_DATA;
> +  UINT16            UpdTerminator;
> +} FSPT_UPD_DATA;
>  
> -GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_DATA FsptUpdDataPtr = {

Please add comments to all the FsptUpdDataPtr initial values below so it is clear which field is being initialized to which value.

>    {
>      0x4450555F54505346,
> -    0x00,
> +    FixedPcdGet8 (PcdFsptUpdHeaderRevision),
>      { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00
>      }
>    },
> +#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
> +  {
> +    0x01,
> +    {
> +      0x00, 0x00, 0x00
> +    },
> +    0x00000020,
> +    0,
> +    {
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +    }
> +  },
> +#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
> +  {
> +    0x02,
> +    {
> +      0x00, 0x00, 0x00
> +    },
> +    0x00000020,
> +    0,
> +    {
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +    }
> +  },
> +#endif
> +#if defined (MDE_CPU_X64)
> +  {
> +    FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
> +    FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),
> +    0,          // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.
> +    FixedPcdGet32 (PcdFlashCodeCacheSize)
> +  },
> +#else
>    {
>      FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
>      FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),
> @@ -31,6 +73,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
>      { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>        0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>      }
> -  }
> +  },
> +#endif
> +  0x55AA
>  };
>  
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
> new file mode 100644
> index 0000000000..c69c3b1116
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
> @@ -0,0 +1,224 @@
> +;------------------------------------------------------------------------------
> +;
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +; Module Name:
> +;
> +;  PeiCoreEntry.nasm
> +;
> +; Abstract:
> +;
> +;   Find and call SecStartup
> +;
> +;------------------------------------------------------------------------------
> +
> +SECTION .text
> +
> +extern ASM_PFX(SecStartup)
> +extern ASM_PFX(PlatformInit)
> +extern ASM_PFX(PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
> +
> +;-----------------------------------------------------------------------------
> +;  Macro:        PUSHA_64
> +;
> +;  Description:  Saves all registers on stack
> +;
> +;  Input:        None
> +;
> +;  Output:       None
> +;-----------------------------------------------------------------------------
> +%macro PUSHA_64   0
> +  push    r8
> +  push    r9
> +  push    r10
> +  push    r11
> +  push    r12
> +  push    r13
> +  push    r14
> +  push    r15
> +  push    rax
> +  push    rcx
> +  push    rdx
> +  push    rbx
> +  push    rsp
> +  push    rbp
> +  push    rsi
> +  push    rdi
> +%endmacro
> +
> +;-----------------------------------------------------------------------------
> +;  Macro:        POPA_64
> +;
> +;  Description:  Restores all registers from stack
> +;
> +;  Input:        None
> +;
> +;  Output:       None
> +;-----------------------------------------------------------------------------
> +%macro POPA_64   0
> +  pop    rdi
> +  pop    rsi
> +  pop    rbp
> +  pop    rsp
> +  pop    rbx
> +  pop    rdx
> +  pop    rcx
> +  pop    rax
> +  pop    r15
> +  pop    r14
> +  pop    r13
> +  pop    r12
> +  pop    r11
> +  pop    r10
> +  pop    r9
> +  pop    r8
> +%endmacro
> +
> +;
> +; args 1:XMM, 2:REG, 3:IDX
> +;
> +%macro LXMMN        3
> +            pextrq  %2, %1, (%3 & 3)
> +            %endmacro

Why are some do some of these macro definitions have %endmacro at the same indent as %macro and some of them have it indented? Please do it the same way for all the macro definitions in the file.

> +
> +;
> +; 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
> +
> +  ;
> +  ; Per X64 calling convention, make sure RSP is 16-byte aligned.
> +  ;
> +  mov     rax, rsp
> +  and     rax, 0fh
> +  sub     rsp, rax
> +
> +  ;
> +  ; Pass entry point of the PEI core
> +  ;
> +  mov     rdi, 0FFFFFFE0h
> +  mov     edi, DWORD [rdi]
> +  mov     r9, rdi
> +
> +  ;
> +  ; Pass BFV into the PEI Core
> +  ;
> +#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
> +  ;
> +  ; Reset Vector and initial sec core (to initialize Temp Ram) is part of FSP O
> +  ; Default UefiCpuPkg Reset Vector locates FSP O as BFV
> +  ; However the Actual Sec Core that launches PEI is part of another Fv.
> +  ; We need to Pass that Fv as BFV to PEI Core
> +  ;
> +  xor     rcx, rcx
> +  mov     r8, ASM_PFX (PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
> +  mov     ecx, DWORD[r8]

This should be UINT64 since it is possible with 64-bit for the reset vector to be > 4 GB.

> +  mov     r8,  rcx
> +#else
> +  mov     r8, r10
> +#endif
> +
> +  ;
> +  ; 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/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
> new file mode 100644
> index 0000000000..c4a95fbe10
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
> @@ -0,0 +1,214 @@
> +;------------------------------------------------------------------------------
> +;
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +; Module Name:
> +;
> +;  SecEntry.nasm
> +;
> +; Abstract:
> +;
> +;  This is the code that passes control to 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)
> +extern   ASM_PFX(BoardBeforeTempRamInit)
> +; Pcds
> +extern   ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
> +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 to PEI core.
> +;
> +; Return:       None
> +;
> +;  MMX Usage:
> +;              MM0 = BIST State
> +;
> +;----------------------------------------------------------------------------
> +
> +BITS 64
> +align 16
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> +#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
> +  ;
> +  ; When Fsp holds reset vector, TempRamInitApi would be called by FSP itself
> +  ; and this will be the first call to FspWrapper. So, Skip calling TempRamInitApi
> +  ; and proceed further to launch PeiCore Entry.
> +  ;
> +  jmp    CallSecFspInit
> +#endif
> +
> +  fninit                                ; clear any pending Floating point exceptions
> +  ;
> +  ; Store the BIST value in mm0
> +  ;
> +  movd    mm0, eax
> +  cli
> +
> +  ;
> +  ; Check INIT# is asserted by port 0xCF9
> +  ;
> +  mov     dx, 0CF9h
> +  in      al, dx
> +  cmp     al, 04h
> +  jz      IssueWarmReset
> +
> +  ; Trigger warm reset if PCIEBAR register is not in reset/default value state
> +  ;
> +  mov     eax, 80000060h ; PCIEX_BAR_REG B0:D0:F0:R60
> +  mov     dx,  0CF8h
> +  out     dx,  eax
> +  mov     dx,  0CFCh
> +  in      eax, dx
> +  cmp     eax, 0
> +  jz      NotWarmStart
> +
> +IssueWarmReset:
> +  ;
> +  ; @note Issue warm reset, since if CPU only reset is issued not all MSRs are restored to their defaults
> +  ;
> +  mov     dx, 0CF9h
> +  mov     al, 06h
> +  out     dx, al
> +  jmp     $
> +
> +NotWarmStart:
> +  ; Find 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         ; move return address to rsp
> +  mov     rcx, ASM_PFX(FsptUpdDataPtr)  ; TempRamInitParams
> +
> +  ; Call the fsp TempRamInit Api
> +  jmp     rax
> +
> +TempRamInitDone:
> +  mov     rbx, 0800000000000000Eh
> +  cmp     rax, rbx                ; Check if EFI_NOT_FOUND returned. Error code for Microcode Update not found.
> +  je      CallSecFspInit          ; If microcode not found, don't hang, but continue.
> +
> +  test    rax, rax                ; Check if EFI_SUCCESS returned.
> +  jnz     FspApiFailed
> +
> +  ; RDX: start of range
> +  ; R8: end of range
> +CallSecFspInit:
> +#if FixedPcdGet8(PcdFspModeSelection) == 1
> +  push    rax
> +  mov     rax, ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
> +  sub     edx, dword [rax]        ; TemporaryRam for FSP
> +  pop     rax
> +#endif
> +
> +  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
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
> new file mode 100644
> index 0000000000..d7ae97c5da
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
> @@ -0,0 +1,72 @@
> +;------------------------------------------------------------------------------
> +;
> +; Copyright (c) 2023, 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
> +
> diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
> similarity index 86%
> rename from Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
> rename to Platform/Intel/MinPlatformPkg/Include/Fsp.h
> index 9f6cdcf476..1b86912583 100644
> --- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
> +++ b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
> @@ -36,7 +36,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  //
>  // Fsp Header
>  //
> -#define FSP_HEADER_IMAGEBASE_OFFSET     0x1C
> -#define FSP_HEADER_TEMPRAMINIT_OFFSET   0x30
> +#define FSP_HEADER_IMAGEBASE_OFFSET   0x1C
> +#define FSP_HEADER_TEMPRAMINIT_OFFSET 0x30
>  
>  #endif
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index a14c6b2db5..8256e62510 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -393,6 +393,27 @@
>    #
>    gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain|TRUE|BOOLEAN|0xF00000A8
>  
> +  ## Reset Vector in FSP
> +  # FALSE: Reset Vector is in FSP Wrapper
> +  # TRUE:  Reset Vector is in FSP
> +  #
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp|FALSE|BOOLEAN|0xF00000A9
> +
> +  ## BFV Location for Reset Vector in FSP
> +  # The default of BFV Location for Reset Vector in FSP is 0xFFFF0000.
> +  #
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp|0xFFFF0000|UINT32|0xF00000AA

This should be UINT64 since it is possible with 64-bit for the reset vector to be > 4 GB.

> +
> +  ## FSP-T UPD Header Revision
> +  # The default of FSP-T UPD Header Revision is 0.
> +  #
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision|0x0|UINT8|0xF00000AC
> +
> +  ## FSP-T ARCH UPD Header Revision
> +  # The default of FSP-T ARCH UPD Header Revision is 0.
> +  #
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision|0x0|UINT8|0xF00000AD
> +
>  [PcdsFeatureFlag]
>  
>    gMinPlatformPkgTokenSpaceGuid.PcdStopAfterDebugInit     |FALSE|BOOLEAN|0xF00000A1
> -- 
> 2.40.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112777): https://edk2.groups.io/g/devel/message/112777
Mute This Topic: https://groups.io/mt/103237142/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Posted by Kuo, Ted 4 months, 1 week ago
Thanks Nate for the feedback. I've sent out patch v2 according to your comments. Can you please review again?

Thanks,
Ted

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
Sent: Thursday, December 21, 2023 8:13 AM
To: Kuo, Ted <ted.kuo@intel.com>; devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Dong, Eric <eric.dong@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64

Hi Ted,

Looking at this code, the X64 version will only work if PcdFspWrapperResetVectorInFsp == TRUE. Moreover, the IA32 version will only work if PcdFspWrapperResetVectorInFsp == FALSE. So... what is the point of having a PCD if the PCD must always be set to one value or the other? Please choose one of these options:

Option 1: Make the PCD work correctly for all the 4 cases:

- IA32 + Bootloader Reset Vector
- IA32 + FSP Reset Vector
- X64 + Bootloader Reset Vector
- X64 + FSP Reset Vector

Option 2: Make a separate instance of PlatformSecLib for the case of FSP-O providing the reset vector.

Additional feedback is below inline.

Thanks,
Nate

> -----Original Message-----
> From: Kuo, Ted <ted.kuo@intel.com>
> Sent: Sunday, December 17, 2023 8:03 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel 
> <chasel.chiu@intel.com>; Desimone, Nathaniel L 
> <nathaniel.l.desimone@intel.com>; Dong, Eric <eric.dong@intel.com>; S, 
> Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B 
> <chinni.b.duggapu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
> Subject: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: 
> Support SecFspWrapperPlatformSecLib in X64
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4623
> 1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
> 2.Made changes in common file to support both IA32 and X64.
> 3.Added the PCDs below for FSP-T UPD revsions and X64 feature.
>  - PcdFspWrapperResetVectorInFsp
>  - PcdFspWrapperBfvforResetVectorInFsp
>  - PcdFsptUpdHeaderRevision
>  - PcdFsptArchUpdHeaderRevision
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
> Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Ted Kuo <ted.kuo@intel.com>
> ---
>  .../SecFspWrapperPlatformSecLib/FsptCoreUpd.h |  25 +-
>  .../Ia32/SecEntry.nasm                        |   4 +-
>  .../SecFspWrapperPlatformSecLib.inf           |  12 +-
>  .../SecGetPerformance.c                       |  11 +-
>  .../SecPlatformInformation.c                  |   8 +-
>  .../SecRamInitData.c                          |  56 ++++-
>  .../X64/PeiCoreEntry.nasm                     | 224 ++++++++++++++++++
>  .../X64/SecEntry.nasm                         | 214 +++++++++++++++++
>  .../X64/Stack.nasm                            |  72 ++++++
>  .../Ia32 => Include}/Fsp.h                    |   4 +-
>  .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  21 ++
>  11 files changed, 629 insertions(+), 22 deletions(-)  create mode 
> 100644 
> Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/X64/PeiCoreEntry.nasm  create mode 100644 
> Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/X64/SecEntry.nasm  create mode 100644 
> Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/X64/Stack.nasm  rename 
> Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatfor
> mSecLib/Ia32 => Include}/Fsp.h (86%)
> 
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/FsptCoreUpd.h 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/FsptCoreUpd.h
> index 7c0f605b92..7c4ddc09a8 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/FsptCoreUpd.h
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/FsptCoreUpd.h
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -10,6 +10,28 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  #pragma pack(1)
>  
> +#if defined (MDE_CPU_X64)
> +/** Fsp T Core UPD
> +**/
> +typedef struct {
> +
> +/** Offset 0x0040
> +**/
> +  EFI_PHYSICAL_ADDRESS        MicrocodeRegionBase;
> +
> +/** Offset 0x0048
> +**/
> +  UINT64                      MicrocodeRegionSize;
> +
> +/** Offset 0x0050
> +**/
> +  EFI_PHYSICAL_ADDRESS        CodeRegionBase;
> +
> +/** Offset 0x0058
> +**/
> +  UINT64                      CodeRegionSize;
> +} FSPT_CORE_UPD;
> +#else
>  /** Fsp T Core UPD
>  **/
>  typedef struct {
> @@ -34,6 +56,7 @@ typedef struct {
>  **/
>    UINT8                       Reserved[16];
>  } FSPT_CORE_UPD;
> +#endif
>  
>  #pragma pack()
>  
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/Ia32/SecEntry.nasm 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/Ia32/SecEntry.nasm
> index 7f6d771e41..de44066a20 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/Ia32/SecEntry.nasm
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/Ia32/SecEntry.nasm
> @@ -1,6 +1,6 @@
>  
> ;---------------------------------------------------------------------
> ---------
>  ;
> -; Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2019 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  ; SPDX-License-Identifier: BSD-2-Clause-Patent  ; Module Name:
>  ;
> @@ -13,7 +13,7 @@
>  ;
>  
> ;---------------------------------------------------------------------
> ---------
>  
> -#include "Fsp.h"
> +#include <Fsp.h>
>  
>  SECTION .text
>  
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecFspWrapperPlatformSecLib.inf 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecFspWrapperPlatformSecLib.inf
> index 2e0d67eae4..99a04cc264 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecFspWrapperPlatformSecLib.inf
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/SecFspWrapperPlatformSecLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Provide FSP wrapper platform sec related function.
>  #
> -#  Copyright (c) 2017 - 2021, Intel Corporation. All rights 
> reserved.<BR>
> +#  Copyright (c) 2017 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  #
>  # SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -47,7 +47,11 @@
>    Ia32/SecEntry.nasm
>    Ia32/PeiCoreEntry.nasm
>    Ia32/Stack.nasm
> -  Ia32/Fsp.h
> +
> +[Sources.X64]
> +  X64/SecEntry.nasm
> +  X64/PeiCoreEntry.nasm
> +  X64/Stack.nasm
>  
>  
> ######################################################################
> ##########
>  #
> @@ -96,3 +100,7 @@
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                  ## CONSUMES
>    gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection                 ## CONSUMES
>    gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain       ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp         ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp   ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision              ## CONSUMES
> +  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision          ## CONSUMES
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecGetPerformance.c 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecGetPerformance.c
> index ac2deeabec..471e9e86a2 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecGetPerformance.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/SecGetPerformance.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Sample to provide SecGetPerformance function.
>  
> -Copyright (c) 2017 - 2019, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -58,6 +58,7 @@ SecGetPerformance (
>    if (EFI_ERROR (Status)) {
>      return EFI_NOT_FOUND;
>    }
> +
>    //
>    // |--------------| <- TopOfTemporaryRam - BL
>    // |   List Ptr   |
> @@ -77,12 +78,12 @@ SecGetPerformance (
>    // |  TSC[31:00]  |
>    // |--------------|
>    //
> -  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
> -  TopOfTemporaryRam -= sizeof (UINT32) * 2;
> -  Count             = *(UINT32 *)(TopOfTemporaryRam - sizeof (UINT32));
> +  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - 
> + sizeof(UINTN);

Why the cast from UINTN to UINT32? This will truncate the value and cause the code to break if we map the reset vector to > 4 GB.

> +  TopOfTemporaryRam -= sizeof(UINTN) * 2;
> +  Count             = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32));
>    Size              = Count * sizeof (UINT32);
>  
> -  Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size - 
> sizeof (UINT32) * 2);
> +  Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - 
> + Size - sizeof (UINT32) * 2);

Any reason for using sizeof (UINT32) * 2 instead of sizeof(UINT64)? To me it seems somewhat obvious that a TSC value would be 64-bit size even though EDX:EAX are used to return it.

Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT64));

>    Performance->ResetEnd = GetTimeInNanoSecond (Ticker);
>  
>    return EFI_SUCCESS;
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecPlatformInformation.c 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecPlatformInformation.c
> index 24d55ed838..68b3fa14c0 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecPlatformInformation.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/SecPlatformInformation.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Provide SecPlatformInformation function.
>  
> -Copyright (c) 2017 - 2019, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -59,9 +59,9 @@ SecPlatformInformation (
>    // This routine copies the BIST information to the buffer pointed by
>    // PlatformInformationRecord for output.
>    //
> -  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);
> -  TopOfTemporaryRam -= sizeof (UINT32) * 2;
> -  Count             = *((UINT32 *)(TopOfTemporaryRam - sizeof (UINT32)));
> +  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof 
> + (UINTN);

Why the cast from UINTN to UINT32? This will truncate the value and cause the code to break if we map the reset vector to > 4 GB.

> +  TopOfTemporaryRam -= sizeof(UINTN) * 2;
> +  Count             = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof (UINT32)));
>    Size              = Count * sizeof (IA32_HANDOFF_STATUS);
>  
>    if ((*StructureSize) < (UINT64) Size) { diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecRamInitData.c 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecRamInitData.c
> index 355d1e6509..928049f13b 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/SecRamInitData.c
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/SecRamInitData.c
> @@ -1,7 +1,7 @@
>  /** @file
>    Provide TempRamInitParams data.
>  
> -Copyright (c) 2017 - 2021, Intel Corporation. All rights 
> reserved.<BR>
> +Copyright (c) 2017 - 2023, Intel Corporation. All rights 
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -12,17 +12,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  typedef struct {
>    FSP_UPD_HEADER    FspUpdHeader;
> +#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
> +  FSPT_ARCH_UPD     FsptArchUpd;
> +#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
> +  FSPT_ARCH2_UPD    FsptArchUpd;
> +#endif
>    FSPT_CORE_UPD     FsptCoreUpd;
> -} FSPT_UPD_CORE_DATA;
> +  UINT16            UpdTerminator;
> +} FSPT_UPD_DATA;
>  
> -GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr 
> = {
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_DATA FsptUpdDataPtr = {

Please add comments to all the FsptUpdDataPtr initial values below so it is clear which field is being initialized to which value.

>    {
>      0x4450555F54505346,
> -    0x00,
> +    FixedPcdGet8 (PcdFsptUpdHeaderRevision),
>      { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00
>      }
>    },
> +#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1
> +  {
> +    0x01,
> +    {
> +      0x00, 0x00, 0x00
> +    },
> +    0x00000020,
> +    0,
> +    {
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +    }
> +  },
> +#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2
> +  {
> +    0x02,
> +    {
> +      0x00, 0x00, 0x00
> +    },
> +    0x00000020,
> +    0,
> +    {
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +      0x00, 0x00, 0x00, 0x00, 0x00, 0x00
> +    }
> +  },
> +#endif
> +#if defined (MDE_CPU_X64)
> +  {
> +    FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
> +    FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),
> +    0,          // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.
> +    FixedPcdGet32 (PcdFlashCodeCacheSize)
> +  },
> +#else
>    {
>      FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),
>      FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 
> (PcdMicrocodeOffsetInFv), @@ -31,6 +73,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
>      { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>        0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>      }
> -  }
> +  },
> +#endif
> +  0x55AA
>  };
>  
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/PeiCoreEntry.nasm 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/PeiCoreEntry.nasm
> new file mode 100644
> index 0000000000..c69c3b1116
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/X64/PeiCoreEntry.nasm
> @@ -0,0 +1,224 @@
> +;--------------------------------------------------------------------
> +----------
> +;
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> ; 
> +SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module Name:
> +;
> +;  PeiCoreEntry.nasm
> +;
> +; Abstract:
> +;
> +;   Find and call SecStartup
> +;
> +;--------------------------------------------------------------------
> +----------
> +
> +SECTION .text
> +
> +extern ASM_PFX(SecStartup)
> +extern ASM_PFX(PlatformInit)
> +extern ASM_PFX(PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
> +
> +;-----------------------------------------------------------------------------
> +;  Macro:        PUSHA_64
> +;
> +;  Description:  Saves all registers on stack ;
> +;  Input:        None
> +;
> +;  Output:       None
> +;-----------------------------------------------------------------------------
> +%macro PUSHA_64   0
> +  push    r8
> +  push    r9
> +  push    r10
> +  push    r11
> +  push    r12
> +  push    r13
> +  push    r14
> +  push    r15
> +  push    rax
> +  push    rcx
> +  push    rdx
> +  push    rbx
> +  push    rsp
> +  push    rbp
> +  push    rsi
> +  push    rdi
> +%endmacro
> +
> +;-----------------------------------------------------------------------------
> +;  Macro:        POPA_64
> +;
> +;  Description:  Restores all registers from stack ;
> +;  Input:        None
> +;
> +;  Output:       None
> +;-----------------------------------------------------------------------------
> +%macro POPA_64   0
> +  pop    rdi
> +  pop    rsi
> +  pop    rbp
> +  pop    rsp
> +  pop    rbx
> +  pop    rdx
> +  pop    rcx
> +  pop    rax
> +  pop    r15
> +  pop    r14
> +  pop    r13
> +  pop    r12
> +  pop    r11
> +  pop    r10
> +  pop    r9
> +  pop    r8
> +%endmacro
> +
> +;
> +; args 1:XMM, 2:REG, 3:IDX
> +;
> +%macro LXMMN        3
> +            pextrq  %2, %1, (%3 & 3)
> +            %endmacro

Why are some do some of these macro definitions have %endmacro at the same indent as %macro and some of them have it indented? Please do it the same way for all the macro definitions in the file.

> +
> +;
> +; 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
> +
> +  ;
> +  ; Per X64 calling convention, make sure RSP is 16-byte aligned.
> +  ;
> +  mov     rax, rsp
> +  and     rax, 0fh
> +  sub     rsp, rax
> +
> +  ;
> +  ; Pass entry point of the PEI core
> +  ;
> +  mov     rdi, 0FFFFFFE0h
> +  mov     edi, DWORD [rdi]
> +  mov     r9, rdi
> +
> +  ;
> +  ; Pass BFV into the PEI Core
> +  ;
> +#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
> +  ;
> +  ; Reset Vector and initial sec core (to initialize Temp Ram) is 
> +part of FSP O
> +  ; Default UefiCpuPkg Reset Vector locates FSP O as BFV
> +  ; However the Actual Sec Core that launches PEI is part of another Fv.
> +  ; We need to Pass that Fv as BFV to PEI Core
> +  ;
> +  xor     rcx, rcx
> +  mov     r8, ASM_PFX (PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))
> +  mov     ecx, DWORD[r8]

This should be UINT64 since it is possible with 64-bit for the reset vector to be > 4 GB.

> +  mov     r8,  rcx
> +#else
> +  mov     r8, r10
> +#endif
> +
> +  ;
> +  ; 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/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/SecEntry.nasm 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/SecEntry.nasm
> new file mode 100644
> index 0000000000..c4a95fbe10
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/X64/SecEntry.nasm
> @@ -0,0 +1,214 @@
> +;--------------------------------------------------------------------
> +----------
> +;
> +; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR> ; 
> +SPDX-License-Identifier: BSD-2-Clause-Patent ; Module Name:
> +;
> +;  SecEntry.nasm
> +;
> +; Abstract:
> +;
> +;  This is the code that passes control to 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)
> +extern   ASM_PFX(BoardBeforeTempRamInit)
> +; Pcds
> +extern   ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
> +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 to PEI core.
> +;
> +; Return:       None
> +;
> +;  MMX Usage:
> +;              MM0 = BIST State
> +;
> +;--------------------------------------------------------------------
> +--------
> +
> +BITS 64
> +align 16
> +global ASM_PFX(_ModuleEntryPoint)
> +ASM_PFX(_ModuleEntryPoint):
> +#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1
> +  ;
> +  ; When Fsp holds reset vector, TempRamInitApi would be called by 
> +FSP itself
> +  ; and this will be the first call to FspWrapper. So, Skip calling 
> +TempRamInitApi
> +  ; and proceed further to launch PeiCore Entry.
> +  ;
> +  jmp    CallSecFspInit
> +#endif
> +
> +  fninit                                ; clear any pending Floating point exceptions
> +  ;
> +  ; Store the BIST value in mm0
> +  ;
> +  movd    mm0, eax
> +  cli
> +
> +  ;
> +  ; Check INIT# is asserted by port 0xCF9  ;
> +  mov     dx, 0CF9h
> +  in      al, dx
> +  cmp     al, 04h
> +  jz      IssueWarmReset
> +
> +  ; Trigger warm reset if PCIEBAR register is not in reset/default 
> + value state  ;
> +  mov     eax, 80000060h ; PCIEX_BAR_REG B0:D0:F0:R60
> +  mov     dx,  0CF8h
> +  out     dx,  eax
> +  mov     dx,  0CFCh
> +  in      eax, dx
> +  cmp     eax, 0
> +  jz      NotWarmStart
> +
> +IssueWarmReset:
> +  ;
> +  ; @note Issue warm reset, since if CPU only reset is issued not all 
> +MSRs are restored to their defaults
> +  ;
> +  mov     dx, 0CF9h
> +  mov     al, 06h
> +  out     dx, al
> +  jmp     $
> +
> +NotWarmStart:
> +  ; Find 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         ; move return address to rsp
> +  mov     rcx, ASM_PFX(FsptUpdDataPtr)  ; TempRamInitParams
> +
> +  ; Call the fsp TempRamInit Api
> +  jmp     rax
> +
> +TempRamInitDone:
> +  mov     rbx, 0800000000000000Eh
> +  cmp     rax, rbx                ; Check if EFI_NOT_FOUND returned. Error code for Microcode Update not found.
> +  je      CallSecFspInit          ; If microcode not found, don't hang, but continue.
> +
> +  test    rax, rax                ; Check if EFI_SUCCESS returned.
> +  jnz     FspApiFailed
> +
> +  ; RDX: start of range
> +  ; R8: end of range
> +CallSecFspInit:
> +#if FixedPcdGet8(PcdFspModeSelection) == 1
> +  push    rax
> +  mov     rax, ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))
> +  sub     edx, dword [rax]        ; TemporaryRam for FSP
> +  pop     rax
> +#endif
> +
> +  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
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/Stack.nasm 
> b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/X64/Stack.nasm
> new file mode 100644
> index 0000000000..d7ae97c5da
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPl
> +++ atformSecLib/X64/Stack.nasm
> @@ -0,0 +1,72 @@
> +;--------------------------------------------------------------------
> +----------
> +;
> +; Copyright (c) 2023, 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
> +
> diff --git 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/Ia32/Fsp.h b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
> similarity index 86%
> rename from 
> Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatform
> SecLib/Ia32/Fsp.h rename to 
> Platform/Intel/MinPlatformPkg/Include/Fsp.h
> index 9f6cdcf476..1b86912583 100644
> --- 
> a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatfo
> rmSecLib/Ia32/Fsp.h
> +++ b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
> @@ -36,7 +36,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  //  // 
> Fsp Header  //
> -#define FSP_HEADER_IMAGEBASE_OFFSET     0x1C
> -#define FSP_HEADER_TEMPRAMINIT_OFFSET   0x30
> +#define FSP_HEADER_IMAGEBASE_OFFSET   0x1C
> +#define FSP_HEADER_TEMPRAMINIT_OFFSET 0x30
>  
>  #endif
> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec 
> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> index a14c6b2db5..8256e62510 100644
> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
> @@ -393,6 +393,27 @@
>    #
>    
> gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain|TRUE|BOO
> LEAN|0xF00000A8
>  
> +  ## Reset Vector in FSP
> +  # FALSE: Reset Vector is in FSP Wrapper  # TRUE:  Reset Vector is 
> + in FSP  #
> +  
> + gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp|FALSE|BO
> + OLEAN|0xF00000A9
> +
> +  ## BFV Location for Reset Vector in FSP  # The default of BFV 
> + Location for Reset Vector in FSP is 0xFFFF0000.
> +  #
> +  
> + gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp|0x
> + FFFF0000|UINT32|0xF00000AA

This should be UINT64 since it is possible with 64-bit for the reset vector to be > 4 GB.

> +
> +  ## FSP-T UPD Header Revision
> +  # The default of FSP-T UPD Header Revision is 0.
> +  #
> +  
> + gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision|0x0|UINT8|0xF
> + 00000AC
> +
> +  ## FSP-T ARCH UPD Header Revision
> +  # The default of FSP-T ARCH UPD Header Revision is 0.
> +  #
> +  
> + gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision|0x0|UINT8
> + |0xF00000AD
> +
>  [PcdsFeatureFlag]
>  
>    gMinPlatformPkgTokenSpaceGuid.PcdStopAfterDebugInit     |FALSE|BOOLEAN|0xF00000A1
> --
> 2.40.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112808): https://edk2.groups.io/g/devel/message/112808
Mute This Topic: https://groups.io/mt/103237142/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64
Posted by Ashraf Ali S 4 months, 1 week ago
Reviewed-by:  S, Ashraf Ali <ashraf.ali.s@intel.com>

Thanks.,
S, Ashraf Ali

-----Original Message-----
From: Kuo, Ted <ted.kuo@intel.com> 
Sent: Monday, December 18, 2023 9:33 AM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Dong, Eric <eric.dong@intel.com>; S, Ashraf Ali <ashraf.ali.s@intel.com>; Duggapu, Chinni B <chinni.b.duggapu@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: [edk2-devel][edk2-platforms][PATCH v1] MinPlatformPkg: Support SecFspWrapperPlatformSecLib in X64

https://bugzilla.tianocore.org/show_bug.cgi?id=4623
1.Added PeiCoreEntry.nasm, SecEntry.nasm and Stack.nasm for X64.
2.Made changes in common file to support both IA32 and X64.
3.Added the PCDs below for FSP-T UPD revsions and X64 feature.
 - PcdFspWrapperResetVectorInFsp
 - PcdFspWrapperBfvforResetVectorInFsp
 - PcdFsptUpdHeaderRevision
 - PcdFsptArchUpdHeaderRevision

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
---
 .../SecFspWrapperPlatformSecLib/FsptCoreUpd.h |  25 +-
 .../Ia32/SecEntry.nasm                        |   4 +-
 .../SecFspWrapperPlatformSecLib.inf           |  12 +-
 .../SecGetPerformance.c                       |  11 +-
 .../SecPlatformInformation.c                  |   8 +-
 .../SecRamInitData.c                          |  56 ++++-
 .../X64/PeiCoreEntry.nasm                     | 224 ++++++++++++++++++
 .../X64/SecEntry.nasm                         | 214 +++++++++++++++++
 .../X64/Stack.nasm                            |  72 ++++++
 .../Ia32 => Include}/Fsp.h                    |   4 +-
 .../Intel/MinPlatformPkg/MinPlatformPkg.dec   |  21 ++
 11 files changed, 629 insertions(+), 22 deletions(-)  create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
 create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
 create mode 100644 Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
 rename Platform/Intel/MinPlatformPkg/{FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32 => Include}/Fsp.h (86%)

diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
index 7c0f605b92..7c4ddc09a8 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/FsptCoreUpd.h
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/FsptCoreUpd.h
@@ -1,6 +1,6 @@
 /** @file -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -10,6 +10,28 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  #pragma pack(1) +#if defined (MDE_CPU_X64)+/** Fsp T Core UPD+**/+typedef struct {++/** Offset 0x0040+**/+  EFI_PHYSICAL_ADDRESS        MicrocodeRegionBase;++/** Offset 0x0048+**/+  UINT64                      MicrocodeRegionSize;++/** Offset 0x0050+**/+  EFI_PHYSICAL_ADDRESS        CodeRegionBase;++/** Offset 0x0058+**/+  UINT64                      CodeRegionSize;+} FSPT_CORE_UPD;+#else /** Fsp T Core UPD **/ typedef struct {@@ -34,6 +56,7 @@ typedef struct {
 **/   UINT8                       Reserved[16]; } FSPT_CORE_UPD;+#endif  #pragma pack() diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
index 7f6d771e41..de44066a20 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/SecEntry.nasm
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/Ia32/SecEntry.nasm
@@ -1,6 +1,6 @@
 ;------------------------------------------------------------------------------ ;-; Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>+; Copyright (c) 2019 - 2023, Intel Corporation. All rights reserved.<BR> ; SPDX-License-Identifier: BSD-2-Clause-Patent ; Module Name: ;@@ -13,7 +13,7 @@  ; ;------------------------------------------------------------------------------ -#include "Fsp.h"+#include <Fsp.h>  SECTION .text diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
index 2e0d67eae4..99a04cc264 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecFspWrapperPlatformSecLib.inf
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecFspWrapperPlatformSecLib.inf
@@ -1,7 +1,7 @@
 ## @file #  Provide FSP wrapper platform sec related function. #-#  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>+#  Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent #@@ -47,7 +47,11 @@
   Ia32/SecEntry.nasm   Ia32/PeiCoreEntry.nasm   Ia32/Stack.nasm-  Ia32/Fsp.h++[Sources.X64]+  X64/SecEntry.nasm+  X64/PeiCoreEntry.nasm+  X64/Stack.nasm  ################################################################################ #@@ -96,3 +100,7 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmBaseAddress                  ## CONSUMES   gIntelFsp2WrapperTokenSpaceGuid.PcdFspModeSelection                 ## CONSUMES   gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain       ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp         ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp   ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision              ## CONSUMES+  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision          ## CONSUMESdiff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
index ac2deeabec..471e9e86a2 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecGetPerformance.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecGetPerformance.c
@@ -1,7 +1,7 @@
 /** @file   Sample to provide SecGetPerformance function. -Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -58,6 +58,7 @@ SecGetPerformance (
   if (EFI_ERROR (Status)) {     return EFI_NOT_FOUND;   }+   //   // |--------------| <- TopOfTemporaryRam - BL   // |   List Ptr   |@@ -77,12 +78,12 @@ SecGetPerformance (
   // |  TSC[31:00]  |   // |--------------|   //-  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);-  TopOfTemporaryRam -= sizeof (UINT32) * 2;-  Count             = *(UINT32 *)(TopOfTemporaryRam - sizeof (UINT32));+  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof(UINTN);+  TopOfTemporaryRam -= sizeof(UINTN) * 2;+  Count             = *(UINT32 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32));   Size              = Count * sizeof (UINT32); -  Ticker = *(UINT64 *) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);+  Ticker = *(UINT64 *) (UINTN) (TopOfTemporaryRam - sizeof (UINT32) - Size - sizeof (UINT32) * 2);   Performance->ResetEnd = GetTimeInNanoSecond (Ticker);    return EFI_SUCCESS;diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
index 24d55ed838..68b3fa14c0 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecPlatformInformation.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecPlatformInformation.c
@@ -1,7 +1,7 @@
 /** @file   Provide SecPlatformInformation function. -Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -59,9 +59,9 @@ SecPlatformInformation (
   // This routine copies the BIST information to the buffer pointed by   // PlatformInformationRecord for output.   //-  TopOfTemporaryRam = (UINTN) TopOfTemporaryRamPpi - sizeof (UINT32);-  TopOfTemporaryRam -= sizeof (UINT32) * 2;-  Count             = *((UINT32 *)(TopOfTemporaryRam - sizeof (UINT32)));+  TopOfTemporaryRam = (UINT32)(UINTN)TopOfTemporaryRamPpi - sizeof (UINTN);+  TopOfTemporaryRam -= sizeof(UINTN) * 2;+  Count             = *((UINT32 *)(UINTN) (TopOfTemporaryRam - sizeof (UINT32)));   Size              = Count * sizeof (IA32_HANDOFF_STATUS);    if ((*StructureSize) < (UINT64) Size) {diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
index 355d1e6509..928049f13b 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/SecRamInitData.c
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/SecRamInitData.c
@@ -1,7 +1,7 @@
 /** @file   Provide TempRamInitParams data. -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2017 - 2023, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -12,17 +12,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  typedef struct {   FSP_UPD_HEADER    FspUpdHeader;+#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1+  FSPT_ARCH_UPD     FsptArchUpd;+#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2+  FSPT_ARCH2_UPD    FsptArchUpd;+#endif   FSPT_CORE_UPD     FsptCoreUpd;-} FSPT_UPD_CORE_DATA;+  UINT16            UpdTerminator;+} FSPT_UPD_DATA; -GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {+GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_DATA FsptUpdDataPtr = {   {     0x4450555F54505346,-    0x00,+    FixedPcdGet8 (PcdFsptUpdHeaderRevision),     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,-      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,+      0x00, 0x00, 0x00     }   },+#if FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 1+  {+    0x01,+    {+      0x00, 0x00, 0x00+    },+    0x00000020,+    0,+    {+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00+    }+  },+#elif FixedPcdGet8 (PcdFsptArchUpdHeaderRevision) == 2+  {+    0x02,+    {+      0x00, 0x00, 0x00+    },+    0x00000020,+    0,+    {+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00+    }+  },+#endif+#if defined (MDE_CPU_X64)+  {+    FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),+    FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),+    0,          // Set CodeRegionBase as 0, so that caching will be 4GB-(CodeRegionSize > LLCSize ? LLCSize : CodeRegionSize) will be used.+    FixedPcdGet32 (PcdFlashCodeCacheSize)+  },+#else   {     FixedPcdGet32 (PcdFlashFvMicrocodeBase) + FixedPcdGet32 (PcdMicrocodeOffsetInFv),     FixedPcdGet32 (PcdFlashFvMicrocodeSize) - FixedPcdGet32 (PcdMicrocodeOffsetInFv),@@ -31,6 +73,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = {
     { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,       0x00, 0x00, 0x00, 0x00, 0x00, 0x00     }-  }+  },+#endif+  0x55AA }; diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/PeiCoreEntry.nasm
new file mode 100644
index 0000000000..c69c3b1116
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/X64/PeiCoreEntry.nasm
@@ -0,0 +1,224 @@
+;------------------------------------------------------------------------------+;+; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>+; SPDX-License-Identifier: BSD-2-Clause-Patent+;+; Module Name:+;+;  PeiCoreEntry.nasm+;+; Abstract:+;+;   Find and call SecStartup+;+;------------------------------------------------------------------------------++SECTION .text++extern ASM_PFX(SecStartup)+extern ASM_PFX(PlatformInit)+extern ASM_PFX(PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))++;-----------------------------------------------------------------------------+;  Macro:        PUSHA_64+;+;  Description:  Saves all registers on stack+;+;  Input:        None+;+;  Output:       None+;-----------------------------------------------------------------------------+%macro PUSHA_64   0+  push    r8+  push    r9+  push    r10+  push    r11+  push    r12+  push    r13+  push    r14+  push    r15+  push    rax+  push    rcx+  push    rdx+  push    rbx+  push    rsp+  push    rbp+  push    rsi+  push    rdi+%endmacro++;-----------------------------------------------------------------------------+;  Macro:        POPA_64+;+;  Description:  Restores all registers from stack+;+;  Input:        None+;+;  Output:       None+;-----------------------------------------------------------------------------+%macro POPA_64   0+  pop    rdi+  pop    rsi+  pop    rbp+  pop    rsp+  pop    rbx+  pop    rdx+  pop    rcx+  pop    rax+  pop    r15+  pop    r14+  pop    r13+  pop    r12+  pop    r11+  pop    r10+  pop    r9+  pop    r8+%endmacro++;+; 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++  ;+  ; Per X64 calling convention, make sure RSP is 16-byte aligned.+  ;+  mov     rax, rsp+  and     rax, 0fh+  sub     rsp, rax++  ;+  ; Pass entry point of the PEI core+  ;+  mov     rdi, 0FFFFFFE0h+  mov     edi, DWORD [rdi]+  mov     r9, rdi++  ;+  ; Pass BFV into the PEI Core+  ;+#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1+  ;+  ; Reset Vector and initial sec core (to initialize Temp Ram) is part of FSP O+  ; Default UefiCpuPkg Reset Vector locates FSP O as BFV+  ; However the Actual Sec Core that launches PEI is part of another Fv.+  ; We need to Pass that Fv as BFV to PEI Core+  ;+  xor     rcx, rcx+  mov     r8, ASM_PFX (PcdGet32 (PcdFspWrapperBfvforResetVectorInFsp))+  mov     ecx, DWORD[r8]+  mov     r8,  rcx+#else+  mov     r8, r10+#endif++  ;+  ; 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/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/SecEntry.nasm
new file mode 100644
index 0000000000..c4a95fbe10
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/X64/SecEntry.nasm
@@ -0,0 +1,214 @@
+;------------------------------------------------------------------------------+;+; Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>+; SPDX-License-Identifier: BSD-2-Clause-Patent+; Module Name:+;+;  SecEntry.nasm+;+; Abstract:+;+;  This is the code that passes control to 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)+extern   ASM_PFX(BoardBeforeTempRamInit)+; Pcds+extern   ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))+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 to PEI core.+;+; Return:       None+;+;  MMX Usage:+;              MM0 = BIST State+;+;----------------------------------------------------------------------------++BITS 64+align 16+global ASM_PFX(_ModuleEntryPoint)+ASM_PFX(_ModuleEntryPoint):+#if FixedPcdGet8(PcdFspWrapperResetVectorInFsp) == 1+  ;+  ; When Fsp holds reset vector, TempRamInitApi would be called by FSP itself+  ; and this will be the first call to FspWrapper. So, Skip calling TempRamInitApi+  ; and proceed further to launch PeiCore Entry.+  ;+  jmp    CallSecFspInit+#endif++  fninit                                ; clear any pending Floating point exceptions+  ;+  ; Store the BIST value in mm0+  ;+  movd    mm0, eax+  cli++  ;+  ; Check INIT# is asserted by port 0xCF9+  ;+  mov     dx, 0CF9h+  in      al, dx+  cmp     al, 04h+  jz      IssueWarmReset++  ; Trigger warm reset if PCIEBAR register is not in reset/default value state+  ;+  mov     eax, 80000060h ; PCIEX_BAR_REG B0:D0:F0:R60+  mov     dx,  0CF8h+  out     dx,  eax+  mov     dx,  0CFCh+  in      eax, dx+  cmp     eax, 0+  jz      NotWarmStart++IssueWarmReset:+  ;+  ; @note Issue warm reset, since if CPU only reset is issued not all MSRs are restored to their defaults+  ;+  mov     dx, 0CF9h+  mov     al, 06h+  out     dx, al+  jmp     $++NotWarmStart:+  ; Find 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         ; move return address to rsp+  mov     rcx, ASM_PFX(FsptUpdDataPtr)  ; TempRamInitParams++  ; Call the fsp TempRamInit Api+  jmp     rax++TempRamInitDone:+  mov     rbx, 0800000000000000Eh+  cmp     rax, rbx                ; Check if EFI_NOT_FOUND returned. Error code for Microcode Update not found.+  je      CallSecFspInit          ; If microcode not found, don't hang, but continue.++  test    rax, rax                ; Check if EFI_SUCCESS returned.+  jnz     FspApiFailed++  ; RDX: start of range+  ; R8: end of range+CallSecFspInit:+#if FixedPcdGet8(PcdFspModeSelection) == 1+  push    rax+  mov     rax, ASM_PFX(PcdGet32 (PcdFspTemporaryRamSize))+  sub     edx, dword [rax]        ; TemporaryRam for FSP+  pop     rax+#endif++  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  TempRamInitDonediff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/X64/Stack.nasm
new file mode 100644
index 0000000000..d7ae97c5da
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlat
+++ formSecLib/X64/Stack.nasm
@@ -0,0 +1,72 @@
+;------------------------------------------------------------------------------+;+; Copyright (c) 2023, 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+diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
similarity index 86%
rename from Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
rename to Platform/Intel/MinPlatformPkg/Include/Fsp.h
index 9f6cdcf476..1b86912583 100644
--- a/Platform/Intel/MinPlatformPkg/FspWrapper/Library/SecFspWrapperPlatformSecLib/Ia32/Fsp.h
+++ b/Platform/Intel/MinPlatformPkg/Include/Fsp.h
@@ -36,7 +36,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 // // Fsp Header //-#define FSP_HEADER_IMAGEBASE_OFFSET     0x1C-#define FSP_HEADER_TEMPRAMINIT_OFFSET   0x30+#define FSP_HEADER_IMAGEBASE_OFFSET   0x1C+#define FSP_HEADER_TEMPRAMINIT_OFFSET 0x30  #endifdiff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
index a14c6b2db5..8256e62510 100644
--- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
+++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec
@@ -393,6 +393,27 @@
   #   gMinPlatformPkgTokenSpaceGuid.PcdFspDispatchModeUseFspPeiMain|TRUE|BOOLEAN|0xF00000A8 +  ## Reset Vector in FSP+  # FALSE: Reset Vector is in FSP Wrapper+  # TRUE:  Reset Vector is in FSP+  #+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperResetVectorInFsp|FALSE|BOOLEAN|0xF00000A9++  ## BFV Location for Reset Vector in FSP+  # The default of BFV Location for Reset Vector in FSP is 0xFFFF0000.+  #+  gMinPlatformPkgTokenSpaceGuid.PcdFspWrapperBfvforResetVectorInFsp|0xFFFF0000|UINT32|0xF00000AA++  ## FSP-T UPD Header Revision+  # The default of FSP-T UPD Header Revision is 0.+  #+  gMinPlatformPkgTokenSpaceGuid.PcdFsptUpdHeaderRevision|0x0|UINT8|0xF00000AC++  ## FSP-T ARCH UPD Header Revision+  # The default of FSP-T ARCH UPD Header Revision is 0.+  #+  gMinPlatformPkgTokenSpaceGuid.PcdFsptArchUpdHeaderRevision|0x0|UINT8|0xF00000AD+ [PcdsFeatureFlag]    gMinPlatformPkgTokenSpaceGuid.PcdStopAfterDebugInit     |FALSE|BOOLEAN|0xF00000A1-- 
2.40.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112639): https://edk2.groups.io/g/devel/message/112639
Mute This Topic: https://groups.io/mt/103237142/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-