public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH V1 0/2] Refactor TDX MmioExit
@ 2022-12-29  8:55 Min Xu
  2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Min Xu @ 2022-12-29  8:55 UTC (permalink / raw)
  To: devel
  Cc: Min Xu, Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
	Tom Lendacky, Ryan Afranji

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4169

The previous TDX MmioExit doesn't handle the Mmio instructions correctly
in some scenarios. This patch-set refactors the implementation to fix the
issues.

Before the refactoring, common X86 instruction codes in CcExitVcHandler.c
are moved to separate files (CcInstruction.h / CcInstruction.c) so that
these codes can be re-used in TDX.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ryan Afranji <afranji@google.com>
Reported-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>

Min M Xu (2):
  OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
  OvmfPkg/CcExitLib: Refactor TDX MmioExit

 OvmfPkg/Library/CcExitLib/CcExitLib.inf     |   1 +
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 697 +++-----------------
 OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498 +++++++++-----
 OvmfPkg/Library/CcExitLib/CcInstruction.c   | 454 +++++++++++++
 OvmfPkg/Library/CcExitLib/CcInstruction.h   | 197 ++++++
 OvmfPkg/Library/CcExitLib/SecCcExitLib.inf  |   1 +
 6 files changed, 1082 insertions(+), 766 deletions(-)
 create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.c
 create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.h

-- 
2.29.2.windows.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
  2022-12-29  8:55 [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
@ 2022-12-29  8:55 ` Min Xu
  2023-01-03 20:27   ` Lendacky, Thomas
  2023-01-06  8:24   ` Yao, Jiewen
  2022-12-29  8:55 ` [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
  2022-12-29  9:00 ` [PATCH V1 0/2] " Min Xu
  2 siblings, 2 replies; 8+ messages in thread
From: Min Xu @ 2022-12-29  8:55 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Gerd Hoffmann, Erdem Aktas, James Bottomley, Jiewen Yao,
	Tom Lendacky

From: Min M Xu <min.m.xu@intel.com>

https://bugzilla.tianocore.org/show_bug.cgi?id=4169

Move common X86 instruction codes from CcExitVcHandler.c to separate
files (CcInstruction.h / CcInstruction.c) so that these codes can be
re-used in TDX.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Library/CcExitLib/CcExitLib.inf     |   1 +
 OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 697 +++-----------------
 OvmfPkg/Library/CcExitLib/CcInstruction.c   | 454 +++++++++++++
 OvmfPkg/Library/CcExitLib/CcInstruction.h   | 197 ++++++
 OvmfPkg/Library/CcExitLib/SecCcExitLib.inf  |   1 +
 5 files changed, 735 insertions(+), 615 deletions(-)
 create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.c
 create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.h

diff --git a/OvmfPkg/Library/CcExitLib/CcExitLib.inf b/OvmfPkg/Library/CcExitLib/CcExitLib.inf
index 131fa6267522..bc75cd5f5a04 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitLib.inf
+++ b/OvmfPkg/Library/CcExitLib/CcExitLib.inf
@@ -25,6 +25,7 @@
   CcExitLib.c
   CcExitVcHandler.c
   CcExitVcHandler.h
+  CcInstruction.c
   PeiDxeCcExitVcHandler.c
   CcExitVeHandler.c
   X64/TdVmcallCpuid.nasm
diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
index 985e5479775c..7fe11c53249e 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
@@ -17,107 +17,7 @@
 #include <IndustryStandard/InstructionParsing.h>
 
 #include "CcExitVcHandler.h"
-
-//
-// Instruction execution mode definition
-//
-typedef enum {
-  LongMode64Bit = 0,
-  LongModeCompat32Bit,
-  LongModeCompat16Bit,
-} SEV_ES_INSTRUCTION_MODE;
-
-//
-// Instruction size definition (for operand and address)
-//
-typedef enum {
-  Size8Bits = 0,
-  Size16Bits,
-  Size32Bits,
-  Size64Bits,
-} SEV_ES_INSTRUCTION_SIZE;
-
-//
-// Intruction segment definition
-//
-typedef enum {
-  SegmentEs = 0,
-  SegmentCs,
-  SegmentSs,
-  SegmentDs,
-  SegmentFs,
-  SegmentGs,
-} SEV_ES_INSTRUCTION_SEGMENT;
-
-//
-// Instruction rep function definition
-//
-typedef enum {
-  RepNone = 0,
-  RepZ,
-  RepNZ,
-} SEV_ES_INSTRUCTION_REP;
-
-typedef struct {
-  UINT8    Rm;
-  UINT8    Reg;
-  UINT8    Mod;
-} SEV_ES_INSTRUCTION_MODRM_EXT;
-
-typedef struct {
-  UINT8    Base;
-  UINT8    Index;
-  UINT8    Scale;
-} SEV_ES_INSTRUCTION_SIB_EXT;
-
-//
-// Instruction opcode definition
-//
-typedef struct {
-  SEV_ES_INSTRUCTION_MODRM_EXT    ModRm;
-
-  SEV_ES_INSTRUCTION_SIB_EXT      Sib;
-
-  UINTN                           RegData;
-  UINTN                           RmData;
-} SEV_ES_INSTRUCTION_OPCODE_EXT;
-
-//
-// Instruction parsing context definition
-//
-typedef struct {
-  GHCB                             *Ghcb;
-
-  SEV_ES_INSTRUCTION_MODE          Mode;
-  SEV_ES_INSTRUCTION_SIZE          DataSize;
-  SEV_ES_INSTRUCTION_SIZE          AddrSize;
-  BOOLEAN                          SegmentSpecified;
-  SEV_ES_INSTRUCTION_SEGMENT       Segment;
-  SEV_ES_INSTRUCTION_REP           RepMode;
-
-  UINT8                            *Begin;
-  UINT8                            *End;
-
-  UINT8                            *Prefixes;
-  UINT8                            *OpCodes;
-  UINT8                            *Displacement;
-  UINT8                            *Immediate;
-
-  INSTRUCTION_REX_PREFIX           RexPrefix;
-
-  BOOLEAN                          ModRmPresent;
-  INSTRUCTION_MODRM                ModRm;
-
-  BOOLEAN                          SibPresent;
-  INSTRUCTION_SIB                  Sib;
-
-  UINTN                            PrefixSize;
-  UINTN                            OpCodeSize;
-  UINTN                            DisplacementSize;
-  UINTN                            ImmediateSize;
-
-  SEV_ES_INSTRUCTION_OPCODE_EXT    Ext;
-} SEV_ES_INSTRUCTION_DATA;
+#include "CcInstruction.h"
 
 //
 // Non-automatic Exit function prototype
@@ -125,9 +25,9 @@ typedef struct {
 typedef
 UINT64
 (*NAE_EXIT) (
-  GHCB                     *Ghcb,
-  EFI_SYSTEM_CONTEXT_X64   *Regs,
-  SEV_ES_INSTRUCTION_DATA  *InstructionData
+  GHCB                    *Ghcb,
+  EFI_SYSTEM_CONTEXT_X64  *Regs,
+  CC_INSTRUCTION_DATA     *InstructionData
   );
 
 //
@@ -155,439 +55,6 @@ typedef PACKED struct {
   SEV_SNP_CPUID_FUNCTION    function[0];
 } SEV_SNP_CPUID_INFO;
 
-/**
-  Return a pointer to the contents of the specified register.
-
-  Based upon the input register, return a pointer to the registers contents
-  in the x86 processor context.
-
-  @param[in] Regs      x64 processor context
-  @param[in] Register  Register to obtain pointer for
-
-  @return              Pointer to the contents of the requested register
-
-**/
-STATIC
-UINT64 *
-GetRegisterPointer (
-  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
-  IN UINT8                   Register
-  )
-{
-  UINT64  *Reg;
-
-  switch (Register) {
-    case 0:
-      Reg = &Regs->Rax;
-      break;
-    case 1:
-      Reg = &Regs->Rcx;
-      break;
-    case 2:
-      Reg = &Regs->Rdx;
-      break;
-    case 3:
-      Reg = &Regs->Rbx;
-      break;
-    case 4:
-      Reg = &Regs->Rsp;
-      break;
-    case 5:
-      Reg = &Regs->Rbp;
-      break;
-    case 6:
-      Reg = &Regs->Rsi;
-      break;
-    case 7:
-      Reg = &Regs->Rdi;
-      break;
-    case 8:
-      Reg = &Regs->R8;
-      break;
-    case 9:
-      Reg = &Regs->R9;
-      break;
-    case 10:
-      Reg = &Regs->R10;
-      break;
-    case 11:
-      Reg = &Regs->R11;
-      break;
-    case 12:
-      Reg = &Regs->R12;
-      break;
-    case 13:
-      Reg = &Regs->R13;
-      break;
-    case 14:
-      Reg = &Regs->R14;
-      break;
-    case 15:
-      Reg = &Regs->R15;
-      break;
-    default:
-      Reg = NULL;
-  }
-
-  ASSERT (Reg != NULL);
-
-  return Reg;
-}
-
-/**
-  Update the instruction parsing context for displacement bytes.
-
-  @param[in, out] InstructionData  Instruction parsing context
-  @param[in]      Size             The instruction displacement size
-
-**/
-STATIC
-VOID
-UpdateForDisplacement (
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData,
-  IN     UINTN                    Size
-  )
-{
-  InstructionData->DisplacementSize = Size;
-  InstructionData->Immediate       += Size;
-  InstructionData->End             += Size;
-}
-
-/**
-  Determine if an instruction address if RIP relative.
-
-  Examine the instruction parsing context to determine if the address offset
-  is relative to the instruction pointer.
-
-  @param[in] InstructionData  Instruction parsing context
-
-  @retval TRUE                Instruction addressing is RIP relative
-  @retval FALSE               Instruction addressing is not RIP relative
-
-**/
-STATIC
-BOOLEAN
-IsRipRelative (
-  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
-  )
-{
-  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
-
-  Ext = &InstructionData->Ext;
-
-  return ((InstructionData->Mode == LongMode64Bit) &&
-          (Ext->ModRm.Mod == 0) &&
-          (Ext->ModRm.Rm == 5)  &&
-          (InstructionData->SibPresent == FALSE));
-}
-
-/**
-  Return the effective address of a memory operand.
-
-  Examine the instruction parsing context to obtain the effective memory
-  address of a memory operand.
-
-  @param[in] Regs             x64 processor context
-  @param[in] InstructionData  Instruction parsing context
-
-  @return                     The memory operand effective address
-
-**/
-STATIC
-UINT64
-GetEffectiveMemoryAddress (
-  IN EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
-  )
-{
-  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
-  UINT64                         EffectiveAddress;
-
-  Ext              = &InstructionData->Ext;
-  EffectiveAddress = 0;
-
-  if (IsRipRelative (InstructionData)) {
-    //
-    // RIP-relative displacement is a 32-bit signed value
-    //
-    INT32  RipRelative;
-
-    RipRelative = *(INT32 *)InstructionData->Displacement;
-
-    UpdateForDisplacement (InstructionData, 4);
-
-    //
-    // Negative displacement is handled by standard UINT64 wrap-around.
-    //
-    return Regs->Rip + (UINT64)RipRelative;
-  }
-
-  switch (Ext->ModRm.Mod) {
-    case 1:
-      UpdateForDisplacement (InstructionData, 1);
-      EffectiveAddress += (UINT64)(*(INT8 *)(InstructionData->Displacement));
-      break;
-    case 2:
-      switch (InstructionData->AddrSize) {
-        case Size16Bits:
-          UpdateForDisplacement (InstructionData, 2);
-          EffectiveAddress += (UINT64)(*(INT16 *)(InstructionData->Displacement));
-          break;
-        default:
-          UpdateForDisplacement (InstructionData, 4);
-          EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData->Displacement));
-          break;
-      }
-
-      break;
-  }
-
-  if (InstructionData->SibPresent) {
-    INT64  Displacement;
-
-    if (Ext->Sib.Index != 4) {
-      CopyMem (
-        &Displacement,
-        GetRegisterPointer (Regs, Ext->Sib.Index),
-        sizeof (Displacement)
-        );
-      Displacement *= (INT64)(1 << Ext->Sib.Scale);
-
-      //
-      // Negative displacement is handled by standard UINT64 wrap-around.
-      //
-      EffectiveAddress += (UINT64)Displacement;
-    }
-
-    if ((Ext->Sib.Base != 5) || Ext->ModRm.Mod) {
-      EffectiveAddress += *GetRegisterPointer (Regs, Ext->Sib.Base);
-    } else {
-      UpdateForDisplacement (InstructionData, 4);
-      EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData->Displacement));
-    }
-  } else {
-    EffectiveAddress += *GetRegisterPointer (Regs, Ext->ModRm.Rm);
-  }
-
-  return EffectiveAddress;
-}
-
-/**
-  Decode a ModRM byte.
-
-  Examine the instruction parsing context to decode a ModRM byte and the SIB
-  byte, if present.
-
-  @param[in]      Regs             x64 processor context
-  @param[in, out] InstructionData  Instruction parsing context
-
-**/
-STATIC
-VOID
-DecodeModRm (
-  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
-  )
-{
-  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
-  INSTRUCTION_REX_PREFIX         *RexPrefix;
-  INSTRUCTION_MODRM              *ModRm;
-  INSTRUCTION_SIB                *Sib;
-
-  RexPrefix = &InstructionData->RexPrefix;
-  Ext       = &InstructionData->Ext;
-  ModRm     = &InstructionData->ModRm;
-  Sib       = &InstructionData->Sib;
-
-  InstructionData->ModRmPresent = TRUE;
-  ModRm->Uint8                  = *(InstructionData->End);
-
-  InstructionData->Displacement++;
-  InstructionData->Immediate++;
-  InstructionData->End++;
-
-  Ext->ModRm.Mod = ModRm->Bits.Mod;
-  Ext->ModRm.Reg = (RexPrefix->Bits.BitR << 3) | ModRm->Bits.Reg;
-  Ext->ModRm.Rm  = (RexPrefix->Bits.BitB << 3) | ModRm->Bits.Rm;
-
-  Ext->RegData = *GetRegisterPointer (Regs, Ext->ModRm.Reg);
-
-  if (Ext->ModRm.Mod == 3) {
-    Ext->RmData = *GetRegisterPointer (Regs, Ext->ModRm.Rm);
-  } else {
-    if (ModRm->Bits.Rm == 4) {
-      InstructionData->SibPresent = TRUE;
-      Sib->Uint8                  = *(InstructionData->End);
-
-      InstructionData->Displacement++;
-      InstructionData->Immediate++;
-      InstructionData->End++;
-
-      Ext->Sib.Scale = Sib->Bits.Scale;
-      Ext->Sib.Index = (RexPrefix->Bits.BitX << 3) | Sib->Bits.Index;
-      Ext->Sib.Base  = (RexPrefix->Bits.BitB << 3) | Sib->Bits.Base;
-    }
-
-    Ext->RmData = GetEffectiveMemoryAddress (Regs, InstructionData);
-  }
-}
-
-/**
-  Decode instruction prefixes.
-
-  Parse the instruction data to track the instruction prefixes that have
-  been used.
-
-  @param[in]      Regs             x64 processor context
-  @param[in, out] InstructionData  Instruction parsing context
-
-**/
-STATIC
-VOID
-DecodePrefixes (
-  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
-  )
-{
-  SEV_ES_INSTRUCTION_MODE  Mode;
-  SEV_ES_INSTRUCTION_SIZE  ModeDataSize;
-  SEV_ES_INSTRUCTION_SIZE  ModeAddrSize;
-  UINT8                    *Byte;
-
-  //
-  // Always in 64-bit mode
-  //
-  Mode         = LongMode64Bit;
-  ModeDataSize = Size32Bits;
-  ModeAddrSize = Size64Bits;
-
-  InstructionData->Mode     = Mode;
-  InstructionData->DataSize = ModeDataSize;
-  InstructionData->AddrSize = ModeAddrSize;
-
-  InstructionData->Prefixes = InstructionData->Begin;
-
-  Byte = InstructionData->Prefixes;
-  for ( ; ; Byte++, InstructionData->PrefixSize++) {
-    //
-    // Check the 0x40 to 0x4F range using an if statement here since some
-    // compilers don't like the "case 0x40 ... 0x4F:" syntax. This avoids
-    // 16 case statements below.
-    //
-    if ((*Byte >= REX_PREFIX_START) && (*Byte <= REX_PREFIX_STOP)) {
-      InstructionData->RexPrefix.Uint8 = *Byte;
-      if ((*Byte & REX_64BIT_OPERAND_SIZE_MASK) != 0) {
-        InstructionData->DataSize = Size64Bits;
-      }
-
-      continue;
-    }
-
-    switch (*Byte) {
-      case OVERRIDE_SEGMENT_CS:
-      case OVERRIDE_SEGMENT_DS:
-      case OVERRIDE_SEGMENT_ES:
-      case OVERRIDE_SEGMENT_SS:
-        if (Mode != LongMode64Bit) {
-          InstructionData->SegmentSpecified = TRUE;
-          InstructionData->Segment          = (*Byte >> 3) & 3;
-        }
-
-        break;
-
-      case OVERRIDE_SEGMENT_FS:
-      case OVERRIDE_SEGMENT_GS:
-        InstructionData->SegmentSpecified = TRUE;
-        InstructionData->Segment          = *Byte & 7;
-        break;
-
-      case OVERRIDE_OPERAND_SIZE:
-        if (InstructionData->RexPrefix.Uint8 == 0) {
-          InstructionData->DataSize =
-            (Mode == LongMode64Bit)       ? Size16Bits :
-            (Mode == LongModeCompat32Bit) ? Size16Bits :
-            (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
-        }
-
-        break;
-
-      case OVERRIDE_ADDRESS_SIZE:
-        InstructionData->AddrSize =
-          (Mode == LongMode64Bit)       ? Size32Bits :
-          (Mode == LongModeCompat32Bit) ? Size16Bits :
-          (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
-        break;
-
-      case LOCK_PREFIX:
-        break;
-
-      case REPZ_PREFIX:
-        InstructionData->RepMode = RepZ;
-        break;
-
-      case REPNZ_PREFIX:
-        InstructionData->RepMode = RepNZ;
-        break;
-
-      default:
-        InstructionData->OpCodes    = Byte;
-        InstructionData->OpCodeSize = (*Byte == TWO_BYTE_OPCODE_ESCAPE) ? 2 : 1;
-
-        InstructionData->End          = Byte + InstructionData->OpCodeSize;
-        InstructionData->Displacement = InstructionData->End;
-        InstructionData->Immediate    = InstructionData->End;
-        return;
-    }
-  }
-}
-
-/**
-  Determine instruction length
-
-  Return the total length of the parsed instruction.
-
-  @param[in] InstructionData  Instruction parsing context
-
-  @return                     Length of parsed instruction
-
-**/
-STATIC
-UINT64
-InstructionLength (
-  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
-  )
-{
-  return (UINT64)(InstructionData->End - InstructionData->Begin);
-}
-
-/**
-  Initialize the instruction parsing context.
-
-  Initialize the instruction parsing context, which includes decoding the
-  instruction prefixes.
-
-  @param[in, out] InstructionData  Instruction parsing context
-  @param[in]      Ghcb             Pointer to the Guest-Hypervisor Communication
-                                   Block
-  @param[in]      Regs             x64 processor context
-
-**/
-STATIC
-VOID
-InitInstructionData (
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData,
-  IN     GHCB                     *Ghcb,
-  IN     EFI_SYSTEM_CONTEXT_X64   *Regs
-  )
-{
-  SetMem (InstructionData, sizeof (*InstructionData), 0);
-  InstructionData->Ghcb  = Ghcb;
-  InstructionData->Begin = (UINT8 *)Regs->Rip;
-  InstructionData->End   = (UINT8 *)Regs->Rip;
-
-  DecodePrefixes (Regs, InstructionData);
-}
-
 /**
   Report an unsupported event to the hypervisor
 
@@ -604,9 +71,9 @@ InitInstructionData (
 STATIC
 UINT64
 UnsupportedExit (
-  IN GHCB                     *Ghcb,
-  IN EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN GHCB                    *Ghcb,
+  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  Status;
@@ -703,9 +170,9 @@ ValidateMmioMemory (
 STATIC
 UINT64
 MmioExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  ExitInfo1, ExitInfo2, Status;
@@ -731,7 +198,7 @@ MmioExit (
     // fall through
     //
     case 0x89:
-      DecodeModRm (Regs, InstructionData);
+      CcDecodeModRm (Regs, InstructionData);
       Bytes = ((Bytes != 0) ? Bytes :
                (InstructionData->DataSize == Size16Bits) ? 2 :
                (InstructionData->DataSize == Size32Bits) ? 4 :
@@ -824,7 +291,7 @@ MmioExit (
     // fall through
     //
     case 0xC7:
-      DecodeModRm (Regs, InstructionData);
+      CcDecodeModRm (Regs, InstructionData);
       Bytes = ((Bytes != 0) ? Bytes :
                (InstructionData->DataSize == Size16Bits) ? 2 :
                (InstructionData->DataSize == Size32Bits) ? 4 :
@@ -860,7 +327,7 @@ MmioExit (
     // fall through
     //
     case 0x8B:
-      DecodeModRm (Regs, InstructionData);
+      CcDecodeModRm (Regs, InstructionData);
       Bytes = ((Bytes != 0) ? Bytes :
                (InstructionData->DataSize == Size16Bits) ? 2 :
                (InstructionData->DataSize == Size32Bits) ? 4 :
@@ -888,7 +355,7 @@ MmioExit (
         return Status;
       }
 
-      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
       if (Bytes == 4) {
         //
         // Zero-extend for 32-bit operation
@@ -967,7 +434,7 @@ MmioExit (
     // fall through
     //
     case 0xB7:
-      DecodeModRm (Regs, InstructionData);
+      CcDecodeModRm (Regs, InstructionData);
       Bytes = (Bytes != 0) ? Bytes : 2;
 
       Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);
@@ -985,7 +452,7 @@ MmioExit (
         return Status;
       }
 
-      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
       SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0);
       CopyMem (Register, Ghcb->SharedBuffer, Bytes);
       break;
@@ -999,7 +466,7 @@ MmioExit (
     // fall through
     //
     case 0xBF:
-      DecodeModRm (Regs, InstructionData);
+      CcDecodeModRm (Regs, InstructionData);
       Bytes = (Bytes != 0) ? Bytes : 2;
 
       Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes);
@@ -1029,7 +496,7 @@ MmioExit (
         SignByte = ((*Data & BIT15) != 0) ? 0xFF : 0x00;
       }
 
-      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
       SetMem (Register, (UINTN)(1 << InstructionData->DataSize), SignByte);
       CopyMem (Register, Ghcb->SharedBuffer, Bytes);
       break;
@@ -1060,12 +527,12 @@ MmioExit (
 STATIC
 UINT64
 MwaitExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   Ghcb->SaveArea.Rax = Regs->Rax;
   CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
@@ -1092,12 +559,12 @@ MwaitExit (
 STATIC
 UINT64
 MonitorExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   Ghcb->SaveArea.Rax = Regs->Rax;  // Identity mapped, so VA = PA
   CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
@@ -1126,9 +593,9 @@ MonitorExit (
 STATIC
 UINT64
 WbinvdExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   return CcExitVmgExit (Ghcb, SVM_EXIT_WBINVD, 0, 0);
@@ -1151,14 +618,14 @@ WbinvdExit (
 STATIC
 UINT64
 RdtscpExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  Status;
 
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   Status = CcExitVmgExit (Ghcb, SVM_EXIT_RDTSCP, 0, 0);
   if (Status != 0) {
@@ -1196,14 +663,14 @@ RdtscpExit (
 STATIC
 UINT64
 VmmCallExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  Status;
 
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   Ghcb->SaveArea.Rax = Regs->Rax;
   CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
@@ -1241,9 +708,9 @@ VmmCallExit (
 STATIC
 UINT64
 MsrExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  ExitInfo1, Status;
@@ -1302,8 +769,8 @@ MsrExit (
 STATIC
 UINT64
 IoioExitInfo (
-  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  ExitInfo;
@@ -1437,9 +904,9 @@ IoioExitInfo (
 STATIC
 UINT64
 IoioExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64   ExitInfo1, ExitInfo2, Status;
@@ -1531,9 +998,9 @@ IoioExit (
 STATIC
 UINT64
 InvdExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   return CcExitVmgExit (Ghcb, SVM_EXIT_INVD, 0, 0);
@@ -1949,9 +1416,9 @@ Out:
 STATIC
 UINT64
 CpuidExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   BOOLEAN  Unsupported;
@@ -2041,9 +1508,9 @@ CpuidFail:
 STATIC
 UINT64
 RdpmcExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  Status;
@@ -2085,9 +1552,9 @@ RdpmcExit (
 STATIC
 UINT64
 RdtscExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
   UINT64  Status;
@@ -2126,25 +1593,25 @@ RdtscExit (
 STATIC
 UINT64
 Dr7WriteExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
-  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
-  SEV_ES_PER_CPU_DATA            *SevEsData;
-  UINT64                         *Register;
-  UINT64                         Status;
+  CC_INSTRUCTION_OPCODE_EXT  *Ext;
+  SEV_ES_PER_CPU_DATA        *SevEsData;
+  UINT64                     *Register;
+  UINT64                     Status;
 
   Ext       = &InstructionData->Ext;
   SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
 
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   //
   // MOV DRn always treats MOD == 3 no matter how encoded
   //
-  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
+  Register = CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
 
   //
   // Using a value of 0 for ExitInfo1 means RAX holds the value
@@ -2179,24 +1646,24 @@ Dr7WriteExit (
 STATIC
 UINT64
 Dr7ReadExit (
-  IN OUT GHCB                     *Ghcb,
-  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
-  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
+  IN OUT GHCB                    *Ghcb,
+  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN     CC_INSTRUCTION_DATA     *InstructionData
   )
 {
-  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
-  SEV_ES_PER_CPU_DATA            *SevEsData;
-  UINT64                         *Register;
+  CC_INSTRUCTION_OPCODE_EXT  *Ext;
+  SEV_ES_PER_CPU_DATA        *SevEsData;
+  UINT64                     *Register;
 
   Ext       = &InstructionData->Ext;
   SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
 
-  DecodeModRm (Regs, InstructionData);
+  CcDecodeModRm (Regs, InstructionData);
 
   //
   // MOV DRn always treats MOD == 3 no matter how encoded
   //
-  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
+  Register = CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
 
   //
   // If there is a cached valued for DR7, return that. Otherwise return the
@@ -2232,12 +1699,12 @@ InternalVmgExitHandleVc (
   IN OUT EFI_SYSTEM_CONTEXT  SystemContext
   )
 {
-  EFI_SYSTEM_CONTEXT_X64   *Regs;
-  NAE_EXIT                 NaeExit;
-  SEV_ES_INSTRUCTION_DATA  InstructionData;
-  UINT64                   ExitCode, Status;
-  EFI_STATUS               VcRet;
-  BOOLEAN                  InterruptState;
+  EFI_SYSTEM_CONTEXT_X64  *Regs;
+  NAE_EXIT                NaeExit;
+  CC_INSTRUCTION_DATA     InstructionData;
+  UINT64                  ExitCode, Status;
+  EFI_STATUS              VcRet;
+  BOOLEAN                 InterruptState;
 
   VcRet = EFI_SUCCESS;
 
@@ -2307,11 +1774,11 @@ InternalVmgExitHandleVc (
       NaeExit = UnsupportedExit;
   }
 
-  InitInstructionData (&InstructionData, Ghcb, Regs);
+  CcInitInstructionData (&InstructionData, Ghcb, Regs);
 
   Status = NaeExit (Ghcb, Regs, &InstructionData);
   if (Status == 0) {
-    Regs->Rip += InstructionLength (&InstructionData);
+    Regs->Rip += CcInstructionLength (&InstructionData);
   } else {
     GHCB_EVENT_INJECTION  Event;
 
diff --git a/OvmfPkg/Library/CcExitLib/CcInstruction.c b/OvmfPkg/Library/CcExitLib/CcInstruction.c
new file mode 100644
index 000000000000..0fb54b3ed553
--- /dev/null
+++ b/OvmfPkg/Library/CcExitLib/CcInstruction.c
@@ -0,0 +1,454 @@
+/** @file
+  X64 Instruction function.
+
+  Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Uefi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Register/Intel/Cpuid.h>
+#include <IndustryStandard/InstructionParsing.h>
+#include "CcInstruction.h"
+
+#define MAX_INSTRUCTION_LENGTH  15
+
+/**
+  Return a pointer to the contents of the specified register.
+
+  Based upon the input register, return a pointer to the registers contents
+  in the x86 processor context.
+
+  @param[in] Regs      x64 processor context
+  @param[in] Register  Register to obtain pointer for
+
+  @return              Pointer to the contents of the requested register
+
+**/
+UINT64 *
+CcGetRegisterPointer (
+  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN UINT8                   Register
+  )
+{
+  UINT64  *Reg;
+
+  switch (Register) {
+    case 0:
+      Reg = &Regs->Rax;
+      break;
+    case 1:
+      Reg = &Regs->Rcx;
+      break;
+    case 2:
+      Reg = &Regs->Rdx;
+      break;
+    case 3:
+      Reg = &Regs->Rbx;
+      break;
+    case 4:
+      Reg = &Regs->Rsp;
+      break;
+    case 5:
+      Reg = &Regs->Rbp;
+      break;
+    case 6:
+      Reg = &Regs->Rsi;
+      break;
+    case 7:
+      Reg = &Regs->Rdi;
+      break;
+    case 8:
+      Reg = &Regs->R8;
+      break;
+    case 9:
+      Reg = &Regs->R9;
+      break;
+    case 10:
+      Reg = &Regs->R10;
+      break;
+    case 11:
+      Reg = &Regs->R11;
+      break;
+    case 12:
+      Reg = &Regs->R12;
+      break;
+    case 13:
+      Reg = &Regs->R13;
+      break;
+    case 14:
+      Reg = &Regs->R14;
+      break;
+    case 15:
+      Reg = &Regs->R15;
+      break;
+    default:
+      Reg = NULL;
+  }
+
+  ASSERT (Reg != NULL);
+
+  return Reg;
+}
+
+/**
+  Update the instruction parsing context for displacement bytes.
+
+  @param[in, out] InstructionData  Instruction parsing context
+  @param[in]      Size             The instruction displacement size
+
+**/
+STATIC
+VOID
+UpdateForDisplacement (
+  IN OUT CC_INSTRUCTION_DATA  *InstructionData,
+  IN     UINTN                Size
+  )
+{
+  InstructionData->DisplacementSize = Size;
+  InstructionData->Immediate       += Size;
+  InstructionData->End             += Size;
+}
+
+/**
+  Determine if an instruction address if RIP relative.
+
+  Examine the instruction parsing context to determine if the address offset
+  is relative to the instruction pointer.
+
+  @param[in] InstructionData  Instruction parsing context
+
+  @retval TRUE                Instruction addressing is RIP relative
+  @retval FALSE               Instruction addressing is not RIP relative
+
+**/
+STATIC
+BOOLEAN
+IsRipRelative (
+  IN CC_INSTRUCTION_DATA  *InstructionData
+  )
+{
+  CC_INSTRUCTION_OPCODE_EXT  *Ext;
+
+  Ext = &InstructionData->Ext;
+
+  return ((InstructionData->Mode == LongMode64Bit) &&
+          (Ext->ModRm.Mod == 0) &&
+          (Ext->ModRm.Rm == 5)  &&
+          (InstructionData->SibPresent == FALSE));
+}
+
+/**
+  Return the effective address of a memory operand.
+
+  Examine the instruction parsing context to obtain the effective memory
+  address of a memory operand.
+
+  @param[in] Regs             x64 processor context
+  @param[in] InstructionData  Instruction parsing context
+
+  @return                     The memory operand effective address
+
+**/
+STATIC
+UINT64
+GetEffectiveMemoryAddress (
+  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN CC_INSTRUCTION_DATA     *InstructionData
+  )
+{
+  CC_INSTRUCTION_OPCODE_EXT  *Ext;
+  UINT64                     EffectiveAddress;
+
+  Ext              = &InstructionData->Ext;
+  EffectiveAddress = 0;
+
+  if (IsRipRelative (InstructionData)) {
+    //
+    // RIP-relative displacement is a 32-bit signed value
+    //
+    INT32  RipRelative;
+
+    RipRelative = *(INT32 *)InstructionData->Displacement;
+
+    UpdateForDisplacement (InstructionData, 4);
+
+    //
+    // Negative displacement is handled by standard UINT64 wrap-around.
+    //
+    return Regs->Rip + (UINT64)RipRelative;
+  }
+
+  switch (Ext->ModRm.Mod) {
+    case 1:
+      UpdateForDisplacement (InstructionData, 1);
+      EffectiveAddress += (UINT64)(*(INT8 *)(InstructionData->Displacement));
+      break;
+    case 2:
+      switch (InstructionData->AddrSize) {
+        case Size16Bits:
+          UpdateForDisplacement (InstructionData, 2);
+          EffectiveAddress += (UINT64)(*(INT16 *)(InstructionData->Displacement));
+          break;
+        default:
+          UpdateForDisplacement (InstructionData, 4);
+          EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData->Displacement));
+          break;
+      }
+
+      break;
+  }
+
+  if (InstructionData->SibPresent) {
+    INT64  Displacement;
+
+    if (Ext->Sib.Index != 4) {
+      CopyMem (
+        &Displacement,
+        CcGetRegisterPointer (Regs, Ext->Sib.Index),
+        sizeof (Displacement)
+        );
+      Displacement *= (INT64)(1 << Ext->Sib.Scale);
+
+      //
+      // Negative displacement is handled by standard UINT64 wrap-around.
+      //
+      EffectiveAddress += (UINT64)Displacement;
+    }
+
+    if ((Ext->Sib.Base != 5) || Ext->ModRm.Mod) {
+      EffectiveAddress += *CcGetRegisterPointer (Regs, Ext->Sib.Base);
+    } else {
+      UpdateForDisplacement (InstructionData, 4);
+      EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData->Displacement));
+    }
+  } else {
+    EffectiveAddress += *CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
+  }
+
+  return EffectiveAddress;
+}
+
+/**
+  Decode a ModRM byte.
+
+  Examine the instruction parsing context to decode a ModRM byte and the SIB
+  byte, if present.
+
+  @param[in]      Regs             x64 processor context
+  @param[in, out] InstructionData  Instruction parsing context
+
+**/
+VOID
+CcDecodeModRm (
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData
+  )
+{
+  CC_INSTRUCTION_OPCODE_EXT  *Ext;
+  INSTRUCTION_REX_PREFIX     *RexPrefix;
+  INSTRUCTION_MODRM          *ModRm;
+  INSTRUCTION_SIB            *Sib;
+
+  RexPrefix = &InstructionData->RexPrefix;
+  Ext       = &InstructionData->Ext;
+  ModRm     = &InstructionData->ModRm;
+  Sib       = &InstructionData->Sib;
+
+  InstructionData->ModRmPresent = TRUE;
+  ModRm->Uint8                  = *(InstructionData->End);
+
+  InstructionData->Displacement++;
+  InstructionData->Immediate++;
+  InstructionData->End++;
+
+  Ext->ModRm.Mod = ModRm->Bits.Mod;
+  Ext->ModRm.Reg = (RexPrefix->Bits.BitR << 3) | ModRm->Bits.Reg;
+  Ext->ModRm.Rm  = (RexPrefix->Bits.BitB << 3) | ModRm->Bits.Rm;
+
+  Ext->RegData = *CcGetRegisterPointer (Regs, Ext->ModRm.Reg);
+
+  if (Ext->ModRm.Mod == 3) {
+    Ext->RmData = *CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
+  } else {
+    if (ModRm->Bits.Rm == 4) {
+      InstructionData->SibPresent = TRUE;
+      Sib->Uint8                  = *(InstructionData->End);
+
+      InstructionData->Displacement++;
+      InstructionData->Immediate++;
+      InstructionData->End++;
+
+      Ext->Sib.Scale = Sib->Bits.Scale;
+      Ext->Sib.Index = (RexPrefix->Bits.BitX << 3) | Sib->Bits.Index;
+      Ext->Sib.Base  = (RexPrefix->Bits.BitB << 3) | Sib->Bits.Base;
+    }
+
+    Ext->RmData = GetEffectiveMemoryAddress (Regs, InstructionData);
+  }
+}
+
+/**
+  Decode instruction prefixes.
+
+  Parse the instruction data to track the instruction prefixes that have
+  been used.
+
+  @param[in]      Regs             x64 processor context
+  @param[in, out] InstructionData  Instruction parsing context
+
+  @retval         EFI_SUCCESS      Successfully decode Prefixes
+  @retval         Others           Other error as indicated
+**/
+STATIC
+EFI_STATUS
+DecodePrefixes (
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData
+  )
+{
+  CC_INSTRUCTION_MODE  Mode;
+  CC_INSTRUCTION_SIZE  ModeDataSize;
+  CC_INSTRUCTION_SIZE  ModeAddrSize;
+  UINT8                *Byte;
+  UINT8                ParsedLength;
+
+  ParsedLength = 0;
+
+  //
+  // Always in 64-bit mode
+  //
+  Mode         = LongMode64Bit;
+  ModeDataSize = Size32Bits;
+  ModeAddrSize = Size64Bits;
+
+  InstructionData->Mode     = Mode;
+  InstructionData->DataSize = ModeDataSize;
+  InstructionData->AddrSize = ModeAddrSize;
+
+  InstructionData->Prefixes = InstructionData->Begin;
+
+  Byte = InstructionData->Prefixes;
+  for ( ; ParsedLength <= MAX_INSTRUCTION_LENGTH; Byte++, InstructionData->PrefixSize++, ParsedLength++) {
+    //
+    // Check the 0x40 to 0x4F range using an if statement here since some
+    // compilers don't like the "case 0x40 ... 0x4F:" syntax. This avoids
+    // 16 case statements below.
+    //
+    if ((*Byte >= REX_PREFIX_START) && (*Byte <= REX_PREFIX_STOP)) {
+      InstructionData->RexPrefix.Uint8 = *Byte;
+      if ((*Byte & REX_64BIT_OPERAND_SIZE_MASK) != 0) {
+        InstructionData->DataSize = Size64Bits;
+      }
+
+      continue;
+    }
+
+    switch (*Byte) {
+      case OVERRIDE_SEGMENT_CS:
+      case OVERRIDE_SEGMENT_DS:
+      case OVERRIDE_SEGMENT_ES:
+      case OVERRIDE_SEGMENT_SS:
+        if (Mode != LongMode64Bit) {
+          InstructionData->SegmentSpecified = TRUE;
+          InstructionData->Segment          = (*Byte >> 3) & 3;
+        }
+
+        break;
+
+      case OVERRIDE_SEGMENT_FS:
+      case OVERRIDE_SEGMENT_GS:
+        InstructionData->SegmentSpecified = TRUE;
+        InstructionData->Segment          = *Byte & 7;
+        break;
+
+      case OVERRIDE_OPERAND_SIZE:
+        if (InstructionData->RexPrefix.Uint8 == 0) {
+          InstructionData->DataSize =
+            (Mode == LongMode64Bit)       ? Size16Bits :
+            (Mode == LongModeCompat32Bit) ? Size16Bits :
+            (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
+        }
+
+        break;
+
+      case OVERRIDE_ADDRESS_SIZE:
+        InstructionData->AddrSize =
+          (Mode == LongMode64Bit)       ? Size32Bits :
+          (Mode == LongModeCompat32Bit) ? Size16Bits :
+          (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
+        break;
+
+      case LOCK_PREFIX:
+        break;
+
+      case REPZ_PREFIX:
+        InstructionData->RepMode = RepZ;
+        break;
+
+      case REPNZ_PREFIX:
+        InstructionData->RepMode = RepNZ;
+        break;
+
+      default:
+        InstructionData->OpCodes    = Byte;
+        InstructionData->OpCodeSize = (*Byte == TWO_BYTE_OPCODE_ESCAPE) ? 2 : 1;
+
+        InstructionData->End          = Byte + InstructionData->OpCodeSize;
+        InstructionData->Displacement = InstructionData->End;
+        InstructionData->Immediate    = InstructionData->End;
+        return EFI_SUCCESS;
+    }
+  }
+
+  return EFI_ABORTED;
+}
+
+/**
+  Determine instruction length
+
+  Return the total length of the parsed instruction.
+
+  @param[in] InstructionData  Instruction parsing context
+
+  @return                     Length of parsed instruction
+
+**/
+UINT64
+CcInstructionLength (
+  IN CC_INSTRUCTION_DATA  *InstructionData
+  )
+{
+  return (UINT64)(InstructionData->End - InstructionData->Begin);
+}
+
+/**
+  Initialize the instruction parsing context.
+
+  Initialize the instruction parsing context, which includes decoding the
+  instruction prefixes.
+
+  @param[in, out] InstructionData  Instruction parsing context
+  @param[in]      Ghcb             Pointer to the Guest-Hypervisor Communication
+                                   Block
+  @param[in]      Regs             x64 processor context
+
+  @retval         EFI_SUCCESS      Successfully initialize InstructionData
+  @retval         Others           Other error as indicated
+**/
+EFI_STATUS
+CcInitInstructionData (
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
+  IN     GHCB                    *Ghcb,
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
+  )
+{
+  SetMem (InstructionData, sizeof (*InstructionData), 0);
+  InstructionData->Ghcb  = Ghcb;
+  InstructionData->Begin = (UINT8 *)Regs->Rip;
+  InstructionData->End   = (UINT8 *)Regs->Rip;
+
+  return DecodePrefixes (Regs, InstructionData);
+}
diff --git a/OvmfPkg/Library/CcExitLib/CcInstruction.h b/OvmfPkg/Library/CcExitLib/CcInstruction.h
new file mode 100644
index 000000000000..a8223a6a7d6d
--- /dev/null
+++ b/OvmfPkg/Library/CcExitLib/CcInstruction.h
@@ -0,0 +1,197 @@
+/** @file
+  Confidential Computing X64 Instruction
+
+  Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef CC_INSTRUCTION_H_
+#define CC_INSTRUCTION_H_
+
+#include <Base.h>
+#include <Uefi.h>
+#include <Register/Amd/Ghcb.h>
+#include <IndustryStandard/InstructionParsing.h>
+#include <Protocol/DebugSupport.h>
+
+//
+// Instruction execution mode definition
+//
+typedef enum {
+  LongMode64Bit = 0,
+  LongModeCompat32Bit,
+  LongModeCompat16Bit,
+} CC_INSTRUCTION_MODE;
+
+//
+// Instruction size definition (for operand and address)
+//
+typedef enum {
+  Size8Bits = 0,
+  Size16Bits,
+  Size32Bits,
+  Size64Bits,
+} CC_INSTRUCTION_SIZE;
+
+//
+// Intruction segment definition
+//
+typedef enum {
+  SegmentEs = 0,
+  SegmentCs,
+  SegmentSs,
+  SegmentDs,
+  SegmentFs,
+  SegmentGs,
+} CC_INSTRUCTION_SEGMENT;
+
+//
+// Instruction rep function definition
+//
+typedef enum {
+  RepNone = 0,
+  RepZ,
+  RepNZ,
+} CC_INSTRUCTION_REP;
+
+typedef struct {
+  UINT8    Rm;
+  UINT8    Reg;
+  UINT8    Mod;
+} CC_INSTRUCTION_MODRM_EXT;
+
+typedef struct {
+  UINT8    Base;
+  UINT8    Index;
+  UINT8    Scale;
+} CC_INSTRUCTION_SIB_EXT;
+
+//
+// Instruction opcode definition
+//
+typedef struct {
+  CC_INSTRUCTION_MODRM_EXT    ModRm;
+
+  CC_INSTRUCTION_SIB_EXT      Sib;
+
+  UINTN                       RegData;
+  UINTN                       RmData;
+} CC_INSTRUCTION_OPCODE_EXT;
+
+//
+// Instruction parsing context definition
+//
+typedef struct {
+  GHCB                         *Ghcb;
+
+  CC_INSTRUCTION_MODE          Mode;
+  CC_INSTRUCTION_SIZE          DataSize;
+  CC_INSTRUCTION_SIZE          AddrSize;
+  BOOLEAN                      SegmentSpecified;
+  CC_INSTRUCTION_SEGMENT       Segment;
+  CC_INSTRUCTION_REP           RepMode;
+
+  UINT8                        *Begin;
+  UINT8                        *End;
+
+  UINT8                        *Prefixes;
+  UINT8                        *OpCodes;
+  UINT8                        *Displacement;
+  UINT8                        *Immediate;
+
+  INSTRUCTION_REX_PREFIX       RexPrefix;
+
+  BOOLEAN                      ModRmPresent;
+  INSTRUCTION_MODRM            ModRm;
+
+  BOOLEAN                      SibPresent;
+  INSTRUCTION_SIB              Sib;
+
+  UINTN                        PrefixSize;
+  UINTN                        OpCodeSize;
+  UINTN                        DisplacementSize;
+  UINTN                        ImmediateSize;
+
+  CC_INSTRUCTION_OPCODE_EXT    Ext;
+} CC_INSTRUCTION_DATA;
+
+EFI_STATUS
+CcInitInstructionData (
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
+  IN     GHCB                    *Ghcb,
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
+  );
+
+/**
+  Return a pointer to the contents of the specified register.
+
+  Based upon the input register, return a pointer to the registers contents
+  in the x86 processor context.
+
+  @param[in] Regs      x64 processor context
+  @param[in] Register  Register to obtain pointer for
+
+  @return              Pointer to the contents of the requested register
+
+**/
+UINT64 *
+CcGetRegisterPointer (
+  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN UINT8                   Register
+  );
+
+/**
+  Decode a ModRM byte.
+
+  Examine the instruction parsing context to decode a ModRM byte and the SIB
+  byte, if present.
+
+  @param[in]      Regs             x64 processor context
+  @param[in, out] InstructionData  Instruction parsing context
+
+**/
+VOID
+CcDecodeModRm (
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData
+  );
+
+/**
+  Determine instruction length
+
+  Return the total length of the parsed instruction.
+
+  @param[in] InstructionData  Instruction parsing context
+
+  @return                     Length of parsed instruction
+
+**/
+UINT64
+CcInstructionLength (
+  IN CC_INSTRUCTION_DATA  *InstructionData
+  );
+
+/**
+  Initialize the instruction parsing context.
+
+  Initialize the instruction parsing context, which includes decoding the
+  instruction prefixes.
+
+  @param[in, out] InstructionData  Instruction parsing context
+  @param[in]      Ghcb             Pointer to the Guest-Hypervisor Communication
+                                   Block
+  @param[in]      Regs             x64 processor context
+
+  @retval         EFI_SUCCESS      Successfully initialize InstructionData
+  @retval         Others           Other error as indicated
+**/
+EFI_STATUS
+CcInitInstructionData (
+  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
+  IN     GHCB                    *Ghcb,
+  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
+  );
+
+#endif
diff --git a/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf b/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
index 1ee22ce0aea1..811269dd2c06 100644
--- a/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
+++ b/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
@@ -24,6 +24,7 @@
   CcExitLib.c
   CcExitVcHandler.c
   CcExitVcHandler.h
+  CcInstruction.c
   SecCcExitVcHandler.c
   CcExitVeHandler.c
   X64/TdVmcallCpuid.nasm
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
  2022-12-29  8:55 [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
  2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
@ 2022-12-29  8:55 ` Min Xu
  2023-01-06  8:51   ` Yao, Jiewen
  2022-12-29  9:00 ` [PATCH V1 0/2] " Min Xu
  2 siblings, 1 reply; 8+ messages in thread
From: Min Xu @ 2022-12-29  8:55 UTC (permalink / raw)
  To: devel
  Cc: Min M Xu, Erdem Aktas, James Bottomley, Jiewen Yao, Gerd Hoffmann,
	Tom Lendacky, Ryan Afranji

From: Min M Xu <min.m.xu@intel.com>

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4169

The previous TDX MmioExit doesn't handle the Mmio instructions correctly
in some scenarios. This patch refactors the implementation to fix the
issues.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ryan Afranji <afranji@google.com>
Reported-by: Ryan Afranji <afranji@google.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
 OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498 ++++++++++++++------
 1 file changed, 347 insertions(+), 151 deletions(-)

diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
index 30d547d5fe55..e0998722af39 100644
--- a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
+++ b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
@@ -13,6 +13,10 @@
 #include <Library/BaseMemoryLib.h>
 #include <IndustryStandard/Tdx.h>
 #include <IndustryStandard/InstructionParsing.h>
+#include "CcInstruction.h"
+
+#define TDX_MMIO_READ   0
+#define TDX_MMIO_WRITE  1
 
 typedef union {
   struct {
@@ -216,14 +220,15 @@ STATIC
 VOID
 EFIAPI
 TdxDecodeInstruction (
-  IN UINT8  *Rip
+  IN UINT8   *Rip,
+  IN UINT32  Length
   )
 {
   UINTN  i;
 
   DEBUG ((DEBUG_INFO, "TDX: #TD[EPT] instruction (%p):", Rip));
-  for (i = 0; i < 15; i++) {
-    DEBUG ((DEBUG_INFO, "%02x:", Rip[i]));
+  for (i = 0; i < MIN (15, Length); i++) {
+    DEBUG ((DEBUG_INFO, "%02x ", Rip[i]));
   }
 
   DEBUG ((DEBUG_INFO, "\n"));
@@ -235,50 +240,324 @@ TdxDecodeInstruction (
     TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \
   }
 
+/**
+ * Tdx MMIO access via TdVmcall.
+ *
+ * @param MmioSize      Size of the MMIO access
+ * @param ReadOrWrite   Read or write operation
+ * @param GuestPA       Guest physical address
+ * @param Val           Pointer to the value which is read or written
+
+ * @retval EFI_SUCCESS  Successfully access the mmio
+ * @retval Others       Other errors as indicated
+ */
 STATIC
-UINT64 *
-EFIAPI
-GetRegFromContext (
-  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
-  IN UINTN                   RegIndex
+UINT64
+TdxMmioReadWrite (
+  IN UINT32  MmioSize,
+  IN UINT32  ReadOrWrite,
+  IN UINT64  GuestPA,
+  IN UINT64  *Val
   )
 {
-  switch (RegIndex) {
-    case 0: return &Regs->Rax;
-      break;
-    case 1: return &Regs->Rcx;
-      break;
-    case 2: return &Regs->Rdx;
-      break;
-    case 3: return &Regs->Rbx;
-      break;
-    case 4: return &Regs->Rsp;
-      break;
-    case 5: return &Regs->Rbp;
-      break;
-    case 6: return &Regs->Rsi;
-      break;
-    case 7: return &Regs->Rdi;
-      break;
-    case 8: return &Regs->R8;
-      break;
-    case 9: return &Regs->R9;
+  UINT64  Status;
+
+  if ((MmioSize != 1) && (MmioSize != 2) && (MmioSize != 4) && (MmioSize != 8)) {
+    ASSERT (FALSE);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (Val == NULL) {
+    ASSERT (FALSE);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (ReadOrWrite == TDX_MMIO_READ) {
+    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_READ, GuestPA, 0, Val);
+  } else if (ReadOrWrite == TDX_MMIO_WRITE) {
+    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_WRITE, GuestPA, *Val, 0);
+  } else {
+    ASSERT (FALSE);
+    Status = EFI_INVALID_PARAMETER;
+  }
+
+  return Status;
+}
+
+typedef struct {
+  UINT8                   OpCode;
+  UINT32                  Bytes;
+  EFI_PHYSICAL_ADDRESS    Address;
+  UINT64                  Val;
+  UINT64                  *Register;
+  UINT32                  ReadOrWrite;
+} MMIO_EXIT_PARSED_INSTRUCTION;
+
+/**
+ * Parse the MMIO instructions.
+ *
+ * @param Regs              Pointer to the EFI_SYSTEM_CONTEXT_X64 which includes the instructions
+ * @param InstructionData   Pointer to the CC_INSTRUCTION_DATA
+ * @param ParsedInstruction Pointer to the parsed instruction data
+ *
+ * @retval EFI_SUCCESS      Successfully parsed the instructions
+ * @retval Others           Other error as indicated
+ */
+STATIC
+EFI_STATUS
+ParseMmioExitInstructions (
+  IN OUT EFI_SYSTEM_CONTEXT_X64     *Regs,
+  IN OUT CC_INSTRUCTION_DATA        *InstructionData,
+  OUT MMIO_EXIT_PARSED_INSTRUCTION  *ParsedInstruction
+  )
+{
+  EFI_STATUS            Status;
+  UINT8                 OpCode;
+  UINT8                 SignByte;
+  UINT32                Bytes;
+  EFI_PHYSICAL_ADDRESS  Address;
+  UINT64                Val;
+  UINT64                *Register;
+  UINT32                ReadOrWrite;
+
+  Address  = 0;
+  Bytes    = 0;
+  Register = NULL;
+  Status   = EFI_SUCCESS;
+  Val      = 0;
+
+  Status = CcInitInstructionData (InstructionData, NULL, Regs);
+  if (Status != EFI_SUCCESS) {
+    DEBUG ((DEBUG_ERROR, "%a: Initialize InstructionData failed! (%r)\n", __FUNCTION__, Status));
+    ASSERT (FALSE);
+    return Status;
+  }
+
+  OpCode = *(InstructionData->OpCodes);
+  if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
+    OpCode = *(InstructionData->OpCodes + 1);
+  }
+
+  switch (OpCode) {
+    //
+    // MMIO write (MOV reg/memX, regX)
+    //
+    case 0x88:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0x89:
+      CcDecodeModRm (Regs, InstructionData);
+      Bytes = ((Bytes != 0) ? Bytes :
+               (InstructionData->DataSize == Size16Bits) ? 2 :
+               (InstructionData->DataSize == Size32Bits) ? 4 :
+               (InstructionData->DataSize == Size64Bits) ? 8 :
+               0);
+
+      if (InstructionData->Ext.ModRm.Mod == 3) {
+        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode: 0x%x)\n", __FUNCTION__, OpCode));
+        ASSERT (FALSE);
+        return EFI_UNSUPPORTED;
+      }
+
+      Address     = InstructionData->Ext.RmData;
+      Val         = InstructionData->Ext.RegData;
+      ReadOrWrite = TDX_MMIO_WRITE;
+
       break;
-    case 10: return &Regs->R10;
+
+    //
+    // MMIO write (MOV moffsetX, aX)
+    //
+    case 0xA2:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0xA3:
+      Bytes = ((Bytes != 0) ? Bytes :
+               (InstructionData->DataSize == Size16Bits) ? 2 :
+               (InstructionData->DataSize == Size32Bits) ? 4 :
+               (InstructionData->DataSize == Size64Bits) ? 8 :
+               0);
+
+      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData->AddrSize);
+      InstructionData->End          += InstructionData->ImmediateSize;
+      CopyMem (&Address, InstructionData->Immediate, InstructionData->ImmediateSize);
+
+      Val         = Regs->Rax;
+      ReadOrWrite = TDX_MMIO_WRITE;
       break;
-    case 11: return &Regs->R11;
+
+    //
+    // MMIO write (MOV reg/memX, immX)
+    //
+    case 0xC6:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0xC7:
+      CcDecodeModRm (Regs, InstructionData);
+      Bytes = ((Bytes != 0) ? Bytes :
+               (InstructionData->DataSize == Size16Bits) ? 2 :
+               (InstructionData->DataSize == Size32Bits) ? 4 :
+               (InstructionData->DataSize == Size64Bits) ? 8 :
+               0);
+
+      InstructionData->ImmediateSize = Bytes;
+      InstructionData->End          += Bytes;
+
+      Val = 0;
+      CopyMem (&Val, InstructionData->Immediate, InstructionData->ImmediateSize);
+
+      Address     = InstructionData->Ext.RmData;
+      ReadOrWrite = TDX_MMIO_WRITE;
+
       break;
-    case 12: return &Regs->R12;
+
+    //
+    // MMIO read (MOV regX, reg/memX)
+    //
+    case 0x8A:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0x8B:
+      CcDecodeModRm (Regs, InstructionData);
+      Bytes = ((Bytes != 0) ? Bytes :
+               (InstructionData->DataSize == Size16Bits) ? 2 :
+               (InstructionData->DataSize == Size32Bits) ? 4 :
+               (InstructionData->DataSize == Size64Bits) ? 8 :
+               0);
+      if (InstructionData->Ext.ModRm.Mod == 3) {
+        //
+        // NPF on two register operands???
+        //
+        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode: 0x%x)\n", __FUNCTION__, OpCode));
+        ASSERT (FALSE);
+        return EFI_UNSUPPORTED;
+      }
+
+      Address     = InstructionData->Ext.RmData;
+      ReadOrWrite = TDX_MMIO_READ;
+
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      if (Bytes == 4) {
+        //
+        // Zero-extend for 32-bit operation
+        //
+        *Register = 0;
+      }
+
       break;
-    case 13: return &Regs->R13;
+
+    //
+    // MMIO read (MOV aX, moffsetX)
+    //
+    case 0xA0:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0xA1:
+      Bytes = ((Bytes != 0) ? Bytes :
+               (InstructionData->DataSize == Size16Bits) ? 2 :
+               (InstructionData->DataSize == Size32Bits) ? 4 :
+               (InstructionData->DataSize == Size64Bits) ? 8 :
+               0);
+
+      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData->AddrSize);
+      InstructionData->End          += InstructionData->ImmediateSize;
+
+      Address = 0;
+      CopyMem (
+        &Address,
+        InstructionData->Immediate,
+        InstructionData->ImmediateSize
+        );
+
+      if (Bytes == 4) {
+        //
+        // Zero-extend for 32-bit operation
+        //
+        Regs->Rax = 0;
+      }
+
+      Register    = &Regs->Rax;
+      ReadOrWrite = TDX_MMIO_READ;
+
       break;
-    case 14: return &Regs->R14;
+
+    //
+    // MMIO read w/ zero-extension ((MOVZX regX, reg/memX)
+    //
+    case 0xB6:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0xB7:
+      CcDecodeModRm (Regs, InstructionData);
+      Bytes   = (Bytes != 0) ? Bytes : 2;
+      Address = InstructionData->Ext.RmData;
+
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0);
+
+      ReadOrWrite = TDX_MMIO_READ;
+
       break;
-    case 15: return &Regs->R15;
+
+    //
+    // MMIO read w/ sign-extension (MOVSX regX, reg/memX)
+    //
+    case 0xBE:
+      Bytes = 1;
+    //
+    // fall through
+    //
+    case 0xBF:
+      CcDecodeModRm (Regs, InstructionData);
+      Bytes = (Bytes != 0) ? Bytes : 2;
+
+      Address = InstructionData->Ext.RmData;
+
+      if (Bytes == 1) {
+        UINT8  *Data;
+        Data     = (UINT8 *)&Val;
+        SignByte = ((*Data & BIT7) != 0) ? 0xFF : 0x00;
+      } else {
+        UINT16  *Data;
+        Data     = (UINT16 *)&Val;
+        SignByte = ((*Data & BIT15) != 0) ? 0xFF : 0x00;
+      }
+
+      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+      SetMem (Register, (UINTN)(1 << InstructionData->DataSize), SignByte);
+
+      ReadOrWrite = TDX_MMIO_READ;
+
       break;
+
+    default:
+      DEBUG ((DEBUG_ERROR, "%a: Invalid MMIO opcode (%x)\n", __FUNCTION__, OpCode));
+      Status = EFI_UNSUPPORTED;
+      ASSERT (FALSE);
+  }
+
+  if (!EFI_ERROR (Status)) {
+    ParsedInstruction->OpCode      = OpCode;
+    ParsedInstruction->Address     = Address;
+    ParsedInstruction->Bytes       = Bytes;
+    ParsedInstruction->Register    = Register;
+    ParsedInstruction->Val         = Val;
+    ParsedInstruction->ReadOrWrite = ReadOrWrite;
   }
 
-  return NULL;
+  return Status;
 }
 
 /**
@@ -300,25 +579,13 @@ MmioExit (
   IN TDCALL_VEINFO_RETURN_DATA   *Veinfo
   )
 {
-  UINT64          Status;
-  UINT32          MmioSize;
-  UINT32          RegSize;
-  UINT8           OpCode;
-  BOOLEAN         SeenRex;
-  UINT64          *Reg;
-  UINT8           *Rip;
-  UINT64          Val;
-  UINT32          OpSize;
-  MODRM           ModRm;
-  REX             Rex;
-  TD_RETURN_DATA  TdReturnData;
-  UINT8           Gpaw;
-  UINT64          TdSharedPageMask;
-
-  Rip     = (UINT8 *)Regs->Rip;
-  Val     = 0;
-  Rex.Val = 0;
-  SeenRex = FALSE;
+  UINT64                        Status;
+  TD_RETURN_DATA                TdReturnData;
+  UINT8                         Gpaw;
+  UINT64                        Val;
+  UINT64                        TdSharedPageMask;
+  CC_INSTRUCTION_DATA           InstructionData;
+  MMIO_EXIT_PARSED_INSTRUCTION  ParsedInstruction;
 
   Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
   if (Status == TDX_EXIT_REASON_SUCCESS) {
@@ -335,112 +602,41 @@ MmioExit (
     CpuDeadLoop ();
   }
 
-  //
-  // Default to 32bit transfer
-  //
-  OpSize = 4;
+  Status = ParseMmioExitInstructions (Regs, &InstructionData, &ParsedInstruction);
+  if (Status != EFI_SUCCESS) {
+    return Status;
+  }
 
-  do {
-    OpCode = *Rip++;
-    if (OpCode == 0x66) {
-      OpSize = 2;
-    } else if ((OpCode == 0x64) || (OpCode == 0x65) || (OpCode == 0x67)) {
-      continue;
-    } else if ((OpCode >= 0x40) && (OpCode <= 0x4f)) {
-      SeenRex = TRUE;
-      Rex.Val = OpCode;
-    } else {
-      break;
+  if (Veinfo->GuestPA != (ParsedInstruction.Address | TdSharedPageMask)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Address is not correct! (%d: 0x%llx != 0x%llx)\n",
+      __FUNCTION__,
+      ParsedInstruction.OpCode,
+      Veinfo->GuestPA,
+      ParsedInstruction.Address
+      ));
+    ASSERT (FALSE);
+    return EFI_ABORTED;
+  }
+
+  if (ParsedInstruction.ReadOrWrite == TDX_MMIO_WRITE ) {
+    Status = TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_WRITE, Veinfo->GuestPA, &ParsedInstruction.Val);
+  } else {
+    Val    = 0;
+    Status = TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_READ, Veinfo->GuestPA, &Val);
+    if (Status == 0) {
+      CopyMem (ParsedInstruction.Register, &Val, ParsedInstruction.Bytes);
     }
-  } while (TRUE);
-
-  //
-  // We need to have at least 2 more bytes for this instruction
-  //
-  TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 13);
-
-  OpCode = *Rip++;
-  //
-  // Two-byte opecode, get next byte
-  //
-  if (OpCode == 0x0F) {
-    OpCode = *Rip++;
-  }
-
-  switch (OpCode) {
-    case 0x88:
-    case 0x8A:
-    case 0xB6:
-      MmioSize = 1;
-      break;
-    case 0xB7:
-      MmioSize = 2;
-      break;
-    default:
-      MmioSize = Rex.Bits.W ? 8 : OpSize;
-      break;
-  }
-
-  /* Punt on AH/BH/CH/DH unless it shows up. */
-  ModRm.Val = *Rip++;
-  TDX_DECODER_BUG_ON (MmioSize == 1 && ModRm.Bits.Reg > 4 && !SeenRex && OpCode != 0xB6);
-  Reg = GetRegFromContext (Regs, ModRm.Bits.Reg | ((int)Rex.Bits.R << 3));
-  TDX_DECODER_BUG_ON (!Reg);
-
-  if (ModRm.Bits.Rm == 4) {
-    ++Rip; /* SIB byte */
-  }
-
-  if ((ModRm.Bits.Mod == 2) || ((ModRm.Bits.Mod == 0) && (ModRm.Bits.Rm == 5))) {
-    Rip += 4; /* DISP32 */
-  } else if (ModRm.Bits.Mod == 1) {
-    ++Rip;  /* DISP8 */
-  }
-
-  switch (OpCode) {
-    case 0x88:
-    case 0x89:
-      CopyMem ((void *)&Val, Reg, MmioSize);
-      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA, Val, 0);
-      break;
-    case 0xC7:
-      CopyMem ((void *)&Val, Rip, OpSize);
-      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA, Val, 0);
-      Rip   += OpSize;
-    default:
-      //
-      // 32-bit write registers are zero extended to the full register
-      // Hence 'MOVZX r[32/64], r/m16' is
-      // hardcoded to reg size 8, and the straight MOV case has a reg
-      // size of 8 in the 32-bit read case.
-      //
-      switch (OpCode) {
-        case 0xB6:
-          RegSize = Rex.Bits.W ? 8 : OpSize;
-          break;
-        case 0xB7:
-          RegSize =  8;
-          break;
-        default:
-          RegSize = MmioSize == 4 ? 8 : MmioSize;
-          break;
-      }
-
-      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 0, Veinfo->GuestPA, 0, &Val);
-      if (Status == 0) {
-        ZeroMem (Reg, RegSize);
-        CopyMem (Reg, (void *)&Val, MmioSize);
-      }
   }
 
   if (Status == 0) {
-    TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 15);
-
     //
     // We change instruction length to reflect true size so handler can
     // bump rip
     //
-    Veinfo->ExitInstructionLength =  (UINT32)((UINT64)Rip - Regs->Rip);
+    Veinfo->ExitInstructionLength =  (UINT32)(CcInstructionLength (&InstructionData));
+    TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo->ExitInstructionLength);
   }
 
   return Status;
-- 
2.29.2.windows.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 0/2] Refactor TDX MmioExit
  2022-12-29  8:55 [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
  2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
  2022-12-29  8:55 ` [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
@ 2022-12-29  9:00 ` Min Xu
  2 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2022-12-29  9:00 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Aktas, Erdem, James Bottomley, Yao, Jiewen, Gerd Hoffmann,
	Tom Lendacky, Ryan Afranji

Code: https://github.com/mxu9/edk2/tree/TdxMmioExit.v1

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Thursday, December 29, 2022 4:56 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Tom Lendacky <thomas.lendacky@amd.com>; Ryan Afranji
> <afranji@google.com>
> Subject: [PATCH V1 0/2] Refactor TDX MmioExit
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4169
> 
> The previous TDX MmioExit doesn't handle the Mmio instructions correctly in
> some scenarios. This patch-set refactors the implementation to fix the issues.
> 
> Before the refactoring, common X86 instruction codes in CcExitVcHandler.c
> are moved to separate files (CcInstruction.h / CcInstruction.c) so that these
> codes can be re-used in TDX.
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ryan Afranji <afranji@google.com>
> Reported-by: Ryan Afranji <afranji@google.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> 
> Min M Xu (2):
>   OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
>   OvmfPkg/CcExitLib: Refactor TDX MmioExit
> 
>  OvmfPkg/Library/CcExitLib/CcExitLib.inf     |   1 +
>  OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 697 +++-----------------
> OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498 +++++++++-----
>  OvmfPkg/Library/CcExitLib/CcInstruction.c   | 454 +++++++++++++
>  OvmfPkg/Library/CcExitLib/CcInstruction.h   | 197 ++++++
>  OvmfPkg/Library/CcExitLib/SecCcExitLib.inf  |   1 +
>  6 files changed, 1082 insertions(+), 766 deletions(-)  create mode 100644
> OvmfPkg/Library/CcExitLib/CcInstruction.c
>  create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.h
> 
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
  2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
@ 2023-01-03 20:27   ` Lendacky, Thomas
  2023-01-06  8:24   ` Yao, Jiewen
  1 sibling, 0 replies; 8+ messages in thread
From: Lendacky, Thomas @ 2023-01-03 20:27 UTC (permalink / raw)
  To: Min Xu, devel; +Cc: Gerd Hoffmann, Erdem Aktas, James Bottomley, Jiewen Yao

On 12/29/22 02:55, Min Xu wrote:
> From: Min M Xu <min.m.xu@intel.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4169
> 
> Move common X86 instruction codes from CcExitVcHandler.c to separate
> files (CcInstruction.h / CcInstruction.c) so that these codes can be
> re-used in TDX.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>   OvmfPkg/Library/CcExitLib/CcExitLib.inf     |   1 +
>   OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 697 +++-----------------
>   OvmfPkg/Library/CcExitLib/CcInstruction.c   | 454 +++++++++++++
>   OvmfPkg/Library/CcExitLib/CcInstruction.h   | 197 ++++++
>   OvmfPkg/Library/CcExitLib/SecCcExitLib.inf  |   1 +
>   5 files changed, 735 insertions(+), 615 deletions(-)
>   create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.c
>   create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.h
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
  2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
  2023-01-03 20:27   ` Lendacky, Thomas
@ 2023-01-06  8:24   ` Yao, Jiewen
  1 sibling, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2023-01-06  8:24 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Gerd Hoffmann, Aktas, Erdem, James Bottomley, Tom Lendacky

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Thursday, December 29, 2022 4:56 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Aktas, Erdem <erdemaktas@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>
> Subject: [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction
> code to separate file
> 
> From: Min M Xu <min.m.xu@intel.com>
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=4169
> 
> Move common X86 instruction codes from CcExitVcHandler.c to separate
> files (CcInstruction.h / CcInstruction.c) so that these codes can be
> re-used in TDX.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  OvmfPkg/Library/CcExitLib/CcExitLib.inf     |   1 +
>  OvmfPkg/Library/CcExitLib/CcExitVcHandler.c | 697 +++-----------------
>  OvmfPkg/Library/CcExitLib/CcInstruction.c   | 454 +++++++++++++
>  OvmfPkg/Library/CcExitLib/CcInstruction.h   | 197 ++++++
>  OvmfPkg/Library/CcExitLib/SecCcExitLib.inf  |   1 +
>  5 files changed, 735 insertions(+), 615 deletions(-)
>  create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.c
>  create mode 100644 OvmfPkg/Library/CcExitLib/CcInstruction.h
> 
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitLib.inf
> b/OvmfPkg/Library/CcExitLib/CcExitLib.inf
> index 131fa6267522..bc75cd5f5a04 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitLib.inf
> +++ b/OvmfPkg/Library/CcExitLib/CcExitLib.inf
> @@ -25,6 +25,7 @@
>    CcExitLib.c
>    CcExitVcHandler.c
>    CcExitVcHandler.h
> +  CcInstruction.c
>    PeiDxeCcExitVcHandler.c
>    CcExitVeHandler.c
>    X64/TdVmcallCpuid.nasm
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> index 985e5479775c..7fe11c53249e 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> +++ b/OvmfPkg/Library/CcExitLib/CcExitVcHandler.c
> @@ -17,107 +17,7 @@
>  #include <IndustryStandard/InstructionParsing.h>
> 
>  #include "CcExitVcHandler.h"
> -
> -//
> -// Instruction execution mode definition
> -//
> -typedef enum {
> -  LongMode64Bit = 0,
> -  LongModeCompat32Bit,
> -  LongModeCompat16Bit,
> -} SEV_ES_INSTRUCTION_MODE;
> -
> -//
> -// Instruction size definition (for operand and address)
> -//
> -typedef enum {
> -  Size8Bits = 0,
> -  Size16Bits,
> -  Size32Bits,
> -  Size64Bits,
> -} SEV_ES_INSTRUCTION_SIZE;
> -
> -//
> -// Intruction segment definition
> -//
> -typedef enum {
> -  SegmentEs = 0,
> -  SegmentCs,
> -  SegmentSs,
> -  SegmentDs,
> -  SegmentFs,
> -  SegmentGs,
> -} SEV_ES_INSTRUCTION_SEGMENT;
> -
> -//
> -// Instruction rep function definition
> -//
> -typedef enum {
> -  RepNone = 0,
> -  RepZ,
> -  RepNZ,
> -} SEV_ES_INSTRUCTION_REP;
> -
> -typedef struct {
> -  UINT8    Rm;
> -  UINT8    Reg;
> -  UINT8    Mod;
> -} SEV_ES_INSTRUCTION_MODRM_EXT;
> -
> -typedef struct {
> -  UINT8    Base;
> -  UINT8    Index;
> -  UINT8    Scale;
> -} SEV_ES_INSTRUCTION_SIB_EXT;
> -
> -//
> -// Instruction opcode definition
> -//
> -typedef struct {
> -  SEV_ES_INSTRUCTION_MODRM_EXT    ModRm;
> -
> -  SEV_ES_INSTRUCTION_SIB_EXT      Sib;
> -
> -  UINTN                           RegData;
> -  UINTN                           RmData;
> -} SEV_ES_INSTRUCTION_OPCODE_EXT;
> -
> -//
> -// Instruction parsing context definition
> -//
> -typedef struct {
> -  GHCB                             *Ghcb;
> -
> -  SEV_ES_INSTRUCTION_MODE          Mode;
> -  SEV_ES_INSTRUCTION_SIZE          DataSize;
> -  SEV_ES_INSTRUCTION_SIZE          AddrSize;
> -  BOOLEAN                          SegmentSpecified;
> -  SEV_ES_INSTRUCTION_SEGMENT       Segment;
> -  SEV_ES_INSTRUCTION_REP           RepMode;
> -
> -  UINT8                            *Begin;
> -  UINT8                            *End;
> -
> -  UINT8                            *Prefixes;
> -  UINT8                            *OpCodes;
> -  UINT8                            *Displacement;
> -  UINT8                            *Immediate;
> -
> -  INSTRUCTION_REX_PREFIX           RexPrefix;
> -
> -  BOOLEAN                          ModRmPresent;
> -  INSTRUCTION_MODRM                ModRm;
> -
> -  BOOLEAN                          SibPresent;
> -  INSTRUCTION_SIB                  Sib;
> -
> -  UINTN                            PrefixSize;
> -  UINTN                            OpCodeSize;
> -  UINTN                            DisplacementSize;
> -  UINTN                            ImmediateSize;
> -
> -  SEV_ES_INSTRUCTION_OPCODE_EXT    Ext;
> -} SEV_ES_INSTRUCTION_DATA;
> +#include "CcInstruction.h"
> 
>  //
>  // Non-automatic Exit function prototype
> @@ -125,9 +25,9 @@ typedef struct {
>  typedef
>  UINT64
>  (*NAE_EXIT) (
> -  GHCB                     *Ghcb,
> -  EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  GHCB                    *Ghcb,
> +  EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  CC_INSTRUCTION_DATA     *InstructionData
>    );
> 
>  //
> @@ -155,439 +55,6 @@ typedef PACKED struct {
>    SEV_SNP_CPUID_FUNCTION    function[0];
>  } SEV_SNP_CPUID_INFO;
> 
> -/**
> -  Return a pointer to the contents of the specified register.
> -
> -  Based upon the input register, return a pointer to the registers contents
> -  in the x86 processor context.
> -
> -  @param[in] Regs      x64 processor context
> -  @param[in] Register  Register to obtain pointer for
> -
> -  @return              Pointer to the contents of the requested register
> -
> -**/
> -STATIC
> -UINT64 *
> -GetRegisterPointer (
> -  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> -  IN UINT8                   Register
> -  )
> -{
> -  UINT64  *Reg;
> -
> -  switch (Register) {
> -    case 0:
> -      Reg = &Regs->Rax;
> -      break;
> -    case 1:
> -      Reg = &Regs->Rcx;
> -      break;
> -    case 2:
> -      Reg = &Regs->Rdx;
> -      break;
> -    case 3:
> -      Reg = &Regs->Rbx;
> -      break;
> -    case 4:
> -      Reg = &Regs->Rsp;
> -      break;
> -    case 5:
> -      Reg = &Regs->Rbp;
> -      break;
> -    case 6:
> -      Reg = &Regs->Rsi;
> -      break;
> -    case 7:
> -      Reg = &Regs->Rdi;
> -      break;
> -    case 8:
> -      Reg = &Regs->R8;
> -      break;
> -    case 9:
> -      Reg = &Regs->R9;
> -      break;
> -    case 10:
> -      Reg = &Regs->R10;
> -      break;
> -    case 11:
> -      Reg = &Regs->R11;
> -      break;
> -    case 12:
> -      Reg = &Regs->R12;
> -      break;
> -    case 13:
> -      Reg = &Regs->R13;
> -      break;
> -    case 14:
> -      Reg = &Regs->R14;
> -      break;
> -    case 15:
> -      Reg = &Regs->R15;
> -      break;
> -    default:
> -      Reg = NULL;
> -  }
> -
> -  ASSERT (Reg != NULL);
> -
> -  return Reg;
> -}
> -
> -/**
> -  Update the instruction parsing context for displacement bytes.
> -
> -  @param[in, out] InstructionData  Instruction parsing context
> -  @param[in]      Size             The instruction displacement size
> -
> -**/
> -STATIC
> -VOID
> -UpdateForDisplacement (
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData,
> -  IN     UINTN                    Size
> -  )
> -{
> -  InstructionData->DisplacementSize = Size;
> -  InstructionData->Immediate       += Size;
> -  InstructionData->End             += Size;
> -}
> -
> -/**
> -  Determine if an instruction address if RIP relative.
> -
> -  Examine the instruction parsing context to determine if the address offset
> -  is relative to the instruction pointer.
> -
> -  @param[in] InstructionData  Instruction parsing context
> -
> -  @retval TRUE                Instruction addressing is RIP relative
> -  @retval FALSE               Instruction addressing is not RIP relative
> -
> -**/
> -STATIC
> -BOOLEAN
> -IsRipRelative (
> -  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
> -  )
> -{
> -  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
> -
> -  Ext = &InstructionData->Ext;
> -
> -  return ((InstructionData->Mode == LongMode64Bit) &&
> -          (Ext->ModRm.Mod == 0) &&
> -          (Ext->ModRm.Rm == 5)  &&
> -          (InstructionData->SibPresent == FALSE));
> -}
> -
> -/**
> -  Return the effective address of a memory operand.
> -
> -  Examine the instruction parsing context to obtain the effective memory
> -  address of a memory operand.
> -
> -  @param[in] Regs             x64 processor context
> -  @param[in] InstructionData  Instruction parsing context
> -
> -  @return                     The memory operand effective address
> -
> -**/
> -STATIC
> -UINT64
> -GetEffectiveMemoryAddress (
> -  IN EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
> -  )
> -{
> -  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
> -  UINT64                         EffectiveAddress;
> -
> -  Ext              = &InstructionData->Ext;
> -  EffectiveAddress = 0;
> -
> -  if (IsRipRelative (InstructionData)) {
> -    //
> -    // RIP-relative displacement is a 32-bit signed value
> -    //
> -    INT32  RipRelative;
> -
> -    RipRelative = *(INT32 *)InstructionData->Displacement;
> -
> -    UpdateForDisplacement (InstructionData, 4);
> -
> -    //
> -    // Negative displacement is handled by standard UINT64 wrap-around.
> -    //
> -    return Regs->Rip + (UINT64)RipRelative;
> -  }
> -
> -  switch (Ext->ModRm.Mod) {
> -    case 1:
> -      UpdateForDisplacement (InstructionData, 1);
> -      EffectiveAddress += (UINT64)(*(INT8 *)(InstructionData->Displacement));
> -      break;
> -    case 2:
> -      switch (InstructionData->AddrSize) {
> -        case Size16Bits:
> -          UpdateForDisplacement (InstructionData, 2);
> -          EffectiveAddress += (UINT64)(*(INT16 *)(InstructionData-
> >Displacement));
> -          break;
> -        default:
> -          UpdateForDisplacement (InstructionData, 4);
> -          EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData-
> >Displacement));
> -          break;
> -      }
> -
> -      break;
> -  }
> -
> -  if (InstructionData->SibPresent) {
> -    INT64  Displacement;
> -
> -    if (Ext->Sib.Index != 4) {
> -      CopyMem (
> -        &Displacement,
> -        GetRegisterPointer (Regs, Ext->Sib.Index),
> -        sizeof (Displacement)
> -        );
> -      Displacement *= (INT64)(1 << Ext->Sib.Scale);
> -
> -      //
> -      // Negative displacement is handled by standard UINT64 wrap-around.
> -      //
> -      EffectiveAddress += (UINT64)Displacement;
> -    }
> -
> -    if ((Ext->Sib.Base != 5) || Ext->ModRm.Mod) {
> -      EffectiveAddress += *GetRegisterPointer (Regs, Ext->Sib.Base);
> -    } else {
> -      UpdateForDisplacement (InstructionData, 4);
> -      EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData-
> >Displacement));
> -    }
> -  } else {
> -    EffectiveAddress += *GetRegisterPointer (Regs, Ext->ModRm.Rm);
> -  }
> -
> -  return EffectiveAddress;
> -}
> -
> -/**
> -  Decode a ModRM byte.
> -
> -  Examine the instruction parsing context to decode a ModRM byte and the
> SIB
> -  byte, if present.
> -
> -  @param[in]      Regs             x64 processor context
> -  @param[in, out] InstructionData  Instruction parsing context
> -
> -**/
> -STATIC
> -VOID
> -DecodeModRm (
> -  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
> -  )
> -{
> -  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
> -  INSTRUCTION_REX_PREFIX         *RexPrefix;
> -  INSTRUCTION_MODRM              *ModRm;
> -  INSTRUCTION_SIB                *Sib;
> -
> -  RexPrefix = &InstructionData->RexPrefix;
> -  Ext       = &InstructionData->Ext;
> -  ModRm     = &InstructionData->ModRm;
> -  Sib       = &InstructionData->Sib;
> -
> -  InstructionData->ModRmPresent = TRUE;
> -  ModRm->Uint8                  = *(InstructionData->End);
> -
> -  InstructionData->Displacement++;
> -  InstructionData->Immediate++;
> -  InstructionData->End++;
> -
> -  Ext->ModRm.Mod = ModRm->Bits.Mod;
> -  Ext->ModRm.Reg = (RexPrefix->Bits.BitR << 3) | ModRm->Bits.Reg;
> -  Ext->ModRm.Rm  = (RexPrefix->Bits.BitB << 3) | ModRm->Bits.Rm;
> -
> -  Ext->RegData = *GetRegisterPointer (Regs, Ext->ModRm.Reg);
> -
> -  if (Ext->ModRm.Mod == 3) {
> -    Ext->RmData = *GetRegisterPointer (Regs, Ext->ModRm.Rm);
> -  } else {
> -    if (ModRm->Bits.Rm == 4) {
> -      InstructionData->SibPresent = TRUE;
> -      Sib->Uint8                  = *(InstructionData->End);
> -
> -      InstructionData->Displacement++;
> -      InstructionData->Immediate++;
> -      InstructionData->End++;
> -
> -      Ext->Sib.Scale = Sib->Bits.Scale;
> -      Ext->Sib.Index = (RexPrefix->Bits.BitX << 3) | Sib->Bits.Index;
> -      Ext->Sib.Base  = (RexPrefix->Bits.BitB << 3) | Sib->Bits.Base;
> -    }
> -
> -    Ext->RmData = GetEffectiveMemoryAddress (Regs, InstructionData);
> -  }
> -}
> -
> -/**
> -  Decode instruction prefixes.
> -
> -  Parse the instruction data to track the instruction prefixes that have
> -  been used.
> -
> -  @param[in]      Regs             x64 processor context
> -  @param[in, out] InstructionData  Instruction parsing context
> -
> -**/
> -STATIC
> -VOID
> -DecodePrefixes (
> -  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
> -  )
> -{
> -  SEV_ES_INSTRUCTION_MODE  Mode;
> -  SEV_ES_INSTRUCTION_SIZE  ModeDataSize;
> -  SEV_ES_INSTRUCTION_SIZE  ModeAddrSize;
> -  UINT8                    *Byte;
> -
> -  //
> -  // Always in 64-bit mode
> -  //
> -  Mode         = LongMode64Bit;
> -  ModeDataSize = Size32Bits;
> -  ModeAddrSize = Size64Bits;
> -
> -  InstructionData->Mode     = Mode;
> -  InstructionData->DataSize = ModeDataSize;
> -  InstructionData->AddrSize = ModeAddrSize;
> -
> -  InstructionData->Prefixes = InstructionData->Begin;
> -
> -  Byte = InstructionData->Prefixes;
> -  for ( ; ; Byte++, InstructionData->PrefixSize++) {
> -    //
> -    // Check the 0x40 to 0x4F range using an if statement here since some
> -    // compilers don't like the "case 0x40 ... 0x4F:" syntax. This avoids
> -    // 16 case statements below.
> -    //
> -    if ((*Byte >= REX_PREFIX_START) && (*Byte <= REX_PREFIX_STOP)) {
> -      InstructionData->RexPrefix.Uint8 = *Byte;
> -      if ((*Byte & REX_64BIT_OPERAND_SIZE_MASK) != 0) {
> -        InstructionData->DataSize = Size64Bits;
> -      }
> -
> -      continue;
> -    }
> -
> -    switch (*Byte) {
> -      case OVERRIDE_SEGMENT_CS:
> -      case OVERRIDE_SEGMENT_DS:
> -      case OVERRIDE_SEGMENT_ES:
> -      case OVERRIDE_SEGMENT_SS:
> -        if (Mode != LongMode64Bit) {
> -          InstructionData->SegmentSpecified = TRUE;
> -          InstructionData->Segment          = (*Byte >> 3) & 3;
> -        }
> -
> -        break;
> -
> -      case OVERRIDE_SEGMENT_FS:
> -      case OVERRIDE_SEGMENT_GS:
> -        InstructionData->SegmentSpecified = TRUE;
> -        InstructionData->Segment          = *Byte & 7;
> -        break;
> -
> -      case OVERRIDE_OPERAND_SIZE:
> -        if (InstructionData->RexPrefix.Uint8 == 0) {
> -          InstructionData->DataSize =
> -            (Mode == LongMode64Bit)       ? Size16Bits :
> -            (Mode == LongModeCompat32Bit) ? Size16Bits :
> -            (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
> -        }
> -
> -        break;
> -
> -      case OVERRIDE_ADDRESS_SIZE:
> -        InstructionData->AddrSize =
> -          (Mode == LongMode64Bit)       ? Size32Bits :
> -          (Mode == LongModeCompat32Bit) ? Size16Bits :
> -          (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
> -        break;
> -
> -      case LOCK_PREFIX:
> -        break;
> -
> -      case REPZ_PREFIX:
> -        InstructionData->RepMode = RepZ;
> -        break;
> -
> -      case REPNZ_PREFIX:
> -        InstructionData->RepMode = RepNZ;
> -        break;
> -
> -      default:
> -        InstructionData->OpCodes    = Byte;
> -        InstructionData->OpCodeSize = (*Byte ==
> TWO_BYTE_OPCODE_ESCAPE) ? 2 : 1;
> -
> -        InstructionData->End          = Byte + InstructionData->OpCodeSize;
> -        InstructionData->Displacement = InstructionData->End;
> -        InstructionData->Immediate    = InstructionData->End;
> -        return;
> -    }
> -  }
> -}
> -
> -/**
> -  Determine instruction length
> -
> -  Return the total length of the parsed instruction.
> -
> -  @param[in] InstructionData  Instruction parsing context
> -
> -  @return                     Length of parsed instruction
> -
> -**/
> -STATIC
> -UINT64
> -InstructionLength (
> -  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
> -  )
> -{
> -  return (UINT64)(InstructionData->End - InstructionData->Begin);
> -}
> -
> -/**
> -  Initialize the instruction parsing context.
> -
> -  Initialize the instruction parsing context, which includes decoding the
> -  instruction prefixes.
> -
> -  @param[in, out] InstructionData  Instruction parsing context
> -  @param[in]      Ghcb             Pointer to the Guest-Hypervisor
> Communication
> -                                   Block
> -  @param[in]      Regs             x64 processor context
> -
> -**/
> -STATIC
> -VOID
> -InitInstructionData (
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData,
> -  IN     GHCB                     *Ghcb,
> -  IN     EFI_SYSTEM_CONTEXT_X64   *Regs
> -  )
> -{
> -  SetMem (InstructionData, sizeof (*InstructionData), 0);
> -  InstructionData->Ghcb  = Ghcb;
> -  InstructionData->Begin = (UINT8 *)Regs->Rip;
> -  InstructionData->End   = (UINT8 *)Regs->Rip;
> -
> -  DecodePrefixes (Regs, InstructionData);
> -}
> -
>  /**
>    Report an unsupported event to the hypervisor
> 
> @@ -604,9 +71,9 @@ InitInstructionData (
>  STATIC
>  UINT64
>  UnsupportedExit (
> -  IN GHCB                     *Ghcb,
> -  IN EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN GHCB                    *Ghcb,
> +  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  Status;
> @@ -703,9 +170,9 @@ ValidateMmioMemory (
>  STATIC
>  UINT64
>  MmioExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  ExitInfo1, ExitInfo2, Status;
> @@ -731,7 +198,7 @@ MmioExit (
>      // fall through
>      //
>      case 0x89:
> -      DecodeModRm (Regs, InstructionData);
> +      CcDecodeModRm (Regs, InstructionData);
>        Bytes = ((Bytes != 0) ? Bytes :
>                 (InstructionData->DataSize == Size16Bits) ? 2 :
>                 (InstructionData->DataSize == Size32Bits) ? 4 :
> @@ -824,7 +291,7 @@ MmioExit (
>      // fall through
>      //
>      case 0xC7:
> -      DecodeModRm (Regs, InstructionData);
> +      CcDecodeModRm (Regs, InstructionData);
>        Bytes = ((Bytes != 0) ? Bytes :
>                 (InstructionData->DataSize == Size16Bits) ? 2 :
>                 (InstructionData->DataSize == Size32Bits) ? 4 :
> @@ -860,7 +327,7 @@ MmioExit (
>      // fall through
>      //
>      case 0x8B:
> -      DecodeModRm (Regs, InstructionData);
> +      CcDecodeModRm (Regs, InstructionData);
>        Bytes = ((Bytes != 0) ? Bytes :
>                 (InstructionData->DataSize == Size16Bits) ? 2 :
>                 (InstructionData->DataSize == Size32Bits) ? 4 :
> @@ -888,7 +355,7 @@ MmioExit (
>          return Status;
>        }
> 
> -      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
>        if (Bytes == 4) {
>          //
>          // Zero-extend for 32-bit operation
> @@ -967,7 +434,7 @@ MmioExit (
>      // fall through
>      //
>      case 0xB7:
> -      DecodeModRm (Regs, InstructionData);
> +      CcDecodeModRm (Regs, InstructionData);
>        Bytes = (Bytes != 0) ? Bytes : 2;
> 
>        Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData,
> Bytes);
> @@ -985,7 +452,7 @@ MmioExit (
>          return Status;
>        }
> 
> -      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
>        SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0);
>        CopyMem (Register, Ghcb->SharedBuffer, Bytes);
>        break;
> @@ -999,7 +466,7 @@ MmioExit (
>      // fall through
>      //
>      case 0xBF:
> -      DecodeModRm (Regs, InstructionData);
> +      CcDecodeModRm (Regs, InstructionData);
>        Bytes = (Bytes != 0) ? Bytes : 2;
> 
>        Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData,
> Bytes);
> @@ -1029,7 +496,7 @@ MmioExit (
>          SignByte = ((*Data & BIT15) != 0) ? 0xFF : 0x00;
>        }
> 
> -      Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
>        SetMem (Register, (UINTN)(1 << InstructionData->DataSize), SignByte);
>        CopyMem (Register, Ghcb->SharedBuffer, Bytes);
>        break;
> @@ -1060,12 +527,12 @@ MmioExit (
>  STATIC
>  UINT64
>  MwaitExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    Ghcb->SaveArea.Rax = Regs->Rax;
>    CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> @@ -1092,12 +559,12 @@ MwaitExit (
>  STATIC
>  UINT64
>  MonitorExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    Ghcb->SaveArea.Rax = Regs->Rax;  // Identity mapped, so VA = PA
>    CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> @@ -1126,9 +593,9 @@ MonitorExit (
>  STATIC
>  UINT64
>  WbinvdExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    return CcExitVmgExit (Ghcb, SVM_EXIT_WBINVD, 0, 0);
> @@ -1151,14 +618,14 @@ WbinvdExit (
>  STATIC
>  UINT64
>  RdtscpExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  Status;
> 
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    Status = CcExitVmgExit (Ghcb, SVM_EXIT_RDTSCP, 0, 0);
>    if (Status != 0) {
> @@ -1196,14 +663,14 @@ RdtscpExit (
>  STATIC
>  UINT64
>  VmmCallExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  Status;
> 
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    Ghcb->SaveArea.Rax = Regs->Rax;
>    CcExitVmgSetOffsetValid (Ghcb, GhcbRax);
> @@ -1241,9 +708,9 @@ VmmCallExit (
>  STATIC
>  UINT64
>  MsrExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  ExitInfo1, Status;
> @@ -1302,8 +769,8 @@ MsrExit (
>  STATIC
>  UINT64
>  IoioExitInfo (
> -  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  ExitInfo;
> @@ -1437,9 +904,9 @@ IoioExitInfo (
>  STATIC
>  UINT64
>  IoioExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64   ExitInfo1, ExitInfo2, Status;
> @@ -1531,9 +998,9 @@ IoioExit (
>  STATIC
>  UINT64
>  InvdExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    return CcExitVmgExit (Ghcb, SVM_EXIT_INVD, 0, 0);
> @@ -1949,9 +1416,9 @@ Out:
>  STATIC
>  UINT64
>  CpuidExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    BOOLEAN  Unsupported;
> @@ -2041,9 +1508,9 @@ CpuidFail:
>  STATIC
>  UINT64
>  RdpmcExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  Status;
> @@ -2085,9 +1552,9 @@ RdpmcExit (
>  STATIC
>  UINT64
>  RdtscExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
>    UINT64  Status;
> @@ -2126,25 +1593,25 @@ RdtscExit (
>  STATIC
>  UINT64
>  Dr7WriteExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
> -  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
> -  SEV_ES_PER_CPU_DATA            *SevEsData;
> -  UINT64                         *Register;
> -  UINT64                         Status;
> +  CC_INSTRUCTION_OPCODE_EXT  *Ext;
> +  SEV_ES_PER_CPU_DATA        *SevEsData;
> +  UINT64                     *Register;
> +  UINT64                     Status;
> 
>    Ext       = &InstructionData->Ext;
>    SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
> 
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    //
>    // MOV DRn always treats MOD == 3 no matter how encoded
>    //
> -  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
> +  Register = CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
> 
>    //
>    // Using a value of 0 for ExitInfo1 means RAX holds the value
> @@ -2179,24 +1646,24 @@ Dr7WriteExit (
>  STATIC
>  UINT64
>  Dr7ReadExit (
> -  IN OUT GHCB                     *Ghcb,
> -  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
> -  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
> +  IN OUT GHCB                    *Ghcb,
> +  IN OUT EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN     CC_INSTRUCTION_DATA     *InstructionData
>    )
>  {
> -  SEV_ES_INSTRUCTION_OPCODE_EXT  *Ext;
> -  SEV_ES_PER_CPU_DATA            *SevEsData;
> -  UINT64                         *Register;
> +  CC_INSTRUCTION_OPCODE_EXT  *Ext;
> +  SEV_ES_PER_CPU_DATA        *SevEsData;
> +  UINT64                     *Register;
> 
>    Ext       = &InstructionData->Ext;
>    SevEsData = (SEV_ES_PER_CPU_DATA *)(Ghcb + 1);
> 
> -  DecodeModRm (Regs, InstructionData);
> +  CcDecodeModRm (Regs, InstructionData);
> 
>    //
>    // MOV DRn always treats MOD == 3 no matter how encoded
>    //
> -  Register = GetRegisterPointer (Regs, Ext->ModRm.Rm);
> +  Register = CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
> 
>    //
>    // If there is a cached valued for DR7, return that. Otherwise return the
> @@ -2232,12 +1699,12 @@ InternalVmgExitHandleVc (
>    IN OUT EFI_SYSTEM_CONTEXT  SystemContext
>    )
>  {
> -  EFI_SYSTEM_CONTEXT_X64   *Regs;
> -  NAE_EXIT                 NaeExit;
> -  SEV_ES_INSTRUCTION_DATA  InstructionData;
> -  UINT64                   ExitCode, Status;
> -  EFI_STATUS               VcRet;
> -  BOOLEAN                  InterruptState;
> +  EFI_SYSTEM_CONTEXT_X64  *Regs;
> +  NAE_EXIT                NaeExit;
> +  CC_INSTRUCTION_DATA     InstructionData;
> +  UINT64                  ExitCode, Status;
> +  EFI_STATUS              VcRet;
> +  BOOLEAN                 InterruptState;
> 
>    VcRet = EFI_SUCCESS;
> 
> @@ -2307,11 +1774,11 @@ InternalVmgExitHandleVc (
>        NaeExit = UnsupportedExit;
>    }
> 
> -  InitInstructionData (&InstructionData, Ghcb, Regs);
> +  CcInitInstructionData (&InstructionData, Ghcb, Regs);
> 
>    Status = NaeExit (Ghcb, Regs, &InstructionData);
>    if (Status == 0) {
> -    Regs->Rip += InstructionLength (&InstructionData);
> +    Regs->Rip += CcInstructionLength (&InstructionData);
>    } else {
>      GHCB_EVENT_INJECTION  Event;
> 
> diff --git a/OvmfPkg/Library/CcExitLib/CcInstruction.c
> b/OvmfPkg/Library/CcExitLib/CcInstruction.c
> new file mode 100644
> index 000000000000..0fb54b3ed553
> --- /dev/null
> +++ b/OvmfPkg/Library/CcExitLib/CcInstruction.c
> @@ -0,0 +1,454 @@
> +/** @file
> +  X64 Instruction function.
> +
> +  Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Uefi.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Register/Intel/Cpuid.h>
> +#include <IndustryStandard/InstructionParsing.h>
> +#include "CcInstruction.h"
> +
> +#define MAX_INSTRUCTION_LENGTH  15
> +
> +/**
> +  Return a pointer to the contents of the specified register.
> +
> +  Based upon the input register, return a pointer to the registers contents
> +  in the x86 processor context.
> +
> +  @param[in] Regs      x64 processor context
> +  @param[in] Register  Register to obtain pointer for
> +
> +  @return              Pointer to the contents of the requested register
> +
> +**/
> +UINT64 *
> +CcGetRegisterPointer (
> +  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN UINT8                   Register
> +  )
> +{
> +  UINT64  *Reg;
> +
> +  switch (Register) {
> +    case 0:
> +      Reg = &Regs->Rax;
> +      break;
> +    case 1:
> +      Reg = &Regs->Rcx;
> +      break;
> +    case 2:
> +      Reg = &Regs->Rdx;
> +      break;
> +    case 3:
> +      Reg = &Regs->Rbx;
> +      break;
> +    case 4:
> +      Reg = &Regs->Rsp;
> +      break;
> +    case 5:
> +      Reg = &Regs->Rbp;
> +      break;
> +    case 6:
> +      Reg = &Regs->Rsi;
> +      break;
> +    case 7:
> +      Reg = &Regs->Rdi;
> +      break;
> +    case 8:
> +      Reg = &Regs->R8;
> +      break;
> +    case 9:
> +      Reg = &Regs->R9;
> +      break;
> +    case 10:
> +      Reg = &Regs->R10;
> +      break;
> +    case 11:
> +      Reg = &Regs->R11;
> +      break;
> +    case 12:
> +      Reg = &Regs->R12;
> +      break;
> +    case 13:
> +      Reg = &Regs->R13;
> +      break;
> +    case 14:
> +      Reg = &Regs->R14;
> +      break;
> +    case 15:
> +      Reg = &Regs->R15;
> +      break;
> +    default:
> +      Reg = NULL;
> +  }
> +
> +  ASSERT (Reg != NULL);
> +
> +  return Reg;
> +}
> +
> +/**
> +  Update the instruction parsing context for displacement bytes.
> +
> +  @param[in, out] InstructionData  Instruction parsing context
> +  @param[in]      Size             The instruction displacement size
> +
> +**/
> +STATIC
> +VOID
> +UpdateForDisplacement (
> +  IN OUT CC_INSTRUCTION_DATA  *InstructionData,
> +  IN     UINTN                Size
> +  )
> +{
> +  InstructionData->DisplacementSize = Size;
> +  InstructionData->Immediate       += Size;
> +  InstructionData->End             += Size;
> +}
> +
> +/**
> +  Determine if an instruction address if RIP relative.
> +
> +  Examine the instruction parsing context to determine if the address offset
> +  is relative to the instruction pointer.
> +
> +  @param[in] InstructionData  Instruction parsing context
> +
> +  @retval TRUE                Instruction addressing is RIP relative
> +  @retval FALSE               Instruction addressing is not RIP relative
> +
> +**/
> +STATIC
> +BOOLEAN
> +IsRipRelative (
> +  IN CC_INSTRUCTION_DATA  *InstructionData
> +  )
> +{
> +  CC_INSTRUCTION_OPCODE_EXT  *Ext;
> +
> +  Ext = &InstructionData->Ext;
> +
> +  return ((InstructionData->Mode == LongMode64Bit) &&
> +          (Ext->ModRm.Mod == 0) &&
> +          (Ext->ModRm.Rm == 5)  &&
> +          (InstructionData->SibPresent == FALSE));
> +}
> +
> +/**
> +  Return the effective address of a memory operand.
> +
> +  Examine the instruction parsing context to obtain the effective memory
> +  address of a memory operand.
> +
> +  @param[in] Regs             x64 processor context
> +  @param[in] InstructionData  Instruction parsing context
> +
> +  @return                     The memory operand effective address
> +
> +**/
> +STATIC
> +UINT64
> +GetEffectiveMemoryAddress (
> +  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN CC_INSTRUCTION_DATA     *InstructionData
> +  )
> +{
> +  CC_INSTRUCTION_OPCODE_EXT  *Ext;
> +  UINT64                     EffectiveAddress;
> +
> +  Ext              = &InstructionData->Ext;
> +  EffectiveAddress = 0;
> +
> +  if (IsRipRelative (InstructionData)) {
> +    //
> +    // RIP-relative displacement is a 32-bit signed value
> +    //
> +    INT32  RipRelative;
> +
> +    RipRelative = *(INT32 *)InstructionData->Displacement;
> +
> +    UpdateForDisplacement (InstructionData, 4);
> +
> +    //
> +    // Negative displacement is handled by standard UINT64 wrap-around.
> +    //
> +    return Regs->Rip + (UINT64)RipRelative;
> +  }
> +
> +  switch (Ext->ModRm.Mod) {
> +    case 1:
> +      UpdateForDisplacement (InstructionData, 1);
> +      EffectiveAddress += (UINT64)(*(INT8 *)(InstructionData->Displacement));
> +      break;
> +    case 2:
> +      switch (InstructionData->AddrSize) {
> +        case Size16Bits:
> +          UpdateForDisplacement (InstructionData, 2);
> +          EffectiveAddress += (UINT64)(*(INT16 *)(InstructionData-
> >Displacement));
> +          break;
> +        default:
> +          UpdateForDisplacement (InstructionData, 4);
> +          EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData-
> >Displacement));
> +          break;
> +      }
> +
> +      break;
> +  }
> +
> +  if (InstructionData->SibPresent) {
> +    INT64  Displacement;
> +
> +    if (Ext->Sib.Index != 4) {
> +      CopyMem (
> +        &Displacement,
> +        CcGetRegisterPointer (Regs, Ext->Sib.Index),
> +        sizeof (Displacement)
> +        );
> +      Displacement *= (INT64)(1 << Ext->Sib.Scale);
> +
> +      //
> +      // Negative displacement is handled by standard UINT64 wrap-around.
> +      //
> +      EffectiveAddress += (UINT64)Displacement;
> +    }
> +
> +    if ((Ext->Sib.Base != 5) || Ext->ModRm.Mod) {
> +      EffectiveAddress += *CcGetRegisterPointer (Regs, Ext->Sib.Base);
> +    } else {
> +      UpdateForDisplacement (InstructionData, 4);
> +      EffectiveAddress += (UINT64)(*(INT32 *)(InstructionData-
> >Displacement));
> +    }
> +  } else {
> +    EffectiveAddress += *CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
> +  }
> +
> +  return EffectiveAddress;
> +}
> +
> +/**
> +  Decode a ModRM byte.
> +
> +  Examine the instruction parsing context to decode a ModRM byte and the
> SIB
> +  byte, if present.
> +
> +  @param[in]      Regs             x64 processor context
> +  @param[in, out] InstructionData  Instruction parsing context
> +
> +**/
> +VOID
> +CcDecodeModRm (
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData
> +  )
> +{
> +  CC_INSTRUCTION_OPCODE_EXT  *Ext;
> +  INSTRUCTION_REX_PREFIX     *RexPrefix;
> +  INSTRUCTION_MODRM          *ModRm;
> +  INSTRUCTION_SIB            *Sib;
> +
> +  RexPrefix = &InstructionData->RexPrefix;
> +  Ext       = &InstructionData->Ext;
> +  ModRm     = &InstructionData->ModRm;
> +  Sib       = &InstructionData->Sib;
> +
> +  InstructionData->ModRmPresent = TRUE;
> +  ModRm->Uint8                  = *(InstructionData->End);
> +
> +  InstructionData->Displacement++;
> +  InstructionData->Immediate++;
> +  InstructionData->End++;
> +
> +  Ext->ModRm.Mod = ModRm->Bits.Mod;
> +  Ext->ModRm.Reg = (RexPrefix->Bits.BitR << 3) | ModRm->Bits.Reg;
> +  Ext->ModRm.Rm  = (RexPrefix->Bits.BitB << 3) | ModRm->Bits.Rm;
> +
> +  Ext->RegData = *CcGetRegisterPointer (Regs, Ext->ModRm.Reg);
> +
> +  if (Ext->ModRm.Mod == 3) {
> +    Ext->RmData = *CcGetRegisterPointer (Regs, Ext->ModRm.Rm);
> +  } else {
> +    if (ModRm->Bits.Rm == 4) {
> +      InstructionData->SibPresent = TRUE;
> +      Sib->Uint8                  = *(InstructionData->End);
> +
> +      InstructionData->Displacement++;
> +      InstructionData->Immediate++;
> +      InstructionData->End++;
> +
> +      Ext->Sib.Scale = Sib->Bits.Scale;
> +      Ext->Sib.Index = (RexPrefix->Bits.BitX << 3) | Sib->Bits.Index;
> +      Ext->Sib.Base  = (RexPrefix->Bits.BitB << 3) | Sib->Bits.Base;
> +    }
> +
> +    Ext->RmData = GetEffectiveMemoryAddress (Regs, InstructionData);
> +  }
> +}
> +
> +/**
> +  Decode instruction prefixes.
> +
> +  Parse the instruction data to track the instruction prefixes that have
> +  been used.
> +
> +  @param[in]      Regs             x64 processor context
> +  @param[in, out] InstructionData  Instruction parsing context
> +
> +  @retval         EFI_SUCCESS      Successfully decode Prefixes
> +  @retval         Others           Other error as indicated
> +**/
> +STATIC
> +EFI_STATUS
> +DecodePrefixes (
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData
> +  )
> +{
> +  CC_INSTRUCTION_MODE  Mode;
> +  CC_INSTRUCTION_SIZE  ModeDataSize;
> +  CC_INSTRUCTION_SIZE  ModeAddrSize;
> +  UINT8                *Byte;
> +  UINT8                ParsedLength;
> +
> +  ParsedLength = 0;
> +
> +  //
> +  // Always in 64-bit mode
> +  //
> +  Mode         = LongMode64Bit;
> +  ModeDataSize = Size32Bits;
> +  ModeAddrSize = Size64Bits;
> +
> +  InstructionData->Mode     = Mode;
> +  InstructionData->DataSize = ModeDataSize;
> +  InstructionData->AddrSize = ModeAddrSize;
> +
> +  InstructionData->Prefixes = InstructionData->Begin;
> +
> +  Byte = InstructionData->Prefixes;
> +  for ( ; ParsedLength <= MAX_INSTRUCTION_LENGTH; Byte++,
> InstructionData->PrefixSize++, ParsedLength++) {
> +    //
> +    // Check the 0x40 to 0x4F range using an if statement here since some
> +    // compilers don't like the "case 0x40 ... 0x4F:" syntax. This avoids
> +    // 16 case statements below.
> +    //
> +    if ((*Byte >= REX_PREFIX_START) && (*Byte <= REX_PREFIX_STOP)) {
> +      InstructionData->RexPrefix.Uint8 = *Byte;
> +      if ((*Byte & REX_64BIT_OPERAND_SIZE_MASK) != 0) {
> +        InstructionData->DataSize = Size64Bits;
> +      }
> +
> +      continue;
> +    }
> +
> +    switch (*Byte) {
> +      case OVERRIDE_SEGMENT_CS:
> +      case OVERRIDE_SEGMENT_DS:
> +      case OVERRIDE_SEGMENT_ES:
> +      case OVERRIDE_SEGMENT_SS:
> +        if (Mode != LongMode64Bit) {
> +          InstructionData->SegmentSpecified = TRUE;
> +          InstructionData->Segment          = (*Byte >> 3) & 3;
> +        }
> +
> +        break;
> +
> +      case OVERRIDE_SEGMENT_FS:
> +      case OVERRIDE_SEGMENT_GS:
> +        InstructionData->SegmentSpecified = TRUE;
> +        InstructionData->Segment          = *Byte & 7;
> +        break;
> +
> +      case OVERRIDE_OPERAND_SIZE:
> +        if (InstructionData->RexPrefix.Uint8 == 0) {
> +          InstructionData->DataSize =
> +            (Mode == LongMode64Bit)       ? Size16Bits :
> +            (Mode == LongModeCompat32Bit) ? Size16Bits :
> +            (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
> +        }
> +
> +        break;
> +
> +      case OVERRIDE_ADDRESS_SIZE:
> +        InstructionData->AddrSize =
> +          (Mode == LongMode64Bit)       ? Size32Bits :
> +          (Mode == LongModeCompat32Bit) ? Size16Bits :
> +          (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
> +        break;
> +
> +      case LOCK_PREFIX:
> +        break;
> +
> +      case REPZ_PREFIX:
> +        InstructionData->RepMode = RepZ;
> +        break;
> +
> +      case REPNZ_PREFIX:
> +        InstructionData->RepMode = RepNZ;
> +        break;
> +
> +      default:
> +        InstructionData->OpCodes    = Byte;
> +        InstructionData->OpCodeSize = (*Byte ==
> TWO_BYTE_OPCODE_ESCAPE) ? 2 : 1;
> +
> +        InstructionData->End          = Byte + InstructionData->OpCodeSize;
> +        InstructionData->Displacement = InstructionData->End;
> +        InstructionData->Immediate    = InstructionData->End;
> +        return EFI_SUCCESS;
> +    }
> +  }
> +
> +  return EFI_ABORTED;
> +}
> +
> +/**
> +  Determine instruction length
> +
> +  Return the total length of the parsed instruction.
> +
> +  @param[in] InstructionData  Instruction parsing context
> +
> +  @return                     Length of parsed instruction
> +
> +**/
> +UINT64
> +CcInstructionLength (
> +  IN CC_INSTRUCTION_DATA  *InstructionData
> +  )
> +{
> +  return (UINT64)(InstructionData->End - InstructionData->Begin);
> +}
> +
> +/**
> +  Initialize the instruction parsing context.
> +
> +  Initialize the instruction parsing context, which includes decoding the
> +  instruction prefixes.
> +
> +  @param[in, out] InstructionData  Instruction parsing context
> +  @param[in]      Ghcb             Pointer to the Guest-Hypervisor
> Communication
> +                                   Block
> +  @param[in]      Regs             x64 processor context
> +
> +  @retval         EFI_SUCCESS      Successfully initialize InstructionData
> +  @retval         Others           Other error as indicated
> +**/
> +EFI_STATUS
> +CcInitInstructionData (
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
> +  IN     GHCB                    *Ghcb,
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
> +  )
> +{
> +  SetMem (InstructionData, sizeof (*InstructionData), 0);
> +  InstructionData->Ghcb  = Ghcb;
> +  InstructionData->Begin = (UINT8 *)Regs->Rip;
> +  InstructionData->End   = (UINT8 *)Regs->Rip;
> +
> +  return DecodePrefixes (Regs, InstructionData);
> +}
> diff --git a/OvmfPkg/Library/CcExitLib/CcInstruction.h
> b/OvmfPkg/Library/CcExitLib/CcInstruction.h
> new file mode 100644
> index 000000000000..a8223a6a7d6d
> --- /dev/null
> +++ b/OvmfPkg/Library/CcExitLib/CcInstruction.h
> @@ -0,0 +1,197 @@
> +/** @file
> +  Confidential Computing X64 Instruction
> +
> +  Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef CC_INSTRUCTION_H_
> +#define CC_INSTRUCTION_H_
> +
> +#include <Base.h>
> +#include <Uefi.h>
> +#include <Register/Amd/Ghcb.h>
> +#include <IndustryStandard/InstructionParsing.h>
> +#include <Protocol/DebugSupport.h>
> +
> +//
> +// Instruction execution mode definition
> +//
> +typedef enum {
> +  LongMode64Bit = 0,
> +  LongModeCompat32Bit,
> +  LongModeCompat16Bit,
> +} CC_INSTRUCTION_MODE;
> +
> +//
> +// Instruction size definition (for operand and address)
> +//
> +typedef enum {
> +  Size8Bits = 0,
> +  Size16Bits,
> +  Size32Bits,
> +  Size64Bits,
> +} CC_INSTRUCTION_SIZE;
> +
> +//
> +// Intruction segment definition
> +//
> +typedef enum {
> +  SegmentEs = 0,
> +  SegmentCs,
> +  SegmentSs,
> +  SegmentDs,
> +  SegmentFs,
> +  SegmentGs,
> +} CC_INSTRUCTION_SEGMENT;
> +
> +//
> +// Instruction rep function definition
> +//
> +typedef enum {
> +  RepNone = 0,
> +  RepZ,
> +  RepNZ,
> +} CC_INSTRUCTION_REP;
> +
> +typedef struct {
> +  UINT8    Rm;
> +  UINT8    Reg;
> +  UINT8    Mod;
> +} CC_INSTRUCTION_MODRM_EXT;
> +
> +typedef struct {
> +  UINT8    Base;
> +  UINT8    Index;
> +  UINT8    Scale;
> +} CC_INSTRUCTION_SIB_EXT;
> +
> +//
> +// Instruction opcode definition
> +//
> +typedef struct {
> +  CC_INSTRUCTION_MODRM_EXT    ModRm;
> +
> +  CC_INSTRUCTION_SIB_EXT      Sib;
> +
> +  UINTN                       RegData;
> +  UINTN                       RmData;
> +} CC_INSTRUCTION_OPCODE_EXT;
> +
> +//
> +// Instruction parsing context definition
> +//
> +typedef struct {
> +  GHCB                         *Ghcb;
> +
> +  CC_INSTRUCTION_MODE          Mode;
> +  CC_INSTRUCTION_SIZE          DataSize;
> +  CC_INSTRUCTION_SIZE          AddrSize;
> +  BOOLEAN                      SegmentSpecified;
> +  CC_INSTRUCTION_SEGMENT       Segment;
> +  CC_INSTRUCTION_REP           RepMode;
> +
> +  UINT8                        *Begin;
> +  UINT8                        *End;
> +
> +  UINT8                        *Prefixes;
> +  UINT8                        *OpCodes;
> +  UINT8                        *Displacement;
> +  UINT8                        *Immediate;
> +
> +  INSTRUCTION_REX_PREFIX       RexPrefix;
> +
> +  BOOLEAN                      ModRmPresent;
> +  INSTRUCTION_MODRM            ModRm;
> +
> +  BOOLEAN                      SibPresent;
> +  INSTRUCTION_SIB              Sib;
> +
> +  UINTN                        PrefixSize;
> +  UINTN                        OpCodeSize;
> +  UINTN                        DisplacementSize;
> +  UINTN                        ImmediateSize;
> +
> +  CC_INSTRUCTION_OPCODE_EXT    Ext;
> +} CC_INSTRUCTION_DATA;
> +
> +EFI_STATUS
> +CcInitInstructionData (
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
> +  IN     GHCB                    *Ghcb,
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
> +  );
> +
> +/**
> +  Return a pointer to the contents of the specified register.
> +
> +  Based upon the input register, return a pointer to the registers contents
> +  in the x86 processor context.
> +
> +  @param[in] Regs      x64 processor context
> +  @param[in] Register  Register to obtain pointer for
> +
> +  @return              Pointer to the contents of the requested register
> +
> +**/
> +UINT64 *
> +CcGetRegisterPointer (
> +  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN UINT8                   Register
> +  );
> +
> +/**
> +  Decode a ModRM byte.
> +
> +  Examine the instruction parsing context to decode a ModRM byte and the
> SIB
> +  byte, if present.
> +
> +  @param[in]      Regs             x64 processor context
> +  @param[in, out] InstructionData  Instruction parsing context
> +
> +**/
> +VOID
> +CcDecodeModRm (
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs,
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData
> +  );
> +
> +/**
> +  Determine instruction length
> +
> +  Return the total length of the parsed instruction.
> +
> +  @param[in] InstructionData  Instruction parsing context
> +
> +  @return                     Length of parsed instruction
> +
> +**/
> +UINT64
> +CcInstructionLength (
> +  IN CC_INSTRUCTION_DATA  *InstructionData
> +  );
> +
> +/**
> +  Initialize the instruction parsing context.
> +
> +  Initialize the instruction parsing context, which includes decoding the
> +  instruction prefixes.
> +
> +  @param[in, out] InstructionData  Instruction parsing context
> +  @param[in]      Ghcb             Pointer to the Guest-Hypervisor
> Communication
> +                                   Block
> +  @param[in]      Regs             x64 processor context
> +
> +  @retval         EFI_SUCCESS      Successfully initialize InstructionData
> +  @retval         Others           Other error as indicated
> +**/
> +EFI_STATUS
> +CcInitInstructionData (
> +  IN OUT CC_INSTRUCTION_DATA     *InstructionData,
> +  IN     GHCB                    *Ghcb,
> +  IN     EFI_SYSTEM_CONTEXT_X64  *Regs
> +  );
> +
> +#endif
> diff --git a/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
> b/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
> index 1ee22ce0aea1..811269dd2c06 100644
> --- a/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
> +++ b/OvmfPkg/Library/CcExitLib/SecCcExitLib.inf
> @@ -24,6 +24,7 @@
>    CcExitLib.c
>    CcExitVcHandler.c
>    CcExitVcHandler.h
> +  CcInstruction.c
>    SecCcExitVcHandler.c
>    CcExitVeHandler.c
>    X64/TdVmcallCpuid.nasm
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
  2022-12-29  8:55 ` [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
@ 2023-01-06  8:51   ` Yao, Jiewen
  2023-01-09  1:18     ` Min Xu
  0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2023-01-06  8:51 UTC (permalink / raw)
  To: Xu, Min M, devel@edk2.groups.io
  Cc: Aktas, Erdem, James Bottomley, Gerd Hoffmann, Tom Lendacky,
	Ryan Afranji

Thanks for the cleanup.
Comments inline.

In general, we need ensure CpuDeadLoop() for *any* parsing error in VE handler. Just ASSERT or VmCall(HALT) is not enough, because ASSERT == nothing in release, while VmCall(HALT) is not trusted.

ASSERT can only be used to handle internal impossible logic, but not input from VMM.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Thursday, December 29, 2022 4:56 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Tom Lendacky <thomas.lendacky@amd.com>; Ryan Afranji
> <afranji@google.com>
> Subject: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
> 
> From: Min M Xu <min.m.xu@intel.com>
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4169
> 
> The previous TDX MmioExit doesn't handle the Mmio instructions correctly
> in some scenarios. This patch refactors the implementation to fix the
> issues.
> 
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ryan Afranji <afranji@google.com>
> Reported-by: Ryan Afranji <afranji@google.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
>  OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498 ++++++++++++++------
>  1 file changed, 347 insertions(+), 151 deletions(-)
> 
> diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> index 30d547d5fe55..e0998722af39 100644
> --- a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> +++ b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> @@ -13,6 +13,10 @@
>  #include <Library/BaseMemoryLib.h>
>  #include <IndustryStandard/Tdx.h>
>  #include <IndustryStandard/InstructionParsing.h>
> +#include "CcInstruction.h"
> +
> +#define TDX_MMIO_READ   0
> +#define TDX_MMIO_WRITE  1
> 
>  typedef union {
>    struct {
> @@ -216,14 +220,15 @@ STATIC
>  VOID
>  EFIAPI
>  TdxDecodeInstruction (
> -  IN UINT8  *Rip
> +  IN UINT8   *Rip,
> +  IN UINT32  Length
>    )
>  {
>    UINTN  i;
> 
>    DEBUG ((DEBUG_INFO, "TDX: #TD[EPT] instruction (%p):", Rip));
> -  for (i = 0; i < 15; i++) {
> -    DEBUG ((DEBUG_INFO, "%02x:", Rip[i]));
> +  for (i = 0; i < MIN (15, Length); i++) {
> +    DEBUG ((DEBUG_INFO, "%02x ", Rip[i]));
>    }
> 
>    DEBUG ((DEBUG_INFO, "\n"));
> @@ -235,50 +240,324 @@ TdxDecodeInstruction (
>      TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \

[Jiewen] Please put CpuDeadLoop() here.

>    }
> 
> +/**
> + * Tdx MMIO access via TdVmcall.
> + *
> + * @param MmioSize      Size of the MMIO access
> + * @param ReadOrWrite   Read or write operation
> + * @param GuestPA       Guest physical address
> + * @param Val           Pointer to the value which is read or written
> +
> + * @retval EFI_SUCCESS  Successfully access the mmio
> + * @retval Others       Other errors as indicated
> + */
>  STATIC
> -UINT64 *
> -EFIAPI
> -GetRegFromContext (
> -  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> -  IN UINTN                   RegIndex
> +UINT64
[Jiewen] According to the return code in the function, I think we need use EFI_STATUS here.


> +TdxMmioReadWrite (
> +  IN UINT32  MmioSize,
> +  IN UINT32  ReadOrWrite,
> +  IN UINT64  GuestPA,
> +  IN UINT64  *Val
>    )
>  {
> -  switch (RegIndex) {
> -    case 0: return &Regs->Rax;
> -      break;
> -    case 1: return &Regs->Rcx;
> -      break;
> -    case 2: return &Regs->Rdx;
> -      break;
> -    case 3: return &Regs->Rbx;
> -      break;
> -    case 4: return &Regs->Rsp;
> -      break;
> -    case 5: return &Regs->Rbp;
> -      break;
> -    case 6: return &Regs->Rsi;
> -      break;
> -    case 7: return &Regs->Rdi;
> -      break;
> -    case 8: return &Regs->R8;
> -      break;
> -    case 9: return &Regs->R9;
> +  UINT64  Status;
> +
> +  if ((MmioSize != 1) && (MmioSize != 2) && (MmioSize != 4) &&
> (MmioSize != 8)) {
> +    ASSERT (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Val == NULL) {
> +    ASSERT (FALSE);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (ReadOrWrite == TDX_MMIO_READ) {
> +    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_READ,
> GuestPA, 0, Val);
> +  } else if (ReadOrWrite == TDX_MMIO_WRITE) {
> +    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_WRITE,
> GuestPA, *Val, 0);
> +  } else {
> +    ASSERT (FALSE);
> +    Status = EFI_INVALID_PARAMETER;
> +  }
> +
> +  return Status;
> +}

[Jiewen] I checked error state.
I think we need add CpuDeadLoo() after TdVmCall(TDVMCALL_HALT) below:

  if (Status) {
    DEBUG ((
      DEBUG_ERROR,
      "#VE Error (0x%llx) returned from host, ExitReason is %d, ExitQualification = 0x%x.\n",
      Status,
      ReturnData.VeInfo.ExitReason,
      ReturnData.VeInfo.ExitQualification.Val
      ));

    TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
  }

In general, please CpuDeadLoop() for all TDVMCALL_HALT in CcExitHandleVe()


> +
> +typedef struct {
> +  UINT8                   OpCode;
> +  UINT32                  Bytes;
> +  EFI_PHYSICAL_ADDRESS    Address;
> +  UINT64                  Val;
> +  UINT64                  *Register;
> +  UINT32                  ReadOrWrite;
> +} MMIO_EXIT_PARSED_INSTRUCTION;
> +
> +/**
> + * Parse the MMIO instructions.
> + *
> + * @param Regs              Pointer to the EFI_SYSTEM_CONTEXT_X64 which
> includes the instructions
> + * @param InstructionData   Pointer to the CC_INSTRUCTION_DATA
> + * @param ParsedInstruction Pointer to the parsed instruction data
> + *
> + * @retval EFI_SUCCESS      Successfully parsed the instructions
> + * @retval Others           Other error as indicated
> + */
> +STATIC
> +EFI_STATUS
> +ParseMmioExitInstructions (
> +  IN OUT EFI_SYSTEM_CONTEXT_X64     *Regs,
> +  IN OUT CC_INSTRUCTION_DATA        *InstructionData,
> +  OUT MMIO_EXIT_PARSED_INSTRUCTION  *ParsedInstruction
> +  )
> +{
> +  EFI_STATUS            Status;
> +  UINT8                 OpCode;
> +  UINT8                 SignByte;
> +  UINT32                Bytes;
> +  EFI_PHYSICAL_ADDRESS  Address;
> +  UINT64                Val;
> +  UINT64                *Register;
> +  UINT32                ReadOrWrite;
> +
> +  Address  = 0;
> +  Bytes    = 0;
> +  Register = NULL;
> +  Status   = EFI_SUCCESS;
> +  Val      = 0;
> +
> +  Status = CcInitInstructionData (InstructionData, NULL, Regs);
> +  if (Status != EFI_SUCCESS) {
> +    DEBUG ((DEBUG_ERROR, "%a: Initialize InstructionData failed! (%r)\n",
> __FUNCTION__, Status));
> +    ASSERT (FALSE);
> +    return Status;
> +  }
> +
> +  OpCode = *(InstructionData->OpCodes);
> +  if (OpCode == TWO_BYTE_OPCODE_ESCAPE) {
> +    OpCode = *(InstructionData->OpCodes + 1);
> +  }
> +
> +  switch (OpCode) {
> +    //
> +    // MMIO write (MOV reg/memX, regX)
> +    //
> +    case 0x88:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0x89:
> +      CcDecodeModRm (Regs, InstructionData);
> +      Bytes = ((Bytes != 0) ? Bytes :
> +               (InstructionData->DataSize == Size16Bits) ? 2 :
> +               (InstructionData->DataSize == Size32Bits) ? 4 :
> +               (InstructionData->DataSize == Size64Bits) ? 8 :
> +               0);
> +
> +      if (InstructionData->Ext.ModRm.Mod == 3) {
> +        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode:
> 0x%x)\n", __FUNCTION__, OpCode));
> +        ASSERT (FALSE);
> +        return EFI_UNSUPPORTED;
> +      }
> +
> +      Address     = InstructionData->Ext.RmData;
> +      Val         = InstructionData->Ext.RegData;
> +      ReadOrWrite = TDX_MMIO_WRITE;
> +
>        break;
> -    case 10: return &Regs->R10;
> +
> +    //
> +    // MMIO write (MOV moffsetX, aX)
> +    //
> +    case 0xA2:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0xA3:
> +      Bytes = ((Bytes != 0) ? Bytes :
> +               (InstructionData->DataSize == Size16Bits) ? 2 :
> +               (InstructionData->DataSize == Size32Bits) ? 4 :
> +               (InstructionData->DataSize == Size64Bits) ? 8 :
> +               0);
> +
> +      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData-
> >AddrSize);
> +      InstructionData->End          += InstructionData->ImmediateSize;
> +      CopyMem (&Address, InstructionData->Immediate, InstructionData-
> >ImmediateSize);
> +
> +      Val         = Regs->Rax;
> +      ReadOrWrite = TDX_MMIO_WRITE;
>        break;
> -    case 11: return &Regs->R11;
> +
> +    //
> +    // MMIO write (MOV reg/memX, immX)
> +    //
> +    case 0xC6:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0xC7:
> +      CcDecodeModRm (Regs, InstructionData);
> +      Bytes = ((Bytes != 0) ? Bytes :
> +               (InstructionData->DataSize == Size16Bits) ? 2 :
> +               (InstructionData->DataSize == Size32Bits) ? 4 :
> +               (InstructionData->DataSize == Size64Bits) ? 8 :
> +               0);
> +
> +      InstructionData->ImmediateSize = Bytes;
> +      InstructionData->End          += Bytes;
> +
> +      Val = 0;
> +      CopyMem (&Val, InstructionData->Immediate, InstructionData-
> >ImmediateSize);
> +
> +      Address     = InstructionData->Ext.RmData;
> +      ReadOrWrite = TDX_MMIO_WRITE;
> +
>        break;
> -    case 12: return &Regs->R12;
> +
> +    //
> +    // MMIO read (MOV regX, reg/memX)
> +    //
> +    case 0x8A:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0x8B:
> +      CcDecodeModRm (Regs, InstructionData);
> +      Bytes = ((Bytes != 0) ? Bytes :
> +               (InstructionData->DataSize == Size16Bits) ? 2 :
> +               (InstructionData->DataSize == Size32Bits) ? 4 :
> +               (InstructionData->DataSize == Size64Bits) ? 8 :
> +               0);
> +      if (InstructionData->Ext.ModRm.Mod == 3) {
> +        //
> +        // NPF on two register operands???
> +        //
> +        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode:
> 0x%x)\n", __FUNCTION__, OpCode));
> +        ASSERT (FALSE);

[Jiewen] I am not sure if it is useful to put ASSERT here.
If this is possible case, but we don't want to support, just return UNSUPPORTED without ASSERT.
It will cause CpuDeadLoop() later anyway.


> +        return EFI_UNSUPPORTED;
> +      }
> +
> +      Address     = InstructionData->Ext.RmData;
> +      ReadOrWrite = TDX_MMIO_READ;
> +
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      if (Bytes == 4) {
> +        //
> +        // Zero-extend for 32-bit operation
> +        //
> +        *Register = 0;
> +      }
> +
>        break;
> -    case 13: return &Regs->R13;
> +
> +    //
> +    // MMIO read (MOV aX, moffsetX)
> +    //
> +    case 0xA0:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0xA1:
> +      Bytes = ((Bytes != 0) ? Bytes :
> +               (InstructionData->DataSize == Size16Bits) ? 2 :
> +               (InstructionData->DataSize == Size32Bits) ? 4 :
> +               (InstructionData->DataSize == Size64Bits) ? 8 :
> +               0);
> +
> +      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData-
> >AddrSize);
> +      InstructionData->End          += InstructionData->ImmediateSize;
> +
> +      Address = 0;
> +      CopyMem (
> +        &Address,
> +        InstructionData->Immediate,
> +        InstructionData->ImmediateSize
> +        );
> +
> +      if (Bytes == 4) {
> +        //
> +        // Zero-extend for 32-bit operation
> +        //
> +        Regs->Rax = 0;
> +      }
> +
> +      Register    = &Regs->Rax;
> +      ReadOrWrite = TDX_MMIO_READ;
> +
>        break;
> -    case 14: return &Regs->R14;
> +
> +    //
> +    // MMIO read w/ zero-extension ((MOVZX regX, reg/memX)
> +    //
> +    case 0xB6:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0xB7:
> +      CcDecodeModRm (Regs, InstructionData);
> +      Bytes   = (Bytes != 0) ? Bytes : 2;
> +      Address = InstructionData->Ext.RmData;
> +
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0);
> +
> +      ReadOrWrite = TDX_MMIO_READ;
> +
>        break;
> -    case 15: return &Regs->R15;
> +
> +    //
> +    // MMIO read w/ sign-extension (MOVSX regX, reg/memX)
> +    //
> +    case 0xBE:
> +      Bytes = 1;
> +    //
> +    // fall through
> +    //
> +    case 0xBF:
> +      CcDecodeModRm (Regs, InstructionData);
> +      Bytes = (Bytes != 0) ? Bytes : 2;
> +
> +      Address = InstructionData->Ext.RmData;
> +
> +      if (Bytes == 1) {
> +        UINT8  *Data;
> +        Data     = (UINT8 *)&Val;
> +        SignByte = ((*Data & BIT7) != 0) ? 0xFF : 0x00;
> +      } else {
> +        UINT16  *Data;
> +        Data     = (UINT16 *)&Val;
> +        SignByte = ((*Data & BIT15) != 0) ? 0xFF : 0x00;
> +      }
> +
> +      Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
> +      SetMem (Register, (UINTN)(1 << InstructionData->DataSize), SignByte);
> +
> +      ReadOrWrite = TDX_MMIO_READ;
> +
>        break;
> +
> +    default:
> +      DEBUG ((DEBUG_ERROR, "%a: Invalid MMIO opcode (%x)\n",
> __FUNCTION__, OpCode));
> +      Status = EFI_UNSUPPORTED;
> +      ASSERT (FALSE);
> +  }
> +
> +  if (!EFI_ERROR (Status)) {
> +    ParsedInstruction->OpCode      = OpCode;
> +    ParsedInstruction->Address     = Address;
> +    ParsedInstruction->Bytes       = Bytes;
> +    ParsedInstruction->Register    = Register;
> +    ParsedInstruction->Val         = Val;
> +    ParsedInstruction->ReadOrWrite = ReadOrWrite;
>    }
> 
> -  return NULL;
> +  return Status;
>  }
> 
>  /**
> @@ -300,25 +579,13 @@ MmioExit (
>    IN TDCALL_VEINFO_RETURN_DATA   *Veinfo
>    )
>  {
> -  UINT64          Status;
> -  UINT32          MmioSize;
> -  UINT32          RegSize;
> -  UINT8           OpCode;
> -  BOOLEAN         SeenRex;
> -  UINT64          *Reg;
> -  UINT8           *Rip;
> -  UINT64          Val;
> -  UINT32          OpSize;
> -  MODRM           ModRm;
> -  REX             Rex;
> -  TD_RETURN_DATA  TdReturnData;
> -  UINT8           Gpaw;
> -  UINT64          TdSharedPageMask;
> -
> -  Rip     = (UINT8 *)Regs->Rip;
> -  Val     = 0;
> -  Rex.Val = 0;
> -  SeenRex = FALSE;
> +  UINT64                        Status;
> +  TD_RETURN_DATA                TdReturnData;
> +  UINT8                         Gpaw;
> +  UINT64                        Val;
> +  UINT64                        TdSharedPageMask;
> +  CC_INSTRUCTION_DATA           InstructionData;
> +  MMIO_EXIT_PARSED_INSTRUCTION  ParsedInstruction;
> 
>    Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
>    if (Status == TDX_EXIT_REASON_SUCCESS) {
> @@ -335,112 +602,41 @@ MmioExit (
>      CpuDeadLoop ();
>    }
> 
> -  //
> -  // Default to 32bit transfer
> -  //
> -  OpSize = 4;
> +  Status = ParseMmioExitInstructions (Regs, &InstructionData,
> &ParsedInstruction);
> +  if (Status != EFI_SUCCESS) {
> +    return Status;
> +  }
> 
> -  do {
> -    OpCode = *Rip++;
> -    if (OpCode == 0x66) {
> -      OpSize = 2;
> -    } else if ((OpCode == 0x64) || (OpCode == 0x65) || (OpCode == 0x67)) {
> -      continue;
> -    } else if ((OpCode >= 0x40) && (OpCode <= 0x4f)) {
> -      SeenRex = TRUE;
> -      Rex.Val = OpCode;
> -    } else {
> -      break;
> +  if (Veinfo->GuestPA != (ParsedInstruction.Address | TdSharedPageMask)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "%a: Address is not correct! (%d: 0x%llx != 0x%llx)\n",
> +      __FUNCTION__,
> +      ParsedInstruction.OpCode,
> +      Veinfo->GuestPA,
> +      ParsedInstruction.Address
> +      ));
> +    ASSERT (FALSE);
[Jiewen] Not sure if ASSERT here is useful. We should DeadLoop() later.

> +    return EFI_ABORTED;
> +  }
> +
> +  if (ParsedInstruction.ReadOrWrite == TDX_MMIO_WRITE ) {
> +    Status = TdxMmioReadWrite (ParsedInstruction.Bytes,
> TDX_MMIO_WRITE, Veinfo->GuestPA, &ParsedInstruction.Val);
> +  } else {
> +    Val    = 0;
> +    Status = TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_READ,
> Veinfo->GuestPA, &Val);
> +    if (Status == 0) {
> +      CopyMem (ParsedInstruction.Register, &Val, ParsedInstruction.Bytes);
>      }
> -  } while (TRUE);
> -
> -  //
> -  // We need to have at least 2 more bytes for this instruction
> -  //
> -  TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 13);
[Jiewen] We need make sure it will DeadLoop() if TDX_DECODER_BUG_ON is ON.

> -
> -  OpCode = *Rip++;
> -  //
> -  // Two-byte opecode, get next byte
> -  //
> -  if (OpCode == 0x0F) {
> -    OpCode = *Rip++;
> -  }
> -
> -  switch (OpCode) {
> -    case 0x88:
> -    case 0x8A:
> -    case 0xB6:
> -      MmioSize = 1;
> -      break;
> -    case 0xB7:
> -      MmioSize = 2;
> -      break;
> -    default:
> -      MmioSize = Rex.Bits.W ? 8 : OpSize;
> -      break;
> -  }
> -
> -  /* Punt on AH/BH/CH/DH unless it shows up. */
> -  ModRm.Val = *Rip++;
> -  TDX_DECODER_BUG_ON (MmioSize == 1 && ModRm.Bits.Reg > 4
> && !SeenRex && OpCode != 0xB6);
> -  Reg = GetRegFromContext (Regs, ModRm.Bits.Reg | ((int)Rex.Bits.R << 3));
> -  TDX_DECODER_BUG_ON (!Reg);
> -
> -  if (ModRm.Bits.Rm == 4) {
> -    ++Rip; /* SIB byte */
> -  }
> -
> -  if ((ModRm.Bits.Mod == 2) || ((ModRm.Bits.Mod == 0) &&
> (ModRm.Bits.Rm == 5))) {
> -    Rip += 4; /* DISP32 */
> -  } else if (ModRm.Bits.Mod == 1) {
> -    ++Rip;  /* DISP8 */
> -  }
> -
> -  switch (OpCode) {
> -    case 0x88:
> -    case 0x89:
> -      CopyMem ((void *)&Val, Reg, MmioSize);
> -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA,
> Val, 0);
> -      break;
> -    case 0xC7:
> -      CopyMem ((void *)&Val, Rip, OpSize);
> -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA,
> Val, 0);
> -      Rip   += OpSize;
> -    default:
> -      //
> -      // 32-bit write registers are zero extended to the full register
> -      // Hence 'MOVZX r[32/64], r/m16' is
> -      // hardcoded to reg size 8, and the straight MOV case has a reg
> -      // size of 8 in the 32-bit read case.
> -      //
> -      switch (OpCode) {
> -        case 0xB6:
> -          RegSize = Rex.Bits.W ? 8 : OpSize;
> -          break;
> -        case 0xB7:
> -          RegSize =  8;
> -          break;
> -        default:
> -          RegSize = MmioSize == 4 ? 8 : MmioSize;
> -          break;
> -      }
> -
> -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 0, Veinfo->GuestPA, 0,
> &Val);
> -      if (Status == 0) {
[Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status)


> -        ZeroMem (Reg, RegSize);
> -        CopyMem (Reg, (void *)&Val, MmioSize);
> -      }
>    }
> 
>    if (Status == 0) {
[Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status)


> -    TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 15);
> -
>      //
>      // We change instruction length to reflect true size so handler can
>      // bump rip
>      //
> -    Veinfo->ExitInstructionLength =  (UINT32)((UINT64)Rip - Regs->Rip);
> +    Veinfo->ExitInstructionLength =  (UINT32)(CcInstructionLength
> (&InstructionData));
> +    TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo-
> >ExitInstructionLength);
>    }
> 
>    return Status;
> --
> 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
  2023-01-06  8:51   ` Yao, Jiewen
@ 2023-01-09  1:18     ` Min Xu
  0 siblings, 0 replies; 8+ messages in thread
From: Min Xu @ 2023-01-09  1:18 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: Aktas, Erdem, James Bottomley, Gerd Hoffmann, Tom Lendacky,
	Afranji, Ryan

Thanks for the comments. It will be updated in the next version.

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, January 6, 2023 4:51 PM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io
> Cc: Aktas, Erdem <erdemaktas@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Gerd Hoffmann <kraxel@redhat.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Ryan Afranji <afranji@google.com>
> Subject: RE: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
> 
> Thanks for the cleanup.
> Comments inline.
> 
> In general, we need ensure CpuDeadLoop() for *any* parsing error in VE
> handler. Just ASSERT or VmCall(HALT) is not enough, because ASSERT ==
> nothing in release, while VmCall(HALT) is not trusted.
> 
> ASSERT can only be used to handle internal impossible logic, but not input
> from VMM.
> 
> Thank you
> Yao, Jiewen
> 
> > -----Original Message-----
> > From: Xu, Min M <min.m.xu@intel.com>
> > Sent: Thursday, December 29, 2022 4:56 PM
> > To: devel@edk2.groups.io
> > Cc: Xu, Min M <min.m.xu@intel.com>; Aktas, Erdem
> > <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>;
> Tom
> > Lendacky <thomas.lendacky@amd.com>; Ryan Afranji
> <afranji@google.com>
> > Subject: [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
> >
> > From: Min M Xu <min.m.xu@intel.com>
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4169
> >
> > The previous TDX MmioExit doesn't handle the Mmio instructions
> > correctly in some scenarios. This patch refactors the implementation
> > to fix the issues.
> >
> > Cc: Erdem Aktas <erdemaktas@google.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Ryan Afranji <afranji@google.com>
> > Reported-by: Ryan Afranji <afranji@google.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> > ---
> >  OvmfPkg/Library/CcExitLib/CcExitVeHandler.c | 498
> > ++++++++++++++------
> >  1 file changed, 347 insertions(+), 151 deletions(-)
> >
> > diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> > b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> > index 30d547d5fe55..e0998722af39 100644
> > --- a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> > +++ b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
> > @@ -13,6 +13,10 @@
> >  #include <Library/BaseMemoryLib.h>
> >  #include <IndustryStandard/Tdx.h>
> >  #include <IndustryStandard/InstructionParsing.h>
> > +#include "CcInstruction.h"
> > +
> > +#define TDX_MMIO_READ   0
> > +#define TDX_MMIO_WRITE  1
> >
> >  typedef union {
> >    struct {
> > @@ -216,14 +220,15 @@ STATIC
> >  VOID
> >  EFIAPI
> >  TdxDecodeInstruction (
> > -  IN UINT8  *Rip
> > +  IN UINT8   *Rip,
> > +  IN UINT32  Length
> >    )
> >  {
> >    UINTN  i;
> >
> >    DEBUG ((DEBUG_INFO, "TDX: #TD[EPT] instruction (%p):", Rip));
> > -  for (i = 0; i < 15; i++) {
> > -    DEBUG ((DEBUG_INFO, "%02x:", Rip[i]));
> > +  for (i = 0; i < MIN (15, Length); i++) {
> > +    DEBUG ((DEBUG_INFO, "%02x ", Rip[i]));
> >    }
> >
> >    DEBUG ((DEBUG_INFO, "\n"));
> > @@ -235,50 +240,324 @@ TdxDecodeInstruction (
> >      TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \
> 
> [Jiewen] Please put CpuDeadLoop() here.
> 
> >    }
> >
> > +/**
> > + * Tdx MMIO access via TdVmcall.
> > + *
> > + * @param MmioSize      Size of the MMIO access
> > + * @param ReadOrWrite   Read or write operation
> > + * @param GuestPA       Guest physical address
> > + * @param Val           Pointer to the value which is read or written
> > +
> > + * @retval EFI_SUCCESS  Successfully access the mmio
> > + * @retval Others       Other errors as indicated
> > + */
> >  STATIC
> > -UINT64 *
> > -EFIAPI
> > -GetRegFromContext (
> > -  IN EFI_SYSTEM_CONTEXT_X64  *Regs,
> > -  IN UINTN                   RegIndex
> > +UINT64
> [Jiewen] According to the return code in the function, I think we need use
> EFI_STATUS here.
> 
> 
> > +TdxMmioReadWrite (
> > +  IN UINT32  MmioSize,
> > +  IN UINT32  ReadOrWrite,
> > +  IN UINT64  GuestPA,
> > +  IN UINT64  *Val
> >    )
> >  {
> > -  switch (RegIndex) {
> > -    case 0: return &Regs->Rax;
> > -      break;
> > -    case 1: return &Regs->Rcx;
> > -      break;
> > -    case 2: return &Regs->Rdx;
> > -      break;
> > -    case 3: return &Regs->Rbx;
> > -      break;
> > -    case 4: return &Regs->Rsp;
> > -      break;
> > -    case 5: return &Regs->Rbp;
> > -      break;
> > -    case 6: return &Regs->Rsi;
> > -      break;
> > -    case 7: return &Regs->Rdi;
> > -      break;
> > -    case 8: return &Regs->R8;
> > -      break;
> > -    case 9: return &Regs->R9;
> > +  UINT64  Status;
> > +
> > +  if ((MmioSize != 1) && (MmioSize != 2) && (MmioSize != 4) &&
> > (MmioSize != 8)) {
> > +    ASSERT (FALSE);
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (Val == NULL) {
> > +    ASSERT (FALSE);
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (ReadOrWrite == TDX_MMIO_READ) {
> > +    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_READ,
> > GuestPA, 0, Val);
> > +  } else if (ReadOrWrite == TDX_MMIO_WRITE) {
> > +    Status = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_WRITE,
> > GuestPA, *Val, 0);
> > +  } else {
> > +    ASSERT (FALSE);
> > +    Status = EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  return Status;
> > +}
> 
> [Jiewen] I checked error state.
> I think we need add CpuDeadLoo() after TdVmCall(TDVMCALL_HALT) below:
> 
>   if (Status) {
>     DEBUG ((
>       DEBUG_ERROR,
>       "#VE Error (0x%llx) returned from host, ExitReason is %d, ExitQualification
> = 0x%x.\n",
>       Status,
>       ReturnData.VeInfo.ExitReason,
>       ReturnData.VeInfo.ExitQualification.Val
>       ));
> 
>     TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
>   }
> 
> In general, please CpuDeadLoop() for all TDVMCALL_HALT in CcExitHandleVe()
> 
> 
> > +
> > +typedef struct {
> > +  UINT8                   OpCode;
> > +  UINT32                  Bytes;
> > +  EFI_PHYSICAL_ADDRESS    Address;
> > +  UINT64                  Val;
> > +  UINT64                  *Register;
> > +  UINT32                  ReadOrWrite;
> > +} MMIO_EXIT_PARSED_INSTRUCTION;
> > +
> > +/**
> > + * Parse the MMIO instructions.
> > + *
> > + * @param Regs              Pointer to the EFI_SYSTEM_CONTEXT_X64 which
> > includes the instructions
> > + * @param InstructionData   Pointer to the CC_INSTRUCTION_DATA
> > + * @param ParsedInstruction Pointer to the parsed instruction data
> > + *
> > + * @retval EFI_SUCCESS      Successfully parsed the instructions
> > + * @retval Others           Other error as indicated
> > + */
> > +STATIC
> > +EFI_STATUS
> > +ParseMmioExitInstructions (
> > +  IN OUT EFI_SYSTEM_CONTEXT_X64     *Regs,
> > +  IN OUT CC_INSTRUCTION_DATA        *InstructionData,
> > +  OUT MMIO_EXIT_PARSED_INSTRUCTION  *ParsedInstruction
> > +  )
> > +{
> > +  EFI_STATUS            Status;
> > +  UINT8                 OpCode;
> > +  UINT8                 SignByte;
> > +  UINT32                Bytes;
> > +  EFI_PHYSICAL_ADDRESS  Address;
> > +  UINT64                Val;
> > +  UINT64                *Register;
> > +  UINT32                ReadOrWrite;
> > +
> > +  Address  = 0;
> > +  Bytes    = 0;
> > +  Register = NULL;
> > +  Status   = EFI_SUCCESS;
> > +  Val      = 0;
> > +
> > +  Status = CcInitInstructionData (InstructionData, NULL, Regs);  if
> > + (Status != EFI_SUCCESS) {
> > +    DEBUG ((DEBUG_ERROR, "%a: Initialize InstructionData failed!
> > + (%r)\n",
> > __FUNCTION__, Status));
> > +    ASSERT (FALSE);
> > +    return Status;
> > +  }
> > +
> > +  OpCode = *(InstructionData->OpCodes);  if (OpCode ==
> > + TWO_BYTE_OPCODE_ESCAPE) {
> > +    OpCode = *(InstructionData->OpCodes + 1);  }
> > +
> > +  switch (OpCode) {
> > +    //
> > +    // MMIO write (MOV reg/memX, regX)
> > +    //
> > +    case 0x88:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0x89:
> > +      CcDecodeModRm (Regs, InstructionData);
> > +      Bytes = ((Bytes != 0) ? Bytes :
> > +               (InstructionData->DataSize == Size16Bits) ? 2 :
> > +               (InstructionData->DataSize == Size32Bits) ? 4 :
> > +               (InstructionData->DataSize == Size64Bits) ? 8 :
> > +               0);
> > +
> > +      if (InstructionData->Ext.ModRm.Mod == 3) {
> > +        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode:
> > 0x%x)\n", __FUNCTION__, OpCode));
> > +        ASSERT (FALSE);
> > +        return EFI_UNSUPPORTED;
> > +      }
> > +
> > +      Address     = InstructionData->Ext.RmData;
> > +      Val         = InstructionData->Ext.RegData;
> > +      ReadOrWrite = TDX_MMIO_WRITE;
> > +
> >        break;
> > -    case 10: return &Regs->R10;
> > +
> > +    //
> > +    // MMIO write (MOV moffsetX, aX)
> > +    //
> > +    case 0xA2:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0xA3:
> > +      Bytes = ((Bytes != 0) ? Bytes :
> > +               (InstructionData->DataSize == Size16Bits) ? 2 :
> > +               (InstructionData->DataSize == Size32Bits) ? 4 :
> > +               (InstructionData->DataSize == Size64Bits) ? 8 :
> > +               0);
> > +
> > +      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData-
> > >AddrSize);
> > +      InstructionData->End          += InstructionData->ImmediateSize;
> > +      CopyMem (&Address, InstructionData->Immediate, InstructionData-
> > >ImmediateSize);
> > +
> > +      Val         = Regs->Rax;
> > +      ReadOrWrite = TDX_MMIO_WRITE;
> >        break;
> > -    case 11: return &Regs->R11;
> > +
> > +    //
> > +    // MMIO write (MOV reg/memX, immX)
> > +    //
> > +    case 0xC6:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0xC7:
> > +      CcDecodeModRm (Regs, InstructionData);
> > +      Bytes = ((Bytes != 0) ? Bytes :
> > +               (InstructionData->DataSize == Size16Bits) ? 2 :
> > +               (InstructionData->DataSize == Size32Bits) ? 4 :
> > +               (InstructionData->DataSize == Size64Bits) ? 8 :
> > +               0);
> > +
> > +      InstructionData->ImmediateSize = Bytes;
> > +      InstructionData->End          += Bytes;
> > +
> > +      Val = 0;
> > +      CopyMem (&Val, InstructionData->Immediate, InstructionData-
> > >ImmediateSize);
> > +
> > +      Address     = InstructionData->Ext.RmData;
> > +      ReadOrWrite = TDX_MMIO_WRITE;
> > +
> >        break;
> > -    case 12: return &Regs->R12;
> > +
> > +    //
> > +    // MMIO read (MOV regX, reg/memX)
> > +    //
> > +    case 0x8A:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0x8B:
> > +      CcDecodeModRm (Regs, InstructionData);
> > +      Bytes = ((Bytes != 0) ? Bytes :
> > +               (InstructionData->DataSize == Size16Bits) ? 2 :
> > +               (InstructionData->DataSize == Size32Bits) ? 4 :
> > +               (InstructionData->DataSize == Size64Bits) ? 8 :
> > +               0);
> > +      if (InstructionData->Ext.ModRm.Mod == 3) {
> > +        //
> > +        // NPF on two register operands???
> > +        //
> > +        DEBUG ((DEBUG_ERROR, "%a: Parse Ext.ModRm.Mod error! (OpCode:
> > 0x%x)\n", __FUNCTION__, OpCode));
> > +        ASSERT (FALSE);
> 
> [Jiewen] I am not sure if it is useful to put ASSERT here.
> If this is possible case, but we don't want to support, just return
> UNSUPPORTED without ASSERT.
> It will cause CpuDeadLoop() later anyway.
> 
> 
> > +        return EFI_UNSUPPORTED;
> > +      }
> > +
> > +      Address     = InstructionData->Ext.RmData;
> > +      ReadOrWrite = TDX_MMIO_READ;
> > +
> > +      Register = CcGetRegisterPointer (Regs, InstructionData-
> >Ext.ModRm.Reg);
> > +      if (Bytes == 4) {
> > +        //
> > +        // Zero-extend for 32-bit operation
> > +        //
> > +        *Register = 0;
> > +      }
> > +
> >        break;
> > -    case 13: return &Regs->R13;
> > +
> > +    //
> > +    // MMIO read (MOV aX, moffsetX)
> > +    //
> > +    case 0xA0:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0xA1:
> > +      Bytes = ((Bytes != 0) ? Bytes :
> > +               (InstructionData->DataSize == Size16Bits) ? 2 :
> > +               (InstructionData->DataSize == Size32Bits) ? 4 :
> > +               (InstructionData->DataSize == Size64Bits) ? 8 :
> > +               0);
> > +
> > +      InstructionData->ImmediateSize = (UINTN)(1 << InstructionData-
> > >AddrSize);
> > +      InstructionData->End          += InstructionData->ImmediateSize;
> > +
> > +      Address = 0;
> > +      CopyMem (
> > +        &Address,
> > +        InstructionData->Immediate,
> > +        InstructionData->ImmediateSize
> > +        );
> > +
> > +      if (Bytes == 4) {
> > +        //
> > +        // Zero-extend for 32-bit operation
> > +        //
> > +        Regs->Rax = 0;
> > +      }
> > +
> > +      Register    = &Regs->Rax;
> > +      ReadOrWrite = TDX_MMIO_READ;
> > +
> >        break;
> > -    case 14: return &Regs->R14;
> > +
> > +    //
> > +    // MMIO read w/ zero-extension ((MOVZX regX, reg/memX)
> > +    //
> > +    case 0xB6:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0xB7:
> > +      CcDecodeModRm (Regs, InstructionData);
> > +      Bytes   = (Bytes != 0) ? Bytes : 2;
> > +      Address = InstructionData->Ext.RmData;
> > +
> > +      Register = CcGetRegisterPointer (Regs, InstructionData-
> >Ext.ModRm.Reg);
> > +      SetMem (Register, (UINTN)(1 << InstructionData->DataSize), 0);
> > +
> > +      ReadOrWrite = TDX_MMIO_READ;
> > +
> >        break;
> > -    case 15: return &Regs->R15;
> > +
> > +    //
> > +    // MMIO read w/ sign-extension (MOVSX regX, reg/memX)
> > +    //
> > +    case 0xBE:
> > +      Bytes = 1;
> > +    //
> > +    // fall through
> > +    //
> > +    case 0xBF:
> > +      CcDecodeModRm (Regs, InstructionData);
> > +      Bytes = (Bytes != 0) ? Bytes : 2;
> > +
> > +      Address = InstructionData->Ext.RmData;
> > +
> > +      if (Bytes == 1) {
> > +        UINT8  *Data;
> > +        Data     = (UINT8 *)&Val;
> > +        SignByte = ((*Data & BIT7) != 0) ? 0xFF : 0x00;
> > +      } else {
> > +        UINT16  *Data;
> > +        Data     = (UINT16 *)&Val;
> > +        SignByte = ((*Data & BIT15) != 0) ? 0xFF : 0x00;
> > +      }
> > +
> > +      Register = CcGetRegisterPointer (Regs, InstructionData-
> >Ext.ModRm.Reg);
> > +      SetMem (Register, (UINTN)(1 << InstructionData->DataSize),
> > + SignByte);
> > +
> > +      ReadOrWrite = TDX_MMIO_READ;
> > +
> >        break;
> > +
> > +    default:
> > +      DEBUG ((DEBUG_ERROR, "%a: Invalid MMIO opcode (%x)\n",
> > __FUNCTION__, OpCode));
> > +      Status = EFI_UNSUPPORTED;
> > +      ASSERT (FALSE);
> > +  }
> > +
> > +  if (!EFI_ERROR (Status)) {
> > +    ParsedInstruction->OpCode      = OpCode;
> > +    ParsedInstruction->Address     = Address;
> > +    ParsedInstruction->Bytes       = Bytes;
> > +    ParsedInstruction->Register    = Register;
> > +    ParsedInstruction->Val         = Val;
> > +    ParsedInstruction->ReadOrWrite = ReadOrWrite;
> >    }
> >
> > -  return NULL;
> > +  return Status;
> >  }
> >
> >  /**
> > @@ -300,25 +579,13 @@ MmioExit (
> >    IN TDCALL_VEINFO_RETURN_DATA   *Veinfo
> >    )
> >  {
> > -  UINT64          Status;
> > -  UINT32          MmioSize;
> > -  UINT32          RegSize;
> > -  UINT8           OpCode;
> > -  BOOLEAN         SeenRex;
> > -  UINT64          *Reg;
> > -  UINT8           *Rip;
> > -  UINT64          Val;
> > -  UINT32          OpSize;
> > -  MODRM           ModRm;
> > -  REX             Rex;
> > -  TD_RETURN_DATA  TdReturnData;
> > -  UINT8           Gpaw;
> > -  UINT64          TdSharedPageMask;
> > -
> > -  Rip     = (UINT8 *)Regs->Rip;
> > -  Val     = 0;
> > -  Rex.Val = 0;
> > -  SeenRex = FALSE;
> > +  UINT64                        Status;
> > +  TD_RETURN_DATA                TdReturnData;
> > +  UINT8                         Gpaw;
> > +  UINT64                        Val;
> > +  UINT64                        TdSharedPageMask;
> > +  CC_INSTRUCTION_DATA           InstructionData;
> > +  MMIO_EXIT_PARSED_INSTRUCTION  ParsedInstruction;
> >
> >    Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
> >    if (Status == TDX_EXIT_REASON_SUCCESS) { @@ -335,112 +602,41 @@
> > MmioExit (
> >      CpuDeadLoop ();
> >    }
> >
> > -  //
> > -  // Default to 32bit transfer
> > -  //
> > -  OpSize = 4;
> > +  Status = ParseMmioExitInstructions (Regs, &InstructionData,
> > &ParsedInstruction);
> > +  if (Status != EFI_SUCCESS) {
> > +    return Status;
> > +  }
> >
> > -  do {
> > -    OpCode = *Rip++;
> > -    if (OpCode == 0x66) {
> > -      OpSize = 2;
> > -    } else if ((OpCode == 0x64) || (OpCode == 0x65) || (OpCode == 0x67)) {
> > -      continue;
> > -    } else if ((OpCode >= 0x40) && (OpCode <= 0x4f)) {
> > -      SeenRex = TRUE;
> > -      Rex.Val = OpCode;
> > -    } else {
> > -      break;
> > +  if (Veinfo->GuestPA != (ParsedInstruction.Address | TdSharedPageMask))
> {
> > +    DEBUG ((
> > +      DEBUG_ERROR,
> > +      "%a: Address is not correct! (%d: 0x%llx != 0x%llx)\n",
> > +      __FUNCTION__,
> > +      ParsedInstruction.OpCode,
> > +      Veinfo->GuestPA,
> > +      ParsedInstruction.Address
> > +      ));
> > +    ASSERT (FALSE);
> [Jiewen] Not sure if ASSERT here is useful. We should DeadLoop() later.
> 
> > +    return EFI_ABORTED;
> > +  }
> > +
> > +  if (ParsedInstruction.ReadOrWrite == TDX_MMIO_WRITE ) {
> > +    Status = TdxMmioReadWrite (ParsedInstruction.Bytes,
> > TDX_MMIO_WRITE, Veinfo->GuestPA, &ParsedInstruction.Val);
> > +  } else {
> > +    Val    = 0;
> > +    Status = TdxMmioReadWrite (ParsedInstruction.Bytes,
> > + TDX_MMIO_READ,
> > Veinfo->GuestPA, &Val);
> > +    if (Status == 0) {
> > +      CopyMem (ParsedInstruction.Register, &Val,
> > + ParsedInstruction.Bytes);
> >      }
> > -  } while (TRUE);
> > -
> > -  //
> > -  // We need to have at least 2 more bytes for this instruction
> > -  //
> > -  TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 13);
> [Jiewen] We need make sure it will DeadLoop() if TDX_DECODER_BUG_ON is
> ON.
> 
> > -
> > -  OpCode = *Rip++;
> > -  //
> > -  // Two-byte opecode, get next byte
> > -  //
> > -  if (OpCode == 0x0F) {
> > -    OpCode = *Rip++;
> > -  }
> > -
> > -  switch (OpCode) {
> > -    case 0x88:
> > -    case 0x8A:
> > -    case 0xB6:
> > -      MmioSize = 1;
> > -      break;
> > -    case 0xB7:
> > -      MmioSize = 2;
> > -      break;
> > -    default:
> > -      MmioSize = Rex.Bits.W ? 8 : OpSize;
> > -      break;
> > -  }
> > -
> > -  /* Punt on AH/BH/CH/DH unless it shows up. */
> > -  ModRm.Val = *Rip++;
> > -  TDX_DECODER_BUG_ON (MmioSize == 1 && ModRm.Bits.Reg > 4
> && !SeenRex
> > && OpCode != 0xB6);
> > -  Reg = GetRegFromContext (Regs, ModRm.Bits.Reg | ((int)Rex.Bits.R <<
> > 3));
> > -  TDX_DECODER_BUG_ON (!Reg);
> > -
> > -  if (ModRm.Bits.Rm == 4) {
> > -    ++Rip; /* SIB byte */
> > -  }
> > -
> > -  if ((ModRm.Bits.Mod == 2) || ((ModRm.Bits.Mod == 0) &&
> > (ModRm.Bits.Rm == 5))) {
> > -    Rip += 4; /* DISP32 */
> > -  } else if (ModRm.Bits.Mod == 1) {
> > -    ++Rip;  /* DISP8 */
> > -  }
> > -
> > -  switch (OpCode) {
> > -    case 0x88:
> > -    case 0x89:
> > -      CopyMem ((void *)&Val, Reg, MmioSize);
> > -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA,
> > Val, 0);
> > -      break;
> > -    case 0xC7:
> > -      CopyMem ((void *)&Val, Rip, OpSize);
> > -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 1, Veinfo->GuestPA,
> > Val, 0);
> > -      Rip   += OpSize;
> > -    default:
> > -      //
> > -      // 32-bit write registers are zero extended to the full register
> > -      // Hence 'MOVZX r[32/64], r/m16' is
> > -      // hardcoded to reg size 8, and the straight MOV case has a reg
> > -      // size of 8 in the 32-bit read case.
> > -      //
> > -      switch (OpCode) {
> > -        case 0xB6:
> > -          RegSize = Rex.Bits.W ? 8 : OpSize;
> > -          break;
> > -        case 0xB7:
> > -          RegSize =  8;
> > -          break;
> > -        default:
> > -          RegSize = MmioSize == 4 ? 8 : MmioSize;
> > -          break;
> > -      }
> > -
> > -      Status = TdVmCall (TDVMCALL_MMIO, MmioSize, 0, Veinfo->GuestPA,
> 0,
> > &Val);
> > -      if (Status == 0) {
> [Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status)
> 
> 
> > -        ZeroMem (Reg, RegSize);
> > -        CopyMem (Reg, (void *)&Val, MmioSize);
> > -      }
> >    }
> >
> >    if (Status == 0) {
> [Jiewen] Status is EFI_STATUS. We need use !EFI_ERROR(Status)
> 
> 
> > -    TDX_DECODER_BUG_ON (((UINT64)Rip - Regs->Rip) > 15);
> > -
> >      //
> >      // We change instruction length to reflect true size so handler can
> >      // bump rip
> >      //
> > -    Veinfo->ExitInstructionLength =  (UINT32)((UINT64)Rip - Regs->Rip);
> > +    Veinfo->ExitInstructionLength =  (UINT32)(CcInstructionLength
> > (&InstructionData));
> > +    TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo-
> > >ExitInstructionLength);
> >    }
> >
> >    return Status;
> > --
> > 2.29.2.windows.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-09  1:19 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-29  8:55 [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
2022-12-29  8:55 ` [PATCH V1 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
2023-01-03 20:27   ` Lendacky, Thomas
2023-01-06  8:24   ` Yao, Jiewen
2022-12-29  8:55 ` [PATCH V1 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
2023-01-06  8:51   ` Yao, Jiewen
2023-01-09  1:18     ` Min Xu
2022-12-29  9:00 ` [PATCH V1 0/2] " Min Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox