* [PATCH v9 1/4] OvmfPkg: Introduce CocoDxe driver
2023-01-13 0:14 [PATCH v9 0/4] Add safe unaccepted memory behavior Dionna Glaze
@ 2023-01-13 0:14 ` Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 2/4] MdePkg: Introduce the MemoryAcceptance protocol Dionna Glaze
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 0:14 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Gerd Hoffmann, James Bottomley, Jiewen Yao,
Tom Lendacky, Ard Biesheuvel, Min M. Xu, Andrew Fish,
Michael D. Kinney
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v9 2/4] MdePkg: Introduce the MemoryAcceptance protocol
2023-01-13 0:14 [PATCH v9 0/4] Add safe unaccepted memory behavior Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 1/4] OvmfPkg: Introduce CocoDxe driver Dionna Glaze
@ 2023-01-13 0:14 ` Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe Dionna Glaze
` (2 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 0:14 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Gerd Hoffmann, James Bottomley, Jiewen Yao,
Tom Lendacky, Ard Biesheuvel, Min M. Xu, Andrew Fish,
Michael D. Kinney
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v9 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
2023-01-13 0:14 [PATCH v9 0/4] Add safe unaccepted memory behavior Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 1/4] OvmfPkg: Introduce CocoDxe driver Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 2/4] MdePkg: Introduce the MemoryAcceptance protocol Dionna Glaze
@ 2023-01-13 0:14 ` Dionna Glaze
2023-01-13 0:14 ` [PATCH v9 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted Dionna Glaze
2023-01-13 3:46 ` [PATCH v9 0/4] Add safe unaccepted memory behavior Yao, Jiewen
4 siblings, 0 replies; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 0:14 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Gerd Hoffmann, James Bottomley, Jiewen Yao,
Tom Lendacky, Ard Biesheuvel, Min M. Xu, Andrew Fish,
Michael D. Kinney
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v9 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
2023-01-13 0:14 [PATCH v9 0/4] Add safe unaccepted memory behavior Dionna Glaze
` (2 preceding siblings ...)
2023-01-13 0:14 ` [PATCH v9 3/4] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe Dionna Glaze
@ 2023-01-13 0:14 ` Dionna Glaze
2023-01-13 3:46 ` [PATCH v9 0/4] Add safe unaccepted memory behavior Yao, Jiewen
4 siblings, 0 replies; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 0:14 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Ard Biescheuvel, Min M. Xu, Gerd Hoffmann,
James Bottomley, Tom Lendacky, Jiewen Yao, Erdem Aktas
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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 0:14 [PATCH v9 0/4] Add safe unaccepted memory behavior Dionna Glaze
` (3 preceding siblings ...)
2023-01-13 0:14 ` [PATCH v9 4/4] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted Dionna Glaze
@ 2023-01-13 3:46 ` Yao, Jiewen
2023-01-13 7:18 ` [edk2-devel] " Gerd Hoffmann
4 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2023-01-13 3:46 UTC (permalink / raw)
To: Dionna Glaze, devel@edk2.groups.io
Cc: Ard Biescheuvel, Xu, Min M, Gerd Hoffmann, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D
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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 3:46 ` [PATCH v9 0/4] Add safe unaccepted memory behavior Yao, Jiewen
@ 2023-01-13 7:18 ` Gerd Hoffmann
2023-01-13 7:32 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Gerd Hoffmann @ 2023-01-13 7:18 UTC (permalink / raw)
To: devel, jiewen.yao
Cc: Dionna Glaze, Ard Biescheuvel, Xu, Min M, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 7:18 ` [edk2-devel] " Gerd Hoffmann
@ 2023-01-13 7:32 ` Yao, Jiewen
2023-01-13 9:32 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2023-01-13 7:32 UTC (permalink / raw)
To: Gerd Hoffmann, devel@edk2.groups.io
Cc: Dionna Glaze, Ard Biescheuvel, Xu, Min M, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D,
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 7:32 ` Yao, Jiewen
@ 2023-01-13 9:32 ` Ard Biesheuvel
2023-01-13 11:11 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-13 9:32 UTC (permalink / raw)
To: devel, jiewen.yao
Cc: Gerd Hoffmann, Dionna Glaze, Xu, Min M, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D
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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 9:32 ` Ard Biesheuvel
@ 2023-01-13 11:11 ` Yao, Jiewen
2023-01-13 11:24 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Yao, Jiewen @ 2023-01-13 11:11 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Gerd Hoffmann, Dionna Glaze, Xu, Min M, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D
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
> >
> >
> >
> >
> >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 11:11 ` Yao, Jiewen
@ 2023-01-13 11:24 ` Ard Biesheuvel
2023-01-13 11:44 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-13 11:24 UTC (permalink / raw)
To: Yao, Jiewen
Cc: devel@edk2.groups.io, Gerd Hoffmann, Dionna Glaze, Xu, Min M,
James Bottomley, Tom Lendacky, Aktas, Erdem, Andrew Fish,
Kinney, Michael D
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
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 11:24 ` Ard Biesheuvel
@ 2023-01-13 11:44 ` Yao, Jiewen
2023-01-13 12:00 ` Ard Biesheuvel
2023-01-13 16:00 ` dave.hansen
0 siblings, 2 replies; 27+ messages in thread
From: Yao, Jiewen @ 2023-01-13 11:44 UTC (permalink / raw)
To: devel@edk2.groups.io, ardb@kernel.org
Cc: Gerd Hoffmann, Dionna Glaze, Xu, Min M, James Bottomley,
Tom Lendacky, Aktas, Erdem, Andrew Fish, Kinney, Michael D
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
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> > >
> >
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 11:44 ` Yao, Jiewen
@ 2023-01-13 12:00 ` Ard Biesheuvel
2023-01-13 16:00 ` dave.hansen
1 sibling, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-13 12:00 UTC (permalink / raw)
To: Yao, Jiewen
Cc: devel@edk2.groups.io, Gerd Hoffmann, Dionna Glaze, Xu, Min M,
James Bottomley, Tom Lendacky, Aktas, Erdem, Andrew Fish,
Kinney, Michael D
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
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
> >
> >
> >
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 11:44 ` Yao, Jiewen
2023-01-13 12:00 ` Ard Biesheuvel
@ 2023-01-13 16:00 ` dave.hansen
2023-01-13 17:06 ` Dionna Glaze
1 sibling, 1 reply; 27+ messages in thread
From: dave.hansen @ 2023-01-13 16:00 UTC (permalink / raw)
To: Yao, Jiewen, devel
[-- 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 --]
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 16:00 ` dave.hansen
@ 2023-01-13 17:06 ` Dionna Glaze
2023-01-13 17:57 ` Dave Hansen
0 siblings, 1 reply; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 17:06 UTC (permalink / raw)
To: devel, dave.hansen; +Cc: Jiewen
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)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 17:06 ` Dionna Glaze
@ 2023-01-13 17:57 ` Dave Hansen
2023-01-13 18:23 ` Dionna Glaze
0 siblings, 1 reply; 27+ messages in thread
From: Dave Hansen @ 2023-01-13 17:57 UTC (permalink / raw)
To: Dionna Amalie Glaze, devel, dave.hansen; +Cc: Jiewen, Shutemov, Kirill
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.
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 17:57 ` Dave Hansen
@ 2023-01-13 18:23 ` Dionna Glaze
2023-01-13 18:34 ` Dave Hansen
0 siblings, 1 reply; 27+ messages in thread
From: Dionna Glaze @ 2023-01-13 18:23 UTC (permalink / raw)
To: Dave Hansen; +Cc: devel, dave.hansen, Jiewen, Shutemov, Kirill
> 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)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 18:23 ` Dionna Glaze
@ 2023-01-13 18:34 ` Dave Hansen
2023-01-16 10:28 ` Gerd Hoffmann
0 siblings, 1 reply; 27+ messages in thread
From: Dave Hansen @ 2023-01-13 18:34 UTC (permalink / raw)
To: devel, dionnaglaze; +Cc: dave.hansen, Jiewen, Shutemov, Kirill
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-13 18:34 ` Dave Hansen
@ 2023-01-16 10:28 ` Gerd Hoffmann
2023-01-24 22:42 ` Lendacky, Thomas
0 siblings, 1 reply; 27+ messages in thread
From: Gerd Hoffmann @ 2023-01-16 10:28 UTC (permalink / raw)
To: devel, dave.hansen; +Cc: dionnaglaze, dave.hansen, Jiewen, Shutemov, Kirill
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-16 10:28 ` Gerd Hoffmann
@ 2023-01-24 22:42 ` Lendacky, Thomas
2023-01-24 22:46 ` Dave Hansen
2023-01-25 9:01 ` Ard Biesheuvel
0 siblings, 2 replies; 27+ messages in thread
From: Lendacky, Thomas @ 2023-01-24 22:42 UTC (permalink / raw)
To: devel, kraxel, dave.hansen
Cc: dionnaglaze, dave.hansen, Jiewen, Shutemov, Kirill
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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-24 22:42 ` Lendacky, Thomas
@ 2023-01-24 22:46 ` Dave Hansen
2023-01-25 9:01 ` Ard Biesheuvel
1 sibling, 0 replies; 27+ messages in thread
From: Dave Hansen @ 2023-01-24 22:46 UTC (permalink / raw)
To: Tom Lendacky, devel, kraxel
Cc: dionnaglaze, dave.hansen, Jiewen, Shutemov, Kirill
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. :)
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-24 22:42 ` Lendacky, Thomas
2023-01-24 22:46 ` Dave Hansen
@ 2023-01-25 9:01 ` Ard Biesheuvel
2023-01-25 9:18 ` Gerd Hoffmann
1 sibling, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-25 9:01 UTC (permalink / raw)
To: devel, thomas.lendacky
Cc: kraxel, dave.hansen, dionnaglaze, dave.hansen, Jiewen,
Shutemov, Kirill
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-25 9:01 ` Ard Biesheuvel
@ 2023-01-25 9:18 ` Gerd Hoffmann
2023-01-25 11:44 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Gerd Hoffmann @ 2023-01-25 9:18 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, thomas.lendacky, dave.hansen, dionnaglaze, dave.hansen,
Jiewen, Shutemov, Kirill
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-25 9:18 ` Gerd Hoffmann
@ 2023-01-25 11:44 ` Ard Biesheuvel
2023-01-25 12:10 ` Gerd Hoffmann
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-25 11:44 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: devel, thomas.lendacky, dave.hansen, dionnaglaze, dave.hansen,
Jiewen, Shutemov, Kirill
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-25 11:44 ` Ard Biesheuvel
@ 2023-01-25 12:10 ` Gerd Hoffmann
2023-01-25 14:52 ` Ard Biesheuvel
0 siblings, 1 reply; 27+ messages in thread
From: Gerd Hoffmann @ 2023-01-25 12:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: devel, thomas.lendacky, dave.hansen, dionnaglaze, dave.hansen,
Jiewen, Shutemov, Kirill
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-25 12:10 ` Gerd Hoffmann
@ 2023-01-25 14:52 ` Ard Biesheuvel
2023-01-25 16:56 ` Yao, Jiewen
0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2023-01-25 14:52 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: devel, thomas.lendacky, dave.hansen, dionnaglaze, dave.hansen,
Jiewen, Shutemov, Kirill
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.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v9 0/4] Add safe unaccepted memory behavior
2023-01-25 14:52 ` Ard Biesheuvel
@ 2023-01-25 16:56 ` Yao, Jiewen
0 siblings, 0 replies; 27+ messages in thread
From: Yao, Jiewen @ 2023-01-25 16:56 UTC (permalink / raw)
To: Ard Biesheuvel, Gerd Hoffmann
Cc: devel@edk2.groups.io, thomas.lendacky@amd.com, Hansen, Dave,
dionnaglaze@google.com, dave.hansen@linux.intel.com,
Shutemov, Kirill
[-- 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 --]
^ permalink raw reply [flat|nested] 27+ messages in thread