On 26 June 2015 at 23:52, Laszlo Ersek <ler...@redhat.com> wrote:
> Jeff,
>
> On 06/25/15 11:22, Ard Biesheuvel wrote:
>> The AcpiS3->S3Save() call needs to occur before the end-of-DXE event
>> is signalled. The end-of-DXE event needs to be signalled prior to
>> invoking any UEFI drivers, applications, or connecting consoles.
>>
>> This means the call to S3Save() that occurs in BdsLibBootViaBootOption()
>> violates the ordering constraints, and should be removed. Since it is
>> the responsibility of the platform BDS to signal the end-of-DXE event,
>> it should also perform the AcpiS3->S3Save() call at an appropriate time.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> ---
>> IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 10
>> ----------
>> .../Library/GenericBdsLib/GenericBdsLib.inf | 1 -
>> IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h | 1 -
>> 3 files changed, 12 deletions(-)
>>
>> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>> b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>> index e02a71015edc..4b7eca7bbd34 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
>> //
>> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
>> b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
>> index 5381e331ff8c..5a138a9169b3 100644
>> --- a/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
>> +++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
>> @@ -119,7 +119,6 @@ [Protocols]
>> gEfiLegacyBiosProtocolGuid ## SOMETIMES_CONSUMES
>> gEfiCpuArchProtocolGuid ## CONSUMES
>> gEfiDevicePathProtocolGuid ## CONSUMES
>> - gEfiAcpiS3SaveProtocolGuid ## SOMETIMES_CONSUMES
>> gEfiGraphicsOutputProtocolGuid ## SOMETIMES_CONSUMES
>> gEfiUgaDrawProtocolGuid |gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ##
>> SOMETIMES_CONSUMES
>> gEfiOEMBadgingProtocolGuid ## SOMETIMES_CONSUMES
>> diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
>> b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
>> index c32579bfc577..7201d8a33527 100644
>> --- a/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
>> +++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/InternalBdsLib.h
>> @@ -33,7 +33,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>> EXPRESS OR IMPLIED.
>> #include <Protocol/SimpleNetwork.h>
>> #include <Protocol/FirmwareVolume2.h>
>> #include <Protocol/PciIo.h>
>> -#include <Protocol/AcpiS3Save.h>
>> #include <Protocol/OEMBadging.h>
>> #include <Protocol/GraphicsOutput.h>
>> #include <Protocol/UgaDraw.h>
>>
>
> please do not commit this patch. As discussed earlier, it would break
> OVMF's S3.
>
> I'm working on a series, and I'll include this patch from Ard in it
> (with the Reviewed-by tags that it has collected), in the right position.
>
I agree. There is no urgency to get this patch merged, as far as I am
concerned, so I am totally fine with waiting for Laszlo to repost it
once OVMF is ready for it.
Thanks,
Ard.
------------------------------------------------------------------------------
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
