[PATCH v9 0/4] Add safe unaccepted memory behavior

[PATCH v9 0/4] Add safe unaccepted memory behavior

[Dionna Glaze]

We make eager memory acceptance the default behavior at ExitBootServices
by using the standard-enforced behavior that if the call returns an
error code, then the map key is incorrect and the caller must re-call
GetMemoryMap to ensure the contents are correct.

Eager memory acceptance is implemented by using the UEFI v2.9-added
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support condition
before changing all unaccepted memory type regions to conventional
memory after first using the MemoryAccept protocol to accept all memory
in each region. This update to the memory map only happens once, since
there are no extra unaccepted memory regions to change on the forced
second call to ExitBootServices.

The new acceptance logic is technology-agnostic and usable across TEE
technologies, so this patch series introduces a Confidenial Compute DXE
driver called CocoDxe.

To allow the OS loader to prevent the eager acceptance, and thus pass
the before-mentioned "support condition", 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.

The MemoryAcceptance protocol is necessary for safe rollout of the
unaccepted memory type, given the gradual update of guest OS kernels.
OVMF cannot rely on the following implication

  (MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ())

  implies

  unaccepted memory is supported by the guest

This implication does not hold for e.g., upstream Linux, and will not
hold generally since both SEV-SNP and TDX define unaccepted memory
support as optional rather than required.

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 v8:
 - First 3 patches removed since they were submitted separately.
 - Later patches rebased on edk2/master and modified to work with the
   current locations and namings of the unaccepted memory constants.
Changes since v7:
 - Rebased onto lazy accept v4 patch series, so memory accept protocol
   has the EDKII prefix, and the unaccepted memory type has the BZ3937
   prefix.
 - Removed a bad #include to a header removed in v7.
 - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as per the
   discussion on the buganizer issue.
 - Uncrustify formatting

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 (4):
  OvmfPkg: Introduce CocoDxe driver
  MdePkg: Introduce the MemoryAcceptance protocol
  OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
  OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted

 MdePkg/Include/Protocol/MemoryAcceptance.h |  40 +++++
 MdePkg/MdePkg.dec                          |   3 +
 OvmfPkg/AmdSev/AmdSevX64.dsc               |   1 +
 OvmfPkg/AmdSev/AmdSevX64.fdf               |   1 +
 OvmfPkg/CocoDxe/CocoDxe.c                  | 175 +++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf                |  46 ++++++
 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 +
 OvmfPkg/PlatformPei/AmdSev.c               |   5 +
 13 files changed, 277 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf

-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v9 1/4] 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        | 147 +++++++++++++++++++++++++++++++
 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, 200 insertions(+)
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
 create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 36100f5fdc..5e5e9887bb 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -749,6 +749,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 5fb3b5d276..ae64693c28 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..da16af32a3
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,147 @@
+/** @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 <Pi/PrePiDxeCis.h>
+#include <Protocol/MemoryAccept.h>
+
+STATIC BOOLEAN  mAcceptAllMemoryAtEBS = TRUE;
+
+STATIC EFI_EVENT  mAcceptAllMemoryEvent = NULL;
+
+STATIC
+EFI_STATUS
+AcceptAllMemory (
+  IN EDKII_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 != EFI_GCD_MEMORY_TYPE_UNACCEPTED) {
+      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
+  )
+{
+  EDKII_MEMORY_ACCEPT_PROTOCOL  *AcceptMemory;
+  EFI_STATUS                    Status;
+
+  if (!mAcceptAllMemoryAtEBS) {
+    return;
+  }
+
+  Status = gBS->LocateProtocol (
+                  &gEdkiiMemoryAcceptProtocolGuid,
+                  NULL,
+                  (VOID **)&AcceptMemory
+                  );
+  if (Status == EFI_NOT_FOUND) {
+    return;
+  }
+
+  ASSERT_EFI_ERROR (Status);
+
+  Status = AcceptAllMemory (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,
+                  &mAcceptAllMemoryEvent
+                  );
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory 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..8d4452e94d
--- /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]
+  gEdkiiMemoryAcceptProtocolGuid
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 81511e3556..c3e64d97c0 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -754,6 +754,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 a57bbcee89..f5765b50eb 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 a9d422bd91..8e4d31bcea 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -966,6 +966,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 4c5bd0dbc3..7d75140fe3 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 8401d73900..a728610c86 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1037,6 +1037,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 8c02dfe11e..9e0aee225c 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.39.0.314.g84b9a713c41-goog

[PATCH v9 2/4] MdePkg: Introduce the MemoryAcceptance 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/MemoryAcceptance.h | 40 ++++++++++++++++++++++
 MdePkg/MdePkg.dec                          |  3 ++
 2 files changed, 43 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h

diff --git a/MdePkg/Include/Protocol/MemoryAcceptance.h b/MdePkg/Include/Protocol/MemoryAcceptance.h
new file mode 100644
index 0000000000..0b305b016f
--- /dev/null
+++ b/MdePkg/Include/Protocol/MemoryAcceptance.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 MEMORY_ACCEPTANCE_H_
+#define MEMORY_ACCEPTANCE_H_
+
+#define BZ3987_MEMORY_ACCEPTANCE_PROTOCOL_GUID \
+  {0xc5a010fe, \
+   0x38a7, \
+   0x4531, \
+   {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49}}
+
+typedef struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL BZ3987_MEMORY_ACCEPTANCE_PROTOCOL;
+
+/**
+  @param This A pointer to a BZ3987_MEMORY_ACCEPTANCE_PROTOCOL.
+**/
+typedef
+  EFI_STATUS
+(EFIAPI *BZ3987_ALLOW_UNACCEPTED_MEMORY)(
+  IN  BZ3987_MEMORY_ACCEPTANCE_PROTOCOL  *This
+  );
+
+///
+/// The BZ3987_MEMORY_ACCEPTANCE_PROTOCOL allows the OS loader to
+/// indicate to EDK2 that ExitBootServices should not accept all memory.
+///
+struct _BZ3987_MEMORY_ACCEPTANCE_PROTOCOL {
+  BZ3987_ALLOW_UNACCEPTED_MEMORY    AllowUnacceptedMemory;
+};
+
+extern EFI_GUID  gBz3987MemoryAcceptanceProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 3d08f20d15..bc3d897248 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1031,6 +1031,9 @@
   gEfiPeiDelayedDispatchPpiGuid  = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e, 0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
 
 [Protocols]
+  ## Include/Protocol/Bz3987MemoryAcceptance.h
+  gBz3987MemoryAcceptanceProtocolGuid = { 0xc5a010fe, 0x38a7, 0x4531, {0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49 }}
+
   ## Include/Protocol/MemoryAccept.h
   gEdkiiMemoryAcceptProtocolGuid = { 0x38c74800, 0x5590, 0x4db4, { 0xa0, 0xf3, 0x67, 0x5d, 0x9b, 0x8e, 0x80, 0x26 }}
 
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v9 3/4] 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   | 28 ++++++++++++++++++++++++++++
 OvmfPkg/CocoDxe/CocoDxe.inf |  1 +
 2 files changed, 29 insertions(+)

diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
index da16af32a3..57169b0481 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -18,11 +18,14 @@
 #include <Library/MemEncryptTdxLib.h>
 #include <Pi/PrePiDxeCis.h>
 #include <Protocol/MemoryAccept.h>
+#include <Protocol/MemoryAcceptance.h>
 
 STATIC BOOLEAN  mAcceptAllMemoryAtEBS = TRUE;
 
 STATIC EFI_EVENT  mAcceptAllMemoryEvent = NULL;
 
+STATIC EFI_HANDLE  mCocoDxeHandle = NULL;
+
 STATIC
 EFI_STATUS
 AcceptAllMemory (
@@ -111,6 +114,21 @@ ResolveUnacceptedMemory (
   ASSERT_EFI_ERROR (Status);
 }
 
+STATIC
+EFI_STATUS
+EFIAPI
+AllowUnacceptedMemory (
+  IN  BZ3987_MEMORY_ACCEPTANCE_PROTOCOL  *This
+  )
+{
+  mAcceptAllMemoryAtEBS = FALSE;
+  return EFI_SUCCESS;
+}
+
+STATIC
+BZ3987_MEMORY_ACCEPTANCE_PROTOCOL
+  mMemoryAcceptanceProtocol = { AllowUnacceptedMemory };
+
 EFI_STATUS
 EFIAPI
 CocoDxeEntryPoint (
@@ -143,5 +161,15 @@ CocoDxeEntryPoint (
     DEBUG ((DEBUG_ERROR, "AllowUnacceptedMemory event creation for EventBeforeExitBootServices failed.\n"));
   }
 
+  Status = gBS->InstallProtocolInterface (
+                  &mCocoDxeHandle,
+                  &gBz3987MemoryAcceptanceProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mMemoryAcceptanceProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Install Bz3987MemoryAcceptanceProtocol failed.\n"));
+  }
+
   return EFI_SUCCESS;
 }
diff --git a/OvmfPkg/CocoDxe/CocoDxe.inf b/OvmfPkg/CocoDxe/CocoDxe.inf
index 8d4452e94d..05c2651a89 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.inf
+++ b/OvmfPkg/CocoDxe/CocoDxe.inf
@@ -42,4 +42,5 @@
   gEfiEventBeforeExitBootServicesGuid
 
 [Protocols]
+  gBz3987MemoryAcceptanceProtocolGuid
   gEdkiiMemoryAcceptProtocolGuid
-- 
2.39.0.314.g84b9a713c41-goog

[PATCH v9 4/4] 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
MEMORY_ACCEPTANCE_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 e4e7b72e67..7d824cc282 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 = BZ3937_EFI_RESOURCE_MEMORY_UNACCEPTED;
+        continue;
+      }
 
       if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
         MemEncryptSevSnpPreValidateSystemRam (
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH v9 0/4] Add safe unaccepted memory behavior

[Yao, Jiewen]

Hi Dionna
I think I understand your intention.
I believe we need OS side and UEFI standard sign-off for this *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the consumer, right?
If so, I suggest you maintain the work in a edk2-stage area for https://github.com/tianocore/edk2-staging.

EDKII main branch is for production. MdePkg can only include the API definition approved by UEFI standard.
EDK2 staging is a place for POC / collaboration. That is why I think edk2 staging is more proper place for this feature.

Without OS and UEFI standard sign-off, I don't think this BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII main branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.


Thank you
Yao, Jiewen

> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Friday, January 13, 2023 8:14 AM
> To: devel@edk2.groups.io
> Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>; Tom
> Lendacky <Thomas.Lendacky@amd.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Aktas, Erdem <erdemaktas@google.com>; Andrew
> Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v9 0/4] Add safe unaccepted memory behavior
> 
> We make eager memory acceptance the default behavior at ExitBootServices
> by using the standard-enforced behavior that if the call returns an
> error code, then the map key is incorrect and the caller must re-call
> GetMemoryMap to ensure the contents are correct.
> 
> Eager memory acceptance is implemented by using the UEFI v2.9-added
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES to check a support
> condition
> before changing all unaccepted memory type regions to conventional
> memory after first using the MemoryAccept protocol to accept all memory
> in each region. This update to the memory map only happens once, since
> there are no extra unaccepted memory regions to change on the forced
> second call to ExitBootServices.
> 
> The new acceptance logic is technology-agnostic and usable across TEE
> technologies, so this patch series introduces a Confidenial Compute DXE
> driver called CocoDxe.
> 
> To allow the OS loader to prevent the eager acceptance, and thus pass
> the before-mentioned "support condition", 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.
> 
> The MemoryAcceptance protocol is necessary for safe rollout of the
> unaccepted memory type, given the gradual update of guest OS kernels.
> OVMF cannot rely on the following implication
> 
>   (MemEncryptSevIsEnabled () || MemEncryptTdxIsEnabled ())
> 
>   implies
> 
>   unaccepted memory is supported by the guest
> 
> This implication does not hold for e.g., upstream Linux, and will not
> hold generally since both SEV-SNP and TDX define unaccepted memory
> support as optional rather than required.
> 
> 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 v8:
>  - First 3 patches removed since they were submitted separately.
>  - Later patches rebased on edk2/master and modified to work with the
>    current locations and namings of the unaccepted memory constants.
> Changes since v7:
>  - Rebased onto lazy accept v4 patch series, so memory accept protocol
>    has the EDKII prefix, and the unaccepted memory type has the BZ3937
>    prefix.
>  - Removed a bad #include to a header removed in v7.
>  - Renamed the protocol to BZ3987_MEMORY_ACCEPTANCE_PROTOCOL as
> per the
>    discussion on the buganizer issue.
>  - Uncrustify formatting
> 
> 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 (4):
>   OvmfPkg: Introduce CocoDxe driver
>   MdePkg: Introduce the MemoryAcceptance protocol
>   OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
>   OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> 
>  MdePkg/Include/Protocol/MemoryAcceptance.h |  40 +++++
>  MdePkg/MdePkg.dec                          |   3 +
>  OvmfPkg/AmdSev/AmdSevX64.dsc               |   1 +
>  OvmfPkg/AmdSev/AmdSevX64.fdf               |   1 +
>  OvmfPkg/CocoDxe/CocoDxe.c                  | 175 +++++++++++++++++++++
>  OvmfPkg/CocoDxe/CocoDxe.inf                |  46 ++++++
>  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 +
>  OvmfPkg/PlatformPei/AmdSev.c               |   5 +
>  13 files changed, 277 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
>  create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
>  create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> 
> --
> 2.39.0.314.g84b9a713c41-goog

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Gerd Hoffmann]

On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> Hi Dionna
> I think I understand your intention.
> I believe we need OS side and UEFI standard sign-off for this *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the consumer, right?
> If so, I suggest you maintain the work in a edk2-stage area for https://github.com/tianocore/edk2-staging.
> 
> EDKII main branch is for production. MdePkg can only include the API definition approved by UEFI standard.
> EDK2 staging is a place for POC / collaboration. That is why I think edk2 staging is more proper place for this feature.
> 
> Without OS and UEFI standard sign-off, I don't think this BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII main branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.

Ok.  Reading through the bug (comment 53) it looks like Intel's take on
this is that it will simply not be needed long-term.

How about adding it to OvmfPkg/Include/Protocol/MemoryAcceptance.h then?

It surely will be very useful short-term.  If it turns out that lazy
accept support indeed becomes a standard feature we might drop this
in 3-5 years.  Or promote it to MdePkg should that not be the case.

take care,
  Gerd

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Yao, Jiewen]

This is API between BIOS and OS.

I would like to see sign-off from OS side at least, before we can merge to EDKII main.



> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Friday, January 13, 2023 3:18 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James Bottomley
> <jejb@linux.ibm.com>; Tom Lendacky <Thomas.Lendacky@amd.com>; Aktas,
> Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> behavior
> 
> On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > Hi Dionna
> > I think I understand your intention.
> > I believe we need OS side and UEFI standard sign-off for this
> *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the consumer,
> right?
> > If so, I suggest you maintain the work in a edk2-stage area for
> https://github.com/tianocore/edk2-staging.
> >
> > EDKII main branch is for production. MdePkg can only include the API
> definition approved by UEFI standard.
> > EDK2 staging is a place for POC / collaboration. That is why I think edk2
> staging is more proper place for this feature.
> >
> > Without OS and UEFI standard sign-off, I don't think this
> BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII main
> branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> 
> Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> this is that it will simply not be needed long-term.
> 
> How about adding it to OvmfPkg/Include/Protocol/MemoryAcceptance.h
> then?
> 
> It surely will be very useful short-term.  If it turns out that lazy
> accept support indeed becomes a standard feature we might drop this
> in 3-5 years.  Or promote it to MdePkg should that not be the case.
> 
> take care,
>   Gerd

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Fri, 13 Jan 2023 at 08:33, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> This is API between BIOS and OS.
>
> I would like to see sign-off from OS side at least, before we can merge to EDKII main.
>

I have already indicated (and am happy to repeat here) that for Linux,
I am fine with this approach, if it amounts to locating a protocol and
invoking it to inform the firmware that it doesn't need to accept all
available memory.

Once we phase out the eager accept from the firmware entirely, we can
remove the protocol as well, and the OS loader will look for it and
simply not find it.


>
>
> > -----Original Message-----
> > From: Gerd Hoffmann <kraxel@redhat.com>
> > Sent: Friday, January 13, 2023 3:18 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> > <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James Bottomley
> > <jejb@linux.ibm.com>; Tom Lendacky <Thomas.Lendacky@amd.com>; Aktas,
> > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > behavior
> >
> > On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > > Hi Dionna
> > > I think I understand your intention.
> > > I believe we need OS side and UEFI standard sign-off for this
> > *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the consumer,
> > right?
> > > If so, I suggest you maintain the work in a edk2-stage area for
> > https://github.com/tianocore/edk2-staging.
> > >
> > > EDKII main branch is for production. MdePkg can only include the API
> > definition approved by UEFI standard.
> > > EDK2 staging is a place for POC / collaboration. That is why I think edk2
> > staging is more proper place for this feature.
> > >
> > > Without OS and UEFI standard sign-off, I don't think this
> > BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII main
> > branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> >
> > Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> > this is that it will simply not be needed long-term.
> >
> > How about adding it to OvmfPkg/Include/Protocol/MemoryAcceptance.h
> > then?
> >
> > It surely will be very useful short-term.  If it turns out that lazy
> > accept support indeed becomes a standard feature we might drop this
> > in 3-5 years.  Or promote it to MdePkg should that not be the case.
> >
> > take care,
> >   Gerd
>
>
>
> 
>
>

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Yao, Jiewen]

Sorry, that I did not say clearly.

When I say: "sign-off", I mean the Linux community and the maintainer have reached the consensus and agree to merge the patch for OS.

Would you please send to me the email from the maintainer, or the URL to record the conversation?

Thank you
Yao, Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, January 13, 2023 5:32 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Dionna Glaze
> <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> <Thomas.Lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>;
> Andrew Fish <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> behavior
> 
> On Fri, 13 Jan 2023 at 08:33, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > This is API between BIOS and OS.
> >
> > I would like to see sign-off from OS side at least, before we can merge to
> EDKII main.
> >
> 
> I have already indicated (and am happy to repeat here) that for Linux,
> I am fine with this approach, if it amounts to locating a protocol and
> invoking it to inform the firmware that it doesn't need to accept all
> available memory.
> 
> Once we phase out the eager accept from the firmware entirely, we can
> remove the protocol as well, and the OS loader will look for it and
> simply not find it.
> 
> 
> >
> >
> > > -----Original Message-----
> > > From: Gerd Hoffmann <kraxel@redhat.com>
> > > Sent: Friday, January 13, 2023 3:18 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> > > <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James Bottomley
> > > <jejb@linux.ibm.com>; Tom Lendacky <Thomas.Lendacky@amd.com>;
> Aktas,
> > > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> Kinney,
> > > Michael D <michael.d.kinney@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > > behavior
> > >
> > > On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > > > Hi Dionna
> > > > I think I understand your intention.
> > > > I believe we need OS side and UEFI standard sign-off for this
> > > *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the
> consumer,
> > > right?
> > > > If so, I suggest you maintain the work in a edk2-stage area for
> > > https://github.com/tianocore/edk2-staging.
> > > >
> > > > EDKII main branch is for production. MdePkg can only include the API
> > > definition approved by UEFI standard.
> > > > EDK2 staging is a place for POC / collaboration. That is why I think edk2
> > > staging is more proper place for this feature.
> > > >
> > > > Without OS and UEFI standard sign-off, I don't think this
> > > BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII
> main
> > > branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> > >
> > > Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> > > this is that it will simply not be needed long-term.
> > >
> > > How about adding it to OvmfPkg/Include/Protocol/MemoryAcceptance.h
> > > then?
> > >
> > > It surely will be very useful short-term.  If it turns out that lazy
> > > accept support indeed becomes a standard feature we might drop this
> > > in 3-5 years.  Or promote it to MdePkg should that not be the case.
> > >
> > > take care,
> > >   Gerd
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Fri, 13 Jan 2023 at 12:11, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Sorry, that I did not say clearly.
>
> When I say: "sign-off", I mean the Linux community and the maintainer have reached the consensus and agree to merge the patch for OS.
>
> Would you please send to me the email from the maintainer, or the URL to record the conversation?
>

I am the maintainer.

>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > Biesheuvel
> > Sent: Friday, January 13, 2023 5:32 PM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>; Dionna Glaze
> > <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> > Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> > <Thomas.Lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>;
> > Andrew Fish <afish@apple.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > behavior
> >
> > On Fri, 13 Jan 2023 at 08:33, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > >
> > > This is API between BIOS and OS.
> > >
> > > I would like to see sign-off from OS side at least, before we can merge to
> > EDKII main.
> > >
> >
> > I have already indicated (and am happy to repeat here) that for Linux,
> > I am fine with this approach, if it amounts to locating a protocol and
> > invoking it to inform the firmware that it doesn't need to accept all
> > available memory.
> >
> > Once we phase out the eager accept from the firmware entirely, we can
> > remove the protocol as well, and the OS loader will look for it and
> > simply not find it.
> >
> >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Gerd Hoffmann <kraxel@redhat.com>
> > > > Sent: Friday, January 13, 2023 3:18 PM
> > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> > > > <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James Bottomley
> > > > <jejb@linux.ibm.com>; Tom Lendacky <Thomas.Lendacky@amd.com>;
> > Aktas,
> > > > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> > Kinney,
> > > > Michael D <michael.d.kinney@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > > > behavior
> > > >
> > > > On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > > > > Hi Dionna
> > > > > I think I understand your intention.
> > > > > I believe we need OS side and UEFI standard sign-off for this
> > > > *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the
> > consumer,
> > > > right?
> > > > > If so, I suggest you maintain the work in a edk2-stage area for
> > > > https://github.com/tianocore/edk2-staging.
> > > > >
> > > > > EDKII main branch is for production. MdePkg can only include the API
> > > > definition approved by UEFI standard.
> > > > > EDK2 staging is a place for POC / collaboration. That is why I think edk2
> > > > staging is more proper place for this feature.
> > > > >
> > > > > Without OS and UEFI standard sign-off, I don't think this
> > > > BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to EDKII
> > main
> > > > branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> > > >
> > > > Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> > > > this is that it will simply not be needed long-term.
> > > >
> > > > How about adding it to OvmfPkg/Include/Protocol/MemoryAcceptance.h
> > > > then?
> > > >
> > > > It surely will be very useful short-term.  If it turns out that lazy
> > > > accept support indeed becomes a standard feature we might drop this
> > > > in 3-5 years.  Or promote it to MdePkg should that not be the case.
> > > >
> > > > take care,
> > > >   Gerd
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> > 
> >
>

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Yao, Jiewen]

Would you please send me the URL for the Linux patch?

I would check with other OS people as well.

Thank you
Yao, Jiewen

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, January 13, 2023 7:24 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; Gerd Hoffmann <kraxel@redhat.com>; Dionna
> Glaze <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> <Thomas.Lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>;
> Andrew Fish <afish@apple.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> behavior
> 
> On Fri, 13 Jan 2023 at 12:11, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > Sorry, that I did not say clearly.
> >
> > When I say: "sign-off", I mean the Linux community and the maintainer
> have reached the consensus and agree to merge the patch for OS.
> >
> > Would you please send to me the email from the maintainer, or the URL to
> record the conversation?
> >
> 
> I am the maintainer.
> 
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > > Biesheuvel
> > > Sent: Friday, January 13, 2023 5:32 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>; Dionna Glaze
> > > <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> > > Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> > > <Thomas.Lendacky@amd.com>; Aktas, Erdem
> <erdemaktas@google.com>;
> > > Andrew Fish <afish@apple.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > > behavior
> > >
> > > On Fri, 13 Jan 2023 at 08:33, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > > >
> > > > This is API between BIOS and OS.
> > > >
> > > > I would like to see sign-off from OS side at least, before we can merge
> to
> > > EDKII main.
> > > >
> > >
> > > I have already indicated (and am happy to repeat here) that for Linux,
> > > I am fine with this approach, if it amounts to locating a protocol and
> > > invoking it to inform the firmware that it doesn't need to accept all
> > > available memory.
> > >
> > > Once we phase out the eager accept from the firmware entirely, we can
> > > remove the protocol as well, and the OS loader will look for it and
> > > simply not find it.
> > >
> > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Gerd Hoffmann <kraxel@redhat.com>
> > > > > Sent: Friday, January 13, 2023 3:18 PM
> > > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > > Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> > > > > <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James
> Bottomley
> > > > > <jejb@linux.ibm.com>; Tom Lendacky
> <Thomas.Lendacky@amd.com>;
> > > Aktas,
> > > > > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> > > Kinney,
> > > > > Michael D <michael.d.kinney@intel.com>
> > > > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted
> memory
> > > > > behavior
> > > > >
> > > > > On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > > > > > Hi Dionna
> > > > > > I think I understand your intention.
> > > > > > I believe we need OS side and UEFI standard sign-off for this
> > > > > *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the
> > > consumer,
> > > > > right?
> > > > > > If so, I suggest you maintain the work in a edk2-stage area for
> > > > > https://github.com/tianocore/edk2-staging.
> > > > > >
> > > > > > EDKII main branch is for production. MdePkg can only include the
> API
> > > > > definition approved by UEFI standard.
> > > > > > EDK2 staging is a place for POC / collaboration. That is why I think
> edk2
> > > > > staging is more proper place for this feature.
> > > > > >
> > > > > > Without OS and UEFI standard sign-off, I don't think this
> > > > > BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to
> EDKII
> > > main
> > > > > branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> > > > >
> > > > > Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> > > > > this is that it will simply not be needed long-term.
> > > > >
> > > > > How about adding it to
> OvmfPkg/Include/Protocol/MemoryAcceptance.h
> > > > > then?
> > > > >
> > > > > It surely will be very useful short-term.  If it turns out that lazy
> > > > > accept support indeed becomes a standard feature we might drop
> this
> > > > > in 3-5 years.  Or promote it to MdePkg should that not be the case.
> > > > >
> > > > > take care,
> > > > >   Gerd
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> >
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Fri, 13 Jan 2023 at 12:45, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Would you please send me the URL for the Linux patch?
>
> I would check with other OS people as well.
>

OK.

I don't remember whether or not a patch was sent to the linux-efi list already.

Dionna: can you please cc jiewen and edk2-devel when you (re)send it? Thanks.


> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > Biesheuvel
> > Sent: Friday, January 13, 2023 7:24 PM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: devel@edk2.groups.io; Gerd Hoffmann <kraxel@redhat.com>; Dionna
> > Glaze <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> > Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> > <Thomas.Lendacky@amd.com>; Aktas, Erdem <erdemaktas@google.com>;
> > Andrew Fish <afish@apple.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > behavior
> >
> > On Fri, 13 Jan 2023 at 12:11, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > >
> > > Sorry, that I did not say clearly.
> > >
> > > When I say: "sign-off", I mean the Linux community and the maintainer
> > have reached the consensus and agree to merge the patch for OS.
> > >
> > > Would you please send to me the email from the maintainer, or the URL to
> > record the conversation?
> > >
> >
> > I am the maintainer.
> >
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > > > Biesheuvel
> > > > Sent: Friday, January 13, 2023 5:32 PM
> > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: Gerd Hoffmann <kraxel@redhat.com>; Dionna Glaze
> > > > <dionnaglaze@google.com>; Xu, Min M <min.m.xu@intel.com>; James
> > > > Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> > > > <Thomas.Lendacky@amd.com>; Aktas, Erdem
> > <erdemaktas@google.com>;
> > > > Andrew Fish <afish@apple.com>; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory
> > > > behavior
> > > >
> > > > On Fri, 13 Jan 2023 at 08:33, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > > > >
> > > > > This is API between BIOS and OS.
> > > > >
> > > > > I would like to see sign-off from OS side at least, before we can merge
> > to
> > > > EDKII main.
> > > > >
> > > >
> > > > I have already indicated (and am happy to repeat here) that for Linux,
> > > > I am fine with this approach, if it amounts to locating a protocol and
> > > > invoking it to inform the firmware that it doesn't need to accept all
> > > > available memory.
> > > >
> > > > Once we phase out the eager accept from the firmware entirely, we can
> > > > remove the protocol as well, and the OS loader will look for it and
> > > > simply not find it.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Gerd Hoffmann <kraxel@redhat.com>
> > > > > > Sent: Friday, January 13, 2023 3:18 PM
> > > > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
> > > > > > Cc: Dionna Glaze <dionnaglaze@google.com>; Ard Biescheuvel
> > > > > > <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>; James
> > Bottomley
> > > > > > <jejb@linux.ibm.com>; Tom Lendacky
> > <Thomas.Lendacky@amd.com>;
> > > > Aktas,
> > > > > > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> > > > Kinney,
> > > > > > Michael D <michael.d.kinney@intel.com>
> > > > > > Subject: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted
> > memory
> > > > > > behavior
> > > > > >
> > > > > > On Fri, Jan 13, 2023 at 03:46:34AM +0000, Yao, Jiewen wrote:
> > > > > > > Hi Dionna
> > > > > > > I think I understand your intention.
> > > > > > > I believe we need OS side and UEFI standard sign-off for this
> > > > > > *BZ3987_MEMORY_ACCEPTANCE_PROTOCOL*, because OS is the
> > > > consumer,
> > > > > > right?
> > > > > > > If so, I suggest you maintain the work in a edk2-stage area for
> > > > > > https://github.com/tianocore/edk2-staging.
> > > > > > >
> > > > > > > EDKII main branch is for production. MdePkg can only include the
> > API
> > > > > > definition approved by UEFI standard.
> > > > > > > EDK2 staging is a place for POC / collaboration. That is why I think
> > edk2
> > > > > > staging is more proper place for this feature.
> > > > > > >
> > > > > > > Without OS and UEFI standard sign-off, I don't think this
> > > > > > BZ3987_MEMORY_ACCEPTANCE_PROTOCOL can be integrated to
> > EDKII
> > > > main
> > > > > > branch, especially in MdePkg/Include/Protocol/MemoryAcceptance.h.
> > > > > >
> > > > > > Ok.  Reading through the bug (comment 53) it looks like Intel's take on
> > > > > > this is that it will simply not be needed long-term.
> > > > > >
> > > > > > How about adding it to
> > OvmfPkg/Include/Protocol/MemoryAcceptance.h
> > > > > > then?
> > > > > >
> > > > > > It surely will be very useful short-term.  If it turns out that lazy
> > > > > > accept support indeed becomes a standard feature we might drop
> > this
> > > > > > in 3-5 years.  Or promote it to MdePkg should that not be the case.
> > > > > >
> > > > > > take care,
> > > > > >   Gerd
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
> >
> > 
> >
>

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[dave.hansen]

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

Hi Folks,

My hope (from the x86 side at least) was that all functional Linux TDX guests will have memory acceptance support.  We don't have fully-functional guest code yet and assuming that Linux memory acceptance support will be in place before we get there.

Basically, I was hoping that Linux could get away without needing any kind of negotiation with the firmware.

(btw, I'm replying in the web interface.  Apologies for any odd formatting)

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

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Dionna Glaze]

Thanks for your perspective, Dave. From what I understand,
distributions lag behind, user kernel configurations can be varied,
and Kirill's patch set is still untested with regards to memory
latency of workloads. We may yet see folks opt for a slow boot for
better latency. This protocol is for safety purposes, since "hope is
not a strategy" as is commonly said at Google.

On Fri, Jan 13, 2023 at 9:01 AM <dave.hansen@linux.intel.com> wrote:
>
> Hi Folks,
>
> My hope (from the x86 side at least) was that all functional Linux TDX guests will have memory acceptance support.  We don't have fully-functional guest code yet and assuming that Linux memory acceptance support will be in place before we get there.
>
> Basically, I was hoping that Linux could get away without needing any kind of negotiation with the firmware.
>
> (btw, I'm replying in the web interface.  Apologies for any odd formatting)
> 



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

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Dave Hansen]

On 1/13/23 09:06, Dionna Amalie Glaze wrote:
> Thanks for your perspective, Dave. From what I understand,
> distributions lag behind, user kernel configurations can be varied,
> and Kirill's patch set is still untested with regards to memory
> latency of workloads. We may yet see folks opt for a slow boot for
> better latency. This protocol is for safety purposes, since "hope is
> not a strategy" as is commonly said at Google.

Hold on a sec though.

There are two basic strategies here:

 1. Boot to userspace as fast as possible without accepting memory.
    This runs apps more sooner after boot, but exposes those apps to the
    latency of memory acceptance.

 2. Accept all memory before running userspace.  This makes the boot
    slower but means that apps never see memory acceptance latency.

Kirill's _initial_ patch does #1.  If anyone desperately wants #2, they
have mechanisms available to make a kernel with only #1 approximate #2.
A user on that kernel could allocate and memset()ing a bunch of memory.
Or, they could have a firmware stub accept the memory before booting the
real kernel.

It also doesn't rule out having a runtime knob or a boot parameter
implement #2.  It's not a lot of code, but it involves new ABI.

However, *NONE* of this points me in the direction of saying that we
should have an OS/firmware protocol to negotiate whether the firmware or
OS does page acceptance other than the existing UEFI memory map bit.

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Dionna Glaze]

> Kirill's _initial_ patch does #1.  If anyone desperately wants #2, they
> have mechanisms available to make a kernel with only #1 approximate #2.
> A user on that kernel could allocate and memset()ing a bunch of memory.
> Or, they could have a firmware stub accept the memory before booting the
> real kernel.
>
> It also doesn't rule out having a runtime knob or a boot parameter
> implement #2.  It's not a lot of code, but it involves new ABI.
>

The new ABI is the safety problem. Without the new code, you have
firmware that makes all but 3 GiB of memory unusable because it's
classified as an unknown type.

> However, *NONE* of this points me in the direction of saying that we
> should have an OS/firmware protocol to negotiate whether the firmware or
> OS does page acceptance other than the existing UEFI memory map bit.

We know of distributions that are going to release SEV-SNP support
without unaccepted memory support, and in so doing, tie the firmware's
hands in trying to maintain safe behavior through a required default
behavior of accepting all memory without explicit information from the
OS in the form of this protocol. TDX support may also get released
this way due to unexpected requirements from the linux community that
push back Kirill's patches. They still haven't been thoroughly
reviewed by a memory system expert, IIRC.

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

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Dave Hansen]

On 1/13/23 10:23, Dionna Glaze via groups.io wrote:
>> However, *NONE* of this points me in the direction of saying that we
>> should have an OS/firmware protocol to negotiate whether the firmware or
>> OS does page acceptance other than the existing UEFI memory map bit.
> We know of distributions that are going to release SEV-SNP support
> without unaccepted memory support,

Well, I guess that's a bit of a different problem.

I'd love to hear from the distros what they're planning on carrying
outside of mainline and what their plans are for the kernel side of this
series.

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Gerd Hoffmann]

On Fri, Jan 13, 2023 at 10:34:15AM -0800, Dave Hansen wrote:
> On 1/13/23 10:23, Dionna Glaze via groups.io wrote:
> >> However, *NONE* of this points me in the direction of saying that we
> >> should have an OS/firmware protocol to negotiate whether the firmware or
> >> OS does page acceptance other than the existing UEFI memory map bit.
> > We know of distributions that are going to release SEV-SNP support
> > without unaccepted memory support,
> 
> Well, I guess that's a bit of a different problem.
> 
> I'd love to hear from the distros what they're planning on carrying
> outside of mainline and what their plans are for the kernel side of this
> series.

Fedora has near zero additional patches, so it pretty much depends on
how mainline merges stuff.  If SEV-SNP or TDX or both will land in an
upstream release before support for unaccepted memory lands too you'll
see that in Fedora kernels, and I guess likewise in most non-enterprise
distros.

RHEL/CentOS typically requires mainline acceptance too for backports, so
it likewise depends on upstream merging, and additionally rhel release
planning comes into play.  In case unaccepted memory support lands later
than TDX it could (depending on timing) very well be that the choices
are to either backport TDX without unaccepted memory support, or move
both TDX support and unaccepted memory support to a later release.

take care,
  Gerd

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Lendacky, Thomas]

On 1/16/23 04:28, Gerd Hoffmann via groups.io wrote:
> On Fri, Jan 13, 2023 at 10:34:15AM -0800, Dave Hansen wrote:
>> On 1/13/23 10:23, Dionna Glaze via groups.io wrote:
>>>> However, *NONE* of this points me in the direction of saying that we
>>>> should have an OS/firmware protocol to negotiate whether the firmware or
>>>> OS does page acceptance other than the existing UEFI memory map bit.
>>> We know of distributions that are going to release SEV-SNP support
>>> without unaccepted memory support,
>>
>> Well, I guess that's a bit of a different problem.
>>
>> I'd love to hear from the distros what they're planning on carrying
>> outside of mainline and what their plans are for the kernel side of this
>> series.
> 
> Fedora has near zero additional patches, so it pretty much depends on
> how mainline merges stuff.  If SEV-SNP or TDX or both will land in an
> upstream release before support for unaccepted memory lands too you'll

Sorry, just saw this...

SEV-SNP guest support has already landed upstream and is part of the 5.19 
kernel. So without this interface, anyone using a newer OVMF with a 5.19 
to whatever kernel just before unaccepted memory finally gets pulled into, 
would have issues.

Thanks,
Tom

> see that in Fedora kernels, and I guess likewise in most non-enterprise
> distros.
> 
> RHEL/CentOS typically requires mainline acceptance too for backports, so
> it likewise depends on upstream merging, and additionally rhel release
> planning comes into play.  In case unaccepted memory support lands later
> than TDX it could (depending on timing) very well be that the choices
> are to either backport TDX without unaccepted memory support, or move
> both TDX support and unaccepted memory support to a later release.
> 
> take care,
>    Gerd
> 
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Dave Hansen]

On 1/24/23 14:42, Tom Lendacky wrote:
>> Fedora has near zero additional patches, so it pretty much depends on
>> how mainline merges stuff.  If SEV-SNP or TDX or both will land in an
>> upstream release before support for unaccepted memory lands too you'll
> 
> Sorry, just saw this...
> 
> SEV-SNP guest support has already landed upstream and is part of the
> 5.19 kernel. So without this interface, anyone using a newer OVMF with a
> 5.19 to whatever kernel just before unaccepted memory finally gets
> pulled into, would have issues.

Also, just to reiterate: my whinging about this has been purely
TDX-specific.  It's a bit of a shame that things happened in this order,
but SEV-SNP does seem to have a legitimate need here, separate from TDX.

I'll be quiet now. :)

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Tue, 24 Jan 2023 at 23:42, Lendacky, Thomas via groups.io
<thomas.lendacky=amd.com@groups.io> wrote:
>
> On 1/16/23 04:28, Gerd Hoffmann via groups.io wrote:
> > On Fri, Jan 13, 2023 at 10:34:15AM -0800, Dave Hansen wrote:
> >> On 1/13/23 10:23, Dionna Glaze via groups.io wrote:
> >>>> However, *NONE* of this points me in the direction of saying that we
> >>>> should have an OS/firmware protocol to negotiate whether the firmware or
> >>>> OS does page acceptance other than the existing UEFI memory map bit.
> >>> We know of distributions that are going to release SEV-SNP support
> >>> without unaccepted memory support,
> >>
> >> Well, I guess that's a bit of a different problem.
> >>
> >> I'd love to hear from the distros what they're planning on carrying
> >> outside of mainline and what their plans are for the kernel side of this
> >> series.
> >
> > Fedora has near zero additional patches, so it pretty much depends on
> > how mainline merges stuff.  If SEV-SNP or TDX or both will land in an
> > upstream release before support for unaccepted memory lands too you'll
>
> Sorry, just saw this...
>
> SEV-SNP guest support has already landed upstream and is part of the 5.19
> kernel. So without this interface, anyone using a newer OVMF with a 5.19
> to whatever kernel just before unaccepted memory finally gets pulled into,
> would have issues.
>

Exactly. And my Fedora kernel has those bits enabled by default.

So I suppose the way forward here is to expose this protocol only on
OVMF builds that target SEV-SNP, instead of introducing it as a
generic CoCo feature.

On the Linux side, we can make its boot time invocation dependent on
CONFIG_AMD_MEM_ENCRYPT as well as CONFIG_UNACCEPTED_MEMORY.

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Gerd Hoffmann]

On Wed, Jan 25, 2023 at 10:01:47AM +0100, Ard Biesheuvel wrote:
 
> Exactly. And my Fedora kernel has those bits enabled by default.
> 
> So I suppose the way forward here is to expose this protocol only on
> OVMF builds that target SEV-SNP, instead of introducing it as a
> generic CoCo feature.

OVMF builds already adapt at runtime, so this needs to be a runtime
check too.  But IIRC the module already checks whenever SNP-SEV or TDX
is active before installing, so we only need to tweak that check to drop
the TDX check.

take care,
  Gerd

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Wed, 25 Jan 2023 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Jan 25, 2023 at 10:01:47AM +0100, Ard Biesheuvel wrote:
>
> > Exactly. And my Fedora kernel has those bits enabled by default.
> >
> > So I suppose the way forward here is to expose this protocol only on
> > OVMF builds that target SEV-SNP, instead of introducing it as a
> > generic CoCo feature.
>
> OVMF builds already adapt at runtime, so this needs to be a runtime
> check too.  But IIRC the module already checks whenever SNP-SEV or TDX
> is active before installing, so we only need to tweak that check to drop
> the TDX check.
>

Sure. But Dionna's series introduces a new CocoDxe driver that is
intended to carry shared logic, but we should probably add this stuff
to AmdSevDxe instead.

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Gerd Hoffmann]

On Wed, Jan 25, 2023 at 12:44:13PM +0100, Ard Biesheuvel wrote:
> On Wed, 25 Jan 2023 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Wed, Jan 25, 2023 at 10:01:47AM +0100, Ard Biesheuvel wrote:
> >
> > > Exactly. And my Fedora kernel has those bits enabled by default.
> > >
> > > So I suppose the way forward here is to expose this protocol only on
> > > OVMF builds that target SEV-SNP, instead of introducing it as a
> > > generic CoCo feature.
> >
> > OVMF builds already adapt at runtime, so this needs to be a runtime
> > check too.  But IIRC the module already checks whenever SNP-SEV or TDX
> > is active before installing, so we only need to tweak that check to drop
> > the TDX check.
> >
> 
> Sure. But Dionna's series introduces a new CocoDxe driver that is
> intended to carry shared logic, but we should probably add this stuff
> to AmdSevDxe instead.

OvmfPkgX64.dsc can run in SEV/TDX modes too, so it is needed there as
well.  We can probably leave it out from IntelTdx (assuming Intel's
upstream merge plans work out as expected and tdx-guest + unaccepted
memory actually land in the same kernel release).

take care,
  Gerd

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Ard Biesheuvel]

On Wed, 25 Jan 2023 at 13:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Jan 25, 2023 at 12:44:13PM +0100, Ard Biesheuvel wrote:
> > On Wed, 25 Jan 2023 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > On Wed, Jan 25, 2023 at 10:01:47AM +0100, Ard Biesheuvel wrote:
> > >
> > > > Exactly. And my Fedora kernel has those bits enabled by default.
> > > >
> > > > So I suppose the way forward here is to expose this protocol only on
> > > > OVMF builds that target SEV-SNP, instead of introducing it as a
> > > > generic CoCo feature.
> > >
> > > OVMF builds already adapt at runtime, so this needs to be a runtime
> > > check too.  But IIRC the module already checks whenever SNP-SEV or TDX
> > > is active before installing, so we only need to tweak that check to drop
> > > the TDX check.
> > >
> >
> > Sure. But Dionna's series introduces a new CocoDxe driver that is
> > intended to carry shared logic, but we should probably add this stuff
> > to AmdSevDxe instead.
>
> OvmfPkgX64.dsc can run in SEV/TDX modes too, so it is needed there as
> well.  We can probably leave it out from IntelTdx (assuming Intel's
> upstream merge plans work out as expected and tdx-guest + unaccepted
> memory actually land in the same kernel release).
>

OvmfPkgX64.dsc already incorporates AmdSevDxe, as do a few other
platforms, so adding it there is still fine afaict.

Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

[Yao, Jiewen]

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

hello
If this protocol is SEV specific, please also add prefix SEV_ or AMD_ for the protocol name.


________________________________
发件人: Ard Biesheuvel <ardb@kernel.org>
发送时间: Wednesday, January 25, 2023 10:52:42 PM
收件人: Gerd Hoffmann <kraxel@redhat.com>
抄送: devel@edk2.groups.io <devel@edk2.groups.io>; thomas.lendacky@amd.com <thomas.lendacky@amd.com>; Hansen, Dave <dave.hansen@intel.com>; dionnaglaze@google.com <dionnaglaze@google.com>; dave.hansen@linux.intel.com <dave.hansen@linux.intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Shutemov, Kirill <kirill.shutemov@intel.com>
主题: Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior

On Wed, 25 Jan 2023 at 13:10, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Wed, Jan 25, 2023 at 12:44:13PM +0100, Ard Biesheuvel wrote:
> > On Wed, 25 Jan 2023 at 10:18, Gerd Hoffmann <kraxel@redhat.com> wrote:
> > >
> > > On Wed, Jan 25, 2023 at 10:01:47AM +0100, Ard Biesheuvel wrote:
> > >
> > > > Exactly. And my Fedora kernel has those bits enabled by default.
> > > >
> > > > So I suppose the way forward here is to expose this protocol only on
> > > > OVMF builds that target SEV-SNP, instead of introducing it as a
> > > > generic CoCo feature.
> > >
> > > OVMF builds already adapt at runtime, so this needs to be a runtime
> > > check too.  But IIRC the module already checks whenever SNP-SEV or TDX
> > > is active before installing, so we only need to tweak that check to drop
> > > the TDX check.
> > >
> >
> > Sure. But Dionna's series introduces a new CocoDxe driver that is
> > intended to carry shared logic, but we should probably add this stuff
> > to AmdSevDxe instead.
>
> OvmfPkgX64.dsc can run in SEV/TDX modes too, so it is needed there as
> well.  We can probably leave it out from IntelTdx (assuming Intel's
> upstream merge plans work out as expected and tdx-guest + unaccepted
> memory actually land in the same kernel release).
>

OvmfPkgX64.dsc already incorporates AmdSevDxe, as do a few other
platforms, so adding it there is still fine afaict.

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