From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@ml01.01.org>
Cc: Jeff Fan <jeff.fan@intel.com>
Subject: [PATCH 4/4] UefiCpuPkg/MpInitLib: support 64-bit AP stack addresses
Date: Thu, 17 Nov 2016 01:17:54 +0100 [thread overview]
Message-ID: <20161117001754.4383-5-lersek@redhat.com> (raw)
In-Reply-To: <20161117001754.4383-1-lersek@redhat.com>
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 <jeff.fan@intel.com>
Fixes: 845c5be1fd9bf7edfac4a103dfab70829686978f
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
UefiCpuPkg/Library/MpInitLib/MpLib.h | 4 +++-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++++----
UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 2 +-
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 5 ++---
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 0ac777a099b1..f73a469ae84f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -126,18 +126,20 @@ typedef struct {
//
// Basic CPU information saved in Guided HOB.
// Because the contents will be shard between PEI and DXE,
// 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,
// this structure will be shared be C code and assembly code.
// It is natural aligned by design.
//
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 3c2e6d6b89d9..15dbfa1e7d6c 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -430,22 +430,22 @@ CollectProcessorCount (
**/
VOID
InitializeApData (
IN OUT CPU_MP_DATA *CpuMpData,
IN UINTN ProcessorNumber,
IN UINT32 BistData,
- IN UINTN ApTopOfStack
+ IN UINT64 ApTopOfStack
)
{
CPU_INFO_IN_HOB *CpuInfoInHob;
CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
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;
if (CpuInfoInHob[ProcessorNumber].InitialApicId >= 0xFF) {
//
// Set x2APIC mode if there are any logical processor reporting
@@ -477,13 +477,13 @@ ApWakeupFunction (
UINTN ProcessorNumber;
EFI_AP_PROCEDURE Procedure;
VOID *Parameter;
UINT32 BistData;
volatile UINT32 *ApStartupSignalBuffer;
CPU_INFO_IN_HOB *CpuInfoInHob;
- UINTN ApTopOfStack;
+ UINT64 ApTopOfStack;
//
// AP finished assembly code and begin to execute C code
//
CpuMpData = ExchangeInfo->CpuMpData;
@@ -497,13 +497,13 @@ ApWakeupFunction (
InterlockedIncrement ((UINT32 *) &CpuMpData->CpuCount);
ProcessorNumber = NumApsExecuting;
//
// 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
//
ApInitializeSync (CpuMpData);
//
// Sync BSP's Control registers to APs
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 4bfa084c85a9..64e51d87ae24 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -178,13 +178,13 @@ GetProcessorNumber:
lea eax, [esi + CpuInfoLocation]
mov edi, [eax]
GetNextProcNumber:
cmp [edi], edx ; APIC ID match?
jz ProgramStack
- add edi, 16
+ add edi, 20
inc ebx
jmp GetNextProcNumber
ProgramStack:
mov esp, [edi + 12]
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 138b97312b1d..aaabb50c5468 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -182,19 +182,18 @@ GetProcessorNumber:
lea eax, [esi + CpuInfoLocation]
mov edi, [eax]
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
xor rbp, rbp ; Clear ebp for call stack trace
push rbp
mov rbp, rsp
--
2.9.2
next prev parent reply other threads:[~2016-11-17 0:17 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-17 0:17 [PATCH 0/4] UefiCpuPkg: correct the fixed address AP stack programming on X64 Laszlo Ersek
2016-11-17 0:17 ` [PATCH 1/4] UefiCpuPkg/DxeMpInitLib: remove duplicate HobLib class dependency Laszlo Ersek
2016-11-17 0:46 ` Fan, Jeff
2016-11-17 0:17 ` [PATCH 2/4] UefiCpuPkg/MpInitLib/X64/MpFuncs.nasm: remove superfluous instruction Laszlo Ersek
2016-11-17 0:46 ` Fan, Jeff
2016-11-17 0:17 ` [PATCH 3/4] UefiCpuPkg/MpInitLib/X64/MpFuncs.nasm: fix fatal typo Laszlo Ersek
2016-11-17 0:56 ` Fan, Jeff
2016-11-17 0:17 ` Laszlo Ersek [this message]
2016-11-17 1:18 ` [PATCH 4/4] UefiCpuPkg/MpInitLib: support 64-bit AP stack addresses Fan, Jeff
2016-11-17 10:01 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161117001754.4383-5-lersek@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox