From: Igor Druzhinin <igor.druzhinin@citrix.com>
To: <devel@edk2.groups.io>
Cc: <jordan.l.justen@intel.com>, <lersek@redhat.com>,
<ard.biesheuvel@arm.com>, <anthony.perard@citrix.com>,
<julien@xen.org>, "Igor Druzhinin" <igor.druzhinin@citrix.com>
Subject: [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load
Date: Tue, 16 Jun 2020 03:49:42 +0100 [thread overview]
Message-ID: <1592275782-9369-1-git-send-email-igor.druzhinin@citrix.com> (raw)
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@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
next reply other threads:[~2020-06-16 2:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-16 2:49 Igor Druzhinin [this message]
2020-06-16 18:42 ` [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load Laszlo Ersek
2020-06-17 3:16 ` Igor Druzhinin
2020-06-17 12:44 ` Laszlo Ersek
2020-06-17 13:51 ` Paolo Bonzini
2020-06-17 15:46 ` Laszlo Ersek
2020-06-17 16:59 ` Paolo Bonzini
2020-06-17 17:23 ` Igor Druzhinin
2020-06-18 8:36 ` Laszlo Ersek
2020-06-18 8:44 ` Laszlo Ersek
2020-06-18 12:10 ` [edk2-devel] " Laszlo Ersek
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=1592275782-9369-1-git-send-email-igor.druzhinin@citrix.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