UefiCpuPkg/CpuDxe/CpuMp.c | 93 ++++++++++++++++++-------------- UefiCpuPkg/CpuMpPei/CpuMpPei.c | 96 +++++++++++++++++++--------------- 2 files changed, 109 insertions(+), 80 deletions(-)
Parallelly run the function to SeparateExceptionStacks for all CPUs and
allocate buffers together for better performance.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiCpuPkg/CpuDxe/CpuMp.c | 93 ++++++++++++++++++--------------
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 96 +++++++++++++++++++---------------
2 files changed, 109 insertions(+), 80 deletions(-)
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index f3ca813d2a..6362e03f1b 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -600,8 +600,9 @@ CollectBistDataFromHob (
// Structure for InitializeSeparateExceptionStacks
//
typedef struct {
- VOID *Buffer;
- UINTN *BufferSize;
+ VOID *Buffer;
+ UINTN BufferSize;
+ EFI_STATUS Status;
} EXCEPTION_STACK_SWITCH_CONTEXT;
/**
@@ -620,9 +621,13 @@ InitializeExceptionStackSwitchHandlers (
)
{
EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
+ UINTN Index;
+ MpInitLibWhoAmI (&Index);
SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
- InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize);
+ if ((SwitchStackData[Index].Status == EFI_NOT_STARTED) || (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL)) {
+ SwitchStackData[Index].Status = InitializeSeparateExceptionStacks (SwitchStackData[Index].Buffer, &SwitchStackData[Index].BufferSize);
+ }
}
/**
@@ -638,53 +643,63 @@ InitializeMpExceptionStackSwitchHandlers (
)
{
UINTN Index;
- UINTN Bsp;
- EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData;
+ EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
UINTN BufferSize;
+ EFI_STATUS Status;
+ VOID *Buffer;
- SwitchStackData.BufferSize = &BufferSize;
- MpInitLibWhoAmI (&Bsp);
+ SwitchStackData = AllocateRuntimeZeroPool (mNumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
for (Index = 0; Index < mNumberOfProcessors; ++Index) {
- SwitchStackData.Buffer = NULL;
- BufferSize = 0;
+ //
+ // Because the procedure may runs multiple times, use the status EFI_NOT_STARTED
+ // to indicate the procedure haven't been run yet.
+ //
+ SwitchStackData[Index].Status = EFI_NOT_STARTED;
+ }
+
+ Status = MpInitLibStartupAllCPUs (
+ InitializeExceptionStackSwitchHandlers,
+ 0,
+ SwitchStackData
+ );
+ ASSERT_EFI_ERROR (Status);
- if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&SwitchStackData);
+ BufferSize = 0;
+ for (Index = 0; Index < mNumberOfProcessors; ++Index) {
+ if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
+ BufferSize += SwitchStackData[Index].BufferSize;
} else {
- //
- // AP might need different buffer size from BSP.
- //
- MpInitLibStartupThisAP (InitializeExceptionStackSwitchHandlers, Index, NULL, 0, (VOID *)&SwitchStackData, NULL);
+ ASSERT (SwitchStackData[Index].Status == EFI_SUCCESS);
}
+ }
- if (BufferSize == 0) {
- continue;
+ if (BufferSize != 0) {
+ Buffer = AllocateRuntimeZeroPool (BufferSize);
+ BufferSize = 0;
+ for (Index = 0; Index < mNumberOfProcessors; ++Index) {
+ if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
+ SwitchStackData[Index].Buffer = (VOID *)(BufferSize + (UINTN)Buffer);
+ BufferSize += SwitchStackData[Index].BufferSize;
+ DEBUG ((
+ DEBUG_INFO,
+ "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%lX\n",
+ (UINT64)(UINTN)Index,
+ (UINT64)(UINTN)SwitchStackData[Index].Buffer,
+ (UINT64)(UINTN)SwitchStackData[Index].BufferSize
+ ));
+ }
}
- SwitchStackData.Buffer = AllocateRuntimeZeroPool (BufferSize);
- ASSERT (SwitchStackData.Buffer != NULL);
- DEBUG ((
- DEBUG_INFO,
- "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%x\n",
- (UINT64)(UINTN)Index,
- (UINT64)(UINTN)SwitchStackData.Buffer,
- (UINT32)BufferSize
- ));
-
- if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&SwitchStackData);
- } else {
- MpInitLibStartupThisAP (
- InitializeExceptionStackSwitchHandlers,
- Index,
- NULL,
- 0,
- (VOID *)&SwitchStackData,
- NULL
- );
- }
+ Status = MpInitLibStartupAllCPUs (
+ InitializeExceptionStackSwitchHandlers,
+ 0,
+ SwitchStackData
+ );
+ ASSERT_EFI_ERROR (Status);
}
+
+ FreePool (SwitchStackData);
}
/**
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index c0be11d3ad..5e4a7a3dcf 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -415,8 +415,9 @@ PeiWhoAmI (
// Structure for InitializeSeparateExceptionStacks
//
typedef struct {
- VOID *Buffer;
- UINTN *BufferSize;
+ VOID *Buffer;
+ UINTN BufferSize;
+ EFI_STATUS Status;
} EXCEPTION_STACK_SWITCH_CONTEXT;
/**
@@ -435,9 +436,13 @@ InitializeExceptionStackSwitchHandlers (
)
{
EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
+ UINTN Index;
+ MpInitLibWhoAmI (&Index);
SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
- InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize);
+ if ((SwitchStackData[Index].Status == EFI_NOT_STARTED) || (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL)) {
+ SwitchStackData[Index].Status = InitializeSeparateExceptionStacks (SwitchStackData[Index].Buffer, &SwitchStackData[Index].BufferSize);
+ }
}
/**
@@ -453,60 +458,69 @@ InitializeMpExceptionStackSwitchHandlers (
)
{
UINTN Index;
- UINTN Bsp;
- EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData;
- UINTN BufferSize;
UINTN NumberOfProcessors;
+ EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData;
+ UINTN BufferSize;
+ EFI_STATUS Status;
+ VOID *Buffer;
if (!PcdGetBool (PcdCpuStackGuard)) {
return;
}
- SwitchStackData.BufferSize = &BufferSize;
MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
- MpInitLibWhoAmI (&Bsp);
+ SwitchStackData = AllocateRuntimeZeroPool (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
for (Index = 0; Index < NumberOfProcessors; ++Index) {
- SwitchStackData.Buffer = NULL;
- BufferSize = 0;
+ //
+ // Because the procedure may runs multiple times, use the status EFI_NOT_STARTED
+ // to indicate the procedure haven't been run yet.
+ //
+ SwitchStackData[Index].Status = EFI_NOT_STARTED;
+ }
- if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&SwitchStackData);
+ Status = MpInitLibStartupAllCPUs (
+ InitializeExceptionStackSwitchHandlers,
+ 0,
+ SwitchStackData
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ BufferSize = 0;
+ for (Index = 0; Index < NumberOfProcessors; ++Index) {
+ if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
+ BufferSize += SwitchStackData[Index].BufferSize;
} else {
- //
- // AP might need different buffer size from BSP.
- //
- MpInitLibStartupThisAP (InitializeExceptionStackSwitchHandlers, Index, NULL, 0, (VOID *)&SwitchStackData, NULL);
+ ASSERT (SwitchStackData[Index].Status == EFI_SUCCESS);
}
+ }
- if (BufferSize == 0) {
- continue;
+ if (BufferSize != 0) {
+ Buffer = AllocateRuntimeZeroPool (BufferSize);
+ BufferSize = 0;
+ for (Index = 0; Index < NumberOfProcessors; ++Index) {
+ if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
+ SwitchStackData[Index].Buffer = (VOID *)(BufferSize + (UINTN)Buffer);
+ BufferSize += SwitchStackData[Index].BufferSize;
+ DEBUG ((
+ DEBUG_INFO,
+ "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%lX\n",
+ (UINT64)(UINTN)Index,
+ (UINT64)(UINTN)SwitchStackData[Index].Buffer,
+ (UINT64)(UINTN)SwitchStackData[Index].BufferSize
+ ));
+ }
}
- SwitchStackData.Buffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize));
- ASSERT (SwitchStackData.Buffer != NULL);
- ZeroMem (SwitchStackData.Buffer, EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (BufferSize)));
- DEBUG ((
- DEBUG_INFO,
- "Buffer[cpu%lu] for InitializeExceptionStackSwitchHandlers: 0x%lX with size 0x%x\n",
- (UINT64)(UINTN)Index,
- (UINT64)(UINTN)SwitchStackData.Buffer,
- (UINT32)BufferSize
- ));
-
- if (Index == Bsp) {
- InitializeExceptionStackSwitchHandlers (&SwitchStackData);
- } else {
- MpInitLibStartupThisAP (
- InitializeExceptionStackSwitchHandlers,
- Index,
- NULL,
- 0,
- (VOID *)&SwitchStackData,
- NULL
- );
- }
+ Status = MpInitLibStartupAllCPUs (
+ InitializeExceptionStackSwitchHandlers,
+ 0,
+ SwitchStackData
+ );
+ ASSERT_EFI_ERROR (Status);
}
+
+ FreePool (SwitchStackData);
}
/**
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92780): https://edk2.groups.io/g/devel/message/92780
Mute This Topic: https://groups.io/mt/93241590/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-> + MpInitLibWhoAmI (&Index);
> SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer;
> - InitializeSeparateExceptionStacks (SwitchStackData->Buffer,
> SwitchStackData->BufferSize);
> + if ((SwitchStackData[Index].Status == EFI_NOT_STARTED) ||
> (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL)) {
> + SwitchStackData[Index].Status = InitializeSeparateExceptionStacks
> (SwitchStackData[Index].Buffer, &SwitchStackData[Index].BufferSize);
> + }
> }
1. Can you add comment to explain why BufferTooSmall is checked?
I guess you want to handle the case when calls in some APs succeed
while some calls in other APs get BufferTooSmall.
So this check makes sure that the 2nd call happens only on those failed APs.
> + SwitchStackData = AllocateRuntimeZeroPool (mNumberOfProcessors *
> sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
2. Why runtime type memory?
> + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
3. Can you add assertion to make sure " SwitchStackData[Index].BufferSize != 0"?
> + SwitchStackData[Index].Buffer = (VOID *)(BufferSize + (UINTN)Buffer);
4. Use "&Buffer[BufferSize]";
>
> - SwitchStackData.BufferSize = &BufferSize;
> MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL);
> - MpInitLibWhoAmI (&Bsp);
> + SwitchStackData = AllocateRuntimeZeroPool (NumberOfProcessors *
> sizeof (EXCEPTION_STACK_SWITCH_CONTEXT));
5. No need for runtime memory.
> + for (Index = 0; Index < NumberOfProcessors; ++Index) {
> + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) {
> + BufferSize += SwitchStackData[Index].BufferSize;
6. add assertion to make sure ".BufferSize == 0" when ".Status == EFI_SUCCESS".
I think assertion here is better than my last comment which requests assertion in
the other place.
7. Please use AllocatePages in PEI because in a many-core platform the pool 64KB
limitation may cause the code fail to allocate.
Thanks,
Ray
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92807): https://edk2.groups.io/g/devel/message/92807
Mute This Topic: https://groups.io/mt/93241590/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Currently, when waking up AP, IDT table of AP will be set in 16 bit code,
and assume the IDT table base is 32 bit. However, the IDT table is created
by BSP. Issue will happen if the BSP allocates memory above 4G for BSP's
IDT table. Moreover, even the IDT table location is below 4G, the handler
function inside the IDT table is 64 bit, and it won't take effect until
CPU transfers to 64 bit long mode. There is no benefit to set IDT table in
such an early phase.
To avoid such issue, this patch moves the LIDT instruction into 64 bit
code.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 1daaa72b1e..cd95b03da8 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -64,9 +64,6 @@ BITS 16
mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
o32 lgdt [cs:si]
- mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
-o32 lidt [cs:si]
-
;
; Switch to protected mode
;
@@ -154,6 +151,11 @@ BITS 64
LongModeStart:
mov esi, ebx
+
+ ; Set IDT table at the start of 64 bit code
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)]
+ lidt [edi]
+
lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
cmp qword [edi], 1 ; ApInitConfig
jnz GetApicId
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92781): https://edk2.groups.io/g/devel/message/92781
Mute This Topic: https://groups.io/mt/93241591/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Liu, Zhiguang <zhiguang.liu@intel.com> > Sent: Thursday, August 25, 2022 10:55 AM > To: devel@edk2.groups.io > Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Dong, Eric > <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul R > <rahul.r.kumar@intel.com> > Subject: [PATCH] UefiCpuPkg/MpInitLib: Fix potential issue when IDT table is > at above 4G > > Currently, when waking up AP, IDT table of AP will be set in 16 bit code, > and assume the IDT table base is 32 bit. However, the IDT table is created > by BSP. Issue will happen if the BSP allocates memory above 4G for BSP's > IDT table. Moreover, even the IDT table location is below 4G, the handler > function inside the IDT table is 64 bit, and it won't take effect until > CPU transfers to 64 bit long mode. There is no benefit to set IDT table in > such an early phase. > To avoid such issue, this patch moves the LIDT instruction into 64 bit > code. > > Cc: Eric Dong <eric.dong@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Rahul Kumar <rahul1.kumar@intel.com> > Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index 1daaa72b1e..cd95b03da8 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > @@ -64,9 +64,6 @@ BITS 16 > mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile) > o32 lgdt [cs:si] > > - mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile) > -o32 lidt [cs:si] > - > ; > ; Switch to protected mode > ; > @@ -154,6 +151,11 @@ BITS 64 > > LongModeStart: > mov esi, ebx > + > + ; Set IDT table at the start of 64 bit code > + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)] > + lidt [edi] > + > lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)] > cmp qword [edi], 1 ; ApInitConfig > jnz GetApicId > -- > 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92808): https://edk2.groups.io/g/devel/message/92808 Mute This Topic: https://groups.io/mt/93241591/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
When switch bsp, old bsp and new bsp put CR0/CR4 into stack, and put IDT
and GDT register into a structure. After they exchange their stack, they
restore these registers. This logic is now implemented by assembly code.
This patch aims to reuse (Save/Restore)VolatileRegisters function to
replace such assembly code for better code readability.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 18 --------
UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++-
UefiCpuPkg/Library/MpInitLib/MpLib.h | 43 +++++++++----------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 21 ---------
4 files changed, 55 insertions(+), 62 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 28301bb8f0..7a4d5b35db 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -286,13 +286,6 @@ ASM_PFX(AsmExchangeRole):
;Store EFLAGS, GDTR and IDTR register to stack
pushfd
- mov eax, cr4
- push eax ; push cr4 firstly
- mov eax, cr0
- push eax
-
- sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
- sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; Store the its StackPointer
mov [esi + CPU_EXCHANGE_ROLE_INFO.StackPointer],esp
@@ -308,13 +301,6 @@ WaitForOtherStored:
jmp WaitForOtherStored
OtherStored:
- ; Since another CPU already stored its state, load them
- ; load GDTR value
- lgdt [edi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
-
- ; load IDTR value
- lidt [edi + CPU_EXCHANGE_ROLE_INFO.Idtr]
-
; load its future StackPointer
mov esp, [edi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
@@ -331,10 +317,6 @@ WaitForOtherLoaded:
OtherLoaded:
; since the other CPU already get the data it want, leave this procedure
- pop eax
- mov cr0, eax
- pop eax
- mov cr4, eax
popfd
popad
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 8d1f24370a..041a32e659 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1,7 +1,7 @@
/** @file
CPU MP Initialize Library common functions.
- Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2022, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -15,6 +15,29 @@
EFI_GUID mCpuInitMpLibHobGuid = CPU_INIT_MP_LIB_HOB_GUID;
+/**
+ Save the volatile registers required to be restored following INIT IPI.
+
+ @param[out] VolatileRegisters Returns buffer saved the volatile resisters
+**/
+VOID
+SaveVolatileRegisters (
+ OUT CPU_VOLATILE_REGISTERS *VolatileRegisters
+ );
+
+/**
+ Restore the volatile registers following INIT IPI.
+
+ @param[in] VolatileRegisters Pointer to volatile resisters
+ @param[in] IsRestoreDr TRUE: Restore DRx if supported
+ FALSE: Do not restore DRx
+**/
+VOID
+RestoreVolatileRegisters (
+ IN CPU_VOLATILE_REGISTERS *VolatileRegisters,
+ IN BOOLEAN IsRestoreDr
+ );
+
/**
The function will check if BSP Execute Disable is enabled.
@@ -83,7 +106,12 @@ FutureBSPProc (
CPU_MP_DATA *DataInHob;
DataInHob = (CPU_MP_DATA *)Buffer;
+ //
+ // Save and restore volatile registers when switch BSP
+ //
+ SaveVolatileRegisters (&DataInHob->APInfo.VolatileRegisters);
AsmExchangeRole (&DataInHob->APInfo, &DataInHob->BSPInfo);
+ RestoreVolatileRegisters (&DataInHob->APInfo.VolatileRegisters, FALSE);
}
/**
@@ -2233,7 +2261,12 @@ SwitchBSPWorker (
//
WakeUpAP (CpuMpData, FALSE, ProcessorNumber, FutureBSPProc, CpuMpData, TRUE);
+ //
+ // Save and restore volatile registers when switch BSP
+ //
+ SaveVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters);
AsmExchangeRole (&CpuMpData->BSPInfo, &CpuMpData->APInfo);
+ RestoreVolatileRegisters (&CpuMpData->BSPInfo.VolatileRegisters, FALSE);
//
// Set the BSP bit of MSR_IA32_APIC_BASE on new BSP
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 974fb76019..47b722cb2f 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -68,14 +68,31 @@ typedef struct {
UINTN Size;
} MICROCODE_PATCH_INFO;
+//
+// CPU volatile registers around INIT-SIPI-SIPI
+//
+typedef struct {
+ UINTN Cr0;
+ UINTN Cr3;
+ UINTN Cr4;
+ UINTN Dr0;
+ UINTN Dr1;
+ UINTN Dr2;
+ UINTN Dr3;
+ UINTN Dr6;
+ UINTN Dr7;
+ IA32_DESCRIPTOR Gdtr;
+ IA32_DESCRIPTOR Idtr;
+ UINT16 Tr;
+} CPU_VOLATILE_REGISTERS;
+
//
// CPU exchange information for switch BSP
//
typedef struct {
- UINT8 State; // offset 0
- UINTN StackPointer; // offset 4 / 8
- IA32_DESCRIPTOR Gdtr; // offset 8 / 16
- IA32_DESCRIPTOR Idtr; // offset 14 / 26
+ UINT8 State; // offset 0
+ UINTN StackPointer; // offset 4 / 8
+ CPU_VOLATILE_REGISTERS VolatileRegisters; // offset 8 / 16
} CPU_EXCHANGE_ROLE_INFO;
//
@@ -112,24 +129,6 @@ typedef enum {
CpuStateDisabled
} CPU_STATE;
-//
-// CPU volatile registers around INIT-SIPI-SIPI
-//
-typedef struct {
- UINTN Cr0;
- UINTN Cr3;
- UINTN Cr4;
- UINTN Dr0;
- UINTN Dr1;
- UINTN Dr2;
- UINTN Dr3;
- UINTN Dr6;
- UINTN Dr7;
- IA32_DESCRIPTOR Gdtr;
- IA32_DESCRIPTOR Idtr;
- UINT16 Tr;
-} CPU_VOLATILE_REGISTERS;
-
//
// AP related data
//
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index cd95b03da8..b7f8d48504 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -482,22 +482,13 @@ ASM_PFX(AsmExchangeRole):
push r14
push r15
- mov rax, cr0
- push rax
-
- mov rax, cr4
- push rax
-
; rsi contains MyInfo pointer
mov rsi, rcx
; rdi contains OthersInfo pointer
mov rdi, rdx
- ;Store EFLAGS, GDTR and IDTR regiter to stack
pushfq
- sgdt [rsi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
- sidt [rsi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; Store the its StackPointer
mov [rsi + CPU_EXCHANGE_ROLE_INFO.StackPointer], rsp
@@ -513,12 +504,6 @@ WaitForOtherStored:
jmp WaitForOtherStored
OtherStored:
- ; Since another CPU already stored its state, load them
- ; load GDTR value
- lgdt [rdi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
-
- ; load IDTR value
- lidt [rdi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; load its future StackPointer
mov rsp, [rdi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
@@ -538,12 +523,6 @@ OtherLoaded:
; since the other CPU already get the data it want, leave this procedure
popfq
- pop rax
- mov cr4, rax
-
- pop rax
- mov cr0, rax
-
pop r15
pop r14
pop r13
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92782): https://edk2.groups.io/g/devel/message/92782
Mute This Topic: https://groups.io/mt/93241592/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-> > ;Store EFLAGS, GDTR and IDTR register to stack > pushfd Can you update the comments? > - mov eax, cr4 > - push eax ; push cr4 firstly > - mov eax, cr0 > - push eax > - > - sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr] > - sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr] > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92805): https://edk2.groups.io/g/devel/message/92805 Mute This Topic: https://groups.io/mt/93241592/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
The API of InitializeSeparateExceptionStacks is just changed before, and
makes the struct CPU_EXCEPTION_INIT_DATA an internal definition.
Furthermore, we can even remove the struct to make core simpler.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
.../CpuExceptionCommon.h | 73 ++-------
.../CpuExceptionHandlerLib/DxeException.c | 94 ++---------
.../Ia32/ArchExceptionHandler.c | 148 ++++++++----------
.../CpuExceptionHandlerLib/PeiCpuException.c | 77 +--------
.../SecPeiCpuExceptionHandlerLib.inf | 7 +-
.../SmmCpuExceptionHandlerLib.inf | 7 +-
.../X64/ArchExceptionHandler.c | 147 ++++++++---------
.../Xcode5SecPeiCpuExceptionHandlerLib.inf | 7 +-
8 files changed, 177 insertions(+), 383 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index 11a5624f51..4593c204a6 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -49,61 +49,6 @@
#define CPU_TSS_GDT_SIZE (SIZE_2KB + CPU_TSS_DESC_SIZE + CPU_TSS_SIZE)
-typedef struct {
- //
- // The address of top of known good stack reserved for *ALL* exceptions
- // listed in field StackSwitchExceptions.
- //
- UINTN KnownGoodStackTop;
- //
- // The size of known good stack for *ONE* exception only.
- //
- UINTN KnownGoodStackSize;
- //
- // Buffer of exception vector list for stack switch.
- //
- UINT8 *StackSwitchExceptions;
- //
- // Number of exception vectors in StackSwitchExceptions.
- //
- UINTN StackSwitchExceptionNumber;
- //
- // Buffer of IDT table. It must be type of IA32_IDT_GATE_DESCRIPTOR.
- // Normally there's no need to change IDT table size.
- //
- VOID *IdtTable;
- //
- // Size of buffer for IdtTable.
- //
- UINTN IdtTableSize;
- //
- // Buffer of GDT table. It must be type of IA32_SEGMENT_DESCRIPTOR.
- //
- VOID *GdtTable;
- //
- // Size of buffer for GdtTable.
- //
- UINTN GdtTableSize;
- //
- // Pointer to start address of descriptor of exception task gate in the
- // GDT table. It must be type of IA32_TSS_DESCRIPTOR.
- //
- VOID *ExceptionTssDesc;
- //
- // Size of buffer for ExceptionTssDesc.
- //
- UINTN ExceptionTssDescSize;
- //
- // Buffer of task-state segment for exceptions. It must be type of
- // IA32_TASK_STATE_SEGMENT.
- //
- VOID *ExceptionTss;
- //
- // Size of buffer for ExceptionTss.
- //
- UINTN ExceptionTssSize;
-} CPU_EXCEPTION_INIT_DATA;
-
//
// Record exception handler information
//
@@ -345,18 +290,22 @@ CommonExceptionHandlerWorker (
);
/**
- Setup separate stack for specific exceptions.
+ Setup separate stacks for certain exception handlers.
- @param[in] StackSwitchData Pointer to data required for setuping up
- stack switch.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.
- @retval EFI_SUCCESS The exceptions have been successfully
- initialized with new stack.
- @retval EFI_INVALID_PARAMETER StackSwitchData contains invalid content.
+ @retval EFI_SUCCESS The stacks are assigned successfully.
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
+ @retval EFI_UNSUPPORTED This function is not supported.
**/
EFI_STATUS
ArchSetupExceptionStack (
- IN CPU_EXCEPTION_INIT_DATA *StackSwitchData
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
);
/**
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index d90c607bd7..076382a9b5 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -23,9 +23,8 @@ EXCEPTION_HANDLER_DATA mExceptionHandlerData = {
mExternalInterruptHandlerTable
};
-UINT8 mNewStack[CPU_STACK_SWITCH_EXCEPTION_NUMBER *
- CPU_KNOWN_GOOD_STACK_SIZE];
-UINT8 mNewGdt[CPU_TSS_GDT_SIZE];
+UINT8 mBuffer[CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE
+ + CPU_TSS_GDT_SIZE];
/**
Common exception handler.
@@ -123,85 +122,18 @@ InitializeSeparateExceptionStacks (
IN OUT UINTN *BufferSize
)
{
- CPU_EXCEPTION_INIT_DATA EssData;
- IA32_DESCRIPTOR Idtr;
- IA32_DESCRIPTOR Gdtr;
- UINTN NeedBufferSize;
- UINTN StackTop;
- UINT8 *NewGdtTable;
-
- //
- // X64 needs only one TSS of current task working for all exceptions
- // because of its IST feature. IA32 needs one TSS for each exception
- // in addition to current task. To simplify the code, we report the
- // needed memory for IA32 case to cover both IA32 and X64 exception
- // stack switch.
- //
- // Layout of memory needed for each processor:
- // --------------------------------
- // | Alignment | (just in case)
- // --------------------------------
- // | |
- // | Original GDT |
- // | |
- // --------------------------------
- // | Current task descriptor |
- // --------------------------------
- // | |
- // | Exception task descriptors | X ExceptionNumber
- // | |
- // --------------------------------
- // | Current task-state segment |
- // --------------------------------
- // | |
- // | Exception task-state segment | X ExceptionNumber
- // | |
- // --------------------------------
- //
- AsmReadGdtr (&Gdtr);
+ VOID *LocalBuffer;
+ UINTN LocalBufferSize;
+ EFI_STATUS Status;
+
if ((Buffer == NULL) && (BufferSize == NULL)) {
- SetMem (mNewGdt, sizeof (mNewGdt), 0);
- StackTop = (UINTN)mNewStack + sizeof (mNewStack);
- NewGdtTable = mNewGdt;
+ SetMem (mBuffer, sizeof (mBuffer), 0);
+ LocalBuffer = mBuffer;
+ LocalBufferSize = sizeof (mBuffer);
+ Status = ArchSetupExceptionStack (LocalBuffer, &LocalBufferSize);
+ ASSERT_EFI_ERROR (Status);
+ return Status;
} else {
- if (BufferSize == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- //
- // Total needed size includes stack size, new GDT table size, TSS size.
- // Add another DESCRIPTOR size for alignment requiremet.
- //
- NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
- CPU_TSS_DESC_SIZE + Gdtr.Limit + 1 +
- CPU_TSS_SIZE +
- sizeof (IA32_TSS_DESCRIPTOR);
- if (*BufferSize < NeedBufferSize) {
- *BufferSize = NeedBufferSize;
- return EFI_BUFFER_TOO_SMALL;
- }
-
- if (Buffer == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
- NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
+ return ArchSetupExceptionStack (Buffer, BufferSize);
}
-
- AsmReadIdtr (&Idtr);
- EssData.KnownGoodStackTop = StackTop;
- EssData.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
- EssData.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
- EssData.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
- EssData.IdtTable = (VOID *)Idtr.Base;
- EssData.IdtTableSize = Idtr.Limit + 1;
- EssData.GdtTable = NewGdtTable;
- EssData.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
- EssData.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
- EssData.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
- EssData.ExceptionTssSize = CPU_TSS_SIZE;
-
- return ArchSetupExceptionStack (&EssData);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index 194d3a499b..8c398ebc5b 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -104,108 +104,97 @@ ArchRestoreExceptionContext (
}
/**
- Setup separate stack for given exceptions.
+ Setup separate stacks for certain exception handlers.
- @param[in] StackSwitchData Pointer to data required for setuping up
- stack switch.
-
- @retval EFI_SUCCESS The exceptions have been successfully
- initialized with new stack.
- @retval EFI_INVALID_PARAMETER StackSwitchData contains invalid content.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.
+ @retval EFI_SUCCESS The stacks are assigned successfully.
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
**/
EFI_STATUS
ArchSetupExceptionStack (
- IN CPU_EXCEPTION_INIT_DATA *StackSwitchData
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
IA32_DESCRIPTOR Gdtr;
IA32_DESCRIPTOR Idtr;
IA32_IDT_GATE_DESCRIPTOR *IdtTable;
IA32_TSS_DESCRIPTOR *TssDesc;
+ IA32_TSS_DESCRIPTOR *TssDescBase;
IA32_TASK_STATE_SEGMENT *Tss;
+ VOID *NewGdtTable;
UINTN StackTop;
UINTN Index;
UINTN Vector;
UINTN TssBase;
- UINTN GdtSize;
+ UINT8 *StackSwitchExceptions;
+ UINTN NeedBufferSize;
EXCEPTION_HANDLER_TEMPLATE_MAP TemplateMap;
- if ((StackSwitchData == NULL) ||
- (StackSwitchData->KnownGoodStackTop == 0) ||
- (StackSwitchData->KnownGoodStackSize == 0) ||
- (StackSwitchData->StackSwitchExceptions == NULL) ||
- (StackSwitchData->StackSwitchExceptionNumber == 0) ||
- (StackSwitchData->StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
- (StackSwitchData->GdtTable == NULL) ||
- (StackSwitchData->IdtTable == NULL) ||
- (StackSwitchData->ExceptionTssDesc == NULL) ||
- (StackSwitchData->ExceptionTss == NULL))
- {
+ if (BufferSize == NULL) {
return EFI_INVALID_PARAMETER;
}
//
- // The caller is responsible for that the GDT table, no matter the existing
- // one or newly allocated, has enough space to hold descriptors for exception
- // task-state segments.
+ // Total needed size includes stack size, new GDT table size, TSS size.
+ // Add another DESCRIPTOR size for alignment requiremet.
//
- if (((UINTN)StackSwitchData->GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
- return EFI_INVALID_PARAMETER;
- }
-
- if ((UINTN)StackSwitchData->ExceptionTssDesc < (UINTN)(StackSwitchData->GdtTable)) {
- return EFI_INVALID_PARAMETER;
- }
-
- if ((UINTN)StackSwitchData->ExceptionTssDesc + StackSwitchData->ExceptionTssDescSize >
- ((UINTN)(StackSwitchData->GdtTable) + StackSwitchData->GdtTableSize))
- {
- return EFI_INVALID_PARAMETER;
- }
-
+ // Layout of memory needed for each processor:
+ // --------------------------------
+ // | |
+ // | Stack Size | X ExceptionNumber
+ // | |
+ // --------------------------------
+ // | Alignment | (just in case)
+ // --------------------------------
+ // | |
+ // | Original GDT |
+ // | |
+ // --------------------------------
+ // | Current task descriptor |
+ // --------------------------------
+ // | |
+ // | Exception task descriptors | X ExceptionNumber
+ // | |
+ // --------------------------------
+ // | Current task-state segment |
+ // --------------------------------
+ // | |
+ // | Exception task-state segment | X ExceptionNumber
+ // | |
+ // --------------------------------
//
- // We need one descriptor and one TSS for current task and every exception
- // specified.
- //
- if (StackSwitchData->ExceptionTssDescSize <
- sizeof (IA32_TSS_DESCRIPTOR) * (StackSwitchData->StackSwitchExceptionNumber + 1))
- {
- return EFI_INVALID_PARAMETER;
+ AsmReadGdtr (&Gdtr);
+ NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
+ sizeof (IA32_TSS_DESCRIPTOR) +
+ Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE +
+ CPU_TSS_SIZE;
+
+ if (*BufferSize < NeedBufferSize) {
+ *BufferSize = NeedBufferSize;
+ return EFI_BUFFER_TOO_SMALL;
}
- if (StackSwitchData->ExceptionTssSize <
- sizeof (IA32_TASK_STATE_SEGMENT) * (StackSwitchData->StackSwitchExceptionNumber + 1))
- {
+ if (Buffer == NULL) {
return EFI_INVALID_PARAMETER;
}
- TssDesc = StackSwitchData->ExceptionTssDesc;
- Tss = StackSwitchData->ExceptionTss;
-
- //
- // Initialize new GDT table and/or IDT table, if any
- //
AsmReadIdtr (&Idtr);
- AsmReadGdtr (&Gdtr);
-
- GdtSize = (UINTN)TssDesc +
- sizeof (IA32_TSS_DESCRIPTOR) *
- (StackSwitchData->StackSwitchExceptionNumber + 1) -
- (UINTN)(StackSwitchData->GdtTable);
- if ((UINTN)StackSwitchData->GdtTable != Gdtr.Base) {
- CopyMem (StackSwitchData->GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
- Gdtr.Base = (UINTN)StackSwitchData->GdtTable;
- Gdtr.Limit = (UINT16)GdtSize - 1;
- }
-
- if ((UINTN)StackSwitchData->IdtTable != Idtr.Base) {
- Idtr.Base = (UINTN)StackSwitchData->IdtTable;
- }
+ StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
+ NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
+ TssDesc = (IA32_TSS_DESCRIPTOR *)((UINTN)NewGdtTable + Gdtr.Limit + 1);
+ Tss = (IA32_TASK_STATE_SEGMENT *)((UINTN)TssDesc + CPU_TSS_DESC_SIZE);
+ TssDescBase = TssDesc;
- if (StackSwitchData->IdtTableSize > 0) {
- Idtr.Limit = (UINT16)(StackSwitchData->IdtTableSize - 1);
- }
+ CopyMem (NewGdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
+ Gdtr.Base = (UINTN)NewGdtTable;
+ Gdtr.Limit = (UINT16)(Gdtr.Limit + CPU_TSS_DESC_SIZE);
//
// Fixup current task descriptor. Task-state segment for current task will
@@ -226,10 +215,10 @@ ArchSetupExceptionStack (
// Fixup exception task descriptor and task-state segment
//
AsmGetTssTemplateMap (&TemplateMap);
- StackTop = StackSwitchData->KnownGoodStackTop - CPU_STACK_ALIGNMENT;
+ StackTop = StackTop - CPU_STACK_ALIGNMENT;
StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT);
- IdtTable = StackSwitchData->IdtTable;
- for (Index = 0; Index < StackSwitchData->StackSwitchExceptionNumber; ++Index) {
+ IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base;
+ for (Index = 0; Index < CPU_STACK_SWITCH_EXCEPTION_NUMBER; ++Index) {
TssDesc += 1;
Tss += 1;
@@ -250,7 +239,7 @@ ArchSetupExceptionStack (
//
// Fixup TSS
//
- Vector = StackSwitchData->StackSwitchExceptions[Index];
+ Vector = StackSwitchExceptions[Index];
if ((Vector >= CPU_EXCEPTION_NUM) ||
(Vector >= (Idtr.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR)))
{
@@ -270,7 +259,7 @@ ArchSetupExceptionStack (
Tss->FS = AsmReadFs ();
Tss->GS = AsmReadGs ();
- StackTop -= StackSwitchData->KnownGoodStackSize;
+ StackTop -= CPU_KNOWN_GOOD_STACK_SIZE;
//
// Update IDT to use Task Gate for given exception
@@ -290,12 +279,7 @@ ArchSetupExceptionStack (
//
// Load current task
//
- AsmWriteTr ((UINT16)((UINTN)StackSwitchData->ExceptionTssDesc - Gdtr.Base));
-
- //
- // Publish IDT
- //
- AsmWriteIdtr (&Idtr);
+ AsmWriteTr ((UINT16)((UINTN)TssDescBase - Gdtr.Base));
return EFI_SUCCESS;
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index 5952295126..940d83a92f 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -170,84 +170,9 @@ InitializeSeparateExceptionStacks (
IN OUT UINTN *BufferSize
)
{
- CPU_EXCEPTION_INIT_DATA EssData;
- IA32_DESCRIPTOR Idtr;
- IA32_DESCRIPTOR Gdtr;
- UINTN NeedBufferSize;
- UINTN StackTop;
- UINT8 *NewGdtTable;
-
- //
- // X64 needs only one TSS of current task working for all exceptions
- // because of its IST feature. IA32 needs one TSS for each exception
- // in addition to current task. To simplify the code, we report the
- // needed memory for IA32 case to cover both IA32 and X64 exception
- // stack switch.
- //
- // Layout of memory needed for each processor:
- // --------------------------------
- // | Alignment | (just in case)
- // --------------------------------
- // | |
- // | Original GDT |
- // | |
- // --------------------------------
- // | Current task descriptor |
- // --------------------------------
- // | |
- // | Exception task descriptors | X ExceptionNumber
- // | |
- // --------------------------------
- // | Current task-state segment |
- // --------------------------------
- // | |
- // | Exception task-state segment | X ExceptionNumber
- // | |
- // --------------------------------
- //
-
if ((Buffer == NULL) && (BufferSize == NULL)) {
return EFI_UNSUPPORTED;
}
- if (BufferSize == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- AsmReadGdtr (&Gdtr);
- //
- // Total needed size includes stack size, new GDT table size, TSS size.
- // Add another DESCRIPTOR size for alignment requiremet.
- //
- NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
- CPU_TSS_DESC_SIZE + Gdtr.Limit + 1 +
- CPU_TSS_SIZE +
- sizeof (IA32_TSS_DESCRIPTOR);
- if (*BufferSize < NeedBufferSize) {
- *BufferSize = NeedBufferSize;
- return EFI_BUFFER_TOO_SMALL;
- }
-
- if (Buffer == NULL) {
- return EFI_INVALID_PARAMETER;
- }
-
- StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
- NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
-
- AsmReadIdtr (&Idtr);
- EssData.KnownGoodStackTop = StackTop;
- EssData.KnownGoodStackSize = CPU_KNOWN_GOOD_STACK_SIZE;
- EssData.StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
- EssData.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;
- EssData.IdtTable = (VOID *)Idtr.Base;
- EssData.IdtTableSize = Idtr.Limit + 1;
- EssData.GdtTable = NewGdtTable;
- EssData.GdtTableSize = CPU_TSS_DESC_SIZE + Gdtr.Limit + 1;
- EssData.ExceptionTssDesc = NewGdtTable + Gdtr.Limit + 1;
- EssData.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;
- EssData.ExceptionTss = NewGdtTable + Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE;
- EssData.ExceptionTssSize = CPU_TSS_SIZE;
-
- return ArchSetupExceptionStack (&EssData);
+ return ArchSetupExceptionStack (Buffer, BufferSize);
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
index 8ae4feae62..6a170286c8 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
@@ -1,7 +1,7 @@
## @file
# CPU Exception Handler library instance for SEC/PEI modules.
#
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -50,6 +50,11 @@
PeCoffGetEntryPointLib
VmgExitLib
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
+
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
index c9f20da058..9dde07612a 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
@@ -1,7 +1,7 @@
## @file
# CPU Exception Handler library instance for SMM modules.
#
-# Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2013 - 2022, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -53,6 +53,11 @@
DebugLib
VmgExitLib
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
+
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index c14ac66c43..e6b2a83575 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -109,19 +109,22 @@ ArchRestoreExceptionContext (
}
/**
- Setup separate stack for given exceptions.
+ Setup separate stacks for certain exception handlers.
- @param[in] StackSwitchData Pointer to data required for setuping up
- stack switch.
-
- @retval EFI_SUCCESS The exceptions have been successfully
- initialized with new stack.
- @retval EFI_INVALID_PARAMETER StackSwitchData contains invalid content.
+ @param[in] Buffer Point to buffer used to separate exception stack.
+ @param[in, out] BufferSize On input, it indicates the byte size of Buffer.
+ If the size is not enough, the return status will
+ be EFI_BUFFER_TOO_SMALL, and output BufferSize
+ will be the size it needs.
+ @retval EFI_SUCCESS The stacks are assigned successfully.
+ @retval EFI_BUFFER_TOO_SMALL This BufferSize is too small.
+ @retval EFI_UNSUPPORTED This function is not supported.
**/
EFI_STATUS
ArchSetupExceptionStack (
- IN CPU_EXCEPTION_INIT_DATA *StackSwitchData
+ IN VOID *Buffer,
+ IN OUT UINTN *BufferSize
)
{
IA32_DESCRIPTOR Gdtr;
@@ -129,86 +132,77 @@ ArchSetupExceptionStack (
IA32_IDT_GATE_DESCRIPTOR *IdtTable;
IA32_TSS_DESCRIPTOR *TssDesc;
IA32_TASK_STATE_SEGMENT *Tss;
+ VOID *NewGdtTable;
UINTN StackTop;
UINTN Index;
UINTN Vector;
UINTN TssBase;
- UINTN GdtSize;
-
- if ((StackSwitchData == NULL) ||
- (StackSwitchData->KnownGoodStackTop == 0) ||
- (StackSwitchData->KnownGoodStackSize == 0) ||
- (StackSwitchData->StackSwitchExceptions == NULL) ||
- (StackSwitchData->StackSwitchExceptionNumber == 0) ||
- (StackSwitchData->StackSwitchExceptionNumber > CPU_EXCEPTION_NUM) ||
- (StackSwitchData->GdtTable == NULL) ||
- (StackSwitchData->IdtTable == NULL) ||
- (StackSwitchData->ExceptionTssDesc == NULL) ||
- (StackSwitchData->ExceptionTss == NULL))
- {
- return EFI_INVALID_PARAMETER;
- }
-
- //
- // The caller is responsible for that the GDT table, no matter the existing
- // one or newly allocated, has enough space to hold descriptors for exception
- // task-state segments.
- //
- if (((UINTN)StackSwitchData->GdtTable & (IA32_GDT_ALIGNMENT - 1)) != 0) {
- return EFI_INVALID_PARAMETER;
- }
-
- if ((UINTN)StackSwitchData->ExceptionTssDesc < (UINTN)(StackSwitchData->GdtTable)) {
- return EFI_INVALID_PARAMETER;
- }
-
- if (((UINTN)StackSwitchData->ExceptionTssDesc + StackSwitchData->ExceptionTssDescSize) >
- ((UINTN)(StackSwitchData->GdtTable) + StackSwitchData->GdtTableSize))
- {
- return EFI_INVALID_PARAMETER;
- }
+ UINT8 *StackSwitchExceptions;
+ UINTN NeedBufferSize;
- //
- // One task gate descriptor and one task-state segment are needed.
- //
- if (StackSwitchData->ExceptionTssDescSize < sizeof (IA32_TSS_DESCRIPTOR)) {
- return EFI_INVALID_PARAMETER;
- }
-
- if (StackSwitchData->ExceptionTssSize < sizeof (IA32_TASK_STATE_SEGMENT)) {
+ if (BufferSize == NULL) {
return EFI_INVALID_PARAMETER;
}
//
// Interrupt stack table supports only 7 vectors.
//
- TssDesc = StackSwitchData->ExceptionTssDesc;
- Tss = StackSwitchData->ExceptionTss;
- if (StackSwitchData->StackSwitchExceptionNumber > ARRAY_SIZE (Tss->IST)) {
- return EFI_INVALID_PARAMETER;
+ if (CPU_STACK_SWITCH_EXCEPTION_NUMBER > ARRAY_SIZE (Tss->IST)) {
+ return EFI_UNSUPPORTED;
}
//
- // Initialize new GDT table and/or IDT table, if any
+ // Total needed size includes stack size, new GDT table size, TSS size.
+ // Add another DESCRIPTOR size for alignment requiremet.
+ //
+ // Layout of memory needed for each processor:
+ // --------------------------------
+ // | |
+ // | Stack Size | X ExceptionNumber
+ // | |
+ // --------------------------------
+ // | Alignment | (just in case)
+ // --------------------------------
+ // | |
+ // | Original GDT |
+ // | |
+ // --------------------------------
+ // | Current task descriptor |
+ // --------------------------------
+ // | |
+ // | Exception task descriptors | X 1
+ // | |
+ // --------------------------------
+ // | |
+ // | Exception task-state segment | X 1
+ // | |
+ // --------------------------------
//
- AsmReadIdtr (&Idtr);
AsmReadGdtr (&Gdtr);
-
- GdtSize = (UINTN)TssDesc + sizeof (IA32_TSS_DESCRIPTOR) -
- (UINTN)(StackSwitchData->GdtTable);
- if ((UINTN)StackSwitchData->GdtTable != Gdtr.Base) {
- CopyMem (StackSwitchData->GdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
- Gdtr.Base = (UINTN)StackSwitchData->GdtTable;
- Gdtr.Limit = (UINT16)GdtSize - 1;
+ NeedBufferSize = CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE +
+ sizeof (IA32_TSS_DESCRIPTOR) +
+ Gdtr.Limit + 1 + CPU_TSS_DESC_SIZE +
+ CPU_TSS_SIZE;
+
+ if (*BufferSize < NeedBufferSize) {
+ *BufferSize = NeedBufferSize;
+ return EFI_BUFFER_TOO_SMALL;
}
- if ((UINTN)StackSwitchData->IdtTable != Idtr.Base) {
- Idtr.Base = (UINTN)StackSwitchData->IdtTable;
+ if (Buffer == NULL) {
+ return EFI_INVALID_PARAMETER;
}
- if (StackSwitchData->IdtTableSize > 0) {
- Idtr.Limit = (UINT16)(StackSwitchData->IdtTableSize - 1);
- }
+ AsmReadIdtr (&Idtr);
+ StackSwitchExceptions = CPU_STACK_SWITCH_EXCEPTION_LIST;
+ StackTop = (UINTN)Buffer + CPU_STACK_SWITCH_EXCEPTION_NUMBER * CPU_KNOWN_GOOD_STACK_SIZE;
+ NewGdtTable = ALIGN_POINTER (StackTop, sizeof (IA32_TSS_DESCRIPTOR));
+ TssDesc = (IA32_TSS_DESCRIPTOR *)((UINTN)NewGdtTable + Gdtr.Limit + 1);
+ Tss = (IA32_TASK_STATE_SEGMENT *)((UINTN)TssDesc + CPU_TSS_DESC_SIZE);
+
+ CopyMem (NewGdtTable, (VOID *)Gdtr.Base, Gdtr.Limit + 1);
+ Gdtr.Base = (UINTN)NewGdtTable;
+ Gdtr.Limit = (UINT16)(Gdtr.Limit + CPU_TSS_DESC_SIZE);
//
// Fixup current task descriptor. Task-state segment for current task will
@@ -231,20 +225,20 @@ ArchSetupExceptionStack (
// Fixup exception task descriptor and task-state segment
//
ZeroMem (Tss, sizeof (*Tss));
- StackTop = StackSwitchData->KnownGoodStackTop - CPU_STACK_ALIGNMENT;
+ StackTop = StackTop - CPU_STACK_ALIGNMENT;
StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT);
- IdtTable = StackSwitchData->IdtTable;
- for (Index = 0; Index < StackSwitchData->StackSwitchExceptionNumber; ++Index) {
+ IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)Idtr.Base;
+ for (Index = 0; Index < CPU_STACK_SWITCH_EXCEPTION_NUMBER; ++Index) {
//
// Fixup IST
//
Tss->IST[Index] = StackTop;
- StackTop -= StackSwitchData->KnownGoodStackSize;
+ StackTop -= CPU_KNOWN_GOOD_STACK_SIZE;
//
// Set the IST field to enable corresponding IST
//
- Vector = StackSwitchData->StackSwitchExceptions[Index];
+ Vector = StackSwitchExceptions[Index];
if ((Vector >= CPU_EXCEPTION_NUM) ||
(Vector >= (Idtr.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR)))
{
@@ -262,12 +256,7 @@ ArchSetupExceptionStack (
//
// Load current task
//
- AsmWriteTr ((UINT16)((UINTN)StackSwitchData->ExceptionTssDesc - Gdtr.Base));
-
- //
- // Publish IDT
- //
- AsmWriteIdtr (&Idtr);
+ AsmWriteTr ((UINT16)((UINTN)TssDesc - Gdtr.Base));
return EFI_SUCCESS;
}
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
index a15f125d5b..6d2f66504a 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHandlerLib.inf
@@ -2,7 +2,7 @@
# CPU Exception Handler library instance for SEC/PEI modules.
#
# Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
# This is the XCODE5 variant of the SEC/PEI CpuExceptionHandlerLib. This
@@ -55,6 +55,11 @@
PeCoffGetEntryPointLib
VmgExitLib
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList
+ gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize
+
[FeaturePcd]
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackGuard ## CONSUMES
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92783): https://edk2.groups.io/g/devel/message/92783
Mute This Topic: https://groups.io/mt/93241593/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-© 2016 - 2024 Red Hat, Inc.