[PATCH v7 0/7] Add safe unaccepted memory behavior

[PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

These seven patches build on the lazy-accept patch series

"Introduce Lazy-accept for Tdx guest"

by adding SEV-SNP support for the MemoryAccept protocol, and
importantly making eager memory acceptance the default behavior.

We implement a standardized event group from UEFI v2.9,
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly
the right invocation point for eagerly accepting memory if eager
acceptance has not been disabled.

To make use of this event group, we add a new driver that is meant to
carry behavior that is needed for all confidential compute technologies,
not just specific platforms, CocoDxe. In CocoDxe we implement the
default safe behavior to accept all unaccepted memory and invalidate
the MemoryMap on ExitBootServices.

To allow the OS loader to prevent the eager acceptance, we add a new
protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
This protocol has one interface, Disable(). The OS loader can inform the
UEFI that it supports the unaccepted memory type and accepts the
responsibility to accept it.

All images that support unaccepted memory must now locate and call this
new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
function.

Changes since v6:
 - Added implementation of EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
 - Changed callback protocol of v5 to instead use the standardized event
   group for before_exit_boot_services.

Changes since v5:
 - Generic callback protocol moved to MdeModulePkg
 - Removed use of EFI_WARN_STALE_DATA and added comment that the callback
   should only return EFI_SUCCESS or EFI_INVALID_PARAMETER.
 - Removed errant log statement and fixed formatting.

Changes since v4:
 - Commit message wording
 - Replaced direct change to DxeMain with a more generic callback
   protocol.
 - Implemented the direct change as an instance of the callback protocol
   from a new CocoDxe driver.
 - Replaced "enable" protocol with a "disable" protocol, since the name
   was confusing. The AcceptAllUnacceptedMemory protocol directly names
   the behavior that is disabling.

Changes since v3:
 - "DxeMain accepts all memory" patch split into 3 to make each patch
   affect only one package at a time.

Changes since v2:
 - Removed the redundant memory accept interface and added the accept
   behavior to the DXE implementation of
   MemEncryptSevSnpPreValidateSystemRam.
 - Fixed missing #include in >=4GB patch.

Changes since v1:
 - Added a patch to classify SEV-SNP memory above 4GB unaccepted.
 - Fixed style problems in EfiMemoryAcceptProtocol implementation.

Cc: Ard Biescheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Dionna Glaze (7):
  OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
  MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
  MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
  OvmfPkg: Introduce CocoDxe driver
  MdePkg: Introduce the AcceptAllUnacceptedMemory protocol
  OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
  OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

 MdeModulePkg/Core/Dxe/DxeMain.inf                                  |   1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c                            |   6 +
 MdePkg/Include/Guid/EventGroup.h                                   |   5 +
 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h          |  40 +++++
 MdePkg/MdePkg.dec                                                  |   8 +-
 OvmfPkg/AmdSev/AmdSevX64.dsc                                       |   1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf                                       |   1 +
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                                      |  55 ++++++-
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                    |   3 +
 OvmfPkg/CocoDxe/CocoDxe.c                                          | 165 ++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf                                        |  46 ++++++
 OvmfPkg/IntelTdx/IntelTdxX64.dsc                                   |   1 +
 OvmfPkg/IntelTdx/IntelTdxX64.fdf                                   |   1 +
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c |  24 ++-
 OvmfPkg/OvmfPkgIa32X64.dsc                                         |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf                                         |   1 +
 OvmfPkg/OvmfPkgX64.dsc                                             |   1 +
 OvmfPkg/OvmfPkgX64.fdf                                             |   1 +
 OvmfPkg/PlatformPei/AmdSev.c                                       |   5 +
 19 files changed, 357 insertions(+), 9 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf

-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe

[Dionna Glaze]

From: Sophia Wolf <phiawolf@google.com>

When a guest OS does not support unaccepted memory, the unaccepted
memory must be accepted before returning a memory map to the caller.

EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                                      | 55 ++++++++++++++++++--
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                    |  3 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++--
 3 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..5f68a56315 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
 #include <Library/UefiBootServicesTableLib.h>
 #include <Guid/ConfidentialComputingSevSnpBlob.h>
 #include <Library/PcdLib.h>
+#include <Protocol/MemoryAccept.h>
 
 STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION  mSnpBootDxeTable = {
   SIGNATURE_32 ('A',                                    'M', 'D', 'E'),
@@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION  mSnpBootDxeTable = {
   FixedPcdGet32 (PcdOvmfCpuidSize),
 };
 
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+#define IS_ALIGNED(x, y)  ((((x) & ((y) - 1)) == 0))
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+  IN EFI_MEMORY_ACCEPT_PROTOCOL *This,
+  IN EFI_PHYSICAL_ADDRESS StartAddress,
+  IN UINTN Size
+)
+{
+  //
+  // The StartAddress must be page-aligned, and the Size must be a positive
+  // multiple of SIZE_4KB. Use an assert instead of returning an erros since
+  // this is an EDK2-internal protocol.
+  //
+  ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+  ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+  ASSERT (Size != 0);
+
+  MemEncryptSevSnpPreValidateSystemRam (
+    StartAddress,
+    EFI_SIZE_TO_PAGES (Size)
+    );
+
+  return EFI_SUCCESS;
+}
+
+STATIC EFI_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+  AmdSevMemoryAccept
+};
+
 EFI_STATUS
 EFIAPI
 AmdSevDxeEntryPoint (
@@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
     }
   }
 
-  //
-  // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
-  // It contains the location for both the Secrets and CPUID page.
-  //
   if (MemEncryptSevSnpIsEnabled ()) {
+    //
+    // Memory acceptance began being required in SEV-SNP, so install the
+    // memory accept protocol implementation for a SEV-SNP active guest.
+    //
+    Status = gBS->InstallProtocolInterface (
+                    &mAmdSevDxeHandle,
+                    &gEfiMemoryAcceptProtocolGuid,
+                    EFI_NATIVE_INTERFACE,
+                    &mMemoryAcceptProtocol
+                    );
+    ASSERT_EFI_ERROR (Status);
+
+    //
+    // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+    // It contains the location for both the Secrets and CPUID page.
+    //
     return gBS->InstallConfigurationTable (
                   &gConfidentialComputingSevSnpBlobGuid,
                   &mSnpBootDxeTable
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..5ddddabc32 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
 
+[Protocols]
+  gEfiMemoryAcceptProtocolGuid
+
 [Guids]
   gConfidentialComputingSevSnpBlobGuid
 
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..ee3710f7b3 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
 #include <Library/MemEncryptSevLib.h>
 
 #include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
 
 /**
   Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
@@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
   IN UINTN             NumPages
   )
 {
+  EFI_STATUS Status;
+
   if (!MemEncryptSevSnpIsEnabled ()) {
     return;
   }
 
-  //
-  // All the pre-validation must be completed in the PEI phase.
-  //
-  ASSERT (FALSE);
+  // DXE pre-validation may happen with the memory accept protocol.
+  // The protocol should only be called outside the prevalidated ranges
+  // that the PEI stage code explicitly skips. Specifically, only memory
+  // ranges that are classified as unaccepted.
+  if (BaseAddress >= SIZE_4GB) {
+    Status = InternalMemEncryptSevCreateIdentityMap1G (
+               0,
+               BaseAddress,
+               EFI_PAGES_TO_SIZE (NumPages)
+               );
+    if (EFI_ERROR (Status)) {
+      ASSERT (FALSE);
+      CpuDeadLoop ();
+    }
+  }
+
+  InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
 }
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID

[Dionna Glaze]

Event group as defined in UEFI standard v2.9.

Cc: Ard Biescheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 MdePkg/Include/Guid/EventGroup.h | 5 +++++
 MdePkg/MdePkg.dec                | 5 ++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..64bfd4bab9 100644
--- a/MdePkg/Include/Guid/EventGroup.h
+++ b/MdePkg/Include/Guid/EventGroup.h
@@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 extern EFI_GUID  gEfiEventExitBootServicesGuid;
 
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+  { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b, 0xfc } }
+
+extern EFI_GUID  gEfiEventBeforeExitBootServicesGuid;
+
 #define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
   { 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51, 0x96 } }
 
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index de3c56758b..32c3501e66 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,7 +408,10 @@
   gEfiEventMemoryMapChangeGuid   = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB, 0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
 
   ## Include/Guid/EventGroup.h
-  gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+  gEfiEventVirtualAddressChangeGuid   = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+
+  ## Include/Guid/EventGroup.h
+  gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80, 0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
 
   ## Include/Guid/EventGroup.h
   gEfiEventExitBootServicesGuid  = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48, 0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Dionna Glaze]

Location of notification is has been specified in UEFI v2.9.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 MdeModulePkg/Core/Dxe/DxeMain.inf       | 1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..35d5bf0dee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -100,6 +100,7 @@
   gEfiEventVirtualAddressChangeGuid             ## CONSUMES             ## Event
   ## CONSUMES   ## Event
   ## PRODUCES   ## Event
+  gEfiEventBeforeExitBootServicesGuid
   gEfiEventExitBootServicesGuid
   gEfiHobMemoryAllocModuleGuid                  ## SOMETIMES_CONSUMES   ## HOB
   gEfiFirmwareFileSystem2Guid                   ## CONSUMES             ## GUID # Used to compare with FV's file system guid and get the FV's file system format
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..4683016ed7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -763,6 +763,12 @@ CoreExitBootServices (
 {
   EFI_STATUS  Status;
 
+  //
+  // Notify other drivers of their last chance to use boot services
+  // before the memory map is terminated.
+  //
+  CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
+
   //
   // Disable Timer
   //
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 4/7] OvmfPkg: Introduce CocoDxe driver

[Dionna Glaze]

This driver is meant as a join point for all Confidential Compute
technologies to put shared behavior that doesn't belong anywhere else.

The first behavior added here is to accept all unaccepted memory at
ExitBootServices if the behavior is not disabled. This allows safe
upgrades for OS loaders to affirm their support for the unaccepted
memory type.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc     |   1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf     |   1 +
 OvmfPkg/CocoDxe/CocoDxe.c        | 140 ++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf      |  45 +++++++
 OvmfPkg/IntelTdx/IntelTdxX64.dsc |   1 +
 OvmfPkg/IntelTdx/IntelTdxX64.fdf |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc       |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf       |   1 +
 OvmfPkg/OvmfPkgX64.dsc           |   1 +
 OvmfPkg/OvmfPkgX64.fdf           |   1 +
 10 files changed, 193 insertions(+)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 90e8a213ef..ad6b73ca4a 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -747,6 +747,7 @@
     <LibraryClasses>
     PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
   }
+  OvmfPkg/CocoDxe/CocoDxe.inf
   OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
   #
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 4658e1d30e..3717ec9094 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -302,6 +302,7 @@ INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF  OvmfPkg/CocoDxe/CocoDxe.inf
 INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
 
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
new file mode 100644
index 0000000000..ae64fbf28e
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,140 @@
+/** @file
+
+  Confidential Compute Dxe driver. This driver installs protocols that are
+  generic over confidential compute techonology.
+
+  Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/MemEncryptTdxLib.h>
+#include <Protocol/ExitBootServicesCallback.h>
+#include <Protocol/MemoryAccept.h>
+
+STATIC BOOLEAN mAcceptAllUnacceptedMemoryEnabled = TRUE;
+
+STATIC EFI_EVENT mAcceptAllUnacceptedMemoryEvent = NULL;
+
+STATIC
+EFI_STATUS
+AcceptAllUnacceptedMemory (
+  IN EFI_MEMORY_ACCEPT_PROTOCOL *AcceptMemory
+  )
+{
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *AllDescMap;
+  UINTN                            NumEntries;
+  UINTN                            Index;
+  EFI_STATUS                       Status;
+
+  DEBUG ((DEBUG_INFO, "Accepting all memory\n"));
+  /*
+   * Get a copy of the memory space map to iterate over while
+   * changing the map.
+   */
+  Status = gDS->GetMemorySpaceMap (&NumEntries, &AllDescMap);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+  for (Index = 0; Index < NumEntries; Index++) {
+    CONST EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *Desc;
+
+    Desc = &AllDescMap[Index];
+    if (Desc->GcdMemoryType != EfiGcdMemoryTypeUnaccepted) {
+      continue;
+    }
+
+    Status = AcceptMemory->AcceptMemory (
+      AcceptMemory,
+      Desc->BaseAddress,
+      Desc->Length
+      );
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+
+    Status = gDS->RemoveMemorySpace(Desc->BaseAddress, Desc->Length);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+
+    Status = gDS->AddMemorySpace (
+      EfiGcdMemoryTypeSystemMemory,
+      Desc->BaseAddress,
+      Desc->Length,
+      EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP
+      );
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+  }
+
+  gBS->FreePool (AllDescMap);
+  return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+  IN EFI_EVENT  Event,
+  IN VOID       *Context
+  )
+{
+  EFI_MEMORY_ACCEPT_PROTOCOL *AcceptMemory;
+  EFI_STATUS                 Status;
+
+  if (!mAcceptAllUnacceptedMemoryEnabled) {
+    return;
+  }
+
+  Status = gBS->LocateProtocol (&gEfiMemoryAcceptProtocolGuid, NULL,
+    (VOID **)&AcceptMemory);
+  if (Status == EFI_NOT_FOUND) {
+    return;
+  }
+  ASSERT_EFI_ERROR (Status);
+
+  Status = AcceptAllUnacceptedMemory(AcceptMemory);
+  ASSERT_EFI_ERROR (Status);
+}
+
+EFI_STATUS
+EFIAPI
+CocoDxeEntryPoint (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS Status;
+
+  //
+  // Do nothing when confidential compute technologies that require memory
+  // acceptance are not enabled.
+  //
+  if (!MemEncryptSevSnpIsEnabled () &&
+      !MemEncryptTdxIsEnabled ()) {
+    return EFI_UNSUPPORTED;
+  }
+
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  ResolveUnacceptedMemory,
+                  NULL,
+                  &gEfiEventBeforeExitBootServicesGuid,
+                  &mAcceptAllUnacceptedMemoryEvent
+                  );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "AcceptAllUnacceptedMemory event creation for EventBeforeExitBootServices failed.\n"));
+  }
+
+  return EFI_SUCCESS;
+}
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
new file mode 100644
index 0000000000..3bbb5fc9cc
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -0,0 +1,45 @@
+#/** @file
+#
+#  Driver installs shared protocols needed for confidential compute
+#  technologies.
+#
+#  Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 1.25
+  BASE_NAME                      = CocoDxe
+  FILE_GUID                      = 08162f1e-5147-4d3e-b5a9-fa48c9808419
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = CocoDxeEntryPoint
+
+[Sources]
+  CocoDxe.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DxeServicesTableLib
+  MemEncryptSevLib
+  MemEncryptTdxLib
+  MemoryAllocationLib
+  UefiDriverEntryPoint
+
+[Depex]
+  TRUE
+
+[Guids]
+  gEfiEventBeforeExitBootServicesGuid
+
+[Protocols]
+  gEfiMemoryAcceptProtocolGuid
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index c0c1a15b09..8136d50eb2 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -753,6 +753,7 @@
   OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
   OvmfPkg/TdxDxe/TdxDxe.inf
+  OvmfPkg/CocoDxe/CocoDxe.inf
 
   #
   # Variable driver stack (non-SMM)
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
index 6923eb8831..e612608c0c 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
@@ -269,6 +269,7 @@ INF  ShellPkg/Application/Shell/Shell.inf
 INF MdeModulePkg/Logo/LogoDxe.inf
 
 INF OvmfPkg/TdxDxe/TdxDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
 
 #
 # Usb Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index af566b953f..2cfb3fbc6b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -965,6 +965,7 @@
     <LibraryClasses>
     PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
   }
+  OvmfPkg/CocoDxe/CocoDxe.inf
   OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
 !if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 80de4fa2c0..2ab7f3b95b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -343,6 +343,7 @@ INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF  OvmfPkg/CocoDxe/CocoDxe.inf
 INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
 !if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f39d9cd117..3ead476b61 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1036,6 +1036,7 @@
   OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
   OvmfPkg/TdxDxe/TdxDxe.inf
+  OvmfPkg/CocoDxe/CocoDxe.inf
 
 !if $(SMM_REQUIRE) == TRUE
   OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index c0f5a1ef3c..5dd452f42b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -370,6 +370,7 @@ INF  OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
 INF  OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
 INF  OvmfPkg/PlatformDxe/Platform.inf
 INF  OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF  OvmfPkg/CocoDxe/CocoDxe.inf
 INF  OvmfPkg/IoMmuDxe/IoMmuDxe.inf
 
 !if $(SMM_REQUIRE) == TRUE
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 5/7] MdePkg: Introduce the AcceptAllUnacceptedMemory protocol

[Dionna Glaze]

The default behavior for unaccepted memory is to accept all memory
when ExitBootServices is called. An OS loader can use this protocol to
Disable this behavior to assume responsibility for memory acceptance and
to affirm that the OS can handle the unaccepted memory type.

This is a candidate for standardization.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h | 40 ++++++++++++++++++++
 MdePkg/MdePkg.dec                                         |  3 ++
 2 files changed, 43 insertions(+)

diff --git a/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h b/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
new file mode 100644
index 0000000000..e50831836c
--- /dev/null
+++ b/MdePkg/Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
@@ -0,0 +1,40 @@
+/** @file
+  The file provides the protocol that disables the behavior that all memory
+  gets accepted at ExitBootServices(). This protocol is only meant to be called
+  by the OS loader, and not EDK2 itself.
+
+  Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#ifndef _ACCEPT_ALL_UNACCEPTED_MEMORY_H_
+#define _ACCEPT_ALL_UNACCEPTED_MEMORY_H_
+
+#define BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL_GUID \
+  {0xc5a010fe, \
+   0x38a7, \
+   0x4531, \
+   {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL
+    BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL;
+
+/**
+  @param This A pointer to a BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *BZ3987_DISABLE_ACCEPT_ALL_UNACCEPTED_MEMORY)(
+  IN  BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL  *This
+  );
+
+///
+/// The BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL {
+  BZ3987_DISABLE_ACCEPT_ALL_UNACCEPTED_MEMORY  Disable;
+};
+
+extern EFI_GUID  gBz3987AcceptAllUnacceptedMemoryProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 32c3501e66..a453b67b7e 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1022,6 +1022,9 @@
   gEfiPeiDelayedDispatchPpiGuid  = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e, 0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
 
 [Protocols]
+  ## Include/Protocol/Bz3987AcceptAllUnacceptedMemory.h
+  gBz3987AcceptAllUnacceptedMemoryProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531, {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
   ## Include/Protocol/MemoryAccept.h
   gEfiMemoryAcceptProtocolGuid   = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3, 0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}
 
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe

[Dionna Glaze]

This protocol implementation disables the accept-all-memory behavior
of the BeforeExitBootServices event this driver adds.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 OvmfPkg/CocoDxe/CocoDxe.c   | 25 ++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf |  1 +
 2 files changed, 26 insertions(+)

diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
index ae64fbf28e..a7c54b36a3 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -16,6 +16,7 @@
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/MemEncryptSevLib.h>
 #include <Library/MemEncryptTdxLib.h>
+#include <Protocol/Bz3987AcceptAllUnacceptedMemory.h>
 #include <Protocol/ExitBootServicesCallback.h>
 #include <Protocol/MemoryAccept.h>
 
@@ -105,6 +106,21 @@ ResolveUnacceptedMemory (
   ASSERT_EFI_ERROR (Status);
 }
 
+STATIC
+EFI_STATUS
+EFIAPI
+DisableAcceptAllUnacceptedMemory (
+  IN  BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL  *This
+  )
+{
+  mAcceptAllUnacceptedMemoryEnabled = FALSE;
+  return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL
+mAcceptAllUnacceptedMemoryProtocol = {DisableAcceptAllUnacceptedMemory};
+
 EFI_STATUS
 EFIAPI
 CocoDxeEntryPoint (
@@ -136,5 +152,14 @@ CocoDxeEntryPoint (
     DEBUG ((DEBUG_ERROR, "AcceptAllUnacceptedMemory event creation for EventBeforeExitBootServices failed.\n"));
   }
 
+  Status = gBS->InstallProtocolInterface (&mCocoDxeHandle,
+                  &gBz3987AcceptAllUnacceptedMemoryProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mAcceptAllUnacceptedMemoryProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Install Bz3987AcceptAllUnacceptedMemoryProtocol failed.\n"));
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
index 3bbb5fc9cc..e24188147a 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.inf
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -42,4 +42,5 @@
   gEfiEventBeforeExitBootServicesGuid
 
 [Protocols]
+  gBz3987AcceptAllUnacceptedMemoryProtocolGuid
   gEfiMemoryAcceptProtocolGuid
-- 
2.38.0.rc1.362.ged0d419d3c-goog

[PATCH v7 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

[Dionna Glaze]

Instead of eagerly accepting all memory in PEI, only accept memory under
the 4GB address. This allows a loaded image to use the
ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL to disable the accept behavior and
indicate that it can interpret the memory type accordingly.

This classification is safe since ExitBootServices will accept and
reclassify the memory as conventional if the disable protocol is not
used.

Cc: Ard Biescheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 OvmfPkg/PlatformPei/AmdSev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c
index 385562b44c..2a52d6f491 100644
--- a/OvmfPkg/PlatformPei/AmdSev.c
+++ b/OvmfPkg/PlatformPei/AmdSev.c
@@ -16,6 +16,7 @@
 #include <Library/MemEncryptSevLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
+#include <Pi/PrePiHob.h>
 #include <PiPei.h>
 #include <Register/Amd/Msr.h>
 #include <Register/Intel/SmramSaveStateMap.h>
@@ -63,6 +64,10 @@ AmdSevSnpInitialize (
   for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
     if ((Hob.Raw != NULL) && (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_RESOURCE_DESCRIPTOR)) {
       ResourceHob = Hob.ResourceDescriptor;
+      if (ResourceHob->PhysicalStart >= SIZE_4GB) {
+        ResourceHob->ResourceType = EFI_RESOURCE_MEMORY_UNACCEPTED;
+        continue;
+      }
 
       if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
         MemEncryptSevSnpPreValidateSystemRam (
-- 
2.38.0.rc1.362.ged0d419d3c-goog

Re: [PATCH v7 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe

[Lendacky, Thomas]

On 10/5/22 15:33, Dionna Glaze wrote:
> From: Sophia Wolf <phiawolf@google.com>
> 
> When a guest OS does not support unaccepted memory, the unaccepted
> memory must be accepted before returning a memory map to the caller.
> 
> EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
> Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---

Re: [PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Lendacky, Thomas]

On 10/5/22 15:33, Dionna Glaze wrote:
> Location of notification is has been specified in UEFI v2.9.
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
>   MdeModulePkg/Core/Dxe/DxeMain.inf       | 1 +
>   MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
>   2 files changed, 7 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index e4bca89577..35d5bf0dee 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -100,6 +100,7 @@
>     gEfiEventVirtualAddressChangeGuid             ## CONSUMES             ## Event
>     ## CONSUMES   ## Event
>     ## PRODUCES   ## Event
> +  gEfiEventBeforeExitBootServicesGuid
>     gEfiEventExitBootServicesGuid
>     gEfiHobMemoryAllocModuleGuid                  ## SOMETIMES_CONSUMES   ## HOB
>     gEfiFirmwareFileSystem2Guid                   ## CONSUMES             ## GUID # Used to compare with FV's file system guid and get the FV's file system format
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 5733f0c8ec..4683016ed7 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -763,6 +763,12 @@ CoreExitBootServices (
>   {
>     EFI_STATUS  Status;
>   
> +  //
> +  // Notify other drivers of their last chance to use boot services
> +  // before the memory map is terminated.
> +  //
> +  CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);

Isn't this supposed to be after disabling the timer?

Thanks,
Tom

> +
>     //
>     // Disable Timer
>     //

Re: [PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Dionna Glaze]

The specification says that disabling the timer should happen right
after. Ard told me it should still work this way.

On Wed, Oct 5, 2022 at 1:50 PM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 10/5/22 15:33, Dionna Glaze wrote:
> > Location of notification is has been specified in UEFI v2.9.
> >
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: "Min M. Xu" <min.m.xu@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> >
> > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> > ---
> >   MdeModulePkg/Core/Dxe/DxeMain.inf       | 1 +
> >   MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
> >   2 files changed, 7 insertions(+)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > index e4bca89577..35d5bf0dee 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > @@ -100,6 +100,7 @@
> >     gEfiEventVirtualAddressChangeGuid             ## CONSUMES             ## Event
> >     ## CONSUMES   ## Event
> >     ## PRODUCES   ## Event
> > +  gEfiEventBeforeExitBootServicesGuid
> >     gEfiEventExitBootServicesGuid
> >     gEfiHobMemoryAllocModuleGuid                  ## SOMETIMES_CONSUMES   ## HOB
> >     gEfiFirmwareFileSystem2Guid                   ## CONSUMES             ## GUID # Used to compare with FV's file system guid and get the FV's file system format
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > index 5733f0c8ec..4683016ed7 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > @@ -763,6 +763,12 @@ CoreExitBootServices (
> >   {
> >     EFI_STATUS  Status;
> >
> > +  //
> > +  // Notify other drivers of their last chance to use boot services
> > +  // before the memory map is terminated.
> > +  //
> > +  CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
>
> Isn't this supposed to be after disabling the timer?
>
> Thanks,
> Tom
>
> > +
> >     //
> >     // Disable Timer
> >     //



-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Dionna Glaze]

> Ard told me it should still work this way.
>

Let me clarify that I've also tested the new code and it does still work.


-- 
-Dionna Glaze, PhD (she/her)

Re: [PATCH v7 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

[Lendacky, Thomas]

On 10/5/22 15:33, Dionna Glaze wrote:
> Instead of eagerly accepting all memory in PEI, only accept memory under
> the 4GB address. This allows a loaded image to use the
> ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL to disable the accept behavior and
> indicate that it can interpret the memory type accordingly.
> 
> This classification is safe since ExitBootServices will accept and
> reclassify the memory as conventional if the disable protocol is not
> used.
> 
> Cc: Ard Biescheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---

Re: [PATCH v7 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID

[Ard Biesheuvel]

(cc Liming & Mike)

On Wed, 5 Oct 2022 at 22:33, Dionna Glaze <dionnaglaze@google.com> wrote:
>
> Event group as defined in UEFI standard v2.9.
>
> Cc: Ard Biescheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  MdePkg/Include/Guid/EventGroup.h | 5 +++++
>  MdePkg/MdePkg.dec                | 5 ++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
> index 063d1f7157..64bfd4bab9 100644
> --- a/MdePkg/Include/Guid/EventGroup.h
> +++ b/MdePkg/Include/Guid/EventGroup.h
> @@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  extern EFI_GUID  gEfiEventExitBootServicesGuid;
>
> +#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
> +  { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b, 0xfc } }
> +
> +extern EFI_GUID  gEfiEventBeforeExitBootServicesGuid;
> +
>  #define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
>    { 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51, 0x96 } }
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index de3c56758b..32c3501e66 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -408,7 +408,10 @@
>    gEfiEventMemoryMapChangeGuid   = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB, 0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
>
>    ## Include/Guid/EventGroup.h
> -  gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> +  gEfiEventVirtualAddressChangeGuid   = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> +
> +  ## Include/Guid/EventGroup.h
> +  gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80, 0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
>
>    ## Include/Guid/EventGroup.h
>    gEfiEventExitBootServicesGuid  = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48, 0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>

Re: [PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Ard Biesheuvel]

(cc Liming & Mike)

On Wed, 5 Oct 2022 at 22:33, Dionna Glaze <dionnaglaze@google.com> wrote:
>
> Location of notification is has been specified in UEFI v2.9.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>

> ---
>  MdeModulePkg/Core/Dxe/DxeMain.inf       | 1 +
>  MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index e4bca89577..35d5bf0dee 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -100,6 +100,7 @@
>    gEfiEventVirtualAddressChangeGuid             ## CONSUMES             ## Event
>    ## CONSUMES   ## Event
>    ## PRODUCES   ## Event
> +  gEfiEventBeforeExitBootServicesGuid
>    gEfiEventExitBootServicesGuid
>    gEfiHobMemoryAllocModuleGuid                  ## SOMETIMES_CONSUMES   ## HOB
>    gEfiFirmwareFileSystem2Guid                   ## CONSUMES             ## GUID # Used to compare with FV's file system guid and get the FV's file system format
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 5733f0c8ec..4683016ed7 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -763,6 +763,12 @@ CoreExitBootServices (
>  {
>    EFI_STATUS  Status;
>
> +  //
> +  // Notify other drivers of their last chance to use boot services
> +  // before the memory map is terminated.
> +  //
> +  CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
> +
>    //
>    // Disable Timer
>    //
> --
> 2.38.0.rc1.362.ged0d419d3c-goog
>

回复: [edk2-devel] [PATCH v7 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[gaoliming]

UEFI spec describes that the event presents the last opportunity to use firmware interfaces in the boot environment.

So, I agree that this event notification is placed before Disable Timer. 

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Dionna Glaze
> via groups.io
> 发送时间: 2022年10月6日 4:59
> 收件人: Tom Lendacky <thomas.lendacky@amd.com>
> 抄送: devel@edk2.groups.io; Gerd Hoffmann <kraxel@redhat.com>; James
> Bottomley <jejb@linux.ibm.com>; Jiewen Yao <jiewen.yao@intel.com>; Ard
> Biesheuvel <ardb@kernel.org>; Min M. Xu <min.m.xu@intel.com>; Andrew
> Fish <afish@apple.com>; Michael D. Kinney <michael.d.kinney@intel.com>;
> Ray Ni <ray.ni@intel.com>
> 主题: Re: [edk2-devel] [PATCH v7 3/7] MdeModulePkg: Notify
> BeforeExitBootServices in CoreExitBootServices
> 
> The specification says that disabling the timer should happen right
> after. Ard told me it should still work this way.
> 
> On Wed, Oct 5, 2022 at 1:50 PM Tom Lendacky <thomas.lendacky@amd.com>
> wrote:
> >
> > On 10/5/22 15:33, Dionna Glaze wrote:
> > > Location of notification is has been specified in UEFI v2.9.
> > >
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: James Bottomley <jejb@linux.ibm.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: "Min M. Xu" <min.m.xu@intel.com>
> > > Cc: Andrew Fish <afish@apple.com>
> > > Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > >
> > > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> > > ---
> > >   MdeModulePkg/Core/Dxe/DxeMain.inf       | 1 +
> > >   MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
> > >   2 files changed, 7 insertions(+)
> > >
> > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > index e4bca89577..35d5bf0dee 100644
> > > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > @@ -100,6 +100,7 @@
> > >     gEfiEventVirtualAddressChangeGuid             ## CONSUMES
> ## Event
> > >     ## CONSUMES   ## Event
> > >     ## PRODUCES   ## Event
> > > +  gEfiEventBeforeExitBootServicesGuid
> > >     gEfiEventExitBootServicesGuid
> > >     gEfiHobMemoryAllocModuleGuid                  ##
> SOMETIMES_CONSUMES   ## HOB
> > >     gEfiFirmwareFileSystem2Guid                   ## CONSUMES
> ## GUID # Used to compare with FV's file system guid and get the FV's file
> system format
> > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > index 5733f0c8ec..4683016ed7 100644
> > > --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> > > @@ -763,6 +763,12 @@ CoreExitBootServices (
> > >   {
> > >     EFI_STATUS  Status;
> > >
> > > +  //
> > > +  // Notify other drivers of their last chance to use boot services
> > > +  // before the memory map is terminated.
> > > +  //
> > > +  CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
> >
> > Isn't this supposed to be after disabling the timer?
> >
> > Thanks,
> > Tom
> >
> > > +
> > >     //
> > >     // Disable Timer
> > >     //
> 
> 
> 
> --
> -Dionna Glaze, PhD (she/her)
> 
> 
> 
> 

回复: [edk2-devel] [PATCH v7 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2022年10月6日 20:45
> 收件人: Dionna Glaze <dionnaglaze@google.com>
> 抄送: devel@edk2.groups.io; Min M. Xu <min.m.xu@intel.com>; Gerd
> Hoffmann <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>;
> Tom Lendacky <Thomas.Lendacky@amd.com>; Jiewen Yao
> <jiewen.yao@intel.com>; Erdem Aktas <erdemaktas@google.com>; Liming
> Gao (Byosoft address) <gaoliming@byosoft.com.cn>; Michael Kinney
> <michael.d.kinney@intel.com>
> 主题: Re: [edk2-devel] [PATCH v7 2/7] MdePkg: Add
> EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> 
> (cc Liming & Mike)
> 
> On Wed, 5 Oct 2022 at 22:33, Dionna Glaze <dionnaglaze@google.com>
> wrote:
> >
> > Event group as defined in UEFI standard v2.9.
> >
> > Cc: Ard Biescheuvel <ardb@kernel.org>
> > Cc: "Min M. Xu" <min.m.xu@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Erdem Aktas <erdemaktas@google.com>
> >
> > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> 
> Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> 
> > ---
> >  MdePkg/Include/Guid/EventGroup.h | 5 +++++
> >  MdePkg/MdePkg.dec                | 5 ++++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdePkg/Include/Guid/EventGroup.h
> b/MdePkg/Include/Guid/EventGroup.h
> > index 063d1f7157..64bfd4bab9 100644
> > --- a/MdePkg/Include/Guid/EventGroup.h
> > +++ b/MdePkg/Include/Guid/EventGroup.h
> > @@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> >  extern EFI_GUID  gEfiEventExitBootServicesGuid;
> >
> > +#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
> > +  { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f,
> 0x3b, 0xfc } }
> > +
> > +extern EFI_GUID  gEfiEventBeforeExitBootServicesGuid;
> > +
> >  #define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
> >    { 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51,
> 0x96 } }
> >
> > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> > index de3c56758b..32c3501e66 100644
> > --- a/MdePkg/MdePkg.dec
> > +++ b/MdePkg/MdePkg.dec
> > @@ -408,7 +408,10 @@
> >    gEfiEventMemoryMapChangeGuid   = { 0x78BEE926, 0x692F, 0x48FD,
> { 0x9E, 0xDB, 0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
> >
> >    ## Include/Guid/EventGroup.h
> > -  gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7,
> { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> > +  gEfiEventVirtualAddressChangeGuid   = { 0x13FA7698, 0xC831, 0x49C7,
> { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> > +
> > +  ## Include/Guid/EventGroup.h
> > +  gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44,
> { 0x80, 0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
> >
> >    ## Include/Guid/EventGroup.h
> >    gEfiEventExitBootServicesGuid  = { 0x27ABF055, 0xB1B8, 0x4C26,
> { 0x80, 0x48, 0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
> > --
> > 2.38.0.rc1.362.ged0d419d3c-goog
> >
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Ni, Ray]

> 
> To allow the OS loader to prevent the eager acceptance, we add a new
> protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
> This protocol has one interface, Disable(). The OS loader can inform the
> UEFI that it supports the unaccepted memory type and accepts the
> responsibility to accept it.
> 
> All images that support unaccepted memory must now locate and call this
> new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
> function.
> 

Can OsIndicationsSupported UEFI variable provide the similar functionality?

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

>
> Can OsIndicationsSupported UEFI variable provide the similar functionality?
>

Similar, but not the same. If we use a UEFI variable, its value will
persist across boots. This can be fine if you only boot one OS, but if
you have two where one supports unaccepted memory and the other
doesn't then you have a problem.
The protocol here is specific to the code that will be calling
ExitBootServices, and thus won't mess up another OS once we reboot and
select it.

-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Fri, 14 Oct 2022 at 23:30, Dionna Glaze via groups.io
<dionnaglaze=google.com@groups.io> wrote:
>
> >
> > Can OsIndicationsSupported UEFI variable provide the similar functionality?
> >
>
> Similar, but not the same. If we use a UEFI variable, its value will
> persist across boots.

That assumes the variable is non-volatile, but I suppose we could use
a volatile variable [other than OsIndications] as well.

> This can be fine if you only boot one OS, but if
> you have two where one supports unaccepted memory and the other
> doesn't then you have a problem.
> The protocol here is specific to the code that will be calling
> ExitBootServices, and thus won't mess up another OS once we reboot and
> select it.
>
> --
> -Dionna Glaze, PhD (she/her)
>
>
> 
>
>

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

> That assumes the variable is non-volatile, but I suppose we could use
> a volatile variable [other than OsIndications] as well.
>

I suppose that's true. Would you prefer volatile versions of
OsIndications/OsIndicationsSupported added to the spec, or this
proposed one-off toggle protocol? No specified global variables seem
overloadable with this duty.

-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Fri, 21 Oct 2022 at 00:37, Dionna Amalie Glaze
<dionnaglaze@google.com> wrote:
>
> > That assumes the variable is non-volatile, but I suppose we could use
> > a volatile variable [other than OsIndications] as well.
> >
>
> I suppose that's true. Would you prefer volatile versions of
> OsIndications/OsIndicationsSupported added to the spec, or this
> proposed one-off toggle protocol? No specified global variables seem
> overloadable with this duty.
>

No it would have to be a completely separate variable, I don't think
we can use OsIndications for this.

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

> > I suppose that's true. Would you prefer volatile versions of
> > OsIndications/OsIndicationsSupported added to the spec, or this
> > proposed one-off toggle protocol? No specified global variables seem
> > overloadable with this duty.
> >
>
> No it would have to be a completely separate variable, I don't think
> we can use OsIndications for this.

Certainly, I meant new variables that are volatile and also play a
similar role as OsIndications[Supported].

-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[aik]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

Hey!

Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.

I tried https://github.com/deeglaze/edk2.git ( https://github.com/deeglaze/edk2.git ) enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
I tried rebasing it on top of https://github.com/mxu9/edk2.git lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.

Thanks!

[-- Attachment #2: Type: text/html, Size: 762 bytes --]

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

>
> Hey!
>
> Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.
>
> I tried https://github.com/deeglaze/edk2.git   enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
> I tried rebasing it on top of https://github.com/mxu9/edk2.git   lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.
>
> Thanks!

Oh that would be because I forgot to git add those files (it built on
my machine TM). The callback.h file is from a previous version of this
series that can be removed. PrePiHob.h is from a lazy accept patch,
"MdePkg: Add UEFI Unaccepted memory definition". I can send out a v8
that removes the unnecessary #include and changes the names to those
suggested in the 3987 bug. I'll fix my github branch to include the
missing file.

Thanks for trying this patch series :)

-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[aik]

Hi Dionna,

Thanks for updating the tree, builds nicely now! However the VM's kernel 
does not boot - the guest kernel reports

EFI stub: ERROR: exit_boot() failed!

and hangs. I am not quite sure how it is supposed to work (still 
learning) but "Accepting all memory" happens twice (should it?) and the 
actual reason for the CoreExitBootService() failure is that MapKey != 
mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9 
or  0x7AE1 vs 0x7AE3 (the diff is always 2).

How do you test it exactly, is there any command line change needed in 
addition to enabling SNP?

My guest kernel uses 
https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/ 
with the TDX prerequisite. Thanks,


On 25/10/2022 02:24, Dionna Amalie Glaze wrote:
>>
>> Hey!
>>
>> Sorry if this was asked. I was wondering if this patchset is in some git repo which I pull from as I struggle to get a buildable tree by applying this patchset to whatever I can find.
>>
>> I tried https://github.com/deeglaze/edk2.git   enable_umv7 but it does not build (missing ExitBootServicesCallback.h and PrePiHob.h).
>> I tried rebasing it on top of https://github.com/mxu9/edk2.git   lazyaccept.v1/2/3/4 but it seems only v1 kinda works but see the above and other versions have some reworks (EFI_RESOURCE_MEMORY_UNACCEPTED vs. BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED.
>>
>> Thanks!
> 
> Oh that would be because I forgot to git add those files (it built on
> my machine TM). The callback.h file is from a previous version of this
> series that can be removed. PrePiHob.h is from a lazy accept patch,
> "MdePkg: Add UEFI Unaccepted memory definition". I can send out a v8
> that removes the unnecessary #include and changes the names to those
> suggested in the 3987 bug. I'll fix my github branch to include the
> missing file.
> 
> Thanks for trying this patch series :)
> 

-- 
Alexey

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> Hi Dionna,
>
> Thanks for updating the tree, builds nicely now! However the VM's kernel
> does not boot - the guest kernel reports
>
> EFI stub: ERROR: exit_boot() failed!
>
> and hangs. I am not quite sure how it is supposed to work (still
> learning) but "Accepting all memory" happens twice (should it?) and the
> actual reason for the CoreExitBootService() failure is that MapKey !=
> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>

"Accepting all memory" may happen twice, but it's idempotent. The
debug_info log happening twice might be confusing, so I can change
that if you'd like.

The first accept will remove all unaccepted memory regions from the
address space map.
CoreExitBootService should fail the first time since the first accept
will change the memory map.
That failure means that the caller should GetMemoryMap again and try
CoreExitBootService again.

> How do you test it exactly, is there any command line change needed in
> addition to enabling SNP?
>
> My guest kernel uses
> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
> with the TDX prerequisite. Thanks,
>

It's a few name changes behind, but this branch of Linux is what I've
been using:

https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum

Specific enablement patch here
https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61

I incorporate Kirill's patch set v7 for basic unaccepted memory
support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
memory, and I have a single patch that calls the protocol.
This branch doesn't have Kirill's TDX patches.
I've run it with a regular SEV-SNP enabled guest kernel too. At this
point all the tests have used kernel injection rather than having the
kernels all baked into the image.


-- 
-Dionna Glaze, PhD (she/her)

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Alexey Kardashevskiy]

On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> Hi Dionna,
>>
>> Thanks for updating the tree, builds nicely now! However the VM's kernel
>> does not boot - the guest kernel reports
>>
>> EFI stub: ERROR: exit_boot() failed!
>>
>> and hangs. I am not quite sure how it is supposed to work (still
>> learning) but "Accepting all memory" happens twice (should it?) and the
>> actual reason for the CoreExitBootService() failure is that MapKey !=
>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>>
> 
> "Accepting all memory" may happen twice, but it's idempotent. The
> debug_info log happening twice might be confusing, so I can change
> that if you'd like.

Nah, it is fine as long as the thing boots.


> The first accept will remove all unaccepted memory regions from the
> address space map.
> CoreExitBootService should fail the first time since the first accept
> will change the memory map.
> That failure means that the caller should GetMemoryMap again and try
> CoreExitBootService again.

Ah, ok.

>> How do you test it exactly, is there any command line change needed in
>> addition to enabling SNP?
>>
>> My guest kernel uses
>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>> with the TDX prerequisite. Thanks,
>>
> 
> It's a few name changes behind, but this branch of Linux is what I've
> been using:
> 
> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum

This does not build though due to unresolved rebase conflict, the fix is 
kinda trivial but I do not get those "Accepting all memory" anymore so I 
wonder what else is missing.

> 
> Specific enablement patch here
> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
> 
> I incorporate Kirill's patch set v7 for basic unaccepted memory
> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
> memory, and I have a single patch that calls the protocol.
> This branch doesn't have Kirill's TDX patches.
> I've run it with a regular SEV-SNP enabled guest kernel too. At this
> point all the tests have used kernel injection rather than having the
> kernels all baked into the image.

I am trying with "-m 2G" and "-m 8G" and see no difference - "Accepting 
all memory" is not appearing with this kernel. What do I miss? Thanks,


-- 
Alexey

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Alexey Kardashevskiy]

On 26/10/2022 12:35, Alexey Kardashevskiy wrote:
> 
> 
> On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
>> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> 
>> wrote:
>>>
>>> Hi Dionna,
>>>
>>> Thanks for updating the tree, builds nicely now! However the VM's kernel
>>> does not boot - the guest kernel reports
>>>
>>> EFI stub: ERROR: exit_boot() failed!
>>>
>>> and hangs. I am not quite sure how it is supposed to work (still
>>> learning) but "Accepting all memory" happens twice (should it?) and the
>>> actual reason for the CoreExitBootService() failure is that MapKey !=
>>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).
>>>
>>
>> "Accepting all memory" may happen twice, but it's idempotent. The
>> debug_info log happening twice might be confusing, so I can change
>> that if you'd like.
> 
> Nah, it is fine as long as the thing boots.
> 
> 
>> The first accept will remove all unaccepted memory regions from the
>> address space map.
>> CoreExitBootService should fail the first time since the first accept
>> will change the memory map.
>> That failure means that the caller should GetMemoryMap again and try
>> CoreExitBootService again.
> 
> Ah, ok.
> 
>>> How do you test it exactly, is there any command line change needed in
>>> addition to enabling SNP?
>>>
>>> My guest kernel uses
>>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>>> with the TDX prerequisite. Thanks,
>>>
>>
>> It's a few name changes behind, but this branch of Linux is what I've
>> been using:
>>
>> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum
> 
> This does not build though due to unresolved rebase conflict, the fix is 
> kinda trivial but I do not get those "Accepting all memory" anymore so I 
> wonder what else is missing.

AH right, stupid me, getting rid of accepting all memory is the purpose 
of these patches. Never mind :) Thanks,

> 
>>
>> Specific enablement patch here
>> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
>>
>> I incorporate Kirill's patch set v7 for basic unaccepted memory
>> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
>> memory, and I have a single patch that calls the protocol.
>> This branch doesn't have Kirill's TDX patches.
>> I've run it with a regular SEV-SNP enabled guest kernel too. At this
>> point all the tests have used kernel injection rather than having the
>> kernels all baked into the image.
> 
> I am trying with "-m 2G" and "-m 8G" and see no difference - "Accepting 
> all memory" is not appearing with this kernel. What do I miss? Thanks,
> 
> 

-- 
Alexey

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Alexey Kardashevskiy]

On 26/10/2022 13:49, Alexey Kardashevskiy wrote:
> 
> 
> On 26/10/2022 12:35, Alexey Kardashevskiy wrote:
>>
>>
>> On 26/10/2022 12:07, Dionna Amalie Glaze wrote:
>>> On Tue, Oct 25, 2022 at 5:23 PM Alexey Kardashevskiy <aik@ozlabs.ru> 
>>> wrote:
>>>>
>>>> Hi Dionna,
>>>>
>>>> Thanks for updating the tree, builds nicely now! However the VM's 
>>>> kernel
>>>> does not boot - the guest kernel reports
>>>>
>>>> EFI stub: ERROR: exit_boot() failed!
>>>>
>>>> and hangs. I am not quite sure how it is supposed to work (still
>>>> learning) but "Accepting all memory" happens twice (should it?) and the
>>>> actual reason for the CoreExitBootService() failure is that MapKey !=
>>>> mMemoryMapKey in CoreTerminateMemoryMap(), these are 0x7AD7 vs 0x7AD9
>>>> or  0x7AE1 vs 0x7AE3 (the diff is always 2).


btw it still fails in CoreTerminateMemoryMap() with the current upstream 
kernel which is not aware of the lazy memory acceptance, is this 
something known? Thanks,


>>>>
>>>
>>> "Accepting all memory" may happen twice, but it's idempotent. The
>>> debug_info log happening twice might be confusing, so I can change
>>> that if you'd like.
>>
>> Nah, it is fine as long as the thing boots.
>>
>>
>>> The first accept will remove all unaccepted memory regions from the
>>> address space map.
>>> CoreExitBootService should fail the first time since the first accept
>>> will change the memory map.
>>> That failure means that the caller should GetMemoryMap again and try
>>> CoreExitBootService again.
>>
>> Ah, ok.
>>
>>>> How do you test it exactly, is there any command line change needed in
>>>> addition to enabling SNP?
>>>>
>>>> My guest kernel uses
>>>> https://patchew.org/linux/cover.1664298261.git.thomas.lendacky@amd.com/
>>>> with the TDX prerequisite. Thanks,
>>>>
>>>
>>> It's a few name changes behind, but this branch of Linux is what I've
>>> been using:
>>>
>>> https://github.com/deeglaze/amdese-linux/tree/v12unacceptedv7v5-enableum
>>
>> This does not build though due to unresolved rebase conflict, the fix 
>> is kinda trivial but I do not get those "Accepting all memory" anymore 
>> so I wonder what else is missing.
> 
> AH right, stupid me, getting rid of accepting all memory is the purpose 
> of these patches. Never mind :) Thanks,
> 
>>
>>>
>>> Specific enablement patch here
>>> https://github.com/AMDESE/linux/commit/5a708081d58d773e767b11735ee1fd17ef5e5f61
>>>
>>> I incorporate Kirill's patch set v7 for basic unaccepted memory
>>> support, Tom Lendacky's v5 patch set for SEV-SNP support of unaccepted
>>> memory, and I have a single patch that calls the protocol.
>>> This branch doesn't have Kirill's TDX patches.
>>> I've run it with a regular SEV-SNP enabled guest kernel too. At this
>>> point all the tests have used kernel injection rather than having the
>>> kernels all baked into the image.
>>
>> I am trying with "-m 2G" and "-m 8G" and see no difference - 
>> "Accepting all memory" is not appearing with this kernel. What do I 
>> miss? Thanks,
>>
>>
> 

-- 
Alexey

Re: [edk2-devel] [PATCH v7 0/7] Add safe unaccepted memory behavior

[Dionna Glaze]

>
> btw it still fails in CoreTerminateMemoryMap() with the current upstream
> kernel which is not aware of the lazy memory acceptance, is this
> something known? Thanks,
>

It wasn't known. I'll take a look. Thanks for the report.

-- 
-Dionna Glaze, PhD (she/her)