CpuExceptionHandlerLib: Add comments to make code more readable

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>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index 04f2ab593c3e2b64a0f5ec728a04b7a356251c21..031d0d35fab930eb6fbdf84229c48ad2e7191b5a 100644 (file)
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
 /** @file\r
   IA32 CPU Exception Handler functons.\r
 \r
-  Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>\r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
   which accompanies this distribution.  The full text of the license may be found at\r
@@ -66,7 +66,9 @@ ArchSaveExceptionContext (
 \r
   ReservedVectors = ExceptionHandlerData->ReservedVectors;\r
   //\r
-  // Save Exception context in global variable\r
+  // Save Exception context in global variable in first entry of the exception handler.\r
+  // So when original exception handler returns to the new exception handler (second entry),\r
+  // the Eflags/Cs/Eip/ExceptionData can be used.\r
   //\r
   ReservedVectors[ExceptionType].OldFlags      = SystemContext.SystemContextIa32->Eflags;\r
   ReservedVectors[ExceptionType].OldCs         = SystemContext.SystemContextIa32->Cs;\r
@@ -79,7 +81,7 @@ ArchSaveExceptionContext (
   Eflags.Bits.IF = 0;\r
   SystemContext.SystemContextIa32->Eflags = Eflags.UintN;\r
   //\r
-  // Modify the EIP in stack, then old IDT handler will return to the stub code\r
+  // Modify the EIP in stack, then old IDT handler will return to HookAfterStubBegin.\r
   //\r
   SystemContext.SystemContextIa32->Eip    = (UINTN) ReservedVectors[ExceptionType].HookAfterStubHeaderCode;\r
 }\r

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
index 1a382e88fb9c3ac1a92166bd3f023c3f94bcf881..64db593194da9e919a94d3d27d2cdb6828e55bc1 100644 (file)
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
@@ -40,7 +40,8 @@ CommonExceptionHandlerWorker (
   switch (ReservedVectors[ExceptionType].Attribute) {\r
   case EFI_VECTOR_HANDOFF_HOOK_BEFORE:\r
     //\r
-    // Need to jmp to old IDT handler after this exception handler\r
+    // The new exception handler registered by RegisterCpuInterruptHandler() is executed BEFORE original handler.\r
+    // Save the original handler to stack so the assembly code can jump to it instead of returning from handler.\r
     //\r
     ExceptionHandlerContext->ExceptionDataFlag = (mErrorCodeFlag & (1 << ExceptionType)) ? TRUE : FALSE;\r
     ExceptionHandlerContext->OldIdtHandler     = ReservedVectors[ExceptionType].ExceptonHandler;\r
@@ -48,11 +49,13 @@ CommonExceptionHandlerWorker (
   case EFI_VECTOR_HANDOFF_HOOK_AFTER:\r
     while (TRUE) {\r
       //\r
-      // If if anyone has gotten SPIN_LOCK for owner running hook after\r
+      // If spin-lock can be acquired, it's the first time entering here.\r
       //\r
       if (AcquireSpinLockOrFail (&ReservedVectors[ExceptionType].SpinLock)) {\r
         //\r
-        // Need to execute old IDT handler before running this exception handler\r
+        // The new exception handler registered by RegisterCpuInterruptHandler() is executed AFTER original handler.\r
+        // Save the original handler to stack but skip running the new handler so the original handler is executed\r
+        // firstly.\r
         //\r
         ReservedVectors[ExceptionType].ApicId = GetApicId ();\r
         ArchSaveExceptionContext (ExceptionType, SystemContext, ExceptionHandlerData);\r
@@ -61,7 +64,8 @@ CommonExceptionHandlerWorker (
         return;\r
       }\r
       //\r
-      // If failed to acquire SPIN_LOCK, check if it was locked by processor itself\r
+      // If spin-lock cannot be acquired, it's the second time entering here.\r
+      // 'break' instead of 'return' is used so the new exception handler can be executed.\r
       //\r
       if (ReservedVectors[ExceptionType].ApicId == GetApicId ()) {\r
         //\r

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 56180f4c17e49bd42396792c8c9be9f8a3800db8..93ecf5ae5a5b9d6deb480b0dfa664a7661519365 100644 (file)
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
 /** @file\r
   x64 CPU Exception Handler.\r
 \r
-  Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.<BR>\r
+  Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>\r
   This program and the accompanying materials\r
   are licensed and made available under the terms and conditions of the BSD License\r
   which accompanies this distribution.  The full text of the license may be found at\r
@@ -67,7 +67,9 @@ ArchSaveExceptionContext (
 \r
   ReservedVectors = ExceptionHandlerData->ReservedVectors;\r
   //\r
-  // Save Exception context in global variable\r
+  // Save Exception context in global variable in first entry of the exception handler.\r
+  // So when original exception handler returns to the new exception handler (second entry),\r
+  // the Eflags/Cs/Eip/ExceptionData can be used.\r
   //\r
   ReservedVectors[ExceptionType].OldSs         = SystemContext.SystemContextX64->Ss;\r
   ReservedVectors[ExceptionType].OldSp         = SystemContext.SystemContextX64->Rsp;\r
@@ -82,7 +84,7 @@ ArchSaveExceptionContext (
   Eflags.Bits.IF = 0;\r
   SystemContext.SystemContextX64->Rflags = Eflags.UintN;\r
   //\r
-  // Modify the EIP in stack, then old IDT handler will return to the stub code\r
+  // Modify the EIP in stack, then old IDT handler will return to HookAfterStubBegin.\r
   //\r
   SystemContext.SystemContextX64->Rip = (UINTN) ReservedVectors[ExceptionType].HookAfterStubHeaderCode;\r
 }\r