Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael > D mailto:michael.d.kin...@intel.com>> > Cc: Dong, Eric mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > Mike > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan > Jeff > Sent: Tuesday, October 31, 2017 7:33 PM > To: Yao, Jiewen mailto:jiewen@intel.com>>; Wang, > Jian J mailto:jian.j.w...@intel.com>>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D > mailto:michael.d.kin...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > stack switch support > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > chosen. > > For long term, I agree we need to move AsmWriteTr, > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric > > mailto:eric.d...@intel.com>>; Yao, Jiewen > > mailto:jiewen@intel.com>>; Kinney, > > Michael D mailto:michael.d.kin...@intel.com>> > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch support > > > > If Stack Guard is enabled and there's really a stack overflow happened > > during boot, a Page Fault exception will be triggered. Because the > > stack is out of usage, the exception handler, which shares the stack > > with normal UEFI driver, cannot be executed and cannot dump the processor > information. > > > > Without those information, it's very difficult for the BIOS developers > > locate the root cause of stack overflow. And without a workable stack, > > the developer cannot event use single step to debug the UEFI driver with > > JTAG > debugger. > > > > In order to make sure the exception handler to execute normally after > > stack overflow. We need separate stacks for exception handlers in case > > of unusable stack. > > > > IA processor allows to switch to a new stack during handling interrupt > > and exception. But X64 and IA32 provides different ways to make it. > > X64 provides interrupt stack table (IST) to allow maximum 7 different > > exceptions to have new stack for its handler. IA32 doesn't have IST > > mechanism and can only use task gate to do it since task switch allows > > to load a new stack through its task-state segment (TSS). > > > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > > functional
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
dk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D > mailto:michael.d.kin...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > stack switch support > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > chosen. > > For long term, I agree we need to move AsmWriteTr, > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric > > mailto:eric.d...@intel.com>>; Yao, Jiewen > > mailto:jiewen@intel.com>>; Kinney, > > Michael D mailto:michael.d.kin...@intel.com>> > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch support > > > > If Stack Guard is enabled and there's really a stack overflow happened > > during boot, a Page Fault exception will be triggered. Because the > > stack is out of usage, the exception handler, which shares the stack > > with normal UEFI driver, cannot be executed and cannot dump the processor > information. > > > > Without those information, it's very difficult for the BIOS developers > > locate the root cause of stack overflow. And without a workable stack, > > the developer cannot event use single step to debug the UEFI driver with > > JTAG > debugger. > > > > In order to make sure the exception handler to execute normally after > > stack overflow. We need separate stacks for exception handlers in case > > of unusable stack. > > > > IA processor allows to switch to a new stack during handling interrupt > > and exception. But X64 and IA32 provides different ways to make it. > > X64 provides interrupt stack table (IST) to allow maximum 7 different > > exceptions to have new stack for its handler. IA32 doesn't have IST > > mechanism and can only use task gate to do it since task switch allows > > to load a new stack through its task-state segment (TSS). > > > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > > functionality works only in the boot phases capable of memory > > allocation (besides the paging, for the sake of Stack Guard). In > > other words, only DXE phase can supports Stack Guard with stack > switch. > > > > Cc: Star Zeng mailto:star.z...@intel.com>> > > Cc: Eric Dong mailto:eric.d...@intel.com>> > > Cc: Jiewen Yao mailto:jiewen@intel.com>> > > Cc: Michael Kinney > > mailto:michael.d.kin...@intel.com>> > > Suggested-by: Ayellet Wolman > > mailto:ayellet.wol...@intel.com>> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > mailto:jian.j.w...@intel.com>> > > --- > &g
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
as BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric > > mailto:eric.d...@intel.com>>; Yao, Jiewen > > mailto:jiewen@intel.com>>; Kinney, > > Michael D mailto:michael.d.kin...@intel.com>> > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch support > > > > If Stack Guard is enabled and there's really a stack overflow happened > > during boot, a Page Fault exception will be triggered. Because the > > stack is out of usage, the exception handler, which shares the stack > > with normal UEFI driver, cannot be executed and cannot dump the processor > information. > > > > Without those information, it's very difficult for the BIOS developers > > locate the root cause of stack overflow. And without a workable stack, > > the developer cannot event use single step to debug the UEFI driver with > > JTAG > debugger. > > > > In order to make sure the exception handler to execute normally after > > stack overflow. We need separate stacks for exception handlers in case > > of unusable stack. > > > > IA processor allows to switch to a new stack during handling interrupt > > and exception. But X64 and IA32 provides different ways to make it. > > X64 provides interrupt stack table (IST) to allow maximum 7 different > > exceptions to have new stack for its handler. IA32 doesn't have IST > > mechanism and can only use task gate to do it since task switch allows > > to load a new stack through its task-state segment (TSS). > > > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > > functionality works only in the boot phases capable of memory > > allocation (besides the paging, for the sake of Stack Guard). In > > other words, only DXE phase can supports Stack Guard with stack > switch. > > > > Cc: Star Zeng mailto:star.z...@intel.com>> > > Cc: Eric Dong mailto:eric.d...@intel.com>> > > Cc: Jiewen Yao mailto:jiewen@intel.com>> > > Cc: Michael Kinney > > mailto:michael.d.kin...@intel.com>> > > Suggested-by: Ayellet Wolman > > mailto:ayellet.wol...@intel.com>> > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > mailto:jian.j.w...@intel.com>> > > --- > > .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ > > .../DxeCpuExceptionHandlerLib.inf | 5 + > > .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + > > .../Ia32/ArchExceptionHandler.c| 135 +++ > > .../Ia32/ArchInterruptDefs.h | 136 +++ > > .../Ia32/ExceptionTssEntryAsm.nasm | 398 > > + > > .../PeiCpuExceptionHandlerLib.inf | 1 + > > .../SecPeiCpuExceptionHandlerLib.inf | 3 + > > .../SmmCpuExceptionHandlerLib.inf |
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Jeff, That’s a good suggestion. Do you know if there’s any pitfall in using data segment memory as stack? Thanks, Jian From: Fan Jeff [mailto:vanjeff_...@hotmail.com] Sent: Friday, November 03, 2017 4:59 PM To: Yao, Jiewen ; Kinney, Michael D ; Wang, Jian J ; edk2-devel@lists.01.org Cc: Dong, Eric ; Zeng, Star Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Jian, For example, you could use the global variable UINT8 NewStack[256] instead of allocate memory for stack and try to use the other global variables for other allocated data. Does it work for your case? Thanks! Jeff From: edk2-devel mailto:edk2-devel-boun...@lists.01.org>> on behalf of Fan Jeff mailto:vanjeff_...@hotmail.com>> Sent: Friday, November 3, 2017 4:21:22 PM To: Yao, Jiewen; Kinney, Michael D; Wang, Jian J; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Dong, Eric; Zeng, Star Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Jian, Could you use some global variables in https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c to avoid adding new API? Jeff 发件人: Yao, Jiewen<mailto:jiewen@intel.com> 发送时间: 2017年11月3日 9:24 收件人: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Fan Jeff<mailto:vanjeff_...@hotmail.com>; Wang, Jian J<mailto:jian.j.w...@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> 抄送: Dong, Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> 主题: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support HI Jian There is another potential problem. This feature is enabled in InitializeCpuInterruptHandlers(). However, we expect this is enabled in InitializeCpuExceptionHandlers(). In order to get the exception stack in InitializeCpuExceptionHandlers(). We can have 2 ways: A) Let InitializeCpuExceptionHandlers() allocate the exception stack. B) Let caller to pass the exception stack to InitializeCpuExceptionHandlers(). For A), it is hard, because DxeCore calls InitializeCpuExceptionHandlers() earlier than CoreInitializeMemoryServices(). For B), InitializeCpuExceptionHandlers() interface does not contain such information. We might need add a new API to add the exception stack information, such as InitializeCpuSwitchStackExceptionHandlers (). Once this feature is done, can we clean up the SMM code to use the new API, instead of duplicate the SMM copy of exception handler ? The existing SMM copy has some assumption on CR4 and FXSAVE/RESTORE, and it broke Quark platform. Thank you Yao Jiewen > -Original Message- > From: Kinney, Michael D > Sent: Wednesday, November 1, 2017 11:43 PM > To: Fan Jeff mailto:vanjeff_...@hotmail.com>>; Yao, > Jiewen mailto:jiewen@intel.com>>; > Wang, Jian J mailto:jian.j.w...@intel.com>>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Kinney, Michael > D mailto:michael.d.kin...@intel.com>> > Cc: Dong, Eric mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > Mike > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan > Jeff > Sent: Tuesday, October 31, 2017 7:33 PM > To: Yao, Jiewen mailto:jiewen@intel.com>>; Wang, > Jian J mailto:jian.j.w...@intel.com>>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D > mailto:michael.d.kin...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > stack switch support > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > chosen. > > For long term, I agree we need to move AsmWriteTr, > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng,
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
I see. Thanks for the explanation. > -Original Message- > From: Yao, Jiewen > Sent: Friday, November 03, 2017 10:27 AM > To: Wang, Jian J ; Kinney, Michael D > ; Fan Jeff ; edk2- > de...@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > Jian > That is for compatibility consideration. > > Once there is UDK release, we never change API. We can only add new one. > Or you may break an old platform, that may use new UDK release. > > Thank you > Yao Jiewen > > > -Original Message- > > From: Wang, Jian J > > Sent: Friday, November 3, 2017 10:10 AM > > To: Yao, Jiewen ; Kinney, Michael D > > ; Fan Jeff ; > > edk2-devel@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch > > support > > > > Hi Jiewen, > > > > > -Original Message- > > > From: Yao, Jiewen > > > Sent: Friday, November 03, 2017 9:25 AM > > > To: Kinney, Michael D ; Fan Jeff > > > ; Wang, Jian J ; edk2- > > > de...@lists.01.org > > > Cc: Dong, Eric ; Zeng, Star > > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch > > > support > > > > > > HI Jian > > > There is another potential problem. > > > > > > This feature is enabled in InitializeCpuInterruptHandlers(). However, we > expect > > > this is enabled in InitializeCpuExceptionHandlers(). > > > > > > In order to get the exception stack in InitializeCpuExceptionHandlers(). > > > We > can > > > have 2 ways: > > > A) Let InitializeCpuExceptionHandlers() allocate the exception stack. > > > B) Let caller to pass the exception stack to > > > InitializeCpuExceptionHandlers(). > > > > > > For A), it is hard, because DxeCore calls InitializeCpuExceptionHandlers() > earlier > > > than CoreInitializeMemoryServices(). > > > For B), InitializeCpuExceptionHandlers() interface does not contain such > > > information. We might need add a new API to add the exception stack > > > information, such as InitializeCpuSwitchStackExceptionHandlers (). > > > > You're right. Enabling stack switch in InitializeCpuExceptionHandlers() is > > a bit > > late for exception handling. But I don't see the difference between adding > > parameters to existing API (InitializeCpuExceptionHandlers) and adding a new > > API (InitializeCpuSwitchStackExceptionHandlers) in this case. If we have to > > change library interface anyway, I'd suggest to add parameters to existing > > API to carry information needed to setup new stack for exceptions. > > > > > > > > Once this feature is done, can we clean up the SMM code to use the new > > > API, > > > instead of duplicate the SMM copy of exception handler ? The existing SMM > > > copy has some assumption on CR4 and FXSAVE/RESTORE, and it broke Quark > > > platform. > > > > I agree. > > > > > > > > Thank you > > > Yao Jiewen > > > > > > > > > > > > > -Original Message- > > > > From: Kinney, Michael D > > > > Sent: Wednesday, November 1, 2017 11:43 PM > > > > To: Fan Jeff ; Yao, Jiewen > > > ; > > > > Wang, Jian J ; edk2-devel@lists.01.org; Kinney, > > > Michael > > > > D > > > > Cc: Dong, Eric ; Zeng, Star > > > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > > switch > > > > support > > > > > > > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > > > > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > > > > > > > Mike > > > > > > > > -Original Message- > > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Fan > > > > Jeff > > > > Sent: Tuesday, October 31, 2017 7:33 PM > > > > To: Yao, Jiewen ; Wang, Jian J > > > ; > > > > edk2-devel@lists.01.org > > > > Cc: Kinney, Michael D ; Dong, Eric > > > > ; Zeng, Star > > > > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: > Add > > > > stack switch support > > > > > > > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be > setup > > &
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Jian That is for compatibility consideration. Once there is UDK release, we never change API. We can only add new one. Or you may break an old platform, that may use new UDK release. Thank you Yao Jiewen > -Original Message- > From: Wang, Jian J > Sent: Friday, November 3, 2017 10:10 AM > To: Yao, Jiewen ; Kinney, Michael D > ; Fan Jeff ; > edk2-devel@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > Hi Jiewen, > > > -Original Message- > > From: Yao, Jiewen > > Sent: Friday, November 03, 2017 9:25 AM > > To: Kinney, Michael D ; Fan Jeff > > ; Wang, Jian J ; edk2- > > de...@lists.01.org > > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch > > support > > > > HI Jian > > There is another potential problem. > > > > This feature is enabled in InitializeCpuInterruptHandlers(). However, we > > expect > > this is enabled in InitializeCpuExceptionHandlers(). > > > > In order to get the exception stack in InitializeCpuExceptionHandlers(). We > > can > > have 2 ways: > > A) Let InitializeCpuExceptionHandlers() allocate the exception stack. > > B) Let caller to pass the exception stack to > > InitializeCpuExceptionHandlers(). > > > > For A), it is hard, because DxeCore calls InitializeCpuExceptionHandlers() > > earlier > > than CoreInitializeMemoryServices(). > > For B), InitializeCpuExceptionHandlers() interface does not contain such > > information. We might need add a new API to add the exception stack > > information, such as InitializeCpuSwitchStackExceptionHandlers (). > > You're right. Enabling stack switch in InitializeCpuExceptionHandlers() is a > bit > late for exception handling. But I don't see the difference between adding > parameters to existing API (InitializeCpuExceptionHandlers) and adding a new > API (InitializeCpuSwitchStackExceptionHandlers) in this case. If we have to > change library interface anyway, I'd suggest to add parameters to existing > API to carry information needed to setup new stack for exceptions. > > > > > Once this feature is done, can we clean up the SMM code to use the new API, > > instead of duplicate the SMM copy of exception handler ? The existing SMM > > copy has some assumption on CR4 and FXSAVE/RESTORE, and it broke Quark > > platform. > > I agree. > > > > > Thank you > > Yao Jiewen > > > > > > > > > -Original Message- > > > From: Kinney, Michael D > > > Sent: Wednesday, November 1, 2017 11:43 PM > > > To: Fan Jeff ; Yao, Jiewen > > ; > > > Wang, Jian J ; edk2-devel@lists.01.org; Kinney, > > Michael > > > D > > > Cc: Dong, Eric ; Zeng, Star > > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch > > > support > > > > > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > > > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > > > > > Mike > > > > > > -Original Message- > > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan > > > Jeff > > > Sent: Tuesday, October 31, 2017 7:33 PM > > > To: Yao, Jiewen ; Wang, Jian J > > ; > > > edk2-devel@lists.01.org > > > Cc: Kinney, Michael D ; Dong, Eric > > > ; Zeng, Star > > > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > > > stack switch support > > > > > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > > > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > > > chosen. > > > > > > For long term, I agree we need to move AsmWriteTr, > > > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as > > BaseLib) > > > For this patch, I have no strong opinion. > > > > > > > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > > > 发送时间: 2017年11月1日 9:56 > > > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > > > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > > > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > stack >
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Hi Jiewen, > -Original Message- > From: Yao, Jiewen > Sent: Friday, November 03, 2017 9:25 AM > To: Kinney, Michael D ; Fan Jeff > ; Wang, Jian J ; edk2- > de...@lists.01.org > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > HI Jian > There is another potential problem. > > This feature is enabled in InitializeCpuInterruptHandlers(). However, we > expect > this is enabled in InitializeCpuExceptionHandlers(). > > In order to get the exception stack in InitializeCpuExceptionHandlers(). We > can > have 2 ways: > A) Let InitializeCpuExceptionHandlers() allocate the exception stack. > B) Let caller to pass the exception stack to InitializeCpuExceptionHandlers(). > > For A), it is hard, because DxeCore calls InitializeCpuExceptionHandlers() > earlier > than CoreInitializeMemoryServices(). > For B), InitializeCpuExceptionHandlers() interface does not contain such > information. We might need add a new API to add the exception stack > information, such as InitializeCpuSwitchStackExceptionHandlers (). You're right. Enabling stack switch in InitializeCpuExceptionHandlers() is a bit late for exception handling. But I don't see the difference between adding parameters to existing API (InitializeCpuExceptionHandlers) and adding a new API (InitializeCpuSwitchStackExceptionHandlers) in this case. If we have to change library interface anyway, I'd suggest to add parameters to existing API to carry information needed to setup new stack for exceptions. > > Once this feature is done, can we clean up the SMM code to use the new API, > instead of duplicate the SMM copy of exception handler ? The existing SMM > copy has some assumption on CR4 and FXSAVE/RESTORE, and it broke Quark > platform. I agree. > > Thank you > Yao Jiewen > > > > > -Original Message- > > From: Kinney, Michael D > > Sent: Wednesday, November 1, 2017 11:43 PM > > To: Fan Jeff ; Yao, Jiewen > ; > > Wang, Jian J ; edk2-devel@lists.01.org; Kinney, > Michael > > D > > Cc: Dong, Eric ; Zeng, Star > > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch > > support > > > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > > > Mike > > > > -Original Message- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan > > Jeff > > Sent: Tuesday, October 31, 2017 7:33 PM > > To: Yao, Jiewen ; Wang, Jian J > ; > > edk2-devel@lists.01.org > > Cc: Kinney, Michael D ; Dong, Eric > > ; Zeng, Star > > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > > stack switch support > > > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > > chosen. > > > > For long term, I agree we need to move AsmWriteTr, > > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as > BaseLib) > > For this patch, I have no strong opinion. > > > > > > 发件人: Yao, Jiewen<mailto:jiewen....@intel.com> > > 发送时间: 2017年11月1日 9:56 > > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch support > > > > Hi Jian > > Thanks for the patch. > > > > Can we move all IA32 defined data structure or function to MdePkg? > > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > > > I am also curious why we use different policy for other boot mode. > > Can we use consistent policy? > > > + if (PcdGetBool (PcdCpuStackGuard)) { > > > +// > > > +// Stack Guard works with the support of page table established and > > > +// memory management. So we have to exclude those boot modes > > > without > > > +// them. > > > +// > > > +switch (GetBootModeHob()) { > > > +case BOOT_ON_FLASH_UPDATE: > > > +case BOOT_IN_RECOVERY_MODE: > > > +case BOOT_ON_S3_RESUME: > > > + break; > > > + > > > +default: > > > + ArchSetupExcpetionStack (IdtTable); > > > + break; > > > +} > >
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
HI Jian There is another potential problem. This feature is enabled in InitializeCpuInterruptHandlers(). However, we expect this is enabled in InitializeCpuExceptionHandlers(). In order to get the exception stack in InitializeCpuExceptionHandlers(). We can have 2 ways: A) Let InitializeCpuExceptionHandlers() allocate the exception stack. B) Let caller to pass the exception stack to InitializeCpuExceptionHandlers(). For A), it is hard, because DxeCore calls InitializeCpuExceptionHandlers() earlier than CoreInitializeMemoryServices(). For B), InitializeCpuExceptionHandlers() interface does not contain such information. We might need add a new API to add the exception stack information, such as InitializeCpuSwitchStackExceptionHandlers (). Once this feature is done, can we clean up the SMM code to use the new API, instead of duplicate the SMM copy of exception handler ? The existing SMM copy has some assumption on CR4 and FXSAVE/RESTORE, and it broke Quark platform. Thank you Yao Jiewen > -Original Message- > From: Kinney, Michael D > Sent: Wednesday, November 1, 2017 11:43 PM > To: Fan Jeff ; Yao, Jiewen ; > Wang, Jian J ; edk2-devel@lists.01.org; Kinney, Michael > D > Cc: Dong, Eric ; Zeng, Star > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, > IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. > > Mike > > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan > Jeff > Sent: Tuesday, October 31, 2017 7:33 PM > To: Yao, Jiewen ; Wang, Jian J ; > edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Dong, Eric > ; Zeng, Star > Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add > stack switch support > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > chosen. > > For long term, I agree we need to move AsmWriteTr, > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org > > Cc: Zeng, Star ; Dong, Eric > > ; Yao, Jiewen ; Kinney, > > Michael D > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > > switch support > > > > If Stack Guard is enabled and there's really a stack overflow happened > > during boot, a Page Fault exception will be triggered. Because the > > stack is out of usage, the exception handler, which shares the stack > > with normal UEFI driver, cannot be executed and cannot dump the processor > information. > > > > Without those information, it's very difficult for the BIOS developers > > locate the root cause of stack overflow. And without a workable stack, > > the developer cannot event use single step to debug the UEFI driver with > > JTAG > debugger. > > > > In order to make sure the exception handler to execute normally after > > stack overflow. We need separate stacks for exception handlers in case > > of unusable stack. > > > > IA processor allows to switch to a new stack duri
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
I agree that adding AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg BaseLib is a good idea. Mike -Original Message- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fan Jeff Sent: Tuesday, October 31, 2017 7:33 PM To: Yao, Jiewen ; Wang, Jian J ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Dong, Eric ; Zeng, Star Subject: [edk2] 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is chosen. For long term, I agree we need to move AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) For this patch, I have no strong opinion. 发件人: Yao, Jiewen<mailto:jiewen@intel.com> 发送时间: 2017年11月1日 9:56 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Hi Jian Thanks for the patch. Can we move all IA32 defined data structure or function to MdePkg? Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR I am also curious why we use different policy for other boot mode. Can we use consistent policy? > + if (PcdGetBool (PcdCpuStackGuard)) { > +// > +// Stack Guard works with the support of page table established and > +// memory management. So we have to exclude those boot modes > without > +// them. > +// > +switch (GetBootModeHob()) { > +case BOOT_ON_FLASH_UPDATE: > +case BOOT_IN_RECOVERY_MODE: > +case BOOT_ON_S3_RESUME: > + break; > + > +default: > + ArchSetupExcpetionStack (IdtTable); > + break; > +} > + } Thank you Yao Jiewen > -Original Message- > From: Wang, Jian J > Sent: Tuesday, October 31, 2017 10:24 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric > ; Yao, Jiewen ; Kinney, > Michael D > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > If Stack Guard is enabled and there's really a stack overflow happened > during boot, a Page Fault exception will be triggered. Because the > stack is out of usage, the exception handler, which shares the stack > with normal UEFI driver, cannot be executed and cannot dump the processor > information. > > Without those information, it's very difficult for the BIOS developers > locate the root cause of stack overflow. And without a workable stack, > the developer cannot event use single step to debug the UEFI driver with JTAG > debugger. > > In order to make sure the exception handler to execute normally after > stack overflow. We need separate stacks for exception handlers in case > of unusable stack. > > IA processor allows to switch to a new stack during handling interrupt > and exception. But X64 and IA32 provides different ways to make it. > X64 provides interrupt stack table (IST) to allow maximum 7 different > exceptions to have new stack for its handler. IA32 doesn't have IST > mechanism and can only use task gate to do it since task switch allows > to load a new stack through its task-state segment (TSS). > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > functionality works only in the boot phases capable of memory > allocation (besides the paging, for the sake of Stack Guard). In > other words, only DXE phase can supports Stack Guard with stack switch. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Jiewen Yao > Cc: Michael Kinney > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ > .../DxeCpuExceptionHandlerLib.inf | 5 + > .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + > .../Ia32/ArchExceptionHandler.c| 135 +++ > .../Ia32/ArchInterruptDefs.h | 136 +++ > .../Ia32/ExceptionTssEntryAsm.nasm | 398 > + > .../PeiCpuExceptionHandlerLib.inf | 1 + > .../SecPeiCpuExceptionHandlerLib.inf | 3 + > .../SmmCpuExceptionHandlerLib.inf | 1 + > .../X64/ArchExceptionHandler.c | 108 ++ > .../CpuExceptionHandlerLib/X64/ArchInterruptDefs.h | 40 +++ > .../X64/ExceptionHandlerAsm.S | 12 + > .../X64/Excepti
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Jeff, Sorry for the misunderstanding. I think stack switch cannot work with NULL exception handler. But without exception handler, what do we need stack switch for? Thanks, Jian > -Original Message- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, > Jian J > Sent: Wednesday, November 01, 2017 11:09 AM > To: Fan Jeff ; Yao, Jiewen ; > edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Dong, Eric > ; Zeng, Star > Subject: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Jeff, > > Stack guard will still work. But the developer cannot know what’s going on > without exception dumping message when stack overflow occurs. > > Thanks, > Jian > > From: Fan Jeff [mailto:vanjeff_...@hotmail.com] > Sent: Wednesday, November 01, 2017 10:59 AM > To: Wang, Jian J ; Yao, Jiewen ; > edk2-devel@lists.01.org > Cc: Kinney, Michael D ; Dong, Eric > ; Zeng, Star > Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Jian, > > No. I suggest to fix #109 in another separate patch. > > But to fix #109, we have to makes sure AsmWriteTr() and some definitions are > in > MdePkg. > But I have no strong opinion to add them into MdePkg in this patch. > > I have another question: If NULL CPU exception handler instance is chosen, > could Stack Switch work if PCD PcdCpuStackGuard is set to TRUE. > > Thanks! > Jeff > > 发件人: Wang, Jian J<mailto:jian.j.w...@intel.com> > 发送时间: 2017年11月1日 10:48 > 收件人: Fan Jeff<mailto:vanjeff_...@hotmail.com>; Yao, > Jiewen<mailto:jiewen@intel.com>; edk2-devel@lists.01.org<mailto:edk2- > de...@lists.01.org> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > Hi Jeff, > > Thanks for the feedback. Are you suggesting to fix Bugzilla 109 in this patch? > > Thanks, > Jian > > From: Fan Jeff [mailto:vanjeff_...@hotmail.com] > Sent: Wednesday, November 01, 2017 10:33 AM > To: Yao, Jiewen mailto:jiewen@intel.com>>; > Wang, Jian J mailto:jian.j.w...@intel.com>>; edk2- > de...@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Kinney, Michael D > mailto:michael.d.kin...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Zeng, Star > mailto:star.z...@intel.com>> > Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup > (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is > chosen. > > For long term, I agree we need to move AsmWriteTr, > IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as > BaseLib) > For this patch, I have no strong opinion. > > > 发件人: Yao, Jiewen<mailto:jiewen....@intel.com> > 发送时间: 2017年11月1日 9:56 > 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; edk2- > de...@lists.01.org<mailto:edk2-devel@lists.01.org> > 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, > Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> > 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack > switch support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, > Eric mailto:eric.d...@intel.com>>; Yao, > > Jiewen mailto:jiewen@intel.com>>;
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Jeff, Stack guard will still work. But the developer cannot know what’s going on without exception dumping message when stack overflow occurs. Thanks, Jian From: Fan Jeff [mailto:vanjeff_...@hotmail.com] Sent: Wednesday, November 01, 2017 10:59 AM To: Wang, Jian J ; Yao, Jiewen ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Dong, Eric ; Zeng, Star Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Jian, No. I suggest to fix #109 in another separate patch. But to fix #109, we have to makes sure AsmWriteTr() and some definitions are in MdePkg. But I have no strong opinion to add them into MdePkg in this patch. I have another question: If NULL CPU exception handler instance is chosen, could Stack Switch work if PCD PcdCpuStackGuard is set to TRUE. Thanks! Jeff 发件人: Wang, Jian J<mailto:jian.j.w...@intel.com> 发送时间: 2017年11月1日 10:48 收件人: Fan Jeff<mailto:vanjeff_...@hotmail.com>; Yao, Jiewen<mailto:jiewen@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> 主题: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Hi Jeff, Thanks for the feedback. Are you suggesting to fix Bugzilla 109 in this patch? Thanks, Jian From: Fan Jeff [mailto:vanjeff_...@hotmail.com] Sent: Wednesday, November 01, 2017 10:33 AM To: Yao, Jiewen mailto:jiewen@intel.com>>; Wang, Jian J mailto:jian.j.w...@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: Kinney, Michael D mailto:michael.d.kin...@intel.com>>; Dong, Eric mailto:eric.d...@intel.com>>; Zeng, Star mailto:star.z...@intel.com>> Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is chosen. For long term, I agree we need to move AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) For this patch, I have no strong opinion. 发件人: Yao, Jiewen<mailto:jiewen@intel.com> 发送时间: 2017年11月1日 9:56 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Hi Jian Thanks for the patch. Can we move all IA32 defined data structure or function to MdePkg? Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR I am also curious why we use different policy for other boot mode. Can we use consistent policy? > + if (PcdGetBool (PcdCpuStackGuard)) { > +// > +// Stack Guard works with the support of page table established and > +// memory management. So we have to exclude those boot modes > without > +// them. > +// > +switch (GetBootModeHob()) { > +case BOOT_ON_FLASH_UPDATE: > +case BOOT_IN_RECOVERY_MODE: > +case BOOT_ON_S3_RESUME: > + break; > + > +default: > + ArchSetupExcpetionStack (IdtTable); > + break; > +} > + } Thank you Yao Jiewen > -Original Message- > From: Wang, Jian J > Sent: Tuesday, October 31, 2017 10:24 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Yao, > Jiewen mailto:jiewen@intel.com>>; Kinney, Michael D > mailto:michael.d.kin...@intel.com>> > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > If Stack Guard is enabled and there's really a stack overflow happened during > boot, a Page Fault exception will be triggered. Because the stack is out of > usage, the exception handler, which shares the stack with normal UEFI driver, > cannot be executed and cannot dump the processor information. > > Without those information, it's very difficult for the BIOS developers locate > the root cause of stack overflow. And without a workable stack, the developer > cannot event use single step to debug the UEFI driver with JTAG debugger. > > In order to make sure the exception handler to execute normally after stack > overflow. We need separate stacks for exception handlers in case of unusable > stack. > > IA processor allows to switch to a new stack during handling interrupt and > exception. But X64 and IA32 provides different ways to make it. X64 provides > interrupt stack table (IST) to allow maximum 7 different exceptions to h
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Hi Jeff, Thanks for the feedback. Are you suggesting to fix Bugzilla 109 in this patch? Thanks, Jian From: Fan Jeff [mailto:vanjeff_...@hotmail.com] Sent: Wednesday, November 01, 2017 10:33 AM To: Yao, Jiewen ; Wang, Jian J ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Dong, Eric ; Zeng, Star Subject: 答复: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Per https://bugzilla.tianocore.org/show_bug.cgi?id=109, TR should be setup (Such as in DxeIplPeim) even though NULL Cpu Exception Handler instance is chosen. For long term, I agree we need to move AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR to MdePkg(Such as BaseLib) For this patch, I have no strong opinion. 发件人: Yao, Jiewen<mailto:jiewen@intel.com> 发送时间: 2017年11月1日 9:56 收件人: Wang, Jian J<mailto:jian.j.w...@intel.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Kinney, Michael D<mailto:michael.d.kin...@intel.com>; Dong, Eric<mailto:eric.d...@intel.com>; Zeng, Star<mailto:star.z...@intel.com> 主题: Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support Hi Jian Thanks for the patch. Can we move all IA32 defined data structure or function to MdePkg? Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR I am also curious why we use different policy for other boot mode. Can we use consistent policy? > + if (PcdGetBool (PcdCpuStackGuard)) { > +// > +// Stack Guard works with the support of page table established and > +// memory management. So we have to exclude those boot modes > without > +// them. > +// > +switch (GetBootModeHob()) { > +case BOOT_ON_FLASH_UPDATE: > +case BOOT_IN_RECOVERY_MODE: > +case BOOT_ON_S3_RESUME: > + break; > + > +default: > + ArchSetupExcpetionStack (IdtTable); > + break; > +} > + } Thank you Yao Jiewen > -Original Message- > From: Wang, Jian J > Sent: Tuesday, October 31, 2017 10:24 PM > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > Cc: Zeng, Star mailto:star.z...@intel.com>>; Dong, Eric > mailto:eric.d...@intel.com>>; Yao, > Jiewen mailto:jiewen@intel.com>>; Kinney, Michael D > mailto:michael.d.kin...@intel.com>> > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > If Stack Guard is enabled and there's really a stack overflow happened during > boot, a Page Fault exception will be triggered. Because the stack is out of > usage, the exception handler, which shares the stack with normal UEFI driver, > cannot be executed and cannot dump the processor information. > > Without those information, it's very difficult for the BIOS developers locate > the root cause of stack overflow. And without a workable stack, the developer > cannot event use single step to debug the UEFI driver with JTAG debugger. > > In order to make sure the exception handler to execute normally after stack > overflow. We need separate stacks for exception handlers in case of unusable > stack. > > IA processor allows to switch to a new stack during handling interrupt and > exception. But X64 and IA32 provides different ways to make it. X64 provides > interrupt stack table (IST) to allow maximum 7 different exceptions to have > new stack for its handler. IA32 doesn't have IST mechanism and can only use > task gate to do it since task switch allows to load a new stack through its > task-state segment (TSS). > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > functionality works only in the boot phases capable of memory > allocation (besides the paging, for the sake of Stack Guard). In > other words, only DXE phase can supports Stack Guard with stack switch. > > Cc: Star Zeng mailto:star.z...@intel.com>> > Cc: Eric Dong mailto:eric.d...@intel.com>> > Cc: Jiewen Yao mailto:jiewen@intel.com>> > Cc: Michael Kinney > mailto:michael.d.kin...@intel.com>> > Suggested-by: Ayellet Wolman > mailto:ayellet.wol...@intel.com>> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > mailto:jian.j.w...@intel.com>> > --- > .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ > .../DxeCpuExceptionHandlerLib.inf | 5 + > .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + > .../Ia32/ArchExceptionHandler.c| 135 +++ > .../Ia32/ArchInterruptDefs.h | 136 +++ > .../Ia32/ExceptionTssEntryAsm.nasm | 398 > + > .../PeiCpuExceptionHandlerLib.inf | 1 + > .../SecPeiCpuExceptionHandlerLib.i
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Hi Jeff, Thanks for the feedback. From: Fan Jeff [mailto:vanjeff_...@hotmail.com] Sent: Wednesday, November 01, 2017 10:37 AM To: Wang, Jian J ; edk2-devel@lists.01.org Cc: Kinney, Michael D ; Yao, Jiewen ; Dong, Eric ; Zeng, Star Subject: 答复: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support 1. Should add EFIAPI for the following function. +VOID +AsmWriteTr ( + UINT16 Selector + ); 2. Should not add EFIAPI for the following function. +VOID +EFIAPI +ArchSetupExcpetionStack ( + IN IA32_IDT_GATE_DESCRIPTOR *IdtTable + ) 发件人: Jian J Wang<mailto:jian.j.w...@intel.com> 发送时间: 2017年10月31日 22:26 收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> 抄送: Michael Kinney<mailto:michael.d.kin...@intel.com>; Jiewen Yao<mailto:jiewen@intel.com>; Eric Dong<mailto:eric.d...@intel.com>; Star Zeng<mailto:star.z...@intel.com> 主题: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support If Stack Guard is enabled and there's really a stack overflow happened during boot, a Page Fault exception will be triggered. Because the stack is out of usage, the exception handler, which shares the stack with normal UEFI driver, cannot be executed and cannot dump the processor information. Without those information, it's very difficult for the BIOS developers locate the root cause of stack overflow. And without a workable stack, the developer cannot event use single step to debug the UEFI driver with JTAG debugger. In order to make sure the exception handler to execute normally after stack overflow. We need separate stacks for exception handlers in case of unusable stack. IA processor allows to switch to a new stack during handling interrupt and exception. But X64 and IA32 provides different ways to make it. X64 provides interrupt stack table (IST) to allow maximum 7 different exceptions to have new stack for its handler. IA32 doesn't have IST mechanism and can only use task gate to do it since task switch allows to load a new stack through its task-state segment (TSS). Note: Stack switch needs to allocate memory pages to be new stacks. So this functionality works only in the boot phases capable of memory allocation (besides the paging, for the sake of Stack Guard). In other words, only DXE phase can supports Stack Guard with stack switch. Cc: Star Zeng mailto:star.z...@intel.com>> Cc: Eric Dong mailto:eric.d...@intel.com>> Cc: Jiewen Yao mailto:jiewen@intel.com>> Cc: Michael Kinney mailto:michael.d.kin...@intel.com>> Suggested-by: Ayellet Wolman mailto:ayellet.wol...@intel.com>> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang mailto:jian.j.w...@intel.com>> --- .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ .../DxeCpuExceptionHandlerLib.inf | 5 + .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + .../Ia32/ArchExceptionHandler.c| 135 +++ .../Ia32/ArchInterruptDefs.h | 136 +++ .../Ia32/ExceptionTssEntryAsm.nasm | 398 + .../PeiCpuExceptionHandlerLib.inf | 1 + .../SecPeiCpuExceptionHandlerLib.inf | 3 + .../SmmCpuExceptionHandlerLib.inf | 1 + .../X64/ArchExceptionHandler.c | 108 ++ .../CpuExceptionHandlerLib/X64/ArchInterruptDefs.h | 40 +++ .../X64/ExceptionHandlerAsm.S | 12 + .../X64/ExceptionHandlerAsm.asm| 12 + .../X64/ExceptionHandlerAsm.nasm | 12 + 14 files changed, 904 insertions(+) create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h index 740a58828b..fd4a26a458 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h @@ -25,6 +25,7 @@ #include #include #include +#include #define CPU_EXCEPTION_NUM 32 #define CPU_INTERRUPT_NUM 256 @@ -288,5 +289,26 @@ CommonExceptionHandlerWorker ( IN EXCEPTION_HANDLER_DATA *ExceptionHandlerData ); +/** + Load given selector into TR register + + @param Selector Task segment selector +**/ +VOID +AsmWriteTr ( + UINT16 Selector + ); + +/** + Setup separate stack for specific exceptions. + + @param[in] IdtTableIDT table base. +**/ +VOID +EFIAPI +ArchSetupExcpetionStack ( + IN IA32_IDT_GATE_DESCRIPTOR *IdtTable + ); + #endif diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf index f4a8d01c80..b099ef4dad 100644 --- a/UefiCpuPk
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Hi Jiewen, Thanks for the feedback. > -Original Message- > From: Yao, Jiewen > Sent: Wednesday, November 01, 2017 9:57 AM > To: Wang, Jian J ; edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; > Kinney, Michael D > Subject: RE: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > Hi Jian > Thanks for the patch. > > Can we move all IA32 defined data structure or function to MdePkg? > Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR > Sure we can. > I am also curious why we use different policy for other boot mode. > Can we use consistent policy? > > + if (PcdGetBool (PcdCpuStackGuard)) { > > +// > > +// Stack Guard works with the support of page table established and > > +// memory management. So we have to exclude those boot modes > > without > > +// them. > > +// > > +switch (GetBootModeHob()) { > > +case BOOT_ON_FLASH_UPDATE: > > +case BOOT_IN_RECOVERY_MODE: > > +case BOOT_ON_S3_RESUME: > > + break; > > + > > +default: > > + ArchSetupExcpetionStack (IdtTable); > > + break; > > +} > > + } > As far as I'm aware of, those boot modes cannot provide both paging and memory management, which are needed by stack guard to work. Let me know if I'm wrong. > > Thank you > Yao Jiewen > > > > -Original Message- > > From: Wang, Jian J > > Sent: Tuesday, October 31, 2017 10:24 PM > > To: edk2-devel@lists.01.org > > Cc: Zeng, Star ; Dong, Eric ; Yao, > > Jiewen ; Kinney, Michael D > > > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > > support > > > > If Stack Guard is enabled and there's really a stack overflow happened > > during > > boot, a Page Fault exception will be triggered. Because the stack is out of > > usage, the exception handler, which shares the stack with normal UEFI > > driver, > > cannot be executed and cannot dump the processor information. > > > > Without those information, it's very difficult for the BIOS developers > > locate > > the root cause of stack overflow. And without a workable stack, the > > developer > > cannot event use single step to debug the UEFI driver with JTAG debugger. > > > > In order to make sure the exception handler to execute normally after stack > > overflow. We need separate stacks for exception handlers in case of unusable > > stack. > > > > IA processor allows to switch to a new stack during handling interrupt and > > exception. But X64 and IA32 provides different ways to make it. X64 provides > > interrupt stack table (IST) to allow maximum 7 different exceptions to have > > new stack for its handler. IA32 doesn't have IST mechanism and can only use > > task gate to do it since task switch allows to load a new stack through its > > task-state segment (TSS). > > > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > > functionality works only in the boot phases capable of memory > > allocation (besides the paging, for the sake of Stack Guard). In > > other words, only DXE phase can supports Stack Guard with stack > > switch. > > > > Cc: Star Zeng > > Cc: Eric Dong > > Cc: Jiewen Yao > > Cc: Michael Kinney > > Suggested-by: Ayellet Wolman > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ > > .../DxeCpuExceptionHandlerLib.inf | 5 + > > .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + > > .../Ia32/ArchExceptionHandler.c| 135 +++ > > .../Ia32/ArchInterruptDefs.h | 136 +++ > > .../Ia32/ExceptionTssEntryAsm.nasm | 398 > > + > > .../PeiCpuExceptionHandlerLib.inf | 1 + > > .../SecPeiCpuExceptionHandlerLib.inf | 3 + > > .../SmmCpuExceptionHandlerLib.inf | 1 + > > .../X64/ArchExceptionHandler.c | 108 ++ > > .../CpuExceptionHandlerLib/X64/ArchInterruptDefs.h | 40 +++ > > .../X64/ExceptionHandlerAsm.S | 12 + > > .../X64/ExceptionHandlerAsm.asm| 12 + > > .../X64/ExceptionHandlerAsm.nasm | 12 + > > 14 files changed, 904 insertions(+) > > create mode 100644 > > > UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm > > > > diff --git > > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > > index 740a58828b..fd4a26a458 100644 > > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > > @@ -25,6 +25,7 @@ > > #include > > #include > > #include > > +#include > > > > #define CPU_EXCEPTION_NUM 32 > > #define CPU_INTERRUPT_NUM 256 > > @@ -288,5 +289,26 @@ CommonExceptionHandler
Re: [edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
Hi Jian Thanks for the patch. Can we move all IA32 defined data structure or function to MdePkg? Such as: AsmWriteTr, IA32_TASK_STATE_SEGMENT, IA32_TSS_DESCRIPTOR I am also curious why we use different policy for other boot mode. Can we use consistent policy? > + if (PcdGetBool (PcdCpuStackGuard)) { > +// > +// Stack Guard works with the support of page table established and > +// memory management. So we have to exclude those boot modes > without > +// them. > +// > +switch (GetBootModeHob()) { > +case BOOT_ON_FLASH_UPDATE: > +case BOOT_IN_RECOVERY_MODE: > +case BOOT_ON_S3_RESUME: > + break; > + > +default: > + ArchSetupExcpetionStack (IdtTable); > + break; > +} > + } Thank you Yao Jiewen > -Original Message- > From: Wang, Jian J > Sent: Tuesday, October 31, 2017 10:24 PM > To: edk2-devel@lists.01.org > Cc: Zeng, Star ; Dong, Eric ; Yao, > Jiewen ; Kinney, Michael D > > Subject: [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch > support > > If Stack Guard is enabled and there's really a stack overflow happened during > boot, a Page Fault exception will be triggered. Because the stack is out of > usage, the exception handler, which shares the stack with normal UEFI driver, > cannot be executed and cannot dump the processor information. > > Without those information, it's very difficult for the BIOS developers locate > the root cause of stack overflow. And without a workable stack, the developer > cannot event use single step to debug the UEFI driver with JTAG debugger. > > In order to make sure the exception handler to execute normally after stack > overflow. We need separate stacks for exception handlers in case of unusable > stack. > > IA processor allows to switch to a new stack during handling interrupt and > exception. But X64 and IA32 provides different ways to make it. X64 provides > interrupt stack table (IST) to allow maximum 7 different exceptions to have > new stack for its handler. IA32 doesn't have IST mechanism and can only use > task gate to do it since task switch allows to load a new stack through its > task-state segment (TSS). > > Note: Stack switch needs to allocate memory pages to be new stacks. So this > functionality works only in the boot phases capable of memory > allocation (besides the paging, for the sake of Stack Guard). In > other words, only DXE phase can supports Stack Guard with stack switch. > > Cc: Star Zeng > Cc: Eric Dong > Cc: Jiewen Yao > Cc: Michael Kinney > Suggested-by: Ayellet Wolman > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang > --- > .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ > .../DxeCpuExceptionHandlerLib.inf | 5 + > .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + > .../Ia32/ArchExceptionHandler.c| 135 +++ > .../Ia32/ArchInterruptDefs.h | 136 +++ > .../Ia32/ExceptionTssEntryAsm.nasm | 398 > + > .../PeiCpuExceptionHandlerLib.inf | 1 + > .../SecPeiCpuExceptionHandlerLib.inf | 3 + > .../SmmCpuExceptionHandlerLib.inf | 1 + > .../X64/ArchExceptionHandler.c | 108 ++ > .../CpuExceptionHandlerLib/X64/ArchInterruptDefs.h | 40 +++ > .../X64/ExceptionHandlerAsm.S | 12 + > .../X64/ExceptionHandlerAsm.asm| 12 + > .../X64/ExceptionHandlerAsm.nasm | 12 + > 14 files changed, 904 insertions(+) > create mode 100644 > UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm > > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > index 740a58828b..fd4a26a458 100644 > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > > #define CPU_EXCEPTION_NUM 32 > #define CPU_INTERRUPT_NUM 256 > @@ -288,5 +289,26 @@ CommonExceptionHandlerWorker ( >IN EXCEPTION_HANDLER_DATA *ExceptionHandlerData >); > > +/** > + Load given selector into TR register > + > + @param Selector Task segment selector > +**/ > +VOID > +AsmWriteTr ( > + UINT16 Selector > + ); > + > +/** > + Setup separate stack for specific exceptions. > + > + @param[in] IdtTableIDT table base. > +**/ > +VOID > +EFIAPI > +ArchSetupExcpetionStack ( > + IN IA32_IDT_GATE_DESCRIPTOR *IdtTable > + ); > + > #endif > > diff --git > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf > index f4a8d01c80..b099ef4dad 100644 > --- > a/UefiCpuPkg/Library/CpuExceptionHan
[edk2] [PATCH 3/3] UefiCpuPkg/CpuExceptionHandlerLib: Add stack switch support
If Stack Guard is enabled and there's really a stack overflow happened during boot, a Page Fault exception will be triggered. Because the stack is out of usage, the exception handler, which shares the stack with normal UEFI driver, cannot be executed and cannot dump the processor information. Without those information, it's very difficult for the BIOS developers locate the root cause of stack overflow. And without a workable stack, the developer cannot event use single step to debug the UEFI driver with JTAG debugger. In order to make sure the exception handler to execute normally after stack overflow. We need separate stacks for exception handlers in case of unusable stack. IA processor allows to switch to a new stack during handling interrupt and exception. But X64 and IA32 provides different ways to make it. X64 provides interrupt stack table (IST) to allow maximum 7 different exceptions to have new stack for its handler. IA32 doesn't have IST mechanism and can only use task gate to do it since task switch allows to load a new stack through its task-state segment (TSS). Note: Stack switch needs to allocate memory pages to be new stacks. So this functionality works only in the boot phases capable of memory allocation (besides the paging, for the sake of Stack Guard). In other words, only DXE phase can supports Stack Guard with stack switch. Cc: Star Zeng Cc: Eric Dong Cc: Jiewen Yao Cc: Michael Kinney Suggested-by: Ayellet Wolman Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../CpuExceptionHandlerLib/CpuExceptionCommon.h| 22 ++ .../DxeCpuExceptionHandlerLib.inf | 5 + .../Library/CpuExceptionHandlerLib/DxeException.c | 19 + .../Ia32/ArchExceptionHandler.c| 135 +++ .../Ia32/ArchInterruptDefs.h | 136 +++ .../Ia32/ExceptionTssEntryAsm.nasm | 398 + .../PeiCpuExceptionHandlerLib.inf | 1 + .../SecPeiCpuExceptionHandlerLib.inf | 3 + .../SmmCpuExceptionHandlerLib.inf | 1 + .../X64/ArchExceptionHandler.c | 108 ++ .../CpuExceptionHandlerLib/X64/ArchInterruptDefs.h | 40 +++ .../X64/ExceptionHandlerAsm.S | 12 + .../X64/ExceptionHandlerAsm.asm| 12 + .../X64/ExceptionHandlerAsm.nasm | 12 + 14 files changed, 904 insertions(+) create mode 100644 UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h index 740a58828b..fd4a26a458 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h @@ -25,6 +25,7 @@ #include #include #include +#include #define CPU_EXCEPTION_NUM 32 #define CPU_INTERRUPT_NUM 256 @@ -288,5 +289,26 @@ CommonExceptionHandlerWorker ( IN EXCEPTION_HANDLER_DATA *ExceptionHandlerData ); +/** + Load given selector into TR register + + @param Selector Task segment selector +**/ +VOID +AsmWriteTr ( + UINT16 Selector + ); + +/** + Setup separate stack for specific exceptions. + + @param[in] IdtTableIDT table base. +**/ +VOID +EFIAPI +ArchSetupExcpetionStack ( + IN IA32_IDT_GATE_DESCRIPTOR *IdtTable + ); + #endif diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf index f4a8d01c80..b099ef4dad 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf @@ -30,6 +30,7 @@ [Sources.Ia32] Ia32/ExceptionHandlerAsm.asm Ia32/ExceptionHandlerAsm.nasm + Ia32/ExceptionTssEntryAsm.nasm Ia32/ExceptionHandlerAsm.S Ia32/ArchExceptionHandler.c Ia32/ArchInterruptDefs.h @@ -47,6 +48,9 @@ PeiDxeSmmCpuException.c DxeException.c +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard + [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec @@ -61,3 +65,4 @@ PeCoffGetEntryPointLib MemoryAllocationLib DebugLib + HobLib diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c index 31febec976..e9dcd00e02 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c @@ -16,6 +16,7 @@ #include "CpuExceptionCommon.h" #include #include +#include CONST UINTNmDoFarReturnFlag = 0; @@ -155,6 +156,24 @@ InitializeCpuInterruptHandlers ( UpdateIdtTable (IdtTable, &TemplateMap, &mExceptionHandlerData); + if (PcdGetBool (PcdCpuStackGuard)) { +// +// Stack Guard works