[PATCH 1/2] CpuExceptionHandlerLib: Add comments to make code more readable

[Ruiyu Ni <ruiyu.ni@intel.com>]

Today's implementation of handling HOOK_BEFORE and HOOK_AFTER is
a bit complex. More comments is better.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Fan Jeff <vanjeff_919@hotmail.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
 .../CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c       |  8 +++++---
 .../Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c   | 12 ++++++++----
 .../CpuExceptionHandlerLib/X64/ArchExceptionHandler.c        |  8 +++++---
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index 04f2ab593c..031d0d35fa 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
 /** @file
   IA32 CPU Exception Handler functons.
 
-  Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
   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
@@ -66,7 +66,9 @@ ArchSaveExceptionContext (
 
   ReservedVectors = ExceptionHandlerData->ReservedVectors;
   //
-  // Save Exception context in global variable
+  // Save Exception context in global variable in first entry of the exception handler.
+  // So when original exception handler returns to the new exception handler (second entry),
+  // the Eflags/Cs/Eip/ExceptionData can be used.
   //
   ReservedVectors[ExceptionType].OldFlags      = SystemContext.SystemContextIa32->Eflags;
   ReservedVectors[ExceptionType].OldCs         = SystemContext.SystemContextIa32->Cs;
@@ -79,7 +81,7 @@ ArchSaveExceptionContext (
   Eflags.Bits.IF = 0;
   SystemContext.SystemContextIa32->Eflags = Eflags.UintN;
   //
-  // Modify the EIP in stack, then old IDT handler will return to the stub code
+  // Modify the EIP in stack, then old IDT handler will return to HookAfterStubBegin.
   //
   SystemContext.SystemContextIa32->Eip    = (UINTN) ReservedVectors[ExceptionType].HookAfterStubHeaderCode;
 }
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
index 1a382e88fb..64db593194 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
@@ -40,7 +40,8 @@ CommonExceptionHandlerWorker (
   switch (ReservedVectors[ExceptionType].Attribute) {
   case EFI_VECTOR_HANDOFF_HOOK_BEFORE:
     //
-    // Need to jmp to old IDT handler after this exception handler
+    // The new exception handler registered by RegisterCpuInterruptHandler() is executed BEFORE original handler.
+    // Save the original handler to stack so the assembly code can jump to it instead of returning from handler.
     //
     ExceptionHandlerContext->ExceptionDataFlag = (mErrorCodeFlag & (1 << ExceptionType)) ? TRUE : FALSE;
     ExceptionHandlerContext->OldIdtHandler     = ReservedVectors[ExceptionType].ExceptonHandler;
@@ -48,11 +49,13 @@ CommonExceptionHandlerWorker (
   case EFI_VECTOR_HANDOFF_HOOK_AFTER:
     while (TRUE) {
       //
-      // If if anyone has gotten SPIN_LOCK for owner running hook after
+      // If spin-lock can be acquired, it's the first time entering here.
       //
       if (AcquireSpinLockOrFail (&ReservedVectors[ExceptionType].SpinLock)) {
         //
-        // Need to execute old IDT handler before running this exception handler
+        // The new exception handler registered by RegisterCpuInterruptHandler() is executed AFTER original handler.
+        // Save the original handler to stack but skip running the new handler so the original handler is executed
+        // firstly.
         //
         ReservedVectors[ExceptionType].ApicId = GetApicId ();
         ArchSaveExceptionContext (ExceptionType, SystemContext, ExceptionHandlerData);
@@ -61,7 +64,8 @@ CommonExceptionHandlerWorker (
         return;
       }
       //
-      // If failed to acquire SPIN_LOCK, check if it was locked by processor itself
+      // If spin-lock cannot be acquired, it's the second time entering here.
+      // 'break' instead of 'return' is used so the new exception handler can be executed.
       //
       if (ReservedVectors[ExceptionType].ApicId == GetApicId ()) {
         //
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 56180f4c17..93ecf5ae5a 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
 /** @file
   x64 CPU Exception Handler.
 
-  Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
   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
@@ -67,7 +67,9 @@ ArchSaveExceptionContext (
 
   ReservedVectors = ExceptionHandlerData->ReservedVectors;
   //
-  // Save Exception context in global variable
+  // Save Exception context in global variable in first entry of the exception handler.
+  // So when original exception handler returns to the new exception handler (second entry),
+  // the Eflags/Cs/Eip/ExceptionData can be used.
   //
   ReservedVectors[ExceptionType].OldSs         = SystemContext.SystemContextX64->Ss;
   ReservedVectors[ExceptionType].OldSp         = SystemContext.SystemContextX64->Rsp;
@@ -82,7 +84,7 @@ ArchSaveExceptionContext (
   Eflags.Bits.IF = 0;
   SystemContext.SystemContextX64->Rflags = Eflags.UintN;
   //
-  // Modify the EIP in stack, then old IDT handler will return to the stub code
+  // Modify the EIP in stack, then old IDT handler will return to HookAfterStubBegin.
   //
   SystemContext.SystemContextX64->Rip = (UINTN) ReservedVectors[ExceptionType].HookAfterStubHeaderCode;
 }
-- 
2.16.1.windows.1