Andrew, I like the system time tracking idea, it solves the problem and
requires no new global variables.
Also consider that the Idle event group or CpuSleep may be signaled in other
circumstances beyond WaitForEvent. Who knows maybe someone has implemented
interrupt-driven I/O and wants to wait until the interrupt fires. :)
Eugene
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, February 12, 2013 2:58 PM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] WaitForEvent Idle Race Condition
Especially since WaitForEvent can only be called at TPL_APPLICATION. Pretty
hard to use in an I/O driver.
Tim
From: Andrew Fish [mailto:af...@apple.com]
Sent: Tuesday, February 12, 2013 1:54 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] WaitForEvent Idle Race Condition
On Feb 12, 2013, at 12:45 PM, "Cohen, Eugene"
<eug...@hp.com<mailto:eug...@hp.com>> wrote:
> It seems like a tradeoff of masking interrupts for a very very long time or
> realizing that you may delay an entire timer tick.
The window can probably be narrowed substantially by maintaining a global like
BOOLEAN gEventFired that you clear at the beginning of the loop and check
afterwards with interrupts off. That way we can eliminate the race condition
without holding off interrupts longer than a few cycles.
It may be possible to track EFI System time and skip the CoreSignalEvent
(gIdleLoopEvent); call if time updates during the loop.
The EFI System time could be retrieved via CoreCurrentSystemTime(). This would
remove the need to mask the interrupts and I guess this would be OK.
> The common case for WaitForEvent() would be sitting at the shell prompt
Agreed that this is the most common case but there's no reason events can be
used for IO paths (at least for those protocols that make use of events).
Well generally it is the main thread or application that does the
WaitForEvent().
Thanks,
Andrew Fish
Eugene
From: Andrew Fish [mailto:af...@apple.com<http://apple.com>]
Sent: Tuesday, February 12, 2013 1:37 PM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: Re: [edk2] WaitForEvent Idle Race Condition
Eugene,
I think this is the way it works. CoreCheckEvent() and CoreSignalEven() both
have locks that raise to TPL_HIGH_LEVEL. But we never signal (call an events
Notify Function) an event at TPL_HIGH_LEVEL.
What if NumberOfEvents > 1 and the 1st event is checked, and then a timer tick
happens that makes the 1st event complete? So your ">> what if a timer tick
happens here?? <<" window is quite large. It seems like a tradeoff of masking
interrupts for a very very long time or realizing that you may delay an entire
timer tick. Given the point is to save power waiting a little extra time helps
save power. There will always be a tradeoff between saving power and
performance.
You could chose not to implement the gIdleLoopEventGuid for your platform if
you are concerned about high performance in a WaitForEvent(). You could also
crank up your timer period.
The common case for WaitForEvent() would be sitting at the shell prompt (or the
message loop for some GUI UI). You burn a lot of power doing nothing (running C
code that will not accomplish anything until the next tick). Since you are
generally waiting for a human they generally will not notice that you were in a
lower power state for an extra tick. We are boot firmware and not an RTOS after
all ,and we have a signal model, not threads.
Andrew Fish
https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2/MdeModulePkg/Core/Dxe/Event/Event.c
On Feb 12, 2013, at 11:05 AM, "Cohen, Eugene"
<eug...@hp.com<mailto:eug...@hp.com>> wrote:
It appears that there is a race condition in WaitForEvent... after all events
are checked the idle event is signaled. But if a timer tick interrupt comes in
and schedules work (SignalEvent) after the loop is done but before the event is
signaled, we will delay unnecessarily for an additional timer tick interrupt
(due to an HLT or WFI) before the event is recognized. This could have a
performance impact if WaitForEvent is used in a tight loop waiting for IO.
for(;;) {
for(Index = 0; Index < NumberOfEvents; Index++) {
Status = CoreCheckEvent (UserEvents[Index]);
//
// provide index of event that caused problem
//
if (Status != EFI_NOT_READY) {
*UserIndex = Index;
return Status;
}
}
>> what if a timer tick happens here?? <<
//
// Signal the Idle event
//
CoreSignalEvent (gIdleLoopEvent);
}
}
I think the solution is that interrupts must be disabled while deciding if
there is work to do. It looks like ARM, IA32, and X64 architectures have this
exposure.
I researched if it possible to do WFI/HLT with interrupts masked off. On ARM
it is valid to issue a WFI with IRQ and FIQ masked off since it will unblock
with pending (but masked interrupts). On IA it's a bit trickier since HLT will
hang forever if interrupts are masked off. From what I've read (although I
could not find an authoritative statement in the IA documentation), the
solution is that an 'STI; HLT' together will allow the halt to be bypassed if
an interrupt is pending (see
http://lists.freebsd.org/pipermail/freebsd-current/2004-June/029369.html ).
If you have more knowledge about the X64 processor you can do better than Halt
, also called C1 (C0 is running). That CPU specific driver can also set up P
states, basically how much power gets used in C0.
Eugene
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Free Next-Gen Firewall Hardware Offer
Buy your Sophos next-gen firewall before the end March 2013
and get the hardware for free! Learn more.
http://p.sf.net/sfu/sophos-d2d-feb
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
