Hi, Brian Yes. This is one regression. Thanks your fix.
Reviewed-by: Jeff Fan <jeff....@intel.com> There is another issue on enable CPU interrupt in DEBUG_AGENT_INIT_POSTMEM_SEC case. I will fix it soon. Thanks! Jeff -----Original Message----- From: Brian J. Johnson [mailto:bjohn...@sgi.com] Sent: Thursday, July 09, 2015 3:34 AM To: edk2-devel@lists.sourceforge.net Cc: Fan, Jeff Subject: [PATCH] SourceLevelDebugPkg: Fix PEI timer interrupt regression Recent changes to debug timer initialization (commit 2befbc82, svn 17572) modified the Sec/Pei InitializeDebugAgent() routine to enable debug timer interrupts. This causes problems in the DEBUG_AGENT_INIT_POSTMEM_SEC case: the callers appear to assume that if they block timer interrupts before the call, interrupts will remain blocked afterwards. It is not always safe to have interrupts enabled on return from InitializeDebugAgent(). For instance, after calling InitializeDebugAgent(), OvmfPkg's TemporaryRamMigration() moves the stack, heap, and IDT to RAM, then switches to the new stack. Only then does it reenable timer interrupts. Taking an interrupt during this process can corrupt state, causing crashes. Do not unmask the debug timer interrupt in the DEBUG_AGENT_INIT_POSTMEM_SEC case. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Brian J. Johnson <bjohn...@sgi.com> --- This fix has been built with OvmfPkg/OvmfPkgIa32X64.dsc, using the GCC48 toolchain under Ubuntu. It builds and boots to shell under Qemu/tcg, but I'm not set up to test remote debugging with OVMF. An equivalent fix works well on hardware in a proprietary BIOS, built with the WINDDK compilers. This is a minimal fix. I'm not sure if timer interrupts should be enabled in any of the other InitializeDebugAgent() code paths... if not, the call to SaveAndSetDebugTimerInterrupt (TRUE) can just be removed. Someone more familiar with them may want to audit the callers. The DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64 case is especially unclear to me; I'm not quite sure what the caller (MdeModulePkg/Universal/CapsulePei) is up to. We also call SaveAndSetDebugTimerInterrupt (TRUE) in InitializeDebugAgentPhase2(), where it's necessary to enable interrupts for the DEBUG_AGENT_INIT_PREMEM_SEC case. Thanks, Brian -------------------------------------------------------------------- My statements are my own, are not authorized by SGI, and do not necessarily represent SGI’s positions. .../Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c index ea75742..9fbe4ce 100644 --- a/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c +++ b/SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgent/SecPeiDebugAgentLib.c @@ -612,9 +612,11 @@ InitializeDebugAgent ( } // - // Enable Debug Timer interrupt + // Enable Debug Timer interrupt. In post-memory SEC, the caller enables it. // - SaveAndSetDebugTimerInterrupt (TRUE); + if (InitFlag != DEBUG_AGENT_INIT_POSTMEM_SEC) { + SaveAndSetDebugTimerInterrupt (TRUE); } // // Enable CPU interrupts so debug timer interrupts can be delivered // ------------------------------------------------------------------------------ Don't Limit Your Business. Reach for the Cloud. GigeNET's Cloud Solutions provide you with the tools and support that you need to offload your IT needs and focus on growing your business. Configured For All Businesses. Start Your Cloud Today. https://www.gigenetcloud.com/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
