[PATCH v8 11/32] OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values


Brijesh Singh
 

From: Michael Roth <michael.roth@amd.com>

SEV-SNP firmware allows a special guest page to be populated with
guest CPUID values so that they can be validated against supported
host features before being loaded into encrypted guest memory to be
used instead of hypervisor-provided values [1].

Add handling for this in the CPUID #VC handler and use it whenever
SEV-SNP is enabled. To do so, existing CPUID handling via VmgExit is
moved to a helper, GetCpuidHyp(), and a new helper that uses the CPUID
page to do the lookup, GetCpuidFw(), is used instead when SNP is
enabled. For cases where SNP CPUID lookups still rely on fetching
specific CPUID fields from hypervisor, GetCpuidHyp() is used there as
well.

[1]: SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6

Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++--
3 files changed, 419 insertions(+), 30 deletions(-)

diff --git a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
index e6f6ea7972fd..78207fa0f9c9 100644
--- a/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf
@@ -42,4 +42,6 @@ [LibraryClasses]
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize

diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
index c66c68726cdb..7963670e7d30 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
@@ -38,3 +38,6 @@ [LibraryClasses]
LocalApicLib
MemEncryptSevLib

+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidBase
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfCpuidSize
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 41b0c8cc5312..cab4e2b230d0 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -17,6 +17,7 @@
#include <IndustryStandard/InstructionParsing.h>

#include "VmgExitVcHandler.h"
+//#include <Library/MemEncryptSevLib.h>

//
// Instruction execution mode definition
@@ -130,6 +131,32 @@ UINT64
SEV_ES_INSTRUCTION_DATA *InstructionData
);

+//
+// SEV-SNP Cpuid table entry/function
+//
+typedef PACKED struct {
+ UINT32 EaxIn;
+ UINT32 EcxIn;
+ UINT64 Unused;
+ UINT64 Unused2;
+ UINT32 Eax;
+ UINT32 Ebx;
+ UINT32 Ecx;
+ UINT32 Edx;
+ UINT64 Reserved;
+} SEV_SNP_CPUID_FUNCTION;
+
+//
+// SEV-SNP Cpuid page format
+//
+typedef PACKED struct {
+ UINT32 Count;
+ UINT32 Reserved1;
+ UINT64 Reserved2;
+ SEV_SNP_CPUID_FUNCTION function[0];
+} SEV_SNP_CPUID_INFO;
+
+
/**
Return a pointer to the contents of the specified register.

@@ -1496,58 +1523,415 @@ InvdExit (
}

/**
- Handle a CPUID event.
+ Fetch CPUID leaf/function via hypervisor/VMGEXIT.

- Use the VMGEXIT instruction to handle a CPUID event.
+ @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication
+ Block
+ @param[in] EaxIn EAX input for cpuid instruction
+ @param[in] EcxIn ECX input for cpuid instruction
+ @param[in] Xcr0In XCR0 at time of cpuid instruction
+ @param[in, out] Eax Pointer to store leaf's EAX value
+ @param[in, out] Ebx Pointer to store leaf's EBX value
+ @param[in, out] Ecx Pointer to store leaf's ECX value
+ @param[in, out] Edx Pointer to store leaf's EDX value
+ @param[in, out] Status Pointer to store status from VMGEXIT (always 0
+ unless return value indicates failure)
+ @param[in, out] Unsupported Pointer to store indication of unsupported
+ VMGEXIT (always false unless return value
+ indicates failure)

- @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication
- Block
- @param[in, out] Regs x64 processor context
- @param[in] InstructionData Instruction parsing context
-
- @retval 0 Event handled successfully
- @return New exception value to propagate
+ @retval TRUE CPUID leaf fetch successfully.
+ @retval FALSE Error occurred while fetching CPUID leaf. Callers
+ should Status and Unsupported and handle
+ accordingly if they indicate a more precise
+ error condition.

**/
STATIC
-UINT64
-CpuidExit (
+BOOLEAN
+GetCpuidHyp (
IN OUT GHCB *Ghcb,
- IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs,
- IN SEV_ES_INSTRUCTION_DATA *InstructionData
+ IN UINT32 EaxIn,
+ IN UINT32 EcxIn,
+ IN UINT64 XCr0,
+ IN OUT UINT32 *Eax,
+ IN OUT UINT32 *Ebx,
+ IN OUT UINT32 *Ecx,
+ IN OUT UINT32 *Edx,
+ IN OUT UINT64 *Status,
+ IN OUT BOOLEAN *UnsupportedExit
)
{
- UINT64 Status;
-
- Ghcb->SaveArea.Rax = Regs->Rax;
+ *UnsupportedExit = FALSE;
+ Ghcb->SaveArea.Rax = EaxIn;
VmgSetOffsetValid (Ghcb, GhcbRax);
- Ghcb->SaveArea.Rcx = Regs->Rcx;
+ Ghcb->SaveArea.Rcx = EcxIn;
VmgSetOffsetValid (Ghcb, GhcbRcx);
- if (Regs->Rax == CPUID_EXTENDED_STATE) {
- IA32_CR4 Cr4;
-
- Cr4.UintN = AsmReadCr4 ();
- Ghcb->SaveArea.XCr0 = (Cr4.Bits.OSXSAVE == 1) ? AsmXGetBv (0) : 1;
+ if (EaxIn == CPUID_EXTENDED_STATE) {
+ Ghcb->SaveArea.XCr0 = XCr0;
VmgSetOffsetValid (Ghcb, GhcbXCr0);
}

- Status = VmgExit (Ghcb, SVM_EXIT_CPUID, 0, 0);
- if (Status != 0) {
- return Status;
+ *Status = VmgExit (Ghcb, SVM_EXIT_CPUID, 0, 0);
+ if (*Status != 0) {
+ return FALSE;
}

if (!VmgIsOffsetValid (Ghcb, GhcbRax) ||
!VmgIsOffsetValid (Ghcb, GhcbRbx) ||
!VmgIsOffsetValid (Ghcb, GhcbRcx) ||
!VmgIsOffsetValid (Ghcb, GhcbRdx)) {
- return UnsupportedExit (Ghcb, Regs, InstructionData);
+ *UnsupportedExit = TRUE;
+ return FALSE;
}
- Regs->Rax = Ghcb->SaveArea.Rax;
- Regs->Rbx = Ghcb->SaveArea.Rbx;
- Regs->Rcx = Ghcb->SaveArea.Rcx;
- Regs->Rdx = Ghcb->SaveArea.Rdx;
+
+ if (Eax) {
+ *Eax = Ghcb->SaveArea.Rax;
+ }
+ if (Ebx) {
+ *Ebx = Ghcb->SaveArea.Rbx;
+ }
+ if (Ecx) {
+ *Ecx = Ghcb->SaveArea.Rcx;
+ }
+ if (Edx) {
+ *Edx = Ghcb->SaveArea.Rdx;
+ }
+
+ return TRUE;
+}
+
+/**
+ Check if SEV-SNP enabled.
+
+ @retval TRUE SEV-SNP is enabled.
+ @retval FALSE SEV-SNP is disabled.
+
+**/
+STATIC
+BOOLEAN
+SnpEnabled (VOID)
+{
+ MSR_SEV_STATUS_REGISTER Msr;
+
+ Msr.Uint32 = AsmReadMsr32(MSR_SEV_STATUS);
+
+ return !!Msr.Bits.SevSnpBit;
+}
+
+/**
+ Calculate the total XSAVE area size for enabled XSAVE areas
+
+ @param[in] XFeaturesEnabled Bit-mask of enabled XSAVE features/areas as
+ indicated by XCR0/MSR_IA32_XSS bits
+ @param[in] XSaveBaseSize Base/legacy XSAVE area size (e.g. when
+ XCR0 is 1)
+ @param[in, out] XSaveSize Pointer to storage for calculated XSAVE area
+ size
+ @param[in] Compacted Whether or not the calculation is for the
+ normal XSAVE area size (leaf 0xD,0x0,EBX) or
+ compacted XSAVE area size (leaf 0xD,0x1,EBX)
+
+
+ @retval TRUE XSAVE size calculation was successful.
+ @retval FALSE XSAVE size calculation was unsuccessful.
+**/
+STATIC
+BOOLEAN
+GetCpuidXSaveSize (
+ IN UINT64 XFeaturesEnabled,
+ IN UINT32 XSaveBaseSize,
+ IN OUT UINT32 *XSaveSize,
+ IN BOOLEAN Compacted
+ )
+{
+ SEV_SNP_CPUID_INFO *CpuidInfo;
+ UINT64 XFeaturesFound = 0;
+ UINT32 Idx;
+
+ *XSaveSize = XSaveBaseSize;
+ CpuidInfo = (SEV_SNP_CPUID_INFO *)(UINT64)PcdGet32(PcdOvmfCpuidBase);
+
+ for (Idx = 0; Idx < CpuidInfo->Count; Idx++) {
+ SEV_SNP_CPUID_FUNCTION *CpuidFn = &CpuidInfo->function[Idx];
+
+ if (!(CpuidFn->EaxIn == 0xD &&
+ (CpuidFn->EcxIn == 0 || CpuidFn->EcxIn == 1))) {
+ continue;
+ }
+
+ if (XFeaturesFound & (1UL << CpuidFn->EcxIn) ||
+ !(XFeaturesEnabled & (1UL << CpuidFn->EcxIn))) {
+ continue;
+ }
+
+ XFeaturesFound |= (1UL << CpuidFn->EcxIn);
+ if (Compacted) {
+ *XSaveSize += CpuidFn->Eax;
+ } else {
+ *XSaveSize = MAX(*XSaveSize, CpuidFn->Eax + CpuidFn->Ebx);
+ }
+ }
+
+ /*
+ * Either the guest set unsupported XCR0/XSS bits, or the corresponding
+ * entries in the CPUID table were not present. This is an invalid state.
+ */
+ if (XFeaturesFound != (XFeaturesEnabled & ~3UL)) {
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+/**
+ Check if a CPUID leaf/function is indexed via ECX sub-leaf/sub-function
+
+ @param[in] EaxIn EAX input for cpuid instruction
+
+ @retval FALSE cpuid leaf/function is not indexed by ECX input
+ @retval TRUE cpuid leaf/function is indexed by ECX input
+
+**/
+STATIC
+BOOLEAN
+IsFunctionIndexed (
+ IN UINT32 EaxIn
+ )
+{
+ switch (EaxIn) {
+ case CPUID_CACHE_PARAMS:
+ case CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS:
+ case CPUID_EXTENDED_TOPOLOGY:
+ case CPUID_EXTENDED_STATE:
+ case CPUID_INTEL_RDT_MONITORING:
+ case CPUID_INTEL_RDT_ALLOCATION:
+ case CPUID_INTEL_SGX:
+ case CPUID_INTEL_PROCESSOR_TRACE:
+ case CPUID_DETERMINISTIC_ADDRESS_TRANSLATION_PARAMETERS:
+ case CPUID_V2_EXTENDED_TOPOLOGY:
+ case 0x8000001D: /* Cache Topology Information */
+ return TRUE;
+ }
+ return FALSE;
+}
+
+/**
+ Fetch CPUID leaf/function via SEV-SNP CPUID table.
+
+ @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication
+ Block
+ @param[in] EaxIn EAX input for cpuid instruction
+ @param[in] EcxIn ECX input for cpuid instruction
+ @param[in] Xcr0In XCR0 at time of cpuid instruction
+ @param[in, out] Eax Pointer to store leaf's EAX value
+ @param[in, out] Ebx Pointer to store leaf's EBX value
+ @param[in, out] Ecx Pointer to store leaf's ECX value
+ @param[in, out] Edx Pointer to store leaf's EDX value
+ @param[in, out] Status Pointer to store status from VMGEXIT (always 0
+ unless return value indicates failure)
+ @param[in, out] Unsupported Pointer to store indication of unsupported
+ VMGEXIT (always false unless return value
+ indicates failure)
+
+ @retval TRUE CPUID leaf fetch successfully.
+ @retval FALSE Error occurred while fetching CPUID leaf. Callers
+ should Status and Unsupported and handle
+ accordingly if they indicate a more precise
+ error condition.
+
+**/
+STATIC
+BOOLEAN
+GetCpuidFw (
+ IN OUT GHCB *Ghcb,
+ IN UINT32 EaxIn,
+ IN UINT32 EcxIn,
+ IN UINT64 XCr0,
+ IN OUT UINT32 *Eax,
+ IN OUT UINT32 *Ebx,
+ IN OUT UINT32 *Ecx,
+ IN OUT UINT32 *Edx,
+ IN OUT UINT64 *Status,
+ IN OUT BOOLEAN *Unsupported
+ )
+{
+ SEV_SNP_CPUID_INFO *CpuidInfo;
+ BOOLEAN Found;
+ UINT32 Idx;
+
+ CpuidInfo = (SEV_SNP_CPUID_INFO *)(UINT64)PcdGet32(PcdOvmfCpuidBase);
+ Found = FALSE;
+
+ for (Idx = 0; Idx < CpuidInfo->Count; Idx++) {
+ SEV_SNP_CPUID_FUNCTION *CpuidFn = &CpuidInfo->function[Idx];
+
+ if (CpuidFn->EaxIn != EaxIn) {
+ continue;
+ }
+
+ if (IsFunctionIndexed(CpuidFn->EaxIn) && CpuidFn->EcxIn != EcxIn) {
+ continue;
+ }
+
+ *Eax = CpuidFn->Eax;
+ *Ebx = CpuidFn->Ebx;
+ *Ecx = CpuidFn->Ecx;
+ *Edx = CpuidFn->Edx;
+
+ Found = TRUE;
+ break;
+ }
+
+ if (!Found) {
+ *Eax = *Ebx = *Ecx = *Edx = 0;
+ goto Out;
+ }
+
+ if (EaxIn == CPUID_VERSION_INFO) {
+ IA32_CR4 Cr4;
+ UINT32 Ebx2;
+ UINT32 Edx2;
+
+ if (!GetCpuidHyp (Ghcb, EaxIn, EcxIn, XCr0, NULL, &Ebx2, NULL, &Edx2,
+ Status, Unsupported)) {
+ return FALSE;
+ }
+
+ /* initial APIC ID */
+ *Ebx = (*Ebx & 0x00FFFFFF) | (Ebx2 & 0xFF000000);
+ /* APIC enabled bit */
+ *Edx = (*Edx & ~BIT9) | (Edx2 & BIT9);
+ /* OSXSAVE enabled bit */
+ Cr4.UintN = AsmReadCr4 ();
+ *Ecx = (Cr4.Bits.OSXSAVE) ? (*Ecx & ~BIT27) | (*Ecx & BIT27)
+ : (*Ecx & ~BIT27);
+ } else if (EaxIn == CPUID_STRUCTURED_EXTENDED_FEATURE_FLAGS) {
+ IA32_CR4 Cr4;
+
+ Cr4.UintN = AsmReadCr4 ();
+ /* OSPKE enabled bit */
+ *Ecx = (Cr4.Bits.PKE) ? (*Ecx | BIT4) : (*Ecx & ~BIT4);
+ } else if (EaxIn == CPUID_EXTENDED_TOPOLOGY) {
+ if (!GetCpuidHyp (Ghcb, EaxIn, EcxIn, XCr0, NULL, NULL, NULL, Edx,
+ Status, Unsupported)) {
+ return FALSE;
+ }
+ } else if (EaxIn == CPUID_EXTENDED_STATE && (EcxIn == 0 || EcxIn == 1)) {
+ MSR_IA32_XSS_REGISTER XssMsr;
+ BOOLEAN Compacted;
+ UINT32 XSaveSize;
+
+ XssMsr.Uint64 = 0;
+ if (EcxIn == 1) {
+ /*
+ * The PPR and APM aren't clear on what size should be encoded in
+ * 0xD:0x1:EBX when compaction is not enabled by either XSAVEC or
+ * XSAVES, as these are generally fixed to 1 on real CPUs. Report
+ * this undefined case as an error.
+ */
+ if (!(*Eax & (BIT3 | BIT1))) { /* (XSAVES | XSAVEC) */
+ return FALSE;
+ }
+
+ Compacted = TRUE;
+ XssMsr.Uint64 = AsmReadMsr64 (MSR_IA32_XSS);
+ }
+
+ if (!GetCpuidXSaveSize (XCr0 | XssMsr.Uint64, *Ebx, &XSaveSize,
+ Compacted)) {
+ return FALSE;
+ }
+
+ *Ebx = XSaveSize;
+ } else if (EaxIn == 0x8000001E) {
+ UINT32 Ebx2;
+ UINT32 Ecx2;
+
+ /* extended APIC ID */
+ if (!GetCpuidHyp (Ghcb, EaxIn, EcxIn, XCr0, Eax, &Ebx2, &Ecx2, NULL,
+ Status, Unsupported)) {
+ return FALSE;
+ }
+ /* compute ID */
+ *Ebx = (*Ebx & 0xFFFFFF00) | (Ebx2 & 0x000000FF);
+ /* node ID */
+ *Ecx = (*Ecx & 0xFFFFFF00) | (Ecx2 & 0x000000FF);
+ }
+
+Out:
+ *Status = 0;
+ *Unsupported = FALSE;
+ return TRUE;
+}
+
+/**
+ Handle a CPUID event.
+
+ Use VMGEXIT instruction or CPUID table to handle a CPUID event.
+
+ @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication
+ Block
+ @param[in, out] Regs x64 processor context
+ @param[in] InstructionData Instruction parsing context
+
+ @retval 0 Event handled successfully
+ @return New exception value to propagate
+
+**/
+STATIC
+UINT64
+CpuidExit (
+ IN OUT GHCB *Ghcb,
+ IN OUT EFI_SYSTEM_CONTEXT_X64 *Regs,
+ IN SEV_ES_INSTRUCTION_DATA *InstructionData
+ )
+{
+ BOOLEAN Unsupported;
+ UINT64 Status;
+ UINT32 EaxIn;
+ UINT32 EcxIn;
+ UINT64 XCr0;
+ UINT32 Eax;
+ UINT32 Ebx;
+ UINT32 Ecx;
+ UINT32 Edx;
+
+ EaxIn = Regs->Rax;
+ EcxIn = Regs->Rcx;
+
+ if (EaxIn == CPUID_EXTENDED_STATE) {
+ IA32_CR4 Cr4;
+
+ Cr4.UintN = AsmReadCr4 ();
+ XCr0 = (Cr4.Bits.OSXSAVE == 1) ? AsmXGetBv (0) : 1;
+ }
+
+ if (SnpEnabled ()) {
+ if (!GetCpuidFw (Ghcb, EaxIn, EcxIn, XCr0, &Eax, &Ebx, &Ecx, &Edx,
+ &Status, &Unsupported)) {
+ goto CpuidFail;
+ }
+ } else {
+ if (!GetCpuidHyp (Ghcb, EaxIn, EcxIn, XCr0, &Eax, &Ebx, &Ecx, &Edx,
+ &Status, &Unsupported)) {
+ goto CpuidFail;
+ }
+ }
+
+ Regs->Rax = Eax;
+ Regs->Rbx = Ebx;
+ Regs->Rcx = Ecx;
+ Regs->Rdx = Edx;

return 0;
+
+CpuidFail:
+ if (Unsupported) {
+ return UnsupportedExit (Ghcb, Regs, InstructionData);
+ }
+
+ return Status;
}

/**
--
2.25.1