Re: [edk2-devel] [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks
Acked-by: Eric Dong -Original Message- From: Ni, Ray Sent: Friday, May 20, 2022 10:16 PM To: devel@edk2.groups.io Cc: Dong, Eric ; Wang, Jian J Subject: [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks Today InitializeCpuExceptionHandlersEx is called from three modules: 1. DxeCore (links to DxeCpuExceptionHandlerLib) DxeCore expects it initializes the IDT entries as well as assigning separate stacks for #DF and #PF. 2. CpuMpPei (links to PeiCpuExceptionHandlerLib) and CpuDxe (links to DxeCpuExceptionHandlerLib) It's called for each thread for only assigning separate stacks for #DF and #PF. The IDT entries initialization is skipped because caller sets InitData->X64.InitDefaultHandlers to FALSE. Additionally, SecPeiCpuExceptionHandlerLib, SmmCpuExceptionHandlerLib also implement such API and the behavior of the API is simply to initialize IDT entries only. Because it mixes the IDT entries initialization and separate stacks assignment for certain exception handlers together, in order to know whether the function call only initializes IDT entries, or assigns stacks, we need to check: 1. value of InitData->X64.InitDefaultHandlers 2. library instance This patch cleans up the code to separate the stack assignment to a new API: InitializeSeparateExceptionStacks(). Only when caller calls the new API, the separate stacks are assigned. With this change, the SecPei and Smm instance can return unsupported which gives caller a very clear status. The old API InitializeCpuExceptionHandlersEx() is removed in this patch. Because no platform module is consuming the old API, the impact is none. Signed-off-by: Ray Ni Cc: Eric Dong Cc: Jian J Wang --- MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 2 +- .../Include/Library/CpuExceptionHandlerLib.h | 24 ++--- .../CpuExceptionHandlerLibNull.c | 26 ++ UefiCpuPkg/CpuDxe/CpuMp.c | 6 +- UefiCpuPkg/CpuMpPei/CpuMpPei.c| 4 +- .../CpuExceptionHandlerLib/DxeException.c | 91 ++- .../CpuExceptionHandlerLib/PeiCpuException.c | 51 ++- .../SecPeiCpuException.c | 27 ++ .../CpuExceptionHandlerLib/SmmException.c | 27 ++ 9 files changed, 74 insertions(+), 184 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c index 2c27fc0695..83f49d7c00 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c @@ -253,7 +253,7 @@ DxeMain ( VectorInfoList = (EFI_VECTOR_HANDOFF_INFO *)(GET_GUID_HOB_DATA (GuidHob)); } - Status = InitializeCpuExceptionHandlersEx (VectorInfoList, NULL);+ Status = InitializeCpuExceptionHandlers (VectorInfoList); ASSERT_EFI_ERROR (Status);//diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h index d4649bebe1..9a495081f7 100644 --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h @@ -103,32 +103,20 @@ InitializeCpuExceptionHandlers ( ); /**- Initializes all CPU exceptions entries with optional extra initializations.+ Setup separate stacks for certain exception handlers. - By default, this method should include all functionalities implemented by- InitializeCpuExceptionHandlers(), plus extra initialization works, if any.- This could be done by calling InitializeCpuExceptionHandlers() directly- in this method besides the extra works.+ InitData is optional and processor arch dependent. - InitData is optional and its use and content are processor arch dependent.- The typical usage of it is to convey resources which have to be reserved- elsewhere and are necessary for the extra initializations of exception.+ @param[in] InitData Pointer to data optional for information about how+to assign stacks for certain exception handlers. - @param[in] VectorInfoPointer to reserved vector list.- @param[in] InitData Pointer to data optional for extra initializations- of exception.-- @retval EFI_SUCCESS The exceptions have been successfully- initialized.- @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains invalid- content.+ @retval EFI_SUCCESS The stacks are assigned successfully. @retval EFI_UNSUPPORTED This function is not supported. **/ EFI_STATUS EFIAPI-InitializeCpuExceptionHandlersEx (- IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL,+InitializeSeparateExceptionStacks ( IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ); diff --git a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c index 54f38788fe..8aeedcb4d1 100644 --- a/M
Re: [edk2-devel] [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks
Reviewed-by: Jian J Wang Regards, Jian > -Original Message- > From: Ni, Ray > Sent: Friday, May 20, 2022 10:16 PM > To: devel@edk2.groups.io > Cc: Dong, Eric ; Wang, Jian J > Subject: [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks > > Today InitializeCpuExceptionHandlersEx is called from three modules: > 1. DxeCore (links to DxeCpuExceptionHandlerLib) > DxeCore expects it initializes the IDT entries as well as > assigning separate stacks for #DF and #PF. > 2. CpuMpPei (links to PeiCpuExceptionHandlerLib) >and CpuDxe (links to DxeCpuExceptionHandlerLib) > It's called for each thread for only assigning separate stacks for > #DF and #PF. The IDT entries initialization is skipped because > caller sets InitData->X64.InitDefaultHandlers to FALSE. > > Additionally, SecPeiCpuExceptionHandlerLib, SmmCpuExceptionHandlerLib > also implement such API and the behavior of the API is simply to initialize > IDT entries only. > > Because it mixes the IDT entries initialization and separate stacks > assignment for certain exception handlers together, in order to know > whether the function call only initializes IDT entries, or assigns stacks, > we need to check: > 1. value of InitData->X64.InitDefaultHandlers > 2. library instance > > This patch cleans up the code to separate the stack assignment to a new API: > InitializeSeparateExceptionStacks(). > > Only when caller calls the new API, the separate stacks are assigned. > With this change, the SecPei and Smm instance can return unsupported which > gives caller a very clear status. > > The old API InitializeCpuExceptionHandlersEx() is removed in this patch. > Because no platform module is consuming the old API, the impact is none. > > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Jian J Wang > --- > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 2 +- > .../Include/Library/CpuExceptionHandlerLib.h | 24 ++--- > .../CpuExceptionHandlerLibNull.c | 26 ++ > UefiCpuPkg/CpuDxe/CpuMp.c | 6 +- > UefiCpuPkg/CpuMpPei/CpuMpPei.c| 4 +- > .../CpuExceptionHandlerLib/DxeException.c | 91 ++- > .../CpuExceptionHandlerLib/PeiCpuException.c | 51 ++- > .../SecPeiCpuException.c | 27 ++ > .../CpuExceptionHandlerLib/SmmException.c | 27 ++ > 9 files changed, 74 insertions(+), 184 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > index 2c27fc0695..83f49d7c00 100644 > --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > @@ -253,7 +253,7 @@ DxeMain ( > VectorInfoList = (EFI_VECTOR_HANDOFF_INFO *)(GET_GUID_HOB_DATA > (GuidHob)); > >} > > > > - Status = InitializeCpuExceptionHandlersEx (VectorInfoList, NULL); > > + Status = InitializeCpuExceptionHandlers (VectorInfoList); > >ASSERT_EFI_ERROR (Status); > > > >// > > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > index d4649bebe1..9a495081f7 100644 > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > @@ -103,32 +103,20 @@ InitializeCpuExceptionHandlers ( >); > > > > /** > > - Initializes all CPU exceptions entries with optional extra initializations. > > + Setup separate stacks for certain exception handlers. > > > > - By default, this method should include all functionalities implemented by > > - InitializeCpuExceptionHandlers(), plus extra initialization works, if any. > > - This could be done by calling InitializeCpuExceptionHandlers() directly > > - in this method besides the extra works. > > + InitData is optional and processor arch dependent. > > > > - InitData is optional and its use and content are processor arch dependent. > > - The typical usage of it is to convey resources which have to be reserved > > - elsewhere and are necessary for the extra initializations of exception. > > + @param[in] InitData Pointer to data optional for information about > how > > +to assign stacks for certain exception handlers. > > > > - @param[in] VectorInfoPointer to reserved vector list. > > - @param[in] InitData Pointer to data optional for extra > initializations > > -of exception. > > - > > - @retval EFI_SUCCESS The exceptions have been successfully > > - initialized. > > - @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains invalid > > - content. > > + @retval EFI_SUCCESS The stacks are assigned successfully. > >@retval EFI_UNSUPPORTED This function is not supported. > > > > **/ > > EFI_STATUS > > EFIAPI > > -InitializeCpuExceptionHandlers
[edk2-devel] [PATCH 5/5] CpuException: Add InitializeSeparateExceptionStacks
Today InitializeCpuExceptionHandlersEx is called from three modules: 1. DxeCore (links to DxeCpuExceptionHandlerLib) DxeCore expects it initializes the IDT entries as well as assigning separate stacks for #DF and #PF. 2. CpuMpPei (links to PeiCpuExceptionHandlerLib) and CpuDxe (links to DxeCpuExceptionHandlerLib) It's called for each thread for only assigning separate stacks for #DF and #PF. The IDT entries initialization is skipped because caller sets InitData->X64.InitDefaultHandlers to FALSE. Additionally, SecPeiCpuExceptionHandlerLib, SmmCpuExceptionHandlerLib also implement such API and the behavior of the API is simply to initialize IDT entries only. Because it mixes the IDT entries initialization and separate stacks assignment for certain exception handlers together, in order to know whether the function call only initializes IDT entries, or assigns stacks, we need to check: 1. value of InitData->X64.InitDefaultHandlers 2. library instance This patch cleans up the code to separate the stack assignment to a new API: InitializeSeparateExceptionStacks(). Only when caller calls the new API, the separate stacks are assigned. With this change, the SecPei and Smm instance can return unsupported which gives caller a very clear status. The old API InitializeCpuExceptionHandlersEx() is removed in this patch. Because no platform module is consuming the old API, the impact is none. Signed-off-by: Ray Ni Cc: Eric Dong Cc: Jian J Wang --- MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 2 +- .../Include/Library/CpuExceptionHandlerLib.h | 24 ++--- .../CpuExceptionHandlerLibNull.c | 26 ++ UefiCpuPkg/CpuDxe/CpuMp.c | 6 +- UefiCpuPkg/CpuMpPei/CpuMpPei.c| 4 +- .../CpuExceptionHandlerLib/DxeException.c | 91 ++- .../CpuExceptionHandlerLib/PeiCpuException.c | 51 ++- .../SecPeiCpuException.c | 27 ++ .../CpuExceptionHandlerLib/SmmException.c | 27 ++ 9 files changed, 74 insertions(+), 184 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c index 2c27fc0695..83f49d7c00 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c @@ -253,7 +253,7 @@ DxeMain ( VectorInfoList = (EFI_VECTOR_HANDOFF_INFO *)(GET_GUID_HOB_DATA (GuidHob)); } - Status = InitializeCpuExceptionHandlersEx (VectorInfoList, NULL); + Status = InitializeCpuExceptionHandlers (VectorInfoList); ASSERT_EFI_ERROR (Status); // diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h index d4649bebe1..9a495081f7 100644 --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h @@ -103,32 +103,20 @@ InitializeCpuExceptionHandlers ( ); /** - Initializes all CPU exceptions entries with optional extra initializations. + Setup separate stacks for certain exception handlers. - By default, this method should include all functionalities implemented by - InitializeCpuExceptionHandlers(), plus extra initialization works, if any. - This could be done by calling InitializeCpuExceptionHandlers() directly - in this method besides the extra works. + InitData is optional and processor arch dependent. - InitData is optional and its use and content are processor arch dependent. - The typical usage of it is to convey resources which have to be reserved - elsewhere and are necessary for the extra initializations of exception. + @param[in] InitData Pointer to data optional for information about how +to assign stacks for certain exception handlers. - @param[in] VectorInfoPointer to reserved vector list. - @param[in] InitData Pointer to data optional for extra initializations -of exception. - - @retval EFI_SUCCESS The exceptions have been successfully - initialized. - @retval EFI_INVALID_PARAMETER VectorInfo or InitData contains invalid - content. + @retval EFI_SUCCESS The stacks are assigned successfully. @retval EFI_UNSUPPORTED This function is not supported. **/ EFI_STATUS EFIAPI -InitializeCpuExceptionHandlersEx ( - IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL, +InitializeSeparateExceptionStacks ( IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ); diff --git a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c index 54f38788fe..8aeedcb4d1 100644 --- a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c +++ b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c @@ -82,34 +82,22 @@ DumpCpuContext ( } /** - Initi
