comments below
On 06/24/15 21:48, Ard Biesheuvel wrote:
> According to paragraph 5.1.2.1 of the PI spec vol 2 version 1.4, the
> EFI_END_OF_DXE_EVENT_GUID event must be signalled prior to invoking any
> UEFI drivers, applications, or connecting consoles.
>
> This implements the event signalling in a new GenericBdsLib function
> BdsLibSignalEndOfDxeEvent (), and adds the invocation to BdsEntry () right
> before the point where it hands over to PlatformBdsInit().
>
> In order to retain the mandated ordering between saving the ACPI S3 script
> and signalling the EFI_END_OF_DXE_EVENT_GUID event, saving of the S3 script
> is moved to BdsLibSignalEndOfDxeEvent () as well, right before the event
> is signalled.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>
> I am having trouble wrapping my head around all the interdependencies, but I
> do
> think that this may be a feasible approach.
>
> IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h | 14 +++
> IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 70
> ++++++++++--
> IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf | 1 +
> IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h | 1 +
> IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c | 3 +
> 5 files changed, 79 insertions(+), 10 deletions(-)
>
> diff --git a/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
> b/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
> index ecd68a003bbb..86cda6615bf7 100644
> --- a/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
> +++ b/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h
> @@ -1110,5 +1110,19 @@ DisableQuietBoot (
> VOID
> );
>
> +
> +/**
> + Signal the event EFI_END_OF_DXE_EVENT_GUID as specified by the PI spec
> + vol 2 version 1.4 paragraph 5.1.2.1
> +
> + @retval EFI_SUCCESS The event was signalled successfully
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +BdsLibSignalEndOfDxeEvent (
> + VOID
> + );
> +
> #endif
>
> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
> b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
> index e02a71015edc..d84b0c1a453b 100644
> --- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
> +++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
> @@ -2233,7 +2233,6 @@ BdsLibBootViaBootOption (
> EFI_DEVICE_PATH_PROTOCOL *FilePath;
> EFI_LOADED_IMAGE_PROTOCOL *ImageInfo;
> EFI_DEVICE_PATH_PROTOCOL *WorkingDevicePath;
> - EFI_ACPI_S3_SAVE_PROTOCOL *AcpiS3Save;
> LIST_ENTRY TempBootLists;
> EFI_BOOT_LOGO_PROTOCOL *BootLogo;
>
> @@ -2241,15 +2240,6 @@ BdsLibBootViaBootOption (
> *ExitData = NULL;
>
> //
> - // Notes: this code can be remove after the s3 script table
> - // hook on the event EVT_SIGNAL_READY_TO_BOOT or
> - // EVT_SIGNAL_LEGACY_BOOT
> - //
> - Status = gBS->LocateProtocol (&gEfiAcpiS3SaveProtocolGuid, NULL, (VOID **)
> &AcpiS3Save);
> - if (!EFI_ERROR (Status)) {
> - AcpiS3Save->S3Save (AcpiS3Save, NULL);
> - }
> - //
> // If it's Device Path that starts with a hard drive path, append it with
> the front part to compose a
> // full device path
> //
> @@ -4363,3 +4353,63 @@ BdsLibUpdateFvFileDevicePath (
> }
> return EFI_NOT_FOUND;
> }
> +
> +/**
> + An empty function to pass error checking of CreateEventEx ().
> +
> + @param Event Event whose notification function is being
> invoked.
> + @param Context Pointer to the notification function's
> context,
> + which is implementation-dependent.
> +
> +**/
> +STATIC VOID
> +EFIAPI
> +BdsEmptyCallbackFunction (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> +}
> +
The code below would cause OVMF to order the three steps I described
earlier as (1), (3), (2):
> +/**
> + Signal the event EFI_END_OF_DXE_EVENT_GUID as specified by the PI spec
> + vol 2 version 1.4 paragraph 5.1.2.1
> +
> + @retval EFI_SUCCESS The event was signalled successfully
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +BdsLibSignalEndOfDxeEvent (
> + VOID
> + )
> +{
> + EFI_ACPI_S3_SAVE_PROTOCOL *AcpiS3Save;
> + EFI_EVENT EndOfDxeEvent;
> + EFI_STATUS Status;
> +
> + //
> + // Save the S3 resume script before signalling the end-of-DXE event.
> + //
> + Status = gBS->LocateProtocol (&gEfiAcpiS3SaveProtocolGuid, NULL, (VOID **)
> &AcpiS3Save);
> + if (!EFI_ERROR (Status)) {
> + AcpiS3Save->S3Save (AcpiS3Save, NULL);
> + }
This is step (1). In OVMF's implementation, that step actually includes
(3), as final sub-step.
> +
> + //
> + // Signal EndOfDxe PI Event
> + //
> + Status = gBS->CreateEventEx (
> + EVT_NOTIFY_SIGNAL,
> + TPL_NOTIFY,
> + BdsEmptyCallbackFunction,
> + NULL,
> + &gEfiEndOfDxeEventGroupGuid,
> + &EndOfDxeEvent
> + );
This is (2).
We could fix this up in OVMF of course (the current code is there just
to work around the BDS driver peculiarity you're addressing with this
patch). However, based on Jiewen's email, your original approach might
work after all. Let me follow up on Jiewen's email.
Thanks
Laszlo
> + if (!EFI_ERROR (Status)) {
> + gBS->SignalEvent (EndOfDxeEvent);
> + }
> +
> + return Status;
> +}
> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> index 5381e331ff8c..42ac1185f3ab 100644
> --- a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> +++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> @@ -104,6 +104,7 @@ [Guids]
> ## SOMETIMES_CONSUMES ## Variable:L"LegacyDevOrder"
> gEfiLegacyDevOrderVariableGuid
> gEdkiiStatusCodeDataTypeVariableGuid ## SOMETIMES_CONSUMES ## GUID
> + gEfiEndOfDxeEventGroupGuid
>
> [Protocols]
> gEfiSimpleFileSystemProtocolGuid ## SOMETIMES_CONSUMES
> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
> b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
> index c32579bfc577..658097ec22ea 100644
> --- a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
> +++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
> @@ -52,6 +52,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> #include <Guid/LastEnumLang.h>
> #include <Guid/LegacyDevOrder.h>
> #include <Guid/StatusCodeDataTypeVariable.h>
> +#include <Guid/EventGroup.h>
>
> #include <Library/PrintLib.h>
> #include <Library/DebugLib.h>
> diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
> b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
> index ae7ad2153c51..0413d34fce02 100644
> --- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -623,6 +623,9 @@ BdsEntry (
> InitializeLanguage (TRUE);
> InitializeFrontPage (TRUE);
>
> + Status = BdsLibSignalEndOfDxeEvent ();
> + ASSERT_EFI_ERROR (Status);
> +
> //
> // Do the platform init, can be customized by OEM/IBV
> //
>
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
