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]
-=-=-=-=-=-=-=-=-=-=-=-
prev parent 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