* [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
* 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
* [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 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
* 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
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