From: vanjeff <vanjeff@6f19259b-4bc3-4df7-8a09-765794883524>
Date: Tue, 16 Dec 2008 09:21:45 +0000 (+0000)
Subject: code scrub:
X-Git-Tag: edk2-stable201903~19203
X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=commitdiff_plain;h=509bc208d835762c31987b00fab708c1d2ff2c5d

code scrub:
1. updated functions header.
2. sync some comments for .S files.
3. fixed some error in AsmFuncs.S

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@7044 6f19259b-4bc3-4df7-8a09-765794883524
---

diff --git a/MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf b/MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf
index 41ed14d453..9b424e31fe 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf
+++ b/MdeModulePkg/Universal/DebugSupportDxe/DebugSupportDxe.inf
@@ -1,7 +1,11 @@
 #/** @file
-# Component description file for DebugSupport module.
+# This driver installs Debug Support protocol for the selected processor.
+#
+# This driver provides the capabilities for debug-agent to gain control of the machine
+# when certain types of events occur, i.e. breakpoint, processor execptions, etc. It also
+# provides debug-agent to periodically gain control during operation of the machine to
+# check for asynchronous commands form the host.
 #
-# This driver installs DebugSupport protocol for the selected processor.
 # Copyright (c) 2006 - 2008, Intel Corporation
 #
 #  All rights reserved. This program and the accompanying materials
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/AsmFuncs.S b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/AsmFuncs.S
index b6f326d805..03fc6c98ba 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/AsmFuncs.S
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/AsmFuncs.S
@@ -1,7 +1,7 @@
 #/**@file
 # Low leve IA32 specific debug support functions.
 #
-# Copyright (c) 2006, Intel Corporation
+# Copyright (c) 2006 - 2008, Intel Corporation. <BR>
 # All rights reserved. This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD License
 # which accompanies this distribution.  The full text of the license may be found at
@@ -18,16 +18,27 @@
 .globl ASM_PFX(CommonIdtEntry)
 .globl ASM_PFX(FxStorSupport)
 
+ASM_PFX(StubSize):       .long   ASM_PFX(InterruptEntryStubEnd) - ASM_PFX(InterruptEntryStub)
 ASM_PFX(AppEsp):         .long   0x11111111 # ?
 ASM_PFX(DebugEsp):       .long   0x22222222 # ?
 ASM_PFX(ExtraPush):      .long   0x33333333 # ?
 ASM_PFX(ExceptData):     .long   0x44444444 # ?
 ASM_PFX(Eflags):         .long   0x55555555 # ?
 ASM_PFX(OrigVector):     .long   0x66666666 # ?
-ASM_PFX(StubSize):       .long   ASM_PFX(InterruptEntryStubEnd) - ASM_PFX(InterruptEntryStub)
 
+#------------------------------------------------------------------------------
+# BOOLEAN
+# FxStorSupport (
+#   void
+#   )
+#
+# Abstract: Returns TRUE if FxStor instructions are supported
+#
 .globl ASM_PFX(FxStorSupport)
 ASM_PFX(FxStorSupport):
+#
+# cpuid corrupts ebx which must be preserved per the C calling convention
+#
         push   %ebx
         mov    $0x1,%eax
         cpuid
@@ -36,6 +47,15 @@ ASM_PFX(FxStorSupport):
         shr    $0x18,%eax
         pop    %ebx
         ret
+#------------------------------------------------------------------------------
+# void
+# Vect2Desc (
+#   DESCRIPTOR * DestDesc,
+#   void (*Vector) (void)
+#   )
+#
+# Abstract: Encodes an IDT descriptor with the given physical address
+#
 
 .globl ASM_PFX(Vect2Desc)
 ASM_PFX(Vect2Desc):
@@ -53,18 +73,68 @@ ASM_PFX(Vect2Desc):
 
 .globl ASM_PFX(InterruptEntryStub)
 ASM_PFX(InterruptEntryStub):
-        mov    %esp,0x0
-        mov    $0x0,%esp
-        push   $0x0
-        jmp    ASM_PFX(CommonIdtEntry)
+        mov    %esp,0x0                    # save stack top
+        mov    $0x0,%esp                   # switch to debugger stack
+        push   $0x0                        # push vector number - will be modified before installed
+        jmp    ASM_PFX(CommonIdtEntry)     # jump CommonIdtEntry
 .globl ASM_PFX(InterruptEntryStubEnd)
 ASM_PFX(InterruptEntryStubEnd):
 
+#------------------------------------------------------------------------------
+# CommonIdtEntry
+#
+# Abstract: This code is not a function, but is the common part for all IDT
+#               vectors.
+#
 .globl ASM_PFX(CommonIdtEntry)
 ASM_PFX(CommonIdtEntry):
+##
+## At this point, the stub has saved the current application stack esp into AppEsp
+## and switched stacks to the debug stack, where it pushed the vector number
+##
+## The application stack looks like this:
+##
+##              ...
+##              (last application stack entry)
+##              eflags from interrupted task
+##              CS from interrupted task
+##              EIP from interrupted task
+##              Error code <-------------------- Only present for some exeption types
+##
+##
+
+
+## The stub switched us to the debug stack and pushed the interrupt number.
+##
+## Next, construct the context record.  It will be build on the debug stack by
+## pushing the registers in the correct order so as to create the context structure
+## on the debug stack.  The context record must be built from the end back to the
+## beginning because the stack grows down...
+#
+## For reference, the context record looks like this:
+##
+## typedef
+## struct {
+##   UINT32             ExceptionData;
+##   FX_SAVE_STATE_IA32 FxSaveState;
+##   UINT32             Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
+##   UINT32             Cr0, Cr2, Cr3, Cr4;
+##   UINT32             EFlags;
+##   UINT32             Ldtr, Tr;
+##   UINT32             Gdtr[2], Idtr[2];
+##   UINT32             Eip;
+##   UINT32             Gs, Fs, Es, Ds, Cs, Ss;
+##   UINT32             Edi, Esi, Ebp, Esp, Ebx, Edx, Ecx, Eax;
+## } SYSTEM_CONTEXT_IA32;  // 32 bit system context record
+
+## UINT32  Edi, Esi, Ebp, Esp, Ebx, Edx, Ecx, Eax;
         pusha
+## Save interrupt state eflags register...
         pushf
         pop    %eax
+## We need to determine if any extra data was pushed by the exception, and if so, save it
+## To do this, we check the exception number pushed by the stub, and cache the
+## result in a variable since we'll need this again.
         mov    %eax,0x0
         cmpl   $0x8,0x0
         jne    ASM_PFX(CommonIdtEntry+0x20)
@@ -95,6 +165,9 @@ ASM_PFX(CommonIdtEntry):
         movl   $0x1,0x0
         jmp    ASM_PFX(CommonIdtEntry+0xa8)
         movl   $0x0,0x0
+## If there's some extra data, save it also, and modify the saved AppEsp to effectively
+## pop this value off the application's stack.
+
         cmpl   $0x1,0x0
         jne    ASM_PFX(CommonIdtEntry+0xc8)
         mov    0x0,%eax
@@ -104,13 +177,23 @@ ASM_PFX(CommonIdtEntry):
         mov    %eax,0x0
         jmp    ASM_PFX(CommonIdtEntry+0xd2)
         movl   $0x0,0x0
+## The "pushad" above pushed the debug stack esp.  Since what we're actually doing
+## is building the context record on the debug stack, we need to save the pushed
+## debug ESP, and replace it with the application's last stack entry...
         mov    0xc(%esp),%eax
         mov    %eax,0x0
         mov    0x0,%eax
         add    $0xc,%eax
+        # application stack has eflags, cs, & eip, so
+        # last actual application stack entry is
+        # 12 bytes into the application stack.
         mov    %eax,0xc(%esp)
+## continue building context record
+## UINT32  Gs, Fs, Es, Ds, Cs, Ss;  insure high 16 bits of each is zero
         mov    %ss,%eax
         push   %eax
+        
+        # CS from application is one entry back in application stack
         mov    0x0,%eax
         movzwl 0x4(%eax),%eax
         push   %eax
@@ -122,21 +205,35 @@ ASM_PFX(CommonIdtEntry):
         push   %eax
         mov    %gs,%eax
         push   %eax
+
+## UINT32  Eip;
+        # Eip from application is on top of application stack
         mov    0x0,%eax
         pushl  (%eax)
+
+## UINT32  Gdtr[2], Idtr[2];
         push   $0x0
         push   $0x0
         sidtl  (%esp)
         push   $0x0
         push   $0x0
         sgdtl  (%esp)
+
+## UINT32  Ldtr, Tr;
         xor    %eax,%eax
         str    %eax
         push   %eax
         sldt   %eax
         push   %eax
+
+## UINT32  EFlags;
+## Eflags from application is two entries back in application stack
         mov    0x0,%eax
         pushl  0x8(%eax)
+
+## UINT32  Cr0, Cr1, Cr2, Cr3, Cr4;
+## insure FXSAVE/FXRSTOR is enabled in CR4...
+## ... while we're at it, make sure DE is also enabled...
         mov    %cr4,%eax
         or     $0x208,%eax
         mov    %eax,%cr4
@@ -148,12 +245,18 @@ ASM_PFX(CommonIdtEntry):
         push   $0x0
         mov    %cr0,%eax
         push   %eax
+
+## UINT32  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
         mov    %db7,%eax
         push   %eax
+
+## clear Dr7 while executing debugger itself
         xor    %eax,%eax
         mov    %eax,%db7
         mov    %db6,%eax
         push   %eax
+
+## insure all status bits in dr6 are clear...
         xor    %eax,%eax
         mov    %eax,%db6
         mov    %db3,%eax
@@ -164,21 +267,38 @@ ASM_PFX(CommonIdtEntry):
         push   %eax
         mov    %db0,%eax
         push   %eax
+
+## FX_SAVE_STATE_IA32 FxSaveState;
         sub    $0x200,%esp
         mov    %esp,%edi
+        # IMPORTANT!! The debug stack has been carefully constructed to
+        # insure that esp and edi are 16 byte aligned when we get here.
+        # They MUST be.  If they are not, a GP fault will occur.
         fxsave (%edi)
+
+## UINT32  ExceptionData;
         mov    0x0,%eax
         push   %eax
+
+# call to C code which will in turn call registered handler
+# pass in the vector number
         mov    %esp,%eax
         push   %eax
         mov    0x0,%eax
         push   %eax
         call   ASM_PFX(CommonIdtEntry+0x184)
         add    $0x8,%esp
+
+# restore context...
+## UINT32  ExceptionData;
         add    $0x4,%esp
+
+## FX_SAVE_STATE_IA32 FxSaveState;
         mov    %esp,%esi
         fxrstor (%esi)
         add    $0x200,%esp
+
+## UINT32  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
         pop    %eax
         mov    %eax,%db0
         pop    %eax
@@ -187,9 +307,13 @@ ASM_PFX(CommonIdtEntry):
         mov    %eax,%db2
         pop    %eax
         mov    %eax,%db3
+
+## skip restore of dr6.  We cleared dr6 during the context save.
         add    $0x4,%esp
         pop    %eax
         mov    %eax,%db7
+
+## UINT32  Cr0, Cr1, Cr2, Cr3, Cr4;
         pop    %eax
         mov    %eax,%cr0
         add    $0x4,%esp
@@ -199,10 +323,25 @@ ASM_PFX(CommonIdtEntry):
         mov    %eax,%cr3
         pop    %eax
         mov    %eax,%cr4
+
+## UINT32  EFlags;
         mov    0x0,%eax
         popl   0x8(%eax)
+
+## UINT32  Ldtr, Tr;
+## UINT32  Gdtr[2], Idtr[2];
+## Best not let anyone mess with these particular registers...
         add    $0x18,%esp
+
+## UINT32  Eip;
         popl   (%eax)
+
+## UINT32  SegGs, SegFs, SegEs, SegDs, SegCs, SegSs;
+## NOTE - modified segment registers could hang the debugger...  We
+##        could attempt to insulate ourselves against this possibility,
+##        but that poses risks as well.
+##
+
         pop    %gs
         pop    %fs
         pop    %es
@@ -210,35 +349,56 @@ ASM_PFX(CommonIdtEntry):
         popl   0x4(%eax)
         pop    %ss
         mov    0xc(%esp),%ebx
-        mov    0x0,%eax
+
+## The next stuff to restore is the general purpose registers that were pushed
+## using the "pushad" instruction.
+##
+## The value of ESP as stored in the context record is the application ESP
+## including the 3 entries on the application stack caused by the exception
+## itself. It may have been modified by the debug agent, so we need to
+## determine if we need to relocate the application stack.
+
+        mov    0x0,%eax          # move the potentially modified AppEsp into ebx
         add    $0xc,%eax
         cmp    %eax,%ebx
         je     ASM_PFX(CommonIdtEntry+0x202)
         mov    0x0,%eax
-        mov    (%eax),%ecx
+        mov    (%eax),%ecx       # EIP
         mov    %ecx,(%ebx)
-        mov    0x4(%eax),%ecx
+        mov    0x4(%eax),%ecx    # CS
         mov    %ecx,0x4(%ebx)
-        mov    0x8(%eax),%ecx
+        mov    0x8(%eax),%ecx    # EFLAGS
         mov    %ecx,0x8(%ebx)
-        mov    %ebx,%eax
+	
+        mov    %ebx,%eax         # modify the saved AppEsp to the new AppEsp
         mov    %eax,0x0
-        mov    0x0,%eax
-        mov    %eax,0xc(%esp)
+        mov    0x0,%eax          # restore the DebugEsp on the debug stack
+	                               # so our "popad" will not cause a stack switch
+        mov    %eax,0xc(%esp)    
         cmpl   $0x68,0x0
         jne    PhonyIretd+0xd
+## Restore eflags so when we chain, the flags will be exactly as if we were never here.
+## We gin up the stack to do an iretd so we can get ALL the flags.
         mov    0x0,%eax
         mov    0x8(%eax),%ebx
-        and    $0xfffffcff,%ebx
+        and    $0xfffffcff,%ebx  # special handling for IF and TF
         push   %ebx
         push   %cs
         push   $0x0
         iret
 
 PhonyIretd:
+## UINT32  Edi, Esi, Ebp, Esp, Ebx, Edx, Ecx, Eax;
         popa
+
+## Switch back to application stack
         mov    0x0,%esp
         jmp    *0x0
+## Jump to original handler
+## UINT32  Edi, Esi, Ebp, Esp, Ebx, Edx, Ecx, Eax;
         popa
+## Switch back to application stack
         mov    0x0,%esp
+
+## We're outa here...
         iret
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupport.c b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupport.c
index 3c96f9b518..ed38d06cae 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupport.c
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupport.c
@@ -1,5 +1,5 @@
 /** @file
-  Generic debug support functions for IA32/x64.
+  IA32/x64 generic functions to support Debug Support protocol.
 
 Copyright (c) 2006 - 2008, Intel Corporation
 All rights reserved. This program and the accompanying materials
@@ -90,11 +90,27 @@ HookEntry (
   //
   OldIntFlagState = SaveAndDisableInterrupts ();
 
+  //
+  // gets IDT Gate descriptor by index
+  //
   ReadIdtGateDescriptor (ExceptionType, &(IdtEntryTable[ExceptionType].OrigDesc));
+  //
+  // stores orignal interrupt handle 
+  //
   IdtEntryTable[ExceptionType].OrigVector = (DEBUG_PROC) GetInterruptHandleFromIdt (&(IdtEntryTable[ExceptionType].OrigDesc));
 
+  // 
+  // encodes new IDT Gate descriptor by stub entry 
+  //
   Vect2Desc (&IdtEntryTable[ExceptionType].NewDesc, IdtEntryTable[ExceptionType].StubEntry);
+  //
+  // stores NewCallback
+  //
   IdtEntryTable[ExceptionType].RegisteredCallback = NewCallback;
+
+  //
+  // writes back new IDT Gate descriptor
+  //
   WriteIdtGateDescriptor (ExceptionType, &(IdtEntryTable[ExceptionType].NewDesc));
 
   //
@@ -214,6 +230,7 @@ RegisterExceptionCallback (
   return ManageIdtEntryTable (ExceptionCallback, ExceptionType);
 }
 
+
 /**
   Invalidates processor instruction cache for a memory range. Subsequent execution in this range
   causes a fresh memory fetch to retrieve code to be executed.                                  
@@ -244,6 +261,7 @@ InvalidateInstructionCache (
   Common piece of code that invokes the registered handlers.
 
   This code executes in exception context so no efi calls are allowed.
+  This code is called from assembly file.
 
   @param  ExceptionType     Exception type
   @param  ContextRecord     System context
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupportIa32.c b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupportIa32.c
index c75c040152..d2073f65fd 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupportIa32.c
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ia32/PlDebugSupportIa32.c
@@ -1,7 +1,7 @@
 /** @file
-  IA32 specific debug support functions
+  IA32 specific functions to support Debug Support protocol.
 
-Copyright (c) 2006 - 2008, Intel Corporation
+Copyright (c) 2008, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/AsmFuncs.s b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/AsmFuncs.s
index 03603eafa1..aa497707c4 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/AsmFuncs.s
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/AsmFuncs.s
@@ -10,8 +10,6 @@
 /// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 /// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 ///
-/// Module Name: AsmFuncs.s
-///
 ///
 
 
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/Ds64Macros.i b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/Ds64Macros.i
index 5855545f01..eb3f63d9aa 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/Ds64Macros.i
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/Ds64Macros.i
@@ -10,8 +10,6 @@
 /// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
 /// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 ///
-/// Module Name: Ds64Macros.i
-///
 ///
 
 
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.c b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.c
index d358b425de..1e1c89d5de 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.c
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.c
@@ -1,5 +1,5 @@
 /** @file
-  IPF specific debug support functions
+  IPF specific functions to support Debug Support protocol.
 
 Copyright (c) 2006 - 2008, Intel Corporation                                                         
 All rights reserved. This program and the accompanying materials                          
@@ -12,9 +12,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
-//
-// private header files
-//
 #include "PlDebugSupport.h"
 
 BOOLEAN  mInHandler = FALSE;
@@ -66,7 +63,9 @@ PlInitializeDebugSupportDriver (
 
 /**
   Unload handler that is called during UnloadImage() - deallocates pool memory
-  used by the driver.  Must be public because it's referenced from DebugSuport.c
+  used by the driver. 
+
+  Must be public because it's referenced from DebugSuport.c
 
   @param  ImageHandle    The firmware allocated handle for the EFI image.
 
@@ -90,10 +89,12 @@ PlUnloadDebugSupportDriver (
 
 /**
   C routine that is called for all registered exceptions.  This is the main
-  exception dispatcher.  Must be public because it's referenced from AsmFuncs.s.
+  exception dispatcher. 
+
+  Must be public because it's referenced from AsmFuncs.s.
 
-  @param  ExceptionType        Exception Type
-  @param  Context              System Context
+  @param  ExceptionType        Specifies which processor exception.
+  @param  Context              System Context.
 **/
 VOID
 CommonHandler (
@@ -170,12 +171,11 @@ GetHandlerEntryPoint (
 /**
   This is the worker function that uninstalls and removes all handlers.
 
-  @param  ExceptionType     Exception Type
-  @param  NewBundles        New Boundles
-  @param  NewCallback       New Callback
+  @param  ExceptionType     Specifies which processor exception.
+  @param  NewBundles        New Boundles.
+  @param  NewCallback       A pointer to the new function to be registered.
 
   @retval EFI_ALEADY_STARTED Ivt already hooked.
-  @retval others             Indicates the request was not satisfied.
   @retval EFI_SUCCESS        Successfully uninstalled.
 
 **/
@@ -248,9 +248,9 @@ ManageIvtEntryTable (
   Saves original IVT contents and inserts a few new bundles which are fixed up
   to store the ExceptionType and then call the common handler.
 
-  @param  ExceptionType      Exception Type
-  @param  NewBundles         New Boundles
-  @param  NewCallback        New Callback
+  @param  ExceptionType      Specifies which processor exception.
+  @param  NewBundles         New Boundles.
+  @param  NewCallback        A pointer to the new function to be hooked.
 
 **/
 VOID
@@ -294,7 +294,7 @@ HookEntry (
 /**
   Restores original IVT contents when unregistering a callback function.
 
-  @param  ExceptionType     Exception Type
+  @param  ExceptionType     Specifies which processor exception.
 
 **/
 VOID
@@ -324,7 +324,7 @@ UnhookEntry (
 
   Records new callback in IvtEntryTable.
 
-  @param  NewCallback     New Callback
+  @param  NewCallback     A pointer to the interrupt handle.
 
 **/
 VOID
@@ -358,19 +358,17 @@ UnchainExternalInterrupt (
   IvtEntryTable[EXCEPT_IPF_EXTERNAL_INTERRUPT].RegisteredCallback = NULL;
 }
 
-//
-// The rest of the functions in this file are all member functions for the
-// DebugSupport protocol
-//
-
 /**
-  This is a DebugSupport protocol member function, hard
-  coded to support only 1 processor for now.
-
-  @param  This                The DebugSupport instance
-  @param  MaxProcessorIndex   The maximuim supported processor index
+  Returns the maximum value that may be used for the ProcessorIndex parameter in
+  RegisterPeriodicCallback() and RegisterExceptionCallback().                   
+    
+  Hard coded to support only 1 processor for now.
 
-  @retval EFI_SUCCESS         Always returned with **MaxProcessorIndex set to 0.
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  MaxProcessorIndex     Pointer to a caller-allocated UINTN in which the maximum supported
+                                processor index is returned. Always 0 returned.                                     
+                                
+  @retval EFI_SUCCESS           Always returned with **MaxProcessorIndex set to 0.
 
 **/
 EFI_STATUS
@@ -385,15 +383,18 @@ GetMaximumProcessorIndex (
 }
 
 /**
-  DebugSupport protocol member function.
-
-  @param  This               The DebugSupport instance
-  @param  ProcessorIndex     Which processor the callback applies to.
-  @param  PeriodicCallback   Callback function
-
-  @retval EFI_SUCCESS        Indicates the callback was registered.
-  @retval others             Callback was not registered.
-
+  Registers a function to be called back periodically in interrupt context.
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor the callback function applies to.
+  @param  PeriodicCallback      A pointer to a function of type PERIODIC_CALLBACK that is the main
+                                periodic entry point of the debug agent.
+                                
+  @retval EFI_SUCCESS           The function completed successfully.  
+  @retval EFI_ALREADY_STARTED   Non-NULL PeriodicCallback parameter when a callback
+                                function was previously registered.                
+  @retval EFI_OUT_OF_RESOURCES  System has insufficient memory resources to register new callback                               
+                                function. 
 **/
 EFI_STATUS
 EFIAPI
@@ -407,16 +408,21 @@ RegisterPeriodicCallback (
 }
 
 /**
-  DebugSupport protocol member function.
-
-  @param  This              The DebugSupport instance
-  @param  ProcessorIndex    Which processor the callback applies to.
-  @param  NewCallback       Callback function
-  @param  ExceptionType     Which exception to hook
-
-  @retval EFI_SUCCESS        Indicates the callback was registered.
-  @retval others             Callback was not registered.
-
+  Registers a function to be called when a given processor exception occurs.
+
+  This code executes in boot services context.
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor the callback function applies to.
+  @param  ExceptionCallback     A pointer to a function of type EXCEPTION_CALLBACK that is called
+                                when the processor exception specified by ExceptionType occurs.  
+  @param  ExceptionType         Specifies which processor exception to hook.                       
+                                
+  @retval EFI_SUCCESS           The function completed successfully.  
+  @retval EFI_ALREADY_STARTED   Non-NULL PeriodicCallback parameter when a callback
+                                function was previously registered.                
+  @retval EFI_OUT_OF_RESOURCES  System has insufficient memory resources to register new callback                               
+                                function.
 **/
 EFI_STATUS
 EFIAPI
@@ -435,14 +441,16 @@ RegisterExceptionCallback (
 }
 
 /**
-  DebugSupport protocol member function.  Calls assembly routine to flush cache.
-
-  @param  This              The DebugSupport instance
-  @param  ProcessorIndex    Which processor the callback applies to.
-  @param  Start             Physical base of the memory range to be invalidated
-  @param  Length            mininum number of bytes in instruction cache to invalidate
-
-  @retval EFI_SUCCESS       Always returned.
+  Invalidates processor instruction cache for a memory range. Subsequent execution in this range
+  causes a fresh memory fetch to retrieve code to be executed.                                  
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor's instruction cache is to be invalidated.
+  @param  Start                 Specifies the physical base of the memory range to be invalidated.                                
+  @param  Length                Specifies the minimum number of bytes in the processor's instruction
+                                cache to invalidate.                                                 
+                                
+  @retval EFI_SUCCESS           Always returned.
 
 **/
 EFI_STATUS
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.h b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.h
index e1e34a449c..9d556a17fe 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.h
+++ b/MdeModulePkg/Universal/DebugSupportDxe/Ipf/PlDebugSupport.h
@@ -1,7 +1,7 @@
 /** @file
-  IPF specific debugsupport types, macros, and definitions.
+  IPF specific types, macros, and definitions for Debug Support Driver.
   
-Copyright (c) 2004 - 2006 Intel Corporation                                                         
+Copyright (c) 2004 - 2008, Intel Corporation                                                       
 All rights reserved. This program and the accompanying materials                          
 are licensed and made available under the terms and conditions of the BSD License         
 which accompanies this distribution.  The full text of the license may be found at        
@@ -50,7 +50,9 @@ PlInitializeDebugSupportDriver (
 
 /**
   Unload handler that is called during UnloadImage() - deallocates pool memory
-  used by the driver.  Must be public because it's referenced from DebugSuport.c
+  used by the driver. 
+
+  Must be public because it's referenced from DebugSuport.c
 
   @param  ImageHandle    The firmware allocated handle for the EFI image.
 
@@ -75,7 +77,7 @@ GetIva (
   );
 
 /**
-  HookStub will be copied from it's loaded location into the IVT when
+  C callable function that HookStub will be copied from it's loaded location into the IVT when
   an IVT entry is hooked.
 
 **/
@@ -85,7 +87,7 @@ HookStub (
   );
 
 /**
-  Chains an interrupt handler.
+  C callable function to chain an interrupt handler.
 
 **/
 VOID
@@ -94,7 +96,7 @@ ChainHandler (
   );
 
 /**
-  Unchains an interrupt handler.
+  C callable function to unchain an interrupt handler.
 
 **/
 VOID
@@ -129,13 +131,16 @@ InstructionCacheFlush (
   );
 
 /**
-  This is a DebugSupport protocol member function, hard
-  coded to support only 1 processor for now.
-
-  @param  This                The DebugSupport instance
-  @param  MaxProcessorIndex   The maximuim supported processor index
+  Returns the maximum value that may be used for the ProcessorIndex parameter in
+  RegisterPeriodicCallback() and RegisterExceptionCallback().                   
+    
+  Hard coded to support only 1 processor for now.
 
-  @retval EFI_SUCCESS         Always returned with **MaxProcessorIndex set to 0.
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  MaxProcessorIndex     Pointer to a caller-allocated UINTN in which the maximum supported
+                                processor index is returned. Always 0 returned.                                     
+                                
+  @retval EFI_SUCCESS           Always returned with **MaxProcessorIndex set to 0.
 
 **/
 EFI_STATUS
@@ -146,15 +151,18 @@ GetMaximumProcessorIndex (
   );
 
 /**
-  DebugSupport protocol member function.
-
-  @param  This               The DebugSupport instance
-  @param  ProcessorIndex     Which processor the callback applies to.
-  @param  PeriodicCallback   Callback function
-
-  @retval EFI_SUCCESS        Indicates the callback was registered.
-  @retval others             Callback was not registered.
-
+  Registers a function to be called back periodically in interrupt context.
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor the callback function applies to.
+  @param  PeriodicCallback      A pointer to a function of type PERIODIC_CALLBACK that is the main
+                                periodic entry point of the debug agent.
+                                
+  @retval EFI_SUCCESS           The function completed successfully.  
+  @retval EFI_ALREADY_STARTED   Non-NULL PeriodicCallback parameter when a callback
+                                function was previously registered.                
+  @retval EFI_OUT_OF_RESOURCES  System has insufficient memory resources to register new callback                               
+                                function. 
 **/
 EFI_STATUS
 EFIAPI
@@ -165,16 +173,21 @@ RegisterPeriodicCallback (
   );
 
 /**
-  DebugSupport protocol member function.
-
-  @param  This              The DebugSupport instance
-  @param  ProcessorIndex    Which processor the callback applies to.
-  @param  NewCallback       Callback function
-  @param  ExceptionType     Which exception to hook
-
-  @retval EFI_SUCCESS        Indicates the callback was registered.
-  @retval others             Callback was not registered.
-
+  Registers a function to be called when a given processor exception occurs.
+
+  This code executes in boot services context.
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor the callback function applies to.
+  @param  ExceptionCallback     A pointer to a function of type EXCEPTION_CALLBACK that is called
+                                when the processor exception specified by ExceptionType occurs.  
+  @param  ExceptionType         Specifies which processor exception to hook.                       
+                                
+  @retval EFI_SUCCESS           The function completed successfully.  
+  @retval EFI_ALREADY_STARTED   Non-NULL PeriodicCallback parameter when a callback
+                                function was previously registered.                
+  @retval EFI_OUT_OF_RESOURCES  System has insufficient memory resources to register new callback                               
+                                function.
 **/
 EFI_STATUS
 EFIAPI
@@ -186,14 +199,16 @@ RegisterExceptionCallback (
   );
 
 /**
-  DebugSupport protocol member function.  Calls assembly routine to flush cache.
-
-  @param  This              The DebugSupport instance
-  @param  ProcessorIndex    Which processor the callback applies to.
-  @param  Start             Physical base of the memory range to be invalidated
-  @param  Length            mininum number of bytes in instruction cache to invalidate
-
-  @retval EFI_SUCCESS       Always returned.
+  Invalidates processor instruction cache for a memory range. Subsequent execution in this range
+  causes a fresh memory fetch to retrieve code to be executed.                                  
+    
+  @param  This                  A pointer to the EFI_DEBUG_SUPPORT_PROTOCOL instance.
+  @param  ProcessorIndex        Specifies which processor's instruction cache is to be invalidated.
+  @param  Start                 Specifies the physical base of the memory range to be invalidated.                                
+  @param  Length                Specifies the minimum number of bytes in the processor's instruction
+                                cache to invalidate.                                                 
+                                
+  @retval EFI_SUCCESS           Always returned.
 
 **/
 EFI_STATUS
@@ -207,10 +222,12 @@ InvalidateInstructionCache (
 
 /**
   C routine that is called for all registered exceptions.  This is the main
-  exception dispatcher.  Must be public because it's referenced from AsmFuncs.s.
+  exception dispatcher. 
+
+  Must be public because it's referenced from AsmFuncs.s.
 
-  @param  ExceptionType        Exception Type
-  @param  Context              System Context
+  @param  ExceptionType        Specifies which processor exception.
+  @param  Context              System Context.
 **/
 VOID
 CommonHandler (
@@ -221,12 +238,11 @@ CommonHandler (
 /**
   This is the worker function that uninstalls and removes all handlers.
 
-  @param  ExceptionType     Exception Type
-  @param  NewBundles        New Boundles
-  @param  NewCallback       New Callback
+  @param  ExceptionType     Specifies which processor exception.
+  @param  NewBundles        New Boundles.
+  @param  NewCallback       A pointer to the new function to be registered.
 
   @retval EFI_ALEADY_STARTED Ivt already hooked.
-  @retval others             Indicates the request was not satisfied.
   @retval EFI_SUCCESS        Successfully uninstalled.
 
 **/
@@ -241,9 +257,9 @@ ManageIvtEntryTable (
   Saves original IVT contents and inserts a few new bundles which are fixed up
   to store the ExceptionType and then call the common handler.
 
-  @param  ExceptionType      Exception Type
-  @param  NewBundles         New Boundles
-  @param  NewCallback        New Callback
+  @param  ExceptionType      Specifies which processor exception.
+  @param  NewBundles         New Boundles.
+  @param  NewCallback        A pointer to the new function to be hooked.
 
 **/
 VOID
@@ -256,7 +272,7 @@ HookEntry (
 /**
   Restores original IVT contents when unregistering a callback function.
 
-  @param  ExceptionType     Exception Type
+  @param  ExceptionType     Specifies which processor exception.
 
 **/
 VOID
@@ -269,7 +285,7 @@ UnhookEntry (
 
   Records new callback in IvtEntryTable.
 
-  @param  NewCallback     New Callback.
+  @param  NewCallback     A pointer to the interrupt handle.
 
 **/
 VOID
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S b/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S
index 864b6e4112..7ac8a1a69a 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S
+++ b/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.S
@@ -14,7 +14,6 @@
 
 .intel_syntax noprefix
 
-
 .globl ASM_PFX(OrigVector)
 .globl ASM_PFX(InterruptEntryStub)
 .globl ASM_PFX(StubSize)
@@ -146,8 +145,6 @@ ASM_PFX(InterruptEntryStub):
 .globl ASM_PFX(InterruptEntryStubEnd)
 ASM_PFX(InterruptEntryStubEnd):
 
-        ret
-
 #------------------------------------------------------------------------------
 # CommonIdtEntry
 #
@@ -199,9 +196,6 @@ ASM_PFX(InterruptEntryStubEnd):
 ##   UINT64            R8, R9, R10, R11, R12, R13, R14, R15;
 ## } SYSTEM_CONTEXT_X64;  // 64
 ASM_PFX(CommonIdtEntry):
-
-        ret
-
 ## NOTE: we save rsp here to prevent compiler put rip reference cause error AppRsp
                 push    rax
                 mov     rax, qword ptr [rsp][8]          # save vector number
@@ -274,9 +268,10 @@ ExtraPushDone:
                 mov     rax, ASM_PFX(AppRsp)
                 add     rax, 40
                 # application stack has ss, rsp, rflags, cs, & rip, so
-                # last actual application stack entry is
-                # 40 bytes into the application stack.
+                # last actual application stack entry is 40 bytes
+                # into the application stack.
                 mov     [rsp + 24], rax
+
 ## continue building context record
 ## UINT64  Gs, Fs, Es, Ds, Cs, Ss;  insure high 16 bits of each is zero
                 mov     rax, ss
@@ -361,14 +356,14 @@ ExtraPushDone:
                 # IMPORTANT!! The debug stack has been carefully constructed to
                 # insure that rsp and rdi are 16 byte aligned when we get here.
                 # They MUST be.  If they are not, a GP fault will occur.
-           #     FXSTOR_RDI
-               .byte       0x0f
-               .byte       0xae
-               .byte       0x07
+          
+                #     FXSTOR_RDI
+                fxsave      [rdi]
 
 ## UINT64  ExceptionData;
                 mov     rax, ASM_PFX(ExceptData)
                 push    rax
+
 # call to C code which will in turn call registered handler
 # pass in the vector number
                 mov     rdx, rsp
@@ -379,14 +374,16 @@ ExtraPushDone:
 # restore context...
 ## UINT64  ExceptionData;
                 add     rsp, 8
+
 ## FX_SAVE_STATE_X64 FxSaveState;
                 mov     rsi, rsp
-            #    FXRSTOR_RSI
-               .byte       0x0f
-               .byte       0xae
-               .byte       0x0e
+            
+                #    FXRSTOR_RSI
+                fxrstor     [rsi]
+
                 add     rsp, 512
-;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
+
+## UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
                 pop     rax
                 mov     dr0, rax
                 pop     rax
@@ -395,10 +392,12 @@ ExtraPushDone:
                 mov     dr2, rax
                 pop     rax
                 mov     dr3, rax
+
 ## skip restore of dr6.  We cleared dr6 during the context save.
                 add     rsp, 8
                 pop     rax
                 mov     dr7, rax
+
 ## UINT64  Cr0, Cr1, Cr2, Cr3, Cr4, Cr8;
                 pop     rax
                 mov     cr0, rax
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm b/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm
index d9ba0be191..6b047066ca 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm
+++ b/MdeModulePkg/Universal/DebugSupportDxe/X64/AsmFuncs.asm
@@ -1,7 +1,7 @@
 ;/** @file
 ;  Low level x64 routines used by the debug support driver.
 ;
-;  Copyright (c) 2007 - 2008, Intel Corporation. <BR>
+;  Copyright (c) 2007 - 2008, Intel Corporation.
 ;  All rights reserved. This program and the accompanying materials
 ;  are licensed and made available under the terms and conditions of the BSD License
 ;  which accompanies this distribution.  The full text of the license may be found at
diff --git a/MdeModulePkg/Universal/DebugSupportDxe/X64/PlDebugSupportX64.c b/MdeModulePkg/Universal/DebugSupportDxe/X64/PlDebugSupportX64.c
index d6173bb12f..3143bafee7 100644
--- a/MdeModulePkg/Universal/DebugSupportDxe/X64/PlDebugSupportX64.c
+++ b/MdeModulePkg/Universal/DebugSupportDxe/X64/PlDebugSupportX64.c
@@ -1,7 +1,7 @@
 /** @file
-  X64 specific debug support functions
+  X64 specific functions to support Debug Support protocol.
 
-Copyright (c) 2006 - 2007, Intel Corporation
+Copyright (c) 2008, Intel Corporation
 All rights reserved. This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD License
 which accompanies this distribution.  The full text of the license may be found at
@@ -103,7 +103,7 @@ CreateEntryStub (
 **/
 EFI_STATUS
 ManageIdtEntryTable (
-  VOID (*NewCallback)(),
+  VOID               (*NewCallback)(),
   EFI_EXCEPTION_TYPE ExceptionType
   )
 {