public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Chiu, Chasel" <chasel.chiu@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Duggapu, Chinni B" <chinni.b.duggapu@intel.com>
Cc: "Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>,
	"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-devel] [PATCH v5] IntelFsp2Pkg: Fsp T new ARCH UPD Support
Date: Wed, 3 Apr 2024 17:18:49 +0000	[thread overview]
Message-ID: <BN9PR11MB54839BB8DF9D89207E3734EAE63D2@BN9PR11MB5483.namprd11.prod.outlook.com> (raw)
In-Reply-To: <b9d291c97fb7cbaba42237e5c33b809e7fbaeeb2.1711083677.git.chinni.b.duggapu@intel.com>


Hi Chinni,

Thanks for working on this, please see my feedbacks below inline.

Thanks,
Chasel


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of cbduggap
> Sent: Thursday, March 21, 2024 10:02 PM
> To: devel@edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Desimone, Nathaniel
> L <nathaniel.l.desimone@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>;
> Duggapu, Chinni B <chinni.b.duggapu@intel.com>
> Subject: [edk2-devel] [PATCH v5] IntelFsp2Pkg: Fsp T new ARCH UPD Support
> 
> Changes to support spec changes
> 
> 1. Remove usage of Pcd.
> 2. Change code to validate the Temporary Ram size input.
> 3. Consume the input saved in YMM Register
> 
> Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Chiu Chasel <chasel.chiu@intel.com>
> Cc: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> 
> 
> Signed-off-by: Duggapu Chinni B <chinni.b.duggapu@intel.com>
> ---
>  IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf     |  1 +
>  IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf       |  1 +
>  .../FspSecCore/Ia32/Fsp24ApiEntryM.nasm       |  1 -
>  .../FspSecCore/Ia32/FspApiEntryM.nasm         |  1 -
>  .../FspSecCore/Ia32/FspApiEntryT.nasm         | 62 ++++++++++++++---
>  .../FspSecCore/Ia32/SaveRestoreSseNasm.inc    | 11 +++
>  IntelFsp2Pkg/FspSecCore/SecFsp.c              |  9 +++
>  IntelFsp2Pkg/FspSecCore/SecFsp.h              |  1 +
>  IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 69 +++++++++++++++----
>  IntelFsp2Pkg/Include/FspEas/FspApi.h          |  5 +-
>  IntelFsp2Pkg/Include/Library/FspPlatformLib.h | 13 ++++
>  .../Include/SaveRestoreSseAvxNasm.inc         | 21 ++++++
>  .../BaseFspPlatformLib/FspPlatformMemory.c    | 38 ++++++++++
>  .../SecRamInitData.c                          |  3 +-
>  14 files changed, 209 insertions(+), 27 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
> b/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
> index cb011f99f9..8cb0e6411f 100644
> --- a/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
> +++ b/IntelFsp2Pkg/FspSecCore/Fsp24SecCoreM.inf
> @@ -60,6 +60,7 @@
>    FspSecPlatformLib   CpuLib   FspMultiPhaseLib+  FspPlatformLib  [Pcd]
> gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase              ## CONSUMESdiff
> --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> index 8029832235..ef19c6ae78 100644
> --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> @@ -59,6 +59,7 @@
>    FspCommonLib   FspSecPlatformLib   CpuLib+  FspPlatformLib  [Pcd]
> gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamBase              ## CONSUMESdiff
> --git a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
> index 15f8ecea83..5fa5c03569 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/Fsp24ApiEntryM.nasm
> @@ -11,7 +11,6 @@
>  ; Following are fixed PCDs ; extern
> ASM_PFX(PcdGet32(PcdTemporaryRamBase))-extern
> ASM_PFX(PcdGet32(PcdTemporaryRamSize)) extern
> ASM_PFX(PcdGet32(PcdFspTemporaryRamSize)) extern   ASM_PFX(PcdGet8
> (PcdFspHeapSizePercentage)) diff --git
> a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> index 61ab4612a3..861cce4d01 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryM.nasm
> @@ -11,7 +11,6 @@
>  ; Following are fixed PCDs ; extern
> ASM_PFX(PcdGet32(PcdTemporaryRamBase))-extern
> ASM_PFX(PcdGet32(PcdTemporaryRamSize)) extern
> ASM_PFX(PcdGet32(PcdFspTemporaryRamSize)) extern   ASM_PFX(PcdGet8
> (PcdFspHeapSizePercentage)) diff --git
> a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 900126b93b..f72da0d5a9 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -109,7 +109,8 @@ struc LoadMicrocodeParamsFsp24
>      .FsptArchReserved:        resb    3     .FsptArchLength:          resd
> 1     .FspDebugHandler          resq    1-    .FsptArchUpd:             resd
> 4+    .FspTemporaryRamSize:     resd    1  ; Supported only if ArchRevison is >=
> 3+    .FsptArchUpd:             resd    3     ; }     ; FSPT_CORE_UPD
> {     .MicrocodeCodeAddr:       resq    1@@ -267,7 +268,7 @@
> ASM_PFX(LoadMicrocodeDefault):
>     cmp    byte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2    jb
> Fsp20UpdHeader    cmp    byte [esp +
> LoadMicrocodeParamsFsp22.FsptArchRevision], 2-   je     Fsp24UpdHeader+   jae
> Fsp24UpdHeader    jmp    Fsp22UpdHeader  Fsp20UpdHeader:@@ -405,7 +406,7
> @@ CheckAddress:
>     cmp   byte [esp + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2    jb
> Fsp20UpdHeader1    cmp    byte [esp +
> LoadMicrocodeParamsFsp22.FsptArchRevision], 2-   je     Fsp24UpdHeader1;+
> jae    Fsp24UpdHeader1;    jmp    Fsp22UpdHeader1  Fsp20UpdHeader1:@@ -
> 497,7 +498,8 @@ ASM_PFX(EstablishStackFsp):
>    ; Enable FSP STACK   ;   mov       esp, DWORD [ASM_PFX(PcdGet32
> (PcdTemporaryRamBase))]-  add       esp, DWORD [ASM_PFX(PcdGet32
> (PcdTemporaryRamSize))]+  LOAD_TEMPORARY_RAM_SIZE ecx+  add       esp, ecx
> push      DATA_LEN_OF_MCUD     ; Size of the data region   push
> 4455434Dh            ; Signature of the  data region 'MCUD'@@ -506,7 +508,7 @@
> ASM_PFX(EstablishStackFsp):
>    cmp       byte [edx + LoadMicrocodeParamsFsp22.FspUpdHeaderRevision], 2   jb
> Fsp20UpdHeader2   cmp       byte [esp +
> LoadMicrocodeParamsFsp22.FsptArchRevision], 2-  je        Fsp24UpdHeader2+
> jae       Fsp24UpdHeader2   jmp       Fsp22UpdHeader2  Fsp20UpdHeader2:@@ -
> 554,12 +556,13 @@ ContinueAfterUpdPush:
>    ;   ; Set ECX/EDX to the BootLoader temporary memory range   ;-  mov       ecx,
> [ASM_PFX(PcdGet32 (PcdTemporaryRamBase))]-  mov       edx, ecx-  add       edx,
> [ASM_PFX(PcdGet32 (PcdTemporaryRamSize))]+  mov       edx,
> [ASM_PFX(PcdGet32 (PcdTemporaryRamBase))]+  LOAD_TEMPORARY_RAM_SIZE
> ecx+  add       edx, ecx   sub       edx,  [ASM_PFX(PcdGet32
> (PcdFspReservedBufferSize))]+  mov       ecx,  [ASM_PFX(PcdGet32
> (PcdTemporaryRamBase))] -  cmp       ecx, edx        ;If PcdFspReservedBufferSize
> >= PcdTemporaryRamSize, then error.+  cmp       ecx, edx        ;If
> PcdFspReservedBufferSize >= TemporaryRamSize, then error.   jb
> EstablishStackFspSuccess   mov       eax, 80000003h  ;EFI_UNSUPPORTED   jmp
> EstablishStackFspExit@@ -599,6 +602,47 @@ ASM_PFX(TempRamInitApi):
>    CALL_EBP  ASM_PFX(LoadUpdPointerToECX) ; ECX for UPD param
> SAVE_ECX                               ; save UPD param to slot 3 in xmm6 +  mov       edx,
> ASM_PFX(PcdGet32 (PcdTemporaryRamSize))+  mov       edx, DWORD [edx]+  ;+  ;
> Read ARCH2 UPD input value.+  ;+  mov       ebx, DWORD [ecx +
> LoadMicrocodeParamsFsp24.FspTemporaryRamSize]+  ;+  ; As per spec, if
> Bootloader pass zero, use Fsp defined Size+  ; Irrespective of whether this UPD is
> supported or not, Fallback+  ; to Fsp defined size if input is zero.+  ;+  cmp       ebx,
> 0+  jz        UseTemporaryRamSizePcd++  xor       eax, eax+  mov       ax,  WORD [esi
> + 020h]      ; Read ImageAttribute+  test      ax,  16                     ; check if Bit4 is set+
> jnz       ConsumeInputConfiguration+  ;+  ; Sometimes user may change input value
> even if it is not supported+  ; return error if input is Non-Zero and not same as
> PcdTemporaryRamSize.+  ;+  cmp       ebx, edx+  je
> UseTemporaryRamSizePcd+  mov       eax, 080000002h      ;
> RETURN_INVALID_PARAMETER+  jmp



Potentially old platform bootloader may still input older version of FSPT_ARCH2_UPD buffer even FSP has implemented new FSP-T modules, in this case the FSPT_ARCH2_UPD->FspTemporaryRamSize is reserved bytes which should never cause FSP-T to return error. (current logic did not check structure revision and non-zero reserved bytes might cause FSP-T to return error)
I would recommend that FSP-T to return error only when FSPT_ARCH2_UPD->Revision is 3 or above and wrong FSPT_ARCH2_UPD->FspTemporaryRamSize was given.



> TempRamInitExit+ConsumeInputConfiguration:+  ;+  ; Read Fsp Arch2 revision+  ;+
> cmp       byte [ecx + LoadMicrocodeParamsFsp24.FsptArchRevision], 3+  jb
> UseTemporaryRamSizePcd+  ;+  ; Read ARCH2 UPD value and Save.+  ;+
> SAVE_TEMPORARY_RAM_SIZE ebx+  jmp
> GotTemporaryRamSize+UseTemporaryRamSizePcd:+
> SAVE_TEMPORARY_RAM_SIZE edx+GotTemporaryRamSize:+  LOAD_ECX   ;   ; Sec
> Platform Init   ;diff --git
> a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> index 016f943b43..4d6ec1e984 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/SaveRestoreSseNasm.inc
> @@ -128,6 +128,17 @@
>    SXMMN      xmm5, 1, eax              %endmacro +;+; XMM5 slot 2 for
> TemporaryRamSize+;+%macro LOAD_TEMPORARY_RAM_SIZE    1+  LXMMN
> xmm5, %1, 2+             %endmacro++%macro SAVE_TEMPORARY_RAM_SIZE    1+
> SXMMN      xmm5, 2, %1+             %endmacro+ %macro ENABLE_SSE
> 0             ;             ; Initialize floating point unitsdiff --git
> a/IntelFsp2Pkg/FspSecCore/SecFsp.c b/IntelFsp2Pkg/FspSecCore/SecFsp.c
> index 11be1f97ca..281d39a24b 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFsp.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c
> @@ -54,6 +54,7 @@ SecGetPlatformData (
>    UINT32         TopOfCar;   UINT32         *StackPtr;   UINT32         DwordSize;+
> UINT32         TemporaryRamSize;    FspPlatformData = &FspData->PlatformData;
> @@ -67,12 +68,20 @@ SecGetPlatformData (
>    FspPlatformData->MicrocodeRegionSize = 0;   FspPlatformData-
> >CodeRegionBase      = 0;   FspPlatformData->CodeRegionSize      = 0;+
> TemporaryRamSize                     = 0;    //   // Pointer to the size field   //   TopOfCar
> = PcdGet32 (PcdTemporaryRamBase) + PcdGet32 (PcdTemporaryRamSize);
> StackPtr = (UINT32 *)(TopOfCar - sizeof (UINT32));+  if ((*(StackPtr - 1) !=
> FSP_MCUD_SIGNATURE) && (FspData->FspInfoHeader->ImageAttribute & BIT4))
> {+    ReadTemporaryRamSize (PcdGet32 (PcdTemporaryRamBase),
> &TemporaryRamSize);+    if (TemporaryRamSize) {+      TopOfCar = PcdGet32
> (PcdTemporaryRamBase) + TemporaryRamSize;+      StackPtr = (UINT32
> *)(TopOfCar - sizeof (UINT32));+    }+  }    if (*(StackPtr - 1) ==
> FSP_MCUD_SIGNATURE) {     while (*StackPtr != 0) {diff --git
> a/IntelFsp2Pkg/FspSecCore/SecFsp.h b/IntelFsp2Pkg/FspSecCore/SecFsp.h
> index 693af29f20..c05b46c750 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecFsp.h
> +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.h
> @@ -17,6 +17,7 @@
>  #include <Library/BaseMemoryLib.h> #include <Library/FspCommonLib.h>
> #include <Library/FspSecPlatformLib.h>+#include <Library/FspPlatformLib.h>
> #define FSP_MCUD_SIGNATURE  SIGNATURE_32 ('M', 'C', 'U', 'D') #define
> FSP_PER0_SIGNATURE  SIGNATURE_32 ('P', 'E', 'R', '0')diff --git
> a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> index 698bb063a7..8cd157f2c3 100644
> --- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm
> @@ -16,6 +16,7 @@
>  extern   ASM_PFX(PcdGet32 (PcdTemporaryRamBase)) extern
> ASM_PFX(PcdGet32 (PcdTemporaryRamSize)) extern   ASM_PFX(PcdGet32
> (PcdFspReservedBufferSize))+extern   ASM_PFX(PcdGet32
> (PcdGlobalDataPointerAddress))  ; ; Following functions will be provided in
> PlatformSecLib@@ -76,7 +77,8 @@ struc LoadMicrocodeParamsFsp24
>      .FsptArchReserved:        resb    3     .FsptArchLength:          resd
> 1     .FspDebugHandler          resq    1-    .FsptArchUpd:             resd
> 4+    .FspTemporaryRamSize:     resd    1 ; Supported only if ArchRevison is >=
> 3+    .FsptArchUpd:             resd    3     ; }     ; FSPT_CORE_UPD
> {     .MicrocodeCodeAddr:       resq    1@@ -163,7 +165,7 @@
> ASM_PFX(LoadMicrocodeDefault):
>     cmp    byte [rsp + LoadMicrocodeParamsFsp24.FspUpdHeaderRevision], 2    jb
> ParamError    cmp    byte [rsp + LoadMicrocodeParamsFsp24.FsptArchRevision],
> 2-   jne    ParamError+   jb     ParamError     ; UPD structure is compliant with FSP
> spec 2.4    mov    rax, qword [rsp +
> LoadMicrocodeParamsFsp24.MicrocodeCodeSize]@@ -273,7 +275,7 @@
> CheckAddress:
>     cmp   byte [rsp + LoadMicrocodeParamsFsp24.FspUpdHeaderRevision], 2    jb
> ParamError    cmp   byte [rsp + LoadMicrocodeParamsFsp24.FsptArchRevision], 2-
> jne   ParamError+   jb    ParamError     ; UPD structure is compliant with FSP spec
> 2.4    ; Is automatic size detection ?@@ -337,8 +339,8 @@
> ASM_PFX(EstablishStackFsp):
>    ;   mov       rax, ASM_PFX(PcdGet32 (PcdTemporaryRamBase))   mov       esp,
> DWORD[rax]-  mov       rax, ASM_PFX(PcdGet32 (PcdTemporaryRamSize))-  add
> esp, DWORD[rax]+  LOAD_TEMPORARY_RAM_SIZE rax+  add       esp, eax    sub
> esp, 4   mov       dword[esp], DATA_LEN_OF_MCUD ; Size of the data region@@ -
> 349,7 +351,7 @@ ASM_PFX(EstablishStackFsp):
>    cmp       byte [rdx + LoadMicrocodeParamsFsp24.FspUpdHeaderRevision], 2   jb
> ParamError1   cmp       byte [rdx +
> LoadMicrocodeParamsFsp24.FsptArchRevision], 2-  je        Fsp24UpdHeader+  jnb
> Fsp24UpdHeader  ParamError1:   mov       rax, 08000000000000002h@@ -397,8
> +399,8 @@ ContinueAfterUpdPush:
>    ;   mov       rcx, ASM_PFX(PcdGet32 (PcdTemporaryRamBase))   mov       edx,
> [ecx]-  mov       rcx, ASM_PFX(PcdGet32 (PcdTemporaryRamSize))-  add       edx,
> [ecx]+  LOAD_TEMPORARY_RAM_SIZE rcx+  add       edx, ecx   mov       rcx,
> ASM_PFX(PcdGet32 (PcdFspReservedBufferSize))   sub       edx, [ecx]   mov       rcx,
> ASM_PFX(PcdGet32 (PcdTemporaryRamBase))@@ -439,6 +441,14 @@
> ASM_PFX(TempRamInitApi):
>    ;   SAVE_BFV  rbp +  ;+  ; Save timestamp into YMM6+  ;+  rdtsc+  shl       rdx, 32+
> or        rax, rdx+  SAVE_TS   rax+   ;   ; Save Input Parameter in YMM10   ;@@ -
> 455,14 +465,47 @@ ASM_PFX(TempRamInitApi):
>  ParamValid:   SAVE_RCX +  mov       rdx, ASM_PFX(PcdGet32
> (PcdTemporaryRamSize))+  mov       edx, DWORD [rdx]   ;-  ; Save timestamp into
> YMM6+  ; Read ARCH2 UPD input value.   ;-  rdtsc-  shl       rdx, 32-  or        rax, rdx-
> SAVE_TS   rax+  mov       ebx, DWORD [ecx +
> LoadMicrocodeParamsFsp24.FspTemporaryRamSize]+  ;+  ; As per spec, if
> Bootloader pass zero, use Fsp defined Size+  ; Irrespective of whether this UPD is
> supported or not, Fallback+  ; to Fsp defined size if input is zero.+  ;+  cmp       ebx,
> 0+  jz        UseTemporaryRamSizePcd++  xor       rax, rax+  mov       ax,  WORD [rsi +
> 020h]      ; Read ImageAttribute+  test      ax,  16                     ; check if Bit4 is set+
> jnz       ConsumeInputConfiguration+  ;+  ; Sometimes user may change input value
> even if it is not supported+  ; return error if input is Non-Zero and not same as
> PcdTemporaryRamSize.+  ;+  cmp       ebx, edx+  je
> UseTemporaryRamSizePcd+  mov       rax, 08000000000000002h      ;
> RETURN_INVALID_PARAMETER+  jmp
> TempRamInitExit+ConsumeInputConfiguration:+  ;+  ; Read Fsp Arch2 revision+
> cmp       byte [ecx + LoadMicrocodeParamsFsp24.FsptArchRevision], 3+  jb
> UseTemporaryRamSizePcd+  ;+  ; Read ARCH2 UPD value and Save.+  ; Only low-
> 32 bits of rbx/rdx holds the temporary ram size.+  ;+
> SAVE_TEMPORARY_RAM_SIZE rbx+  jmp
> GotTemporaryRamSize+UseTemporaryRamSizePcd:+
> SAVE_TEMPORARY_RAM_SIZE rdx +GotTemporaryRamSize:   ;   ; Sec Platform
> Init   ;diff --git a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> index 40e063e944..27d5ec3a3c 100644
> --- a/IntelFsp2Pkg/Include/FspEas/FspApi.h
> +++ b/IntelFsp2Pkg/Include/FspEas/FspApi.h
> @@ -139,7 +139,7 @@ typedef struct {
>  /// typedef struct {   ///-  /// Revision of the structure is 2 for this version of the
> specification.+  /// Revision of the structure is 3 for this version of the
> specification.   ///   UINT8                   Revision;   UINT8                   Reserved[3];@@
> -152,7 +152,8 @@ typedef struct {
>    /// occurring during FSP execution.   ///   EFI_PHYSICAL_ADDRESS
> FspDebugHandler;-  UINT8                   Reserved1[16];+  UINT32
> FspTemporaryRamSize;+  UINT8                   Reserved1[12]; } FSPT_ARCH2_UPD;




Please add comments to describe the FspTemporaryRamSize more.
For example:
It is valid only when FSP image attribute saying TemporaryRamSize customization is supported.
Zero means the FSP recommendation TempRamSize will be used.
Non-zero means the desired TempRamSize. (refer to FSP Integration guide for valid TempRamSize range for each platform)




> ///diff --git a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> index 081add6529..03eca5e1fc 100644
> --- a/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> +++ b/IntelFsp2Pkg/Include/Library/FspPlatformLib.h
> @@ -121,4 +121,17 @@ FspTempRamExitDone2 (
>    IN EFI_STATUS  Status   ); +/**+  Calculate TemporaryRam Size using Base
> address++  @param[in]  TemporaryRamBase         the address of target memory+
> @param[out] TemporaryRamSize         the size of target
> memory+**/+VOID+EFIAPI+ReadTemporaryRamSize (+  IN  UINT32
> TemporaryRamBase,+  OUT UINT32 *TemporaryRamSize+  );+ #endifdiff --git
> a/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> b/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> index 002a5a1412..2168564e6d 100644
> --- a/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> +++ b/IntelFsp2Pkg/Include/SaveRestoreSseAvxNasm.inc
> @@ -201,6 +201,27 @@
>              movq    rcx,  xmm5             %endmacro +;+; Save TemporaryRamSize to
> YMM10[192:255]+; arg 1:general purpose register which holds
> TemporaryRamSize+; Modified: XMM5 and YMM10[192:255]+;+%macro
> SAVE_TEMPORARY_RAM_SIZE     1+            LYMMN   ymm10, xmm5, 1+
> SXMMN   xmm5, 1, %1+            SYMMN   ymm10, 1, xmm5+
> %endmacro++;+; Restore TemporaryRamSize from YMM10[192:255]+; arg
> 1:general purpose register where to save TemporaryRamSize+; Modified: XMM5
> and %1+;+%macro LOAD_TEMPORARY_RAM_SIZE     1+            LYMMN   ymm10,
> xmm5, 1+            LXMMN   xmm5, %1, 1+            %endmacro+ ; ; YMM7[128:191]
> for calling stack ; arg 1:Entrydiff --git
> a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformMemory.c
> b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformMemory.c
> index 2573e4e421..4c5c1f824e 100644
> --- a/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformMemory.c
> +++ b/IntelFsp2Pkg/Library/BaseFspPlatformLib/FspPlatformMemory.c
> @@ -6,6 +6,7 @@
>  **/  #include <PiPei.h>+#include <Register/Intel/Msr.h> #include
> <Library/BaseLib.h> #include <Library/BaseMemoryLib.h> #include
> <Library/MemoryAllocationLib.h>@@ -119,3 +120,40 @@
> FspGetSystemMemorySize (
>      Hob.Raw = GET_NEXT_HOB (Hob);   } }++/**+  Calculate TemporaryRam Size
> using Base address++  @param[in]  TemporaryRamBase         the address of
> target memory+  @param[out] TemporaryRamSize         the size of target
> memory+**/+VOID+EFIAPI+ReadTemporaryRamSize (+  IN  UINT32
> TemporaryRamBase,+  OUT UINT32 *TemporaryRamSize+  )+{+
> MSR_IA32_MTRRCAP_REGISTER Msr;+  UINT32  MsrNum;+  UINT32
> MsrNumEnd;++  if (TemporaryRamBase == 0) {+    return ;+  }++
> *TemporaryRamSize = 0;+  Msr.Uint64 = AsmReadMsr64(MSR_IA32_MTRRCAP);+
> MsrNumEnd = MSR_IA32_MTRR_PHYSBASE0 + (2 * (Msr.Bits.VCNT));++  for
> (MsrNum = MSR_IA32_MTRR_PHYSBASE0; MsrNum < MsrNumEnd; MsrNum +=
> 2) {+    if ((AsmReadMsr64 (MsrNum+1) & BIT11) != 0 ) {+      if
> (TemporaryRamBase == (AsmReadMsr64 (MsrNum) & 0xFFFFF000)) {+
> *TemporaryRamSize = (~(AsmReadMsr64 (MsrNum+1) & 0xFFFFF000) + 1);+
> break;+      }+    }+  }+  return;+}+diff --git
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamIn
> itData.c
> b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamIn
> itData.c
> index fb0d9a8683..316c2fa86a 100644
> ---
> a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamIn
> itData.c
> +++ b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecR
> +++ amInitData.c
> @@ -49,8 +49,9 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST
> FSPT_UPD_CORE_DATA  FsptUpdDataPtr = {



1. Seems like old issue.
FSP_UPD_HEADER->Reserved is 23 bytes, but here in FsptUpdDataPtr we only have 17 bytes, better to match it so we do not create confusion.

2. Please split into 2 patches (one patch series) for IntelFsp2Pkg and IntelFsp2WraperPkg to align with development guidelines. (and execute Base/Tools/Scripts/PatchCheck.py to see if any other coding style issues)




>      },     0x00000020,     0x00000000,+    0x00000000,     {-      0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,+      0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00     }   },--
> 2.39.1.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#117027): https://edk2.groups.io/g/devel/message/117027
> Mute This Topic: https://groups.io/mt/105079990/1777047
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.chiu@intel.com] -=-
> =-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117375): https://edk2.groups.io/g/devel/message/117375
Mute This Topic: https://groups.io/mt/105079990/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



      reply	other threads:[~2024-04-03 17:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28  8:29 [edk2-devel] [PATCH v4] IntelFsp2Pkg: Fsp 2.x Changes cbduggap
2024-03-11 20:49 ` Nate DeSimone
2024-03-22  5:01 ` [edk2-devel] [PATCH v5] IntelFsp2Pkg: Fsp T new ARCH UPD Support cbduggap
2024-04-03 17:18   ` Chiu, Chasel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BN9PR11MB54839BB8DF9D89207E3734EAE63D2@BN9PR11MB5483.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox