Bug 856246 - Interrupt handling can be optimized to cause less vm-exits & simplify code
Interrupt handling can be optimized to cause less vm-exits & simplify code
Status: CLOSED WONTFIX
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: spice-qxl-driver-win (Show other bugs)
unspecified
Unspecified Unspecified
low Severity low
: ovirt-4.0.0-beta
: 4.0.0
Assigned To: David Blechter
SPICE QE bug list
: CodeChange
Depends On:
Blocks: 1019461
  Show dependency treegraph
 
Reported: 2012-09-11 10:51 EDT by Hans de Goede
Modified: 2016-05-10 07:02 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-05-10 07:02:23 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: Spice
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Hans de Goede 2012-09-11 10:51:33 EDT
As discussed on irc, currently the windows qxl driver miniport interrupt handling does:

In interrupt handler:
 set int_mask to 0
 QXL_IO_UPDATE_IRQ
 queue bh
In bh:
 read then clear int pending
 reset int mask to ~0
 QXL_IO_UPDATE_IRQ

This needlessly calls QXL_IO_UPDATE_IRQ twice, the *entire* interrupt handler can be simplified to just:

UINT32 pending = VideoPortInterlockedExchange(&dev_ext->ram_header->int_pending, 0);
VideoPortWritePortUchar((PUCHAR)dev_ext->io_base + QXL_IO_UPDATE_IRQ, 0);
VideoPortQueueDpc(dev_extension, InterruptCallback, (void *) pending);

And the bh gets the pending events as its context parameter.

Leading to much nicer code, and only 1 vm_exit per interrupt instead of 2.
Comment 1 David Blechter 2013-05-18 11:48:23 EDT
granted devel_ack+ for 3.3 - there are number of bugs we are planning to fic in 3,3 in qxl driver. It will require whql, adding this optimization at the right time.
Comment 3 Alon Levy 2013-07-29 09:18:02 EDT
Proposed patches on mailing list, tested with xp, and with the following system tap script and python script above it (see below after patches).

commit 8c2457c497066b4ff72e32c025faf69fd77b3605
Author: Alon Levy <alevy@redhat.com>
Date:   Mon Jul 29 16:10:09 2013 +0300

    miniport: halve QXL_IO_UPDATE_IRQ calls
    
    RHBZ: 856246

diff --git a/miniport/qxl.c b/miniport/qxl.c
index f5d6b48..629fbdb 100644
--- a/miniport/qxl.c
+++ b/miniport/qxl.c
@@ -1298,7 +1298,7 @@ err:
 VOID InterruptCallback(PVOID dev_extension, PVOID Context)
 {
     QXLExtension *dev_ext = dev_extension;
-    UINT32 pending = VideoPortInterlockedExchange(&dev_ext->ram_header->int_pending, 0);
+    UINT32 pending = (UINT32)Context;
 
     if (pending & QXL_INTERRUPT_DISPLAY) {
         VideoPortSetEvent(dev_ext, dev_ext->display_event);
@@ -1316,14 +1316,14 @@ VOID InterruptCallback(PVOID dev_extension, PVOID Context)
 BOOLEAN Interrupt(PVOID dev_extension)
 {
     QXLExtension *dev_ext = dev_extension;
+    UINT32 pending;
 
-    if (!(dev_ext->ram_header->int_pending & dev_ext->ram_header->int_mask)) {
+    pending = VideoPortInterlockedExchange(&dev_ext->ram_header->int_pending, 0);
+    if (!(pending & dev_ext->ram_header->int_mask)) {
         return FALSE;
     }
-    dev_ext->ram_header->int_mask = 0;
-    VideoPortWritePortUchar((PUCHAR)dev_ext->io_base + QXL_IO_UPDATE_IRQ, 0);
 
-    if (!VideoPortQueueDpc(dev_extension, InterruptCallback, NULL)) {
+    if (!VideoPortQueueDpc(dev_extension, InterruptCallback, (void *)pending)) {
         VideoPortLogError(dev_extension, NULL, E_UNEXPECTED, QXLERR_INT_DELIVERY);
         dev_ext->ram_header->int_mask = WIN_QXL_INT_MASK;
         VideoPortWritePortUchar((PUCHAR)dev_ext->io_base + QXL_IO_UPDATE_IRQ, 0);


--------------------------

To verify number of io exits QXL_IO_UPDATE_IRQ:

U x86_64 garlic:scripts alon (bz917860.v5)$ cat stap-qxl-interrupts
#!/bin/bash -x
STP=$0.stp
ROOT=`dirname $0`/../
sudo stap -v -I $ROOT/x86_64-softmmu $STP

U x86_64 garlic:scripts alon (bz917860.v5)$ cat stap-qxl-interrupts.stp
probe begin { printf("starting qxl generic probe\n") }

probe process("/home/alon/src/spice_rhel6/qemu-kvm-rhel6/x86_64-softmmu/qemu-system-x86_64").statement("*@hw/qxl.c:1396"){
    if ($io_port == 3 /* QXL_IO_UPDATE_IRQ */ ) {
        printf("%d: %s: %s\n", tid(), pp(), $$vars)
    }
}
U x86_64 garlic:scripts alon (bz917860.v5)$ cat watch-interrupts 
#!/usr/bin/python

import subprocess
import time
import select

p = subprocess.Popen(args=['./stap-qxl-interrupts'], stdout=subprocess.PIPE)

while True:
    start = time.time()
    rlist, wlist, xlist = select.select([p.stdout.fileno()], [], [], 1)
    if len(rlist) != 0:
        data = []
        now = time.time()
        while len(rlist) > 0 and now - start < 1:
            data.append(p.stdout.readline())
            rlist, wlist, xlist = select.select([p.stdout.fileno()], [], [], 1)
            now = time.time()
        print "%d" % len(data)
    else:
        print "nothing"
    now = time.time()
    if now < start + 1:
        time.sleep(start + 1 - now)
Comment 4 David Blechter 2013-12-08 10:11:05 EST
moving to 3.4 as we are at RC phase, and we don't a solution yet.
Comment 5 David Blechter 2014-02-11 18:10:51 EST
no plans for whql in 3.4, moving to the future releases.
Comment 7 Alon Levy 2014-05-01 04:09:57 EDT
"solution" was buggy and reverted, setting back to assigned and lowering priority.
Comment 9 Yaniv Lavi (Dary) 2016-05-09 07:06:57 EDT
oVirt 4.0 Alpha has been released, moving to oVirt 4.0 Beta target.

Note You need to log in before you can comment on or make changes to this bug.