Re: [edk2-devel] [PATCH v8 13/46] OvmfPkg/VmgExitLib: Add support for IOIO_PROT NAE events

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 5/21/20 12:25 PM, Laszlo Ersek wrote:
> On 05/19/20 23:50, Lendacky, Thomas wrote:
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7Ccba3f15d7c694d95e8e908d7fdabffd6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637256787485321976&sdata=W4gPd4XgieGLxMCe4Ze77i1iCOZWE60UqnZmem8hpXE%3D&reserved=0
>>
>> Under SEV-ES, a IOIO_PROT intercept generates a #VC exception. VMGEXIT
>> must be used to allow the hypervisor to handle this intercept.
>>
>> Add support to construct the required GHCB values to support a IOIO_PROT
>> NAE event.  Parse the instruction that generated the #VC exception,
>> setting the required register values in the GHCB and creating the proper
>> SW_EXITINFO1 value in the GHCB.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>   .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 604 +++++++++++++++++-
>>   1 file changed, 590 insertions(+), 14 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> index 036f030d6b34..b4578ae922c1 100644
>> --- a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> +++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c
>> @@ -12,6 +12,573 @@
>>   #include <Library/VmgExitLib.h>
>>   #include <Register/Amd/Msr.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;
>> +
>> +//
>> +// Instruction REX prefix definition
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT8  BitB:1;
>> +    UINT8  BitX:1;
>> +    UINT8  BitR:1;
>> +    UINT8  BitW:1;
>> +    UINT8  Rex:4;
>> +  } Bits;
>> +
>> +  UINT8  Uint8;
>> +} SEV_ES_INSTRUCTION_REX_PREFIX;
>> +
>> +//
>> +// Instruction ModRM definition
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT8  Rm:3;
>> +    UINT8  Reg:3;
>> +    UINT8  Mod:2;
>> +  } Bits;
>> +
>> +  UINT8  Uint8;
>> +} SEV_ES_INSTRUCTION_MODRM;
>> +
>> +typedef struct {
>> +  UINT8  Rm;
>> +  UINT8  Reg;
>> +  UINT8  Mod;
>> +} SEV_ES_INSTRUCTION_MODRM_EXT;
>> +
>> +//
>> +// Instruction SIB definition
>> +//
>> +typedef union {
>> +  struct {
>> +    UINT8  Base:3;
>> +    UINT8  Index:3;
>> +    UINT8  Scale:2;
>> +  } Bits;
>> +
>> +  UINT8  Uint8;
>> +} SEV_ES_INSTRUCTION_SIB;
>> +
>> +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;
>> +
>> +  SEV_ES_INSTRUCTION_REX_PREFIX  RexPrefix;
>> +
>> +  BOOLEAN                        ModRmPresent;
>> +  SEV_ES_INSTRUCTION_MODRM       ModRm;
>> +
>> +  BOOLEAN                        SibPresent;
>> +  SEV_ES_INSTRUCTION_SIB         Sib;
>> +
>> +  UINTN                          PrefixSize;
>> +  UINTN                          OpCodeSize;
>> +  UINTN                          DisplacementSize;
>> +  UINTN                          ImmediateSize;
>> +
>> +  SEV_ES_INSTRUCTION_OPCODE_EXT  Ext;
>> +} SEV_ES_INSTRUCTION_DATA;
>> +
>> +//
>> +// Non-automatic Exit function prototype
>> +//
>> +typedef
>> +UINT64
>> +(*NAE_EXIT) (
>> +  GHCB                     *Ghcb,
>> +  EFI_SYSTEM_CONTEXT_X64   *Regs,
>> +  SEV_ES_INSTRUCTION_DATA  *InstructionData
>> +  );
>> +
> 
> (1) From the typedefs above, can we move those that are defined in
> industry specs (such as AMD SEV specs) to header file(s)? For example,
> under OvmfPkg/Include/Register or OvmfPkg/Include/IndustryStandard.

Yes, I'll move them to a header file.

> 
>> +
>> +/**
>> +  Checks the GHCB to determine if the specified register has been marked valid.
>> +
>> +  The ValidBitmap area represents the areas of the GHCB that have been marked
>> +  valid. Return an indication of whether the area of the GHCB that holds the
>> +  specified register has been marked valid.
>> +
>> +  @param[in] Ghcb    Pointer to the Guest-Hypervisor Communication Block
>> +  @param[in] Reg     Offset in the GHCB of the register to check
>> +
>> +  @retval TRUE       Register has been marked vald in the GHCB
>> +  @retval FALSE      Register has not been marked valid in the GHCB
>> +
>> +**/
>> +STATIC
>> +BOOLEAN
>> +GhcbIsRegValid (
>> +  IN GHCB                *Ghcb,
>> +  IN GHCB_REGISTER       Reg
>> +  )
>> +{
>> +  UINT32  RegIndex;
>> +  UINT32  RegBit;
>> +
>> +  RegIndex = Reg / 8;
>> +  RegBit   = Reg & 0x07;
>> +
>> +  return (Ghcb->SaveArea.ValidBitmap[RegIndex] & (1 << RegBit));
>> +}
>> +
>> +/**
>> +  Marks a register as valid in the GHCB.
>> +
>> +  The ValidBitmap area represents the areas of the GHCB that have been marked
>> +  valid. Set the area of the GHCB that holds the specified register as valid.
>> +
>> +  @param[in, out] Ghcb    Pointer to the Guest-Hypervisor Communication Block
>> +  @param[in] Reg          Offset in the GHCB of the register to mark valid
>> +
>> +**/
>> +STATIC
>> +VOID
>> +GhcbSetRegValid (
>> +  IN OUT GHCB                *Ghcb,
>> +  IN     GHCB_REGISTER       Reg
>> +  )
>> +{
>> +  UINT32  RegIndex;
>> +  UINT32  RegBit;
>> +
>> +  RegIndex = Reg / 8;
>> +  RegBit   = Reg & 0x07;
>> +
>> +  Ghcb->SaveArea.ValidBitmap[RegIndex] |= (1 << RegBit);
>> +}
>> +
>> +/**
>> +  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;
>> +
>> +  /*TODO: Determine current mode - 64-bit for now */
> 
> (2) I'm OK with a TODO comment, but please use the idiomatic style.

Will do.

> 
>> +  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 >= 0x40) && (*Byte <= 0x4F)) {
>> +      InstructionData->RexPrefix.Uint8 = *Byte;
>> +      if (*Byte & 0x08)
>> +        InstructionData->DataSize = Size64Bits;
>> +      continue;
>> +    }
>> +
>> +    switch (*Byte) {
>> +    case 0x26:
>> +    case 0x2E:
>> +    case 0x36:
>> +    case 0x3E:
>> +      if (Mode != LongMode64Bit) {
>> +        InstructionData->SegmentSpecified = TRUE;
>> +        InstructionData->Segment = (*Byte >> 3) & 3;
>> +      }
>> +      break;
>> +
>> +    case 0x64:
>> +      InstructionData->SegmentSpecified = TRUE;
>> +      InstructionData->Segment = *Byte & 7;
>> +      break;
>> +
>> +    case 0x66:
>> +      if (!InstructionData->RexPrefix.Uint8) {
>> +        InstructionData->DataSize =
>> +          (Mode == LongMode64Bit)       ? Size16Bits :
>> +          (Mode == LongModeCompat32Bit) ? Size16Bits :
>> +          (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
>> +      }
>> +      break;
>> +
>> +    case 0x67:
>> +      InstructionData->AddrSize =
>> +        (Mode == LongMode64Bit)       ? Size32Bits :
>> +        (Mode == LongModeCompat32Bit) ? Size16Bits :
>> +        (Mode == LongModeCompat16Bit) ? Size32Bits : 0;
>> +      break;
>> +
>> +    case 0xF0:
>> +      break;
>> +
>> +    case 0xF2:
>> +      InstructionData->RepMode = RepZ;
>> +      break;
>> +
>> +    case 0xF3:
>> +      InstructionData->RepMode = RepNZ;
>> +      break;
>> +
>> +    default:
>> +      InstructionData->OpCodes = Byte;
>> +      InstructionData->OpCodeSize = (*Byte == 0x0F) ? 2 : 1;
>> +
>> +      InstructionData->End = Byte + InstructionData->OpCodeSize;
>> +      InstructionData->Displacement = InstructionData->End;
>> +      InstructionData->Immediate = InstructionData->End;
>> +      return;
>> +    }
>> +  }
>> +}
> 
> (3) Can we introduce macros or enum constants for the prefixes?
> 
> Although, I seem to remember that QEMU's TCG and even KVM's instruction
> parser uses open-coded magic constants for the prefixes, so the above
> code would not be without precedent.
> 
> Can we please add comments (stating the prefix names) near the case
> labels at least?

Will do.

> 
>> +
>> +/**
>> +  Determine instruction length
>> +
>> +  Return the total length of the parsed instruction.
>> +
>> +  @param[in] InstructionData  Instruction parsing context
>> +
>> +  @retval                     Length of parsed instruction
> 
> (4) @retval is for specific return values (enum constants, macros etc).
> For general descriptions, please use @return, not @retval.
> 
> Please review the rest of the patches from this angle.

I'll review all patches for @return/@retval inconsistencies.

> 
>> +
>> +**/
>> +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
>> +
>> +  Use the VMGEXIT support to report an unsupported event to the hypervisor.
>> +
>> +  @param[in] Ghcb             Pointer to the Guest-Hypervisor Communication
>> +                              Block
>> +  @param[in] Regs             x64 processor context
>> +  @param[in] InstructionData  Instruction parsing context
>> +
>> +  @retval                     New exception value to propagate
>> +
>> +**/
>> +STATIC
>> +UINT64
>> +UnsupportedExit (
>> +  IN GHCB                     *Ghcb,
>> +  IN EFI_SYSTEM_CONTEXT_X64   *Regs,
>> +  IN SEV_ES_INSTRUCTION_DATA  *InstructionData
>> +  )
>> +{
>> +  UINT64  Status;
>> +
>> +  Status = VmgExit (Ghcb, SVM_EXIT_UNSUPPORTED, Regs->ExceptionData, 0);
>> +  if (Status == 0) {
>> +    GHCB_EVENT_INJECTION  Event;
>> +
>> +    Event.Uint64 = 0;
>> +    Event.Elements.Vector = GP_EXCEPTION;
>> +    Event.Elements.Type   = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;
>> +    Event.Elements.Valid  = 1;
>> +
>> +    Status = Event.Uint64;
>> +  }
>> +
>> +  return Status;
>> +}
>> +
>> +#define IOIO_TYPE_STR       (1 << 2)
>> +#define IOIO_TYPE_IN        1
>> +#define IOIO_TYPE_INS       (IOIO_TYPE_IN | IOIO_TYPE_STR)
>> +#define IOIO_TYPE_OUT       0
>> +#define IOIO_TYPE_OUTS      (IOIO_TYPE_OUT | IOIO_TYPE_STR)
>> +
>> +#define IOIO_REP            (1 << 3)
>> +
>> +#define IOIO_ADDR_64        (1 << 9)
>> +#define IOIO_ADDR_32        (1 << 8)
>> +#define IOIO_ADDR_16        (1 << 7)
>> +
>> +#define IOIO_DATA_32        (1 << 6)
>> +#define IOIO_DATA_16        (1 << 5)
>> +#define IOIO_DATA_8         (1 << 4)
>> +#define IOIO_DATA_BYTES(x)  (((x) & 0x70) >> 4)
>> +
>> +#define IOIO_SEG_ES         (0 << 10)
>> +#define IOIO_SEG_DS         (3 << 10)
> 
> (5) I feel like these macros belong in:
> 
>    MdePkg/Include/Register/Amd/Ghcb.h
> 
> Do you agree?

Yes, I can put them in Ghcb.h or some other file.

> 
> If that exact header file is not the best, then I'd request a new header
> file under OvmfPkg/Include/Register.
> 
> (6) I'd also suggest using BIT2, BIT3, BIT9, rather than the open-coded
> shifts. BITx reads more natural in edk2 source.

Will do.

> 
> 
>> +
>> +/**
>> +  Build the IOIO event information.
>> +
>> +  The IOIO event information identifies the type of IO operation to be performed
>> +  by the hypervisor. Build this information based on the instruction data.
>> +
>> +  @param[in]       Regs             x64 processor context
>> +  @param[in, out]  InstructionData  Instruction parsing context
>> +
>> +  @retval Others                    IOIO event information value
> 
> (7) Even though "@retval Others" is a common pattern in edk2, I consider
> it a mis-use of "@retval". Whenever we're tempted to write "@retval
> Others", we should just use "@return". Here, for example:
> 
>    @return  IOIO event information values.
> 
> If you strongly disagree, I won't insist; I'm not trying to create
> busywork for you. Otherwise, please check the rest of the OvmfPkg
> patches for "@retval Others".
> 
>> +
>> +**/
>> +STATIC
>> +UINT64
>> +IoioExitInfo (
>> +  IN     EFI_SYSTEM_CONTEXT_X64   *Regs,
>> +  IN OUT SEV_ES_INSTRUCTION_DATA  *InstructionData
>> +  )
>> +{
>> +  UINT64  ExitInfo;
>> +
>> +  ExitInfo = 0;
>> +
>> +  switch (*(InstructionData->OpCodes)) {
>> +  // IN immediate opcodes
> 
> (8) Right, I love these comments; please do use one of the idiomatic
> comment styles though. Either prepend and append empty "//" lines, or
> move the single-line "//" to the right of the case labels.

Will do.

> 
>> +  case 0xE4:
>> +  case 0xE5:
>> +    InstructionData->ImmediateSize = 1;
>> +    InstructionData->End++;
>> +    ExitInfo |= IOIO_TYPE_IN;
>> +    ExitInfo |= ((*(InstructionData->OpCodes + 1)) << 16);
>> +    break;
>> +
>> +  // OUT immediate opcodes
>> +  case 0xE6:
>> +  case 0xE7:
>> +    InstructionData->ImmediateSize = 1;
>> +    InstructionData->End++;
>> +    ExitInfo |= IOIO_TYPE_OUT;
>> +    ExitInfo |= ((*(InstructionData->OpCodes + 1)) << 16) | IOIO_TYPE_OUT;
>> +    break;
>> +
>> +  // IN register opcodes
>> +  case 0xEC:
>> +  case 0xED:
>> +    ExitInfo |= IOIO_TYPE_IN;
>> +    ExitInfo |= ((Regs->Rdx & 0xffff) << 16);
>> +    break;
>> +
>> +  // OUT register opcodes
>> +  case 0xEE:
>> +  case 0xEF:
>> +    ExitInfo |= IOIO_TYPE_OUT;
>> +    ExitInfo |= ((Regs->Rdx & 0xffff) << 16);
>> +    break;
>> +
>> +  default:
>> +    return 0;
>> +  }
>> +
>> +  switch (*(InstructionData->OpCodes)) {
>> +  case 0xE4:
>> +  case 0xE6:
>> +  case 0xEC:
>> +  case 0xEE:
>> +    // Single-byte opcodes
> 
> (9) Please make both the location and the style of this comment
> consistent with (8).

Yup.

> 
> All my comments are superficial, and that's fine. I totally intend to
> let you do what you need to do in this library (I can tell that writing
> a new instruction decoder must have been hellish work), so I don't want
> to get in your way -- just please make this easier to browse with "edk2
> eyes".
> 
> I'm ready to ACK the patch once all comments have been addressed -- I'm
> not giving an A-b at once because some of my requests / proposals need
> concrete values filled in (such as header file names).
> 

Thanks!
Tom

> Thanks!
> Laszlo
> 
>> +    ExitInfo |= IOIO_DATA_8;
>> +    break;
>> +
>> +  default:
>> +    // Length determined by instruction parsing
>> +    ExitInfo |= (InstructionData->DataSize == Size16Bits) ? IOIO_DATA_16
>> +                                                          : IOIO_DATA_32;
>> +  }
>> +
>> +  switch (InstructionData->AddrSize) {
>> +  case Size16Bits:
>> +    ExitInfo |= IOIO_ADDR_16;
>> +    break;
>> +
>> +  case Size32Bits:
>> +    ExitInfo |= IOIO_ADDR_32;
>> +    break;
>> +
>> +  case Size64Bits:
>> +    ExitInfo |= IOIO_ADDR_64;
>> +    break;
>> +
>> +  default:
>> +    break;
>> +  }
>> +
>> +  if (InstructionData->RepMode) {
>> +    ExitInfo |= IOIO_REP;
>> +  }
>> +
>> +  return ExitInfo;
>> +}
>> +
>> +/**
>> +  Handle an IOIO event.
>> +
>> +  Use the VMGEXIT instruction to handle an IOIO event.
>> +
>> +  @param[in, out] Ghcb             Pointer to the Guest-Hypervisor Communication
>> +                                   Block
>> +  @param[in, out] Regs             x64 processor context
>> +  @param[in]      InstructionData  Instruction parsing context
>> +
>> +  @retval 0                        Event handled successfully
>> +  @retval Others                   New exception value to propagate
>> +
>> +**/
>> +STATIC
>> +UINT64
>> +IoioExit (
>> +  IN OUT GHCB                     *Ghcb,
>> +  IN OUT EFI_SYSTEM_CONTEXT_X64   *Regs,
>> +  IN     SEV_ES_INSTRUCTION_DATA  *InstructionData
>> +  )
>> +{
>> +  UINT64  ExitInfo1, Status;
>> +
>> +  ExitInfo1 = IoioExitInfo (Regs, InstructionData);
>> +  if (!ExitInfo1) {
>> +    return UnsupportedExit (Ghcb, Regs, InstructionData);
>> +  }
>> +
>> +  if (ExitInfo1 & IOIO_TYPE_IN) {
>> +    Ghcb->SaveArea.Rax = 0;
>> +  } else {
>> +    CopyMem (&Ghcb->SaveArea.Rax, &Regs->Rax, IOIO_DATA_BYTES (ExitInfo1));
>> +  }
>> +  GhcbSetRegValid (Ghcb, GhcbRax);
>> +
>> +  Status = VmgExit (Ghcb, SVM_EXIT_IOIO_PROT, ExitInfo1, 0);
>> +  if (Status) {
>> +    return Status;
>> +  }
>> +
>> +  if (ExitInfo1 & IOIO_TYPE_IN) {
>> +    if (!GhcbIsRegValid (Ghcb, GhcbRax)) {
>> +      return UnsupportedExit (Ghcb, Regs, InstructionData);
>> +    }
>> +    CopyMem (&Regs->Rax, &Ghcb->SaveArea.Rax, IOIO_DATA_BYTES (ExitInfo1));
>> +  }
>> +
>> +  return 0;
>> +}
>> +
>>   /**
>>     Handle a #VC exception.
>>   
>> @@ -38,6 +605,8 @@ VmgExitHandleVc (
>>     MSR_SEV_ES_GHCB_REGISTER  Msr;
>>     EFI_SYSTEM_CONTEXT_X64    *Regs;
>>     GHCB                      *Ghcb;
>> +  NAE_EXIT                  NaeExit;
>> +  SEV_ES_INSTRUCTION_DATA   InstructionData;
>>     UINT64                    ExitCode, Status;
>>     EFI_STATUS                VcRet;
>>   
>> @@ -54,24 +623,31 @@ VmgExitHandleVc (
>>   
>>     ExitCode = Regs->ExceptionData;
>>     switch (ExitCode) {
>> +  case SVM_EXIT_IOIO_PROT:
>> +    NaeExit = IoioExit;
>> +    break;
>> +
>>     default:
>> -    Status = VmgExit (Ghcb, SVM_EXIT_UNSUPPORTED, ExitCode, 0);
>> -    if (Status == 0) {
>> -      Regs->ExceptionData = 0;
>> -      *ExceptionType = GP_EXCEPTION;
>> +    NaeExit = UnsupportedExit;
>> +  }
>> +
>> +  InitInstructionData (&InstructionData, Ghcb, Regs);
>> +
>> +  Status = NaeExit (Ghcb, Regs, &InstructionData);
>> +  if (Status == 0) {
>> +    Regs->Rip += InstructionLength (&InstructionData);
>> +  } else {
>> +    GHCB_EVENT_INJECTION  Event;
>> +
>> +    Event.Uint64 = Status;
>> +    if (Event.Elements.ErrorCodeValid) {
>> +      Regs->ExceptionData = Event.Elements.ErrorCode;
>>       } else {
>> -      GHCB_EVENT_INJECTION  Event;
>> -
>> -      Event.Uint64 = Status;
>> -      if (Event.Elements.ErrorCodeValid) {
>> -        Regs->ExceptionData = Event.Elements.ErrorCode;
>> -      } else {
>> -        Regs->ExceptionData = 0;
>> -      }
>> -
>> -      *ExceptionType = Event.Elements.Vector;
>> +      Regs->ExceptionData = 0;
>>       }
>>   
>> +    *ExceptionType = Event.Elements.Vector;
>> +
>>       VcRet = EFI_PROTOCOL_ERROR;
>>     }
>>   
>>
>