* [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers @ 2022-08-26 7:04 Zhiguang Liu 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Zhiguang Liu ` (2 more replies) 0 siblings, 3 replies; 6+ messages in thread From: Zhiguang Liu @ 2022-08-26 7:04 UTC (permalink / raw) To: devel; +Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar 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 | 104 ++++++++++++++++++++------------ UefiCpuPkg/CpuMpPei/CpuMpPei.c | 107 ++++++++++++++++++++------------- 2 files changed, 131 insertions(+), 80 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c index f3ca813d2a..e7575d9b80 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,18 @@ InitializeExceptionStackSwitchHandlers ( ) { EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData; + UINTN Index; + MpInitLibWhoAmI (&Index); SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer; - InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize); + + // + // This may be called twice for each Cpu. Only run InitializeSeparateExceptionStacks + // if this is the first call or the first call failed because of size too small. + // + 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 +648,69 @@ InitializeMpExceptionStackSwitchHandlers ( ) { UINTN Index; - UINTN Bsp; - EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData; + EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData; UINTN BufferSize; + EFI_STATUS Status; + UINT8 *Buffer; - SwitchStackData.BufferSize = &BufferSize; - MpInitLibWhoAmI (&Bsp); - + SwitchStackData = AllocateZeroPool (mNumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)); + ASSERT (SwitchStackData != NULL); 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; + } - if (Index == Bsp) { - InitializeExceptionStackSwitchHandlers (&SwitchStackData); + Status = MpInitLibStartupAllCPUs ( + InitializeExceptionStackSwitchHandlers, + 0, + SwitchStackData + ); + ASSERT_EFI_ERROR (Status); + + BufferSize = 0; + for (Index = 0; Index < mNumberOfProcessors; ++Index) { + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) { + ASSERT (SwitchStackData[Index].BufferSize != 0); + 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); + ASSERT (SwitchStackData[Index].BufferSize == 0); } + } - if (BufferSize == 0) { - continue; + if (BufferSize != 0) { + Buffer = AllocateRuntimeZeroPool (BufferSize); + ASSERT (Buffer != NULL); + BufferSize = 0; + for (Index = 0; Index < mNumberOfProcessors; ++Index) { + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) { + SwitchStackData[Index].Buffer = (VOID *)(&Buffer[BufferSize]); + 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); + for (Index = 0; Index < mNumberOfProcessors; ++Index) { + ASSERT (SwitchStackData[Index].Status == EFI_SUCCESS); } } + + FreePool (SwitchStackData); } /** diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c index c0be11d3ad..f2eb582e9b 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,18 @@ InitializeExceptionStackSwitchHandlers ( ) { EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData; + UINTN Index; + MpInitLibWhoAmI (&Index); SwitchStackData = (EXCEPTION_STACK_SWITCH_CONTEXT *)Buffer; - InitializeSeparateExceptionStacks (SwitchStackData->Buffer, SwitchStackData->BufferSize); + + // + // This function may be called twice for each Cpu. Only run InitializeSeparateExceptionStacks + // if this is the first call or the first call failed because of size too small. + // + 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 +463,75 @@ InitializeMpExceptionStackSwitchHandlers ( ) { UINTN Index; - UINTN Bsp; - EXCEPTION_STACK_SWITCH_CONTEXT SwitchStackData; - UINTN BufferSize; UINTN NumberOfProcessors; + EXCEPTION_STACK_SWITCH_CONTEXT *SwitchStackData; + UINTN BufferSize; + EFI_STATUS Status; + UINT8 *Buffer; if (!PcdGetBool (PcdCpuStackGuard)) { return; } - SwitchStackData.BufferSize = &BufferSize; MpInitLibGetNumberOfProcessors (&NumberOfProcessors, NULL); - MpInitLibWhoAmI (&Bsp); - + SwitchStackData = AllocateZeroPool (NumberOfProcessors * sizeof (EXCEPTION_STACK_SWITCH_CONTEXT)); + ASSERT (SwitchStackData != NULL); 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; + } + + Status = MpInitLibStartupAllCPUs ( + InitializeExceptionStackSwitchHandlers, + 0, + SwitchStackData + ); + ASSERT_EFI_ERROR (Status); - if (Index == Bsp) { - InitializeExceptionStackSwitchHandlers (&SwitchStackData); + BufferSize = 0; + for (Index = 0; Index < NumberOfProcessors; ++Index) { + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) { + ASSERT (SwitchStackData[Index].BufferSize != 0); + 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); + ASSERT (SwitchStackData[Index].BufferSize == 0); } + } - if (BufferSize == 0) { - continue; + if (BufferSize != 0) { + Buffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize)); + ASSERT (Buffer != NULL); + BufferSize = 0; + for (Index = 0; Index < NumberOfProcessors; ++Index) { + if (SwitchStackData[Index].Status == EFI_BUFFER_TOO_SMALL) { + SwitchStackData[Index].Buffer = (VOID *)(&Buffer[BufferSize]); + 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); + for (Index = 0; Index < NumberOfProcessors; ++Index) { + ASSERT (SwitchStackData[Index].Status == EFI_SUCCESS); } } + + FreePool (SwitchStackData); } /** -- 2.31.1.windows.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp 2022-08-26 7:04 [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Zhiguang Liu @ 2022-08-26 7:04 ` Zhiguang Liu 2022-08-26 7:27 ` Ni, Ray 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks Zhiguang Liu 2022-08-26 7:21 ` [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Ni, Ray 2 siblings, 1 reply; 6+ messages in thread From: Zhiguang Liu @ 2022-08-26 7:04 UTC (permalink / raw) To: devel; +Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar 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 | 20 +-------- UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++- UefiCpuPkg/Library/MpInitLib/MpLib.h | 43 +++++++++---------- UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 21 --------- 4 files changed, 56 insertions(+), 63 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm index 28301bb8f0..1d67f510e9 100644 --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm @@ -284,15 +284,8 @@ ASM_PFX(AsmExchangeRole): ; edi contains OthersInfo pointer mov edi, [ebp + 28h] - ;Store EFLAGS, GDTR and IDTR register to stack + ;Store EFLAGS 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Zhiguang Liu @ 2022-08-26 7:27 ` Ni, Ray 0 siblings, 0 replies; 6+ messages in thread From: Ni, Ray @ 2022-08-26 7:27 UTC (permalink / raw) To: Liu, Zhiguang, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Liu, Zhiguang <zhiguang.liu@intel.com> > Sent: Friday, August 26, 2022 3:05 PM > 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 v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp > > 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 | 20 +-------- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 35 ++++++++++++++- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 43 +++++++++---------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 21 --------- > 4 files changed, 56 insertions(+), 63 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 28301bb8f0..1d67f510e9 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > @@ -284,15 +284,8 @@ ASM_PFX(AsmExchangeRole): > ; edi contains OthersInfo pointer > mov edi, [ebp + 28h] > > - ;Store EFLAGS, GDTR and IDTR register to stack > + ;Store EFLAGS 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks 2022-08-26 7:04 [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Zhiguang Liu 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Zhiguang Liu @ 2022-08-26 7:04 ` Zhiguang Liu 2022-08-26 7:28 ` Ni, Ray 2022-08-26 7:21 ` [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Ni, Ray 2 siblings, 1 reply; 6+ messages in thread From: Zhiguang Liu @ 2022-08-26 7:04 UTC (permalink / raw) To: devel; +Cc: Zhiguang Liu, Eric Dong, Ray Ni, Rahul Kumar 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 | 92 ++--------- .../Ia32/ArchExceptionHandler.c | 148 ++++++++---------- .../CpuExceptionHandlerLib/PeiCpuException.c | 77 +-------- .../SecPeiCpuExceptionHandlerLib.inf | 7 +- .../SmmCpuExceptionHandlerLib.inf | 7 +- .../X64/ArchExceptionHandler.c | 145 ++++++++--------- .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 7 +- 8 files changed, 173 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..ee989bf079 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,16 @@ 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); + 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); + LocalBufferSize = sizeof (mBuffer); + Status = ArchSetupExceptionStack (mBuffer, &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..80e9f08e5b 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,75 @@ 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 | + // | | + // -------------------------------- + // | | + // | 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 +223,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 +254,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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks Zhiguang Liu @ 2022-08-26 7:28 ` Ni, Ray 0 siblings, 0 replies; 6+ messages in thread From: Ni, Ray @ 2022-08-26 7:28 UTC (permalink / raw) To: Liu, Zhiguang, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Liu, Zhiguang <zhiguang.liu@intel.com> > Sent: Friday, August 26, 2022 3:05 PM > 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 v2] UefiCpuPkg: Simplify the implementation when > separate exception stacks > > 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 | 92 ++--------- > .../Ia32/ArchExceptionHandler.c | 148 ++++++++---------- > .../CpuExceptionHandlerLib/PeiCpuException.c | 77 +-------- > .../SecPeiCpuExceptionHandlerLib.inf | 7 +- > .../SmmCpuExceptionHandlerLib.inf | 7 +- > .../X64/ArchExceptionHandler.c | 145 ++++++++--------- > .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 7 +- > 8 files changed, 173 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..ee989bf079 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,16 @@ 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); > + 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); > + LocalBufferSize = sizeof (mBuffer); > + Status = ArchSetupExceptionStack (mBuffer, &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/SecPeiCpuExceptionHandler > Lib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler > Lib.inf > index 8ae4feae62..6a170286c8 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler > Lib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler > Lib.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/SmmCpuExceptionHandlerLi > b.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi > b.inf > index c9f20da058..9dde07612a 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi > b.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi > b.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..80e9f08e5b 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,75 @@ 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 | > + // | | > + // -------------------------------- > + // | | > + // | 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 +223,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 +254,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/Xcode5SecPeiCpuException > HandlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException > HandlerLib.inf > index a15f125d5b..6d2f66504a 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException > HandlerLib.inf > +++ > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException > HandlerLib.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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers 2022-08-26 7:04 [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Zhiguang Liu 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Zhiguang Liu 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks Zhiguang Liu @ 2022-08-26 7:21 ` Ni, Ray 2 siblings, 0 replies; 6+ messages in thread From: Ni, Ray @ 2022-08-26 7:21 UTC (permalink / raw) To: Liu, Zhiguang, devel@edk2.groups.io; +Cc: Dong, Eric, Kumar, Rahul R > + SwitchStackData = AllocateZeroPool (NumberOfProcessors * sizeof > (EXCEPTION_STACK_SWITCH_CONTEXT)); Please use AllocatePage() ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-08-26 7:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-08-26 7:04 [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Zhiguang Liu 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg/MpInitLib: Simplify logic in SwitchBsp Zhiguang Liu 2022-08-26 7:27 ` Ni, Ray 2022-08-26 7:04 ` [PATCH v2] UefiCpuPkg: Simplify the implementation when separate exception stacks Zhiguang Liu 2022-08-26 7:28 ` Ni, Ray 2022-08-26 7:21 ` [PATCH v2] UefiCpuPkg: Enhance logic in InitializeMpExceptionStackSwitchHandlers Ni, Ray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox