[edk2-devel] [PATCH RFC v2 21/28] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM

Brijesh Singh posted 28 patches 1 month, 2 weeks ago

[edk2-devel] [PATCH RFC v2 21/28] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM

Posted by Brijesh Singh 1 month, 2 weeks ago
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275

Many of the integrity guarantees of SEV-SNP are enforced through the
Reverse Map Table (RMP). Each RMP entry contains the GPA at which a
particular page of DRAM should be mapped. The guest can request the
hypervisor to add pages in the RMP table via the Page State Change VMGEXIT
defined in the GHCB specification section 2.5.1 and 4.1.6. Inside each RMP
entry is a Validated flag; this flag is automatically cleared to 0 by the
CPU hardware when a new RMP entry is created for a guest. Each VM page
can be either validated or invalidated, as indicated by the Validated
flag in the RMP entry. Memory access to a private page that is not
validated generates a #VC. A VM can use the PVALIDATE instruction to
validate the private page before using it.

During the guest creation, the boot ROM memory is pre-validated by the
AMD-SEV firmware. The MemEncryptSevSnpValidateSystemRam() can be called
during the SEC and PEI phase to validate the detected system RAM.

One of the fields in the Page State Change NAE is the RMP page size. The
page size input parameter indicates that either a 4KB or 2MB page should
be used while adding the RMP entry. During the validation, when possible,
the MemEncryptSevSnpValidateSystemRam() will use the 2MB entry. A
hypervisor backing the memory may choose to use the different page size
in the RMP entry. In those cases, the PVALIDATE instruction should return
SIZEMISMATCH. If a SIZEMISMATCH is detected, then validate all 512-pages
constituting a 2MB region.

Upon completion, the PVALIDATE instruction sets the rFLAGS.CF to 0 if
instruction changed the RMP entry and to 1 if the instruction did not
change the RMP entry. The rFlags.CF will be 1 only when a memory region
is already validated. We should not double validate a memory
as it could lead to a security compromise. If double validation is
detected, terminate the boot.

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: Laszlo Ersek <lersek@redhat.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 OvmfPkg/Include/Library/MemEncryptSevLib.h                            |  14 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf          |   2 +
 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c          |  17 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf          |   3 +
 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf          |   3 +
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c    |  40 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c    |  36 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c    |  36 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h         |  31 +++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c | 261 ++++++++++++++++++++
 OvmfPkg/OvmfPkgIa32.dsc                                               |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                                            |   1 +
 12 files changed, 445 insertions(+)

diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
index 42caa6497b..c4e89abe0d 100644
--- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
+++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
@@ -237,4 +237,18 @@ MemEncryptSevClearMmioPageEncMask (
   IN UINTN                    NumPages
   );
 
+/**
+  Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
+
+  @param[in]  BaseAddress             Base address
+  @param[in]  NumPages                Number of pages starting from the base address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpPreValidateSystemRam (
+  IN PHYSICAL_ADDRESS           BaseAddress,
+  IN UINTN                      NumPages
+  );
+
 #endif // _MEM_ENCRYPT_SEV_LIB_H_
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
index f2e162d680..2158e1cba3 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeMemEncryptSevLib.inf
@@ -34,8 +34,10 @@
   PeiDxeMemEncryptSevLibInternal.c
 
 [Sources.X64]
+  X64/DxeSnpSystemRamValidate.c
   X64/MemEncryptSevLib.c
   X64/PeiDxeVirtualMemory.c
+  X64/SnpPageStateChangeInternal.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
index 34e7c59e2c..2ca3a400bd 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
@@ -136,3 +136,20 @@ MemEncryptSevClearMmioPageEncMask (
   //
   return RETURN_UNSUPPORTED;
 }
+
+/**
+  Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
+
+  @param[in]  BaseAddress             Base address
+  @param[in]  NumPages                Number of pages starting from the base address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpPreValidateSystemRam (
+  IN PHYSICAL_ADDRESS           BaseAddress,
+  IN UINTN                      NumPages
+  )
+{
+  ASSERT (FALSE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
index 03a78c32df..0402e49a10 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiMemEncryptSevLib.inf
@@ -36,6 +36,8 @@
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/PeiDxeVirtualMemory.c
+  X64/PeiSnpSystemRamValidate.c
+  X64/SnpPageStateChangeInternal.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -49,6 +51,7 @@
   DebugLib
   MemoryAllocationLib
   PcdLib
+  VmgExitLib
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
index 279c38bfbc..939af0a91e 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
@@ -35,6 +35,8 @@
 [Sources.X64]
   X64/MemEncryptSevLib.c
   X64/SecVirtualMemory.c
+  X64/SecSnpSystemRamValidate.c
+  X64/SnpPageStateChangeInternal.c
   X64/VirtualMemory.c
   X64/VirtualMemory.h
 
@@ -46,6 +48,7 @@
   CpuLib
   DebugLib
   PcdLib
+  VmgExitLib
 
 [FixedPcd]
   gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
new file mode 100644
index 0000000000..d46a8408f8
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -0,0 +1,40 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemEncryptSevLib.h>
+
+#include "SnpPageStateChange.h"
+
+/**
+  Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
+
+  @param[in]  BaseAddress             Base address
+  @param[in]  NumPages                Number of pages starting from the base address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpPreValidateSystemRam (
+  IN PHYSICAL_ADDRESS                   BaseAddress,
+  IN UINTN                              NumPages
+  )
+{
+  if (!MemEncryptSevSnpIsEnabled ()) {
+    return;
+  }
+
+  //
+  // All the pre-validation must be completed in the PEI phase.
+  //
+  ASSERT (FALSE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
new file mode 100644
index 0000000000..ba673d193b
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
@@ -0,0 +1,36 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+#include <Library/MemEncryptSevLib.h>
+
+#include "SnpPageStateChange.h"
+
+/**
+  Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
+
+  @param[in]  BaseAddress             Base address
+  @param[in]  NumPages                Number of pages starting from the base address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpPreValidateSystemRam (
+  IN PHYSICAL_ADDRESS                   BaseAddress,
+  IN UINTN                              NumPages
+  )
+{
+  if (!MemEncryptSevSnpIsEnabled ()) {
+    return;
+  }
+
+  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
new file mode 100644
index 0000000000..ba673d193b
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
@@ -0,0 +1,36 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+#include <Library/MemEncryptSevLib.h>
+
+#include "SnpPageStateChange.h"
+
+/**
+  Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
+
+  @param[in]  BaseAddress             Base address
+  @param[in]  NumPages                Number of pages starting from the base address
+
+**/
+VOID
+EFIAPI
+MemEncryptSevSnpPreValidateSystemRam (
+  IN PHYSICAL_ADDRESS                   BaseAddress,
+  IN UINTN                              NumPages
+  )
+{
+  if (!MemEncryptSevSnpIsEnabled ()) {
+    return;
+  }
+
+  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
+}
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
new file mode 100644
index 0000000000..0dd90d4a16
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
@@ -0,0 +1,31 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SNP_PAGE_STATE_INTERNAL_H_
+#define SNP_PAGE_STATE_INTERNAL_H_
+
+//
+// SEV-SNP Page states
+//
+typedef enum {
+  SevSnpPagePrivate,
+  SevSnpPageShared,
+
+} SEV_SNP_PAGE_STATE;
+
+VOID
+InternalSetPageState (
+  IN EFI_PHYSICAL_ADDRESS             BaseAddress,
+  IN UINTN                            NumPages,
+  IN SEV_SNP_PAGE_STATE               State,
+  IN BOOLEAN                          UseLargeEntry
+  );
+
+#endif
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
new file mode 100644
index 0000000000..182f6040ec
--- /dev/null
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
@@ -0,0 +1,261 @@
+/** @file
+
+  SEV-SNP Page Validation functions.
+
+  Copyright (c) 2020 - 2021, AMD Incorporated. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/DebugLib.h>
+#include <Library/VmgExitLib.h>
+
+#include <Register/Amd/Ghcb.h>
+#include <Register/Amd/Msr.h>
+
+#include "SnpPageStateChange.h"
+
+#define IS_ALIGNED(x, y)        ((((x) & (y - 1)) == 0))
+#define PAGES_PER_LARGE_ENTRY   512
+
+STATIC
+UINTN
+MemoryStateToGhcbOp (
+  IN SEV_SNP_PAGE_STATE   State
+  )
+{
+  UINTN Cmd;
+
+  switch (State) {
+    case SevSnpPageShared: Cmd = SNP_PAGE_STATE_SHARED; break;
+    case SevSnpPagePrivate: Cmd = SNP_PAGE_STATE_PRIVATE; break;
+    default: ASSERT(0);
+  }
+
+  return Cmd;
+}
+
+STATIC
+VOID
+SnpPageStateFailureTerminate (
+  VOID
+  )
+{
+  MSR_SEV_ES_GHCB_REGISTER  Msr;
+
+  //
+  // Use the GHCB MSR Protocol to request termination by the hypervisor
+  //
+  Msr.GhcbPhysicalAddress = 0;
+  Msr.GhcbTerminate.Function = GHCB_INFO_TERMINATE_REQUEST;
+  Msr.GhcbTerminate.ReasonCodeSet = GHCB_TERMINATE_GHCB;
+  Msr.GhcbTerminate.ReasonCode = GHCB_TERMINATE_GHCB_GENERAL;
+  AsmWriteMsr64 (MSR_SEV_ES_GHCB, Msr.GhcbPhysicalAddress);
+
+  AsmVmgExit ();
+
+  ASSERT (FALSE);
+  CpuDeadLoop ();
+}
+
+STATIC
+UINTN
+IssuePvalidate (
+  IN    UINTN       Address,
+  IN    UINTN       RmpPageSize,
+  IN    BOOLEAN     Validate
+  )
+{
+  IA32_EFLAGS32         EFlags;
+  UINTN                 Ret;
+
+  Ret = AsmPvalidate (RmpPageSize, Validate, Address, &EFlags);
+
+  //
+  // Check the rFlags.CF to verify that PVALIDATE updated the RMP entry.
+  // If there was a no change in the RMP entry then we are either double
+  // validating or invalidating the memory. This can lead to a security compromise.
+  //
+  if (EFlags.Bits.CF) {
+    DEBUG ((
+      DEBUG_ERROR, "%a:%a: Double %a detected for address 0x%Lx\n",
+      gEfiCallerBaseName,
+      __FUNCTION__,
+      Validate ? "Validate" : "Invalidate",
+      Address
+      ));
+    SnpPageStateFailureTerminate ();
+  }
+
+  return Ret;
+}
+
+/**
+ This function issues the PVALIDATE instruction to validate or invalidate the memory
+ range specified. If PVALIDATE returns size mismatch then it retry validating with
+ smaller page size.
+
+ */
+STATIC
+VOID
+PvalidateRange (
+  IN  SNP_PAGE_STATE_CHANGE_INFO    *Info,
+  IN  UINTN                         StartIndex,
+  IN  UINTN                         EndIndex,
+  IN  BOOLEAN                       Validate
+  )
+{
+  UINTN         Address, RmpPageSize, Ret, i;
+
+  for (; StartIndex < EndIndex; StartIndex++) {
+    //
+    // Get the address and the page size from the Info.
+    //
+    Address = Info->Entry[StartIndex].GuestFrameNumber << EFI_PAGE_SHIFT;
+    RmpPageSize = Info->Entry[StartIndex].PageSize;
+
+    Ret = IssuePvalidate (Address, RmpPageSize, Validate);
+
+    //
+    // If we fail to validate due to size mismatch then try with the
+    // smaller page size. This senario will occur if the backing page in
+    // the RMP entry is 4K and we are validating it as a 2MB.
+    //
+    if ((Ret == PvalidateRetFailSizemismatch) && (RmpPageSize == PvalidatePageSize2MB)) {
+      for (i = 0; i < PAGES_PER_LARGE_ENTRY; i++) {
+        Ret = IssuePvalidate (Address, PvalidatePageSize4K, Validate);
+        if (Ret) {
+          break;
+        }
+
+        Address = Address + EFI_PAGE_SIZE;
+      }
+    }
+
+    //
+    // If validation failed then do not continue.
+    //
+    if (Ret) {
+      DEBUG ((
+        DEBUG_ERROR, "%a:%a: Failed to %a address 0x%Lx Error code %d\n",
+        gEfiCallerBaseName,
+        __FUNCTION__,
+        Validate ? "Validate" : "Invalidate",
+        Address,
+        Ret
+        ));
+      SnpPageStateFailureTerminate ();
+    }
+  }
+}
+
+/**
+ The function is used to set the page state when SEV-SNP is active. The page state
+ transition consist of changing the page ownership in the RMP table, and using the
+ PVALIDATE instruction to update the Validated bit in RMP table.
+
+ When the UseLargeEntry is set to TRUE, then function will try to use the large RMP
+ entry (whevever possible).
+ */
+VOID
+InternalSetPageState (
+  IN EFI_PHYSICAL_ADDRESS             BaseAddress,
+  IN UINTN                            NumPages,
+  IN SEV_SNP_PAGE_STATE               State,
+  IN BOOLEAN                          UseLargeEntry
+  )
+{
+  EFI_STATUS                      Status;
+  GHCB                            *Ghcb;
+  EFI_PHYSICAL_ADDRESS            NextAddress, EndAddress;
+  MSR_SEV_ES_GHCB_REGISTER        Msr;
+  BOOLEAN                         InterruptState;
+  SNP_PAGE_STATE_CHANGE_INFO      *Info;
+  UINTN                           i, RmpPageSize;
+
+  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
+  Ghcb = Msr.Ghcb;
+
+  EndAddress = BaseAddress + EFI_PAGES_TO_SIZE (NumPages);
+
+  DEBUG ((
+    DEBUG_VERBOSE, "%a:%a Address 0x%Lx - 0x%Lx State = %a LargeEntry = %d\n",
+    gEfiCallerBaseName,
+    __FUNCTION__,
+    BaseAddress,
+    EndAddress,
+    State == SevSnpPageShared ? "Shared" : "Private",
+    UseLargeEntry
+    ));
+
+  for (; BaseAddress < EndAddress; BaseAddress = NextAddress) {
+    //
+    // Initialize the GHCB and setup scratch sw to point to shared buffer.
+    //
+    VmgInit (Ghcb, &InterruptState);
+    Info = (SNP_PAGE_STATE_CHANGE_INFO *) Ghcb->SharedBuffer;
+
+    SetMem (Info, sizeof (*Info), 0);
+
+    //
+    // Build page state change buffer
+    //
+    for (i = 0; (EndAddress > BaseAddress) && i < SNP_PAGE_STATE_MAX_ENTRY;
+        BaseAddress = NextAddress, i++) {
+      //
+      // Is this a 2MB aligned page? Check if we can use the Large RMP entry.
+      //
+      if (UseLargeEntry && IS_ALIGNED (BaseAddress, SIZE_2MB) &&
+        ((EndAddress - BaseAddress) >= SIZE_2MB)) {
+        RmpPageSize = PvalidatePageSize2MB;
+        NextAddress = BaseAddress + SIZE_2MB;
+      } else {
+        RmpPageSize = PvalidatePageSize4K;
+        NextAddress = BaseAddress + EFI_PAGE_SIZE;
+      }
+
+      Info->Entry[i].GuestFrameNumber = BaseAddress >> EFI_PAGE_SHIFT;
+      Info->Entry[i].PageSize = RmpPageSize;
+      Info->Entry[i].Op = MemoryStateToGhcbOp (State);
+      Info->Entry[i].CurrentPage = 0;
+    }
+
+    Info->Header.CurrentEntry = 0;
+    Info->Header.EndEntry = i - 1;
+
+    //
+    // If the request page state change is shared then invalidate the pages before
+    // adding the page in the RMP table.
+    //
+    if (State == SevSnpPageShared) {
+      PvalidateRange (Info, 0, i, FALSE);
+    }
+
+    //
+    // Issue the VMGEXIT and retry if hypervisor failed to process all the entries.
+    //
+    Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer;
+    VmgSetOffsetValid (Ghcb, GhcbSwScratch);
+    while (Info->Header.CurrentEntry <= Info->Header.EndEntry) {
+      Status = VmgExit (Ghcb, SVM_EXIT_SNP_PAGE_STATE_CHANGE, 0, 0);
+      if (EFI_ERROR (Status)) {
+        SnpPageStateFailureTerminate ();
+      }
+    }
+
+    //
+    // If the request page state change is shared then invalidate the pages before
+    // adding the page in the RMP table.
+    //
+    if (State == SevSnpPagePrivate) {
+      PvalidateRange (Info, 0, i, TRUE);
+    }
+
+    VmgDone (Ghcb, InterruptState);
+  }
+}
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f9355172d6..1c1e911bd0 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -267,6 +267,7 @@
 !else
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
 !endif
+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
 
 [LibraryClasses.common.PEI_CORE]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 3f27d7b90d..804f5d62be 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -271,6 +271,7 @@
 !else
   CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
 !endif
+  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
 
 [LibraryClasses.common.PEI_CORE]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
-- 
2.17.1



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


Re: [edk2-devel] [PATCH RFC v2 21/28] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM

Posted by Erdem Aktas via groups.io 1 month, 2 weeks ago
> +    //
> +    // If the request page state change is shared then invalidate the pages before
> +    // adding the page in the RMP table.
> +    //
> +    if (State == SevSnpPagePrivate) {
> +      PvalidateRange (Info, 0, i, TRUE);
> +    }
Looks like some copy-paste mistake in the comment.
Also, it checks the  if hypervisor failed to process all the entries
for shared pages, but I do not see that it is checked if for the
private pages. Is there any reason for that?


> +    VmgDone (Ghcb, InterruptState);
> +  }
> +}
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index f9355172d6..1c1e911bd0 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -267,6 +267,7 @@
>  !else
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>  !endif
> +  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
>
>  [LibraryClasses.common.PEI_CORE]
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3f27d7b90d..804f5d62be 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -271,6 +271,7 @@
>  !else
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>  !endif
> +  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
>
>  [LibraryClasses.common.PEI_CORE]
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> --
> 2.17.1
>


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


Re: [edk2-devel] [PATCH RFC v2 21/28] OvmfPkg/MemEncryptSevLib: Add support to validate system RAM

Posted by Brijesh Singh 1 month, 2 weeks ago
On 5/3/21 9:04 AM, Erdem Aktas wrote:
>> +    //
>> +    // If the request page state change is shared then invalidate the pages before
>> +    // adding the page in the RMP table.
>> +    //
>> +    if (State == SevSnpPagePrivate) {
>> +      PvalidateRange (Info, 0, i, TRUE);
>> +    }
> Looks like some copy-paste mistake in the comment.

Ah, thanks for pointing. Yes its copy-paste error.


> Also, it checks the  if hypervisor failed to process all the entries
> for shared pages, but I do not see that it is checked if for the
> private pages. Is there any reason for that?

Sorry, I am not able to follow, can you please expand your review
comment. In the current patch the approach is:

- If page change request is shared then invalidate the range before
requesting the page state change in the RMP.

- Issue the page state change request. Make sure all the entries are
processed by the hypervisor.

- If the page change request is private then validate the range after
its added in the RMP table.


>
>
>> +    VmgDone (Ghcb, InterruptState);
>> +  }
>> +}
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index f9355172d6..1c1e911bd0 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -267,6 +267,7 @@
>>  !else
>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>>  !endif
>> +  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
>>
>>  [LibraryClasses.common.PEI_CORE]
>>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 3f27d7b90d..804f5d62be 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -271,6 +271,7 @@
>>  !else
>>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExceptionHandlerLib.inf
>>  !endif
>> +  MemEncryptSevLib|OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLib.inf
>>
>>  [LibraryClasses.common.PEI_CORE]
>>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>> --
>> 2.17.1
>>


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