* [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit
@ 2023-01-17 7:43 Min Xu
2023-01-17 7:43 ` [PATCH V3 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Min Xu @ 2023-01-17 7:43 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.
Code: https://github.com/mxu9/edk2/tree/TdxMmioExit.v3
v3 changes:
- Handle the error if an error is returned from TdxMmioReadWrite.
- Add more check in ParseMmioExitInstructions.
v2 changes:
- Add CpuDeadLoop () after each TDVMCALL(HALT) in VE handler. Because
TDVMCALL(HALT) is not trusted.
- Other minor changes such as deleting ASSERT in VE handler. Because
any error in VE handler will trigger CpuDeadLoop (). So ASSERT is not
needed any more.
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 | 546 ++++++++++-----
OvmfPkg/Library/CcExitLib/CcInstruction.c | 454 +++++++++++++
OvmfPkg/Library/CcExitLib/CcInstruction.h | 197 ++++++
OvmfPkg/Library/CcExitLib/SecCcExitLib.inf | 1 +
6 files changed, 1117 insertions(+), 779 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] 5+ messages in thread
* [PATCH V3 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file
2023-01-17 7:43 [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
@ 2023-01-17 7:43 ` Min Xu
2023-01-17 7:43 ` [PATCH V3 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
2023-01-17 11:33 ` [PATCH V3 0/2] [PATCH V1 0/2] " Gerd Hoffmann
2 siblings, 0 replies; 5+ messages in thread
From: Min Xu @ 2023-01-17 7:43 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>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: 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] 5+ messages in thread
* [PATCH V3 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit
2023-01-17 7:43 [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
2023-01-17 7:43 ` [PATCH V3 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
@ 2023-01-17 7:43 ` Min Xu
2023-01-17 11:33 ` [PATCH V3 0/2] [PATCH V1 0/2] " Gerd Hoffmann
2 siblings, 0 replies; 5+ messages in thread
From: Min Xu @ 2023-01-17 7:43 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 | 546 ++++++++++++++------
1 file changed, 382 insertions(+), 164 deletions(-)
diff --git a/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c b/OvmfPkg/Library/CcExitLib/CcExitVeHandler.c
index 30d547d5fe55..b8979ec2c0c0 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"));
@@ -233,52 +238,339 @@ TdxDecodeInstruction (
if ((x)) { \
TdxDecodeInstruction(Rip); \
TdVmCall(TDVMCALL_HALT, 0, 0, 0, 0, 0); \
+ CpuDeadLoop (); \
}
+/**
+ * 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
+EFI_STATUS
+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 TdStatus;
+
+ if ((MmioSize != 1) && (MmioSize != 2) && (MmioSize != 4) && (MmioSize != 8)) {
+ DEBUG ((DEBUG_ERROR, "%a: Invalid MmioSize - %d\n", __FUNCTION__, MmioSize));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Val == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ TdStatus = 0;
+ if (ReadOrWrite == TDX_MMIO_READ) {
+ TdStatus = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_READ, GuestPA, 0, Val);
+ } else if (ReadOrWrite == TDX_MMIO_WRITE) {
+ TdStatus = TdVmCall (TDVMCALL_MMIO, MmioSize, TDX_MMIO_WRITE, GuestPA, *Val, 0);
+ } else {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (TdStatus != 0) {
+ DEBUG ((DEBUG_ERROR, "%a: TdVmcall failed with %llx\n", __FUNCTION__, TdStatus));
+ return EFI_ABORTED;
+ }
+
+ return EFI_SUCCESS;
+}
+
+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 (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Initialize InstructionData failed! (%r)\n", __FUNCTION__, Status));
+ 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));
+ 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));
+ return EFI_UNSUPPORTED;
+ }
+
+ Address = InstructionData->Ext.RmData;
+ ReadOrWrite = TDX_MMIO_READ;
+
+ Register = CcGetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg);
+ if (Register == NULL) {
+ return EFI_ABORTED;
+ }
+
+ 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);
+ if (Register == NULL) {
+ return EFI_ABORTED;
+ }
+
+ 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);
+ if (Register == NULL) {
+ return EFI_ABORTED;
+ }
+
+ 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;
+ }
+
+ 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;
}
/**
@@ -290,160 +582,84 @@ GetRegFromContext (
@param[in] Veinfo VE Info
@retval 0 Event handled successfully
- @return New exception value to propagate
**/
STATIC
-INTN
+UINT64
EFIAPI
MmioExit (
IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs,
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;
+ UINT64 TdStatus;
+ EFI_STATUS Status;
+ TD_RETURN_DATA TdReturnData;
+ UINT8 Gpaw;
+ UINT64 Val;
+ UINT64 TdSharedPageMask;
+ CC_INSTRUCTION_DATA InstructionData;
+ MMIO_EXIT_PARSED_INSTRUCTION ParsedInstruction;
- Rip = (UINT8 *)Regs->Rip;
- Val = 0;
- Rex.Val = 0;
- SeenRex = FALSE;
-
- Status = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
- if (Status == TDX_EXIT_REASON_SUCCESS) {
+ TdStatus = TdCall (TDCALL_TDINFO, 0, 0, 0, &TdReturnData);
+ if (TdStatus == TDX_EXIT_REASON_SUCCESS) {
Gpaw = (UINT8)(TdReturnData.TdInfo.Gpaw & 0x3f);
TdSharedPageMask = 1ULL << (Gpaw - 1);
} else {
- DEBUG ((DEBUG_ERROR, "TDCALL failed with status=%llx\n", Status));
- return Status;
+ DEBUG ((DEBUG_ERROR, "%a: TDCALL failed with status=%llx\n", __FUNCTION__, TdStatus));
+ goto FatalError;
}
if ((Veinfo->GuestPA & TdSharedPageMask) == 0) {
- DEBUG ((DEBUG_ERROR, "EPT-violation #VE on private memory is not allowed!"));
- TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
- CpuDeadLoop ();
+ DEBUG ((DEBUG_ERROR, "%a: EPT-violation #VE on private memory is not allowed!", __FUNCTION__));
+ goto FatalError;
}
- //
- // Default to 32bit transfer
- //
- OpSize = 4;
+ Status = ParseMmioExitInstructions (Regs, &InstructionData, &ParsedInstruction);
+ if (EFI_ERROR (Status)) {
+ goto FatalError;
+ }
+
+ 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
+ ));
+ goto FatalError;
+ }
- 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 (ParsedInstruction.ReadOrWrite == TDX_MMIO_WRITE ) {
+ Status = TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_WRITE, Veinfo->GuestPA, &ParsedInstruction.Val);
+ } else if (ParsedInstruction.ReadOrWrite == TDX_MMIO_READ) {
+ Val = 0;
+ Status = TdxMmioReadWrite (ParsedInstruction.Bytes, TDX_MMIO_READ, Veinfo->GuestPA, &Val);
+ if (!EFI_ERROR (Status)) {
+ 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;
+ } else {
+ goto FatalError;
}
- /* 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 (EFI_ERROR (Status)) {
+ goto FatalError;
}
- 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)(CcInstructionLength (&InstructionData));
+ TdxDecodeInstruction ((UINT8 *)Regs->Rip, Veinfo->ExitInstructionLength);
- //
- // We change instruction length to reflect true size so handler can
- // bump rip
- //
- Veinfo->ExitInstructionLength = (UINT32)((UINT64)Rip - Regs->Rip);
- }
+ return 0;
- return Status;
+FatalError:
+ TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+ CpuDeadLoop ();
+ return 0;
}
/**
@@ -479,6 +695,7 @@ CcExitHandleVe (
if (Status != 0) {
DEBUG ((DEBUG_ERROR, "#VE happened. TDGETVEINFO failed with Status = 0x%llx\n", Status));
TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+ CpuDeadLoop ();
}
switch (ReturnData.VeInfo.ExitReason) {
@@ -571,6 +788,7 @@ CcExitHandleVe (
));
TdVmCall (TDVMCALL_HALT, 0, 0, 0, 0, 0);
+ CpuDeadLoop ();
}
SystemContext.SystemContextX64->Rip += ReturnData.VeInfo.ExitInstructionLength;
--
2.29.2.windows.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit
2023-01-17 7:43 [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
2023-01-17 7:43 ` [PATCH V3 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
2023-01-17 7:43 ` [PATCH V3 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
@ 2023-01-17 11:33 ` Gerd Hoffmann
2023-01-18 3:48 ` [edk2-devel] " Yao, Jiewen
2 siblings, 1 reply; 5+ messages in thread
From: Gerd Hoffmann @ 2023-01-17 11:33 UTC (permalink / raw)
To: Min Xu
Cc: devel, Erdem Aktas, James Bottomley, Jiewen Yao, Tom Lendacky,
Ryan Afranji
On Tue, Jan 17, 2023 at 03:43:28PM +0800, Min Xu wrote:
> 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.
>
> Code: https://github.com/mxu9/edk2/tree/TdxMmioExit.v3
>
> v3 changes:
> - Handle the error if an error is returned from TdxMmioReadWrite.
> - Add more check in ParseMmioExitInstructions.
>
> v2 changes:
> - Add CpuDeadLoop () after each TDVMCALL(HALT) in VE handler. Because
> TDVMCALL(HALT) is not trusted.
> - Other minor changes such as deleting ASSERT in VE handler. Because
> any error in VE handler will trigger CpuDeadLoop (). So ASSERT is not
> needed any more.
Series:
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit
2023-01-17 11:33 ` [PATCH V3 0/2] [PATCH V1 0/2] " Gerd Hoffmann
@ 2023-01-18 3:48 ` Yao, Jiewen
0 siblings, 0 replies; 5+ messages in thread
From: Yao, Jiewen @ 2023-01-18 3:48 UTC (permalink / raw)
To: devel@edk2.groups.io, kraxel@redhat.com, Xu, Min M
Cc: Aktas, Erdem, James Bottomley, Tom Lendacky, Afranji, Ryan
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Merged https://github.com/tianocore/edk2/pull/3918
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, January 17, 2023 7:33 PM
> To: Xu, Min M <min.m.xu@intel.com>
> Cc: devel@edk2.groups.io; Aktas, Erdem <erdemaktas@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Afranji, Ryan <afranji@google.com>
> Subject: Re: [edk2-devel] [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX
> MmioExit
>
> On Tue, Jan 17, 2023 at 03:43:28PM +0800, Min Xu wrote:
> > 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.
> >
> > Code: https://github.com/mxu9/edk2/tree/TdxMmioExit.v3
> >
> > v3 changes:
> > - Handle the error if an error is returned from TdxMmioReadWrite.
> > - Add more check in ParseMmioExitInstructions.
> >
> > v2 changes:
> > - Add CpuDeadLoop () after each TDVMCALL(HALT) in VE handler. Because
> > TDVMCALL(HALT) is not trusted.
> > - Other minor changes such as deleting ASSERT in VE handler. Because
> > any error in VE handler will trigger CpuDeadLoop (). So ASSERT is not
> > needed any more.
>
> Series:
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-01-18 3:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-17 7:43 [PATCH V3 0/2] [PATCH V1 0/2] Refactor TDX MmioExit Min Xu
2023-01-17 7:43 ` [PATCH V3 1/2] OvmfPkg/CcExitLib: Move common X86 instruction code to separate file Min Xu
2023-01-17 7:43 ` [PATCH V3 2/2] OvmfPkg/CcExitLib: Refactor TDX MmioExit Min Xu
2023-01-17 11:33 ` [PATCH V3 0/2] [PATCH V1 0/2] " Gerd Hoffmann
2023-01-18 3:48 ` [edk2-devel] " Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox