Acked-by: Eric Dong <eric.d...@intel.com>
-----Original Message-----
From: Ni, Ray <ray...@intel.com>
Sent: Friday, May 20, 2022 10:16 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.d...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>
Subject: [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks
Today InitializeCpuExceptionHandlersEx is called from three modules:
1. DxeCore (links to DxeCpuExceptionHandlerLib)
DxeCore expects it initializes the IDT entries as well as
assigning separate stacks for #DF and #PF.
2. CpuMpPei (links to PeiCpuExceptionHandlerLib)
and CpuDxe (links to DxeCpuExceptionHandlerLib)
It's called for each thread for only assigning separate stacks for
#DF and #PF. The IDT entries initialization is skipped because
caller sets InitData->X64.InitDefaultHandlers to FALSE.
Additionally, SecPeiCpuExceptionHandlerLib, SmmCpuExceptionHandlerLib also
implement such API and the behavior of the API is simply to initialize IDT
entries only.
Because it mixes the IDT entries initialization and separate stacks assignment
for certain exception handlers together, in order to know whether the function
call only initializes IDT entries, or assigns stacks, we need to check:
1. value of InitData->X64.InitDefaultHandlers 2. library instance
This patch cleans up the code to separate the stack assignment to a new API:
InitializeSeparateExceptionStacks().
Only when caller calls the new API, the separate stacks are assigned.
With this change, the SecPei and Smm instance can return unsupported which
gives caller a very clear status.
The old API InitializeCpuExceptionHandlersEx() is removed in this patch.
Because no platform module is consuming the old API, the impact is none.
Signed-off-by: Ray Ni <ray...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 2 +-
.../Include/Library/CpuExceptionHandlerLib.h | 24 ++---
.../CpuExceptionHandlerLibNull.c | 26 ++----
UefiCpuPkg/CpuDxe/CpuMp.c | 6 +-
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 4 +-
.../CpuExceptionHandlerLib/DxeException.c | 91 ++++++-------------
.../CpuExceptionHandlerLib/PeiCpuException.c | 51 ++---------
.../SecPeiCpuException.c | 27 ++----
.../CpuExceptionHandlerLib/SmmException.c | 27 ++----
9 files changed, 74 insertions(+), 184 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 2c27fc0695..83f49d7c00 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -253,7 +253,7 @@ DxeMain (
VectorInfoList = (EFI_VECTOR_HANDOFF_INFO *)(GET_GUID_HOB_DATA (GuidHob));
} - Status = InitializeCpuExceptionHandlersEx (VectorInfoList, NULL);+
Status = InitializeCpuExceptionHandlers (VectorInfoList); ASSERT_EFI_ERROR
(Status); //diff --git
a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
index d4649bebe1..9a495081f7 100644
--- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
+++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
@@ -103,32 +103,20 @@ InitializeCpuExceptionHandlers (
); /**- Initializes all CPU exceptions entries with optional extra
initializations.+ Setup separate stacks for certain exception handlers. - By
default, this method should include all functionalities implemented by-
InitializeCpuExceptionHandlers(), plus extra initialization works, if any.-
This could be done by calling InitializeCpuExceptionHandlers() directly- in
this method besides the extra works.+ InitData is optional and processor arch
dependent. - InitData is optional and its use and content are processor arch
dependent.- The typical usage of it is to convey resources which have to be
reserved- elsewhere and are necessary for the extra initializations of
exception.+ @param[in] InitData Pointer to data optional for information
about how+ to assign stacks for certain exception
handlers. - @param[in] VectorInfo Pointer to reserved vector list.-
@param[in] InitData Pointer to data optional for extra initializations-
of exception.-- @retval EFI_SUCCESS The
exceptions have been successfully-
initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains
invalid- content.+ @retval EFI_SUCCESS
The stacks are assigned successfully. @retval EFI_UNSUPPORTED
This function is not supported. **/ EFI_STATUS
EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO
*VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN
CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ); diff --git
a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
index 54f38788fe..8aeedcb4d1 100644
---
a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c
+++ b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandle
+++ rLibNull.c
@@ -82,34 +82,22 @@ DumpCpuContext (
} /**- Initializes all CPU exceptions entries with optional extra
initializations.+ Setup separate stacks for certain exception handlers. - By
default, this method should include all functionalities implemented by-
InitializeCpuExceptionHandlers(), plus extra initialization works, if any.-
This could be done by calling InitializeCpuExceptionHandlers() directly- in
this method besides the extra works.+ InitData is optional and processor arch
dependent. - InitData is optional and its use and content are processor arch
dependent.- The typical usage of it is to convey resources which have to be
reserved- elsewhere and are necessary for the extra initializations of
exception.+ @param[in] InitData Pointer to data optional for information
about how+ to assign stacks for certain exception
handlers. - @param[in] VectorInfo Pointer to reserved vector list.-
@param[in] InitData Pointer to data optional for extra initializations-
of exception.-- @retval EFI_SUCCESS The
exceptions have been successfully-
initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains
invalid- content.+ @retval EFI_SUCCESS
The stacks are assigned successfully. @retval EFI_UNSUPPORTED
This function is not supported. **/ EFI_STATUS
EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO
*VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN
CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ) {- return
InitializeCpuExceptionHandlers (VectorInfo);+ return EFI_UNSUPPORTED; }diff
--git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
index 1f218367b3..e385f585c7 100644
--- a/UefiCpuPkg/CpuDxe/CpuMp.c
+++ b/UefiCpuPkg/CpuDxe/CpuMp.c
@@ -1,7 +1,7 @@
/** @file CPU DXE Module to produce CPU MP Protocol. - Copyright (c) 2008 -
2017, Intel Corporation. All rights reserved.<BR>+ Copyright (c) 2008 - 2022,
Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier:
BSD-2-Clause-Patent **/@@ -617,7 +617,7 @@ GetGdtr (
/** Initializes CPU exceptions handlers for the sake of stack switch
requirement. - This function is a wrapper of InitializeCpuExceptionHandlersEx.
It's mainly+ This function is a wrapper of InitializeSeparateExceptionStacks.
It's mainly for the sake of AP's init because of EFI_AP_PROCEDURE API
requirement. @param[in,out] Buffer The pointer to private data buffer.@@
-641,7 +641,7 @@ InitializeExceptionStackSwitchHandlers (
AsmReadIdtr (&Idtr); EssData->Ia32.IdtTable = (VOID *)Idtr.Base;
EssData->Ia32.IdtTableSize = Idtr.Limit + 1;- Status =
InitializeCpuExceptionHandlersEx (NULL, EssData);+ Status
= InitializeSeparateExceptionStacks (EssData); ASSERT_EFI_ERROR (Status); }
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index 1e68c91d95..d4786979fa 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -432,7 +432,7 @@ GetGdtr (
/** Initializes CPU exceptions handlers for the sake of stack switch
requirement. - This function is a wrapper of InitializeCpuExceptionHandlersEx.
It's mainly+ This function is a wrapper of InitializeSeparateExceptionStacks.
It's mainly for the sake of AP's init because of EFI_AP_PROCEDURE API
requirement. @param[in,out] Buffer The pointer to private data buffer.@@
-456,7 +456,7 @@ InitializeExceptionStackSwitchHandlers (
AsmReadIdtr (&Idtr); EssData->Ia32.IdtTable = (VOID *)Idtr.Base;
EssData->Ia32.IdtTableSize = Idtr.Limit + 1;- Status =
InitializeCpuExceptionHandlersEx (NULL, EssData);+ Status
= InitializeSeparateExceptionStacks (EssData); ASSERT_EFI_ERROR (Status); }
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
index c7c1fe31d2..e62bb5e6c0 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c
@@ -103,82 +103,49 @@ RegisterCpuInterruptHandler (
} /**- Initializes CPU exceptions entries and setup stack switch for given
exceptions.+ Setup separate stacks for certain exception handlers. - This
method will call InitializeCpuExceptionHandlers() to setup default- exception
handlers unless indicated not to do it explicitly.+ InitData is optional and
processor arch dependent. - If InitData is passed with NULL, this method will
use the resource reserved- by global variables to initialize it; Otherwise it
will use data in InitData- to setup stack switch. This is for the different
use cases in DxeCore and- Cpu MP exception initialization.+ @param[in]
InitData Pointer to data optional for information about how+
to assign stacks for certain exception handlers. - @param[in]
VectorInfo Pointer to reserved vector list.- @param[in] InitData
Pointer to data required to setup stack switch for-
given exceptions.-- @retval EFI_SUCCESS The exceptions have been
successfully- initialized.- @retval
EFI_INVALID_PARAMETER VectorInfo or InitData contains invalid-
content.+ @retval EFI_SUCCESS The stacks are
assigned successfully.+ @retval EFI_UNSUPPORTED This function is not
supported. **/ EFI_STATUS EFIAPI-InitializeCpuExceptionHandlersEx (- IN
EFI_VECTOR_HANDOFF_INFO *VectorInfo
OPTIONAL,+InitializeSeparateExceptionStacks ( IN CPU_EXCEPTION_INIT_DATA
*InitData OPTIONAL ) {- EFI_STATUS Status;
CPU_EXCEPTION_INIT_DATA EssData; IA32_DESCRIPTOR Idtr;
IA32_DESCRIPTOR Gdtr; - //- // To avoid repeat initialization of
default handlers, the caller should pass- // an extended init data with
InitDefaultHandlers set to FALSE. There's no- // need to call this method to
just initialize default handlers. Call non-ex- // version instead; or this
method must be implemented as a simple wrapper of- // non-ex version of it, if
this version has to be called.- //- if ((InitData == NULL) ||
InitData->X64.InitDefaultHandlers) {- Status =
InitializeCpuExceptionHandlers (VectorInfo);- } else {- Status =
EFI_SUCCESS;- }-- if (!EFI_ERROR (Status)) {- //- // Initializing stack
switch is only necessary for Stack Guard functionality.- //- if
(PcdGetBool (PcdCpuStackGuard)) {- if (InitData == NULL) {- SetMem
(mNewGdt, sizeof (mNewGdt), 0);-- AsmReadIdtr (&Idtr);-
AsmReadGdtr (&Gdtr);-- EssData.X64.Revision =
CPU_EXCEPTION_INIT_DATA_REV;- EssData.X64.KnownGoodStackTop =
(UINTN)mNewStack + sizeof (mNewStack);- EssData.X64.KnownGoodStackSize
= CPU_KNOWN_GOOD_STACK_SIZE;- EssData.X64.StackSwitchExceptions
= CPU_STACK_SWITCH_EXCEPTION_LIST;-
EssData.X64.StackSwitchExceptionNumber = CPU_STACK_SWITCH_EXCEPTION_NUMBER;-
EssData.X64.IdtTable = (VOID *)Idtr.Base;-
EssData.X64.IdtTableSize = Idtr.Limit + 1;-
EssData.X64.GdtTable = mNewGdt;-
EssData.X64.GdtTableSize = sizeof (mNewGdt);-
EssData.X64.ExceptionTssDesc = mNewGdt + Gdtr.Limit + 1;-
EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;-
EssData.X64.ExceptionTss = mNewGdt + Gdtr.Limit + 1 +
CPU_TSS_DESC_SIZE;- EssData.X64.ExceptionTssSize =
CPU_TSS_SIZE;-- InitData = &EssData;- }-- Status =
ArchSetupExceptionStack (InitData);- }+ if (InitData == NULL) {+ SetMem
(mNewGdt, sizeof (mNewGdt), 0);++ AsmReadIdtr (&Idtr);+ AsmReadGdtr
(&Gdtr);++ EssData.X64.Revision =
CPU_EXCEPTION_INIT_DATA_REV;+ EssData.X64.KnownGoodStackTop =
(UINTN)mNewStack + sizeof (mNewStack);+ EssData.X64.KnownGoodStackSize
= CPU_KNOWN_GOOD_STACK_SIZE;+ EssData.X64.StackSwitchExceptions =
CPU_STACK_SWITCH_EXCEPTION_LIST;+ EssData.X64.StackSwitchExceptionNumber =
CPU_STACK_SWITCH_EXCEPTION_NUMBER;+ EssData.X64.IdtTable =
(VOID *)Idtr.Base;+ EssData.X64.IdtTableSize = Idtr.Limit +
1;+ EssData.X64.GdtTable = mNewGdt;+
EssData.X64.GdtTableSize = sizeof (mNewGdt);+
EssData.X64.ExceptionTssDesc = mNewGdt + Gdtr.Limit + 1;+
EssData.X64.ExceptionTssDescSize = CPU_TSS_DESC_SIZE;+
EssData.X64.ExceptionTss = mNewGdt + Gdtr.Limit + 1 +
CPU_TSS_DESC_SIZE;+ EssData.X64.ExceptionTssSize = CPU_TSS_SIZE;++
InitData = &EssData; } - return Status;+ return ArchSetupExceptionStack
(InitData); }diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index 1ae611c75e..494c2ab433 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -150,57 +150,26 @@ InitializeCpuExceptionHandlers (
} /**- Initializes all CPU exceptions entries with optional extra
initializations.+ Setup separate stacks for certain exception handlers. - By
default, this method should include all functionalities implemented by-
InitializeCpuExceptionHandlers(), plus extra initialization works, if any.-
This could be done by calling InitializeCpuExceptionHandlers() directly- in
this method besides the extra works.+ InitData is optional and processor arch
dependent. - InitData is optional and its use and content are processor arch
dependent.- The typical usage of it is to convey resources which have to be
reserved- elsewhere and are necessary for the extra initializations of
exception.+ @param[in] InitData Pointer to data optional for information
about how+ to assign stacks for certain exception
handlers. - @param[in] VectorInfo Pointer to reserved vector list.-
@param[in] InitData Pointer to data optional for extra initializations-
of exception.-- @retval EFI_SUCCESS The
exceptions have been successfully-
initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains
invalid- content.+ @retval EFI_SUCCESS
The stacks are assigned successfully.+ @retval EFI_UNSUPPORTED
This function is not supported. **/ EFI_STATUS
EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO
*VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN
CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ) {- EFI_STATUS Status;-- //-
// To avoid repeat initialization of default handlers, the caller should pass-
// an extended init data with InitDefaultHandlers set to FALSE. There's no- //
need to call this method to just initialize default handlers. Call non-ex- //
version instead; or this method must be implemented as a simple wrapper of- //
non-ex version of it, if this version has to be called.- //- if ((InitData ==
NULL) || InitData->Ia32.InitDefaultHandlers) {- Status =
InitializeCpuExceptionHandlers (VectorInfo);- } else {- Status =
EFI_SUCCESS;- }-- if (!EFI_ERROR (Status)) {- //- // Initializing stack
switch is only necessary for Stack Guard functionality.- //- if
(PcdGetBool (PcdCpuStackGuard) && (InitData != NULL)) {- Status =
ArchSetupExceptionStack (InitData);- }+ if (InitData == NULL) {+ return
EFI_UNSUPPORTED; } - return Status;+ return ArchSetupExceptionStack
(InitData); }diff --git
a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
index e894ead612..4313cc5582 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
@@ -200,33 +200,22 @@ RegisterCpuInterruptHandler (
} /**- Initializes all CPU exceptions entries with optional extra
initializations.+ Setup separate stacks for certain exception handlers. - By
default, this method should include all functionalities implemented by-
InitializeCpuExceptionHandlers(), plus extra initialization works, if any.-
This could be done by calling InitializeCpuExceptionHandlers() directly- in
this method besides the extra works.+ InitData is optional and processor arch
dependent. - InitData is optional and its use and content are processor arch
dependent.- The typical usage of it is to convey resources which have to be
reserved- elsewhere and are necessary for the extra initializations of
exception.+ @param[in] InitData Pointer to data optional for information
about how+ to assign stacks for certain exception
handlers. - @param[in] VectorInfo Pointer to reserved vector list.-
@param[in] InitData Pointer to data optional for extra initializations-
of exception.-- @retval EFI_SUCCESS The
exceptions have been successfully-
initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains
invalid- content.+ @retval EFI_SUCCESS
The stacks are assigned successfully.+ @retval EFI_UNSUPPORTED
This function is not supported. **/ EFI_STATUS
EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO
*VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN
CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ) {- return
InitializeCpuExceptionHandlers (VectorInfo);+ return EFI_UNSUPPORTED; }diff
--git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
index ec643556c7..1c97dab926 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmException.c
@@ -96,33 +96,22 @@ RegisterCpuInterruptHandler (
} /**- Initializes all CPU exceptions entries with optional extra
initializations.+ Setup separate stacks for certain exception handlers. - By
default, this method should include all functionalities implemented by-
InitializeCpuExceptionHandlers(), plus extra initialization works, if any.-
This could be done by calling InitializeCpuExceptionHandlers() directly- in
this method besides the extra works.+ InitData is optional and processor arch
dependent. - InitData is optional and its use and content are processor arch
dependent.- The typical usage of it is to convey resources which have to be
reserved- elsewhere and are necessary for the extra initializations of
exception.+ @param[in] InitData Pointer to data optional for information
about how+ to assign stacks for certain exception
handlers. - @param[in] VectorInfo Pointer to reserved vector list.-
@param[in] InitData Pointer to data optional for extra initializations-
of exception.-- @retval EFI_SUCCESS The
exceptions have been successfully-
initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains
invalid- content.+ @retval EFI_SUCCESS
The stacks are assigned successfully.+ @retval EFI_UNSUPPORTED
This function is not supported. **/ EFI_STATUS
EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO
*VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN
CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ) {- return
InitializeCpuExceptionHandlers (VectorInfo);+ return EFI_UNSUPPORTED; }--
2.35.1.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90240): https://edk2.groups.io/g/devel/message/90240
Mute This Topic: https://groups.io/mt/91231771/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
