OvmfPkg/BaseMemEncryptSevLib: Re-organize page state change support

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4654

In preparation for running under an SVSM at VMPL1 or higher (higher
numerically, lower privilege), re-organize the way a page state change
is performed in order to free up the GHCB for use by the SVSM support.

Currently, the page state change logic directly uses the GHCB shared
buffer to build the page state change structures. However, this will be
in conflict with the use of the GHCB should an SVSM call be required.

Instead, use a separate buffer (an area in the workarea during SEC and
an allocated page during PEI/DXE) to hold the page state change request
and only update the GHCB shared buffer as needed.

Since the information is copied to, and operated on, in the GHCB shared
buffer this has the added benefit of not requiring to save the start and
end entries for use when validating the memory during the page state
change sequence.

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: Min Xu <min.m.xu@intel.com>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>

7 files changed, 146 insertions, 52 deletions

diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index b1c7045ce1..e3b415db2c 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -2,7 +2,7 @@
 

   Work Area structure definition

 

-  Copyright (c) 2021, AMD Inc.

+  Copyright (c) 2021 - 2024, AMD Inc.

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 **/

@@ -54,6 +54,13 @@ typedef struct _SEC_SEV_ES_WORK_AREA {
   // detection in OvmfPkg/ResetVector/Ia32/AmdSev.c

   //

   UINT8     ReceivedVc;

+  UINT8     Reserved[7];

+

+  // Used by SEC to generate Page State Change requests. This should be

+  // sized less than an equal to the GHCB shared buffer area to allow a

+  // single call to the hypervisor.

+  //

+  UINT8     WorkBuffer[1024];

 } SEC_SEV_ES_WORK_AREA;

 

 //

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index cbcdd46f52..2515425e46 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -2,7 +2,7 @@
 

   SEV-SNP Page Validation functions.

 

-  Copyright (c) 2021 AMD Incorporated. All rights reserved.<BR>

+  Copyright (c) 2021 - 2024, AMD Incorporated. All rights reserved.<BR>

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -16,6 +16,8 @@
 #include "SnpPageStateChange.h"

 #include "VirtualMemory.h"

 

+STATIC VOID  *mPscBuffer = NULL;

+

 /**

   Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.

 

@@ -52,5 +54,10 @@ MemEncryptSevSnpPreValidateSystemRam (
     }

   }

 

-  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);

+  if (mPscBuffer == NULL) {

+    mPscBuffer = AllocateReservedPages (1);

+    ASSERT (mPscBuffer != NULL);

+  }

+

+  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE, mPscBuffer, EFI_PAGE_SIZE);

 }

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
index dee3fb8914..337a7d926b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c
@@ -3,7 +3,7 @@
   Virtual Memory Management Services to set or clear the memory encryption bit

 

   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>

-  Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>

+  Copyright (c) 2017 - 2024, AMD Incorporated. All rights reserved.<BR>

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -23,6 +23,8 @@ STATIC BOOLEAN          mAddressEncMaskChecked = FALSE;
 STATIC UINT64           mAddressEncMask;

 STATIC PAGE_TABLE_POOL  *mPageTablePool = NULL;

 

+STATIC VOID  *mPscBuffer = NULL;

+

 typedef enum {

   SetCBit,

   ClearCBit

@@ -786,7 +788,19 @@ SetMemoryEncDec (
   // The InternalSetPageState() is used for setting the page state in the RMP table.

   //

   if (!Mmio && (Mode == ClearCBit) && MemEncryptSevSnpIsEnabled ()) {

-    InternalSetPageState (PhysicalAddress, EFI_SIZE_TO_PAGES (Length), SevSnpPageShared, FALSE);

+    if (mPscBuffer == NULL) {

+      mPscBuffer = AllocateReservedPages (1);

+      ASSERT (mPscBuffer != NULL);

+    }

+

+    InternalSetPageState (

+      PhysicalAddress,

+      EFI_SIZE_TO_PAGES (Length),

+      SevSnpPageShared,

+      FALSE,

+      mPscBuffer,

+      EFI_PAGE_SIZE

+      );

   }

 

   //

@@ -975,11 +989,18 @@ SetMemoryEncDec (
   // The InternalSetPageState() is used for setting the page state in the RMP table.

   //

   if ((Mode == SetCBit) && MemEncryptSevSnpIsEnabled ()) {

+    if (mPscBuffer == NULL) {

+      mPscBuffer = AllocateReservedPages (1);

+      ASSERT (mPscBuffer != NULL);

+    }

+

     InternalSetPageState (

       OrigPhysicalAddress,

       EFI_SIZE_TO_PAGES (OrigLength),

       SevSnpPagePrivate,

-      FALSE

+      FALSE,

+      mPscBuffer,

+      EFI_PAGE_SIZE

       );

   }

 

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
index 4970165444..0040700f03 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -2,7 +2,7 @@
 

   SEV-SNP Page Validation functions.

 

-  Copyright (c) 2021 AMD Incorporated. All rights reserved.<BR>

+  Copyright (c) 2021 - 2024, AMD Incorporated. All rights reserved.<BR>

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -17,6 +17,8 @@
 #include "SnpPageStateChange.h"

 #include "VirtualMemory.h"

 

+STATIC UINT8  mPscBufferPage[EFI_PAGE_SIZE];

+

 typedef struct {

   UINT64    StartAddress;

   UINT64    EndAddress;

@@ -113,7 +115,14 @@ MemEncryptSevSnpPreValidateSystemRam (
       if (BaseAddress < OverlapRange.StartAddress) {

         NumPages = EFI_SIZE_TO_PAGES (OverlapRange.StartAddress - BaseAddress);

 

-        InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);

+        InternalSetPageState (

+          BaseAddress,

+          NumPages,

+          SevSnpPagePrivate,

+          TRUE,

+          mPscBufferPage,

+          sizeof (mPscBufferPage)

+          );

       }

 

       BaseAddress = OverlapRange.EndAddress;

@@ -122,7 +131,14 @@ MemEncryptSevSnpPreValidateSystemRam (
 

     // Validate the remaining pages.

     NumPages = EFI_SIZE_TO_PAGES (EndAddress - BaseAddress);

-    InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);

+    InternalSetPageState (

+      BaseAddress,

+      NumPages,

+      SevSnpPagePrivate,

+      TRUE,

+      mPscBufferPage,

+      sizeof (mPscBufferPage)

+      );

     BaseAddress = EndAddress;

   }

 }

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
index be43a44e4e..ca279d7727 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
@@ -10,6 +10,7 @@
 

 #include <Uefi/UefiBaseType.h>

 #include <Library/BaseLib.h>

+#include <Library/DebugLib.h>

 #include <Library/MemEncryptSevLib.h>

 

 #include "SnpPageStateChange.h"

@@ -65,6 +66,8 @@ MemEncryptSevSnpPreValidateSystemRam (
   IN UINTN             NumPages

   )

 {

+  SEC_SEV_ES_WORK_AREA  *SevEsWorkArea;

+

   if (!MemEncryptSevSnpIsEnabled ()) {

     return;

   }

@@ -78,5 +81,14 @@ MemEncryptSevSnpPreValidateSystemRam (
     SnpPageStateFailureTerminate ();

   }

 

-  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);

+  SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *)FixedPcdGet32 (PcdSevEsWorkAreaBase);

+

+  InternalSetPageState (

+    BaseAddress,

+    NumPages,

+    SevSnpPagePrivate,

+    TRUE,

+    SevEsWorkArea->WorkBuffer,

+    sizeof (SevEsWorkArea->WorkBuffer)

+    );

 }

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
index 43319cc9ed..5d23d1828b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
@@ -2,7 +2,7 @@
 

   SEV-SNP Page Validation functions.

 

-  Copyright (c) 2021 AMD Incorporated. All rights reserved.<BR>

+  Copyright (c) 2021 - 2024, AMD Incorporated. All rights reserved.<BR>

 

   SPDX-License-Identifier: BSD-2-Clause-Patent

 

@@ -24,7 +24,9 @@ InternalSetPageState (
   IN EFI_PHYSICAL_ADDRESS  BaseAddress,

   IN UINTN                 NumPages,

   IN SEV_SNP_PAGE_STATE    State,

-  IN BOOLEAN               UseLargeEntry

+  IN BOOLEAN               UseLargeEntry,

+  IN VOID                  *PscBuffer,

+  IN UINTN                 PscBufferSize

   );

 

 VOID

diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
index 60b176ab14..bcc0798d6b 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -72,16 +72,19 @@ SnpPageStateFailureTerminate (
 STATIC

 VOID

 PvalidateRange (

-  IN  SNP_PAGE_STATE_CHANGE_INFO  *Info,

-  IN  UINTN                       StartIndex,

-  IN  UINTN                       EndIndex,

-  IN  BOOLEAN                     Validate

+  IN  SNP_PAGE_STATE_CHANGE_INFO  *Info

   )

 {

   UINTN                 RmpPageSize;

+  UINTN                 StartIndex;

+  UINTN                 EndIndex;

   UINTN                 Index;

   UINTN                 Ret;

   EFI_PHYSICAL_ADDRESS  Address;

+  BOOLEAN               Validate;

+

+  StartIndex = Info->Header.CurrentEntry;

+  EndIndex   = Info->Header.EndEntry;

 

   for ( ; StartIndex <= EndIndex; StartIndex++) {

     //

@@ -89,6 +92,7 @@ PvalidateRange (
     //

     Address     = ((EFI_PHYSICAL_ADDRESS)Info->Entry[StartIndex].GuestFrameNumber) << EFI_PAGE_SHIFT;

     RmpPageSize = Info->Entry[StartIndex].PageSize;

+    Validate    = Info->Entry[StartIndex].Operation == SNP_PAGE_STATE_PRIVATE;

 

     Ret = AsmPvalidate (RmpPageSize, Validate, Address);

 

@@ -182,11 +186,29 @@ BuildPageStateBuffer (
 STATIC

 VOID

 PageStateChangeVmgExit (

-  IN GHCB                        *Ghcb,

-  IN SNP_PAGE_STATE_CHANGE_INFO  *Info

+  IN GHCB                  *Ghcb,

+  IN SNP_PAGE_STATE_ENTRY  *Start,

+  IN UINT16                Count

   )

 {

-  EFI_STATUS  Status;

+  SNP_PAGE_STATE_CHANGE_INFO  *GhcbInfo;

+  EFI_STATUS                  Status;

+  BOOLEAN                     InterruptState;

+

+  ASSERT (Count <= SNP_PAGE_STATE_MAX_ENTRY);

+  if (Count > SNP_PAGE_STATE_MAX_ENTRY) {

+    SnpPageStateFailureTerminate ();

+  }

+

+  //

+  // Initialize the GHCB

+  //

+  CcExitVmgInit (Ghcb, &InterruptState);

+

+  GhcbInfo                      = (SNP_PAGE_STATE_CHANGE_INFO *)Ghcb->SharedBuffer;

+  GhcbInfo->Header.CurrentEntry = 0;

+  GhcbInfo->Header.EndEntry     = Count - 1;

+  CopyMem (GhcbInfo->Entry, Start, sizeof (*Start) * Count);

 

   //

   // As per the GHCB specification, the hypervisor can resume the guest before

@@ -197,7 +219,7 @@ PageStateChangeVmgExit (
   // page state was not successful, then later memory access will result

   // in the crash.

   //

-  while (Info->Header.CurrentEntry <= Info->Header.EndEntry) {

+  while (GhcbInfo->Header.CurrentEntry <= GhcbInfo->Header.EndEntry) {

     Ghcb->SaveArea.SwScratch = (UINT64)Ghcb->SharedBuffer;

     CcExitVmgSetOffsetValid (Ghcb, GhcbSwScratch);

 

@@ -211,6 +233,34 @@ PageStateChangeVmgExit (
       SnpPageStateFailureTerminate ();

     }

   }

+

+  CcExitVmgDone (Ghcb, InterruptState);

+}

+

+STATIC

+VOID

+PageStateChange (

+  IN SNP_PAGE_STATE_CHANGE_INFO  *Info

+  )

+{

+  GHCB                      *Ghcb;

+  MSR_SEV_ES_GHCB_REGISTER  Msr;

+  SNP_PAGE_STATE_HEADER     *Header;

+  UINT16                    Index;

+  UINT16                    Count;

+

+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);

+  Ghcb                    = Msr.Ghcb;

+

+  Header = &Info->Header;

+

+  for (Index = Header->CurrentEntry; Index <= Header->EndEntry;) {

+    Count = MIN (Header->EndEntry - Index + 1, SNP_PAGE_STATE_MAX_ENTRY);

+

+    PageStateChangeVmgExit (Ghcb, &Info->Entry[Index], Count);

+

+    Index += Count;

+  }

 }

 

 /**

@@ -226,18 +276,14 @@ InternalSetPageState (
   IN EFI_PHYSICAL_ADDRESS  BaseAddress,

   IN UINTN                 NumPages,

   IN SEV_SNP_PAGE_STATE    State,

-  IN BOOLEAN               UseLargeEntry

+  IN BOOLEAN               UseLargeEntry,

+  IN VOID                  *PscBuffer,

+  IN UINTN                 PscBufferSize

   )

 {

-  GHCB                        *Ghcb;

   EFI_PHYSICAL_ADDRESS        NextAddress, EndAddress;

-  MSR_SEV_ES_GHCB_REGISTER    Msr;

-  BOOLEAN                     InterruptState;

   SNP_PAGE_STATE_CHANGE_INFO  *Info;

 

-  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);

-  Ghcb                    = Msr.Ghcb;

-

   EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);

 

   DEBUG ((

@@ -251,57 +297,40 @@ InternalSetPageState (
     UseLargeEntry

     ));

 

-  while (BaseAddress < EndAddress) {

-    UINTN  CurrentEntry, EndEntry;

-

-    //

-    // Initialize the GHCB

-    //

-    CcExitVmgInit (Ghcb, &InterruptState);

+  Info = (SNP_PAGE_STATE_CHANGE_INFO *)PscBuffer;

 

+  for (NextAddress = BaseAddress; NextAddress < EndAddress;) {

     //

     // Build the page state structure

     //

-    Info        = (SNP_PAGE_STATE_CHANGE_INFO *)Ghcb->SharedBuffer;

     NextAddress = BuildPageStateBuffer (

-                    BaseAddress,

+                    NextAddress,

                     EndAddress,

                     State,

                     UseLargeEntry,

-                    Info,

-                    sizeof (Ghcb->SharedBuffer)

+                    PscBuffer,

+                    PscBufferSize

                     );

 

     //

-    // Save the current and end entry from the page state structure. We need

-    // it later.

-    //

-    CurrentEntry = Info->Header.CurrentEntry;

-    EndEntry     = Info->Header.EndEntry;

-

-    //

     // If the caller requested to change the page state to shared then

     // invalidate the pages before making the page shared in the RMP table.

     //

     if (State == SevSnpPageShared) {

-      PvalidateRange (Info, CurrentEntry, EndEntry, FALSE);

+      PvalidateRange (Info);

     }

 

     //

     // Invoke the page state change VMGEXIT.

     //

-    PageStateChangeVmgExit (Ghcb, Info);

+    PageStateChange (Info);

 

     //

     // If the caller requested to change the page state to private then

     // validate the pages after it has been added in the RMP table.

     //

     if (State == SevSnpPagePrivate) {

-      PvalidateRange (Info, CurrentEntry, EndEntry, TRUE);

+      PvalidateRange (Info);

     }

-

-    CcExitVmgDone (Ghcb, InterruptState);

-

-    BaseAddress = NextAddress;

   }

 }