mirror of https://github.com/acidanthera/audk.git
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: 437eb3f7a8
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>
This commit is contained in:
parent
fdce11226c
commit
1b0db1ec87
|
@ -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 ();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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
|
||||
);
|
||||
|
||||
/**
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
|
|
@ -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);
|
||||
}
|
||||
|
||||
//
|
||||
|
|
|
@ -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
|
||||
)
|
||||
{
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue