[PATCH v2 0/4] SEV-SNP accepted memory and BeforeExitBootServices

[PATCH v2 0/4] SEV-SNP accepted memory and BeforeExitBootServices

[Dionna Glaze]

This is the first half of the patch series

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

These patches add SEV-SNP support for the MemoryAccept protocol, and
implement an already standardized mechanism for performing any actions
just before terminating the memory map.

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 (i.e., unaccepted memory is enabled).

The use of the BeforeExitBootServices addition will come in the second
half of this series.

Changes since v1:
  * Added EFI_EVENT_GROUP_AFTER_READY_TO_BOOT and interpretation.

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

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

Dionna Glaze (4):
  OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe
  MdePkg: Add event groups for boot events
  MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices
  MdePkg: Signal AfterReadyToBoot after ReadyToBoot

 MdeModulePkg/Core/Dxe/DxeMain.inf                                  |  1 +
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c                            |  6 +++
 MdePkg/Include/Guid/EventGroup.h                                   | 10 ++++
 MdePkg/Include/Library/UefiLib.h                                   |  2 +
 MdePkg/Library/UefiLib/UefiLib.inf                                 |  1 +
 MdePkg/Library/UefiLib/UefiNotTiano.c                              | 18 +++++++
 MdePkg/MdePkg.dec                                                  |  8 ++-
 OvmfPkg/AmdSevDxe/AmdSevDxe.c                                      | 55 ++++++++++++++++++--
 OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                    |  3 ++
 OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c | 24 +++++++--
 10 files changed, 119 insertions(+), 9 deletions(-)

-- 
2.38.1.431.g37b22c650d-goog

[PATCH v2 1/4] OvmfPkg: Realize EfiMemoryAcceptProtocol in AmdSevDxe

[Dionna Glaze]

From: Sophia Wolf <phiawolf@google.com>

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

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

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>

Reviewed-by: 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.1.431.g37b22c650d-goog

[PATCH v2 2/4] MdePkg: Add event groups for boot events

[Dionna Glaze]

Add EFI_EVENT_BEFORE_EXIT_BOOT_SERVICES_GUID
Add EFI_EVENT_GROUP_AFTER_READY_TO_BOOT

Both 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 | 10 ++++++++++
 MdePkg/MdePkg.dec                |  8 +++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Guid/EventGroup.h b/MdePkg/Include/Guid/EventGroup.h
index 063d1f7157..821b728e59 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 } }
 
@@ -29,6 +34,11 @@ extern EFI_GUID  gEfiEventMemoryMapChangeGuid;
 
 extern EFI_GUID  gEfiEventReadyToBootGuid;
 
+#define EFI_EVENT_GROUP_AFTER_READY_TO_BOOT \
+ { 0x3a2a00ad, 0x98b9, 0x4cdf, { 0xa4, 0x78, 0x70, 0x27, 0x77, 0xf1, 0xc1, 0xb } }
+
+extern EFI_GUID  gEfiEventAfterReadyToBootGuid;
+
 #define EFI_EVENT_GROUP_DXE_DISPATCH_GUID \
   { 0x7081e22f, 0xcac6, 0x4053, { 0x94, 0x68, 0x67, 0x57, 0x82, 0xcf, 0x88, 0xe5 }}
 
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 6b6bfbec29..c4ccec935a 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -408,11 +408,17 @@
   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 }}
 
+  ## Include/Guid/EventGroup.h
+  gEfiEventAfterReadyToBootGuid = { 0x3A2A00AD, 0x98B9, 0x4CDF, { 0xA4, 0x78, 0x70, 0x27, 0x77, 0xF1, 0xC1, 0xB }}
+
   ## Include/Protocol/DebugPort.h
   gEfiDebugPortVariableGuid      = { 0xEBA4E8D2, 0x3858, 0x41EC, { 0xA2, 0x81, 0x26, 0x47, 0xBA, 0x96, 0x60, 0xD0 }}
 
-- 
2.38.1.431.g37b22c650d-goog

[PATCH v2 3/4] MdeModulePkg: Notify BeforeExitBootServices in CoreExitBootServices

[Dionna Glaze]

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

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

Acked-by: Jiewen Yao <Jiewen.yao@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.1.431.g37b22c650d-goog

[PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Dionna Glaze]

The Uefi v2.9 specification adds this event group in section 3.1.7,
with its GUID defined in the Related Definitions section of
EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.

Cc: "Michael D Kinney" <michael.d.kinney@intel.com>
Cc: Ard Biesheuvel <ardb@kernel.org>
Cc: Gerd Hoffman <kraxel@redhat.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>

Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 MdePkg/Include/Library/UefiLib.h      |  2 ++
 MdePkg/Library/UefiLib/UefiLib.inf    |  1 +
 MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++++++++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index be7da7fdf7..2c3342351e 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
 /**
   Create, Signal, and Close the Ready to Boot event using EfiSignalEventReadyToBoot().
 
+  If successful, then create, signal and close the After Ready to Boot event.
+
   This function abstracts the signaling of the Ready to Boot Event. The Framework moved
   from a proprietary to UEFI 2.0 based mechanism. This library abstracts the caller
   from how this event is created to prevent to code form having to change with the
diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
index 01ed92092d..5c4064eafa 100644
--- a/MdePkg/Library/UefiLib/UefiLib.inf
+++ b/MdePkg/Library/UefiLib/UefiLib.inf
@@ -60,6 +60,7 @@
   gEfiGlobalVariableGuid                        ## SOMETIMES_CONSUMES  ## Variable
   gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
   gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
+  gEfiEventAfterReadyToBootGuid                 ## SOMETIMES_CONSUMES  ## Event
 
 [Protocols]
   gEfiDriverBindingProtocolGuid                   ## SOMETIMES_PRODUCES
diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c b/MdePkg/Library/UefiLib/UefiNotTiano.c
index d84e91fd01..04fe42f9f7 100644
--- a/MdePkg/Library/UefiLib/UefiNotTiano.c
+++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
@@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
 /**
   Create, Signal, and Close the Ready to Boot event using EfiSignalEventReadyToBoot().
 
+  If successful, then create, signal and close the After Ready to Boot event.
+  
   This function abstracts the signaling of the Ready to Boot Event. The Framework moved
   from a proprietary to UEFI 2.0 based mechanism. This library abstracts the caller
   from how this event is created to prevent to code form having to change with the
@@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
 {
   EFI_STATUS  Status;
   EFI_EVENT   ReadyToBootEvent;
+  EFI_EVENT   AfterReadyToBootEvent;
 
   Status = EfiCreateEventReadyToBoot (&ReadyToBootEvent);
   if (!EFI_ERROR (Status)) {
     gBS->SignalEvent (ReadyToBootEvent);
     gBS->CloseEvent (ReadyToBootEvent);
+    return;
+  }
+
+  /* Then immediately signal the after ready to boot event group */
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  EfiEventEmptyFunction,
+                  NULL,
+                  &gEfiEventAfterReadyToBootGuid,
+                  AfterReadyToBootEvent
+                  );
+  if (!EFI_ERROR(Status)) {
+    gBS->SignalEvent (AfterReadyToBootEvent);
+    gBS->CloseEvent (AfterReadyToBootEvent);
   }
 }
 
-- 
2.38.1.431.g37b22c650d-goog

Re: [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Michael D Kinney]

Hi Dionna,

I gave feedback about After Exit Boot Services event.

Why is an After Ready To Boot signal now part of this series?

Mike

> -----Original Message-----
> From: Dionna Glaze <dionnaglaze@google.com>
> Sent: Tuesday, November 8, 2022 1:16 PM
> To: devel@edk2.groups.io
> Cc: Dionna Glaze <dionnaglaze@google.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Ard Biesheuvel <ardb@kernel.org>;
> Gerd Hoffman <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
> 
> The Uefi v2.9 specification adds this event group in section 3.1.7,
> with its GUID defined in the Related Definitions section of
> EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.
> 
> Cc: "Michael D Kinney" <michael.d.kinney@intel.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Gerd Hoffman <kraxel@redhat.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
>  MdePkg/Include/Library/UefiLib.h      |  2 ++
>  MdePkg/Library/UefiLib/UefiLib.inf    |  1 +
>  MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++++++++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
> index be7da7fdf7..2c3342351e 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
>  /**
>    Create, Signal, and Close the Ready to Boot event using EfiSignalEventReadyToBoot().
> 
> +  If successful, then create, signal and close the After Ready to Boot event.
> +
>    This function abstracts the signaling of the Ready to Boot Event. The Framework moved
>    from a proprietary to UEFI 2.0 based mechanism. This library abstracts the caller
>    from how this event is created to prevent to code form having to change with the
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf b/MdePkg/Library/UefiLib/UefiLib.inf
> index 01ed92092d..5c4064eafa 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -60,6 +60,7 @@
>    gEfiGlobalVariableGuid                        ## SOMETIMES_CONSUMES  ## Variable
>    gEfiAcpi20TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
>    gEfiAcpi10TableGuid                           ## SOMETIMES_CONSUMES  ## SystemTable
> +  gEfiEventAfterReadyToBootGuid                 ## SOMETIMES_CONSUMES  ## Event
> 
>  [Protocols]
>    gEfiDriverBindingProtocolGuid                   ## SOMETIMES_PRODUCES
> diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c b/MdePkg/Library/UefiLib/UefiNotTiano.c
> index d84e91fd01..04fe42f9f7 100644
> --- a/MdePkg/Library/UefiLib/UefiNotTiano.c
> +++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
> @@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
>  /**
>    Create, Signal, and Close the Ready to Boot event using EfiSignalEventReadyToBoot().
> 
> +  If successful, then create, signal and close the After Ready to Boot event.
> +
>    This function abstracts the signaling of the Ready to Boot Event. The Framework moved
>    from a proprietary to UEFI 2.0 based mechanism. This library abstracts the caller
>    from how this event is created to prevent to code form having to change with the
> @@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
>  {
>    EFI_STATUS  Status;
>    EFI_EVENT   ReadyToBootEvent;
> +  EFI_EVENT   AfterReadyToBootEvent;
> 
>    Status = EfiCreateEventReadyToBoot (&ReadyToBootEvent);
>    if (!EFI_ERROR (Status)) {
>      gBS->SignalEvent (ReadyToBootEvent);
>      gBS->CloseEvent (ReadyToBootEvent);
> +    return;
> +  }
> +
> +  /* Then immediately signal the after ready to boot event group */
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  EfiEventEmptyFunction,
> +                  NULL,
> +                  &gEfiEventAfterReadyToBootGuid,
> +                  AfterReadyToBootEvent
> +                  );
> +  if (!EFI_ERROR(Status)) {
> +    gBS->SignalEvent (AfterReadyToBootEvent);
> +    gBS->CloseEvent (AfterReadyToBootEvent);
>    }
>  }
> 
> --
> 2.38.1.431.g37b22c650d-goog

Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Dionna Glaze]

> I gave feedback about After Exit Boot Services event.
>

I saw no such event in the specification

> Why is an After Ready To Boot signal now part of this series?
>

I thought that's the event you meant, since its mantis number is 2042,
whereas before exit boot services is 2043. That fits the "same time"
you mentioned in the other email.


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

Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Michael D Kinney]

Hi Dionna,

My mistake.  I misread the UEFI 2.9 Spec content and thought they were both for exit boot services.

Your patch series with only Before Exit Boot Services is appropriate for your change.

The After Ready To Boot can be in its own patch series.

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dionna Glaze via groups.io
> Sent: Wednesday, November 9, 2022 9:12 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>; Gerd Hoffman <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
> 
> > I gave feedback about After Exit Boot Services event.
> >
> 
> I saw no such event in the specification
> 
> > Why is an After Ready To Boot signal now part of this series?
> >
> 
> I thought that's the event you meant, since its mantis number is 2042,
> whereas before exit boot services is 2043. That fits the "same time"
> you mentioned in the other email.
> 
> 
> --
> -Dionna Glaze, PhD (she/her)
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Dionna Glaze]

Okay, so drop v2 and go back to review v1? Nothing changed other than this.

On Wed, Nov 9, 2022 at 9:35 AM Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> Hi Dionna,
>
> My mistake.  I misread the UEFI 2.9 Spec content and thought they were both for exit boot services.
>
> Your patch series with only Before Exit Boot Services is appropriate for your change.
>
> The After Ready To Boot can be in its own patch series.
>
> Best regards,
>
> Mike
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dionna Glaze via groups.io
> > Sent: Wednesday, November 9, 2022 9:12 AM
> > To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>; Gerd Hoffman <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot
> >
> > > I gave feedback about After Exit Boot Services event.
> > >
> >
> > I saw no such event in the specification
> >
> > > Why is an After Ready To Boot signal now part of this series?
> > >
> >
> > I thought that's the event you meant, since its mantis number is 2042,
> > whereas before exit boot services is 2043. That fits the "same time"
> > you mentioned in the other email.
> >
> >
> > --
> > -Dionna Glaze, PhD (she/her)
> >
> >
> > 
> >
>


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

回复: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[gaoliming]

Dionna:
  I add my comments below. 

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Dionna Glaze
> via groups.io
> 发送时间: 2022年11月9日 5:16
> 收件人: devel@edk2.groups.io
> 抄送: Dionna Glaze <dionnaglaze@google.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Ard Biesheuvel <ardb@kernel.org>; Gerd
> Hoffman <kraxel@redhat.com>; Jiewen Yao <jiewen.yao@intel.com>
> 主题: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after
> ReadyToBoot
> 
> The Uefi v2.9 specification adds this event group in section 3.1.7,
> with its GUID defined in the Related Definitions section of
> EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.
> 
> Cc: "Michael D Kinney" <michael.d.kinney@intel.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Gerd Hoffman <kraxel@redhat.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
>  MdePkg/Include/Library/UefiLib.h      |  2 ++
>  MdePkg/Library/UefiLib/UefiLib.inf    |  1 +
>  MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++++++++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> index be7da7fdf7..2c3342351e 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
>  /**
>    Create, Signal, and Close the Ready to Boot event using
> EfiSignalEventReadyToBoot().
> 
> +  If successful, then create, signal and close the After Ready to Boot event.
> +
>    This function abstracts the signaling of the Ready to Boot Event. The
> Framework moved
>    from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> the caller
>    from how this event is created to prevent to code form having to change
> with the
> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> b/MdePkg/Library/UefiLib/UefiLib.inf
> index 01ed92092d..5c4064eafa 100644
> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> @@ -60,6 +60,7 @@
>    gEfiGlobalVariableGuid                        ##
> SOMETIMES_CONSUMES  ## Variable
>    gEfiAcpi20TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
>    gEfiAcpi10TableGuid                           ##
> SOMETIMES_CONSUMES  ## SystemTable
> +  gEfiEventAfterReadyToBootGuid                 ##
> SOMETIMES_CONSUMES  ## Event
> 
>  [Protocols]
>    gEfiDriverBindingProtocolGuid                   ##
> SOMETIMES_PRODUCES
> diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c
> b/MdePkg/Library/UefiLib/UefiNotTiano.c
> index d84e91fd01..04fe42f9f7 100644
> --- a/MdePkg/Library/UefiLib/UefiNotTiano.c
> +++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
> @@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
>  /**
>    Create, Signal, and Close the Ready to Boot event using
> EfiSignalEventReadyToBoot().
> 
> +  If successful, then create, signal and close the After Ready to Boot event.
> +
>    This function abstracts the signaling of the Ready to Boot Event. The
> Framework moved
>    from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> the caller
>    from how this event is created to prevent to code form having to change
> with the
> @@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
>  {
>    EFI_STATUS  Status;
>    EFI_EVENT   ReadyToBootEvent;
> +  EFI_EVENT   AfterReadyToBootEvent;
> 
>    Status = EfiCreateEventReadyToBoot (&ReadyToBootEvent);
>    if (!EFI_ERROR (Status)) {
>      gBS->SignalEvent (ReadyToBootEvent);
>      gBS->CloseEvent (ReadyToBootEvent);
> +    return;
> +  }
> +
Return should not be here. This means ReadyToBoot event creates successfully and return.
But, the behavior should be ReadyToBoot and AfterReadyToBoot event both trig.

Thanks
Liming
> +  /* Then immediately signal the after ready to boot event group */
> +  Status = gBS->CreateEventEx (
> +                  EVT_NOTIFY_SIGNAL,
> +                  TPL_CALLBACK,
> +                  EfiEventEmptyFunction,
> +                  NULL,
> +                  &gEfiEventAfterReadyToBootGuid,
> +                  AfterReadyToBootEvent
> +                  );
> +  if (!EFI_ERROR(Status)) {
> +    gBS->SignalEvent (AfterReadyToBootEvent);
> +    gBS->CloseEvent (AfterReadyToBootEvent);
>    }
>  }
> 
> --
> 2.38.1.431.g37b22c650d-goog
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after ReadyToBoot

[Dionna Glaze]

On Tue, Dec 6, 2022 at 5:26 PM gaoliming <gaoliming@byosoft.com.cn> wrote:
>
> Dionna:
>   I add my comments below.
>
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Dionna Glaze
> > via groups.io
> > 发送时间: 2022年11月9日 5:16
> > 收件人: devel@edk2.groups.io
> > 抄送: Dionna Glaze <dionnaglaze@google.com>; Michael D Kinney
> > <michael.d.kinney@intel.com>; Ard Biesheuvel <ardb@kernel.org>; Gerd
> > Hoffman <kraxel@redhat.com>; Jiewen Yao <jiewen.yao@intel.com>
> > 主题: [edk2-devel] [PATCH v2 4/4] MdePkg: Signal AfterReadyToBoot after
> > ReadyToBoot
> >
> > The Uefi v2.9 specification adds this event group in section 3.1.7,
> > with its GUID defined in the Related Definitions section of
> > EFI_BOOT_SERVICES.CreateEventEx() in chapter 7.
> >
> > Cc: "Michael D Kinney" <michael.d.kinney@intel.com>
> > Cc: Ard Biesheuvel <ardb@kernel.org>
> > Cc: Gerd Hoffman <kraxel@redhat.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> >
> > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> > ---
> >  MdePkg/Include/Library/UefiLib.h      |  2 ++
> >  MdePkg/Library/UefiLib/UefiLib.inf    |  1 +
> >  MdePkg/Library/UefiLib/UefiNotTiano.c | 18 ++++++++++++++++++
> >  3 files changed, 21 insertions(+)
> >
> > diff --git a/MdePkg/Include/Library/UefiLib.h
> > b/MdePkg/Include/Library/UefiLib.h
> > index be7da7fdf7..2c3342351e 100644
> > --- a/MdePkg/Include/Library/UefiLib.h
> > +++ b/MdePkg/Include/Library/UefiLib.h
> > @@ -890,6 +890,8 @@ UnicodeStringDisplayLength (
> >  /**
> >    Create, Signal, and Close the Ready to Boot event using
> > EfiSignalEventReadyToBoot().
> >
> > +  If successful, then create, signal and close the After Ready to Boot event.
> > +
> >    This function abstracts the signaling of the Ready to Boot Event. The
> > Framework moved
> >    from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> > the caller
> >    from how this event is created to prevent to code form having to change
> > with the
> > diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> > b/MdePkg/Library/UefiLib/UefiLib.inf
> > index 01ed92092d..5c4064eafa 100644
> > --- a/MdePkg/Library/UefiLib/UefiLib.inf
> > +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> > @@ -60,6 +60,7 @@
> >    gEfiGlobalVariableGuid                        ##
> > SOMETIMES_CONSUMES  ## Variable
> >    gEfiAcpi20TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> >    gEfiAcpi10TableGuid                           ##
> > SOMETIMES_CONSUMES  ## SystemTable
> > +  gEfiEventAfterReadyToBootGuid                 ##
> > SOMETIMES_CONSUMES  ## Event
> >
> >  [Protocols]
> >    gEfiDriverBindingProtocolGuid                   ##
> > SOMETIMES_PRODUCES
> > diff --git a/MdePkg/Library/UefiLib/UefiNotTiano.c
> > b/MdePkg/Library/UefiLib/UefiNotTiano.c
> > index d84e91fd01..04fe42f9f7 100644
> > --- a/MdePkg/Library/UefiLib/UefiNotTiano.c
> > +++ b/MdePkg/Library/UefiLib/UefiNotTiano.c
> > @@ -208,6 +208,8 @@ EfiCreateEventReadyToBootEx (
> >  /**
> >    Create, Signal, and Close the Ready to Boot event using
> > EfiSignalEventReadyToBoot().
> >
> > +  If successful, then create, signal and close the After Ready to Boot event.
> > +
> >    This function abstracts the signaling of the Ready to Boot Event. The
> > Framework moved
> >    from a proprietary to UEFI 2.0 based mechanism. This library abstracts
> > the caller
> >    from how this event is created to prevent to code form having to change
> > with the
> > @@ -222,11 +224,27 @@ EfiSignalEventReadyToBoot (
> >  {
> >    EFI_STATUS  Status;
> >    EFI_EVENT   ReadyToBootEvent;
> > +  EFI_EVENT   AfterReadyToBootEvent;
> >
> >    Status = EfiCreateEventReadyToBoot (&ReadyToBootEvent);
> >    if (!EFI_ERROR (Status)) {
> >      gBS->SignalEvent (ReadyToBootEvent);
> >      gBS->CloseEvent (ReadyToBootEvent);
> > +    return;
> > +  }
> > +
> Return should not be here. This means ReadyToBoot event creates successfully and return.
> But, the behavior should be ReadyToBoot and AfterReadyToBoot event both trig.
>

Thanks Gao, given Michael's feedback about this patch not making much
sense in the series that adds before_exit_boot_services, we decided to
go ahead and consider v1 of this series as final. Let's let Robert
Phelps take it from here, since they appear to need it such that
they're in a better position to test their implementation.

> Thanks
> Liming
> > +  /* Then immediately signal the after ready to boot event group */
> > +  Status = gBS->CreateEventEx (
> > +                  EVT_NOTIFY_SIGNAL,
> > +                  TPL_CALLBACK,
> > +                  EfiEventEmptyFunction,
> > +                  NULL,
> > +                  &gEfiEventAfterReadyToBootGuid,
> > +                  AfterReadyToBootEvent
> > +                  );
> > +  if (!EFI_ERROR(Status)) {
> > +    gBS->SignalEvent (AfterReadyToBootEvent);
> > +    gBS->CloseEvent (AfterReadyToBootEvent);
> >    }
> >  }
> >
> > --
> > 2.38.1.431.g37b22c650d-goog
> >
> >
> >
> > 
> >
>
>
>


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