update NestedInterruptTplLib patches

0016-OvmfPkg-Clarify-invariants-for-NestedInterruptTplLib.patch

@@ -0,0 +1,66 @@
+From 51846ff74e3352151f99cfcfbe091c09f3ec8097 Mon Sep 17 00:00:00 2001
+From: Michael Brown <mcb30@ipxe.org>
+Date: Tue, 9 May 2023 12:09:30 +0000
+Subject: [PATCH 16/18] OvmfPkg: Clarify invariants for NestedInterruptTplLib
+
+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 e19d98878eb7..e921a09c5599 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.40.1
+

0016-OvmfPkg-NestedInterruptTplLib-replace-ASSERT-with-a-.patch

@@ -1,40 +0,0 @@
-From 7c65395d2b45e388c6c106cb619ab93c2359f5e4 Mon Sep 17 00:00:00 2001
-From: Gerd Hoffmann <kraxel@redhat.com>
-Date: Thu, 27 Apr 2023 12:14:16 +0200
-Subject: [PATCH 16/17] OvmfPkg/NestedInterruptTplLib: replace ASSERT() with a
- warning logged.
-
-OVMF can't guarantee that the ASSERT() doesn't happen.  Misbehaving
-EFI applications can trigger this.  So log a warning instead and try
-to continue.
-
-Reproducer: Fetch windows 11 22H2 iso image, boot it in qemu with OVMF.
-
-Traced to BootServices->Stall() being called with IPL=TPL_HIGH_LEVEL
-and Interrupts /enabled/ while windows is booting.
-
-Cc: Michael Brown <mcb30@ipxe.org>
-Cc: Laszlo Ersek <lersek@redhat.com>
-Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
----
- OvmfPkg/Library/NestedInterruptTplLib/Tpl.c | 4 +++-
- 1 file changed, 3 insertions(+), 1 deletion(-)
-
-diff --git a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
-index e19d98878eb7..fdd7d15c4ba8 100644
---- a/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
-+++ b/OvmfPkg/Library/NestedInterruptTplLib/Tpl.c
-@@ -39,7 +39,9 @@ NestedInterruptRaiseTPL (
-   //
-   ASSERT (GetInterruptState () == FALSE);
-   InterruptedTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
--  ASSERT (InterruptedTPL < TPL_HIGH_LEVEL);
-+  if (InterruptedTPL >= TPL_HIGH_LEVEL) {
-+    DEBUG ((DEBUG_WARN, "%a: Called at IPL %d\n", __func__, InterruptedTPL));
-+  }
- 
-   return InterruptedTPL;
- }
--- 
-2.40.1
-

0017-OvmfPkg-Relax-assertion-that-interrupts-do-not-occur.patch

@@ -0,0 +1,75 @@
+From 7272c2fbe66941f0785be7ec437ed79ab9e35b80 Mon Sep 17 00:00:00 2001
+From: Michael Brown <mcb30@ipxe.org>
+Date: Tue, 9 May 2023 12:09:33 +0000
+Subject: [PATCH 17/18] OvmfPkg: Relax assertion that interrupts do not occur
+ at 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
+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 e921a09c5599..d56c12a44529 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.40.1
+

edk2.spec

@@ -98,7 +98,8 @@ Patch0012: 0012-OvmfPkg-QemuKernelLoaderFsDxe-suppress-error-on-no-k.patch
 Patch0013: 0013-SecurityPkg-Tcg2Dxe-suppress-error-on-no-swtpm-in-si.patch
 Patch0014: 0014-SecurityPkg-add-TIS-sanity-check-tpm2.patch
 Patch0015: 0015-SecurityPkg-add-TIS-sanity-check-tpm12.patch
-Patch0016: 0016-OvmfPkg-NestedInterruptTplLib-replace-ASSERT-with-a-.patch
+Patch0016: 0016-OvmfPkg-Clarify-invariants-for-NestedInterruptTplLib.patch
+Patch0017: 0017-OvmfPkg-Relax-assertion-that-interrupts-do-not-occur.patch
 
 
 # python3-devel and libuuid-devel are required for building tools.