Re: [edk2] [PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib: Avoid calling PEI services from AP
Validated-by: Jian J Wang
Reviewed-by: Jian J Wang
> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, August 31, 2018 4:36 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Fan Jeff
> Subject: [PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib: Avoid calling PEI
> services from AP
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1128
>
> When an exception happens in AP, system hangs at
> GetPeiServicesTablePointer(), complaining the PeiServices retrieved
> from memory before IDT is NULL.
>
> Due to the following commit:
> c563077a380437c114aba4c95be65eb963ebc1f3
> * UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
> the IDT used by AP no longer preserve PeiServices pointer in the
> very beginning.
> But the implementation of PeiExceptionHandlerLib still assumes
> the PeiServices pointer is there, so the assertion happens.
>
> The patch fixes the exception handler library to not call
> PEI services from AP.
>
> The patch duplicates the #0 exception stub header in an allocated
> pool but with extra 4-byte/8-byte to store the exception handler
> data which was originally stored in HOB.
> When AP exception happens, the code gets the exception handler data
> from the exception handler for #0.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni
> Cc: Jian J Wang
> Cc: Fan Jeff
> ---
> .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 7 +-
> .../CpuExceptionHandlerLib/PeiCpuException.c | 77
> +++---
> 2 files changed, 55 insertions(+), 29 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> index e10d9379d5..459f06ac84 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> @@ -1,7 +1,7 @@
> /** @file
>Common header file for CPU Exception Handler Library.
>
> - Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.
> + Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution. The full text of the license may be
> found
> at
> @@ -43,11 +43,6 @@
>
> #include "ArchInterruptDefs.h"
>
> -#define CPU_EXCEPTION_HANDLER_LIB_HOB_GUID \
> - { \
> -0xb21d9148, 0x9211, 0x4d8f, { 0xad, 0xd3, 0x66, 0xb1, 0x89, 0xc9, 0x2c,
> 0x83 } \
> - }
> -
> #define CPU_STACK_SWITCH_EXCEPTION_NUMBER \
>FixedPcdGetSize (PcdCpuStackSwitchExceptionList)
>
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
> index 6f271983f2..5dd8423d2f 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
> @@ -1,7 +1,7 @@
> /** @file
>CPU exception handler library implementation for PEIM module.
>
> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials are licensed and made available
> under
> the terms and conditions of the BSD License that accompanies this
> distribution.
> The full text of the license may be found at
> @@ -20,10 +20,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>
> CONST UINTNmDoFarReturnFlag = 0;
>
> -EFI_GUID mCpuExceptrionHandlerLibHobGuid =
> CPU_EXCEPTION_HANDLER_LIB_HOB_GUID;
> +typedef struct {
> + UINT8 ExceptionStubHeader[HOOKAFTER_STUB_SIZE];
> + EXCEPTION_HANDLER_DATA *ExceptionHandlerData;
> +} EXCEPTION0_STUB_HEADER;
>
> /**
> - Get exception handler data pointer from GUIDed HOb.
> + Get exception handler data pointer from IDT[0].
> +
> + The exception #0 stub header is duplicated in an allocated pool with extra
> 4-
> byte/8-byte to store the
> + exception handler data. The new allocated memory layout follows structure
> EXCEPTION0_STUB_HEADER.
> + The code assumes that all processors uses the same exception handler for #0
> exception.
>
>@return pointer to exception handler data.
> **/
> @@ -32,18 +39,50 @@ GetExceptionHandlerData (
>VOID
>)
> {
> - EFI_HOB_GUID_TYPE *GuidHob;
> - VOID*DataInHob;
> - EXCEPTION_HANDLER_DATA *ExceptionHandlerData;
> + IA32_DESCRIPTOR IdtDescriptor;
> + IA32_IDT_GATE_DESCRIPTOR *IdtTable;
> + EXCEPTION0_STUB_HEADER *Exception0StubHeader;
> +
> + AsmReadIdtr ();
> + IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)IdtDescriptor.Base;
> +
> + Exception0StubHeader = (EXCEPTION0_STUB_HEADER *)ArchGetIdtHandler
> ([0]);
> + return Exception0StubHeader->ExceptionHandlerData;
> +}
>
> - ExceptionHandlerData =
[edk2] [PATCH 2/2] UefiCpuPkg/CpuExceptionHandlerLib: Avoid calling PEI services from AP
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1128
When an exception happens in AP, system hangs at
GetPeiServicesTablePointer(), complaining the PeiServices retrieved
from memory before IDT is NULL.
Due to the following commit:
c563077a380437c114aba4c95be65eb963ebc1f3
* UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP
the IDT used by AP no longer preserve PeiServices pointer in the
very beginning.
But the implementation of PeiExceptionHandlerLib still assumes
the PeiServices pointer is there, so the assertion happens.
The patch fixes the exception handler library to not call
PEI services from AP.
The patch duplicates the #0 exception stub header in an allocated
pool but with extra 4-byte/8-byte to store the exception handler
data which was originally stored in HOB.
When AP exception happens, the code gets the exception handler data
from the exception handler for #0.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni
Cc: Jian J Wang
Cc: Fan Jeff
---
.../CpuExceptionHandlerLib/CpuExceptionCommon.h| 7 +-
.../CpuExceptionHandlerLib/PeiCpuException.c | 77 +++---
2 files changed, 55 insertions(+), 29 deletions(-)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
index e10d9379d5..459f06ac84 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
@@ -1,7 +1,7 @@
/** @file
Common header file for CPU Exception Handler Library.
- Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.
+ Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD
License
which accompanies this distribution. The full text of the license may be
found at
@@ -43,11 +43,6 @@
#include "ArchInterruptDefs.h"
-#define CPU_EXCEPTION_HANDLER_LIB_HOB_GUID \
- { \
-0xb21d9148, 0x9211, 0x4d8f, { 0xad, 0xd3, 0x66, 0xb1, 0x89, 0xc9, 0x2c,
0x83 } \
- }
-
#define CPU_STACK_SWITCH_EXCEPTION_NUMBER \
FixedPcdGetSize (PcdCpuStackSwitchExceptionList)
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
index 6f271983f2..5dd8423d2f 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c
@@ -1,7 +1,7 @@
/** @file
CPU exception handler library implementation for PEIM module.
-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
This program and the accompanying materials are licensed and made available
under
the terms and conditions of the BSD License that accompanies this distribution.
The full text of the license may be found at
@@ -20,10 +20,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
CONST UINTNmDoFarReturnFlag = 0;
-EFI_GUID mCpuExceptrionHandlerLibHobGuid = CPU_EXCEPTION_HANDLER_LIB_HOB_GUID;
+typedef struct {
+ UINT8 ExceptionStubHeader[HOOKAFTER_STUB_SIZE];
+ EXCEPTION_HANDLER_DATA *ExceptionHandlerData;
+} EXCEPTION0_STUB_HEADER;
/**
- Get exception handler data pointer from GUIDed HOb.
+ Get exception handler data pointer from IDT[0].
+
+ The exception #0 stub header is duplicated in an allocated pool with extra
4-byte/8-byte to store the
+ exception handler data. The new allocated memory layout follows structure
EXCEPTION0_STUB_HEADER.
+ The code assumes that all processors uses the same exception handler for #0
exception.
@return pointer to exception handler data.
**/
@@ -32,18 +39,50 @@ GetExceptionHandlerData (
VOID
)
{
- EFI_HOB_GUID_TYPE *GuidHob;
- VOID*DataInHob;
- EXCEPTION_HANDLER_DATA *ExceptionHandlerData;
+ IA32_DESCRIPTOR IdtDescriptor;
+ IA32_IDT_GATE_DESCRIPTOR *IdtTable;
+ EXCEPTION0_STUB_HEADER *Exception0StubHeader;
+
+ AsmReadIdtr ();
+ IdtTable = (IA32_IDT_GATE_DESCRIPTOR *)IdtDescriptor.Base;
+
+ Exception0StubHeader = (EXCEPTION0_STUB_HEADER *)ArchGetIdtHandler
([0]);
+ return Exception0StubHeader->ExceptionHandlerData;
+}
- ExceptionHandlerData = NULL;
- GuidHob = GetFirstGuidHob ();
- if (GuidHob != NULL) {
-DataInHob = GET_GUID_HOB_DATA (GuidHob);
-ExceptionHandlerData = (EXCEPTION_HANDLER_DATA *)(*(UINTN *)DataInHob);
- }
- ASSERT (ExceptionHandlerData != NULL);
- return ExceptionHandlerData;
+/**
+ Set exception handler data pointer to IDT[0].
+
+ The exception #0 stub header is duplicated in an allocated pool with extra
4-byte/8-byte to store the
+ exception handler data. The new allocated memory layout follows structure
EXCEPTION0_STUB_HEADER.
+
