Re: [PATCH] UefiCpuPkg CpuExceptionHandlerLib: Enhance DumpModuleImageInfo()

["Yao, Jiewen" <jiewen.yao@intel.com>]

reviewed by jiewen.yao@intel.com

thank you!
Yao, Jiewen


> 在 2017年12月27日，下午7:07，Zeng, Star <star.zeng@intel.com> 写道：
> 
> Enhance DumpModuleImageInfo() for page fault with I/D set.
> 
> If it is page fault with I/D set, the (E/R)IP in SystemContext
> could not be used for DumpModuleImageInfo(), instead of, the next
> IP of the IP triggering this page fault could be found from stack
> by (E/R)SP in SystemContext.
> 
> IA32 SDM:
> ― I/D flag (bit 4).
> This flag is 1 if the access causing the page-fault exception was
> an instruction fetch. This flag describes the access causing the
> page-fault exception, not the access rights specified by paging.
> 
> The idea comes from SmiPFHandler () in
> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c and
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
> .../Library/CpuExceptionHandlerLib/CpuExceptionCommon.c       |  4 ++--
> .../CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c        | 11 ++++++++++-
> .../Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 11 ++++++++++-
> 3 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> index dbfaae1d3038..01b06103647b 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> @@ -1,7 +1,7 @@
> /** @file
>   CPU Exception Handler Library common functions.
> 
> -  Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2012 - 2017, 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
> @@ -131,7 +131,7 @@ DumpModuleImageInfo (
>     if (EFI_ERROR (Status)) {
>       EntryPoint = NULL;
>     }
> -    InternalPrintMessage ("!!!! Find image ");
> +    InternalPrintMessage ("!!!! Find image based on IP(0x%x) ", CurrentEip);
>     PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
>     if (PdbPointer != NULL) {
>       InternalPrintMessage ("%a", PdbPointer);
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> index 6ac8549839ce..04f2ab593c3e 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> @@ -414,5 +414,14 @@ DumpImageAndCpuContent (
>   //
>   // Dump module image base and module entry point by EIP
>   //
> -  DumpModuleImageInfo (SystemContext.SystemContextIa32->Eip);
> +  if ((ExceptionType == EXCEPT_IA32_PAGE_FAULT) &&
> +      ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0)) {
> +    //
> +    // The EIP in SystemContext could not be used
> +    // if it is page fault with I/D set.
> +    //
> +    DumpModuleImageInfo ((*(UINTN *)(UINTN)SystemContext.SystemContextIa32->Esp));
> +  } else {
> +    DumpModuleImageInfo (SystemContext.SystemContextIa32->Eip);
> +  }
> }
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> index 1dcf4277dea9..56180f4c17e4 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
> @@ -414,5 +414,14 @@ DumpImageAndCpuContent (
>   //
>   // Dump module image base and module entry point by RIP
>   //
> -  DumpModuleImageInfo (SystemContext.SystemContextX64->Rip);
> +  if ((ExceptionType == EXCEPT_IA32_PAGE_FAULT) &&
> +      ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0)) {
> +    //
> +    // The RIP in SystemContext could not be used
> +    // if it is page fault with I/D set.
> +    //
> +    DumpModuleImageInfo ((*(UINTN *)(UINTN)SystemContext.SystemContextX64->Rsp));
> +  } else {
> +    DumpModuleImageInfo (SystemContext.SystemContextX64->Rip);
> +  }
> }
> -- 
> 2.7.0.windows.1
>