Martin:
I go through code. But, I don't find which code trig it. Could you highlight
it?
ExitBootServices() REPORT_STATUS_CODE() --> ReportStatusCode() -->
InternalReportStatusCode() --> (*mReportStatusCode) (Type, Value, Instance,
(EFI_GUID *)CallerId, Data);
IntelFrameworkModulePkg StatusCodeRuntimeDxe and MdeModulePkg
ReportStatusCodeRouterRuntimeDxe driver ReportStatusCode () service doesn't
call RestoreTpl() on this path.
Thanks
Liming
From: Olivier Martin [mailto:olivier.mar...@arm.com]
Sent: Tuesday, February 11, 2014 5:51 PM
To: Gao, Liming
Cc: edk2-devel@lists.sourceforge.net
Subject: RE: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices -
Interrupt disabling (critical defect)
It is a real issue. And I use 'common' ReportStatusCodeLib implementations - so
you should have the same issue:
ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiReportStatusCodeLib.inf
ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
ReportStatusCodeLib|IntelFrameworkModulePkg/Library/DxeReportStatusCodeLibFramework/DxeReportStatusCodeLib.inf
To trigger it, use the IntelFrameworkModulePkg implementation and check the
status of your interrupt after ExitBootServices.
From: Gao, Liming [mailto:liming....@intel.com]
Sent: 11 February 2014 05:00
To: Olivier Martin
Subject: RE: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices -
Interrupt disabling (critical defect)
Martin:
Is it a real issue or potential issue? If it is a real issue, how to trig it?
From EDKII MdeModulePkg implementation, I don't think this issue can happen. Is
it trigged by your ReportStatusCodeLib instance or ReportStatusCodeHandler
implementation?
Thanks
Liming
From: Gao, Liming [mailto:liming....@intel.com]
Sent: Tuesday, January 28, 2014 6:33 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices -
Interrupt disabling (critical defect)
Hi,
This REPORT_STATUS_CODE() message will be handled by
FirmwarePerformanceDataTableDxe StatusCodeHandler to record ACPI5.0 FPDT table
ExitBootServicesExit field. Per ACPI spec, ExitBootServicesExit is Timer value
logged at the point just prior to the OS loader gaining control back from the
ExitBootServices function for UEFI compatible firmware. So, we place it as late
as possible in DxeCore ExitBootServices() implementation.
I review EDKII MdeModulePkg ReportStatusCodeLib instance,
ReportStatusCodeRouter and ReportStatusCodeHandler. For this
REPORT_STATUS_CODE(), those implementation don't call any boot services. So, I
guess your ReportStatusCodeLib instance or ReportStatusCodeHandler
implementation don't aware it and cause this issue. Right?
For this issue, I agree to enhance DxeCore to handle the different cases.
Meanwhile, I also suggest you update your ReportStatusCodeLib instance.
Thanks
Liming
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
Sent: Tuesday, January 28, 2014 5:45 AM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices -
Interrupt disabling (critical defect)
Andrew,
I agreed that there is an issue in the implementation of ExitBootServices()
itself. The biggest issue is using CPU AP to disable interrupts and then
continuing to potentially use UEFI Services. That puts the Raise/Restore TPL
services into an inconsistent state. I am not in favor of changing the
execution assumptions for the ReportStatusCodeLib at this time. I would prefer
to fix the DXE Core.
My first recommendation was to raise TPL to TPL_HIGH_LEVEL before disabling
interrupts with CPU AP. Moving the REPORT_STATUS_CODE() call point increases
the likelihood that the message will be produced. Maybe both changes should be
done. If the goal is to get a REPORT_STATUS_CODE() message out when
ExitBootServices() is called, then maybe we should have 2 calls to
REPORT_STATUS_CODE(). One on entry to ExitBootSerevices() before any system
state changes are made. And one from TPL_HIGH_LEVEL after all ExitBootServices
events have been processed. Depending on attributes of the specific
ReportStatusCodeLib implementation, the 2nd messages may or may not make it
out. It is legal to call REPORT_STATUS_CODE() at any TPL level. Some
implementations may not be able to honor the request if they are not compatible
with TPL level they are called at.
Best regards,
Mike
From: Andrew Fish [mailto:af...@apple.com]
Sent: Monday, January 27, 2014 1:05 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] [PATCH] MdeModulePkg/DxeCore: Fixed CoreExitBootServices -
Interrupt disabling (critical defect)
On Jan 27, 2014, at 12:31 PM, Kinney, Michael D
<michael.d.kin...@intel.com<mailto: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<mailto: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<mailto: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<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered
in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England & Wales, Company No: 2548782
------------------------------------------------------------------------------
Android apps run on BlackBerry 10
Introducing the new BlackBerry 10.2.1 Runtime for Android apps.
Now with support for Jelly Bean, Bluetooth, Mapview and more.
Get your Android app in front of a whole new audience. Start now.
http://pubads.g.doubleclick.net/gampad/clk?id=124407151&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
