From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Permerror (SPF Permanent Error: Two or more type TXT spf records found.) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 0C1BA21109FFE for ; Fri, 31 Aug 2018 01:35:36 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Aug 2018 01:35:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,309,1531810800"; d="scan'208";a="258748751" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.8]) by fmsmga005.fm.intel.com with ESMTP; 31 Aug 2018 01:35:35 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Date: Fri, 31 Aug 2018 16:36:19 +0800 Message-Id: <20180831083620.303688-2-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 In-Reply-To: <20180831083620.303688-1-ruiyu.ni@intel.com> References: <20180831083620.303688-1-ruiyu.ni@intel.com> Subject: [PATCH 1/2] CpuExceptionHandlerLib: Add comments to make code more readable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 31 Aug 2018 08:35:36 -0000 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 Cc: Fan Jeff Cc: Jian J Wang --- .../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.
+ Copyright (c) 2012 - 2018, 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 @@ -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.
+ Copyright (c) 2012 - 2018, 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 @@ -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