* [PATCH 0/4] Refactor MpInitLib
@ 2022-05-07 15:13 Ni, Ray
2022-05-07 15:13 ` [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase Ni, Ray
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Ni, Ray @ 2022-05-07 15:13 UTC (permalink / raw)
To: devel
Ray Ni (4):
MpInitLib: Allocate code buffer for PEI phase
MpInitLib: remove unneeded global ASM_PFX
MpInitLib: Put SEV logic in separate file
MpInitLib: Only allocate below 1MB memory for 16bit code
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +-
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +-
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +-
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------
UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +-
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +-----------------
9 files changed, 216 insertions(+), 236 deletions(-)
--
2.32.0.windows.1
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase 2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray @ 2022-05-07 15:13 ` Ni, Ray 2022-05-07 15:13 ` [PATCH 2/4] MpInitLib: remove unneeded global ASM_PFX Ni, Ray ` (4 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Ni, Ray @ 2022-05-07 15:13 UTC (permalink / raw) To: devel; +Cc: Eric Dong Today's implementation assumes PEI phase runs at 32bit so the execution-disable feature is not applicable. It's not always TRUE. The patch allocates 32bit&64bit code buffer for PEI phase as well. Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- UefiCpuPkg/Library/MpInitLib/MpLib.c | 2 +- UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 ++++++++++----- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index 60d14a5a0e..78cc3e2b93 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -162,7 +162,7 @@ GetWakeupBuffer ( @retval 0 Cannot find free memory below 4GB. **/ UINTN -GetModeTransitionBuffer ( +AllocateCodeBuffer ( IN UINTN BufferSize ) { diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 91c7afaeb2..3dc1b9f872 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1058,7 +1058,7 @@ AllocateResetVector ( (CpuMpData->WakeupBuffer + CpuMpData->AddressMap.RendezvousFunnelSize + CpuMpData->AddressMap.SwitchToRealSize); - CpuMpData->WakeupBufferHigh = GetModeTransitionBuffer ( + CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( CpuMpData->AddressMap.RendezvousFunnelSize + CpuMpData->AddressMap.SwitchToRealSize - CpuMpData->AddressMap.ModeTransitionOffset diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index f8c52426dd..59ab960897 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -442,7 +442,7 @@ GetWakeupBuffer ( @retval 0 Cannot find free memory below 4GB. **/ UINTN -GetModeTransitionBuffer ( +AllocateCodeBuffer ( IN UINTN BufferSize ); diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c index efce574727..65400b95a2 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -299,14 +299,19 @@ GetWakeupBuffer ( @retval 0 Cannot find free memory below 4GB. **/ UINTN -GetModeTransitionBuffer ( +AllocateCodeBuffer ( IN UINTN BufferSize ) { - // - // PEI phase doesn't need to do such transition. So simply return 0. - // - return 0; + EFI_STATUS Status; + EFI_PHYSICAL_ADDRESS Address; + + Status = PeiServicesAllocatePages (EfiBootServicesCode, EFI_SIZE_TO_PAGES (BufferSize), &Address); + if (EFI_ERROR (Status)) { + Address = 0; + } + + return (UINTN)Address; } /** -- 2.32.0.windows.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/4] MpInitLib: remove unneeded global ASM_PFX 2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray 2022-05-07 15:13 ` [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase Ni, Ray @ 2022-05-07 15:13 ` Ni, Ray 2022-05-07 15:13 ` [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray ` (3 subsequent siblings) 5 siblings, 0 replies; 14+ messages in thread From: Ni, Ray @ 2022-05-07 15:13 UTC (permalink / raw) To: devel global in NASM file is used for symbols that are referenced in C files. Remove unneeded global keyword in NASM file. Signed-off-by: Ray Ni <ray.ni@intel.com> --- UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 8 +------- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 10 ++-------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 7bd2415670..8981c32722 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -1,5 +1,5 @@ ;------------------------------------------------------------------------------ ; -; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR> +; Copyright (c) 2015 - 2022, Intel Corporation. All rights reserved.<BR> ; SPDX-License-Identifier: BSD-2-Clause-Patent ; ; Module Name: @@ -24,8 +24,6 @@ SECTION .text ;ALSO THIS PROCEDURE IS EXECUTED BY APs ONLY ON 16 BIT MODE. HENCE THIS PROC ;IS IN MACHINE CODE. ;------------------------------------------------------------------------------------- -global ASM_PFX(RendezvousFunnelProc) -ASM_PFX(RendezvousFunnelProc): RendezvousFunnelProcStart: ; At this point CS = 0x(vv00) and ip= 0x0. BITS 16 @@ -207,8 +205,6 @@ RendezvousFunnelProcEnd: ;SwitchToRealProc procedure follows. ;NOT USED IN 32 BIT MODE. ;------------------------------------------------------------------------------------- -global ASM_PFX(SwitchToRealProc) -ASM_PFX(SwitchToRealProc): SwitchToRealProcStart: jmp $ ; Never reach here SwitchToRealProcEnd: @@ -219,8 +215,6 @@ SwitchToRealProcEnd: ; The last three parameters (Pm16CodeSegment, SevEsAPJumpTable and WakeupBuffer) are ; specific to SEV-ES support and are not applicable on IA32. ;------------------------------------------------------------------------------------- -global ASM_PFX(AsmRelocateApLoop) -ASM_PFX(AsmRelocateApLoop): AsmRelocateApLoopStart: mov eax, esp mov esp, [eax + 16] ; TopOfApStack diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index f1422fd30a..d7e0e1fabd 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -35,8 +35,6 @@ SECTION .text ;ALSO THIS PROCEDURE IS EXECUTED BY APs ONLY ON 16 BIT MODE. HENCE THIS PROC ;IS IN MACHINE CODE. ;------------------------------------------------------------------------------------- -global ASM_PFX(RendezvousFunnelProc) -ASM_PFX(RendezvousFunnelProc): RendezvousFunnelProcStart: ; At this point CS = 0x(vv00) and ip= 0x0. ; Save BIST information to ebp firstly @@ -279,8 +277,6 @@ RendezvousFunnelProcEnd: ; r8 - Code32 Selector Offset ; r9 - Stack Start ;------------------------------------------------------------------------------------- -global ASM_PFX(SwitchToRealProc) -ASM_PFX(SwitchToRealProc): SwitchToRealProcStart: BITS 64 cli @@ -421,8 +417,6 @@ SwitchToRealProcEnd: ;------------------------------------------------------------------------------------- ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); ;------------------------------------------------------------------------------------- -global ASM_PFX(AsmRelocateApLoop) -ASM_PFX(AsmRelocateApLoop): AsmRelocateApLoopStart: BITS 64 cmp qword [rsp + 56], 0 ; SevEsAPJumpTable @@ -594,11 +588,11 @@ AsmRelocateApLoopEnd: ;------------------------------------------------------------------------------------- global ASM_PFX(AsmGetAddressMap) ASM_PFX(AsmGetAddressMap): - lea rax, [ASM_PFX(RendezvousFunnelProc)] + lea rax, [RendezvousFunnelProcStart] mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], rax mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], LongModeStart - RendezvousFunnelProcStart mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart - lea rax, [ASM_PFX(AsmRelocateApLoop)] + lea rax, [AsmRelocateApLoopStart] mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], rax mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart -- 2.32.0.windows.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/4] MpInitLib: Put SEV logic in separate file 2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray 2022-05-07 15:13 ` [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase Ni, Ray 2022-05-07 15:13 ` [PATCH 2/4] MpInitLib: remove unneeded global ASM_PFX Ni, Ray @ 2022-05-07 15:13 ` Ni, Ray 2022-05-12 14:13 ` Lendacky, Thomas 2022-05-07 15:13 ` [PATCH 4/4] MpInitLib: Only allocate below 1MB memory for 16bit code Ni, Ray ` (2 subsequent siblings) 5 siblings, 1 reply; 14+ messages in thread From: Ni, Ray @ 2022-05-07 15:13 UTC (permalink / raw) To: devel Cc: Eric Dong, Rahul Kumar, Michael Roth, James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky, Jordan Justen, Ard Biesheuvel, Erdem Aktas, Gerd Hoffmann Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Michael Roth <michael.roth@amd.com> Cc: James Bottomley <jejb@linux.ibm.com> Cc: Min Xu <min.m.xu@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Erdem Aktas <erdemaktas@google.com> Cc: Gerd Hoffmann <kraxel@redhat.com> --- .../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +- UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +- UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++ UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +----------------- 4 files changed, 159 insertions(+), 162 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 8981c32722..67f9ed05cf 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -199,7 +199,6 @@ CProcedureInvoke: call eax ; Invoke C function jmp $ ; Never reach here -RendezvousFunnelProcEnd: ;------------------------------------------------------------------------------------- ;SwitchToRealProc procedure follows. @@ -209,6 +208,8 @@ SwitchToRealProcStart: jmp $ ; Never reach here SwitchToRealProcEnd: +RendezvousFunnelProcEnd: + ;------------------------------------------------------------------------------------- ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); ; diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 3dc1b9f872..722ff3fd42 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -938,8 +938,7 @@ FillExchangeInfoData ( // EfiBootServicesCode to avoid page fault if NX memory protection is enabled. // if (CpuMpData->WakeupBufferHigh != 0) { - Size = CpuMpData->AddressMap.RendezvousFunnelSize + - CpuMpData->AddressMap.SwitchToRealSize - + Size = CpuMpData->AddressMap.RendezvousFunnelSize - CpuMpData->AddressMap.ModeTransitionOffset; CopyMem ( (VOID *)CpuMpData->WakeupBufferHigh, @@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer ( CopyMem ( (VOID *)CpuMpData->WakeupBuffer, (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress, - CpuMpData->AddressMap.RendezvousFunnelSize + - CpuMpData->AddressMap.SwitchToRealSize + CpuMpData->AddressMap.RendezvousFunnelSize ); } @@ -1031,7 +1029,6 @@ GetApResetVectorSize ( UINTN Size; Size = AddressMap->RendezvousFunnelSize + - AddressMap->SwitchToRealSize + sizeof (MP_CPU_EXCHANGE_INFO); return Size; @@ -1056,11 +1053,9 @@ AllocateResetVector ( CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize); CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN) (CpuMpData->WakeupBuffer + - CpuMpData->AddressMap.RendezvousFunnelSize + - CpuMpData->AddressMap.SwitchToRealSize); + CpuMpData->AddressMap.RendezvousFunnelSize); CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( - CpuMpData->AddressMap.RendezvousFunnelSize + - CpuMpData->AddressMap.SwitchToRealSize - + CpuMpData->AddressMap.RendezvousFunnelSize - CpuMpData->AddressMap.ModeTransitionOffset ); // diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm index 8bb1161fa0..7c2469f9c5 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm @@ -198,3 +198,151 @@ RestoreGhcb: SevEsGetApicIdExit: OneTimeCallRet SevEsGetApicId + + +;------------------------------------------------------------------------------------- +;SwitchToRealProc procedure follows. +;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC +;IS IN MACHINE CODE. +; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) +; rcx - Buffer Start +; rdx - Code16 Selector Offset +; r8 - Code32 Selector Offset +; r9 - Stack Start +;------------------------------------------------------------------------------------- +SwitchToRealProcStart: +BITS 64 + cli + + ; + ; Get RDX reset value before changing stacks since the + ; new stack won't be able to accomodate a #VC exception. + ; + push rax + push rbx + push rcx + push rdx + + mov rax, 1 + cpuid + mov rsi, rax ; Save off the reset value for RDX + + pop rdx + pop rcx + pop rbx + pop rax + + ; + ; Establish stack below 1MB + ; + mov rsp, r9 + + ; + ; Push ultimate Reset Vector onto the stack + ; + mov rax, rcx + shr rax, 4 + push word 0x0002 ; RFLAGS + push ax ; CS + push word 0x0000 ; RIP + push word 0x0000 ; For alignment, will be discarded + + ; + ; Get address of "16-bit operand size" label + ; + lea rbx, [PM16Mode] + + ; + ; Push addresses used to change to compatibility mode + ; + lea rax, [CompatMode] + push r8 + push rax + + ; + ; Clear R8 - R15, for reset, before going into 32-bit mode + ; + xor r8, r8 + xor r9, r9 + xor r10, r10 + xor r11, r11 + xor r12, r12 + xor r13, r13 + xor r14, r14 + xor r15, r15 + + ; + ; Far return into 32-bit mode + ; + retfq + +BITS 32 +CompatMode: + ; + ; Set up stack to prepare for exiting protected mode + ; + push edx ; Code16 CS + push ebx ; PM16Mode label address + + ; + ; Disable paging + ; + mov eax, cr0 ; Read CR0 + btr eax, 31 ; Set PG=0 + mov cr0, eax ; Write CR0 + + ; + ; Disable long mode + ; + mov ecx, 0c0000080h ; EFER MSR number + rdmsr ; Read EFER + btr eax, 8 ; Set LME=0 + wrmsr ; Write EFER + + ; + ; Disable PAE + ; + mov eax, cr4 ; Read CR4 + btr eax, 5 ; Set PAE=0 + mov cr4, eax ; Write CR4 + + mov edx, esi ; Restore RDX reset value + + ; + ; Switch to 16-bit operand size + ; + retf + +BITS 16 + ; + ; At entry to this label + ; - RDX will have its reset value + ; - On the top of the stack + ; - Alignment data (two bytes) to be discarded + ; - IP for Real Mode (two bytes) + ; - CS for Real Mode (two bytes) + ; + ; This label is also used with AsmRelocateApLoop. During MP finalization, + ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of + ; the WakeupBuffer, allowing a parked AP to be booted by an OS. + ; +PM16Mode: + mov eax, cr0 ; Read CR0 + btr eax, 0 ; Set PE=0 + mov cr0, eax ; Write CR0 + + pop ax ; Discard alignment data + + ; + ; Clear registers (except RDX and RSP) before going into 16-bit mode + ; + xor eax, eax + xor ebx, ebx + xor ecx, ecx + xor esi, esi + xor edi, edi + xor ebp, ebp + + iret + +SwitchToRealProcEnd: diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index d7e0e1fabd..53df478661 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -152,11 +152,6 @@ SkipEnable5LevelPaging: BITS 64 -; -; Required for the AMD SEV helper functions -; -%include "AmdSev.nasm" - LongModeStart: mov esi, ebx lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)] @@ -265,154 +260,12 @@ CProcedureInvoke: add rsp, 20h jmp $ ; Should never reach here -RendezvousFunnelProcEnd: - -;------------------------------------------------------------------------------------- -;SwitchToRealProc procedure follows. -;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC -;IS IN MACHINE CODE. -; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) -; rcx - Buffer Start -; rdx - Code16 Selector Offset -; r8 - Code32 Selector Offset -; r9 - Stack Start -;------------------------------------------------------------------------------------- -SwitchToRealProcStart: -BITS 64 - cli - - ; - ; Get RDX reset value before changing stacks since the - ; new stack won't be able to accomodate a #VC exception. - ; - push rax - push rbx - push rcx - push rdx - - mov rax, 1 - cpuid - mov rsi, rax ; Save off the reset value for RDX - - pop rdx - pop rcx - pop rbx - pop rax - - ; - ; Establish stack below 1MB - ; - mov rsp, r9 - - ; - ; Push ultimate Reset Vector onto the stack - ; - mov rax, rcx - shr rax, 4 - push word 0x0002 ; RFLAGS - push ax ; CS - push word 0x0000 ; RIP - push word 0x0000 ; For alignment, will be discarded - - ; - ; Get address of "16-bit operand size" label - ; - lea rbx, [PM16Mode] - - ; - ; Push addresses used to change to compatibility mode - ; - lea rax, [CompatMode] - push r8 - push rax - - ; - ; Clear R8 - R15, for reset, before going into 32-bit mode - ; - xor r8, r8 - xor r9, r9 - xor r10, r10 - xor r11, r11 - xor r12, r12 - xor r13, r13 - xor r14, r14 - xor r15, r15 - - ; - ; Far return into 32-bit mode - ; - retfq - -BITS 32 -CompatMode: - ; - ; Set up stack to prepare for exiting protected mode - ; - push edx ; Code16 CS - push ebx ; PM16Mode label address - - ; - ; Disable paging - ; - mov eax, cr0 ; Read CR0 - btr eax, 31 ; Set PG=0 - mov cr0, eax ; Write CR0 - - ; - ; Disable long mode - ; - mov ecx, 0c0000080h ; EFER MSR number - rdmsr ; Read EFER - btr eax, 8 ; Set LME=0 - wrmsr ; Write EFER - - ; - ; Disable PAE - ; - mov eax, cr4 ; Read CR4 - btr eax, 5 ; Set PAE=0 - mov cr4, eax ; Write CR4 - - mov edx, esi ; Restore RDX reset value - - ; - ; Switch to 16-bit operand size - ; - retf - -BITS 16 - ; - ; At entry to this label - ; - RDX will have its reset value - ; - On the top of the stack - ; - Alignment data (two bytes) to be discarded - ; - IP for Real Mode (two bytes) - ; - CS for Real Mode (two bytes) - ; - ; This label is also used with AsmRelocateApLoop. During MP finalization, - ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of - ; the WakeupBuffer, allowing a parked AP to be booted by an OS. - ; -PM16Mode: - mov eax, cr0 ; Read CR0 - btr eax, 0 ; Set PE=0 - mov cr0, eax ; Write CR0 - - pop ax ; Discard alignment data - - ; - ; Clear registers (except RDX and RSP) before going into 16-bit mode - ; - xor eax, eax - xor ebx, ebx - xor ecx, ecx - xor esi, esi - xor edi, edi - xor ebp, ebp - - iret +; +; Required for the AMD SEV helper functions +; +%include "AmdSev.nasm" -SwitchToRealProcEnd: +RendezvousFunnelProcEnd: ;------------------------------------------------------------------------------------- ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); -- 2.32.0.windows.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] MpInitLib: Put SEV logic in separate file 2022-05-07 15:13 ` [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray @ 2022-05-12 14:13 ` Lendacky, Thomas 2022-05-16 3:51 ` Ni, Ray 0 siblings, 1 reply; 14+ messages in thread From: Lendacky, Thomas @ 2022-05-12 14:13 UTC (permalink / raw) To: Ray Ni, devel Cc: Eric Dong, Rahul Kumar, Michael Roth, James Bottomley, Min Xu, Jiewen Yao, Jordan Justen, Ard Biesheuvel, Erdem Aktas, Gerd Hoffmann On 5/7/22 10:13, Ray Ni wrote: Probably should have a commit message here explaining the reason for the changes. Overall, this works, but it does seem strange to have the SwitchToRealProc procedure in the middle of the RendezvousFunnelProc procedure. Not sure if it would be worth just creating a second SEV nasm file (and maybe renaming the first one): AmdSevRendevous.nasm AmdSevSwitchToReal.nasm and then including the two in different locations. Then you wouldn't have to change any of the size calculations either. If you want to keep the function within the function and eliminate the use of SwitchToRealSize, you should probably update the struct in MpLib.h to remove SwitchToRealSize and then update the Ia32 and X64 AsmGetAddressMap function to no longer set SwitchToRealSize (or just reserve the field so that none of the other offsets change and just remove the set in AsmGetAddressMap). Thanks, Tom > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Michael Roth <michael.roth@amd.com> > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Min Xu <min.m.xu@intel.com> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > --- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +- > UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +----------------- > 4 files changed, 159 insertions(+), 162 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 8981c32722..67f9ed05cf 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > @@ -199,7 +199,6 @@ CProcedureInvoke: > call eax ; Invoke C function > > jmp $ ; Never reach here > -RendezvousFunnelProcEnd: > > ;------------------------------------------------------------------------------------- > ;SwitchToRealProc procedure follows. > @@ -209,6 +208,8 @@ SwitchToRealProcStart: > jmp $ ; Never reach here > SwitchToRealProcEnd: > > +RendezvousFunnelProcEnd: > + > ;------------------------------------------------------------------------------------- > ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); > ; > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 3dc1b9f872..722ff3fd42 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -938,8 +938,7 @@ FillExchangeInfoData ( > // EfiBootServicesCode to avoid page fault if NX memory protection is enabled. > // > if (CpuMpData->WakeupBufferHigh != 0) { > - Size = CpuMpData->AddressMap.RendezvousFunnelSize + > - CpuMpData->AddressMap.SwitchToRealSize - > + Size = CpuMpData->AddressMap.RendezvousFunnelSize - > CpuMpData->AddressMap.ModeTransitionOffset; > CopyMem ( > (VOID *)CpuMpData->WakeupBufferHigh, > @@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer ( > CopyMem ( > (VOID *)CpuMpData->WakeupBuffer, > (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress, > - CpuMpData->AddressMap.RendezvousFunnelSize + > - CpuMpData->AddressMap.SwitchToRealSize > + CpuMpData->AddressMap.RendezvousFunnelSize > ); > } > > @@ -1031,7 +1029,6 @@ GetApResetVectorSize ( > UINTN Size; > > Size = AddressMap->RendezvousFunnelSize + > - AddressMap->SwitchToRealSize + > sizeof (MP_CPU_EXCHANGE_INFO); > > return Size; > @@ -1056,11 +1053,9 @@ AllocateResetVector ( > CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize); > CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN) > (CpuMpData->WakeupBuffer + > - CpuMpData->AddressMap.RendezvousFunnelSize + > - CpuMpData->AddressMap.SwitchToRealSize); > + CpuMpData->AddressMap.RendezvousFunnelSize); > CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( > - CpuMpData->AddressMap.RendezvousFunnelSize + > - CpuMpData->AddressMap.SwitchToRealSize - > + CpuMpData->AddressMap.RendezvousFunnelSize - > CpuMpData->AddressMap.ModeTransitionOffset > ); > // > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > index 8bb1161fa0..7c2469f9c5 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > @@ -198,3 +198,151 @@ RestoreGhcb: > > SevEsGetApicIdExit: > OneTimeCallRet SevEsGetApicId > + > + > +;------------------------------------------------------------------------------------- > +;SwitchToRealProc procedure follows. > +;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC > +;IS IN MACHINE CODE. > +; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) > +; rcx - Buffer Start > +; rdx - Code16 Selector Offset > +; r8 - Code32 Selector Offset > +; r9 - Stack Start > +;------------------------------------------------------------------------------------- > +SwitchToRealProcStart: > +BITS 64 > + cli > + > + ; > + ; Get RDX reset value before changing stacks since the > + ; new stack won't be able to accomodate a #VC exception. > + ; > + push rax > + push rbx > + push rcx > + push rdx > + > + mov rax, 1 > + cpuid > + mov rsi, rax ; Save off the reset value for RDX > + > + pop rdx > + pop rcx > + pop rbx > + pop rax > + > + ; > + ; Establish stack below 1MB > + ; > + mov rsp, r9 > + > + ; > + ; Push ultimate Reset Vector onto the stack > + ; > + mov rax, rcx > + shr rax, 4 > + push word 0x0002 ; RFLAGS > + push ax ; CS > + push word 0x0000 ; RIP > + push word 0x0000 ; For alignment, will be discarded > + > + ; > + ; Get address of "16-bit operand size" label > + ; > + lea rbx, [PM16Mode] > + > + ; > + ; Push addresses used to change to compatibility mode > + ; > + lea rax, [CompatMode] > + push r8 > + push rax > + > + ; > + ; Clear R8 - R15, for reset, before going into 32-bit mode > + ; > + xor r8, r8 > + xor r9, r9 > + xor r10, r10 > + xor r11, r11 > + xor r12, r12 > + xor r13, r13 > + xor r14, r14 > + xor r15, r15 > + > + ; > + ; Far return into 32-bit mode > + ; > + retfq > + > +BITS 32 > +CompatMode: > + ; > + ; Set up stack to prepare for exiting protected mode > + ; > + push edx ; Code16 CS > + push ebx ; PM16Mode label address > + > + ; > + ; Disable paging > + ; > + mov eax, cr0 ; Read CR0 > + btr eax, 31 ; Set PG=0 > + mov cr0, eax ; Write CR0 > + > + ; > + ; Disable long mode > + ; > + mov ecx, 0c0000080h ; EFER MSR number > + rdmsr ; Read EFER > + btr eax, 8 ; Set LME=0 > + wrmsr ; Write EFER > + > + ; > + ; Disable PAE > + ; > + mov eax, cr4 ; Read CR4 > + btr eax, 5 ; Set PAE=0 > + mov cr4, eax ; Write CR4 > + > + mov edx, esi ; Restore RDX reset value > + > + ; > + ; Switch to 16-bit operand size > + ; > + retf > + > +BITS 16 > + ; > + ; At entry to this label > + ; - RDX will have its reset value > + ; - On the top of the stack > + ; - Alignment data (two bytes) to be discarded > + ; - IP for Real Mode (two bytes) > + ; - CS for Real Mode (two bytes) > + ; > + ; This label is also used with AsmRelocateApLoop. During MP finalization, > + ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of > + ; the WakeupBuffer, allowing a parked AP to be booted by an OS. > + ; > +PM16Mode: > + mov eax, cr0 ; Read CR0 > + btr eax, 0 ; Set PE=0 > + mov cr0, eax ; Write CR0 > + > + pop ax ; Discard alignment data > + > + ; > + ; Clear registers (except RDX and RSP) before going into 16-bit mode > + ; > + xor eax, eax > + xor ebx, ebx > + xor ecx, ecx > + xor esi, esi > + xor edi, edi > + xor ebp, ebp > + > + iret > + > +SwitchToRealProcEnd: > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index d7e0e1fabd..53df478661 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > @@ -152,11 +152,6 @@ SkipEnable5LevelPaging: > > BITS 64 > > -; > -; Required for the AMD SEV helper functions > -; > -%include "AmdSev.nasm" > - > LongModeStart: > mov esi, ebx > lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)] > @@ -265,154 +260,12 @@ CProcedureInvoke: > add rsp, 20h > jmp $ ; Should never reach here > > -RendezvousFunnelProcEnd: > - > -;------------------------------------------------------------------------------------- > -;SwitchToRealProc procedure follows. > -;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC > -;IS IN MACHINE CODE. > -; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) > -; rcx - Buffer Start > -; rdx - Code16 Selector Offset > -; r8 - Code32 Selector Offset > -; r9 - Stack Start > -;------------------------------------------------------------------------------------- > -SwitchToRealProcStart: > -BITS 64 > - cli > - > - ; > - ; Get RDX reset value before changing stacks since the > - ; new stack won't be able to accomodate a #VC exception. > - ; > - push rax > - push rbx > - push rcx > - push rdx > - > - mov rax, 1 > - cpuid > - mov rsi, rax ; Save off the reset value for RDX > - > - pop rdx > - pop rcx > - pop rbx > - pop rax > - > - ; > - ; Establish stack below 1MB > - ; > - mov rsp, r9 > - > - ; > - ; Push ultimate Reset Vector onto the stack > - ; > - mov rax, rcx > - shr rax, 4 > - push word 0x0002 ; RFLAGS > - push ax ; CS > - push word 0x0000 ; RIP > - push word 0x0000 ; For alignment, will be discarded > - > - ; > - ; Get address of "16-bit operand size" label > - ; > - lea rbx, [PM16Mode] > - > - ; > - ; Push addresses used to change to compatibility mode > - ; > - lea rax, [CompatMode] > - push r8 > - push rax > - > - ; > - ; Clear R8 - R15, for reset, before going into 32-bit mode > - ; > - xor r8, r8 > - xor r9, r9 > - xor r10, r10 > - xor r11, r11 > - xor r12, r12 > - xor r13, r13 > - xor r14, r14 > - xor r15, r15 > - > - ; > - ; Far return into 32-bit mode > - ; > - retfq > - > -BITS 32 > -CompatMode: > - ; > - ; Set up stack to prepare for exiting protected mode > - ; > - push edx ; Code16 CS > - push ebx ; PM16Mode label address > - > - ; > - ; Disable paging > - ; > - mov eax, cr0 ; Read CR0 > - btr eax, 31 ; Set PG=0 > - mov cr0, eax ; Write CR0 > - > - ; > - ; Disable long mode > - ; > - mov ecx, 0c0000080h ; EFER MSR number > - rdmsr ; Read EFER > - btr eax, 8 ; Set LME=0 > - wrmsr ; Write EFER > - > - ; > - ; Disable PAE > - ; > - mov eax, cr4 ; Read CR4 > - btr eax, 5 ; Set PAE=0 > - mov cr4, eax ; Write CR4 > - > - mov edx, esi ; Restore RDX reset value > - > - ; > - ; Switch to 16-bit operand size > - ; > - retf > - > -BITS 16 > - ; > - ; At entry to this label > - ; - RDX will have its reset value > - ; - On the top of the stack > - ; - Alignment data (two bytes) to be discarded > - ; - IP for Real Mode (two bytes) > - ; - CS for Real Mode (two bytes) > - ; > - ; This label is also used with AsmRelocateApLoop. During MP finalization, > - ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of > - ; the WakeupBuffer, allowing a parked AP to be booted by an OS. > - ; > -PM16Mode: > - mov eax, cr0 ; Read CR0 > - btr eax, 0 ; Set PE=0 > - mov cr0, eax ; Write CR0 > - > - pop ax ; Discard alignment data > - > - ; > - ; Clear registers (except RDX and RSP) before going into 16-bit mode > - ; > - xor eax, eax > - xor ebx, ebx > - xor ecx, ecx > - xor esi, esi > - xor edi, edi > - xor ebp, ebp > - > - iret > +; > +; Required for the AMD SEV helper functions > +; > +%include "AmdSev.nasm" > > -SwitchToRealProcEnd: > +RendezvousFunnelProcEnd: > > ;------------------------------------------------------------------------------------- > ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer); ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/4] MpInitLib: Put SEV logic in separate file 2022-05-12 14:13 ` Lendacky, Thomas @ 2022-05-16 3:51 ` Ni, Ray 0 siblings, 0 replies; 14+ messages in thread From: Ni, Ray @ 2022-05-16 3:51 UTC (permalink / raw) To: Tom Lendacky, devel@edk2.groups.io Cc: Dong, Eric, Kumar, Rahul1, Michael Roth, James Bottomley, Xu, Min M, Yao, Jiewen, Justen, Jordan L, Ard Biesheuvel, Aktas, Erdem, Gerd Hoffmann > -----Original Message----- > From: Tom Lendacky <thomas.lendacky@amd.com> > Sent: Thursday, May 12, 2022 10:13 PM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Michael Roth <michael.roth@amd.com>; > James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, > Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Aktas, Erdem <erdemaktas@google.com>; > Gerd Hoffmann <kraxel@redhat.com> > Subject: Re: [PATCH 3/4] MpInitLib: Put SEV logic in separate file > > On 5/7/22 10:13, Ray Ni wrote: > > Probably should have a commit message here explaining the reason for the > changes. > > Overall, this works, but it does seem strange to have the SwitchToRealProc > procedure in the middle of the RendezvousFunnelProc procedure. Not sure if > it would be worth just creating a second SEV nasm file (and maybe renaming > the first one): > > AmdSevRendevous.nasm > AmdSevSwitchToReal.nasm > > and then including the two in different locations. I would prefer to keep them in one file AmdSev.nasm. > > Then you wouldn't have to change any of the size calculations either. > > If you want to keep the function within the function and eliminate the use > of SwitchToRealSize, you should probably update the struct in MpLib.h to > remove SwitchToRealSize and then update the Ia32 and X64 AsmGetAddressMap > function to no longer set SwitchToRealSize (or just reserve the field so > that none of the other offsets change and just remove the set in > AsmGetAddressMap). You are right. I will post v2 patches to remove the SwitchToRealSize/Offset fields. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] MpInitLib: Only allocate below 1MB memory for 16bit code 2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray ` (2 preceding siblings ...) 2022-05-07 15:13 ` [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray @ 2022-05-07 15:13 ` Ni, Ray [not found] ` <16ECDB685492F55B.14104@groups.io> 2022-05-09 21:39 ` [edk2-devel] [PATCH 0/4] Refactor MpInitLib Lendacky, Thomas 5 siblings, 0 replies; 14+ messages in thread From: Ni, Ray @ 2022-05-07 15:13 UTC (permalink / raw) To: devel; +Cc: Eric Dong Today's implementation allocates below 1MB memory for the 16bit, 32bit and 64bit code. But it's not necessary since now the 32bit and 64bit code run at high memory no matter in PEI and DXE phase. The patch simplifies the logic to remove the code that handles the case when WakeupBufferHigh is 0. It also reduce the memory foot print under 1MB by allocating memory for 16bit code only. MP_CPU_EXCHANGE_INFO is still under 1MB which is immediate after the 16bit code. Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Eric Dong <eric.dong@intel.com> --- UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- UefiCpuPkg/Library/MpInitLib/MpLib.c | 94 ++++++++++++-------------- 3 files changed, 46 insertions(+), 56 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/AmdSev.c b/UefiCpuPkg/Library/MpInitLib/AmdSev.c index b4a344ee6b..4e4c63a52d 100644 --- a/UefiCpuPkg/Library/MpInitLib/AmdSev.c +++ b/UefiCpuPkg/Library/MpInitLib/AmdSev.c @@ -110,11 +110,7 @@ MpInitLibSevEsAPReset ( Code16 = GetProtectedMode16CS (); Code32 = GetProtectedMode32CS (); - if (CpuMpData->WakeupBufferHigh != 0) { - APResetFn = (AP_RESET *)(CpuMpData->WakeupBufferHigh + CpuMpData->AddressMap.SwitchToRealNoNxOffset); - } else { - APResetFn = (AP_RESET *)(CpuMpData->MpCpuExchangeInfo->BufferStart + CpuMpData->AddressMap.SwitchToRealOffset); - } + APResetFn = (AP_RESET *)(CpuMpData->WakeupBufferHigh + CpuMpData->AddressMap.SwitchToRealNoNxOffset); BufferStart = CpuMpData->MpCpuExchangeInfo->BufferStart; StackStart = CpuMpData->SevEsAPResetStackStart - diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc index aba53f5720..831fcc2cc7 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc @@ -97,5 +97,5 @@ struc MP_CPU_EXCHANGE_INFO .ExtTopoAvail: CTYPE_BOOLEAN 1 endstruc -MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - RendezvousFunnelProcStart) +MP_CPU_EXCHANGE_INFO_OFFSET equ (Flat32Start - RendezvousFunnelProcStart) %define MP_CPU_EXCHANGE_INFO_FIELD(Field) (MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO. %+ Field) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 722ff3fd42..df297b5bf5 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -850,6 +850,30 @@ WaitApWakeup ( } } +/** + Calculate the size of the reset vector. + + @param[in] AddressMap The pointer to Address Map structure. + + @return Total amount of memory required for the AP reset area +**/ +STATIC +VOID +GetApResetVectorSize ( + IN MP_ASSEMBLY_ADDRESS_MAP *AddressMap, + OUT UINTN *SizeBelow1Mb OPTIONAL, + OUT UINTN *SizeAbove1Mb OPTIONAL + ) +{ + if (SizeBelow1Mb != NULL) { + *SizeBelow1Mb = AddressMap->ModeTransitionOffset + sizeof (MP_CPU_EXCHANGE_INFO); + } + + if (SizeAbove1Mb != NULL) { + *SizeAbove1Mb = AddressMap->RendezvousFunnelSize - AddressMap->ModeTransitionOffset; + } +} + /** This function will fill the exchange info structure. @@ -937,21 +961,15 @@ FillExchangeInfoData ( // Copy all 32-bit code and 64-bit code into memory with type of // EfiBootServicesCode to avoid page fault if NX memory protection is enabled. // - if (CpuMpData->WakeupBufferHigh != 0) { - Size = CpuMpData->AddressMap.RendezvousFunnelSize - - CpuMpData->AddressMap.ModeTransitionOffset; - CopyMem ( - (VOID *)CpuMpData->WakeupBufferHigh, - CpuMpData->AddressMap.RendezvousFunnelAddress + - CpuMpData->AddressMap.ModeTransitionOffset, - Size - ); + GetApResetVectorSize (&CpuMpData->AddressMap, NULL, &Size); + CopyMem ( + (VOID *)CpuMpData->WakeupBufferHigh, + CpuMpData->AddressMap.RendezvousFunnelAddress + + CpuMpData->AddressMap.ModeTransitionOffset, + Size + ); - ExchangeInfo->ModeTransitionMemory = (UINT32)CpuMpData->WakeupBufferHigh; - } else { - ExchangeInfo->ModeTransitionMemory = (UINT32) - (ExchangeInfo->BufferStart + CpuMpData->AddressMap.ModeTransitionOffset); - } + ExchangeInfo->ModeTransitionMemory = (UINT32)CpuMpData->WakeupBufferHigh; ExchangeInfo->ModeHighMemory = ExchangeInfo->ModeTransitionMemory + (UINT32)ExchangeInfo->ModeOffset - @@ -992,7 +1010,7 @@ BackupAndPrepareWakeupBuffer ( CopyMem ( (VOID *)CpuMpData->WakeupBuffer, (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress, - CpuMpData->AddressMap.RendezvousFunnelSize + CpuMpData->BackupBufferSize - sizeof (MP_CPU_EXCHANGE_INFO) ); } @@ -1013,27 +1031,6 @@ RestoreWakeupBuffer ( ); } -/** - Calculate the size of the reset vector. - - @param[in] AddressMap The pointer to Address Map structure. - - @return Total amount of memory required for the AP reset area -**/ -STATIC -UINTN -GetApResetVectorSize ( - IN MP_ASSEMBLY_ADDRESS_MAP *AddressMap - ) -{ - UINTN Size; - - Size = AddressMap->RendezvousFunnelSize + - sizeof (MP_CPU_EXCHANGE_INFO); - - return Size; -} - /** Allocate reset vector buffer. @@ -1044,20 +1041,17 @@ AllocateResetVector ( IN OUT CPU_MP_DATA *CpuMpData ) { - UINTN ApResetVectorSize; + UINTN ApResetVectorSizeBelow1Mb; + UINTN ApResetVectorSizeAbove1Mb; UINTN ApResetStackSize; if (CpuMpData->WakeupBuffer == (UINTN)-1) { - ApResetVectorSize = GetApResetVectorSize (&CpuMpData->AddressMap); + GetApResetVectorSize (&CpuMpData->AddressMap, &ApResetVectorSizeBelow1Mb, &ApResetVectorSizeAbove1Mb); - CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize); + CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSizeBelow1Mb); CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN) - (CpuMpData->WakeupBuffer + - CpuMpData->AddressMap.RendezvousFunnelSize); - CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( - CpuMpData->AddressMap.RendezvousFunnelSize - - CpuMpData->AddressMap.ModeTransitionOffset - ); + (CpuMpData->WakeupBuffer + ApResetVectorSizeBelow1Mb - sizeof (MP_CPU_EXCHANGE_INFO)); + CpuMpData->WakeupBufferHigh = AllocateCodeBuffer (ApResetVectorSizeAbove1Mb); // // The AP reset stack is only used by SEV-ES guests. Do not allocate it // if SEV-ES is not enabled. An SEV-SNP guest is also considered @@ -1796,7 +1790,7 @@ MpInitLibInitialize ( UINT8 ApLoopMode; UINT8 *MonitorBuffer; UINTN Index; - UINTN ApResetVectorSize; + UINTN ApResetVectorSizeBelow1Mb; UINTN BackupBufferAddr; UINTN ApIdtBase; @@ -1814,7 +1808,7 @@ MpInitLibInitialize ( ASSERT (MaxLogicalProcessorNumber != 0); AsmGetAddressMap (&AddressMap); - ApResetVectorSize = GetApResetVectorSize (&AddressMap); + GetApResetVectorSize (&AddressMap, &ApResetVectorSizeBelow1Mb, NULL); ApStackSize = PcdGet32 (PcdCpuApStackSize); ApLoopMode = GetApLoopMode (&MonitorFilterSize); @@ -1825,7 +1819,7 @@ MpInitLibInitialize ( BufferSize = ApStackSize * MaxLogicalProcessorNumber; BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber; - BufferSize += ApResetVectorSize; + BufferSize += ApResetVectorSizeBelow1Mb; BufferSize = ALIGN_VALUE (BufferSize, 8); BufferSize += VolatileRegisters.Idtr.Limit + 1; BufferSize += sizeof (CPU_MP_DATA); @@ -1858,12 +1852,12 @@ MpInitLibInitialize ( // MonitorBuffer = (UINT8 *)(Buffer + ApStackSize * MaxLogicalProcessorNumber); BackupBufferAddr = (UINTN)MonitorBuffer + MonitorFilterSize * MaxLogicalProcessorNumber; - ApIdtBase = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8); + ApIdtBase = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSizeBelow1Mb, 8); CpuMpData = (CPU_MP_DATA *)(ApIdtBase + VolatileRegisters.Idtr.Limit + 1); CpuMpData->Buffer = Buffer; CpuMpData->CpuApStackSize = ApStackSize; CpuMpData->BackupBuffer = BackupBufferAddr; - CpuMpData->BackupBufferSize = ApResetVectorSize; + CpuMpData->BackupBufferSize = ApResetVectorSizeBelow1Mb; CpuMpData->WakeupBuffer = (UINTN)-1; CpuMpData->CpuCount = 1; CpuMpData->BspNumber = 0; -- 2.32.0.windows.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
[parent not found: <16ECDB685492F55B.14104@groups.io>]
* Re: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file [not found] ` <16ECDB685492F55B.14104@groups.io> @ 2022-05-09 11:54 ` Ni, Ray 2022-05-09 16:35 ` Lendacky, Thomas 0 siblings, 1 reply; 14+ messages in thread From: Ni, Ray @ 2022-05-09 11:54 UTC (permalink / raw) To: Tom Lendacky Cc: Dong, Eric, Kumar, Rahul1, Michael Roth, devel@edk2.groups.io, Xu, Min M, Yao, Jiewen, Tom Lendacky, Justen, Jordan L, Ard Biesheuvel, Aktas, Erdem, Gerd Hoffmann, James Bottomley Tom, Can you please review this change? Does it cause any regression to SEV feature? > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray > Sent: Saturday, May 7, 2022 11:13 PM > To: devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Michael Roth <michael.roth@amd.com>; > James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom > Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel > <ardb+tianocore@kernel.org>; Aktas, Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com> > Subject: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file > > Signed-off-by: Ray Ni <ray.ni@intel.com> > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Cc: Michael Roth <michael.roth@amd.com> > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Min Xu <min.m.xu@intel.com> > Cc: Jiewen Yao <jiewen.yao@intel.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Jordan Justen <jordan.l.justen@intel.com> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> > Cc: Erdem Aktas <erdemaktas@google.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > --- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +- > UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +----------------- > 4 files changed, 159 insertions(+), 162 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 8981c32722..67f9ed05cf 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > @@ -199,7 +199,6 @@ CProcedureInvoke: > call eax ; Invoke C function > > > > jmp $ ; Never reach here > > -RendezvousFunnelProcEnd: > > > > ;------------------------------------------------------------------------------------- > > ;SwitchToRealProc procedure follows. > > @@ -209,6 +208,8 @@ SwitchToRealProcStart: > jmp $ ; Never reach here > > SwitchToRealProcEnd: > > > > +RendezvousFunnelProcEnd: > > + > > ;------------------------------------------------------------------------------------- > > ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, > SevEsAPJumpTable, WakeupBuffer); > > ; > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 3dc1b9f872..722ff3fd42 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -938,8 +938,7 @@ FillExchangeInfoData ( > // EfiBootServicesCode to avoid page fault if NX memory protection is enabled. > > // > > if (CpuMpData->WakeupBufferHigh != 0) { > > - Size = CpuMpData->AddressMap.RendezvousFunnelSize + > > - CpuMpData->AddressMap.SwitchToRealSize - > > + Size = CpuMpData->AddressMap.RendezvousFunnelSize - > > CpuMpData->AddressMap.ModeTransitionOffset; > > CopyMem ( > > (VOID *)CpuMpData->WakeupBufferHigh, > > @@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer ( > CopyMem ( > > (VOID *)CpuMpData->WakeupBuffer, > > (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress, > > - CpuMpData->AddressMap.RendezvousFunnelSize + > > - CpuMpData->AddressMap.SwitchToRealSize > > + CpuMpData->AddressMap.RendezvousFunnelSize > > ); > > } > > > > @@ -1031,7 +1029,6 @@ GetApResetVectorSize ( > UINTN Size; > > > > Size = AddressMap->RendezvousFunnelSize + > > - AddressMap->SwitchToRealSize + > > sizeof (MP_CPU_EXCHANGE_INFO); > > > > return Size; > > @@ -1056,11 +1053,9 @@ AllocateResetVector ( > CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize); > > CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN) > > (CpuMpData->WakeupBuffer + > > - CpuMpData->AddressMap.RendezvousFunnelSize + > > - CpuMpData->AddressMap.SwitchToRealSize); > > + CpuMpData->AddressMap.RendezvousFunnelSize); > > CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( > > - CpuMpData->AddressMap.RendezvousFunnelSize + > > - CpuMpData->AddressMap.SwitchToRealSize - > > + CpuMpData->AddressMap.RendezvousFunnelSize - > > CpuMpData->AddressMap.ModeTransitionOffset > > ); > > // > > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > index 8bb1161fa0..7c2469f9c5 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm > @@ -198,3 +198,151 @@ RestoreGhcb: > > > SevEsGetApicIdExit: > > OneTimeCallRet SevEsGetApicId > > + > > + > > +;------------------------------------------------------------------------------------- > > +;SwitchToRealProc procedure follows. > > +;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC > > +;IS IN MACHINE CODE. > > +; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) > > +; rcx - Buffer Start > > +; rdx - Code16 Selector Offset > > +; r8 - Code32 Selector Offset > > +; r9 - Stack Start > > +;------------------------------------------------------------------------------------- > > +SwitchToRealProcStart: > > +BITS 64 > > + cli > > + > > + ; > > + ; Get RDX reset value before changing stacks since the > > + ; new stack won't be able to accomodate a #VC exception. > > + ; > > + push rax > > + push rbx > > + push rcx > > + push rdx > > + > > + mov rax, 1 > > + cpuid > > + mov rsi, rax ; Save off the reset value for RDX > > + > > + pop rdx > > + pop rcx > > + pop rbx > > + pop rax > > + > > + ; > > + ; Establish stack below 1MB > > + ; > > + mov rsp, r9 > > + > > + ; > > + ; Push ultimate Reset Vector onto the stack > > + ; > > + mov rax, rcx > > + shr rax, 4 > > + push word 0x0002 ; RFLAGS > > + push ax ; CS > > + push word 0x0000 ; RIP > > + push word 0x0000 ; For alignment, will be discarded > > + > > + ; > > + ; Get address of "16-bit operand size" label > > + ; > > + lea rbx, [PM16Mode] > > + > > + ; > > + ; Push addresses used to change to compatibility mode > > + ; > > + lea rax, [CompatMode] > > + push r8 > > + push rax > > + > > + ; > > + ; Clear R8 - R15, for reset, before going into 32-bit mode > > + ; > > + xor r8, r8 > > + xor r9, r9 > > + xor r10, r10 > > + xor r11, r11 > > + xor r12, r12 > > + xor r13, r13 > > + xor r14, r14 > > + xor r15, r15 > > + > > + ; > > + ; Far return into 32-bit mode > > + ; > > + retfq > > + > > +BITS 32 > > +CompatMode: > > + ; > > + ; Set up stack to prepare for exiting protected mode > > + ; > > + push edx ; Code16 CS > > + push ebx ; PM16Mode label address > > + > > + ; > > + ; Disable paging > > + ; > > + mov eax, cr0 ; Read CR0 > > + btr eax, 31 ; Set PG=0 > > + mov cr0, eax ; Write CR0 > > + > > + ; > > + ; Disable long mode > > + ; > > + mov ecx, 0c0000080h ; EFER MSR number > > + rdmsr ; Read EFER > > + btr eax, 8 ; Set LME=0 > > + wrmsr ; Write EFER > > + > > + ; > > + ; Disable PAE > > + ; > > + mov eax, cr4 ; Read CR4 > > + btr eax, 5 ; Set PAE=0 > > + mov cr4, eax ; Write CR4 > > + > > + mov edx, esi ; Restore RDX reset value > > + > > + ; > > + ; Switch to 16-bit operand size > > + ; > > + retf > > + > > +BITS 16 > > + ; > > + ; At entry to this label > > + ; - RDX will have its reset value > > + ; - On the top of the stack > > + ; - Alignment data (two bytes) to be discarded > > + ; - IP for Real Mode (two bytes) > > + ; - CS for Real Mode (two bytes) > > + ; > > + ; This label is also used with AsmRelocateApLoop. During MP finalization, > > + ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of > > + ; the WakeupBuffer, allowing a parked AP to be booted by an OS. > > + ; > > +PM16Mode: > > + mov eax, cr0 ; Read CR0 > > + btr eax, 0 ; Set PE=0 > > + mov cr0, eax ; Write CR0 > > + > > + pop ax ; Discard alignment data > > + > > + ; > > + ; Clear registers (except RDX and RSP) before going into 16-bit mode > > + ; > > + xor eax, eax > > + xor ebx, ebx > > + xor ecx, ecx > > + xor esi, esi > > + xor edi, edi > > + xor ebp, ebp > > + > > + iret > > + > > +SwitchToRealProcEnd: > > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index d7e0e1fabd..53df478661 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > @@ -152,11 +152,6 @@ SkipEnable5LevelPaging: > > > BITS 64 > > > > -; > > -; Required for the AMD SEV helper functions > > -; > > -%include "AmdSev.nasm" > > - > > LongModeStart: > > mov esi, ebx > > lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)] > > @@ -265,154 +260,12 @@ CProcedureInvoke: > add rsp, 20h > > jmp $ ; Should never reach here > > > > -RendezvousFunnelProcEnd: > > - > > -;------------------------------------------------------------------------------------- > > -;SwitchToRealProc procedure follows. > > -;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC > > -;IS IN MACHINE CODE. > > -; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) > > -; rcx - Buffer Start > > -; rdx - Code16 Selector Offset > > -; r8 - Code32 Selector Offset > > -; r9 - Stack Start > > -;------------------------------------------------------------------------------------- > > -SwitchToRealProcStart: > > -BITS 64 > > - cli > > - > > - ; > > - ; Get RDX reset value before changing stacks since the > > - ; new stack won't be able to accomodate a #VC exception. > > - ; > > - push rax > > - push rbx > > - push rcx > > - push rdx > > - > > - mov rax, 1 > > - cpuid > > - mov rsi, rax ; Save off the reset value for RDX > > - > > - pop rdx > > - pop rcx > > - pop rbx > > - pop rax > > - > > - ; > > - ; Establish stack below 1MB > > - ; > > - mov rsp, r9 > > - > > - ; > > - ; Push ultimate Reset Vector onto the stack > > - ; > > - mov rax, rcx > > - shr rax, 4 > > - push word 0x0002 ; RFLAGS > > - push ax ; CS > > - push word 0x0000 ; RIP > > - push word 0x0000 ; For alignment, will be discarded > > - > > - ; > > - ; Get address of "16-bit operand size" label > > - ; > > - lea rbx, [PM16Mode] > > - > > - ; > > - ; Push addresses used to change to compatibility mode > > - ; > > - lea rax, [CompatMode] > > - push r8 > > - push rax > > - > > - ; > > - ; Clear R8 - R15, for reset, before going into 32-bit mode > > - ; > > - xor r8, r8 > > - xor r9, r9 > > - xor r10, r10 > > - xor r11, r11 > > - xor r12, r12 > > - xor r13, r13 > > - xor r14, r14 > > - xor r15, r15 > > - > > - ; > > - ; Far return into 32-bit mode > > - ; > > - retfq > > - > > -BITS 32 > > -CompatMode: > > - ; > > - ; Set up stack to prepare for exiting protected mode > > - ; > > - push edx ; Code16 CS > > - push ebx ; PM16Mode label address > > - > > - ; > > - ; Disable paging > > - ; > > - mov eax, cr0 ; Read CR0 > > - btr eax, 31 ; Set PG=0 > > - mov cr0, eax ; Write CR0 > > - > > - ; > > - ; Disable long mode > > - ; > > - mov ecx, 0c0000080h ; EFER MSR number > > - rdmsr ; Read EFER > > - btr eax, 8 ; Set LME=0 > > - wrmsr ; Write EFER > > - > > - ; > > - ; Disable PAE > > - ; > > - mov eax, cr4 ; Read CR4 > > - btr eax, 5 ; Set PAE=0 > > - mov cr4, eax ; Write CR4 > > - > > - mov edx, esi ; Restore RDX reset value > > - > > - ; > > - ; Switch to 16-bit operand size > > - ; > > - retf > > - > > -BITS 16 > > - ; > > - ; At entry to this label > > - ; - RDX will have its reset value > > - ; - On the top of the stack > > - ; - Alignment data (two bytes) to be discarded > > - ; - IP for Real Mode (two bytes) > > - ; - CS for Real Mode (two bytes) > > - ; > > - ; This label is also used with AsmRelocateApLoop. During MP finalization, > > - ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of > > - ; the WakeupBuffer, allowing a parked AP to be booted by an OS. > > - ; > > -PM16Mode: > > - mov eax, cr0 ; Read CR0 > > - btr eax, 0 ; Set PE=0 > > - mov cr0, eax ; Write CR0 > > - > > - pop ax ; Discard alignment data > > - > > - ; > > - ; Clear registers (except RDX and RSP) before going into 16-bit mode > > - ; > > - xor eax, eax > > - xor ebx, ebx > > - xor ecx, ecx > > - xor esi, esi > > - xor edi, edi > > - xor ebp, ebp > > - > > - iret > > +; > > +; Required for the AMD SEV helper functions > > +; > > +%include "AmdSev.nasm" > > > > -SwitchToRealProcEnd: > > +RendezvousFunnelProcEnd: > > > > ;------------------------------------------------------------------------------------- > > ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, > SevEsAPJumpTable, WakeupBuffer); > > -- > 2.32.0.windows.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#89578): https://edk2.groups.io/g/devel/message/89578 > Mute This Topic: https://groups.io/mt/90954628/1712937 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file 2022-05-09 11:54 ` [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray @ 2022-05-09 16:35 ` Lendacky, Thomas 0 siblings, 0 replies; 14+ messages in thread From: Lendacky, Thomas @ 2022-05-09 16:35 UTC (permalink / raw) To: Ni, Ray Cc: Dong, Eric, Kumar, Rahul1, Michael Roth, devel@edk2.groups.io, Xu, Min M, Yao, Jiewen, Justen, Jordan L, Ard Biesheuvel, Aktas, Erdem, Gerd Hoffmann, James Bottomley On 5/9/22 06:54, Ni, Ray wrote: > Tom, > Can you please review this change? Does it cause any regression to SEV feature? Hi Ray, I just got back from vacation today and I'm going through all my email. I'll take a look as soon as I can. Thanks, Tom > >> -----Original Message----- >> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray >> Sent: Saturday, May 7, 2022 11:13 PM >> To: devel@edk2.groups.io >> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Michael Roth <michael.roth@amd.com>; >> James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom >> Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel >> <ardb+tianocore@kernel.org>; Aktas, Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com> >> Subject: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file >> >> Signed-off-by: Ray Ni <ray.ni@intel.com> >> Cc: Eric Dong <eric.dong@intel.com> >> Cc: Ray Ni <ray.ni@intel.com> >> Cc: Rahul Kumar <rahul1.kumar@intel.com> >> Cc: Michael Roth <michael.roth@amd.com> >> Cc: James Bottomley <jejb@linux.ibm.com> >> Cc: Min Xu <min.m.xu@intel.com> >> Cc: Jiewen Yao <jiewen.yao@intel.com> >> Cc: Tom Lendacky <thomas.lendacky@amd.com> >> Cc: Jordan Justen <jordan.l.justen@intel.com> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> >> Cc: Erdem Aktas <erdemaktas@google.com> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> --- >> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +- >> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++ >> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +----------------- >> 4 files changed, 159 insertions(+), 162 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >> index 8981c32722..67f9ed05cf 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm >> @@ -199,7 +199,6 @@ CProcedureInvoke: >> call eax ; Invoke C function >> >> >> >> jmp $ ; Never reach here >> >> -RendezvousFunnelProcEnd: >> >> >> >> ;------------------------------------------------------------------------------------- >> >> ;SwitchToRealProc procedure follows. >> >> @@ -209,6 +208,8 @@ SwitchToRealProcStart: >> jmp $ ; Never reach here >> >> SwitchToRealProcEnd: >> >> >> >> +RendezvousFunnelProcEnd: >> >> + >> >> ;------------------------------------------------------------------------------------- >> >> ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, >> SevEsAPJumpTable, WakeupBuffer); >> >> ; >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> index 3dc1b9f872..722ff3fd42 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >> @@ -938,8 +938,7 @@ FillExchangeInfoData ( >> // EfiBootServicesCode to avoid page fault if NX memory protection is enabled. >> >> // >> >> if (CpuMpData->WakeupBufferHigh != 0) { >> >> - Size = CpuMpData->AddressMap.RendezvousFunnelSize + >> >> - CpuMpData->AddressMap.SwitchToRealSize - >> >> + Size = CpuMpData->AddressMap.RendezvousFunnelSize - >> >> CpuMpData->AddressMap.ModeTransitionOffset; >> >> CopyMem ( >> >> (VOID *)CpuMpData->WakeupBufferHigh, >> >> @@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer ( >> CopyMem ( >> >> (VOID *)CpuMpData->WakeupBuffer, >> >> (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress, >> >> - CpuMpData->AddressMap.RendezvousFunnelSize + >> >> - CpuMpData->AddressMap.SwitchToRealSize >> >> + CpuMpData->AddressMap.RendezvousFunnelSize >> >> ); >> >> } >> >> >> >> @@ -1031,7 +1029,6 @@ GetApResetVectorSize ( >> UINTN Size; >> >> >> >> Size = AddressMap->RendezvousFunnelSize + >> >> - AddressMap->SwitchToRealSize + >> >> sizeof (MP_CPU_EXCHANGE_INFO); >> >> >> >> return Size; >> >> @@ -1056,11 +1053,9 @@ AllocateResetVector ( >> CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize); >> >> CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN) >> >> (CpuMpData->WakeupBuffer + >> >> - CpuMpData->AddressMap.RendezvousFunnelSize + >> >> - CpuMpData->AddressMap.SwitchToRealSize); >> >> + CpuMpData->AddressMap.RendezvousFunnelSize); >> >> CpuMpData->WakeupBufferHigh = AllocateCodeBuffer ( >> >> - CpuMpData->AddressMap.RendezvousFunnelSize + >> >> - CpuMpData->AddressMap.SwitchToRealSize - >> >> + CpuMpData->AddressMap.RendezvousFunnelSize - >> >> CpuMpData->AddressMap.ModeTransitionOffset >> >> ); >> >> // >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm >> index 8bb1161fa0..7c2469f9c5 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm >> +++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm >> @@ -198,3 +198,151 @@ RestoreGhcb: >> >> >> SevEsGetApicIdExit: >> >> OneTimeCallRet SevEsGetApicId >> >> + >> >> + >> >> +;------------------------------------------------------------------------------------- >> >> +;SwitchToRealProc procedure follows. >> >> +;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC >> >> +;IS IN MACHINE CODE. >> >> +; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) >> >> +; rcx - Buffer Start >> >> +; rdx - Code16 Selector Offset >> >> +; r8 - Code32 Selector Offset >> >> +; r9 - Stack Start >> >> +;------------------------------------------------------------------------------------- >> >> +SwitchToRealProcStart: >> >> +BITS 64 >> >> + cli >> >> + >> >> + ; >> >> + ; Get RDX reset value before changing stacks since the >> >> + ; new stack won't be able to accomodate a #VC exception. >> >> + ; >> >> + push rax >> >> + push rbx >> >> + push rcx >> >> + push rdx >> >> + >> >> + mov rax, 1 >> >> + cpuid >> >> + mov rsi, rax ; Save off the reset value for RDX >> >> + >> >> + pop rdx >> >> + pop rcx >> >> + pop rbx >> >> + pop rax >> >> + >> >> + ; >> >> + ; Establish stack below 1MB >> >> + ; >> >> + mov rsp, r9 >> >> + >> >> + ; >> >> + ; Push ultimate Reset Vector onto the stack >> >> + ; >> >> + mov rax, rcx >> >> + shr rax, 4 >> >> + push word 0x0002 ; RFLAGS >> >> + push ax ; CS >> >> + push word 0x0000 ; RIP >> >> + push word 0x0000 ; For alignment, will be discarded >> >> + >> >> + ; >> >> + ; Get address of "16-bit operand size" label >> >> + ; >> >> + lea rbx, [PM16Mode] >> >> + >> >> + ; >> >> + ; Push addresses used to change to compatibility mode >> >> + ; >> >> + lea rax, [CompatMode] >> >> + push r8 >> >> + push rax >> >> + >> >> + ; >> >> + ; Clear R8 - R15, for reset, before going into 32-bit mode >> >> + ; >> >> + xor r8, r8 >> >> + xor r9, r9 >> >> + xor r10, r10 >> >> + xor r11, r11 >> >> + xor r12, r12 >> >> + xor r13, r13 >> >> + xor r14, r14 >> >> + xor r15, r15 >> >> + >> >> + ; >> >> + ; Far return into 32-bit mode >> >> + ; >> >> + retfq >> >> + >> >> +BITS 32 >> >> +CompatMode: >> >> + ; >> >> + ; Set up stack to prepare for exiting protected mode >> >> + ; >> >> + push edx ; Code16 CS >> >> + push ebx ; PM16Mode label address >> >> + >> >> + ; >> >> + ; Disable paging >> >> + ; >> >> + mov eax, cr0 ; Read CR0 >> >> + btr eax, 31 ; Set PG=0 >> >> + mov cr0, eax ; Write CR0 >> >> + >> >> + ; >> >> + ; Disable long mode >> >> + ; >> >> + mov ecx, 0c0000080h ; EFER MSR number >> >> + rdmsr ; Read EFER >> >> + btr eax, 8 ; Set LME=0 >> >> + wrmsr ; Write EFER >> >> + >> >> + ; >> >> + ; Disable PAE >> >> + ; >> >> + mov eax, cr4 ; Read CR4 >> >> + btr eax, 5 ; Set PAE=0 >> >> + mov cr4, eax ; Write CR4 >> >> + >> >> + mov edx, esi ; Restore RDX reset value >> >> + >> >> + ; >> >> + ; Switch to 16-bit operand size >> >> + ; >> >> + retf >> >> + >> >> +BITS 16 >> >> + ; >> >> + ; At entry to this label >> >> + ; - RDX will have its reset value >> >> + ; - On the top of the stack >> >> + ; - Alignment data (two bytes) to be discarded >> >> + ; - IP for Real Mode (two bytes) >> >> + ; - CS for Real Mode (two bytes) >> >> + ; >> >> + ; This label is also used with AsmRelocateApLoop. During MP finalization, >> >> + ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of >> >> + ; the WakeupBuffer, allowing a parked AP to be booted by an OS. >> >> + ; >> >> +PM16Mode: >> >> + mov eax, cr0 ; Read CR0 >> >> + btr eax, 0 ; Set PE=0 >> >> + mov cr0, eax ; Write CR0 >> >> + >> >> + pop ax ; Discard alignment data >> >> + >> >> + ; >> >> + ; Clear registers (except RDX and RSP) before going into 16-bit mode >> >> + ; >> >> + xor eax, eax >> >> + xor ebx, ebx >> >> + xor ecx, ecx >> >> + xor esi, esi >> >> + xor edi, edi >> >> + xor ebp, ebp >> >> + >> >> + iret >> >> + >> >> +SwitchToRealProcEnd: >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >> index d7e0e1fabd..53df478661 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm >> @@ -152,11 +152,6 @@ SkipEnable5LevelPaging: >> >> >> BITS 64 >> >> >> >> -; >> >> -; Required for the AMD SEV helper functions >> >> -; >> >> -%include "AmdSev.nasm" >> >> - >> >> LongModeStart: >> >> mov esi, ebx >> >> lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)] >> >> @@ -265,154 +260,12 @@ CProcedureInvoke: >> add rsp, 20h >> >> jmp $ ; Should never reach here >> >> >> >> -RendezvousFunnelProcEnd: >> >> - >> >> -;------------------------------------------------------------------------------------- >> >> -;SwitchToRealProc procedure follows. >> >> -;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC >> >> -;IS IN MACHINE CODE. >> >> -; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart) >> >> -; rcx - Buffer Start >> >> -; rdx - Code16 Selector Offset >> >> -; r8 - Code32 Selector Offset >> >> -; r9 - Stack Start >> >> -;------------------------------------------------------------------------------------- >> >> -SwitchToRealProcStart: >> >> -BITS 64 >> >> - cli >> >> - >> >> - ; >> >> - ; Get RDX reset value before changing stacks since the >> >> - ; new stack won't be able to accomodate a #VC exception. >> >> - ; >> >> - push rax >> >> - push rbx >> >> - push rcx >> >> - push rdx >> >> - >> >> - mov rax, 1 >> >> - cpuid >> >> - mov rsi, rax ; Save off the reset value for RDX >> >> - >> >> - pop rdx >> >> - pop rcx >> >> - pop rbx >> >> - pop rax >> >> - >> >> - ; >> >> - ; Establish stack below 1MB >> >> - ; >> >> - mov rsp, r9 >> >> - >> >> - ; >> >> - ; Push ultimate Reset Vector onto the stack >> >> - ; >> >> - mov rax, rcx >> >> - shr rax, 4 >> >> - push word 0x0002 ; RFLAGS >> >> - push ax ; CS >> >> - push word 0x0000 ; RIP >> >> - push word 0x0000 ; For alignment, will be discarded >> >> - >> >> - ; >> >> - ; Get address of "16-bit operand size" label >> >> - ; >> >> - lea rbx, [PM16Mode] >> >> - >> >> - ; >> >> - ; Push addresses used to change to compatibility mode >> >> - ; >> >> - lea rax, [CompatMode] >> >> - push r8 >> >> - push rax >> >> - >> >> - ; >> >> - ; Clear R8 - R15, for reset, before going into 32-bit mode >> >> - ; >> >> - xor r8, r8 >> >> - xor r9, r9 >> >> - xor r10, r10 >> >> - xor r11, r11 >> >> - xor r12, r12 >> >> - xor r13, r13 >> >> - xor r14, r14 >> >> - xor r15, r15 >> >> - >> >> - ; >> >> - ; Far return into 32-bit mode >> >> - ; >> >> - retfq >> >> - >> >> -BITS 32 >> >> -CompatMode: >> >> - ; >> >> - ; Set up stack to prepare for exiting protected mode >> >> - ; >> >> - push edx ; Code16 CS >> >> - push ebx ; PM16Mode label address >> >> - >> >> - ; >> >> - ; Disable paging >> >> - ; >> >> - mov eax, cr0 ; Read CR0 >> >> - btr eax, 31 ; Set PG=0 >> >> - mov cr0, eax ; Write CR0 >> >> - >> >> - ; >> >> - ; Disable long mode >> >> - ; >> >> - mov ecx, 0c0000080h ; EFER MSR number >> >> - rdmsr ; Read EFER >> >> - btr eax, 8 ; Set LME=0 >> >> - wrmsr ; Write EFER >> >> - >> >> - ; >> >> - ; Disable PAE >> >> - ; >> >> - mov eax, cr4 ; Read CR4 >> >> - btr eax, 5 ; Set PAE=0 >> >> - mov cr4, eax ; Write CR4 >> >> - >> >> - mov edx, esi ; Restore RDX reset value >> >> - >> >> - ; >> >> - ; Switch to 16-bit operand size >> >> - ; >> >> - retf >> >> - >> >> -BITS 16 >> >> - ; >> >> - ; At entry to this label >> >> - ; - RDX will have its reset value >> >> - ; - On the top of the stack >> >> - ; - Alignment data (two bytes) to be discarded >> >> - ; - IP for Real Mode (two bytes) >> >> - ; - CS for Real Mode (two bytes) >> >> - ; >> >> - ; This label is also used with AsmRelocateApLoop. During MP finalization, >> >> - ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of >> >> - ; the WakeupBuffer, allowing a parked AP to be booted by an OS. >> >> - ; >> >> -PM16Mode: >> >> - mov eax, cr0 ; Read CR0 >> >> - btr eax, 0 ; Set PE=0 >> >> - mov cr0, eax ; Write CR0 >> >> - >> >> - pop ax ; Discard alignment data >> >> - >> >> - ; >> >> - ; Clear registers (except RDX and RSP) before going into 16-bit mode >> >> - ; >> >> - xor eax, eax >> >> - xor ebx, ebx >> >> - xor ecx, ecx >> >> - xor esi, esi >> >> - xor edi, edi >> >> - xor ebp, ebp >> >> - >> >> - iret >> >> +; >> >> +; Required for the AMD SEV helper functions >> >> +; >> >> +%include "AmdSev.nasm" >> >> >> >> -SwitchToRealProcEnd: >> >> +RendezvousFunnelProcEnd: >> >> >> >> ;------------------------------------------------------------------------------------- >> >> ; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, >> SevEsAPJumpTable, WakeupBuffer); >> >> -- >> 2.32.0.windows.1 >> >> >> >> -=-=-=-=-=-= >> Groups.io Links: You receive all messages sent to this group. >> View/Reply Online (#89578): https://edk2.groups.io/g/devel/message/89578 >> Mute This Topic: https://groups.io/mt/90954628/1712937 >> Group Owner: devel+owner@edk2.groups.io >> Unsubscribe: https://edk2.groups.io/g/devel/unsub >> -=-=-=-=-=-= >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib 2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray ` (4 preceding siblings ...) [not found] ` <16ECDB685492F55B.14104@groups.io> @ 2022-05-09 21:39 ` Lendacky, Thomas 2022-05-09 23:16 ` Ni, Ray 5 siblings, 1 reply; 14+ messages in thread From: Lendacky, Thomas @ 2022-05-09 21:39 UTC (permalink / raw) To: devel, ray.ni Hi Ray, Do you have a public git tree with these patches that I can use to test with? I'm having lots of problems pulling these patches out of my mail client and applying them. Thanks, Tom On 5/7/22 10:13, Ni, Ray via groups.io wrote: > > Ray Ni (4): > MpInitLib: Allocate code buffer for PEI phase > MpInitLib: remove unneeded global ASM_PFX > MpInitLib: Put SEV logic in separate file > MpInitLib: Only allocate below 1MB memory for 16bit code > > UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------ > UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +- > UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +----------------- > 9 files changed, 216 insertions(+), 236 deletions(-) > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib 2022-05-09 21:39 ` [edk2-devel] [PATCH 0/4] Refactor MpInitLib Lendacky, Thomas @ 2022-05-09 23:16 ` Ni, Ray 2022-05-10 14:44 ` Lendacky, Thomas 0 siblings, 1 reply; 14+ messages in thread From: Ni, Ray @ 2022-05-09 23:16 UTC (permalink / raw) To: devel@edk2.groups.io, thomas.lendacky@amd.com [-- Attachment #1: Type: text/plain, Size: 1537 bytes --] https://github.com/niruiyu/edk2/tree/refactormp thanks, ray ________________________________ From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io> Sent: Tuesday, May 10, 2022 5:39:51 AM To: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com> Subject: Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib Hi Ray, Do you have a public git tree with these patches that I can use to test with? I'm having lots of problems pulling these patches out of my mail client and applying them. Thanks, Tom On 5/7/22 10:13, Ni, Ray via groups.io wrote: > > Ray Ni (4): > MpInitLib: Allocate code buffer for PEI phase > MpInitLib: remove unneeded global ASM_PFX > MpInitLib: Put SEV logic in separate file > MpInitLib: Only allocate below 1MB memory for 16bit code > > UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +- > UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------ > UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +- > UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++ > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +----------------- > 9 files changed, 216 insertions(+), 236 deletions(-) > [-- Attachment #2: Type: text/html, Size: 3153 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib 2022-05-09 23:16 ` Ni, Ray @ 2022-05-10 14:44 ` Lendacky, Thomas 2022-05-10 15:13 ` Lendacky, Thomas 0 siblings, 1 reply; 14+ messages in thread From: Lendacky, Thomas @ 2022-05-10 14:44 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io On 5/9/22 18:16, Ni, Ray wrote: > https://github.com/niruiyu/edk2/tree/refactormp<https://github.com/niruiyu/edk2/tree/refactormp Thanks for the tree, Ray. I was able to build and test against legacy, SEV, SEV-ES and SEV-SNP guests and found everything worked well. I did notice a regression in the tree, un-related to your patches, when booting an SEV-SNP guest. The following message appears for each AP: APIC: Stale IRR: 00000000,00000000,00000000,00000000,00000000,00000000,00000001,00000000 ISR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000 So I'll start bisecting to see which commit introduced that. Thanks, Tom > > thanks, > ray > -------------------------------------------------------------------------- > *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Lendacky, > Thomas via groups.io <thomas.lendacky=amd.com@groups.io> > *Sent:* Tuesday, May 10, 2022 5:39:51 AM > *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com> > *Subject:* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib > Hi Ray, > > Do you have a public git tree with these patches that I can use to test > with? I'm having lots of problems pulling these patches out of my mail > client and applying them. > > Thanks, > Tom > > On 5/7/22 10:13, Ni, Ray via groups.io wrote: >> >> Ray Ni (4): >> MpInitLib: Allocate code buffer for PEI phase >> MpInitLib: remove unneeded global ASM_PFX >> MpInitLib: Put SEV logic in separate file >> MpInitLib: Only allocate below 1MB memory for 16bit code >> >> UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- >> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- >> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +- >> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- >> UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------ >> UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- >> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +- >> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++ >> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +----------------- >> 9 files changed, 216 insertions(+), 236 deletions(-) >> > > > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib 2022-05-10 14:44 ` Lendacky, Thomas @ 2022-05-10 15:13 ` Lendacky, Thomas 2022-05-12 1:21 ` Ni, Ray 0 siblings, 1 reply; 14+ messages in thread From: Lendacky, Thomas @ 2022-05-10 15:13 UTC (permalink / raw) To: Ni, Ray, devel@edk2.groups.io On 5/10/22 09:44, Tom Lendacky wrote: > On 5/9/22 18:16, Ni, Ray wrote: >> https://github.com/niruiyu/edk2/tree/refactormp<https://github.com/niruiyu/edk2/tree/refactormp >> > > Thanks for the tree, Ray. I was able to build and test against legacy, > SEV, SEV-ES and SEV-SNP guests and found everything worked well. > > I did notice a regression in the tree, un-related to your patches, when > booting an SEV-SNP guest. The following message appears for each AP: > > APIC: Stale IRR: > 00000000,00000000,00000000,00000000,00000000,00000000,00000001,00000000 > ISR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000 > > So I'll start bisecting to see which commit introduced that. This was introduced in the debug hack I needed to boot multiple vCPUs successfully (since Min's fix isn't in your tree, yet). I hadn't noticed this in Min's MpLib fix, but after investigating I do see it now. I'll follow up with Min. Thanks, Tom > > Thanks, > Tom > >> >> thanks, >> ray >> -------------------------------------------------------------------------- >> *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of >> Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io> >> *Sent:* Tuesday, May 10, 2022 5:39:51 AM >> *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray >> <ray.ni@intel.com> >> *Subject:* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib >> Hi Ray, >> >> Do you have a public git tree with these patches that I can use to test >> with? I'm having lots of problems pulling these patches out of my mail >> client and applying them. >> >> Thanks, >> Tom >> >> On 5/7/22 10:13, Ni, Ray via groups.io wrote: >>> >>> Ray Ni (4): >>> MpInitLib: Allocate code buffer for PEI phase >>> MpInitLib: remove unneeded global ASM_PFX >>> MpInitLib: Put SEV logic in separate file >>> MpInitLib: Only allocate below 1MB memory for 16bit code >>> >>> UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- >>> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- >>> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +- >>> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- >>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------ >>> UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- >>> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +- >>> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++ >>> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +----------------- >>> 9 files changed, 216 insertions(+), 236 deletions(-) >>> >> >> >> >> >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib 2022-05-10 15:13 ` Lendacky, Thomas @ 2022-05-12 1:21 ` Ni, Ray 0 siblings, 0 replies; 14+ messages in thread From: Ni, Ray @ 2022-05-12 1:21 UTC (permalink / raw) To: Tom Lendacky, devel@edk2.groups.io Can you please help to review the changes and give a Reviewed-by? > -----Original Message----- > From: Tom Lendacky <thomas.lendacky@amd.com> > Sent: Tuesday, May 10, 2022 11:13 PM > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io > Subject: Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib > > On 5/10/22 09:44, Tom Lendacky wrote: > > On 5/9/22 18:16, Ni, Ray wrote: > >> https://github.com/niruiyu/edk2/tree/refactormp<https://github.com/niruiyu/edk2/tree/refactormp > >> > > > > Thanks for the tree, Ray. I was able to build and test against legacy, > > SEV, SEV-ES and SEV-SNP guests and found everything worked well. > > > > I did notice a regression in the tree, un-related to your patches, when > > booting an SEV-SNP guest. The following message appears for each AP: > > > > APIC: Stale IRR: > > 00000000,00000000,00000000,00000000,00000000,00000000,00000001,00000000 > > ISR: 00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000 > > > > So I'll start bisecting to see which commit introduced that. > > This was introduced in the debug hack I needed to boot multiple vCPUs > successfully (since Min's fix isn't in your tree, yet). > > I hadn't noticed this in Min's MpLib fix, but after investigating I do see > it now. I'll follow up with Min. > > Thanks, > Tom > > > > > Thanks, > > Tom > > > >> > >> thanks, > >> ray > >> -------------------------------------------------------------------------- > >> *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of > >> Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io> > >> *Sent:* Tuesday, May 10, 2022 5:39:51 AM > >> *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray > >> <ray.ni@intel.com> > >> *Subject:* Re: [edk2-devel] [PATCH 0/4] Refactor MpInitLib > >> Hi Ray, > >> > >> Do you have a public git tree with these patches that I can use to test > >> with? I'm having lots of problems pulling these patches out of my mail > >> client and applying them. > >> > >> Thanks, > >> Tom > >> > >> On 5/7/22 10:13, Ni, Ray via groups.io wrote: > >>> > >>> Ray Ni (4): > >>> MpInitLib: Allocate code buffer for PEI phase > >>> MpInitLib: remove unneeded global ASM_PFX > >>> MpInitLib: Put SEV logic in separate file > >>> MpInitLib: Only allocate below 1MB memory for 16bit code > >>> > >>> UefiCpuPkg/Library/MpInitLib/AmdSev.c | 6 +- > >>> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 2 +- > >>> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 11 +- > >>> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +- > >>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 99 +++++------ > >>> UefiCpuPkg/Library/MpInitLib/MpLib.h | 2 +- > >>> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 +- > >>> UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 ++++++++++++++++ > >>> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 167 +----------------- > >>> 9 files changed, 216 insertions(+), 236 deletions(-) > >>> > >> > >> > >> > >> > >> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-05-16 3:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray
2022-05-07 15:13 ` [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase Ni, Ray
2022-05-07 15:13 ` [PATCH 2/4] MpInitLib: remove unneeded global ASM_PFX Ni, Ray
2022-05-07 15:13 ` [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray
2022-05-12 14:13 ` Lendacky, Thomas
2022-05-16 3:51 ` Ni, Ray
2022-05-07 15:13 ` [PATCH 4/4] MpInitLib: Only allocate below 1MB memory for 16bit code Ni, Ray
[not found] ` <16ECDB685492F55B.14104@groups.io>
2022-05-09 11:54 ` [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray
2022-05-09 16:35 ` Lendacky, Thomas
2022-05-09 21:39 ` [edk2-devel] [PATCH 0/4] Refactor MpInitLib Lendacky, Thomas
2022-05-09 23:16 ` Ni, Ray
2022-05-10 14:44 ` Lendacky, Thomas
2022-05-10 15:13 ` Lendacky, Thomas
2022-05-12 1:21 ` Ni, Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox