On Thu, 5 Jan 2023 at 07:21, Yuanhao Xie <yuanhao....@intel.com> wrote:
>
> Kept 4GB allocation limit for the case that APs are still transferred to
> 32-bit protected mode on long mode DXE.
> Fixed AsmRelocateApLoopStart stack offset in Ia32.
>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=4234
>
> Cc: Eric Dong eric.d...@intel.com
> Cc: Ray Ni ray...@intel.com
> Cc: Rahul Kumar rahul1.ku...@intel.com
> Signed-off-by: Yuanhao Xie <yuanhao....@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 35 ++++++++++++-------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 9 ++---
> 2 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index beab06a5b1..1994ee44c2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -389,7 +389,7 @@ RelocateApLoop (
> MpInitLibWhoAmI (&ProcessorNumber);
> CpuMpData = GetCpuMpData ();
> MwaitSupport = IsMwaitSupport ();
> - if (StandardSignatureIsAuthenticAMD ()) {
> + if (StandardSignatureIsAuthenticAMD () && (sizeof (UINTN) == sizeof
> (UINT64))) {
> StackStart = CpuMpData->UseSevEsAPMethod ?
> CpuMpData->SevEsAPResetStackStart : mReservedTopOfApStack;
> AsmRelocateApLoopFuncAmd =
> (ASM_RELOCATE_AP_LOOP_AMD)(UINTN)mReservedApLoopFunc;
> AsmRelocateApLoopFuncAmd (
> @@ -480,6 +480,7 @@ InitMpGlobalData (
> EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc;
> UINTN StackBase;
> CPU_INFO_IN_HOB *CpuInfoInHob;
> + EFI_PHYSICAL_ADDRESS Address;
>
> SaveCpuMpData (CpuMpData);
>
> @@ -536,9 +537,9 @@ InitMpGlobalData (
>
> //
> // Avoid APs access invalid buffer data which allocated by BootServices,
> - // so we will allocate reserved data for AP loop code. We also need to
> - // allocate this buffer below 4GB due to APs may be transferred to 32bit
> - // protected mode on long mode DXE.
> + // so we will allocate reserved data for AP loop code. We need to
> + // allocate this buffer below 4GB for the case that APs are transferred
> + // to 32-bit protected mode on long mode DXE.
> // Allocating it in advance since memory services are not available in
> // Exit Boot Services callback function.
> //
> @@ -555,19 +556,25 @@ InitMpGlobalData (
> )
> );
>
> - mReservedTopOfApStack = (UINTN)AllocateReservedPages (EFI_SIZE_TO_PAGES
> (ApSafeBufferSize));
> - ASSERT (mReservedTopOfApStack != 0);
> - ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
> - ASSERT ((AP_SAFE_STACK_SIZE & (CPU_STACK_ALIGNMENT - 1)) == 0);
> -
> - mReservedApLoopFunc = (VOID *)(mReservedTopOfApStack + CpuMpData->CpuCount
> * AP_SAFE_STACK_SIZE);
> - if (StandardSignatureIsAuthenticAMD ()) {
> + if (StandardSignatureIsAuthenticAMD () && (sizeof (UINTN) == sizeof
> (UINT64))) {
This looks the wrong way around.
> + Address = BASE_4GB - 1;
> + Status = gBS->AllocatePages (
> + AllocateMaxAddress,
> + EfiReservedMemoryType,
> + EFI_SIZE_TO_PAGES (ApSafeBufferSize),
> + &Address
> + );
> + ASSERT_EFI_ERROR (Status);
> + mReservedApLoopFunc = (VOID *)((UINTN)Address + CpuMpData->CpuCount *
> AP_SAFE_STACK_SIZE);
> CopyMem (
> mReservedApLoopFunc,
> CpuMpData->AddressMap.RelocateApLoopFuncAddressAmd,
> CpuMpData->AddressMap.RelocateApLoopFuncSizeAmd
> );
> } else {
> + Address = (UINTN)AllocateReservedPages (EFI_SIZE_TO_PAGES
> (ApSafeBufferSize));
> + ASSERT (Address != 0);
Isn't this code path still used for the IA32 version?
> + mReservedApLoopFunc = (VOID *)((UINTN)Address + CpuMpData->CpuCount *
> AP_SAFE_STACK_SIZE);
> CopyMem (
> mReservedApLoopFunc,
> CpuMpData->AddressMap.RelocateApLoopFuncAddress,
> @@ -575,12 +582,14 @@ InitMpGlobalData (
> );
>
> mApPageTable = CreatePageTable (
> - mReservedTopOfApStack,
> + (UINTN)Address,
> ApSafeBufferSize
> );
> }
>
> - mReservedTopOfApStack += CpuMpData->CpuCount * AP_SAFE_STACK_SIZE;
> + mReservedTopOfApStack = (UINTN)Address + CpuMpData->CpuCount *
> AP_SAFE_STACK_SIZE;
> + ASSERT ((mReservedTopOfApStack & (UINTN)(CPU_STACK_ALIGNMENT - 1)) == 0);
> + ASSERT ((AP_SAFE_STACK_SIZE & (CPU_STACK_ALIGNMENT - 1)) == 0);
>
> Status = gBS->CreateEvent (
> EVT_TIMER | EVT_NOTIFY_SIGNAL,
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index bfcdbd31c1..5cffa632ab 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -219,20 +219,17 @@ SwitchToRealProcEnd:
> RendezvousFunnelProcEnd:
>
>
> ;-------------------------------------------------------------------------------------
> -; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment,
> TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);
> -;
> -; The last three parameters (Pm16CodeSegment, SevEsAPJumpTable and
> WakeupBuffer) are
> -; specific to SEV-ES support and are not applicable on IA32.
> +; AsmRelocateApLoop (MwaitSupport, ApTargetCState, TopOfApStack,
> CountTofinish, Cr3);
>
> ;-------------------------------------------------------------------------------------
> AsmRelocateApLoopStart:
> mov eax, esp
> - mov esp, [eax + 16] ; TopOfApStack
> + mov esp, [eax + 12] ; TopOfApStack
> push dword [eax] ; push return address for stack trace
> push ebp
> mov ebp, esp
> mov ebx, [eax + 8] ; ApTargetCState
> mov ecx, [eax + 4] ; MwaitSupport
> - mov eax, [eax + 20] ; CountTofinish
> + mov eax, [eax + 16] ; CountTofinish
> lock dec dword [eax] ; (*CountTofinish)--
> cmp cl, 1 ; Check mwait-monitor support
> jnz HltLoop
> --
> 2.36.1.windows.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97979): https://edk2.groups.io/g/devel/message/97979
Mute This Topic: https://groups.io/mt/96067843/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
