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.chaga...@intel.com> Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> Cc: Chiu Chasel <chasel.c...@intel.com> Signed-off-by: cbduggap <chinni.b.dugg...@intel.com> --- .../FspSecCore/Ia32/Fsp24ApiEntryM.nasm | 1 - .../FspSecCore/Ia32/FspApiEntryM.nasm | 1 - .../FspSecCore/Ia32/FspApiEntryT.nasm | 110 ++++++++++++------ .../FspSecCore/Ia32/SaveRestoreSseNasm.inc | 11 ++ IntelFsp2Pkg/FspSecCore/SecFsp.c | 1 - IntelFsp2Pkg/FspSecCore/SecFspApiChk.c | 4 +- IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm | 69 +++++++++-- IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm | 6 +- IntelFsp2Pkg/Include/FspEas/FspApi.h | 5 +- .../Include/SaveRestoreSseAvxNasm.inc | 21 ++++ .../SecRamInitData.c | 3 +- 11 files changed, 175 insertions(+), 57 deletions(-) diff --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..5fca46ca7a 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 @@ -178,29 +179,6 @@ endstruc jmp ebp ; restore EIP from EBP %endmacro -; -; Load UPD region pointer in ECX -; -global ASM_PFX(LoadUpdPointerToECX) -ASM_PFX(LoadUpdPointerToECX): - ; - ; esp + 4 is input UPD parameter - ; If esp + 4 is NULL the default UPD should be used - ; ecx will be the UPD region that should be used - ; - mov ecx, dword [esp + 4] - cmp ecx, 0 - jnz ParamValid - - ; - ; Fall back to default UPD region - ; - CALL_EDI ASM_PFX(AsmGetFspInfoHeaderNoStack) - mov ecx, DWORD [eax + 01Ch] ; Read FsptImageBaseAddress - add ecx, DWORD [eax + 024h] ; Get Cfg Region base address = FsptImageBaseAddress + CfgRegionOffset -ParamValid: - RET_EBP - ; ; @todo: The strong/weak implementation does not work. ; This needs to be reviewed later. @@ -267,7 +245,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 +383,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 +475,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 eax + add esp, eax push DATA_LEN_OF_MCUD ; Size of the data region push 4455434Dh ; Signature of the data region 'MCUD' @@ -506,7 +485,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 +533,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 @@ -589,6 +569,30 @@ ASM_PFX(TempRamInitApi): ; SAVE_REGS + ; + ; + ; Read Fsp Info header and save in esi + ; + CALL_EDI ASM_PFX(AsmGetFspInfoHeaderNoStack) + xor esi, esi + mov esi, eax + ; + ; esp + 4 is input UPD parameter + ; If esp + 4 is NULL the default UPD should be used + ; ecx will be the UPD region that should be used + ; + mov ecx, dword [esp + 4] + cmp ecx, 0 + jnz ParamValid + + ; + ; Fall back to default UPD region + ; + xor ecx, ecx + mov ecx, DWORD [esi + 01Ch] ; Read FsptImageBaseAddress + add ecx, DWORD [esi + 024h] ; Get Cfg Region base address = FsptImageBaseAddress + CfgRegionOffset +ParamValid: + SAVE_ECX ; ; Save timestamp into XMM6 ; @@ -596,9 +600,47 @@ ASM_PFX(TempRamInitApi): SAVE_EAX SAVE_EDX - CALL_EBP ASM_PFX(LoadUpdPointerToECX) ; ECX for UPD param - SAVE_ECX ; save UPD param to slot 3 in xmm6 + 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 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 units diff --git a/IntelFsp2Pkg/FspSecCore/SecFsp.c b/IntelFsp2Pkg/FspSecCore/SecFsp.c index 11be1f97ca..4d519f726a 100644 --- a/IntelFsp2Pkg/FspSecCore/SecFsp.c +++ b/IntelFsp2Pkg/FspSecCore/SecFsp.c @@ -147,7 +147,6 @@ FspGlobalDataInit ( // It may have multiple FVs, so look into the last one for FSP header // PeiFspData->FspInfoHeader = (FSP_INFO_HEADER *)(UINTN)AsmGetFspInfoHeader (); - SecGetPlatformData (PeiFspData); // // Set API calling mode diff --git a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c index 5f59938518..33aaac66c1 100644 --- a/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c +++ b/IntelFsp2Pkg/FspSecCore/SecFspApiChk.c @@ -42,9 +42,7 @@ FspApiCallingCheck ( // // FspMemoryInit check // - if (((UINTN)FspData != MAX_ADDRESS) && ((UINTN)FspData != MAX_UINT32)) { - Status = EFI_UNSUPPORTED; - } else if (ApiParam == NULL) { + if (ApiParam == NULL) { Status = EFI_SUCCESS; } else if (EFI_ERROR (FspUpdSignatureCheck (ApiIdx, ApiParam))) { Status = EFI_INVALID_PARAMETER; diff --git a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm index 698bb063a7..feb8164134 100644 --- a/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm +++ b/IntelFsp2Pkg/FspSecCore/X64/FspApiEntryT.nasm @@ -76,7 +76,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 +164,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 +274,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 +338,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 +350,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 +398,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 +440,12 @@ ASM_PFX(TempRamInitApi): ; SAVE_BFV rbp + ; + ; Read Fsp Info header and save in rsi + ; + CALL_RDI ASM_PFX(AsmGetFspInfoHeaderNoStack) + xor rsi, rsi + mov rsi, rax ; ; Save Input Parameter in YMM10 ; @@ -448,10 +455,9 @@ ASM_PFX(TempRamInitApi): ; ; Fall back to default UPD ; - CALL_RDI ASM_PFX(AsmGetFspInfoHeaderNoStack) xor rcx, rcx - mov ecx, DWORD [rax + 01Ch] ; Read FsptImageBaseAddress - add ecx, DWORD [rax + 024h] ; Get Cfg Region base address = FsptImageBaseAddress + CfgRegionOffset + mov ecx, DWORD [rsi + 01Ch] ; Read FsptImageBaseAddress + add ecx, DWORD [rsi + 024h] ; Get Cfg Region base address = FsptImageBaseAddress + CfgRegionOffset ParamValid: SAVE_RCX @@ -463,6 +469,47 @@ ParamValid: or rax, rdx SAVE_TS rax + mov rdx, ASM_PFX(PcdGet32 (PcdTemporaryRamSize)) + mov edx, DWORD [rdx] + ; + ; 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 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/FspSecCore/X64/FspHelper.nasm b/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm index 32a60270b8..61ebaa8ccf 100644 --- a/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm +++ b/IntelFsp2Pkg/FspSecCore/X64/FspHelper.nasm @@ -28,7 +28,7 @@ ASM_PFX(FspInfoHeaderRelativeOff): global ASM_PFX(AsmGetFspInfoHeaderNoStack) ASM_PFX(AsmGetFspInfoHeaderNoStack): lea rax, [ASM_PFX(AsmGetFspInfoHeader)] - lea rcx, [ASM_PFX(FspInfoHeaderRelativeOff)] - mov ecx, [rcx] - sub rax, rcx + lea rsi, [ASM_PFX(FspInfoHeaderRelativeOff)] + mov esi, [rsi] + sub rax, rsi jmp rdi 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; /// diff --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:Entry diff --git a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamInitData.c b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamInitData.c index fb0d9a8683..316c2fa86a 100644 --- a/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamInitData.c +++ b/IntelFsp2WrapperPkg/Library/SecFspWrapperPlatformSecLibSample/SecRamInitData.c @@ -49,8 +49,9 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST FSPT_UPD_CORE_DATA FsptUpdDataPtr = { }, 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 (#115527): https://edk2.groups.io/g/devel/message/115527 Mute This Topic: https://groups.io/mt/104376655/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-