On 06/25/15 13:00, Ard Biesheuvel wrote: > Currently, the ArmVirtPkg platforms built with the Intel BDS fail > to signal the end-of-DXE event 'gEfiEndOfDxeEventGroupGuid' when > entering the BDS phase, which results in some loss of functionality, > i.e., variable reclaim in the VariableDxe drivers, and the splitting > of the memory regions that is part of the recently added UEFI 2.5 > properties table feature. > > As discussed on the edk2-devel mailing list here: > > http://thread.gmane.org/gmane.comp.bios.tianocore.devel/16088/focus=16109 > > it is up to the platform BDS to signal that event, since there may be > platform specific ordering constraints with respect to the signalling > of the event that are difficult to honor at the generic level. > > So add the SignalEvent () call to PlatformBdsInit () of ArmVirtPkg's > PlatformBdsLib implementation for the Intel BDS. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > --- > v2: > - use STATIC for empty callback function > - use TPL_CALLBACK priority level > - close EFI_END_OF_DXE_EVENT_GROUP_GUID event group after signalling it > - indentation fixes > > ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c | 36 > ++++++++++++++++++++ > ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf | 1 + > 2 files changed, 37 insertions(+) > > diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c > b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c > index 499cce5dcde6..13830cbe3665 100644 > --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c > +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/IntelBdsPlatform.c > @@ -24,6 +24,7 @@ > #include <Protocol/GraphicsOutput.h> > #include <Protocol/PciIo.h> > #include <Protocol/PciRootBridgeIo.h> > +#include <Guid/EventGroup.h> > > #include "IntelBdsPlatform.h" > > @@ -118,6 +119,23 @@ STATIC PLATFORM_USB_KEYBOARD mUsbKeyboard = { > } > }; > > +/** > + 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 > +EmptyCallbackFunction ( > + IN EFI_EVENT Event, > + IN VOID *Context > + ) > +{ > +} > > // > // BDS Platform Functions > @@ -133,6 +151,24 @@ PlatformBdsInit ( > VOID > ) > { > + EFI_EVENT EndOfDxeEvent; > + EFI_STATUS Status; > + > + // > + // Signal EndOfDxe PI Event > + // > + Status = gBS->CreateEventEx ( > + EVT_NOTIFY_SIGNAL, > + TPL_CALLBACK, > + EmptyCallbackFunction, > + NULL, > + &gEfiEndOfDxeEventGroupGuid, > + &EndOfDxeEvent > + ); > + if (!EFI_ERROR (Status)) { > + gBS->SignalEvent (EndOfDxeEvent); > + gBS->CloseEvent (EndOfDxeEvent); > + } > } > > > diff --git a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf > b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf > index d8f892642c2e..d9982167e81d 100644 > --- a/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf > +++ b/ArmVirtPkg/Library/PlatformIntelBdsLib/PlatformIntelBdsLib.inf > @@ -65,6 +65,7 @@ [Guids] > gEfiFileInfoGuid > gEfiFileSystemInfoGuid > gEfiFileSystemVolumeLabelInfoIdGuid > + gEfiEndOfDxeEventGroupGuid > > [Protocols] > gEfiDevicePathProtocolGuid >
Looks good, but you're going to hate me nonetheless. Because, our exchange has been too quick, and my thoughts about addressing the same in OvmfPkg had not matured enough until I posted my v1 review. Namely, both OvmfPkg and ArmPlatformPkg contain the following snippet at the very end of their respective PlatformBdsPolicyBehavior() functions: BdsLibEnumerateAllBootOption (BootOptionList); SetBootOrderFromQemu (BootOptionList); // // The BootOrder variable may have changed, reload the in-memory list with // it. // BdsLibBuildOptionFromVar (BootOptionList, L"BootOrder"); PlatformBdsEnterFrontPage (GetFrontPageTimeoutFromQemu(), TRUE); The pair (BdsLibEnumerateAllBootOption(), SetBootOrderFromQemu()) massages UEFI non-volatile variables (boot options) quite heavily. The first creates a bunch of new boot options, while the second prunes quite a few of those. (This is necessary because a VM's hardware might change dynamically before every single boot, so everything needs to be enumerated, and then filtered / reordered according to the fw_cfg boot order file, every time.) Since one of your stated goals is "variable reclaim" before booting the OS, I propose (for both ArmVirtPkg and my future OvmfPkg patch(es)) that we signal the event right before the PlatformBdsEnterFrontPage() call -- ie. as the penultimate step of PlatformBdsPolicyBehavior() --, rather than in PlatformBdsInit(). Then the reclaim would cover the variable churn incurred by those two functions I named above. What do you think? Thanks Laszlo ------------------------------------------------------------------------------ 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
