public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Brown" <mcb30@ipxe.org>
To: devel@edk2.groups.io
Cc: Michael Brown <mcb30@ipxe.org>, Laszlo Ersek <lersek@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH 3/3] OvmfPkg: Use NestedInterruptTplLib in nested interrupt handlers
Date: Fri, 9 Dec 2022 10:20:27 +0000	[thread overview]
Message-ID: <01020184f6668e77-e04c6ec0-66ec-4118-85a1-2da9c260bf95-000000@eu-west-1.amazonses.com> (raw)
In-Reply-To: <cover.1670328507.git.mcb30@ipxe.org>

Prevent stack underrun in the event of a timer interrupt storm in
LocalApicTimerDxe and 8254TimerDxe interrupt handlers by using the
helper functions provided by NestedInterruptTplLib.

This fixes the same problem as addressed in commit 239b50a86
("OvmfPkg: End timer interrupt later to avoid stack overflow under
load"), but does so without breaking nested timer interrupts.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2815
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4162
Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
 OvmfPkg/8254TimerDxe/8254Timer.inf              | 1 +
 OvmfPkg/8254TimerDxe/Timer.c                    | 9 ++++++---
 OvmfPkg/AmdSev/AmdSevX64.dsc                    | 1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc                  | 1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc                | 1 +
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c   | 9 ++++++---
 OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf | 1 +
 OvmfPkg/Microvm/MicrovmX64.dsc                  | 1 +
 OvmfPkg/OvmfPkgIa32.dsc                         | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc                      | 1 +
 OvmfPkg/OvmfPkgX64.dsc                          | 1 +
 OvmfPkg/OvmfXen.dsc                             | 1 +
 12 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/8254TimerDxe/8254Timer.inf b/OvmfPkg/8254TimerDxe/8254Timer.inf
index 8a07c8247e..8fbab896f0 100644
--- a/OvmfPkg/8254TimerDxe/8254Timer.inf
+++ b/OvmfPkg/8254TimerDxe/8254Timer.inf
@@ -24,6 +24,7 @@
   UefiBootServicesTableLib
   BaseLib
   DebugLib
+  NestedInterruptTplLib
   UefiDriverEntryPoint
   IoLib
 
diff --git a/OvmfPkg/8254TimerDxe/Timer.c b/OvmfPkg/8254TimerDxe/Timer.c
index 7feaaaba2b..5382b01615 100644
--- a/OvmfPkg/8254TimerDxe/Timer.c
+++ b/OvmfPkg/8254TimerDxe/Timer.c
@@ -6,6 +6,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Library/NestedInterruptTplLib.h>
+
 #include "Timer.h"
 
 //
@@ -76,9 +78,10 @@ TimerInterruptHandler (
   IN EFI_SYSTEM_CONTEXT  SystemContext
   )
 {
-  EFI_TPL  OriginalTPL;
+  STATIC NESTED_INTERRUPT_STATE  NestedInterruptState;
+  EFI_TPL                        OriginalTPL;
 
-  OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+  OriginalTPL = NestedInterruptRaiseTPL ();
 
   mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
 
@@ -89,7 +92,7 @@ TimerInterruptHandler (
     mTimerNotifyFunction (mTimerPeriod);
   }
 
-  gBS->RestoreTPL (OriginalTPL);
+  NestedInterruptRestoreTPL (OriginalTPL, SystemContext, &NestedInterruptState);
 }
 
 /**
diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 8f7cae787e..3433725697 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -353,6 +353,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/GenericQemuLoadImageLib/GenericQemuLoadImageLib.inf
 
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index ce277cb239..6d5a702b05 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -404,6 +404,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 3458926515..994d09c751 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -314,6 +314,7 @@
   LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
index 3e04b49d6b..67f4dcde37 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.c
@@ -8,6 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
+#include <Library/NestedInterruptTplLib.h>
+
 #include "LocalApicTimerDxe.h"
 
 //
@@ -58,9 +60,10 @@ TimerInterruptHandler (
   IN EFI_SYSTEM_CONTEXT  SystemContext
   )
 {
-  EFI_TPL  OriginalTPL;
+  STATIC NESTED_INTERRUPT_STATE  NestedInterruptState;
+  EFI_TPL                        OriginalTPL;
 
-  OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+  OriginalTPL = NestedInterruptRaiseTPL ();
 
   SendApicEoi ();
 
@@ -71,7 +74,7 @@ TimerInterruptHandler (
     mTimerNotifyFunction (mTimerPeriod);
   }
 
-  gBS->RestoreTPL (OriginalTPL);
+  NestedInterruptRestoreTPL (OriginalTPL, SystemContext, &NestedInterruptState);
 }
 
 /**
diff --git a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
index 3ad28a148c..b85965c75e 100644
--- a/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
+++ b/OvmfPkg/LocalApicTimerDxe/LocalApicTimerDxe.inf
@@ -27,6 +27,7 @@
   UefiBootServicesTableLib
   BaseLib
   DebugLib
+  NestedInterruptTplLib
   UefiDriverEntryPoint
   LocalApicLib
 
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 994a02d301..db449384c9 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -403,6 +403,7 @@
   PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 6f774baf90..15ce1261ec 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -409,6 +409,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index c851764dec..da8d182b54 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -415,6 +415,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 63c3a47aea..b2d24a798b 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -435,6 +435,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
   QemuLoadImageLib|OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.inf
 
diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 8bb497088b..c328987e84 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -340,6 +340,7 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+  NestedInterruptTplLib|OvmfPkg/Library/NestedInterruptTplLib/NestedInterruptTplLib.inf
   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
 
 [LibraryClasses.common.UEFI_APPLICATION]
-- 
2.38.1


      parent reply	other threads:[~2022-12-09 10:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1670328507.git.mcb30@ipxe.org>
2022-12-09 10:20 ` [PATCH 1/3] OvmfPkg: Send EOI before RestoreTPL() in timer interrupt handlers Michael Brown
2022-12-09 10:20 ` [PATCH 2/3] OvmfPkg: Add library to handle TPL from within nested " Michael Brown
2022-12-09 15:02   ` [edk2-devel] " Gerd Hoffmann
2022-12-09 15:22     ` Michael Brown
     [not found]     ` <ac60f07a-df27-bcfd-4129-0d6bfcf7ac77@ipxe.org>
2022-12-20 15:02       ` Michael Brown
2022-12-21  8:06         ` Laszlo Ersek
2022-12-23 14:25           ` Ard Biesheuvel
2022-12-09 10:20 ` Michael Brown [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=01020184f6668e77-e04c6ec0-66ec-4118-85a1-2da9c260bf95-000000@eu-west-1.amazonses.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox