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. BTW Liming the author name seems incorrect: Wei6 Xu and Xu, Wei6. >>> + >>> + 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 (#44567): https://edk2.groups.io/g/devel/message/44567 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] -=-=-=-=-=-=-=-=-=-=-=-
