* Tue May 16 2023 Miroslav Rezanina <mrezanin@redhat.com> - 20230301gitf80f052277c8-4
- edk2-OvmfPkg-Clarify-invariants-for-NestedInterruptTplLib.patch [bz#2189136] - edk2-OvmfPkg-Relax-assertion-that-interrupts-do-not-occur.patch [bz#2189136] - Resolves: bz#2189136 (windows 11 installation broken with edk2-20230301gitf80f052277c8-1.el9)
This commit is contained in:
Miroslav Rezanina
parent
a19f633550
commit
f53835584a
@ -0,0 +1,72 @@
|
||||
From d259d959bab00b76e7187e79731201c9a3628d7c Mon Sep 17 00:00:00 2001
|
||||
From: Michael Brown <mcb30@ipxe.org>
|
||||
Date: Tue, 9 May 2023 12:09:30 +0000
|
||||
Subject: [PATCH 1/2] OvmfPkg: Clarify invariants for NestedInterruptTplLib
|
||||
|
||||
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
|
||||
RH-MergeRequest: 35: OvmfPkg: backport NestedInterruptTplLib updates
|
||||
RH-Bugzilla: 2189136
|
||||
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||||
RH-Commit: [1/2] 3f4c655bd43f38f43769c01eadbf96ce578818d0 (kraxel/centos-edk2)
|
||||
|
||||
NestedInterruptTplLib relies on CPU interrupts being disabled to
|
||||
guarantee exclusive (and hence atomic) access to the shared state in
|
||||
IsrState. Nothing in the calling interrupt handler should have
|
||||
re-enabled interrupts before calling NestedInterruptRestoreTPL(), and
|
||||
the loop in NestedInterruptRestoreTPL() itself maintains the invariant
|
||||
that interrupts are disabled at the start of each iteration.
|
||||
|
||||
Add assertions to clarify this invariant, and expand the comments
|
||||
around the calls to RestoreTPL() and DisableInterrupts() to clarify
|
||||
the expectations around enabling and disabling interrupts.
|
||||
|
||||
Signed-off-by: Michael Brown <mcb30@ipxe.org>
|
||||
Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||||
(cherry picked from commit ae0be176a83efebe9a8c13d2124151f7dd13443a)
|
||||
---
|
||||
OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 10 ++++++++--
|
||||
1 file changed, 8 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
index e19d98878e..e921a09c55 100644
|
||||
--- a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
+++ b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
@@ -104,6 +104,7 @@ NestedInterruptRestoreTPL (
|
||||
// defer our call to RestoreTPL() to the in-progress outer instance
|
||||
// of the same interrupt handler.
|
||||
//
|
||||
+ ASSERT (GetInterruptState () == FALSE);
|
||||
if (InterruptedTPL == IsrState->InProgressRestoreTPL) {
|
||||
//
|
||||
// Trigger outer instance of this interrupt handler to perform the
|
||||
@@ -153,6 +154,7 @@ NestedInterruptRestoreTPL (
|
||||
//
|
||||
// Check shared state loop invariants.
|
||||
//
|
||||
+ ASSERT (GetInterruptState () == FALSE);
|
||||
ASSERT (IsrState->InProgressRestoreTPL < InterruptedTPL);
|
||||
ASSERT (IsrState->DeferredRestoreTPL == FALSE);
|
||||
|
||||
@@ -167,13 +169,17 @@ NestedInterruptRestoreTPL (
|
||||
|
||||
//
|
||||
// Call RestoreTPL() to allow event notifications to be
|
||||
- // dispatched. This will implicitly re-enable interrupts.
|
||||
+ // dispatched. This will implicitly re-enable interrupts (if
|
||||
+ // InterruptedTPL is below TPL_HIGH_LEVEL), even though we are
|
||||
+ // still inside the interrupt handler.
|
||||
//
|
||||
gBS->RestoreTPL (InterruptedTPL);
|
||||
|
||||
//
|
||||
// Re-disable interrupts after the call to RestoreTPL() to ensure
|
||||
- // that we have exclusive access to the shared state.
|
||||
+ // that we have exclusive access to the shared state. Interrupts
|
||||
+ // will be re-enabled by the IRET or equivalent instruction when
|
||||
+ // we subsequently return from the interrupt handler.
|
||||
//
|
||||
DisableInterrupts ();
|
||||
|
||||
--
|
||||
2.39.1
|
||||
|
||||
@ -0,0 +1,81 @@
|
||||
From 28c2cf5f8fa5021ff3a6b83fe07dc519084143c0 Mon Sep 17 00:00:00 2001
|
||||
From: Michael Brown <mcb30@ipxe.org>
|
||||
Date: Tue, 9 May 2023 12:09:33 +0000
|
||||
Subject: [PATCH 2/2] OvmfPkg: Relax assertion that interrupts do not occur at
|
||||
TPL_HIGH_LEVEL
|
||||
|
||||
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
|
||||
RH-MergeRequest: 35: OvmfPkg: backport NestedInterruptTplLib updates
|
||||
RH-Bugzilla: 2189136
|
||||
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||||
RH-Commit: [2/2] ffb3cf6132b774295a49d16d4ef35164b61c0de3 (kraxel/centos-edk2)
|
||||
|
||||
At TPL_HIGH_LEVEL, CPU interrupts are disabled (as per the UEFI
|
||||
specification) and so we should never encounter a situation in which
|
||||
an interrupt occurs at TPL_HIGH_LEVEL. The specification also
|
||||
restricts usage of TPL_HIGH_LEVEL to the firmware itself.
|
||||
|
||||
However, nothing actually prevents a UEFI application from calling
|
||||
gBS->RaiseTPL(TPL_HIGH_LEVEL) and then violating the invariant by
|
||||
enabling interrupts via the STI or equivalent instruction. Some
|
||||
versions of the Microsoft Windows bootloader are known to do this.
|
||||
|
||||
NestedInterruptTplLib maintains the invariant that interrupts are
|
||||
disabled at TPL_HIGH_LEVEL (even when performing the dark art of
|
||||
deliberately manipulating the stack so that IRET will return with
|
||||
interrupts still disabled), but does not itself rely on external code
|
||||
maintaining this invariant.
|
||||
|
||||
Relax the assertion that the interrupted TPL is below TPL_HIGH_LEVEL
|
||||
to an error message, to allow UEFI applications such as these versions
|
||||
of the Microsoft Windows bootloader to continue to function.
|
||||
|
||||
Debugged-by: Gerd Hoffmann <kraxel@redhat.com>
|
||||
Debugged-by: Laszlo Ersek <lersek@redhat.com>
|
||||
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=2189136
|
||||
Signed-off-by: Michael Brown <mcb30@ipxe.org>
|
||||
Acked-by: Laszlo Ersek <lersek@redhat.com>
|
||||
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
|
||||
(cherry picked from commit bee67e0c142af6599a85aa7640094816b8a24c4f)
|
||||
---
|
||||
OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 21 ++++++++++++++++++---
|
||||
1 file changed, 18 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
index e921a09c55..d56c12a445 100644
|
||||
--- a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
+++ b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
|
||||
@@ -34,12 +34,27 @@ NestedInterruptRaiseTPL (
|
||||
|
||||
//
|
||||
// Raise TPL and assert that we were called from within an interrupt
|
||||
- // handler (i.e. with TPL below TPL_HIGH_LEVEL but with interrupts
|
||||
- // disabled).
|
||||
+ // handler (i.e. with interrupts already disabled before raising the
|
||||
+ // TPL).
|
||||
//
|
||||
ASSERT (GetInterruptState () == FALSE);
|
||||
InterruptedTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
|
||||
- ASSERT (InterruptedTPL < TPL_HIGH_LEVEL);
|
||||
+
|
||||
+ //
|
||||
+ // At TPL_HIGH_LEVEL, CPU interrupts are disabled (as per the UEFI
|
||||
+ // specification) and so we should never encounter a situation in
|
||||
+ // which InterruptedTPL==TPL_HIGH_LEVEL. The specification also
|
||||
+ // restricts usage of TPL_HIGH_LEVEL to the firmware itself.
|
||||
+ //
|
||||
+ // However, nothing actually prevents a UEFI application from
|
||||
+ // invalidly calling gBS->RaiseTPL(TPL_HIGH_LEVEL) and then
|
||||
+ // violating the invariant by enabling interrupts via the STI or
|
||||
+ // equivalent instruction. Some versions of the Microsoft Windows
|
||||
+ // bootloader are known to do this.
|
||||
+ //
|
||||
+ if (InterruptedTPL >= TPL_HIGH_LEVEL) {
|
||||
+ DEBUG ((DEBUG_ERROR, "ERROR: Interrupts enabled at TPL_HIGH_LEVEL!\n"));
|
||||
+ }
|
||||
|
||||
return InterruptedTPL;
|
||||
}
|
||||
--
|
||||
2.39.1
|
||||
|
||||
12
edk2.spec
12
edk2.spec
@ -18,7 +18,7 @@ ExclusiveArch: x86_64 aarch64
|
||||
|
||||
Name: edk2
|
||||
Version: %{GITDATE}git%{GITCOMMIT}
|
||||
Release: 3%{?dist}
|
||||
Release: 4%{?dist}
|
||||
Summary: UEFI firmware for 64-bit virtual machines
|
||||
License: BSD-2-Clause-Patent and OpenSSL and MIT
|
||||
URL: http://www.tianocore.org
|
||||
@ -76,6 +76,10 @@ Patch0025: 0025-MdePkg-Remove-Itanium-leftover-data-structure-RH-onl.patch
|
||||
Patch0027: 0027-CryptoPkg-OpensslLib-list-RHEL8-specific-OpenSSL-fil.patch
|
||||
Patch0028: 0028-Revert-ArmVirtPkg-ArmVirtQemu-enable-initial-ID-map-.patch
|
||||
Patch0029: 0029-OvmfPkg-disable-dynamic-mmio-window-rhel-only.patch
|
||||
# For bz#2189136 - windows 11 installation broken with edk2-20230301gitf80f052277c8-1.el9
|
||||
Patch30: edk2-OvmfPkg-Clarify-invariants-for-NestedInterruptTplLib.patch
|
||||
# For bz#2189136 - windows 11 installation broken with edk2-20230301gitf80f052277c8-1.el9
|
||||
Patch31: edk2-OvmfPkg-Relax-assertion-that-interrupts-do-not-occur.patch
|
||||
|
||||
|
||||
# python3-devel and libuuid-devel are required for building tools.
|
||||
@ -395,6 +399,12 @@ install -m 0644 \
|
||||
|
||||
|
||||
%changelog
|
||||
* Tue May 16 2023 Miroslav Rezanina <mrezanin@redhat.com> - 20230301gitf80f052277c8-4
|
||||
- edk2-OvmfPkg-Clarify-invariants-for-NestedInterruptTplLib.patch [bz#2189136]
|
||||
- edk2-OvmfPkg-Relax-assertion-that-interrupts-do-not-occur.patch [bz#2189136]
|
||||
- Resolves: bz#2189136
|
||||
(windows 11 installation broken with edk2-20230301gitf80f052277c8-1.el9)
|
||||
|
||||
* Mon May 08 2023 Miroslav Rezanina <mrezanin@redhat.com> - 20230301gitf80f052277c8-3
|
||||
- edk2-add-aarch64-qcow2-images.patch [bz#2186754]
|
||||
- edk2-update-json-files.patch [bz#2186754]
|
||||
|
||||
Loading…
Reference in New Issue
Block a user