[edk2-devel] [PATCH v2 10/11] UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB

Lendacky, Thomas posted 11 patches 5 years, 3 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 10/11] UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
Posted by Lendacky, Thomas 5 years, 3 months ago
From: Tom Lendacky <thomas.lendacky@amd.com>

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>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 UefiCpuPkg/Include/Library/VmgExitLib.h               | 14 ++++++++---
 OvmfPkg/Library/VmgExitLib/VmgExitLib.c               | 26 +++++++++++++++++---
 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         |  5 ++--
 OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |  5 ++--
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |  5 ++--
 UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  7 +++---
 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    | 18 ++++++++------
 7 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/VmgExitLib.h b/UefiCpuPkg/Include/Library/VmgExitLib.h
index ba5ea024839e..617b6cf8d2e7 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/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
index ae86d850ba61..18b102df5f9a 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 9bf9d160179c..1671db3a01b1 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 f9b21b54137d..1b0742967f71 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/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 2c00d72ddefe..7839c249760e 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 07426274f639..4f4b26a7c196 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 b000232c472e..24defd624c63 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
   )
 {
 }
-- 
2.28.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66337): https://edk2.groups.io/g/devel/message/66337
Mute This Topic: https://groups.io/mt/77553982/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v2 10/11] UefiCpuPkg, OvmfPkg: Disable interrupts when using the GHCB
Posted by Laszlo Ersek 5 years, 3 months ago
On 10/16/20 18:09, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> 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>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  UefiCpuPkg/Include/Library/VmgExitLib.h               | 14 ++++++++---
>  OvmfPkg/Library/VmgExitLib/VmgExitLib.c               | 26 +++++++++++++++++---
>  OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c         |  5 ++--
>  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c |  5 ++--
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c               |  5 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                  |  7 +++---
>  UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c    | 18 ++++++++------
>  7 files changed, 55 insertions(+), 25 deletions(-)
> 
> diff --git a/UefiCpuPkg/Include/Library/VmgExitLib.h b/UefiCpuPkg/Include/Library/VmgExitLib.h
> index ba5ea024839e..617b6cf8d2e7 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/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c
> index ae86d850ba61..18b102df5f9a 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 9bf9d160179c..1671db3a01b1 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 f9b21b54137d..1b0742967f71 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/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index 2c00d72ddefe..7839c249760e 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 07426274f639..4f4b26a7c196 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 b000232c472e..24defd624c63 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
>    )
>  {
>  }
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66413): https://edk2.groups.io/g/devel/message/66413
Mute This Topic: https://groups.io/mt/77553982/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-