[edk2-devel] [PATCH 0/3] OvmfPkg: Fix handling of nested interrupts
Ard Biesheuvel
ardb at kernel.org
Fri Dec 9 10:27:58 UTC 2022
(cc Gerd)
On Fri, 9 Dec 2022 at 11:20, Michael Brown <mcb30 at ipxe.org> wrote:
>
> Commit 239b50a86 ("OvmfPkg: End timer interrupt later to avoid stack
> overflow under load") deferred the call to SendApicEoi() until after
> the call to RestoreTPL(), in order to avoid the potential stack
> underflow as described in bug 2815.
>
> This has the unwanted side effect that any callbacks invoked as a
> result of RestoreTPL() will now run before the EOI is sent to the
> APIC. Until the callbacks return, no further timer interrupts will be
> raised and the system time value in mEfiSystemTime will remain
> unchanging.
>
> If any callbacks invoked by RestoreTPL() include a timeout loop based
> on mEfiSystemTime (e.g. via SetTimer() and CheckEvent()), this loop
> will never time out and the system will appear to hang.
>
> This situation can be reproduced by e.g. attaching a USB network card
> connected via an xHCI USB controller. The "system poll timer"
> callback to MnpSystemPoll() will eventually cause the following
> sequence of events:
>
> - Hardware timer interrupt occurs
> - TimerInterruptHandler() in LocalApicTimerDxe is called
> - TimerInterruptHandler() calls RaiseTPL(TPL_HIGH_LEVEL)
> - TimerInterruptHandler() calls RestoreTPL() before sending EOI
> - RestoreTPL() triggers a call to MnpSystemPoll()
> - MnpSystemPoll() ends up calling through to XhcExecTransfer() to poll
> the USB device
> - XhcExecTransfer() enters a loop waiting for the transfer to complete
> or time out
> - XhcExecTransfer() uses CheckEvent() to check for the timeout
> condition
> - Since EOI has not been sent, no further timer interrupts will occur
> and so CheckEvent() always returns EFI_NOT_READY and
> XhcExecTransfer() waits indefinitely in its timeout loop
> - User sees a totally stuck system that is unresponsive to any input
> other than a hard reset
>
> This patch series reverts commit 239b50a86 ("OvmfPkg: End timer
> interrupt later to avoid stack overflow under load") and introduces
> instead a new abstraction NestedInterruptTplLib to handle the
> intricacies required to safely allow nested interrupts without
> changing the core UEFI abstractions for RaiseTPL() and RestoreTPL().
>
> The NestedInterruptTplLib could in principle be used by all timer
> interrupt handlers including those used on physical hardware
> platforms. Doing so would allow those platforms to provide a
> guarantee against interrupt-induced stack underflow.
>
> To simplify the EDK2 review process, the current patch series modifies
> only platforms within OvmfPkg, since those are the only platforms
> currently suffering from the regression.
>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Paolo Bonzini <pbonzini at redhat.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2815
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4162
>
> Michael Brown (3):
> OvmfPkg: Send EOI before RestoreTPL() in timer interrupt handlers
> OvmfPkg: Add library to handle TPL from within nested interrupt
> handlers
> OvmfPkg: Use NestedInterruptTplLib in nested interrupt handlers
>
> OvmfPkg/8254TimerDxe/8254Timer.inf | 1 +
> OvmfPkg/8254TimerDxe/Timer.c | 14 +-
> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
> OvmfPkg/CloudHv/CloudHvX64.dsc | 1 +
> .../Include/Library/NestedInterruptTplLib.h | 87 +++++++
> OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
> OvmfPkg/Library/NestedInterruptTplLib/Iret.c | 62 +++++
> OvmfPkg/Library/NestedInterruptTplLib/Iret.h | 19 ++
> .../NestedInterruptTplLib.inf | 35 +++
> OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 216 ++++++++++++++++++
> OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c | 14 +-
> .../LocalApicTimerDxe/LocalApicTimerDxe.inf | 1 +
> OvmfPkg/Microvm/MicrovmX64.dsc | 1 +
> OvmfPkg/OvmfPkg.dec | 4 +
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/OvmfXen.dsc | 1 +
> 18 files changed, 449 insertions(+), 12 deletions(-)
> create mode 100644 OvmfPkg/Include/Library/NestedInterruptTplLib.h
> create mode 100644 OvmfPkg/Library/NestedInterruptTplLib/Iret.c
> create mode 100644 OvmfPkg/Library/NestedInterruptTplLib/Iret.h
> create mode 100644 OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
> create mode 100644 OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
>
> --
> 2.38.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97177): https://edk2.groups.io/g/devel/message/97177
Mute This Topic: https://groups.io/mt/95557693/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list