summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTom Lendacky <thomas.lendacky@amd.com>2020-11-06 11:53:12 -0600
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2020-11-10 19:07:55 +0000
commit1b0db1ec8762ace9d31ad73ff20a64e8ba3f2a8d (patch)
tree2cef2b1d2ebc67b5a66b97ba5131db2bb834ff18
parentfdce11226c6dbb6d5144c71ad9373c5ee3b0ac5c (diff)
downloadedk2-1b0db1ec8762ace9d31ad73ff20a64e8ba3f2a8d.tar.gz
edk2-1b0db1ec8762ace9d31ad73ff20a64e8ba3f2a8d.tar.bz2
edk2-1b0db1ec8762ace9d31ad73ff20a64e8ba3f2a8d.zip
UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3008 The QemuFlashPtrWrite() flash services runtime uses the GHCB and VmgExit() directly to perform the flash write when running as an SEV-ES guest. If an interrupt arrives between VmgInit() and VmgExit(), the Dr7 read in the interrupt handler will generate a #VC, which can overwrite information in the GHCB that QemuFlashPtrWrite() has set. This has been seen with the timer interrupt firing and the CpuExceptionHandlerLib library code, UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ Xcode5ExceptionHandlerAsm.nasm and ExceptionHandlerAsm.nasm reading the Dr7 register while QemuFlashPtrWrite() is using the GHCB. In general, it is necessary to protect the GHCB whenever it is used, not just in QemuFlashPtrWrite(). Disable interrupts around the usage of the GHCB by modifying the VmgInit() and VmgDone() interfaces: - VmgInit() will take an extra parameter that is a pointer to a BOOLEAN that will hold the interrupt state at the time of invocation. VmgInit() will get and save this interrupt state before updating the GHCB. - VmgDone() will take an extra parameter that is used to indicate whether interrupts are to be (re)enabled. Before exiting, VmgDone() will enable interrupts if that is requested. Fixes: 437eb3f7a8db7681afe0e6064d3a8edb12abb766 Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Rahul Kumar <rahul1.kumar@intel.com> Cc: Jordan Justen <jordan.l.justen@intel.com> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: Brijesh Singh <brijesh.singh@amd.com> Acked-by: Eric Dong <eric.dong@intel.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com> Message-Id: <c326a4fd78253f784b42eb317589176cf7d8592a.1604685192.git.thomas.lendacky@amd.com>
-rw-r--r--OvmfPkg/Library/VmgExitLib/VmgExitLib.c26
-rw-r--r--OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c5
-rw-r--r--OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c5
-rw-r--r--UefiCpuPkg/Include/Library/VmgExitLib.h14
-rw-r--r--UefiCpuPkg/Library/MpInitLib/DxeMpLib.c5
-rw-r--r--UefiCpuPkg/Library/MpInitLib/MpLib.c7
-rw-r--r--UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c18
7 files changed, 55 insertions, 25 deletions
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
index 0540df8a04..bc5cd61d75 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
@@ -132,15 +132,27 @@ VmgExit (
Performs the necessary steps in preparation for invoking VMGEXIT. Must be
called before setting any fields within the GHCB.
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] InterruptState A pointer to hold the current interrupt
+ state, used for restoring in VmgDone ()
**/
VOID
EFIAPI
VmgInit (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN OUT BOOLEAN *InterruptState
)
{
+ //
+ // Be sure that an interrupt can't cause a #VC while the GHCB is
+ // being used.
+ //
+ *InterruptState = GetInterruptState ();
+ if (*InterruptState) {
+ DisableInterrupts ();
+ }
+
SetMem (&Ghcb->SaveArea, sizeof (Ghcb->SaveArea), 0);
}
@@ -150,15 +162,21 @@ VmgInit (
Performs the necessary steps to cleanup after invoking VMGEXIT. Must be
called after obtaining needed fields within the GHCB.
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in] InterruptState An indicator to conditionally (re)enable
+ interrupts
**/
VOID
EFIAPI
VmgDone (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN BOOLEAN InterruptState
)
{
+ if (InterruptState) {
+ EnableInterrupts ();
+ }
}
/**
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 9bf9d16017..1671db3a01 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -1568,6 +1568,7 @@ VmgExitHandleVc (
SEV_ES_INSTRUCTION_DATA InstructionData;
UINT64 ExitCode, Status;
EFI_STATUS VcRet;
+ BOOLEAN InterruptState;
VcRet = EFI_SUCCESS;
@@ -1578,7 +1579,7 @@ VmgExitHandleVc (
Regs = SystemContext.SystemContextX64;
Ghcb = Msr.Ghcb;
- VmgInit (Ghcb);
+ VmgInit (Ghcb, &InterruptState);
ExitCode = Regs->ExceptionData;
switch (ExitCode) {
@@ -1662,7 +1663,7 @@ VmgExitHandleVc (
VcRet = EFI_PROTOCOL_ERROR;
}
- VmgDone (Ghcb);
+ VmgDone (Ghcb, InterruptState);
return VcRet;
}
diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
index f9b21b5413..1b0742967f 100644
--- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
+++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c
@@ -52,6 +52,7 @@ QemuFlashPtrWrite (
if (MemEncryptSevEsIsEnabled ()) {
MSR_SEV_ES_GHCB_REGISTER Msr;
GHCB *Ghcb;
+ BOOLEAN InterruptState;
Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
Ghcb = Msr.Ghcb;
@@ -63,12 +64,12 @@ QemuFlashPtrWrite (
// #VC exception. Instead, use the the VMGEXIT MMIO write support directly
// to perform the update.
//
- VmgInit (Ghcb);
+ VmgInit (Ghcb, &InterruptState);
Ghcb->SharedBuffer[0] = Value;
Ghcb->SaveArea.SwScratch = (UINT64) (UINTN) Ghcb->SharedBuffer;
VmgSetOffsetValid (Ghcb, GhcbSwScratch);
VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, (UINT64) (UINTN) Ptr, 1);
- VmgDone (Ghcb);
+ VmgDone (Ghcb, InterruptState);
} else {
*Ptr = Value;
}
diff --git a/UefiCpuPkg/Include/Library/VmgExitLib.h b/UefiCpuPkg/Include/Library/VmgExitLib.h
index 07e8af6450..061948cf84 100644
--- a/UefiCpuPkg/Include/Library/VmgExitLib.h
+++ b/UefiCpuPkg/Include/Library/VmgExitLib.h
@@ -50,13 +50,16 @@ VmgExit (
Performs the necessary steps in preparation for invoking VMGEXIT. Must be
called before setting any fields within the GHCB.
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] InterruptState A pointer to hold the current interrupt
+ state, used for restoring in VmgDone ()
**/
VOID
EFIAPI
VmgInit (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN OUT BOOLEAN *InterruptState
);
/**
@@ -65,13 +68,16 @@ VmgInit (
Performs the necessary steps to cleanup after invoking VMGEXIT. Must be
called after obtaining needed fields within the GHCB.
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in] InterruptState An indicator to conditionally (re)enable
+ interrupts
**/
VOID
EFIAPI
VmgDone (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN BOOLEAN InterruptState
);
/**
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72dde..7839c24976 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -171,6 +171,7 @@ GetSevEsAPMemory (
EFI_PHYSICAL_ADDRESS StartAddress;
MSR_SEV_ES_GHCB_REGISTER Msr;
GHCB *Ghcb;
+ BOOLEAN InterruptState;
//
// Allocate 1 page for AP jump table page
@@ -192,9 +193,9 @@ GetSevEsAPMemory (
Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
Ghcb = Msr.Ghcb;
- VmgInit (Ghcb);
+ VmgInit (Ghcb, &InterruptState);
VmgExit (Ghcb, SVM_EXIT_AP_JUMP_TABLE, 0, (UINT64) (UINTN) StartAddress);
- VmgDone (Ghcb);
+ VmgDone (Ghcb, InterruptState);
return (UINTN) StartAddress;
}
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 6d977d45bc..1f47ff3f73 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -884,6 +884,7 @@ ApWakeupFunction (
GHCB *Ghcb;
UINT64 Status;
BOOLEAN DoDecrement;
+ BOOLEAN InterruptState;
DoDecrement = (BOOLEAN) (CpuMpData->InitFlag == ApInitConfig);
@@ -891,7 +892,7 @@ ApWakeupFunction (
Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
Ghcb = Msr.Ghcb;
- VmgInit (Ghcb);
+ VmgInit (Ghcb, &InterruptState);
if (DoDecrement) {
DoDecrement = FALSE;
@@ -905,11 +906,11 @@ ApWakeupFunction (
Status = VmgExit (Ghcb, SVM_EXIT_AP_RESET_HOLD, 0, 0);
if ((Status == 0) && (Ghcb->SaveArea.SwExitInfo2 != 0)) {
- VmgDone (Ghcb);
+ VmgDone (Ghcb, InterruptState);
break;
}
- VmgDone (Ghcb);
+ VmgDone (Ghcb, InterruptState);
}
//
diff --git a/UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c b/UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c
index b47e282aff..89b065cb3f 100644
--- a/UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c
+++ b/UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c
@@ -57,15 +57,16 @@ VmgExit (
Performs the necessary steps in preparation for invoking VMGEXIT. Must be
called before setting any fields within the GHCB.
- The base library function does nothing.
-
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] InterruptState A pointer to hold the current interrupt
+ state, used for restoring in VmgDone ()
**/
VOID
EFIAPI
VmgInit (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN OUT BOOLEAN *InterruptState
)
{
}
@@ -76,15 +77,16 @@ VmgInit (
Performs the necessary steps to cleanup after invoking VMGEXIT. Must be
called after obtaining needed fields within the GHCB.
- The base library function does nothing.
-
- @param[in, out] Ghcb A pointer to the GHCB
+ @param[in, out] Ghcb A pointer to the GHCB
+ @param[in] InterruptState An indicator to conditionally (re)enable
+ interrupts
**/
VOID
EFIAPI
VmgDone (
- IN OUT GHCB *Ghcb
+ IN OUT GHCB *Ghcb,
+ IN BOOLEAN InterruptState
)
{
}