On Jan 27, 2014, at 12:31 PM, Kinney, Michael D <michael.d.kin...@intel.com>
wrote:
> Andrew,
>
> Adding an event does not seem like it is required. ExitBootServices() calls
> a function to process all the exit boot services events
> (CoreNotifySignalList()). Why not move REPORT_STATUS_CODE() call so it is
> executed immediately after the list is processed. This means
> REPORT_STATUS_CODE() is processed in the same context as the rest of the
> event notification functions, but is guaranteed to go last so if the
> REPORT_STATUS_CODE() message is seen you know all the events have been
> processed. Proposed now order shown below:
>
Mike,
That would work if all instances of REPORT_STATUS_CODE followed the rules for
an ExitBootServices event. I did not see that requirement in the library class
definition. So the idea would be the exit boot services event in the library
would be required to follow the rules. We could also update the
ReportStatusCode library class definition to place restrictions on the calls so
they are safe to call from an exit boot services event.
It seems Olivier’s library class instance is “following the rules” for the
library class definition, but the DXE Core is making implementation assumptions
that are not part of the library class definition?
Thanks,
Andrew Fish
> EFI_STATUS
> EFIAPI
> CoreExitBootServices (
> IN EFI_HANDLE ImageHandle,
> IN UINTN MapKey
> )
> {
> EFI_STATUS Status;
>
> //
> // Disable Timer
> //
> gTimer->SetTimerPeriod (gTimer, 0);
>
> //
> // Terminate memory services if the MapKey matches
> //
> Status = CoreTerminateMemoryMap (MapKey);
> if (EFI_ERROR (Status)) {
> //
> // Notify other drivers that ExitBootServices fail
> //
> CoreNotifySignalList (&gEventExitBootServicesFailedGuid);
> return Status;
> }
>
> //
> // Notify other drivers that we are exiting boot services.
> //
> CoreNotifySignalList (&gEfiEventExitBootServicesGuid);
>
> //
> // Report that ExitBootServices() has been called
> //
> REPORT_STATUS_CODE (
> EFI_PROGRESS_CODE,
> (EFI_SOFTWARE_EFI_BOOT_SERVICE | EFI_SW_BS_PC_EXIT_BOOT_SERVICES)
> );
>
> //
> // Disable interrupt of Debug timer.
> //
> SaveAndSetDebugTimerInterrupt (FALSE);
>
> //
> // Disable CPU Interrupts
> //
> gCpu->DisableInterrupt (gCpu);
>
> Best regards,
>
> Mike
>
> From: Olivier Martin [mailto:olivier.mar...@arm.com]
> Sent: Monday, January 27, 2014 11:51 AM
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices
> - Interrupt disabling (critical defect)
>
> I was also thinking about the same things – removing REPORT_STATUS_CODE call.
> REPORT_STATUS_CODE invokes BootServices functions (LocateProtocol() and TPL
> functions are examples). And I am not sure it is ‘legal’ at this stage of
> ExitBootServices().
>
> I have not thought about declaring an ExitBootServices event for
> ExitBootServices. But it might be a good fix.
>
>
> From: Andrew Fish [mailto:af...@apple.com]
> Sent: 27 January 2014 19:44
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices
> - Interrupt disabling (critical defect)
>
>
> I’m wondering if we should remove the call to REPORT_STATUS_CODE? I don’t see
> any place in the library class definition that restricts allocating memory.
> Maybe it would make more sense for the ReportStatusCode lib to have an exit
> boot services event?
>
> Thanks,
>
> Andrew Fish
>
> ------------------------------------------------------------------------------
> CenturyLink Cloud: The Leader in Enterprise Cloud Services.
> Learn Why More Businesses Are Choosing CenturyLink Cloud For
> Critical Workloads, Development Environments & Everything In Between.
> Get a Quote or Start a Free Trial Today.
> http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk_______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
CenturyLink Cloud: The Leader in Enterprise Cloud Services.
Learn Why More Businesses Are Choosing CenturyLink Cloud For
Critical Workloads, Development Environments & Everything In Between.
Get a Quote or Start a Free Trial Today.
http://pubads.g.doubleclick.net/gampad/clk?id=119420431&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
