* [PATCH v8 0/7] Add safe unaccepted memory behavior
@ 2022-10-24 20:41 Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Ard Biescheuvel, Min M. Xu, Gerd Hoffmann,
James Bottomley, Tom Lendacky, Jiewen Yao, Erdem Aktas,
Andrew Fish, Michael D. Kinney
These seven patches build on the lazy-accept patch series
"Introduce Lazy-accept for Tdx guest"
by adding SEV-SNP support for the MemoryAccept protocol, and
importantly making eager memory acceptance the default behavior.
We implement a standardized event group from UEFI v2.9,
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly
the right invocation point for eagerly accepting memory if eager
acceptance has not been disabled.
To make use of this event group, we add a new driver that is meant to
carry behavior that is needed for all confidential compute technologies,
not just specific platforms, CocoDxe. In CocoDxe we implement the
default safe behavior to accept all unaccepted memory and invalidate
the MemoryMap on ExitBootServices.
To allow the OS loader to prevent the eager acceptance, we add a new
protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
This protocol has one interface, Disable(). The OS loader can inform the
UEFI that it supports the unaccepted memory type and accepts the
responsibility to accept it.
All images that support unaccepted memory must now locate and call this
new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
function.
Changes since 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 (7):
OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
OvmfPkg: Introduce CocoDxe driver
MdePkg: Introduce the MemoryAcceptance protocol
OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 +
MdePkg/Include/Guid/EventGroup.h | 5 +
MdePkg/Include/Protocol/MemoryAcceptance.h | 40 +++++
MdePkg/MdePkg.dec | 8 +-
OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
OvmfPkg/CocoDxe/CocoDxe.c | 174 ++++++++++++++++++++
OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 ++-
OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
OvmfPkg/OvmfPkgX64.dsc | 1 +
OvmfPkg/OvmfPkgX64.fdf | 1 +
OvmfPkg/PlatformPei/AmdSev.c | 5 +
19 files changed, 366 insertions(+), 9 deletions(-)
create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-11-08 16:01 ` Yao, Jiewen
2022-11-08 16:24 ` Lendacky, Thomas
2022-10-24 20:41 ` [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID Dionna Glaze
` (6 subsequent siblings)
7 siblings, 2 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Gerd Hoffmann, James Bottomley, Jiewen Yao,
Tom Lendacky
From: Sophia Wolf <phiawolf@google.com>
When a guest OS does not support unaccepted memory, the unaccepted
memory must be accepted before returning a memory map to the caller.
EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++++++++++++++--
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++--
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
index 662d3c4ccb..f7600c3c81 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
@@ -20,6 +20,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Guid/ConfidentialComputingSevSnpBlob.h>
#include <Library/PcdLib.h>
+#include <Protocol/MemoryAccept.h>
STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
SIGNATURE_32 ('A', 'M', 'D', 'E'),
@@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
FixedPcdGet32 (PcdOvmfCpuidSize),
};
+STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
+
+#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
+
+STATIC
+EFI_STATUS
+EFIAPI
+AmdSevMemoryAccept (
+ IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
+ IN EFI_PHYSICAL_ADDRESS StartAddress,
+ IN UINTN Size
+ )
+{
+ //
+ // The StartAddress must be page-aligned, and the Size must be a positive
+ // multiple of SIZE_4KB. Use an assert instead of returning an erros since
+ // this is an EDK2-internal protocol.
+ //
+ ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
+ ASSERT (IS_ALIGNED (Size, SIZE_4KB));
+ ASSERT (Size != 0);
+
+ MemEncryptSevSnpPreValidateSystemRam (
+ StartAddress,
+ EFI_SIZE_TO_PAGES (Size)
+ );
+
+ return EFI_SUCCESS;
+}
+
+STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
+ AmdSevMemoryAccept
+};
+
EFI_STATUS
EFIAPI
AmdSevDxeEntryPoint (
@@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
}
}
- //
- // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
- // It contains the location for both the Secrets and CPUID page.
- //
if (MemEncryptSevSnpIsEnabled ()) {
+ //
+ // Memory acceptance began being required in SEV-SNP, so install the
+ // memory accept protocol implementation for a SEV-SNP active guest.
+ //
+ Status = gBS->InstallProtocolInterface (
+ &mAmdSevDxeHandle,
+ &gEdkiiMemoryAcceptProtocolGuid,
+ EFI_NATIVE_INTERFACE,
+ &mMemoryAcceptProtocol
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
+ // It contains the location for both the Secrets and CPUID page.
+ //
return gBS->InstallConfigurationTable (
&gConfidentialComputingSevSnpBlobGuid,
&mSnpBootDxeTable
diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
index 9acf860cf2..cd1b686c53 100644
--- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
@@ -47,6 +47,9 @@
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
+[Protocols]
+ gEdkiiMemoryAcceptProtocolGuid
+
[Guids]
gConfidentialComputingSevSnpBlobGuid
diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
index d3a95e4913..cbcdd46f52 100644
--- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
+++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
@@ -14,6 +14,7 @@
#include <Library/MemEncryptSevLib.h>
#include "SnpPageStateChange.h"
+#include "VirtualMemory.h"
/**
Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
@@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
IN UINTN NumPages
)
{
+ EFI_STATUS Status;
+
if (!MemEncryptSevSnpIsEnabled ()) {
return;
}
- //
- // All the pre-validation must be completed in the PEI phase.
- //
- ASSERT (FALSE);
+ // DXE pre-validation may happen with the memory accept protocol.
+ // The protocol should only be called outside the prevalidated ranges
+ // that the PEI stage code explicitly skips. Specifically, only memory
+ // ranges that are classified as unaccepted.
+ if (BaseAddress >= SIZE_4GB) {
+ Status = InternalMemEncryptSevCreateIdentityMap1G (
+ 0,
+ BaseAddress,
+ EFI_PAGES_TO_SIZE (NumPages)
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (FALSE);
+ CpuDeadLoop ();
+ }
+ }
+
+ InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
}
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-11-08 16:02 ` Yao, Jiewen
2022-10-24 20:41 ` [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices Dionna Glaze
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 UTC (permalink / raw)
To: devel
Cc: Dionna Glaze, Ard Biescheuvel, Min M. Xu, Gerd Hoffmann,
James Bottomley, Tom Lendacky, Jiewen Yao, Erdem Aktas
Event group as defined in UEFI standard v2.9.
Cc: Ard Biescheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
MdePkg/Include/Guid/EventGroup.h | 5 +++++
MdePkg/MdePkg.dec | 5 ++++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..64bfd4bab9 100644
--- a/MdePkg/Include/Guid/EventGroup.h
+++ b/MdePkg/Include/Guid/EventGroup.h
@@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
extern EFI_GUID gEfiEventExitBootServicesGuid;
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+ { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b, 0xfc } }
+
+extern EFI_GUID gEfiEventBeforeExitBootServicesGuid;
+
#define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
{ 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51, 0x96 } }
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6b6bfbec29..359a85ea10 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,7 +408,10 @@
gEfiEventMemoryMapChangeGuid = { 0x78BEE926, 0x692F, 0x48FD, { 0x9E, 0xDB, 0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
## Include/Guid/EventGroup.h
- gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+ gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7, { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
+
+ ## Include/Guid/EventGroup.h
+ gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44, { 0x80, 0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
## Include/Guid/EventGroup.h
gEfiEventExitBootServicesGuid = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80, 0x48, 0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-11-08 16:01 ` Yao, Jiewen
2022-10-24 20:41 ` [PATCH v8 4/7] OvmfPkg: Introduce CocoDxe driver Dionna Glaze
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 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, Ray Ni
Location of notification is has been specified in UEFI v2.9.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: "Min M. Xu" <min.m.xu@intel.com>
Cc: Andrew Fish <afish@apple.com>
Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index e4bca89577..35d5bf0dee 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -100,6 +100,7 @@
gEfiEventVirtualAddressChangeGuid ## CONSUMES ## Event
## CONSUMES ## Event
## PRODUCES ## Event
+ gEfiEventBeforeExitBootServicesGuid
gEfiEventExitBootServicesGuid
gEfiHobMemoryAllocModuleGuid ## SOMETIMES_CONSUMES ## HOB
gEfiFirmwareFileSystem2Guid ## CONSUMES ## GUID # Used to compare with FV's file system guid and get the FV's file system format
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 5733f0c8ec..4683016ed7 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -763,6 +763,12 @@ CoreExitBootServices (
{
EFI_STATUS Status;
+ //
+ // Notify other drivers of their last chance to use boot services
+ // before the memory map is terminated.
+ //
+ CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
+
//
// Disable Timer
//
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 4/7] OvmfPkg: Introduce CocoDxe driver
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
` (2 preceding siblings ...)
2022-10-24 20:41 ` [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 5/7] MdePkg: Introduce the MemoryAcceptance protocol Dionna Glaze
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 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 | 146 ++++++++++++++++++++
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, 199 insertions(+)
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 90e8a213ef..ad6b73ca4a 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -747,6 +747,7 @@
<LibraryClasses>
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
}
+ OvmfPkg/CocoDxe/CocoDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
#
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 4658e1d30e..3717ec9094 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -302,6 +302,7 @@ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
diff --git a/OvmfPkg/CocoDxe/CocoDxe.c b/OvmfPkg/CocoDxe/CocoDxe.c
new file mode 100644
index 0000000000..98874e6cfc
--- /dev/null
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -0,0 +1,146 @@
+/** @file
+
+ Confidential Compute Dxe driver. This driver installs protocols that are
+ generic over confidential compute techonology.
+
+ Copyright (c) 2022, Google LLC. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/MemEncryptSevLib.h>
+#include <Library/MemEncryptTdxLib.h>
+#include <Protocol/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 != EfiGcdMemoryTypeUnaccepted) {
+ continue;
+ }
+
+ Status = AcceptMemory->AcceptMemory (
+ AcceptMemory,
+ Desc->BaseAddress,
+ Desc->Length
+ );
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ Status = gDS->RemoveMemorySpace (Desc->BaseAddress, Desc->Length);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeSystemMemory,
+ Desc->BaseAddress,
+ Desc->Length,
+ EFI_MEMORY_CPU_CRYPTO | EFI_MEMORY_XP | EFI_MEMORY_RO | EFI_MEMORY_RP
+ );
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ }
+
+ gBS->FreePool (AllDescMap);
+ return Status;
+}
+
+VOID
+EFIAPI
+ResolveUnacceptedMemory (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ 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 c0c1a15b09..8136d50eb2 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -753,6 +753,7 @@
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
OvmfPkg/TdxDxe/TdxDxe.inf
+ OvmfPkg/CocoDxe/CocoDxe.inf
#
# Variable driver stack (non-SMM)
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.fdf b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
index 6923eb8831..e612608c0c 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.fdf
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.fdf
@@ -269,6 +269,7 @@ INF ShellPkg/Application/Shell/Shell.inf
INF MdeModulePkg/Logo/LogoDxe.inf
INF OvmfPkg/TdxDxe/TdxDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
#
# Usb Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index af566b953f..2cfb3fbc6b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -965,6 +965,7 @@
<LibraryClasses>
PciLib|MdePkg/Library/BasePciLibCf8/BasePciLibCf8.inf
}
+ OvmfPkg/CocoDxe/CocoDxe.inf
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 80de4fa2c0..2ab7f3b95b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -343,6 +343,7 @@ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
!if $(SMM_REQUIRE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index f39d9cd117..3ead476b61 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1036,6 +1036,7 @@
OvmfPkg/IoMmuDxe/IoMmuDxe.inf
OvmfPkg/TdxDxe/TdxDxe.inf
+ OvmfPkg/CocoDxe/CocoDxe.inf
!if $(SMM_REQUIRE) == TRUE
OvmfPkg/SmmAccess/SmmAccess2Dxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index c0f5a1ef3c..5dd452f42b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -370,6 +370,7 @@ INF OvmfPkg/QemuRamfbDxe/QemuRamfbDxe.inf
INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
INF OvmfPkg/PlatformDxe/Platform.inf
INF OvmfPkg/AmdSevDxe/AmdSevDxe.inf
+INF OvmfPkg/CocoDxe/CocoDxe.inf
INF OvmfPkg/IoMmuDxe/IoMmuDxe.inf
!if $(SMM_REQUIRE) == TRUE
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 5/7] MdePkg: Introduce the MemoryAcceptance protocol
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
` (3 preceding siblings ...)
2022-10-24 20:41 ` [PATCH v8 4/7] OvmfPkg: Introduce CocoDxe driver Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe Dionna Glaze
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 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(+)
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 359a85ea10..5c639c1b98 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1022,6 +1022,9 @@
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e, 0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}
[Protocols]
+ ## Include/Protocol/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.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
` (4 preceding siblings ...)
2022-10-24 20:41 ` [PATCH v8 5/7] MdePkg: Introduce the MemoryAcceptance protocol Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted Dionna Glaze
2022-11-08 14:37 ` [PATCH v8 0/7] Add safe unaccepted memory behavior Lendacky, Thomas
7 siblings, 0 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 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 98874e6cfc..14fbcf60d7 100644
--- a/OvmfPkg/CocoDxe/CocoDxe.c
+++ b/OvmfPkg/CocoDxe/CocoDxe.c
@@ -17,11 +17,14 @@
#include <Library/MemEncryptSevLib.h>
#include <Library/MemEncryptTdxLib.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 (
@@ -110,6 +113,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 (
@@ -142,5 +160,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.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v8 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
` (5 preceding siblings ...)
2022-10-24 20:41 ` [PATCH v8 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe Dionna Glaze
@ 2022-10-24 20:41 ` Dionna Glaze
2022-11-08 14:37 ` [PATCH v8 0/7] Add safe unaccepted memory behavior Lendacky, Thomas
7 siblings, 0 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-10-24 20:41 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 385562b44c..4cb6da4437 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_RESOURCE_MEMORY_UNACCEPTED;
+ continue;
+ }
if (ResourceHob->ResourceType == EFI_RESOURCE_SYSTEM_MEMORY) {
MemEncryptSevSnpPreValidateSystemRam (
--
2.38.0.135.g90850a2211-goog
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
` (6 preceding siblings ...)
2022-10-24 20:41 ` [PATCH v8 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted Dionna Glaze
@ 2022-11-08 14:37 ` Lendacky, Thomas
2022-11-08 16:09 ` [edk2-devel] " Yao, Jiewen
7 siblings, 1 reply; 19+ messages in thread
From: Lendacky, Thomas @ 2022-11-08 14:37 UTC (permalink / raw)
To: Dionna Glaze, devel
Cc: Ard Biescheuvel, Min M. Xu, Gerd Hoffmann, James Bottomley,
Jiewen Yao, Erdem Aktas, Andrew Fish, Michael D. Kinney
On 10/24/22 15:41, Dionna Glaze wrote:
> These seven patches build on the lazy-accept patch series
>
> "Introduce Lazy-accept for Tdx guest"
Since the above series was accepted into the EDK2 tree, can this series
also be pulled in so that both TDX and SNP can support unaccepted memory
in the same release?
Thanks,
Tom
>
> by adding SEV-SNP support for the MemoryAccept protocol, and
> importantly making eager memory acceptance the default behavior.
>
> We implement a standardized event group from UEFI v2.9,
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides exactly
> the right invocation point for eagerly accepting memory if eager
> acceptance has not been disabled.
>
> To make use of this event group, we add a new driver that is meant to
> carry behavior that is needed for all confidential compute technologies,
> not just specific platforms, CocoDxe. In CocoDxe we implement the
> default safe behavior to accept all unaccepted memory and invalidate
> the MemoryMap on ExitBootServices.
>
> To allow the OS loader to prevent the eager acceptance, we add a new
> protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
> This protocol has one interface, Disable(). The OS loader can inform the
> UEFI that it supports the unaccepted memory type and accepts the
> responsibility to accept it.
>
> All images that support unaccepted memory must now locate and call this
> new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call the Disable
> function.
>
> Changes since 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 (7):
> OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
> OvmfPkg: Introduce CocoDxe driver
> MdePkg: Introduce the MemoryAcceptance protocol
> OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
>
> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 +
> MdePkg/Include/Guid/EventGroup.h | 5 +
> MdePkg/Include/Protocol/MemoryAcceptance.h | 40 +++++
> MdePkg/MdePkg.dec | 8 +-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> OvmfPkg/CocoDxe/CocoDxe.c | 174 ++++++++++++++++++++
> OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 ++-
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.fdf | 1 +
> OvmfPkg/PlatformPei/AmdSev.c | 5 +
> 19 files changed, 366 insertions(+), 9 deletions(-)
> create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
> create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
2022-10-24 20:41 ` [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices Dionna Glaze
@ 2022-11-08 16:01 ` Yao, Jiewen
0 siblings, 0 replies; 19+ messages in thread
From: Yao, Jiewen @ 2022-11-08 16:01 UTC (permalink / raw)
To: Dionna Glaze, devel@edk2.groups.io
Cc: Gerd Hoffmann, James Bottomley, Tom Lendacky, Ard Biesheuvel,
Xu, Min M, Andrew Fish, Kinney, Michael D, Ni, Ray
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Tuesday, October 25, 2022 4:41 AM
> To: devel@edk2.groups.io
> Cc: Dionna Glaze <dionnaglaze@google.com>; Gerd Hoffmann
> <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Ard Biesheuvel <ardb@kernel.org>; Xu,
> Min M <min.m.xu@intel.com>; Andrew Fish <afish@apple.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
> Subject: [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in
> CoreExitBootServices
>
> Location of notification is has been specified in UEFI v2.9.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Cc: "Michael D. Kinney" <michael.d.kinney@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index e4bca89577..35d5bf0dee 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -100,6 +100,7 @@
> gEfiEventVirtualAddressChangeGuid ## CONSUMES ##
> Event
> ## CONSUMES ## Event
> ## PRODUCES ## Event
> + gEfiEventBeforeExitBootServicesGuid
> gEfiEventExitBootServicesGuid
> gEfiHobMemoryAllocModuleGuid ## SOMETIMES_CONSUMES
> ## HOB
> gEfiFirmwareFileSystem2Guid ## CONSUMES ## GUID #
> Used to compare with FV's file system guid and get the FV's file system
> format
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 5733f0c8ec..4683016ed7 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -763,6 +763,12 @@ CoreExitBootServices (
> {
> EFI_STATUS Status;
>
> + //
> + // Notify other drivers of their last chance to use boot services
> + // before the memory map is terminated.
> + //
> + CoreNotifySignalList (&gEfiEventBeforeExitBootServicesGuid);
> +
> //
> // Disable Timer
> //
> --
> 2.38.0.135.g90850a2211-goog
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
@ 2022-11-08 16:01 ` Yao, Jiewen
2022-11-08 16:24 ` Lendacky, Thomas
1 sibling, 0 replies; 19+ messages in thread
From: Yao, Jiewen @ 2022-11-08 16:01 UTC (permalink / raw)
To: Dionna Glaze, devel@edk2.groups.io
Cc: Gerd Hoffmann, James Bottomley, Tom Lendacky
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
Need AMD-SEV people to give Reviewed-by.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Tuesday, October 25, 2022 4:41 AM
> To: devel@edk2.groups.io
> Cc: Dionna Glaze <dionnaglaze@google.com>; Gerd Hoffmann
> <kraxel@redhat.com>; James Bottomley <jejb@linux.ibm.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>
> Subject: [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in
> AmdSevDxe
>
> From: Sophia Wolf <phiawolf@google.com>
>
> When a guest OS does not support unaccepted memory, the unaccepted
> memory must be accepted before returning a memory map to the caller.
>
> EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
> Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
> ++++++++++++++++++--
> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++
>
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> c | 24 +++++++--
> 3 files changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 662d3c4ccb..f7600c3c81 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -20,6 +20,7 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Guid/ConfidentialComputingSevSnpBlob.h>
> #include <Library/PcdLib.h>
> +#include <Protocol/MemoryAccept.h>
>
> STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION
> mSnpBootDxeTable = {
> SIGNATURE_32 ('A', 'M', 'D', 'E'),
> @@ -31,6 +32,40 @@ STATIC
> CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
> FixedPcdGet32 (PcdOvmfCpuidSize),
> };
>
> +STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
> +
> +#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +AmdSevMemoryAccept (
> + IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS StartAddress,
> + IN UINTN Size
> + )
> +{
> + //
> + // The StartAddress must be page-aligned, and the Size must be a
> positive
> + // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> + // this is an EDK2-internal protocol.
> + //
> + ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> + ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> + ASSERT (Size != 0);
> +
> + MemEncryptSevSnpPreValidateSystemRam (
> + StartAddress,
> + EFI_SIZE_TO_PAGES (Size)
> + );
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
> + AmdSevMemoryAccept
> +};
> +
> EFI_STATUS
> EFIAPI
> AmdSevDxeEntryPoint (
> @@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
> }
> }
>
> - //
> - // If its SEV-SNP active guest then install the
> CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
> - // It contains the location for both the Secrets and CPUID page.
> - //
> if (MemEncryptSevSnpIsEnabled ()) {
> + //
> + // Memory acceptance began being required in SEV-SNP, so install the
> + // memory accept protocol implementation for a SEV-SNP active guest.
> + //
> + Status = gBS->InstallProtocolInterface (
> + &mAmdSevDxeHandle,
> + &gEdkiiMemoryAcceptProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + &mMemoryAcceptProtocol
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> + // If its SEV-SNP active guest then install the
> CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
> + // It contains the location for both the Secrets and CPUID page.
> + //
> return gBS->InstallConfigurationTable (
> &gConfidentialComputingSevSnpBlobGuid,
> &mSnpBootDxeTable
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> index 9acf860cf2..cd1b686c53 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> @@ -47,6 +47,9 @@
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
>
> +[Protocols]
> + gEdkiiMemoryAcceptProtocolGuid
> +
> [Guids]
> gConfidentialComputingSevSnpBlobGuid
>
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidat
> e.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValida
> te.c
> index d3a95e4913..cbcdd46f52 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidat
> e.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValida
> te.c
> @@ -14,6 +14,7 @@
> #include <Library/MemEncryptSevLib.h>
>
> #include "SnpPageStateChange.h"
> +#include "VirtualMemory.h"
>
> /**
> Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
> @@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
> IN UINTN NumPages
> )
> {
> + EFI_STATUS Status;
> +
> if (!MemEncryptSevSnpIsEnabled ()) {
> return;
> }
>
> - //
> - // All the pre-validation must be completed in the PEI phase.
> - //
> - ASSERT (FALSE);
> + // DXE pre-validation may happen with the memory accept protocol.
> + // The protocol should only be called outside the prevalidated ranges
> + // that the PEI stage code explicitly skips. Specifically, only memory
> + // ranges that are classified as unaccepted.
> + if (BaseAddress >= SIZE_4GB) {
> + Status = InternalMemEncryptSevCreateIdentityMap1G (
> + 0,
> + BaseAddress,
> + EFI_PAGES_TO_SIZE (NumPages)
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> + }
> + }
> +
> + InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate,
> TRUE);
> }
> --
> 2.38.0.135.g90850a2211-goog
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
2022-10-24 20:41 ` [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID Dionna Glaze
@ 2022-11-08 16:02 ` Yao, Jiewen
0 siblings, 0 replies; 19+ messages in thread
From: Yao, Jiewen @ 2022-11-08 16:02 UTC (permalink / raw)
To: Dionna Glaze, devel@edk2.groups.io
Cc: Ard Biescheuvel, Xu, Min M, Gerd Hoffmann, James Bottomley,
Tom Lendacky, Aktas, Erdem
Acked-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Tuesday, October 25, 2022 4:41 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>
> Subject: [PATCH v8 2/7] MdePkg: Add
> EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
>
> Event group as defined in UEFI standard v2.9.
>
> Cc: Ard Biescheuvel <ardb@kernel.org>
> Cc: "Min M. Xu" <min.m.xu@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
> MdePkg/Include/Guid/EventGroup.h | 5 +++++
> MdePkg/MdePkg.dec | 5 ++++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/MdePkg/Include/Guid/EventGroup.h
> b/MdePkg/Include/Guid/EventGroup.h
> index 063d1f7157..64bfd4bab9 100644
> --- a/MdePkg/Include/Guid/EventGroup.h
> +++ b/MdePkg/Include/Guid/EventGroup.h
> @@ -14,6 +14,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> extern EFI_GUID gEfiEventExitBootServicesGuid;
>
> +#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
> + { 0x8be0e274, 0x3970, 0x4b44, { 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b,
> 0xfc } }
> +
> +extern EFI_GUID gEfiEventBeforeExitBootServicesGuid;
> +
> #define EFI_EVENT_GROUP_VIRTUAL_ADDRESS_CHANGE \
> { 0x13fa7698, 0xc831, 0x49c7, { 0x87, 0xea, 0x8f, 0x43, 0xfc, 0xc2, 0x51,
> 0x96 } }
>
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 6b6bfbec29..359a85ea10 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -408,7 +408,10 @@
> gEfiEventMemoryMapChangeGuid = { 0x78BEE926, 0x692F, 0x48FD,
> { 0x9E, 0xDB, 0x01, 0x42, 0x2E, 0xF0, 0xD7, 0xAB }}
>
> ## Include/Guid/EventGroup.h
> - gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7,
> { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> + gEfiEventVirtualAddressChangeGuid = { 0x13FA7698, 0xC831, 0x49C7,
> { 0x87, 0xEA, 0x8F, 0x43, 0xFC, 0xC2, 0x51, 0x96 }}
> +
> + ## Include/Guid/EventGroup.h
> + gEfiEventBeforeExitBootServicesGuid = { 0x8BE0E274, 0x3970, 0x4B44,
> { 0x80, 0xC5, 0x1A, 0xB9, 0x50, 0x2F, 0x3B, 0xFC }}
>
> ## Include/Guid/EventGroup.h
> gEfiEventExitBootServicesGuid = { 0x27ABF055, 0xB1B8, 0x4C26, { 0x80,
> 0x48, 0x74, 0x8F, 0x37, 0xBA, 0xA2, 0xDF }}
> --
> 2.38.0.135.g90850a2211-goog
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 14:37 ` [PATCH v8 0/7] Add safe unaccepted memory behavior Lendacky, Thomas
@ 2022-11-08 16:09 ` Yao, Jiewen
2022-11-08 16:22 ` Dionna Glaze
2022-11-08 22:24 ` Lendacky, Thomas
0 siblings, 2 replies; 19+ messages in thread
From: Yao, Jiewen @ 2022-11-08 16:09 UTC (permalink / raw)
To: devel@edk2.groups.io, thomas.lendacky@amd.com, Dionna Glaze
Cc: Ard Biescheuvel, Xu, Min M, Gerd Hoffmann, James Bottomley,
Aktas, Erdem, Andrew Fish, Kinney, Michael D
Hi Tom/Dionna
I think this patch is addressing https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
For patch 1~3, I am OK for the API which we already agreed, such as EfiMemoryAcceptProtocol and EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
For patch 4, it changed the behavior to accept all. I don’t believe it should be 4. It should be the latest one. (please correct me if I am wrong.)
For patch 5~6, I cannot give R-B for BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does not address my concern. Please refer to the discussion in the Bugzilla.
If you want to split the patch series and submit 1~3 as standalone one, that will match SEV to TDX on lazy accept part. I believe we may merge them after we get R-B from right person.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Lendacky, Thomas via groups.io
> Sent: Tuesday, November 8, 2022 10:38 PM
> To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
> Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>;
> Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
> <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> behavior
>
> On 10/24/22 15:41, Dionna Glaze wrote:
> > These seven patches build on the lazy-accept patch series
> >
> > "Introduce Lazy-accept for Tdx guest"
>
> Since the above series was accepted into the EDK2 tree, can this series
> also be pulled in so that both TDX and SNP can support unaccepted
> memory
> in the same release?
>
> Thanks,
> Tom
>
> >
> > by adding SEV-SNP support for the MemoryAccept protocol, and
> > importantly making eager memory acceptance the default behavior.
> >
> > We implement a standardized event group from UEFI v2.9,
> > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
> exactly
> > the right invocation point for eagerly accepting memory if eager
> > acceptance has not been disabled.
> >
> > To make use of this event group, we add a new driver that is meant to
> > carry behavior that is needed for all confidential compute technologies,
> > not just specific platforms, CocoDxe. In CocoDxe we implement the
> > default safe behavior to accept all unaccepted memory and invalidate
> > the MemoryMap on ExitBootServices.
> >
> > To allow the OS loader to prevent the eager acceptance, we add a new
> > protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
> > This protocol has one interface, Disable(). The OS loader can inform the
> > UEFI that it supports the unaccepted memory type and accepts the
> > responsibility to accept it.
> >
> > All images that support unaccepted memory must now locate and call
> this
> > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call
> the Disable
> > function.
> >
> > Changes since 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 (7):
> > OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> > MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> > MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
> > OvmfPkg: Introduce CocoDxe driver
> > MdePkg: Introduce the MemoryAcceptance protocol
> > OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> > OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> >
> > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 +
> > MdePkg/Include/Guid/EventGroup.h | 5 +
> > MdePkg/Include/Protocol/MemoryAcceptance.h | 40
> +++++
> > MdePkg/MdePkg.dec | 8 +-
> > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> > OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
> > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> > OvmfPkg/CocoDxe/CocoDxe.c | 174
> ++++++++++++++++++++
> > OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> > OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> >
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> c | 24 ++-
> > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > OvmfPkg/OvmfPkgX64.fdf | 1 +
> > OvmfPkg/PlatformPei/AmdSev.c | 5 +
> > 19 files changed, 366 insertions(+), 9 deletions(-)
> > create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
> > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> >
>
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 16:09 ` [edk2-devel] " Yao, Jiewen
@ 2022-11-08 16:22 ` Dionna Glaze
2022-11-08 16:36 ` Yao, Jiewen
2022-11-08 22:24 ` Lendacky, Thomas
1 sibling, 1 reply; 19+ messages in thread
From: Dionna Glaze @ 2022-11-08 16:22 UTC (permalink / raw)
To: Yao, Jiewen
Cc: devel@edk2.groups.io, thomas.lendacky@amd.com, Ard Biescheuvel,
Xu, Min M, Gerd Hoffmann, James Bottomley, Aktas, Erdem,
Andrew Fish, Kinney, Michael D
On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi Tom/Dionna
> I think this patch is addressing https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
>
> For patch 1~3, I am OK for the API which we already agreed, such as EfiMemoryAcceptProtocol and EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
> I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
>
> For patch 4, it changed the behavior to accept all. I don’t believe it should be 4. It should be the latest one. (please correct me if I am wrong.)
>
The acceptance protocol (5-6) and the accept-all behavior (4) could be
swapped in order, but they really only make sense as a pair. No client
would use a new protocol to disable the behavior of something that
doesn't happen.
The accept-all behavior (4) must be default before SEV-SNP introduces
unaccepted memory (7).
> For patch 5~6, I cannot give R-B for BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does not address my concern. Please refer to the discussion in the Bugzilla.
>
I don't think your concern is addressable given the constraints of a
smooth upgrade path. I think the right folks to discuss this with you
from the Intel side are Min Xu and Kirill Shutemov. Kirill will know
what is feasible for the OS to inform the UEFI of in terms of memory
required, and how to not break OSes that don't use the new protocol,
whereas Min should know where that kind of information should be
communicated.
>
> If you want to split the patch series and submit 1~3 as standalone one, that will match SEV to TDX on lazy accept part. I believe we may merge them after we get R-B from right person.
>
I can do that.
> Thank you
> Yao, Jiewen
>
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > Lendacky, Thomas via groups.io
> > Sent: Tuesday, November 8, 2022 10:38 PM
> > To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
> > Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>;
> > Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
> > <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > behavior
> >
> > On 10/24/22 15:41, Dionna Glaze wrote:
> > > These seven patches build on the lazy-accept patch series
> > >
> > > "Introduce Lazy-accept for Tdx guest"
> >
> > Since the above series was accepted into the EDK2 tree, can this series
> > also be pulled in so that both TDX and SNP can support unaccepted
> > memory
> > in the same release?
> >
> > Thanks,
> > Tom
> >
> > >
> > > by adding SEV-SNP support for the MemoryAccept protocol, and
> > > importantly making eager memory acceptance the default behavior.
> > >
> > > We implement a standardized event group from UEFI v2.9,
> > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
> > exactly
> > > the right invocation point for eagerly accepting memory if eager
> > > acceptance has not been disabled.
> > >
> > > To make use of this event group, we add a new driver that is meant to
> > > carry behavior that is needed for all confidential compute technologies,
> > > not just specific platforms, CocoDxe. In CocoDxe we implement the
> > > default safe behavior to accept all unaccepted memory and invalidate
> > > the MemoryMap on ExitBootServices.
> > >
> > > To allow the OS loader to prevent the eager acceptance, we add a new
> > > protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
> > > This protocol has one interface, Disable(). The OS loader can inform the
> > > UEFI that it supports the unaccepted memory type and accepts the
> > > responsibility to accept it.
> > >
> > > All images that support unaccepted memory must now locate and call
> > this
> > > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call
> > the Disable
> > > function.
> > >
> > > Changes since 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 (7):
> > > OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> > > MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> > > MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
> > > OvmfPkg: Introduce CocoDxe driver
> > > MdePkg: Introduce the MemoryAcceptance protocol
> > > OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> > > OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> > >
> > > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 +
> > > MdePkg/Include/Guid/EventGroup.h | 5 +
> > > MdePkg/Include/Protocol/MemoryAcceptance.h | 40
> > +++++
> > > MdePkg/MdePkg.dec | 8 +-
> > > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> > > OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> > > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
> > > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> > > OvmfPkg/CocoDxe/CocoDxe.c | 174
> > ++++++++++++++++++++
> > > OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> > > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> > > OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> > >
> > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> > c | 24 ++-
> > > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > > OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> > > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > > OvmfPkg/OvmfPkgX64.fdf | 1 +
> > > OvmfPkg/PlatformPei/AmdSev.c | 5 +
> > > 19 files changed, 366 insertions(+), 9 deletions(-)
> > > create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
> > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> > >
> >
> >
> >
> >
>
--
-Dionna Glaze, PhD (she/her)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
2022-11-08 16:01 ` Yao, Jiewen
@ 2022-11-08 16:24 ` Lendacky, Thomas
1 sibling, 0 replies; 19+ messages in thread
From: Lendacky, Thomas @ 2022-11-08 16:24 UTC (permalink / raw)
To: Dionna Glaze, devel; +Cc: Gerd Hoffmann, James Bottomley, Jiewen Yao
On 10/24/22 15:41, Dionna Glaze wrote:
> From: Sophia Wolf <phiawolf@google.com>
>
> When a guest OS does not support unaccepted memory, the unaccepted
> memory must be accepted before returning a memory map to the caller.
>
> EfiMemoryAcceptProtocol is defined in MdePkg and is implemented /
> Installed in AmdSevDxe for AMD SEV-SNP memory acceptance.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++++++++++++++--
> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 ++
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++--
> 3 files changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> index 662d3c4ccb..f7600c3c81 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c
> @@ -20,6 +20,7 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Guid/ConfidentialComputingSevSnpBlob.h>
> #include <Library/PcdLib.h>
> +#include <Protocol/MemoryAccept.h>
>
> STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
> SIGNATURE_32 ('A', 'M', 'D', 'E'),
> @@ -31,6 +32,40 @@ STATIC CONFIDENTIAL_COMPUTING_SNP_BLOB_LOCATION mSnpBootDxeTable = {
> FixedPcdGet32 (PcdOvmfCpuidSize),
> };
>
> +STATIC EFI_HANDLE mAmdSevDxeHandle = NULL;
> +
> +#define IS_ALIGNED(x, y) ((((x) & ((y) - 1)) == 0))
> +
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +AmdSevMemoryAccept (
> + IN EDKII_MEMORY_ACCEPT_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS StartAddress,
> + IN UINTN Size
> + )
> +{
> + //
> + // The StartAddress must be page-aligned, and the Size must be a positive
> + // multiple of SIZE_4KB. Use an assert instead of returning an erros since
> + // this is an EDK2-internal protocol.
> + //
> + ASSERT (IS_ALIGNED (StartAddress, SIZE_4KB));
> + ASSERT (IS_ALIGNED (Size, SIZE_4KB));
> + ASSERT (Size != 0);
> +
> + MemEncryptSevSnpPreValidateSystemRam (
> + StartAddress,
> + EFI_SIZE_TO_PAGES (Size)
> + );
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC EDKII_MEMORY_ACCEPT_PROTOCOL mMemoryAcceptProtocol = {
> + AmdSevMemoryAccept
> +};
> +
> EFI_STATUS
> EFIAPI
> AmdSevDxeEntryPoint (
> @@ -147,11 +182,23 @@ AmdSevDxeEntryPoint (
> }
> }
>
> - //
> - // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
> - // It contains the location for both the Secrets and CPUID page.
> - //
> if (MemEncryptSevSnpIsEnabled ()) {
> + //
> + // Memory acceptance began being required in SEV-SNP, so install the
> + // memory accept protocol implementation for a SEV-SNP active guest.
> + //
> + Status = gBS->InstallProtocolInterface (
> + &mAmdSevDxeHandle,
> + &gEdkiiMemoryAcceptProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + &mMemoryAcceptProtocol
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> + // If its SEV-SNP active guest then install the CONFIDENTIAL_COMPUTING_SEV_SNP_BLOB.
> + // It contains the location for both the Secrets and CPUID page.
> + //
> return gBS->InstallConfigurationTable (
> &gConfidentialComputingSevSnpBlobGuid,
> &mSnpBootDxeTable
> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> index 9acf860cf2..cd1b686c53 100644
> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf
> @@ -47,6 +47,9 @@
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize
>
> +[Protocols]
> + gEdkiiMemoryAcceptProtocolGuid
> +
> [Guids]
> gConfidentialComputingSevSnpBlobGuid
>
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
> index d3a95e4913..cbcdd46f52 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
> @@ -14,6 +14,7 @@
> #include <Library/MemEncryptSevLib.h>
>
> #include "SnpPageStateChange.h"
> +#include "VirtualMemory.h"
>
> /**
> Pre-validate the system RAM when SEV-SNP is enabled in the guest VM.
> @@ -29,12 +30,27 @@ MemEncryptSevSnpPreValidateSystemRam (
> IN UINTN NumPages
> )
> {
> + EFI_STATUS Status;
> +
> if (!MemEncryptSevSnpIsEnabled ()) {
> return;
> }
>
> - //
> - // All the pre-validation must be completed in the PEI phase.
> - //
> - ASSERT (FALSE);
> + // DXE pre-validation may happen with the memory accept protocol.
> + // The protocol should only be called outside the prevalidated ranges
> + // that the PEI stage code explicitly skips. Specifically, only memory
> + // ranges that are classified as unaccepted.
> + if (BaseAddress >= SIZE_4GB) {
> + Status = InternalMemEncryptSevCreateIdentityMap1G (
> + 0,
> + BaseAddress,
> + EFI_PAGES_TO_SIZE (NumPages)
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT (FALSE);
> + CpuDeadLoop ();
> + }
> + }
> +
> + InternalSetPageState (BaseAddress, NumPages, SevSnpPagePrivate, TRUE);
> }
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 16:22 ` Dionna Glaze
@ 2022-11-08 16:36 ` Yao, Jiewen
2022-11-08 16:43 ` Dionna Glaze
0 siblings, 1 reply; 19+ messages in thread
From: Yao, Jiewen @ 2022-11-08 16:36 UTC (permalink / raw)
To: Dionna Amalie Glaze
Cc: devel@edk2.groups.io, thomas.lendacky@amd.com, Ard Biescheuvel,
Xu, Min M, Gerd Hoffmann, James Bottomley, Aktas, Erdem,
Andrew Fish, Kinney, Michael D
Right, Dionna. In intel, we are doing POC to see what is the best size the BIOS should accept.
The total time for memory accept is 4 part:
1) vBIOS early phase, single thread accept
2) vBIOS runtime phase, multi thread accept
3) OS early phase, single thread accept
4) OS runtime phase, multi thread accept
We hope 1) is zero.
And we are trying to balance between 2) and 3).
I hope to discuss more after we get the data.
Then we can discuss how to achieve best performance.
BTW: Is there any data for AMD-SEV?
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Dionna Amalie Glaze <dionnaglaze@google.com>
> Sent: Wednesday, November 9, 2022 12:23 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; thomas.lendacky@amd.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>; Aktas,
> Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> behavior
>
> On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > Hi Tom/Dionna
> > I think this patch is addressing
> https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
> >
> > For patch 1~3, I am OK for the API which we already agreed, such as
> EfiMemoryAcceptProtocol and
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
> > I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
> >
> > For patch 4, it changed the behavior to accept all. I don’t believe it should
> be 4. It should be the latest one. (please correct me if I am wrong.)
> >
>
> The acceptance protocol (5-6) and the accept-all behavior (4) could be
> swapped in order, but they really only make sense as a pair. No client
> would use a new protocol to disable the behavior of something that
> doesn't happen.
> The accept-all behavior (4) must be default before SEV-SNP introduces
> unaccepted memory (7).
>
> > For patch 5~6, I cannot give R-B for
> BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does
> not address my concern. Please refer to the discussion in the Bugzilla.
> >
>
> I don't think your concern is addressable given the constraints of a
> smooth upgrade path. I think the right folks to discuss this with you
> from the Intel side are Min Xu and Kirill Shutemov. Kirill will know
> what is feasible for the OS to inform the UEFI of in terms of memory
> required, and how to not break OSes that don't use the new protocol,
> whereas Min should know where that kind of information should be
> communicated.
>
> >
> > If you want to split the patch series and submit 1~3 as standalone one,
> that will match SEV to TDX on lazy accept part. I believe we may merge
> them after we get R-B from right person.
> >
>
> I can do that.
>
> > Thank you
> > Yao, Jiewen
> >
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > Lendacky, Thomas via groups.io
> > > Sent: Tuesday, November 8, 2022 10:38 PM
> > > To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
> > > Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M
> <min.m.xu@intel.com>;
> > > Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
> > > <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > > behavior
> > >
> > > On 10/24/22 15:41, Dionna Glaze wrote:
> > > > These seven patches build on the lazy-accept patch series
> > > >
> > > > "Introduce Lazy-accept for Tdx guest"
> > >
> > > Since the above series was accepted into the EDK2 tree, can this series
> > > also be pulled in so that both TDX and SNP can support unaccepted
> > > memory
> > > in the same release?
> > >
> > > Thanks,
> > > Tom
> > >
> > > >
> > > > by adding SEV-SNP support for the MemoryAccept protocol, and
> > > > importantly making eager memory acceptance the default behavior.
> > > >
> > > > We implement a standardized event group from UEFI v2.9,
> > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
> > > exactly
> > > > the right invocation point for eagerly accepting memory if eager
> > > > acceptance has not been disabled.
> > > >
> > > > To make use of this event group, we add a new driver that is meant to
> > > > carry behavior that is needed for all confidential compute
> technologies,
> > > > not just specific platforms, CocoDxe. In CocoDxe we implement the
> > > > default safe behavior to accept all unaccepted memory and invalidate
> > > > the MemoryMap on ExitBootServices.
> > > >
> > > > To allow the OS loader to prevent the eager acceptance, we add a new
> > > > protocol, up for standardization,
> AcceptAllUnacceptedMemoryProtocol.
> > > > This protocol has one interface, Disable(). The OS loader can inform
> the
> > > > UEFI that it supports the unaccepted memory type and accepts the
> > > > responsibility to accept it.
> > > >
> > > > All images that support unaccepted memory must now locate and call
> > > this
> > > > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and
> call
> > > the Disable
> > > > function.
> > > >
> > > > Changes since 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 (7):
> > > > OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> > > > MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> > > > MdeModulePkg: Notify BeforeExitBootServices in
> CoreExitBootServices
> > > > OvmfPkg: Introduce CocoDxe driver
> > > > MdePkg: Introduce the MemoryAcceptance protocol
> > > > OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> > > > OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> > > >
> > > > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > > > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6
> +
> > > > MdePkg/Include/Guid/EventGroup.h | 5 +
> > > > MdePkg/Include/Protocol/MemoryAcceptance.h | 40
> > > +++++
> > > > MdePkg/MdePkg.dec | 8 +-
> > > > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> > > > OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> > > > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
> ++++++-
> > > > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> > > > OvmfPkg/CocoDxe/CocoDxe.c | 174
> > > ++++++++++++++++++++
> > > > OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> > > > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> > > > OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> > > >
> > >
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> > > c | 24 ++-
> > > > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > > > OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> > > > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > > > OvmfPkg/OvmfPkgX64.fdf | 1 +
> > > > OvmfPkg/PlatformPei/AmdSev.c | 5 +
> > > > 19 files changed, 366 insertions(+), 9 deletions(-)
> > > > create mode 100644
> MdePkg/Include/Protocol/MemoryAcceptance.h
> > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> > > >
> > >
> > >
> > >
> > >
> >
>
>
> --
> -Dionna Glaze, PhD (she/her)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 16:36 ` Yao, Jiewen
@ 2022-11-08 16:43 ` Dionna Glaze
2022-11-08 18:44 ` Dionna Glaze
0 siblings, 1 reply; 19+ messages in thread
From: Dionna Glaze @ 2022-11-08 16:43 UTC (permalink / raw)
To: Yao, Jiewen
Cc: devel@edk2.groups.io, thomas.lendacky@amd.com, Ard Biescheuvel,
Xu, Min M, Gerd Hoffmann, James Bottomley, Aktas, Erdem,
Andrew Fish, Kinney, Michael D
>
> BTW: Is there any data for AMD-SEV?
>
Earlier this year, I tried to make the lazy accept patches work for
SEV-SNP, but the boot would always crash in the Qemu try boot kernel
step IIRC:
https://github.com/AMDESE/ovmf/pull/4
Tom suggested accepting the first 4GiB and I didn't go back to try to
make the more complicated solution work, since accepting the HOB up to
the MMIO hole took roughly 30ms, which is well within our boot budget
given the outsized cost of pinning memory. The launch sequence (start,
update_data*, finish) for OVMF takes about 190ms, which we can't make
any lower.
>
> Thank you
> Yao, Jiewen
>
>
> > -----Original Message-----
> > From: Dionna Amalie Glaze <dionnaglaze@google.com>
> > Sent: Wednesday, November 9, 2022 12:23 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: devel@edk2.groups.io; thomas.lendacky@amd.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>; Aktas,
> > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > behavior
> >
> > On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > >
> > > Hi Tom/Dionna
> > > I think this patch is addressing
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
> > >
> > > For patch 1~3, I am OK for the API which we already agreed, such as
> > EfiMemoryAcceptProtocol and
> > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
> > > I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
> > >
> > > For patch 4, it changed the behavior to accept all. I don’t believe it should
> > be 4. It should be the latest one. (please correct me if I am wrong.)
> > >
> >
> > The acceptance protocol (5-6) and the accept-all behavior (4) could be
> > swapped in order, but they really only make sense as a pair. No client
> > would use a new protocol to disable the behavior of something that
> > doesn't happen.
> > The accept-all behavior (4) must be default before SEV-SNP introduces
> > unaccepted memory (7).
> >
> > > For patch 5~6, I cannot give R-B for
> > BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does
> > not address my concern. Please refer to the discussion in the Bugzilla.
> > >
> >
> > I don't think your concern is addressable given the constraints of a
> > smooth upgrade path. I think the right folks to discuss this with you
> > from the Intel side are Min Xu and Kirill Shutemov. Kirill will know
> > what is feasible for the OS to inform the UEFI of in terms of memory
> > required, and how to not break OSes that don't use the new protocol,
> > whereas Min should know where that kind of information should be
> > communicated.
> >
> > >
> > > If you want to split the patch series and submit 1~3 as standalone one,
> > that will match SEV to TDX on lazy accept part. I believe we may merge
> > them after we get R-B from right person.
> > >
> >
> > I can do that.
> >
> > > Thank you
> > > Yao, Jiewen
> > >
> > >
> > > > -----Original Message-----
> > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > Lendacky, Thomas via groups.io
> > > > Sent: Tuesday, November 8, 2022 10:38 PM
> > > > To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
> > > > Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M
> > <min.m.xu@intel.com>;
> > > > Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
> > > > <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > > > behavior
> > > >
> > > > On 10/24/22 15:41, Dionna Glaze wrote:
> > > > > These seven patches build on the lazy-accept patch series
> > > > >
> > > > > "Introduce Lazy-accept for Tdx guest"
> > > >
> > > > Since the above series was accepted into the EDK2 tree, can this series
> > > > also be pulled in so that both TDX and SNP can support unaccepted
> > > > memory
> > > > in the same release?
> > > >
> > > > Thanks,
> > > > Tom
> > > >
> > > > >
> > > > > by adding SEV-SNP support for the MemoryAccept protocol, and
> > > > > importantly making eager memory acceptance the default behavior.
> > > > >
> > > > > We implement a standardized event group from UEFI v2.9,
> > > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
> > > > exactly
> > > > > the right invocation point for eagerly accepting memory if eager
> > > > > acceptance has not been disabled.
> > > > >
> > > > > To make use of this event group, we add a new driver that is meant to
> > > > > carry behavior that is needed for all confidential compute
> > technologies,
> > > > > not just specific platforms, CocoDxe. In CocoDxe we implement the
> > > > > default safe behavior to accept all unaccepted memory and invalidate
> > > > > the MemoryMap on ExitBootServices.
> > > > >
> > > > > To allow the OS loader to prevent the eager acceptance, we add a new
> > > > > protocol, up for standardization,
> > AcceptAllUnacceptedMemoryProtocol.
> > > > > This protocol has one interface, Disable(). The OS loader can inform
> > the
> > > > > UEFI that it supports the unaccepted memory type and accepts the
> > > > > responsibility to accept it.
> > > > >
> > > > > All images that support unaccepted memory must now locate and call
> > > > this
> > > > > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and
> > call
> > > > the Disable
> > > > > function.
> > > > >
> > > > > Changes since 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 (7):
> > > > > OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> > > > > MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> > > > > MdeModulePkg: Notify BeforeExitBootServices in
> > CoreExitBootServices
> > > > > OvmfPkg: Introduce CocoDxe driver
> > > > > MdePkg: Introduce the MemoryAcceptance protocol
> > > > > OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> > > > > OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> > > > >
> > > > > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > > > > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6
> > +
> > > > > MdePkg/Include/Guid/EventGroup.h | 5 +
> > > > > MdePkg/Include/Protocol/MemoryAcceptance.h | 40
> > > > +++++
> > > > > MdePkg/MdePkg.dec | 8 +-
> > > > > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> > > > > OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> > > > > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
> > ++++++-
> > > > > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> > > > > OvmfPkg/CocoDxe/CocoDxe.c | 174
> > > > ++++++++++++++++++++
> > > > > OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> > > > > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> > > > > OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> > > > >
> > > >
> > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> > > > c | 24 ++-
> > > > > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > > > > OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> > > > > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > > > > OvmfPkg/OvmfPkgX64.fdf | 1 +
> > > > > OvmfPkg/PlatformPei/AmdSev.c | 5 +
> > > > > 19 files changed, 366 insertions(+), 9 deletions(-)
> > > > > create mode 100644
> > MdePkg/Include/Protocol/MemoryAcceptance.h
> > > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> > > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
> >
> > --
> > -Dionna Glaze, PhD (she/her)
--
-Dionna Glaze, PhD (she/her)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 16:43 ` Dionna Glaze
@ 2022-11-08 18:44 ` Dionna Glaze
0 siblings, 0 replies; 19+ messages in thread
From: Dionna Glaze @ 2022-11-08 18:44 UTC (permalink / raw)
To: Yao, Jiewen
Cc: devel@edk2.groups.io, thomas.lendacky@amd.com, Ard Biescheuvel,
Xu, Min M, Gerd Hoffmann, James Bottomley, Aktas, Erdem,
Andrew Fish, Kinney, Michael D, Mikolaj Lisik
> The total time for memory accept is 4 part:
>1) vBIOS early phase, single thread accept
>2) vBIOS runtime phase, multi thread accept
>3) OS early phase, single thread accept
>4) OS runtime phase, multi thread accept
>
>We hope 1) is zero.
>And we are trying to balance between 2) and 3).
Do you have a sense of how once you have that balance determined you
would specify accepting less than 4GB when an OS supports it, and
accepting 4GB when an OS doesn't?
I'm not familiar enough with vBIOS memory allocation to know how
runtime stacks are created. If we're in the situation images are
"self-contained" in that the only memory that must be accepted is just
enough to fit the binary size of an image, and then the image uses
AllocatePages to allocate its own stack and whatever else, then we
could be okay to not require any pre-acceptance. The lazy accept Min
proposed in Page.c seems like it'd be fine enough to handle all memory
use prior to the accept decision at EBS.
This doesn't appear to be the world in which we live, given the
crashes I experienced in the above mentioned PR, but I could try
again.
If we're not in the "self-contained" image situation, I don't know
what allocation decisions happen during the StartImage process. If an
image expects memory to be accessible that isn't part of the image
binary itself, other than the two tables passed into the entry point,
I don't know what that would be. That implicit expectation would need
to become explicit with a binary format change to all EFI modules, to
state their requirements before starting. If an EFI module doesn't
specify its requirements, we assume that the 4GB amount must be
accepted. A bootloader would need to do something similar when parsing
the OS binary header: what memory is needed? Accept that if specified,
or accept the 4GB for safety.
These are very tricky cross-community problems to solve, and I don't
think it's appropriate to lump it in with a simple protocol that we
know is safe and performs well enough.
>
> I hope to discuss more after we get the data.
Do you think that'll happen before mid-December?
Thanks,
-Dionna
On Tue, Nov 8, 2022 at 8:43 AM Dionna Amalie Glaze
<dionnaglaze@google.com> wrote:
>
> >
> > BTW: Is there any data for AMD-SEV?
> >
>
> Earlier this year, I tried to make the lazy accept patches work for
> SEV-SNP, but the boot would always crash in the Qemu try boot kernel
> step IIRC:
>
> https://github.com/AMDESE/ovmf/pull/4
>
> Tom suggested accepting the first 4GiB and I didn't go back to try to
> make the more complicated solution work, since accepting the HOB up to
> the MMIO hole took roughly 30ms, which is well within our boot budget
> given the outsized cost of pinning memory. The launch sequence (start,
> update_data*, finish) for OVMF takes about 190ms, which we can't make
> any lower.
>
> >
> > Thank you
> > Yao, Jiewen
> >
> >
> > > -----Original Message-----
> > > From: Dionna Amalie Glaze <dionnaglaze@google.com>
> > > Sent: Wednesday, November 9, 2022 12:23 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: devel@edk2.groups.io; thomas.lendacky@amd.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>; Aktas,
> > > Erdem <erdemaktas@google.com>; Andrew Fish <afish@apple.com>;
> > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > > behavior
> > >
> > > On Tue, Nov 8, 2022 at 8:09 AM Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > > >
> > > > Hi Tom/Dionna
> > > > I think this patch is addressing
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
> > > >
> > > > For patch 1~3, I am OK for the API which we already agreed, such as
> > > EfiMemoryAcceptProtocol and
> > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
> > > > I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
> > > >
> > > > For patch 4, it changed the behavior to accept all. I don’t believe it should
> > > be 4. It should be the latest one. (please correct me if I am wrong.)
> > > >
> > >
> > > The acceptance protocol (5-6) and the accept-all behavior (4) could be
> > > swapped in order, but they really only make sense as a pair. No client
> > > would use a new protocol to disable the behavior of something that
> > > doesn't happen.
> > > The accept-all behavior (4) must be default before SEV-SNP introduces
> > > unaccepted memory (7).
> > >
> > > > For patch 5~6, I cannot give R-B for
> > > BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does
> > > not address my concern. Please refer to the discussion in the Bugzilla.
> > > >
> > >
> > > I don't think your concern is addressable given the constraints of a
> > > smooth upgrade path. I think the right folks to discuss this with you
> > > from the Intel side are Min Xu and Kirill Shutemov. Kirill will know
> > > what is feasible for the OS to inform the UEFI of in terms of memory
> > > required, and how to not break OSes that don't use the new protocol,
> > > whereas Min should know where that kind of information should be
> > > communicated.
> > >
> > > >
> > > > If you want to split the patch series and submit 1~3 as standalone one,
> > > that will match SEV to TDX on lazy accept part. I believe we may merge
> > > them after we get R-B from right person.
> > > >
> > >
> > > I can do that.
> > >
> > > > Thank you
> > > > Yao, Jiewen
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > > > > Lendacky, Thomas via groups.io
> > > > > Sent: Tuesday, November 8, 2022 10:38 PM
> > > > > To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
> > > > > Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M
> > > <min.m.xu@intel.com>;
> > > > > Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
> > > > > <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
> > > > > behavior
> > > > >
> > > > > On 10/24/22 15:41, Dionna Glaze wrote:
> > > > > > These seven patches build on the lazy-accept patch series
> > > > > >
> > > > > > "Introduce Lazy-accept for Tdx guest"
> > > > >
> > > > > Since the above series was accepted into the EDK2 tree, can this series
> > > > > also be pulled in so that both TDX and SNP can support unaccepted
> > > > > memory
> > > > > in the same release?
> > > > >
> > > > > Thanks,
> > > > > Tom
> > > > >
> > > > > >
> > > > > > by adding SEV-SNP support for the MemoryAccept protocol, and
> > > > > > importantly making eager memory acceptance the default behavior.
> > > > > >
> > > > > > We implement a standardized event group from UEFI v2.9,
> > > > > > EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
> > > > > exactly
> > > > > > the right invocation point for eagerly accepting memory if eager
> > > > > > acceptance has not been disabled.
> > > > > >
> > > > > > To make use of this event group, we add a new driver that is meant to
> > > > > > carry behavior that is needed for all confidential compute
> > > technologies,
> > > > > > not just specific platforms, CocoDxe. In CocoDxe we implement the
> > > > > > default safe behavior to accept all unaccepted memory and invalidate
> > > > > > the MemoryMap on ExitBootServices.
> > > > > >
> > > > > > To allow the OS loader to prevent the eager acceptance, we add a new
> > > > > > protocol, up for standardization,
> > > AcceptAllUnacceptedMemoryProtocol.
> > > > > > This protocol has one interface, Disable(). The OS loader can inform
> > > the
> > > > > > UEFI that it supports the unaccepted memory type and accepts the
> > > > > > responsibility to accept it.
> > > > > >
> > > > > > All images that support unaccepted memory must now locate and call
> > > > > this
> > > > > > new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and
> > > call
> > > > > the Disable
> > > > > > function.
> > > > > >
> > > > > > Changes since 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 (7):
> > > > > > OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
> > > > > > MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
> > > > > > MdeModulePkg: Notify BeforeExitBootServices in
> > > CoreExitBootServices
> > > > > > OvmfPkg: Introduce CocoDxe driver
> > > > > > MdePkg: Introduce the MemoryAcceptance protocol
> > > > > > OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
> > > > > > OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
> > > > > >
> > > > > > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > > > > > MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6
> > > +
> > > > > > MdePkg/Include/Guid/EventGroup.h | 5 +
> > > > > > MdePkg/Include/Protocol/MemoryAcceptance.h | 40
> > > > > +++++
> > > > > > MdePkg/MdePkg.dec | 8 +-
> > > > > > OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> > > > > > OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
> > > > > > OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55
> > > ++++++-
> > > > > > OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
> > > > > > OvmfPkg/CocoDxe/CocoDxe.c | 174
> > > > > ++++++++++++++++++++
> > > > > > OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
> > > > > > OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> > > > > > OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
> > > > > >
> > > > >
> > > OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
> > > > > c | 24 ++-
> > > > > > OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> > > > > > OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> > > > > > OvmfPkg/OvmfPkgX64.dsc | 1 +
> > > > > > OvmfPkg/OvmfPkgX64.fdf | 1 +
> > > > > > OvmfPkg/PlatformPei/AmdSev.c | 5 +
> > > > > > 19 files changed, 366 insertions(+), 9 deletions(-)
> > > > > > create mode 100644
> > > MdePkg/Include/Protocol/MemoryAcceptance.h
> > > > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
> > > > > > create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
> > > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > -Dionna Glaze, PhD (she/her)
>
>
>
> --
> -Dionna Glaze, PhD (she/her)
--
-Dionna Glaze, PhD (she/her)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory behavior
2022-11-08 16:09 ` [edk2-devel] " Yao, Jiewen
2022-11-08 16:22 ` Dionna Glaze
@ 2022-11-08 22:24 ` Lendacky, Thomas
1 sibling, 0 replies; 19+ messages in thread
From: Lendacky, Thomas @ 2022-11-08 22:24 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io, Dionna Glaze
Cc: Ard Biescheuvel, Xu, Min M, Gerd Hoffmann, James Bottomley,
Aktas, Erdem, Andrew Fish, Kinney, Michael D
On 11/8/22 10:09, Yao, Jiewen wrote:
> Hi Tom/Dionna
> I think this patch is addressing https://bugzilla.tianocore.org/show_bug.cgi?id=3987.
>
> For patch 1~3, I am OK for the API which we already agreed, such as EfiMemoryAcceptProtocol and EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES.
> I have given Acked by: Jiewen Yao <Jiewen.yao@intel.com>
>
> For patch 4, it changed the behavior to accept all. I don’t believe it should be 4. It should be the latest one. (please correct me if I am wrong.)
>
> For patch 5~6, I cannot give R-B for BZ3987_MEMORY_ACCEPTANCE_PROTOCAL proposed here because it does not address my concern. Please refer to the discussion in the Bugzilla.
>
>
> If you want to split the patch series and submit 1~3 as standalone one, that will match SEV to TDX on lazy accept part. I believe we may merge them after we get R-B from right person.
Just FYI, without the last half of this series, I believe that TDX will
only be accepting 4GB of memory when using OvmfPkgX64.dsc (see
OvmfPkg/Library/PlatformInitLib/IntelTdx.c), while SNP will be accepting
all memory. This would seem to present a problem for TDX with
OvmfPkgX64.dsc if the OS does not have support for unaccepted memory.
Thanks,
Tom
>
> Thank you
> Yao, Jiewen
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
>> Lendacky, Thomas via groups.io
>> Sent: Tuesday, November 8, 2022 10:38 PM
>> To: Dionna Glaze <dionnaglaze@google.com>; devel@edk2.groups.io
>> Cc: Ard Biescheuvel <ardb@kernel.org>; Xu, Min M <min.m.xu@intel.com>;
>> Gerd Hoffmann <kraxel@redhat.com>; James Bottomley
>> <jejb@linux.ibm.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: Re: [edk2-devel] [PATCH v8 0/7] Add safe unaccepted memory
>> behavior
>>
>> On 10/24/22 15:41, Dionna Glaze wrote:
>>> These seven patches build on the lazy-accept patch series
>>>
>>> "Introduce Lazy-accept for Tdx guest"
>>
>> Since the above series was accepted into the EDK2 tree, can this series
>> also be pulled in so that both TDX and SNP can support unaccepted
>> memory
>> in the same release?
>>
>> Thanks,
>> Tom
>>
>>>
>>> by adding SEV-SNP support for the MemoryAccept protocol, and
>>> importantly making eager memory acceptance the default behavior.
>>>
>>> We implement a standardized event group from UEFI v2.9,
>>> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES, since it provides
>> exactly
>>> the right invocation point for eagerly accepting memory if eager
>>> acceptance has not been disabled.
>>>
>>> To make use of this event group, we add a new driver that is meant to
>>> carry behavior that is needed for all confidential compute technologies,
>>> not just specific platforms, CocoDxe. In CocoDxe we implement the
>>> default safe behavior to accept all unaccepted memory and invalidate
>>> the MemoryMap on ExitBootServices.
>>>
>>> To allow the OS loader to prevent the eager acceptance, we add a new
>>> protocol, up for standardization, AcceptAllUnacceptedMemoryProtocol.
>>> This protocol has one interface, Disable(). The OS loader can inform the
>>> UEFI that it supports the unaccepted memory type and accepts the
>>> responsibility to accept it.
>>>
>>> All images that support unaccepted memory must now locate and call
>> this
>>> new BZ3987_ACCEPT_ALL_UNACCEPTED_MEMORY_PROTOCOL and call
>> the Disable
>>> function.
>>>
>>> Changes since 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 (7):
>>> OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
>>> MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
>>> MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
>>> OvmfPkg: Introduce CocoDxe driver
>>> MdePkg: Introduce the MemoryAcceptance protocol
>>> OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe
>>> OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted
>>>
>>> MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
>>> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 6 +
>>> MdePkg/Include/Guid/EventGroup.h | 5 +
>>> MdePkg/Include/Protocol/MemoryAcceptance.h | 40
>> +++++
>>> MdePkg/MdePkg.dec | 8 +-
>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
>>> OvmfPkg/AmdSev/AmdSevX64.fdf | 1 +
>>> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 55 ++++++-
>>> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 3 +
>>> OvmfPkg/CocoDxe/CocoDxe.c | 174
>> ++++++++++++++++++++
>>> OvmfPkg/CocoDxe/CocoDxe.inf | 46 ++++++
>>> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
>>> OvmfPkg/IntelTdx/IntelTdxX64.fdf | 1 +
>>>
>> OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.
>> c | 24 ++-
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>>> OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
>>> OvmfPkg/OvmfPkgX64.dsc | 1 +
>>> OvmfPkg/OvmfPkgX64.fdf | 1 +
>>> OvmfPkg/PlatformPei/AmdSev.c | 5 +
>>> 19 files changed, 366 insertions(+), 9 deletions(-)
>>> create mode 100644 MdePkg/Include/Protocol/MemoryAcceptance.h
>>> create mode 100644 OvmfPkg/CocoDxe/CocoDxe.c
>>> create mode 100644 OvmfPkg/CocoDxe/CocoDxe.inf
>>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-11-08 22:24 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-24 20:41 [PATCH v8 0/7] Add safe unaccepted memory behavior Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 1/7] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe Dionna Glaze
2022-11-08 16:01 ` Yao, Jiewen
2022-11-08 16:24 ` Lendacky, Thomas
2022-10-24 20:41 ` [PATCH v8 2/7] MdePkg: Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID Dionna Glaze
2022-11-08 16:02 ` Yao, Jiewen
2022-10-24 20:41 ` [PATCH v8 3/7] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices Dionna Glaze
2022-11-08 16:01 ` Yao, Jiewen
2022-10-24 20:41 ` [PATCH v8 4/7] OvmfPkg: Introduce CocoDxe driver Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 5/7] MdePkg: Introduce the MemoryAcceptance protocol Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 6/7] OvmfPkg: Implement AcceptAllUnacceptedMemory in CocoDxe Dionna Glaze
2022-10-24 20:41 ` [PATCH v8 7/7] OvmfPkg/PlatformPei: SEV-SNP make >=4GB unaccepted Dionna Glaze
2022-11-08 14:37 ` [PATCH v8 0/7] Add safe unaccepted memory behavior Lendacky, Thomas
2022-11-08 16:09 ` [edk2-devel] " Yao, Jiewen
2022-11-08 16:22 ` Dionna Glaze
2022-11-08 16:36 ` Yao, Jiewen
2022-11-08 16:43 ` Dionna Glaze
2022-11-08 18:44 ` Dionna Glaze
2022-11-08 22:24 ` Lendacky, Thomas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox