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

Reply via email to