On 29/02/2024 19:04, Paolo Bonzini wrote:
On 2/29/24 14:02, Ray Ni wrote:
In the end, it will lower the TPL to TPL_APPLICATION with interrupt
enabled.
However, it's possible that another timer interrupt happens just in
the end
of RestoreTPL() function when TPL is TPL_APPLICATION.
How do non-OVMF platforms solve the issue? Do they just have the same
bug as in https://bugzilla.tianocore.org/show_bug.cgi?id=4162 ?
Yes, they have that bug (or one of the bugs mentioned in that long
discussion, depending on which particular implementation choices have
been made).
The design of NestedInterruptTplLib is that each nested interrupt must
increase the TPL, but if I understand correctly there is a hole here:
//
// Call RestoreTPL() to allow event notifications to be
// dispatched. This will implicitly re-enable interrupts.
//
gBS->RestoreTPL (InterruptedTPL);
//
// Re-disable interrupts after the call to RestoreTPL() to ensure
// that we have exclusive access to the shared state.
//
DisableInterrupts ();
because gBS->RestoreTPL will unconditionally enable interrupts if
InterruptedTPL < TPL_HIGH_LEVEL.
There's no hole there.
Yes, interrupts will be temporarily reenabled, but the whole function of
NestedInterruptTplLib is to safely allow for this window in which
interrupts have been (annoyingly) enabled by RestoreTPL().
If another interrupt *does* occur within that window, the inner
interrupt handler will call NestedInterruptRestoreTPL(), which will take
the code path leading to the "DEFERRAL INVOCATION POINT", and will
therefore *not* call RestoreTPL() within that stack frame. The inner
interrupt returns at TPL_HIGH_LEVEL with interrupts still disabled, and
execution is therefore guaranteed to immediately reach the "DEFERRAL
RETURN POINT" in the outer interrupt handler. The deferred call to
RestoreTPL() is then safely executed in the context of the outer
interrupt handler (i.e. with zero increase in stack usage, hence a
guarantee of no stack overflow).
See the comments in the code for further details - I made them fairly
extensive. :)
If possible, the easiest solution would be to merge
NestedInterruptTplLib into Core DXE.
The question with that approach would be how to cleanly violate the
abstraction layer that separates the timer interrupt handler (existing
in a separate DXE driver executable) from the implementation of
CoreRestoreTplInternal() (existing in core DXE and not exposed via the
boot services table).
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116185): https://edk2.groups.io/g/devel/message/116185
Mute This Topic: https://groups.io/mt/104642317/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
