Philippe: > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Philippe Mathieu-Daudé > Sent: Tuesday, July 30, 2019 1:49 AM > To: Laszlo Ersek <ler...@redhat.com>; devel@edk2.groups.io; Xu, Wei6 > <wei6...@intel.com> > Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming > <liming....@intel.com> > Subject: Re: [edk2-devel][Patch 1/3] MdePkg/UefiDebugLibConOut: Add > destructor to CloseEvent > > On 7/29/19 6:09 PM, Laszlo Ersek wrote: > > On 07/29/19 16:38, Philippe Mathieu-Daudé wrote: > >> Hi, > >> > >> On 7/26/19 5:10 AM, Xu, Wei6 wrote: > >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2012 > >>> > >>> When driver is unloaded, the ExitBootSerivesEvent must be closed at > >>> the same time. Otherwise exception will occur when ExitBootServices. > >>> > >>> Cc: Michael D Kinney <michael.d.kin...@intel.com> > >>> Cc: Liming Gao <liming....@intel.com> > >>> Signed-off-by: Wei6 Xu <wei6...@intel.com> > >>> --- > >>> .../UefiDebugLibConOut/DebugLibConstructor.c | 23 > >>> ++++++++++++++++++++++ > >>> .../UefiDebugLibConOut/UefiDebugLibConOut.inf | 1 + > >>> 2 files changed, 24 insertions(+) > >>> > >>> diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c > b/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c > >>> index 8005370372..ed73f92818 100644 > >>> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c > >>> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c > >>> @@ -73,5 +73,28 @@ DxeDebugLibConstructor( > >>> &mExitBootServicesEvent > >>> ); > >>> > >>> return EFI_SUCCESS; > >>> } > >>> + > >>> +/** > >>> + The destructor closes Exit Boot Services Event. > >>> + > >>> + @param ImageHandle The firmware allocated handle for the EFI image. > >>> + @param SystemTable A pointer to the EFI System Table. > >>> + > >>> + @retval EFI_SUCCESS The destructor always returns EFI_SUCCESS. > >>> + > >>> +**/ > >>> +EFI_STATUS > >>> +EFIAPI > >>> +DxeDebugLibDestructor( > >>> + IN EFI_HANDLE ImageHandle, > >>> + IN EFI_SYSTEM_TABLE *SystemTable > >>> + ) > >>> +{ > >>> + if (mExitBootServicesEvent != NULL) { > >>> + SystemTable->BootServices->CloseEvent (mExitBootServicesEvent); > >>> + } > >> > >> Is it OK to let mDebugST (pointer to SystemTable) initialized? > > > > Yes, ignoring "mDebugST" in this function should be. > > > > "mDebugST" is a global variable (= an object with file scope and static > > storage duration) defined in > > "MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c". > > > > The library instance (for which the destructor function is being added) > > is linked into driver and application modules. The destructor function > > is invoked whenever the driver or application is about to be unloaded > > from memory. As part of the unloading, the ExitBootServices() > > notification function -- which was automatically registered via the > > constructor function when the driver or application started up -- must > > be de-registered, otherwise the platform firmware will be left with a > > dangling callback pointer. That's what the CloseEvent() above takes care > > of. But it's OK to ignore "mDebugST" altogether, as the memory that > > "mDebugST" lives inside is about to be reclaimed by the platform > > firmware anyway. > > > > The library destructor is invoked: > > - when a UEFI application exits (with success or failure), by returning > > from its entry point function, or by calling the Exit() boot service > > > > - when a UEFI driver exits with failure (hence it will be unloaded > > automatically) > > > > - when a UEFI driver exited with success (hence staying resident), and > > it supports unloading, and now another agent is unloading it with the > > UnloadImage() boot service (such as the UEFI shell with the "unload" > > command, IIRC). > > Thanks Laszlo for the explanations! > > I noticed (late) this series is already pushed (commits > e92bdcb3ecbf..28bc6992400) so it doesn't require review anymore.
Sorry, I forget to send the mail to say I have pushed these serials. These changes are clear to me. > > BTW Liming the author name seems incorrect: Wei6 Xu and Xu, Wei6. > Seemly, the author doesn't configure his name in the consistent way. Author name may be Wei Xu. Thanks Liming > >>> + > >>> + return EFI_SUCCESS; > >>> +} > >>> diff --git a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf > b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf > >>> index 4c279a5bf2..b577d52ac6 100644 > >>> --- a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf > >>> +++ b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf > >>> @@ -20,10 +20,11 @@ > >>> MODULE_TYPE = UEFI_DRIVER > >>> VERSION_STRING = 1.0 > >>> LIBRARY_CLASS = DebugLib|DXE_CORE DXE_DRIVER > >>> DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER > >>> > >>> CONSTRUCTOR = DxeDebugLibConstructor > >>> + DESTRUCTOR = DxeDebugLibDestructor > >>> > >>> # > >>> # VALID_ARCHITECTURES = IA32 X64 EBC > >>> # > >>> > >>> > >> > >> > >> > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44576): https://edk2.groups.io/g/devel/message/44576 Mute This Topic: https://groups.io/mt/32605738/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
