On 06/29/15 09:42, Tian, Hot wrote: > I think you'd better do more comments clean up, e.g.: > AcpiS3Save.h (maybe we can totally remove this .h): This is an > implementation of the ACPI S3 Save protocol. This is defined in S3 boot path > specification 0.9. > AcpiS3Save.c: This is an implementation of the ACPI S3 Save protocol. This > is defined in S3 boot path specification 0.9. > Comments for InstallAcpiS3Save(): The function is the driver > Entry point which will produce AcpiS3SaveProtocol. > AcpiS3SaveDxe.uni: #string STR_MODULE_ABSTRACT #language en-US > "Installs ACPI S3 Save protocol to prepare S3 boot data" > #string STR_MODULE_DESCRIPTION #language > en-US "AcpiS3Save module installs ACPI S3 Save protocol to prepare S3 boot > data." > etc.
Will do, thanks. > > Thanks, > Hot > > -----Original Message----- > From: Yao, Jiewen [mailto:jiewen....@intel.com] > Sent: Saturday, June 27, 2015 19:56 > To: Laszlo Ersek; edk2-devel@lists.sourceforge.net > Cc: He, Tim; Fan, Jeff > Subject: Re: [edk2] [PATCH 8/9] IntelFrameworkModulePkg: AcpiS3SaveDxe: drop > EFI_ACPI_S3_SAVE_PROTOCOL > > Reviewed by: Yao, Jiewen <jiewen....@intel.com> > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Saturday, June 27, 2015 9:08 AM > To: edk2-devel@lists.sourceforge.net > Cc: Yao, Jiewen; Fan, Jeff; Wei, David; He, Tim > Subject: [PATCH 8/9] IntelFrameworkModulePkg: AcpiS3SaveDxe: drop > EFI_ACPI_S3_SAVE_PROTOCOL > > At this point, nothing in the edk2 tree calls EFI_ACPI_S3_SAVE_PROTOCOL > member functions; simplify the code by dropping this protocol interface. > > Cc: Yao Jiewen <jiewen....@intel.com> > Cc: Jeff Fan <jeff....@intel.com> > Cc: David Wei <david....@intel.com> > Cc: Tim He <tim...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf | 4 > +- > IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.h | 28 > +------ > IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.c | 78 > +------------------- > 3 files changed, 6 insertions(+), 104 deletions(-) > > diff --git > a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf > b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf > index e5fb92e..c6fd1dc 100644 > --- a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf > +++ b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe > +++ .inf > @@ -1,5 +1,5 @@ > ## @file > -# AcpiS3Save module installs ACPI S3 Save protocol to prepare S3 boot data. > +# AcpiS3Save module installs EndOfDxe callback to prepare S3 boot data. > # > # Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR> # > @@ -63,14 +63,12 @@ [Guids] > gEfiEndOfDxeEventGroupGuid ## CONSUMES ## Event > > [Protocols] > - gEfiAcpiS3SaveProtocolGuid ## PRODUCES > gFrameworkEfiMpServiceProtocolGuid ## SOMETIMES_CONSUMES > ## NOTIFY > ## SOMETIMES_CONSUMES > gEdkiiVariableLockProtocolGuid > > [FeaturePcd] > - gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPlatformCsmSupport > ## CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdFrameworkCompatibilitySupport > ## CONSUMES > gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode > ## CONSUMES > > diff --git > a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.h > b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.h > index 65974a3..6d8a9bb 100644 > --- a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.h > +++ b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.h > @@ -19,41 +19,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #define _ACPI_S3_SAVE_H_ > > /** > - Gets the buffer of legacy memory below 1 MB > - This function is to get the buffer in legacy memory below 1MB that is > required during S3 resume. > - > - @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance. > - @param Size The returned size of legacy memory below 1 MB. > - > - @retval EFI_SUCCESS Size is successfully returned. > - @retval EFI_INVALID_PARAMETER The pointer Size is NULL. > - > -**/ > -EFI_STATUS > -EFIAPI > -LegacyGetS3MemorySize ( > - IN EFI_ACPI_S3_SAVE_PROTOCOL * This, > - OUT UINTN * Size > - ); > - > -/** > Prepares all information that is needed in the S3 resume boot path. > > Allocate the resources or prepare informations and save in ACPI variable > set for S3 resume boot path > > - @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL > instance. > - @param LegacyMemoryAddress The base address of legacy memory. > - > - @retval EFI_NOT_FOUND Some necessary information cannot be found. > @retval EFI_SUCCESS All information was saved successfully. > - @retval EFI_OUT_OF_RESOURCES Resources were insufficient to save all the > information. > - @retval EFI_INVALID_PARAMETER The memory range is not located below 1 MB. > - > **/ > EFI_STATUS > EFIAPI > S3Ready ( > - IN EFI_ACPI_S3_SAVE_PROTOCOL *This, > - IN VOID *LegacyMemoryAddress > + VOID > ); > #endif > diff --git > a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.c > b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.c > index e0fa866..3d7d3cd 100644 > --- a/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.c > +++ b/IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3Save.c > @@ -29,7 +29,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include <Guid/AcpiS3Context.h> > #include <Guid/Acpi.h> > #include <Guid/EventGroup.h> > -#include <Protocol/AcpiS3Save.h> > #include <IndustryStandard/Acpi.h> > > #include "AcpiS3Save.h" > @@ -52,13 +51,6 @@ S3ReadyThunkPlatform ( > IN ACPI_S3_CONTEXT *AcpiS3Context > ); > > -UINTN mLegacyRegionSize; > - > -EFI_ACPI_S3_SAVE_PROTOCOL mS3Save = { > - LegacyGetS3MemorySize, > - S3Ready, > -}; > - > EFI_GUID mAcpiS3IdtrProfileGuid = { > 0xdea652b0, 0xd587, 0x4c54, { 0xb5, 0xb4, 0xc6, 0x82, 0xe7, 0xa0, 0xaa, > 0x3d } }; @@ -407,52 +399,16 @@ S3CreateIdentityMappingPageTables ( } > > /** > - Gets the buffer of legacy memory below 1 MB > - This function is to get the buffer in legacy memory below 1MB that is > required during S3 resume. > - > - @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL instance. > - @param Size The returned size of legacy memory below 1 MB. > - > - @retval EFI_SUCCESS Size is successfully returned. > - @retval EFI_INVALID_PARAMETER The pointer Size is NULL. > - > -**/ > -EFI_STATUS > -EFIAPI > -LegacyGetS3MemorySize ( > - IN EFI_ACPI_S3_SAVE_PROTOCOL *This, > - OUT UINTN *Size > - ) > -{ > - ASSERT (FALSE); > - > - if (Size == NULL) { > - return EFI_INVALID_PARAMETER; > - } > - > - *Size = mLegacyRegionSize; > - return EFI_SUCCESS; > -} > - > -/** > Prepares all information that is needed in the S3 resume boot path. > > Allocate the resources or prepare informations and save in ACPI variable > set for S3 resume boot path > > - @param This A pointer to the EFI_ACPI_S3_SAVE_PROTOCOL > instance. > - @param LegacyMemoryAddress The base address of legacy memory. > - > - @retval EFI_NOT_FOUND Some necessary information cannot be found. > @retval EFI_SUCCESS All information was saved successfully. > - @retval EFI_OUT_OF_RESOURCES Resources were insufficient to save all the > information. > - @retval EFI_INVALID_PARAMETER The memory range is not located below 1 MB. > - > **/ > EFI_STATUS > EFIAPI > S3Ready ( > - IN EFI_ACPI_S3_SAVE_PROTOCOL *This, > - IN VOID *LegacyMemoryAddress > + VOID > ) > { > EFI_STATUS Status; > @@ -464,17 +420,12 @@ S3Ready ( > > DEBUG ((EFI_D_INFO, "S3Ready!\n")); > > - // > - // Platform may invoke AcpiS3Save->S3Save() before ExitPmAuth, because we > need save S3 information there, while BDS ReadyToBoot may invoke it again. > - // So if 2nd S3Save() is triggered later, we need ignore it. > - // > + ASSERT (!AlreadyEntered); > if (AlreadyEntered) { > return EFI_SUCCESS; > } > AlreadyEntered = TRUE; > > - ASSERT (LegacyMemoryAddress == NULL); > - > AcpiS3Context = AllocateMemoryBelow4G (EfiReservedMemoryType, > sizeof(*AcpiS3Context)); > ASSERT (AcpiS3Context != NULL); > AcpiS3ContextBuffer = (EFI_PHYSICAL_ADDRESS)(UINTN)AcpiS3Context; > @@ -567,13 +518,9 @@ OnEndOfDxe ( > EFI_STATUS Status; > > // > - // Our S3Ready() function ignores both of its parameters, and always > - // succeeds. > + // Our S3Ready() function always succeeds. > // > - Status = S3Ready ( > - NULL, // This > - NULL // LegacyMemoryAddress > - ); > + Status = S3Ready (); > ASSERT_EFI_ERROR (Status); > > // > @@ -607,27 +554,10 @@ InstallAcpiS3Save ( > EFI_STATUS Status; > EFI_EVENT EndOfDxeEvent; > > - if (!FeaturePcdGet(PcdPlatformCsmSupport)) { > - // > - // More memory for no CSM tip, because GDT need relocation > - // > - mLegacyRegionSize = 0x250; > - } else { > - mLegacyRegionSize = 0x100; > - } > - > if (FeaturePcdGet(PcdFrameworkCompatibilitySupport)) { > InstallAcpiS3SaveThunk (); > } > > - Status = gBS->InstallProtocolInterface ( > - &ImageHandle, > - &gEfiAcpiS3SaveProtocolGuid, > - EFI_NATIVE_INTERFACE, > - &mS3Save > - ); > - ASSERT_EFI_ERROR (Status); > - > Status = gBS->CreateEventEx ( > EVT_NOTIFY_SIGNAL, > TPL_CALLBACK, > -- > 1.8.3.1 > > > > ------------------------------------------------------------------------------ > 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 > ------------------------------------------------------------------------------ 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