From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from esa3.hc3370-68.iphmx.com (esa3.hc3370-68.iphmx.com [216.71.145.155]) by mx.groups.io with SMTP id smtpd.web12.2622.1592275802821098287 for ; Mon, 15 Jun 2020 19:50:02 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: citrix.com, ip: 216.71.145.155, mailfrom: igor.druzhinin@citrix.com) Authentication-Results: esa3.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none IronPort-SDR: KzmC4IlJmvFM5fUok7jMyCcojCWEoAVzzm536ptJeJ/uOrzKfXn0+x8PVe9E9SdEb4JVCBRK3W PnOTlJqanb45w2BOygID5GKmFZrgUtSeH7mnlKwiMKNkqbfrLcxU7FoTXM+3gDMroxuN7/UA9m PjLnxgREMIcuhscaHIJyRlyZbmQAfb+uYUZrKUwebEJuBIm8NmP486dLtDc6p0ys1C5SwMQGTh Nc1RH2RFDSQ4Q2+l85NkymWRFk0qS2shoIsehF1oBYrB4Gi7rEq/OZlyK+7IP4Qy99tOqfmIRO wg4= X-SBRS: 2.7 X-MesageID: 20118697 X-Ironport-Server: esa3.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.73,517,1583211600"; d="scan'208";a="20118697" From: Igor Druzhinin To: CC: , , , , , "Igor Druzhinin" Subject: [PATCH] OvmfPkg: End timer interrupt later to avoid stack overflow under load Date: Tue, 16 Jun 2020 03:49:42 +0100 Message-ID: <1592275782-9369-1-git-send-email-igor.druzhinin@citrix.com> X-Mailer: git-send-email 2.7.4 Return-Path: igor.druzhinin@citrix.com MIME-Version: 1.0 Content-Type: text/plain 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 --- 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