Re: [edk2-devel] [PATCH 2/2] MdeModulePkg/DxeCore: Fix stack overflow issue due to nested interrupts

["Michael Brown" <mcb30@ipxe.org>]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-