Laszlo, Does the library destructor not get called? Shouldn't that destructor unregister the protocol notify and leave the callback pointer in DXE core correct?
-Jaben > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Laszlo Ersek > Sent: Wednesday, April 27, 2016 12:21 PM > To: edk2-devel-01 <edk2-de...@ml01.01.org> > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Justen, Jordan L > <jordan.l.jus...@intel.com>; Tian, Feng <feng.t...@intel.com>; Yao, Jiewen > <jiewen....@intel.com>; Zeng, Star <star.z...@intel.com> > Subject: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor > PcdAcpiS3Enable > > In the edk2 tree, there are currently four drivers that consume > PcdAcpiS3Enable: > > > IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.in > f > > MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorD > xe.inf > MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf > MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.inf > > From these, AcpiS3SaveDxe is the only one that isn't also a client of the > S3BootScriptLib class; all the others (BootScriptExecutorDxe, > S3SaveStateDxe, SmmS3SaveState) are clients of the S3BootScriptLib class. > > In turn, the edk2 tree contains only one non-Null instance of the > S3BootScriptLib class: > > MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > > Therefore we can safely state that BootScriptExecutorDxe, S3SaveStateDxe, > and SmmS3SaveState are all linked against PiDxeS3BootScriptLib. > > Now, if PcdAcpiS3Enable is FALSE when either of BootScriptExecutorDxe, > SmmS3SaveState, or SmmS3SaveState is dispatched, then the following > happens: > > - The constructor of PiDxeS3BootScriptLib, function > S3BootScriptLibInitialize(), registers a protocol installation callback > for gEfiDxeSmmReadyToLockProtocolGuid. Namely, the function > S3BootScriptEventCallBack(). > > - The driver immediately exits with EFI_UNSUPPORTED from its entry point > function, upon seeing PcdAcpiS3Enable == FALSE. (See commits > 800c02fbe2da6, 125e093876414, and d2d38610603f6.) > > - This leaves a dangling callback pointer in the DXE core. > > - When Platform BDS installs gEfiDxeSmmReadyToLockProtocolGuid (which is > a > valid thing to do for locking down SMM, even in the absence of S3 > support!), things blow up. > > Fix this issue by returning immediately from S3BootScriptLibInitialize() > if PcdAcpiS3Enable is FALSE -- it is useless to initialize the library > instance if the containing driver module exits first thing in its entry > point. > > Cc: Feng Tian <feng.t...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 + > MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git > a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > index 6b7540a5eab9..4e0919ea2c79 100644 > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf > @@ -67,7 +67,8 @@ [Pcd] > ## SOMETIMES_PRODUCES > gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateDataPtr > ## CONSUMES > ## SOMETIMES_PRODUCES > > gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPtr > > gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePa > geNumber ## CONSUMES > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable > ## > CONSUMES > > diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > index e7d2a2492e84..d954503f864d 100644 > --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c > @@ -434,12 +434,16 @@ S3BootScriptLibInitialize ( > EFI_SMM_BASE2_PROTOCOL *SmmBase2; > BOOLEAN InSmm; > EFI_SMM_SYSTEM_TABLE2 *Smst; > EFI_PHYSICAL_ADDRESS Buffer; > EFI_EVENT Event; > > + if (!PcdGetBool (PcdAcpiS3Enable)) { > + return RETURN_SUCCESS; > + } > + > S3TablePtr = > (SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePriv > ateDataPtr); > // > // The Boot script private data is not be initialized. create it > // > if (S3TablePtr == 0) { > Buffer = SIZE_4GB - 1; > -- > 1.8.3.1 > > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
