From dd3fa0cd72de2cd6c7d9e3c68ff9b847aa9a8c9f Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Wed, 16 Nov 2016 23:31:11 +0100 Subject: [PATCH] UefiCpuPkg/MpInitLib: support 64-bit AP stack addresses The cached "CPU_INFO_IN_HOB.ApTopOfStack" field currently has type UINT32. This is not ideal because the AP stacks are located within "CpuMpData->Buffer", which is allocated with a plain AllocatePages() call in MpInitLibInitialize(): platform CpuMpPei included PEI RAM > 4GB result -------- ----------------- ------------- ------ Ia32 * n/a good Ia32X64 no n/a BAD Ia32X64 yes n/a good X64 no * BAD X64 yes no good X64 yes yes BAD - If we are on an Ia32X64 or X64 platform that does not include CpuMpPei, then CpuDxe cannot reuse the CPU_INFO_IN_HOB structures preallocated by CpuMpPei (through the CpuInitMpLib GUID HOB), and then AllocatePages() -- invoked first in 64-bit DXE -- could return an address outside of 32-bit address space. - If we are on an X64 platform where the permanent PEI RAM extends above the 32-bit address space, then the same issue can surface even if CpuMpPei is included: even the original allocation of the CPU_INFO_IN_HOB structures, by CpuMpPei, could be satisfied from above 4GB. The original "AP init" branch in "X64/MpFuncs.nasm" correctly considers a 64-bit stack start: the "MP_CPU_EXCHANGE_INFO.StackStart" field has type UINTN, and the code uses QWORD addition and movement to set RSP from it. Adapt the "GetApicId" branch of "X64/MpFuncs.nasm": - change the type of "CPU_INFO_IN_HOB.ApTopOfStack" to UINT64, - remove the explicit truncation to UINT32 in InitializeApData(), - update the "GetNextProcNumber" iteration size to the new size of "CPU_INFO_IN_HOB", - set RSP with a QWORD movement from "CPU_INFO_IN_HOB.ApTopOfStack". Because the same CPU_INFO_IN_HOB structure is used by "Ia32/MpFuncs.nasm", we have to update the "GetNextProcNumber" iteration size there as well. The ESP setting can be preserved as a DWORD movement from the original offset (decimal 12), since our integers are little endian. Cc: Jeff Fan Fixes: 845c5be1fd9bf7edfac4a103dfab70829686978f Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek Reviewed-by: Jeff Fan --- UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 2 +- UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++++---- UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +++- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 5 ++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 4bfa084c85..64e51d87ae 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -181,7 +181,7 @@ GetProcessorNumber: GetNextProcNumber: cmp [edi], edx ; APIC ID match? jz ProgramStack - add edi, 16 + add edi, 20 inc ebx jmp GetNextProcNumber diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 3c2e6d6b89..15dbfa1e7d 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -433,7 +433,7 @@ InitializeApData ( IN OUT CPU_MP_DATA *CpuMpData, IN UINTN ProcessorNumber, IN UINT32 BistData, - IN UINTN ApTopOfStack + IN UINT64 ApTopOfStack ) { CPU_INFO_IN_HOB *CpuInfoInHob; @@ -442,7 +442,7 @@ InitializeApData ( CpuInfoInHob[ProcessorNumber].InitialApicId = GetInitialApicId (); CpuInfoInHob[ProcessorNumber].ApicId = GetApicId (); CpuInfoInHob[ProcessorNumber].Health = BistData; - CpuInfoInHob[ProcessorNumber].ApTopOfStack = (UINT32) ApTopOfStack; + CpuInfoInHob[ProcessorNumber].ApTopOfStack = ApTopOfStack; CpuMpData->CpuData[ProcessorNumber].Waiting = FALSE; CpuMpData->CpuData[ProcessorNumber].CpuHealthy = (BistData == 0) ? TRUE : FALSE; @@ -480,7 +480,7 @@ ApWakeupFunction ( UINT32 BistData; volatile UINT32 *ApStartupSignalBuffer; CPU_INFO_IN_HOB *CpuInfoInHob; - UINTN ApTopOfStack; + UINT64 ApTopOfStack; // // AP finished assembly code and begin to execute C code @@ -500,7 +500,7 @@ ApWakeupFunction ( // This is first time AP wakeup, get BIST information from AP stack // ApTopOfStack = CpuMpData->Buffer + (ProcessorNumber + 1) * CpuMpData->CpuApStackSize; - BistData = *(UINT32 *) (ApTopOfStack - sizeof (UINTN)); + BistData = *(UINT32 *) ((UINTN) ApTopOfStack - sizeof (UINTN)); // // Do some AP initialize sync // diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h index 0ac777a099..f73a469ae8 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -129,12 +129,14 @@ typedef struct { // we need to make sure the each fields offset same in different // architecture. // +#pragma pack (1) typedef struct { UINT32 InitialApicId; UINT32 ApicId; UINT32 Health; - UINT32 ApTopOfStack; + UINT64 ApTopOfStack; } CPU_INFO_IN_HOB; +#pragma pack () // // AP reset code information including code address and size, diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm index 138b97312b..aaabb50c54 100644 --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm @@ -185,13 +185,12 @@ GetProcessorNumber: GetNextProcNumber: cmp dword [edi], edx ; APIC ID match? jz ProgramStack - add edi, 16 + add edi, 20 inc ebx jmp GetNextProcNumber ProgramStack: - xor rsp, rsp - mov esp, dword [edi + 12] + mov rsp, qword [edi + 12] CProcedureInvoke: push rbp ; Push BIST data at top of AP stack -- 2.39.2