Re: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers
Acked-by: Eric Dong eric.d...@intel.com<mailto:eric.d...@intel.com> From: Ni, Ray Sent: Tuesday, May 24, 2022 4:04 PM To: Wang, Jian J ; devel@edk2.groups.io Cc: Dong, Eric Subject: Re: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers Jian, I think we need discussion on where to put the common CPU_INTERRUPT_NUM definition. Do you agree that we can leave that to another patch? From: Wang, Jian J mailto:jian.j.w...@intel.com>> Sent: Monday, May 23, 2022 0:27 To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray Cc: Dong, Eric Subject: RE: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers Hi Ray, Both CpuDxe.c and CpuExceptionCommon.h have CPU_INTERRUPT_NUM defined. I'd suggest to move it to a common place, such as BaseLib.h. I don't see any issue if they are defined to different value. It just gives me a feeling that it might cause potential problems sometimes in the future. Regards, Jian > -Original Message- > From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray > Sent: Friday, May 20, 2022 10:16 PM > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> > Cc: Dong, Eric mailto:eric.d...@intel.com>> > Subject: [edk2-devel] [PATCH 4/5] CpuException: Remove > InitializeCpuInterruptHandlers > > InitializeCpuExceptionHandlers() expects caller allocates IDT while > InitializeCpuInterruptHandlers() allocates 256 IDT entries itself. > > InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated > by caller. If caller allocates 10 entries, the API just fills 10 IDT > entries. > > The inconsistency between the two APIs makes code hard to > unerstand and hard to share. > > Because there is only one caller (CpuDxe) for > InitializeCpuInterruptHandler(), this patch updates CpuDxe driver > to allocates 256 IDT entries then call > InitializeCpuExceptionHandlers(). > > With this change, InitializeCpuInterruptHandlers() is removed > completely. > > And InitializeCpuExceptionHandlers() fills max 32 entries for PEI > and SMM instance, max 256 entries for DXE instance. > Such behavior matches to the original one. > > Signed-off-by: Ray Ni mailto:ray...@intel.com>> > Cc: Eric Dong mailto:eric.d...@intel.com>> > --- > .../Include/Library/CpuExceptionHandlerLib.h | 28 +-- > .../CpuExceptionHandlerLibNull.c | 31 +-- > UefiCpuPkg/CpuDxe/CpuDxe.c| 33 ++-- > .../CpuExceptionHandlerLib/DxeException.c | 80 ++- > .../CpuExceptionHandlerLib/PeiCpuException.c | 61 +- > .../PeiDxeSmmCpuException.c | 19 ++--- > .../SecPeiCpuException.c | 31 +-- > .../CpuExceptionHandlerLib/SmmException.c | 35 ++-- > 8 files changed, 56 insertions(+), 262 deletions(-) > > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > index 22a4408f9f..d4649bebe1 100644 > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > @@ -2,7 +2,7 @@ >CPU Exception library provides the default CPU interrupt/exception handler. > >It also provides capability to register user interrupt/exception handler. > > > > - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. > > + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. > >SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -132,28 +132,6 @@ InitializeCpuExceptionHandlersEx ( >IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL > >); > > > > -/** > > - Initializes all CPU interrupt/exceptions entries and provides the default > interrupt/exception handlers. > > - > > - Caller should try to get an array of interrupt and/or exception vectors > that are > in use and need to > > - persist by EFI_VECTOR_HANDOFF_INFO defined in PI 1.3 specification. > > - If caller cannot get reserved vector list or it does not exists, set > VectorInfo to > NULL. > > - If VectorInfo is not NULL, the exception vectors will be initialized per > vector > attribute accordingly. > > - > > - @param[in] VectorInfoPointer to reserved vector list. > > - > > - @retval EFI_SUCCESS All CPU interrupt/exception entries have been > successfully initialized > > -with default interrupt/exception handlers. > > - @retval EFI_INVALID_PARAMETER VectorInfo includes the invalid content if > VectorI
Re: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers
Another patch works for me. Reviewed-by: Jian J Wang Regards, Jian > -Original Message- > From: Ni, Ray > Sent: Tuesday, May 24, 2022 4:04 PM > To: Wang, Jian J ; devel@edk2.groups.io > Cc: Dong, Eric > Subject: Re: [edk2-devel] [PATCH 4/5] CpuException: Remove > InitializeCpuInterruptHandlers > > Jian, > I think we need discussion on where to put the common CPU_INTERRUPT_NUM > definition. > > Do you agree that we can leave that to another patch? > > > From: Wang, Jian J > Sent: Monday, May 23, 2022 0:27 > To: devel@edk2.groups.io; Ni, Ray > Cc: Dong, Eric > Subject: RE: [edk2-devel] [PATCH 4/5] CpuException: Remove > InitializeCpuInterruptHandlers > > Hi Ray, > > Both CpuDxe.c and CpuExceptionCommon.h have CPU_INTERRUPT_NUM > defined. > I'd suggest to move it to a common place, such as BaseLib.h. I don't see any > issue > if they are defined to different value. It just gives me a feeling that it > might > cause > potential problems sometimes in the future. > > > Regards, > Jian > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of Ni, Ray > > Sent: Friday, May 20, 2022 10:16 PM > > To: devel@edk2.groups.io > > Cc: Dong, Eric > > Subject: [edk2-devel] [PATCH 4/5] CpuException: Remove > > InitializeCpuInterruptHandlers > > > > InitializeCpuExceptionHandlers() expects caller allocates IDT while > > InitializeCpuInterruptHandlers() allocates 256 IDT entries itself. > > > > InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated > > by caller. If caller allocates 10 entries, the API just fills 10 IDT > > entries. > > > > The inconsistency between the two APIs makes code hard to > > unerstand and hard to share. > > > > Because there is only one caller (CpuDxe) for > > InitializeCpuInterruptHandler(), this patch updates CpuDxe driver > > to allocates 256 IDT entries then call > > InitializeCpuExceptionHandlers(). > > > > With this change, InitializeCpuInterruptHandlers() is removed > > completely. > > > > And InitializeCpuExceptionHandlers() fills max 32 entries for PEI > > and SMM instance, max 256 entries for DXE instance. > > Such behavior matches to the original one. > > > > Signed-off-by: Ray Ni > > Cc: Eric Dong > > --- > > .../Include/Library/CpuExceptionHandlerLib.h | 28 +-- > > .../CpuExceptionHandlerLibNull.c | 31 +-- > > UefiCpuPkg/CpuDxe/CpuDxe.c| 33 ++-- > > .../CpuExceptionHandlerLib/DxeException.c | 80 ++- > > .../CpuExceptionHandlerLib/PeiCpuException.c | 61 +- > > .../PeiDxeSmmCpuException.c | 19 ++--- > > .../SecPeiCpuException.c | 31 +-- > > .../CpuExceptionHandlerLib/SmmException.c | 35 ++-- > > 8 files changed, 56 insertions(+), 262 deletions(-) > > > > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > > index 22a4408f9f..d4649bebe1 100644 > > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > > @@ -2,7 +2,7 @@ > >CPU Exception library provides the default CPU interrupt/exception > > handler. > > > >It also provides capability to register user interrupt/exception handler. > > > > > > > > - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. > > > > + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. > > > >SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > **/ > > > > @@ -132,28 +132,6 @@ InitializeCpuExceptionHandlersEx ( > >IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL > > > >); > > > > > > > > -/** > > > > - Initializes all CPU interrupt/exceptions entries and provides the default > > interrupt/exception handlers. > > > > - > > > > - Caller should try to get an array of interrupt and/or exception vectors > > that > are > > in use and need to > > > > - persist by EFI_VECTOR_HANDOFF_INFO defined in PI 1.3 specification. > > > > - If caller cannot get reserved vector list or it does not exists, set > > VectorInfo > to > > NULL. > > > > - If VectorInfo is not NULL, the exception vectors will be initialized per >
Re: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers
Jian, I think we need discussion on where to put the common CPU_INTERRUPT_NUM definition. Do you agree that we can leave that to another patch? From: Wang, Jian J Sent: Monday, May 23, 2022 0:27 To: devel@edk2.groups.io; Ni, Ray Cc: Dong, Eric Subject: RE: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers Hi Ray, Both CpuDxe.c and CpuExceptionCommon.h have CPU_INTERRUPT_NUM defined. I'd suggest to move it to a common place, such as BaseLib.h. I don't see any issue if they are defined to different value. It just gives me a feeling that it might cause potential problems sometimes in the future. Regards, Jian > -Original Message- > From: devel@edk2.groups.io On Behalf Of Ni, Ray > Sent: Friday, May 20, 2022 10:16 PM > To: devel@edk2.groups.io > Cc: Dong, Eric > Subject: [edk2-devel] [PATCH 4/5] CpuException: Remove > InitializeCpuInterruptHandlers > > InitializeCpuExceptionHandlers() expects caller allocates IDT while > InitializeCpuInterruptHandlers() allocates 256 IDT entries itself. > > InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated > by caller. If caller allocates 10 entries, the API just fills 10 IDT > entries. > > The inconsistency between the two APIs makes code hard to > unerstand and hard to share. > > Because there is only one caller (CpuDxe) for > InitializeCpuInterruptHandler(), this patch updates CpuDxe driver > to allocates 256 IDT entries then call > InitializeCpuExceptionHandlers(). > > With this change, InitializeCpuInterruptHandlers() is removed > completely. > > And InitializeCpuExceptionHandlers() fills max 32 entries for PEI > and SMM instance, max 256 entries for DXE instance. > Such behavior matches to the original one. > > Signed-off-by: Ray Ni > Cc: Eric Dong > --- > .../Include/Library/CpuExceptionHandlerLib.h | 28 +-- > .../CpuExceptionHandlerLibNull.c | 31 +-- > UefiCpuPkg/CpuDxe/CpuDxe.c| 33 ++-- > .../CpuExceptionHandlerLib/DxeException.c | 80 ++- > .../CpuExceptionHandlerLib/PeiCpuException.c | 61 +- > .../PeiDxeSmmCpuException.c | 19 ++--- > .../SecPeiCpuException.c | 31 +-- > .../CpuExceptionHandlerLib/SmmException.c | 35 ++-- > 8 files changed, 56 insertions(+), 262 deletions(-) > > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > index 22a4408f9f..d4649bebe1 100644 > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > @@ -2,7 +2,7 @@ >CPU Exception library provides the default CPU interrupt/exception handler. > >It also provides capability to register user interrupt/exception handler. > > > > - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. > > + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. > >SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -132,28 +132,6 @@ InitializeCpuExceptionHandlersEx ( >IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL > >); > > > > -/** > > - Initializes all CPU interrupt/exceptions entries and provides the default > interrupt/exception handlers. > > - > > - Caller should try to get an array of interrupt and/or exception vectors > that are > in use and need to > > - persist by EFI_VECTOR_HANDOFF_INFO defined in PI 1.3 specification. > > - If caller cannot get reserved vector list or it does not exists, set > VectorInfo to > NULL. > > - If VectorInfo is not NULL, the exception vectors will be initialized per > vector > attribute accordingly. > > - > > - @param[in] VectorInfoPointer to reserved vector list. > > - > > - @retval EFI_SUCCESS All CPU interrupt/exception entries have been > successfully initialized > > -with default interrupt/exception handlers. > > - @retval EFI_INVALID_PARAMETER VectorInfo includes the invalid content if > VectorInfo is not NULL. > > - @retval EFI_UNSUPPORTED This function is not supported. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -InitializeCpuInterruptHandlers ( > > - IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL > > - ); > > - > > /** > >Registers a function to be called from the processor interrupt handler. > > > > @@ -161,8 +139,8 @@ InitializeCpuInterruptHandlers ( >interrupt or exception type specified by InterruptType. If > InterruptHandler is > N
Re: [edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers
Hi Ray, Both CpuDxe.c and CpuExceptionCommon.h have CPU_INTERRUPT_NUM defined. I'd suggest to move it to a common place, such as BaseLib.h. I don't see any issue if they are defined to different value. It just gives me a feeling that it might cause potential problems sometimes in the future. Regards, Jian > -Original Message- > From: devel@edk2.groups.io On Behalf Of Ni, Ray > Sent: Friday, May 20, 2022 10:16 PM > To: devel@edk2.groups.io > Cc: Dong, Eric > Subject: [edk2-devel] [PATCH 4/5] CpuException: Remove > InitializeCpuInterruptHandlers > > InitializeCpuExceptionHandlers() expects caller allocates IDT while > InitializeCpuInterruptHandlers() allocates 256 IDT entries itself. > > InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated > by caller. If caller allocates 10 entries, the API just fills 10 IDT > entries. > > The inconsistency between the two APIs makes code hard to > unerstand and hard to share. > > Because there is only one caller (CpuDxe) for > InitializeCpuInterruptHandler(), this patch updates CpuDxe driver > to allocates 256 IDT entries then call > InitializeCpuExceptionHandlers(). > > With this change, InitializeCpuInterruptHandlers() is removed > completely. > > And InitializeCpuExceptionHandlers() fills max 32 entries for PEI > and SMM instance, max 256 entries for DXE instance. > Such behavior matches to the original one. > > Signed-off-by: Ray Ni > Cc: Eric Dong > --- > .../Include/Library/CpuExceptionHandlerLib.h | 28 +-- > .../CpuExceptionHandlerLibNull.c | 31 +-- > UefiCpuPkg/CpuDxe/CpuDxe.c| 33 ++-- > .../CpuExceptionHandlerLib/DxeException.c | 80 ++- > .../CpuExceptionHandlerLib/PeiCpuException.c | 61 +- > .../PeiDxeSmmCpuException.c | 19 ++--- > .../SecPeiCpuException.c | 31 +-- > .../CpuExceptionHandlerLib/SmmException.c | 35 ++-- > 8 files changed, 56 insertions(+), 262 deletions(-) > > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > index 22a4408f9f..d4649bebe1 100644 > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h > @@ -2,7 +2,7 @@ >CPU Exception library provides the default CPU interrupt/exception handler. > >It also provides capability to register user interrupt/exception handler. > > > > - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. > > + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. > >SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > @@ -132,28 +132,6 @@ InitializeCpuExceptionHandlersEx ( >IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL > >); > > > > -/** > > - Initializes all CPU interrupt/exceptions entries and provides the default > interrupt/exception handlers. > > - > > - Caller should try to get an array of interrupt and/or exception vectors > that are > in use and need to > > - persist by EFI_VECTOR_HANDOFF_INFO defined in PI 1.3 specification. > > - If caller cannot get reserved vector list or it does not exists, set > VectorInfo to > NULL. > > - If VectorInfo is not NULL, the exception vectors will be initialized per > vector > attribute accordingly. > > - > > - @param[in] VectorInfoPointer to reserved vector list. > > - > > - @retval EFI_SUCCESS All CPU interrupt/exception entries have been > successfully initialized > > -with default interrupt/exception handlers. > > - @retval EFI_INVALID_PARAMETER VectorInfo includes the invalid content if > VectorInfo is not NULL. > > - @retval EFI_UNSUPPORTED This function is not supported. > > - > > -**/ > > -EFI_STATUS > > -EFIAPI > > -InitializeCpuInterruptHandlers ( > > - IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL > > - ); > > - > > /** > >Registers a function to be called from the processor interrupt handler. > > > > @@ -161,8 +139,8 @@ InitializeCpuInterruptHandlers ( >interrupt or exception type specified by InterruptType. If > InterruptHandler is > NULL, then the > >handler for the processor interrupt or exception type specified by > InterruptType is uninstalled. > >The installed handler is called once for each processor interrupt or > exception. > > - NOTE: This function should be invoked after > InitializeCpuExceptionHandlers() > or &
[edk2-devel] [PATCH 4/5] CpuException: Remove InitializeCpuInterruptHandlers
InitializeCpuExceptionHandlers() expects caller allocates IDT while InitializeCpuInterruptHandlers() allocates 256 IDT entries itself. InitializeCpuExceptionHandlers() fills max 32 IDT entries allocated by caller. If caller allocates 10 entries, the API just fills 10 IDT entries. The inconsistency between the two APIs makes code hard to unerstand and hard to share. Because there is only one caller (CpuDxe) for InitializeCpuInterruptHandler(), this patch updates CpuDxe driver to allocates 256 IDT entries then call InitializeCpuExceptionHandlers(). With this change, InitializeCpuInterruptHandlers() is removed completely. And InitializeCpuExceptionHandlers() fills max 32 entries for PEI and SMM instance, max 256 entries for DXE instance. Such behavior matches to the original one. Signed-off-by: Ray Ni Cc: Eric Dong --- .../Include/Library/CpuExceptionHandlerLib.h | 28 +-- .../CpuExceptionHandlerLibNull.c | 31 +-- UefiCpuPkg/CpuDxe/CpuDxe.c| 33 ++-- .../CpuExceptionHandlerLib/DxeException.c | 80 ++- .../CpuExceptionHandlerLib/PeiCpuException.c | 61 +- .../PeiDxeSmmCpuException.c | 19 ++--- .../SecPeiCpuException.c | 31 +-- .../CpuExceptionHandlerLib/SmmException.c | 35 ++-- 8 files changed, 56 insertions(+), 262 deletions(-) diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h index 22a4408f9f..d4649bebe1 100644 --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h @@ -2,7 +2,7 @@ CPU Exception library provides the default CPU interrupt/exception handler. It also provides capability to register user interrupt/exception handler. - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -132,28 +132,6 @@ InitializeCpuExceptionHandlersEx ( IN CPU_EXCEPTION_INIT_DATA *InitData OPTIONAL ); -/** - Initializes all CPU interrupt/exceptions entries and provides the default interrupt/exception handlers. - - Caller should try to get an array of interrupt and/or exception vectors that are in use and need to - persist by EFI_VECTOR_HANDOFF_INFO defined in PI 1.3 specification. - If caller cannot get reserved vector list or it does not exists, set VectorInfo to NULL. - If VectorInfo is not NULL, the exception vectors will be initialized per vector attribute accordingly. - - @param[in] VectorInfoPointer to reserved vector list. - - @retval EFI_SUCCESS All CPU interrupt/exception entries have been successfully initialized -with default interrupt/exception handlers. - @retval EFI_INVALID_PARAMETER VectorInfo includes the invalid content if VectorInfo is not NULL. - @retval EFI_UNSUPPORTED This function is not supported. - -**/ -EFI_STATUS -EFIAPI -InitializeCpuInterruptHandlers ( - IN EFI_VECTOR_HANDOFF_INFO *VectorInfo OPTIONAL - ); - /** Registers a function to be called from the processor interrupt handler. @@ -161,8 +139,8 @@ InitializeCpuInterruptHandlers ( interrupt or exception type specified by InterruptType. If InterruptHandler is NULL, then the handler for the processor interrupt or exception type specified by InterruptType is uninstalled. The installed handler is called once for each processor interrupt or exception. - NOTE: This function should be invoked after InitializeCpuExceptionHandlers() or - InitializeCpuInterruptHandlers() invoked, otherwise EFI_UNSUPPORTED returned. + NOTE: This function should be invoked after InitializeCpuExceptionHandlers() is invoked, + otherwise EFI_UNSUPPORTED returned. @param[in] InterruptType Defines which interrupt or exception to hook. @param[in] InterruptHandler A pointer to a function of type EFI_CPU_INTERRUPT_HANDLER that is called diff --git a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c index 35ab5a8db5..54f38788fe 100644 --- a/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c +++ b/MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.c @@ -1,7 +1,7 @@ /** @file CPU Exception Handler library implementition with empty functions. - Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. + Copyright (c) 2012 - 2022, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -33,31 +33,6 @@ InitializeCpuExceptionHandlers ( return EFI_SUCCESS; } -/** - Initializes all CPU interrupt/exceptions entries and provides the default interrupt/exception handlers. - - Caller should try to get an array of interrupt and/or
