Re: [RFC PATCH v3 22/43] UefiCpuPkg/CpuExceptionHandler: Add support for DR7 Read/Write NAE events

["Ni, Ray" <ray.ni@intel.com>]

Tom,
Why all DR registers are not pushed to stack in VC handler?
I thought only DR7 pushing is skipped.

Thanks,
Ray

> -----Original Message-----
> From: Tom Lendacky <thomas.lendacky@amd.com>
> Sent: Thursday, November 21, 2019 4:07 AM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Dong,
> Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>
> Subject: [RFC PATCH v3 22/43] UefiCpuPkg/CpuExceptionHandler: Add support for DR7 Read/Write NAE events
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> Under SEV-ES, a DR7 read or write intercept generates a #VC exception.
> The #VC handler must provide special support to the guest for this. On
> a DR7 write, the #VC handler must cache the value and issue a VMGEXIT
> to notify the hypervisor of the write. However, the #VC handler must
> not actually set the value of the DR7 register. On a DR7 read, the #VC
> handler must return the cached value of the DR7 register to the guest.
> VMGEXIT is not invoked for a DR7 register read.
> 
> 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 saved context.
> 
> 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>
> ---
>  .../X64/AMDSevVcCommon.c                      | 68 +++++++++++++++++++
>  .../X64/ExceptionHandlerAsm.nasm              | 15 ++++
>  2 files changed, 83 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/AMDSevVcCommon.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/AMDSevVcCommon.c
> index 1d7c34e7e442..22393f72d795 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/AMDSevVcCommon.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/AMDSevVcCommon.c
> @@ -13,6 +13,12 @@
> 
>  #define CR4_OSXSAVE (1 << 18)
> 
> +#define DR7_RESET_VALUE 0x400
> +typedef struct {
> +  BOOLEAN  Dr7Cached;
> +  UINT64   Dr7;
> +} SEV_ES_PER_CPU_DATA;
> +
>  typedef enum {
>    LongMode64Bit        = 0,
>    LongModeCompat32Bit,
> @@ -1081,6 +1087,60 @@ RdtscExit (
>    return 0;
>  }
> 
> +STATIC
> +UINTN
> +Dr7WriteExit (
> +  GHCB                     *Ghcb,
> +  EFI_SYSTEM_CONTEXT_X64   *Regs,
> +  SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  )
> +{
> +  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext = &InstructionData->Ext;
> +  SEV_ES_PER_CPU_DATA            *SevEsData = (SEV_ES_PER_CPU_DATA *) (Ghcb + 1);
> +  INTN                           *Register;
> +  UINTN                          Status;
> +
> +  DecodeModRm (Regs, InstructionData);
> +
> +  /* MOV DRn always treats MOD == 3 no matter how encoded */
> +  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
> +
> +  /* Using a value of 0 for ExitInfo1 means RAX holds the value */
> +  Ghcb->SaveArea.Rax = *Register;
> +  GhcbSetRegValid (Ghcb, GhcbRax);
> +
> +  Status = VmgExit (Ghcb, SvmExitDr7Write, 0, 0);
> +  if (Status) {
> +    return Status;
> +  }
> +
> +  SevEsData->Dr7 = *Register;
> +  SevEsData->Dr7Cached = TRUE;
> +
> +  return 0;
> +}
> +
> +STATIC
> +UINTN
> +Dr7ReadExit (
> +  GHCB                     *Ghcb,
> +  EFI_SYSTEM_CONTEXT_X64   *Regs,
> +  SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  )
> +{
> +  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext = &InstructionData->Ext;
> +  SEV_ES_PER_CPU_DATA            *SevEsData = (SEV_ES_PER_CPU_DATA *) (Ghcb + 1);
> +  INTN                           *Register;
> +
> +  DecodeModRm (Regs, InstructionData);
> +
> +  /* MOV DRn always treats MOD == 3 no matter how encoded */
> +  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
> +  *Register = (SevEsData->Dr7Cached) ? SevEsData->Dr7 : DR7_RESET_VALUE;
> +
> +  return 0;
> +}
> +
>  UINTN
>  DoVcCommon (
>    GHCB                *Ghcb,
> @@ -1097,6 +1157,14 @@ DoVcCommon (
> 
>    ExitCode = Regs->ExceptionData;
>    switch (ExitCode) {
> +  case SvmExitDr7Read:
> +    NaeExit = Dr7ReadExit;
> +    break;
> +
> +  case SvmExitDr7Write:
> +    NaeExit = Dr7WriteExit;
> +    break;
> +
>    case SvmExitRdtsc:
>      NaeExit = RdtscExit;
>      break;
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> index 19198f273137..a0549f7ae6bd 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm
> @@ -225,6 +225,9 @@ HasErrorCode:
>      push    rax
> 
>  ;; UINT64  Dr0, Dr1, Dr2, Dr3, Dr6, Dr7;
> +    cmp     qword [rbp + 8], 29
> +    je      VcDebugRegs          ; For SEV-ES (#VC) Debug registers ignored
> +
>      mov     rax, dr7
>      push    rax
>      mov     rax, dr6
> @@ -237,7 +240,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.17.1