Re: [PATCH v9 11/46] UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong <eric.dong@intel.com>

Thanks,
Eric

> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Friday, June 5, 2020 9:27 PM
> To: devel@edk2.groups.io
> Cc: Brijesh Singh <brijesh.singh@amd.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Dong, Eric <eric.dong@intel.com>; Justen,
> Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>;
> Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v9 11/46] UefiCpuPkg/CpuExceptionHandler: Add base
> support for the #VC exception
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> Add base support to handle #VC exceptions. Update the common exception
> handlers to invoke the VmgExitHandleVc () function of the VmgExitLib
> library when a #VC is encountered. A non-zero return code will propagate
> to the targeted exception handler.
> 
> Under SEV-ES, a DR7 read or write intercept generates a #VC exception.
> To avoid exception recursion, a #VC exception will not try to read and
> push the actual debug registers into the EFI_SYSTEM_CONTEXT_X64 struct
> and instead push zeroes. The #VC exception handler does not make use of
> the debug registers from the saved context and the exception processing
> exit code does not attempt to restore the debug register values.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.in
> f          |  1 +
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
> |  1 +
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib
> .inf       |  1 +
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.i
> nf          |  1 +
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuExceptionHa
> ndlerLib.inf |  1 +
>  UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> | 10 +++++++++-
>  UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
> | 20 +++++++++++++++++++-
>  UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> | 19 +++++++++++++++++++
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> m           | 17 +++++++++++++++++
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandlerA
> sm.nasm     | 17 +++++++++++++++++
>  10 files changed, 86 insertions(+), 2 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.
> inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib
> .inf
> index 61e2ec30b089..07b34c92a892 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.
> inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib
> .inf
> @@ -57,3 +57,4 @@ [LibraryClasses]
>    PeCoffGetEntryPointLib
> 
>    MemoryAllocationLib
> 
>    DebugLib
> 
> +  VmgExitLib
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> index 093374944df6..feae7b3e06de 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.i
> nf
> @@ -52,6 +52,7 @@ [LibraryClasses]
>    HobLib
> 
>    MemoryAllocationLib
> 
>    SynchronizationLib
> 
> +  VmgExitLib
> 
> 
> 
>  [Pcd]
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard    # CONSUMES
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> index 6d25cafe2ca3..967cb61ba6d9 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandler
> Lib.inf
> @@ -48,3 +48,4 @@ [LibraryClasses]
>    PrintLib
> 
>    LocalApicLib
> 
>    PeCoffGetEntryPointLib
> 
> +  VmgExitLib
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> index 2ffbbccc302f..4cdb11c04ea0 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLi
> b.inf
> @@ -51,4 +51,5 @@ [LibraryClasses]
>    LocalApicLib
> 
>    PeCoffGetEntryPointLib
> 
>    DebugLib
> 
> +  VmgExitLib
> 
> 
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> index 7e21beaab6f2..743c2aa76684 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Xcode5SecPeiCpuException
> HandlerLib.inf
> @@ -53,3 +53,4 @@ [LibraryClasses]
>    PrintLib
> 
>    LocalApicLib
> 
>    PeCoffGetEntryPointLib
> 
> +  VmgExitLib
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> index 8adbd43fefb4..c9003b10e552 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> @@ -14,7 +14,7 @@
>  //
> 
>  // 1 means an error code will be pushed, otherwise 0
> 
>  //
> 
> -CONST UINT32 mErrorCodeFlag = 0x00227d00;
> 
> +CONST UINT32 mErrorCodeFlag = 0x20227d00;
> 
> 
> 
>  //
> 
>  // Define the maximum message length
> 
> @@ -45,6 +45,14 @@ CONST CHAR8 *mExceptionNameStr[] = {
>    "#XM - SIMD floating-point",
> 
>    "#VE - Virtualization",
> 
>    "#CP - Control Protection"
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "Reserved",
> 
> +  "#VC - VMM Communication",
> 
>  };
> 
> 
> 
>  #define EXCEPTION_KNOWN_NAME_NUM  (sizeof (mExceptionNameStr) /
> sizeof (CHAR8 *))
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
> index 6a2670d55918..892d349d4b37 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c
> @@ -6,8 +6,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  **/
> 
> 
> 
> -#include "CpuExceptionCommon.h"
> 
>  #include <Library/DebugLib.h>
> 
> +#include <Library/VmgExitLib.h>
> 
> +#include "CpuExceptionCommon.h"
> 
> 
> 
>  /**
> 
>    Internal worker function for common exception handler.
> 
> @@ -27,6 +28,23 @@ CommonExceptionHandlerWorker (
>    RESERVED_VECTORS_DATA          *ReservedVectors;
> 
>    EFI_CPU_INTERRUPT_HANDLER      *ExternalInterruptHandler;
> 
> 
> 
> +  if (ExceptionType == VC_EXCEPTION) {
> 
> +    EFI_STATUS  Status;
> 
> +    //
> 
> +    // #VC needs to be handled immediately upon enabling exception
> handling
> 
> +    // and therefore can't use the RegisterCpuInterruptHandler() interface.
> 
> +    //
> 
> +    // Handle the #VC:
> 
> +    //   On EFI_SUCCESS - Exception has been handled, return
> 
> +    //   On other       - ExceptionType contains (possibly new) exception
> 
> +    //                    value
> 
> +    //
> 
> +    Status = VmgExitHandleVc (&ExceptionType, SystemContext);
> 
> +    if (!EFI_ERROR (Status)) {
> 
> +      return;
> 
> +    }
> 
> +  }
> 
> +
> 
>    ExceptionHandlerContext  = (EXCEPTION_HANDLER_CONTEXT *) (UINTN)
> (SystemContext.SystemContextIa32);
> 
>    ReservedVectors          = ExceptionHandlerData->ReservedVectors;
> 
>    ExternalInterruptHandler = ExceptionHandlerData-
> >ExternalInterruptHandler;
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> index 20148db74cf8..52c6886f0372 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> @@ -7,6 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  **/
> 
> 
> 
>  #include <PiPei.h>
> 
> +#include <Library/VmgExitLib.h>
> 
>  #include "CpuExceptionCommon.h"
> 
> 
> 
>  CONST UINTN    mDoFarReturnFlag  = 0;
> 
> @@ -24,6 +25,24 @@ CommonExceptionHandler (
>    IN EFI_SYSTEM_CONTEXT   SystemContext
> 
>    )
> 
>  {
> 
> +  if (ExceptionType == VC_EXCEPTION) {
> 
> +    EFI_STATUS  Status;
> 
> +    //
> 
> +    // #VC needs to be handled immediately upon enabling exception
> handling
> 
> +    // and therefore can't use the RegisterCpuInterruptHandler() interface
> 
> +    // (which isn't supported under Sec and Pei anyway).
> 
> +    //
> 
> +    // Handle the #VC:
> 
> +    //   On EFI_SUCCESS - Exception has been handled, return
> 
> +    //   On other       - ExceptionType contains (possibly new) exception
> 
> +    //                    value
> 
> +    //
> 
> +    Status = VmgExitHandleVc (&ExceptionType, SystemContext);
> 
> +    if (!EFI_ERROR (Status)) {
> 
> +      return;
> 
> +    }
> 
> +  }
> 
> +
> 
>    //
> 
>    // Initialize the serial port before dumping.
> 
>    //
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> index 3814f9de3703..2a5545ecfd41 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> @@ -18,6 +18,8 @@
>  ; CommonExceptionHandler()
> 
>  ;
> 
> 
> 
> +%define VC_EXCEPTION 29
> 
> +
> 
>  extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
> 
>  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag
> 
>  extern ASM_PFX(CommonExceptionHandler)
> 
> @@ -224,6 +226,9 @@ HasErrorCode:
>      push    rax
> 
> 
> 
>  ;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
> 
> +    cmp     qword [rbp + 8], VC_EXCEPTION
> 
> +    je      VcDebugRegs          ; For SEV-ES (#VC) Debug registers ignored
> 
> +
> 
>      mov     rax, dr7
> 
>      push    rax
> 
>      mov     rax, dr6
> 
> @@ -236,7 +241,19 @@ HasErrorCode:
>      push    rax
> 
>      mov     rax, dr0
> 
>      push    rax
> 
> +    jmp     DrFinish
> 
> 
> 
> +VcDebugRegs:
> 
> +;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception
> recursion
> 
> +    xor     rax, rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +
> 
> +DrFinish:
> 
>  ;; FX_SAVE_STATE_X64 FxSaveState;
> 
>      sub rsp, 512
> 
>      mov rdi, rsp
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> index 19198f273137..26cae56cc5cf 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/Xcode5ExceptionHandle
> rAsm.nasm
> @@ -18,6 +18,8 @@
>  ; CommonExceptionHandler()
> 
>  ;
> 
> 
> 
> +%define VC_EXCEPTION 29
> 
> +
> 
>  extern ASM_PFX(mErrorCodeFlag)    ; Error code flags for exceptions
> 
>  extern ASM_PFX(mDoFarReturnFlag)  ; Do far return flag
> 
>  extern ASM_PFX(CommonExceptionHandler)
> 
> @@ -225,6 +227,9 @@ HasErrorCode:
>      push    rax
> 
> 
> 
>  ;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
> 
> +    cmp     qword [rbp + 8], VC_EXCEPTION
> 
> +    je      VcDebugRegs          ; For SEV-ES (#VC) Debug registers ignored
> 
> +
> 
>      mov     rax, dr7
> 
>      push    rax
> 
>      mov     rax, dr6
> 
> @@ -237,7 +242,19 @@ HasErrorCode:
>      push    rax
> 
>      mov     rax, dr0
> 
>      push    rax
> 
> +    jmp     DrFinish
> 
> 
> 
> +VcDebugRegs:
> 
> +;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7 are skipped for #VC to avoid exception
> recursion
> 
> +    xor     rax, rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +    push    rax
> 
> +
> 
> +DrFinish:
> 
>  ;; FX_SAVE_STATE_X64 FxSaveState;
> 
>      sub rsp, 512
> 
>      mov rdi, rsp
> 
> --
> 2.27.0