RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 856246 - Interrupt handling can be optimized to cause less vm-exits & simplify code
Summary: Interrupt handling can be optimized to cause less vm-exits & simplify code
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 8
Classification: Red Hat
Component: spice-qxl-xddm
Version: ---
Hardware: Unspecified
OS: Unspecified
low
low
Target Milestone: rc
: ---
Assignee: David Blechter
QA Contact: SPICE QE bug list
URL:
Whiteboard:
Depends On:
Blocks: 1019461
TreeView+ depends on / blocked
 
Reported: 2012-09-11 14:51 UTC by Hans de Goede
Modified: 2019-10-10 13:47 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-05-10 11:02:23 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Hans de Goede 2012-09-11 14:51:33 UTC
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 15:48:23 UTC
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 13:18:02 UTC
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>
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 15:11:05 UTC
moving to 3.4 as we are at RC phase, and we don't a solution yet.

Comment 5 David Blechter 2014-02-11 23:10:51 UTC
no plans for whql in 3.4, moving to the future releases.

Comment 7 Alon Levy 2014-05-01 08:09:57 UTC
"solution" was buggy and reverted, setting back to assigned and lowering priority.

Comment 9 Yaniv Lavi 2016-05-09 11:06:57 UTC
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.