[edk2-devel] [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load

[Igor Druzhinin]

RestoreTPL called while at TPL_HIGH_LEVEL unconditionally enables interrupts
even if called in interrupt handler. That opens a window while interrupt
is not completely handled but another interrupt could be accepted.

If a VM starts on a heavily loaded host hundreds of periodic timer interrupts
might be queued while vCPU is descheduled (the behavior is typical for
a Xen host). The next time vCPU is scheduled again all of them get
delivered back to back causing OVMF to accept each one without finishing
a previous one and cleaning up the stack. That quickly results in stack
overflow and a triple fault.

Fix it by postponing sending EOI until we finished processing the current
tick giving interrupt handler opportunity to clean up the stack before
accepting the next tick.

Signed-off-by: Igor Druzhinin <igor.druzhinin at citrix.com>
---

Laszlo, Anthony,

Do you think it's the right way to address it?

Alternatively, we might avoid calling RaiseTPL in interrupt handler at all
like it's done in HpetTimer implementation for instance.

Or we might try to address it in Raise/RestoreTPL calls by saving/restoring
interrupt state along with TPL.

---
 OvmfPkg/8254TimerDxe/Timer.c      | 5 +++--
 OvmfPkg/XenTimerDxe/XenTimerDxe.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/8254TimerDxe/Timer.c b/OvmfPkg/8254TimerDxe/Timer.c
index 67e22f5..fd1691b 100644
--- a/OvmfPkg/8254TimerDxe/Timer.c
+++ b/OvmfPkg/8254TimerDxe/Timer.c
@@ -79,8 +79,6 @@ TimerInterruptHandler (
 
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
-  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
-
   if (mTimerNotifyFunction != NULL) {
     //
     // @bug : This does not handle missed timer interrupts
@@ -89,6 +87,9 @@ TimerInterruptHandler (
   }
 
   gBS->RestoreTPL (OriginalTPL);
+
+  DisableInterrupts ();
+  mLegacy8259->EndOfInterrupt (mLegacy8259, Efi8259Irq0);
 }
 
 /**
diff --git a/OvmfPkg/XenTimerDxe/XenTimerDxe.c b/OvmfPkg/XenTimerDxe/XenTimerDxe.c
index 9f9e047..0bec593 100644
--- a/OvmfPkg/XenTimerDxe/XenTimerDxe.c
+++ b/OvmfPkg/XenTimerDxe/XenTimerDxe.c
@@ -61,8 +61,6 @@ TimerInterruptHandler (
 
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
-  SendApicEoi();
-
   if (mTimerNotifyFunction != NULL) {
     //
     // @bug : This does not handle missed timer interrupts
@@ -71,6 +69,9 @@ TimerInterruptHandler (
   }
 
   gBS->RestoreTPL (OriginalTPL);
+
+  DisableInterrupts ();
+  SendApicEoi ();
 }
 
 /**
-- 
2.7.4


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61329): https://edk2.groups.io/g/devel/message/61329
Mute This Topic: https://groups.io/mt/74913405/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-