[Qemu-devel] [PULL 00/10] spice patch queue

[Qemu-devel] [PULL 00/10] spice patch queue

[Gerd Hoffmann]

  Hi,

This pull brings monitor configuration support for qxl which is needed
by the upcoming kms qxl driver.  Also a nice speedup for qxl in vga mode
and as usual some bugfixes.

please pull,
  Gerd

The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:

  Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)

are available in the git repository at:

  git://anongit.freedesktop.org/spice/qemu spice.v60

Alon Levy (3):
      hw/qxl: tracing fixes
      qxl: add trace-event for QXL_IO_LOG
      hw/qxl: support client monitor configuration via device

Dunrong Huang (1):
      qxl: dont update invalid area

Gerd Hoffmann (4):
      spice: switch to queue for vga mode updates
      spice: split qemu_spice_create_update
      spice: add screen mirror
      spice: send updates only for changed screen content

Hans de Goede (1):
      qxl: Ignore set_client_capabilities pre/post migrate

Uri Lublin (1):
      qxl: better cleanup for surface destroy

 configure          |    7 +++
 hw/qxl.c           |  107 ++++++++++++++++++++++++++++++++++++++--
 trace-events       |   11 ++++-
 ui/spice-display.c |  136 ++++++++++++++++++++++++++++++++++++++++------------
 ui/spice-display.h |    4 +-
 5 files changed, 225 insertions(+), 40 deletions(-)

[Qemu-devel] [PATCH 01/10] spice: switch to queue for vga mode updates

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c           |    6 +++---
 ui/spice-display.c |   25 ++++++++++++++-----------
 ui/spice-display.h |    3 ++-
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 5b3f484..257a37d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -597,9 +597,9 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
     case QXL_MODE_VGA:
         ret = false;
         qemu_mutex_lock(&qxl->ssd.lock);
-        if (qxl->ssd.update != NULL) {
-            update = qxl->ssd.update;
-            qxl->ssd.update = NULL;
+        update = QTAILQ_FIRST(&qxl->ssd.updates);
+        if (update != NULL) {
+            QTAILQ_REMOVE(&qxl->ssd.updates, update, next);
             *ext = update->ext;
             ret = true;
         }
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 99bc665..59c5fd7 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,7 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
 #endif
 }
 
-static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
     SimpleSpiceUpdate *update;
     QXLDrawable *drawable;
@@ -175,7 +175,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     struct timespec time_space;
 
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-        return NULL;
+        return;
     };
 
     trace_qemu_spice_create_update(
@@ -239,7 +239,7 @@ static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     cmd->data = (uintptr_t)drawable;
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
-    return update;
+    QTAILQ_INSERT_TAIL(&ssd->updates, update, next);
 }
 
 /*
@@ -315,6 +315,7 @@ void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
 {
     ssd->ds = ds;
     qemu_mutex_init(&ssd->lock);
+    QTAILQ_INIT(&ssd->updates);
     ssd->mouse_x = -1;
     ssd->mouse_y = -1;
     if (ssd->num_surfaces == 0) {
@@ -345,6 +346,8 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
 
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
 {
+    SimpleSpiceUpdate *update;
+
     dprint(1, "%s:\n", __FUNCTION__);
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
@@ -352,9 +355,9 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
     ssd->conv = NULL;
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update != NULL) {
-        qemu_spice_destroy_update(ssd, ssd->update);
-        ssd->update = NULL;
+    while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) {
+        QTAILQ_REMOVE(&ssd->updates, update, next);
+        qemu_spice_destroy_update(ssd, update);
     }
     qemu_mutex_unlock(&ssd->lock);
     qemu_spice_destroy_host_primary(ssd);
@@ -384,8 +387,8 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
     vga_hw_update();
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update == NULL) {
-        ssd->update = qemu_spice_create_update(ssd);
+    if (QTAILQ_EMPTY(&ssd->updates)) {
+        qemu_spice_create_update(ssd);
         ssd->notify++;
     }
     qemu_spice_cursor_refresh_unlocked(ssd);
@@ -442,9 +445,9 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
     dprint(3, "%s:\n", __FUNCTION__);
 
     qemu_mutex_lock(&ssd->lock);
-    if (ssd->update != NULL) {
-        update = ssd->update;
-        ssd->update = NULL;
+    update = QTAILQ_FIRST(&ssd->updates);
+    if (update != NULL) {
+        QTAILQ_REMOVE(&ssd->updates, update, next);
         *ext = update->ext;
         ret = true;
     }
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 512ab78..3fcb6fe 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -92,7 +92,7 @@ struct SimpleSpiceDisplay {
      * to them must be protected by the lock.
      */
     QemuMutex lock;
-    SimpleSpiceUpdate *update;
+    QTAILQ_HEAD(, SimpleSpiceUpdate) updates;
     QEMUCursor *cursor;
     int mouse_x, mouse_y;
 };
@@ -102,6 +102,7 @@ struct SimpleSpiceUpdate {
     QXLImage image;
     QXLCommandExt ext;
     uint8_t *bitmap;
+    QTAILQ_ENTRY(SimpleSpiceUpdate) next;
 };
 
 int qemu_spice_rect_is_empty(const QXLRect* r);
-- 
1.7.1

[Qemu-devel] [PATCH 02/10] spice: split qemu_spice_create_update

[Gerd Hoffmann]

Creating one function which creates a single update for a given
rectangle.  And one (for now) pretty simple wrapper around it to
queue up screen updates for the dirty region.

[ v2: also update bounding box ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   31 ++++++++++++++++++-------------
 1 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 59c5fd7..6f68f28 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -164,7 +164,8 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
 #endif
 }
 
-static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
+                                         QXLRect *rect)
 {
     SimpleSpiceUpdate *update;
     QXLDrawable *drawable;
@@ -174,24 +175,20 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     int by, bw, bh;
     struct timespec time_space;
 
-    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
-        return;
-    };
-
     trace_qemu_spice_create_update(
-           ssd->dirty.left, ssd->dirty.right,
-           ssd->dirty.top, ssd->dirty.bottom);
+           rect->left, rect->right,
+           rect->top, rect->bottom);
 
     update   = g_malloc0(sizeof(*update));
     drawable = &update->drawable;
     image    = &update->image;
     cmd      = &update->ext.cmd;
 
-    bw       = ssd->dirty.right - ssd->dirty.left;
-    bh       = ssd->dirty.bottom - ssd->dirty.top;
+    bw       = rect->right - rect->left;
+    bh       = rect->bottom - rect->top;
     update->bitmap = g_malloc(bw * bh * 4);
 
-    drawable->bbox            = ssd->dirty;
+    drawable->bbox            = *rect;
     drawable->clip.type       = SPICE_CLIP_TYPE_NONE;
     drawable->effect          = QXL_EFFECT_OPAQUE;
     drawable->release_info.id = (uintptr_t)update;
@@ -226,8 +223,8 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     }
 
     src = ds_get_data(ssd->ds) +
-        ssd->dirty.top * ds_get_linesize(ssd->ds) +
-        ssd->dirty.left * ds_get_bytes_per_pixel(ssd->ds);
+        rect->top * ds_get_linesize(ssd->ds) +
+        rect->left * ds_get_bytes_per_pixel(ssd->ds);
     dst = update->bitmap;
     for (by = 0; by < bh; by++) {
         qemu_pf_conv_run(ssd->conv, dst, src, bw);
@@ -238,10 +235,18 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     cmd->type = QXL_CMD_DRAW;
     cmd->data = (uintptr_t)drawable;
 
-    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     QTAILQ_INSERT_TAIL(&ssd->updates, update, next);
 }
 
+static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
+{
+    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
+        return;
+    };
+    qemu_spice_create_one_update(ssd, &ssd->dirty);
+    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
+}
+
 /*
  * Called from spice server thread context (via interface_release_ressource)
  * We do *not* hold the global qemu mutex here, so extra care is needed
-- 
1.7.1

[Qemu-devel] [PATCH 03/10] spice: add screen mirror

[Gerd Hoffmann]

Create a screen mirror, keep there a copy of the most recent update
passed on to spice-server.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   32 ++++++++++++++++++++++----------
 ui/spice-display.h |    1 +
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 6f68f28..973cd53 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -171,8 +171,8 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
     QXLDrawable *drawable;
     QXLImage *image;
     QXLCommand *cmd;
-    uint8_t *src, *dst;
-    int by, bw, bh;
+    uint8_t *src, *mirror, *dst;
+    int by, bw, bh, offset, bytes;
     struct timespec time_space;
 
     trace_qemu_spice_create_update(
@@ -216,19 +216,18 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
     image->bitmap.palette = 0;
     image->bitmap.format = SPICE_BITMAP_FMT_32BIT;
 
-    if (ssd->conv == NULL) {
-        PixelFormat dst = qemu_default_pixelformat(32);
-        ssd->conv = qemu_pf_conv_get(&dst, &ssd->ds->surface->pf);
-        assert(ssd->conv);
-    }
-
-    src = ds_get_data(ssd->ds) +
+    offset =
         rect->top * ds_get_linesize(ssd->ds) +
         rect->left * ds_get_bytes_per_pixel(ssd->ds);
+    bytes = ds_get_bytes_per_pixel(ssd->ds) * bw;
+    src = ds_get_data(ssd->ds) + offset;
+    mirror = ssd->ds_mirror + offset;
     dst = update->bitmap;
     for (by = 0; by < bh; by++) {
-        qemu_pf_conv_run(ssd->conv, dst, src, bw);
+        memcpy(mirror, src, bytes);
+        qemu_pf_conv_run(ssd->conv, dst, mirror, bw);
         src += ds_get_linesize(ssd->ds);
+        mirror += ds_get_linesize(ssd->ds);
         dst += image->bitmap.stride;
     }
 
@@ -243,6 +242,17 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
         return;
     };
+
+    if (ssd->conv == NULL) {
+        PixelFormat dst = qemu_default_pixelformat(32);
+        ssd->conv = qemu_pf_conv_get(&dst, &ssd->ds->surface->pf);
+        assert(ssd->conv);
+    }
+    if (ssd->ds_mirror == NULL) {
+        int size = ds_get_height(ssd->ds) * ds_get_linesize(ssd->ds);
+        ssd->ds_mirror = g_malloc0(size);
+    }
+
     qemu_spice_create_one_update(ssd, &ssd->dirty);
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
 }
@@ -358,6 +368,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd)
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     qemu_pf_conv_put(ssd->conv);
     ssd->conv = NULL;
+    g_free(ssd->ds_mirror);
+    ssd->ds_mirror = NULL;
 
     qemu_mutex_lock(&ssd->lock);
     while ((update = QTAILQ_FIRST(&ssd->updates)) != NULL) {
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 3fcb6fe..dea41c1 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -72,6 +72,7 @@ typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
 struct SimpleSpiceDisplay {
     DisplayState *ds;
+    uint8_t *ds_mirror;
     void *buf;
     int bufsize;
     QXLWorker *worker;
-- 
1.7.1

[Qemu-devel] [PATCH 04/10] spice: send updates only for changed screen content

[Gerd Hoffmann]

when creating screen updates go compare the current guest screen
against the mirror (which holds the most recent update sent), then
only create updates for the screen areas which did actually change.

[ v2: drop redundant qemu_spice_create_one_update call ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c |   56 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 55 insertions(+), 1 deletions(-)

diff --git a/ui/spice-display.c b/ui/spice-display.c
index 973cd53..d062765 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -239,6 +239,13 @@ static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
 
 static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
+    static const int blksize = 32;
+    int blocks = (ds_get_width(ssd->ds) + blksize - 1) / blksize;
+    int dirty_top[blocks];
+    int y, yoff, x, xoff, blk, bw;
+    int bpp = ds_get_bytes_per_pixel(ssd->ds);
+    uint8_t *guest, *mirror;
+
     if (qemu_spice_rect_is_empty(&ssd->dirty)) {
         return;
     };
@@ -253,7 +260,54 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
         ssd->ds_mirror = g_malloc0(size);
     }
 
-    qemu_spice_create_one_update(ssd, &ssd->dirty);
+    for (blk = 0; blk < blocks; blk++) {
+        dirty_top[blk] = -1;
+    }
+
+    guest = ds_get_data(ssd->ds);
+    mirror = ssd->ds_mirror;
+    for (y = ssd->dirty.top; y < ssd->dirty.bottom; y++) {
+        yoff = y * ds_get_linesize(ssd->ds);
+        for (x = ssd->dirty.left; x < ssd->dirty.right; x += blksize) {
+            xoff = x * bpp;
+            blk = x / blksize;
+            bw = MIN(blksize, ssd->dirty.right - x);
+            if (memcmp(guest + yoff + xoff,
+                       mirror + yoff + xoff,
+                       bw * bpp) == 0) {
+                if (dirty_top[blk] != -1) {
+                    QXLRect update = {
+                        .top    = dirty_top[blk],
+                        .bottom = y,
+                        .left   = x,
+                        .right  = x + bw,
+                    };
+                    qemu_spice_create_one_update(ssd, &update);
+                    dirty_top[blk] = -1;
+                }
+            } else {
+                if (dirty_top[blk] == -1) {
+                    dirty_top[blk] = y;
+                }
+            }
+        }
+    }
+
+    for (x = ssd->dirty.left; x < ssd->dirty.right; x += blksize) {
+        blk = x / blksize;
+        bw = MIN(blksize, ssd->dirty.right - x);
+        if (dirty_top[blk] != -1) {
+            QXLRect update = {
+                .top    = dirty_top[blk],
+                .bottom = ssd->dirty.bottom,
+                .left   = x,
+                .right  = x + bw,
+            };
+            qemu_spice_create_one_update(ssd, &update);
+            dirty_top[blk] = -1;
+        }
+    }
+
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 05/10] qxl: dont update invalid area

[Gerd Hoffmann]

From: Dunrong Huang <riegamaths@gmail.com>

This patch fixes the following error:

$ ~/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -spice port=5900,disable-ticketing -vga qxl -cdrom ~/Images/linuxmint-13-mate-dvd-32bit.iso
(/home/mathslinux/usr/bin/qemu-system-x86_64:10068): SpiceWorker-CRITICAL **: red_worker.c:4599:red_update_area: condition `area->left >= 0 && area->top >= 0 && area->left < area->right && area->top < area->bottom' failed
Aborted

spice server terminates QEMU process if we pass invalid area to it,
so dont update those invalid areas.

Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 257a37d..0176b1a 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1470,6 +1470,13 @@ async_common:
             return;
         }
 
+        if (update.left < 0 || update.top < 0 || update.left >= update.right ||
+            update.top >= update.bottom) {
+            qxl_set_guest_bug(d, "QXL_IO_UPDATE_AREA: "
+                              "invalid area(%d,%d,%d,%d)\n", update.left,
+                              update.right, update.top, update.bottom);
+            break;
+        }
         if (async == QXL_ASYNC) {
             cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
                                     QXL_IO_UPDATE_AREA_ASYNC);
-- 
1.7.1

[Qemu-devel] [PATCH 06/10] qxl: Ignore set_client_capabilities pre/post migrate

[Gerd Hoffmann]

From: Hans de Goede <hdegoede@redhat.com>

The recent introduction of set_client_capabilities has broken
(seamless) migration by trying to call qxl_send_events pre (seamless
incoming) and post (*) migration, triggering the following assert:
qxl_send_events: Assertion `qemu_spice_display_is_running(&d->ssd)' failed.

The solution is easy, pre migration the guest will have already received
the client caps on the migration source side, and post migration there no
longer is a guest, so we can simply ignore the set_client_capabilities call
in both those scenarios.

*) Post migration, so not fatal for to the migration itself, but still a crash

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 0176b1a..e539134 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -953,6 +953,11 @@ static void interface_set_client_capabilities(QXLInstance *sin,
 {
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
 
+    if (runstate_check(RUN_STATE_INMIGRATE) ||
+        runstate_check(RUN_STATE_POSTMIGRATE)) {
+        return;
+    }
+
     qxl->shadow_rom.client_present = client_present;
     memcpy(qxl->shadow_rom.client_capabilities, caps, sizeof(caps));
     qxl->rom->client_present = client_present;
-- 
1.7.1

[Qemu-devel] [PATCH 07/10] qxl: better cleanup for surface destroy

[Gerd Hoffmann]

From: Uri Lublin <uril@redhat.com>

Add back a call to qxl_spice_destroy_surface_wait_complete() in qxl_spice_destroy_surface_wait(),
that was removed by commit c480bb7da465186b84d8427e068ef7502e47ffbf

It is needed to complete surface-removal cleanup, for non async.
For async, qxl_spice_destroy_surface_wait_complete is called upon operation completion.

Signed-off-by: Uri Lublin <uril@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e539134..12597e7 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -201,6 +201,7 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
         spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uintptr_t)cookie);
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+        qxl_spice_destroy_surface_wait_complete(qxl, id);
     }
 }
 
-- 
1.7.1

[Qemu-devel] [PATCH 08/10] hw/qxl: tracing fixes

[Gerd Hoffmann]

From: Alon Levy <alevy@redhat.com>

Add two new trace events:
qxl_send_events(int qid, uint32_t events) "%d %d"
qxl_set_guest_bug(int qid) "%d"

Change qxl_io_unexpected_vga_mode parameters to be equivalent to those
of qxl_io_write for easier grouping under a single systemtap probe.

Change d to qxl in one place.

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c     |    8 +++++---
 trace-events |    6 ++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 12597e7..8c46766 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -141,6 +141,7 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
 
 void qxl_set_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
 {
+    trace_qxl_set_guest_bug(qxl->id);
     qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
     qxl->guest_bug = 1;
     if (qxl->guestdebug) {
@@ -1408,7 +1409,7 @@ static void ioport_write(void *opaque, target_phys_addr_t addr,
             break;
         }
         trace_qxl_io_unexpected_vga_mode(d->id,
-            io_port, io_port_to_string(io_port));
+            addr, val, io_port_to_string(io_port));
         /* be nice to buggy guest drivers */
         if (io_port >= QXL_IO_UPDATE_AREA_ASYNC &&
             io_port < QXL_IO_RANGE_SIZE) {
@@ -1607,9 +1608,9 @@ cancel_async:
 static uint64_t ioport_read(void *opaque, target_phys_addr_t addr,
                             unsigned size)
 {
-    PCIQXLDevice *d = opaque;
+    PCIQXLDevice *qxl = opaque;
 
-    trace_qxl_io_read_unexpected(d->id);
+    trace_qxl_io_read_unexpected(qxl->id);
     return 0xff;
 }
 
@@ -1639,6 +1640,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
     uint32_t old_pending;
     uint32_t le_events = cpu_to_le32(events);
 
+    trace_qxl_send_events(d->id, events);
     assert(qemu_spice_display_is_running(&d->ssd));
     old_pending = __sync_fetch_and_or(&d->ram->int_pending, le_events);
     if ((old_pending & le_events) == le_events) {
diff --git a/trace-events b/trace-events
index b25ae1c..6169c29 100644
--- a/trace-events
+++ b/trace-events
@@ -933,7 +933,7 @@ qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
 qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
 qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
 qxl_io_read_unexpected(int qid) "%d"
-qxl_io_unexpected_vga_mode(int qid, uint32_t io_port, const char *desc) "%d 0x%x (%s)"
+qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
 qxl_io_write(int qid, const char *mode, uint64_t addr, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " val=%"PRIu64" size=%u async=%d"
 qxl_memslot_add_guest(int qid, uint32_t slot_id, uint64_t guest_start, uint64_t guest_end) "%d %u: guest phys 0x%"PRIx64 " - 0x%" PRIx64
 qxl_post_load(int qid, const char *mode) "%d %s"
@@ -964,7 +964,7 @@ qxl_spice_destroy_surfaces(int qid, int async) "%d async=%d"
 qxl_spice_destroy_surface_wait_complete(int qid, uint32_t id) "%d sid=%d"
 qxl_spice_destroy_surface_wait(int qid, uint32_t id, int async) "%d sid=%d async=%d"
 qxl_spice_flush_surfaces_async(int qid, uint32_t surface_count, uint32_t num_free_res) "%d s#=%d, res#=%d"
-qxl_spice_monitors_config(int id) "%d"
+qxl_spice_monitors_config(int qid) "%d"
 qxl_spice_loadvm_commands(int qid, void *ext, uint32_t count) "%d ext=%p count=%d"
 qxl_spice_oom(int qid) "%d"
 qxl_spice_reset_cursor(int qid) "%d"
@@ -973,6 +973,8 @@ qxl_spice_reset_memslots(int qid) "%d"
 qxl_spice_update_area(int qid, uint32_t surface_id, uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "%d sid=%d [%d,%d,%d,%d]"
 qxl_spice_update_area_rest(int qid, uint32_t num_dirty_rects, uint32_t clear_dirty_region) "%d #d=%d clear=%d"
 qxl_surfaces_dirty(int qid, int surface, int offset, int size) "%d surface=%d offset=%d size=%d"
+qxl_send_events(int qid, uint32_t events) "%d %d"
+qxl_set_guest_bug(int qid) "%d"
 
 # hw/qxl-render.c
 qxl_render_blit_guest_primary_initialized(void) ""
-- 
1.7.1

[Qemu-devel] [PATCH 09/10] qxl: add trace-event for QXL_IO_LOG

[Gerd Hoffmann]

From: Alon Levy <alevy@redhat.com>

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c     |    1 +
 trace-events |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 8c46766..5709e0d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1515,6 +1515,7 @@ async_common:
         qxl_set_mode(d, val, 0);
         break;
     case QXL_IO_LOG:
+        trace_qxl_io_log(d->id, d->ram->log_buf);
         if (d->guestdebug) {
             fprintf(stderr, "qxl/guest-%d: %" PRId64 ": %s", d->id,
                     qemu_get_clock_ns(vm_clock), d->ram->log_buf);
diff --git a/trace-events b/trace-events
index 6169c29..cd638f5 100644
--- a/trace-events
+++ b/trace-events
@@ -932,6 +932,7 @@ qxl_interface_update_area_complete_rest(int qid, uint32_t num_updated_rects) "%d
 qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
 qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
 qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
+qxl_io_log(int qid, const uint8_t *str) "%d %s"
 qxl_io_read_unexpected(int qid) "%d"
 qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
 qxl_io_write(int qid, const char *mode, uint64_t addr, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " val=%"PRIu64" size=%u async=%d"
-- 
1.7.1

[Qemu-devel] [PATCH 10/10] hw/qxl: support client monitor configuration via device

[Gerd Hoffmann]

From: Alon Levy <alevy@redhat.com>

Until now we used only the agent to change the monitor count and each
monitor resolution. This patch introduces the qemu part of using the
device as the mediator instead of the agent via virtio-serial.

Spice (>=0.11.5) calls the new QXLInterface::client_monitors_config,
which returns wether the interrupt is enabled, and if so and given a non
NULL monitors config will
generate an interrupt QXL_INTERRUPT_CLIENT_MONITORS_CONFIG with crc
checksum for the guest to verify a second call hasn't interfered.

The maximal number of monitors is limited on the QXLRom to 64.

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure    |    7 +++++
 hw/qxl.c     |   79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events |    6 +++-
 3 files changed, 91 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 7656c32..c75c51b 100755
--- a/configure
+++ b/configure
@@ -2709,6 +2709,9 @@ EOF
     if $pkg_config --atleast-version=0.12.0 spice-protocol >/dev/null 2>&1; then
         spice_qxl_io_monitors_config_async="yes"
     fi
+    if $pkg_config --atleast-version=0.12.2 spice-protocol > /dev/null 2>&1; then
+        spice_qxl_client_monitors_config="yes"
+    fi
   else
     if test "$spice" = "yes" ; then
       feature_not_found "spice"
@@ -3456,6 +3459,10 @@ if test "$spice_qxl_io_monitors_config_async" = "yes" ; then
   echo "CONFIG_QXL_IO_MONITORS_CONFIG_ASYNC=y" >> $config_host_mak
 fi
 
+if test "$spice_qxl_client_monitors_config" = "yes" ; then
+  echo "CONFIG_QXL_CLIENT_MONITORS_CONFIG=y" >> $config_host_mak
+fi
+
 if test "$smartcard" = "yes" ; then
   echo "CONFIG_SMARTCARD=y" >> $config_host_mak
 fi
diff --git a/hw/qxl.c b/hw/qxl.c
index 5709e0d..c464408 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -18,6 +18,8 @@
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <zlib.h>
+
 #include "qemu-common.h"
 #include "qemu-timer.h"
 #include "qemu-queue.h"
@@ -971,6 +973,79 @@ static void interface_set_client_capabilities(QXLInstance *sin,
 
 #endif
 
+#if defined(CONFIG_QXL_CLIENT_MONITORS_CONFIG) \
+    && SPICE_SERVER_VERSION >= 0x000b05
+
+static uint32_t qxl_crc32(const uint8_t *p, unsigned len)
+{
+    /*
+     * zlib xors the seed with 0xffffffff, and xors the result
+     * again with 0xffffffff; Both are not done with linux's crc32,
+     * which we want to be compatible with, so undo that.
+     */
+    return crc32(0xffffffff, p, len) ^ 0xffffffff;
+}
+
+/* called from main context only */
+static int interface_client_monitors_config(QXLInstance *sin,
+                                        VDAgentMonitorsConfig *monitors_config)
+{
+    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
+    QXLRom *rom = memory_region_get_ram_ptr(&qxl->rom_bar);
+    int i;
+
+    /*
+     * Older windows drivers set int_mask to 0 when their ISR is called,
+     * then later set it to ~0. So it doesn't relate to the actual interrupts
+     * handled. However, they are old, so clearly they don't support this
+     * interrupt
+     */
+    if (qxl->ram->int_mask == 0 || qxl->ram->int_mask == ~0 ||
+        !(qxl->ram->int_mask & QXL_INTERRUPT_CLIENT_MONITORS_CONFIG)) {
+        trace_qxl_client_monitors_config_unsupported_by_guest(qxl->id,
+                                                            qxl->ram->int_mask,
+                                                            monitors_config);
+        return 0;
+    }
+    if (!monitors_config) {
+        return 1;
+    }
+    memset(&rom->client_monitors_config, 0,
+           sizeof(rom->client_monitors_config));
+    rom->client_monitors_config.count = monitors_config->num_of_monitors;
+    /* monitors_config->flags ignored */
+    if (rom->client_monitors_config.count >=
+            ARRAY_SIZE(rom->client_monitors_config.heads)) {
+        trace_qxl_client_monitors_config_capped(qxl->id,
+                                monitors_config->num_of_monitors,
+                                ARRAY_SIZE(rom->client_monitors_config.heads));
+        rom->client_monitors_config.count =
+            ARRAY_SIZE(rom->client_monitors_config.heads);
+    }
+    for (i = 0 ; i < rom->client_monitors_config.count ; ++i) {
+        VDAgentMonConfig *monitor = &monitors_config->monitors[i];
+        QXLURect *rect = &rom->client_monitors_config.heads[i];
+        /* monitor->depth ignored */
+        rect->left = monitor->x;
+        rect->top = monitor->y;
+        rect->right = monitor->x + monitor->width;
+        rect->bottom = monitor->y + monitor->height;
+    }
+    rom->client_monitors_config_crc = qxl_crc32(
+            (const uint8_t *)&rom->client_monitors_config,
+            sizeof(rom->client_monitors_config));
+    trace_qxl_client_monitors_config_crc(qxl->id,
+            sizeof(rom->client_monitors_config),
+            rom->client_monitors_config_crc);
+
+    trace_qxl_interrupt_client_monitors_config(qxl->id,
+                        rom->client_monitors_config.count,
+                        rom->client_monitors_config.heads);
+    qxl_send_events(qxl, QXL_INTERRUPT_CLIENT_MONITORS_CONFIG);
+    return 1;
+}
+#endif
+
 static const QXLInterface qxl_interface = {
     .base.type               = SPICE_INTERFACE_QXL,
     .base.description        = "qxl gpu",
@@ -995,6 +1070,10 @@ static const QXLInterface qxl_interface = {
 #if SPICE_SERVER_VERSION >= 0x000b04
     .set_client_capabilities = interface_set_client_capabilities,
 #endif
+#if SPICE_SERVER_VERSION >= 0x000b05 && \
+    defined(CONFIG_QXL_CLIENT_MONITORS_CONFIG)
+    .client_monitors_config = interface_client_monitors_config,
+#endif
 };
 
 static void qxl_enter_vga_mode(PCIQXLDevice *d)
diff --git a/trace-events b/trace-events
index cd638f5..b48fe2d 100644
--- a/trace-events
+++ b/trace-events
@@ -932,7 +932,7 @@ qxl_interface_update_area_complete_rest(int qid, uint32_t num_updated_rects) "%d
 qxl_interface_update_area_complete_overflow(int qid, int max) "%d max=%d"
 qxl_interface_update_area_complete_schedule_bh(int qid, uint32_t num_dirty) "%d #dirty=%d"
 qxl_io_destroy_primary_ignored(int qid, const char *mode) "%d %s"
-qxl_io_log(int qid, const uint8_t *str) "%d %s"
+qxl_io_log(int qid, const uint8_t *log_buf) "%d %s"
 qxl_io_read_unexpected(int qid) "%d"
 qxl_io_unexpected_vga_mode(int qid, uint64_t addr, uint64_t val, const char *desc) "%d 0x%"PRIx64"=%"PRIu64" (%s)"
 qxl_io_write(int qid, const char *mode, uint64_t addr, uint64_t val, unsigned size, int async) "%d %s addr=%"PRIu64 " val=%"PRIu64" size=%u async=%d"
@@ -976,6 +976,10 @@ qxl_spice_update_area_rest(int qid, uint32_t num_dirty_rects, uint32_t clear_dir
 qxl_surfaces_dirty(int qid, int surface, int offset, int size) "%d surface=%d offset=%d size=%d"
 qxl_send_events(int qid, uint32_t events) "%d %d"
 qxl_set_guest_bug(int qid) "%d"
+qxl_interrupt_client_monitors_config(int qid, int num_heads, void *heads) "%d %d %p"
+qxl_client_monitors_config_unsupported_by_guest(int qid, uint32_t int_mask, void *client_monitors_config) "%d %X %p"
+qxl_client_monitors_config_capped(int qid, int requested, int limit) "%d %d %d"
+qxl_client_monitors_config_crc(int qid, unsigned size, uint32_t crc32) "%d %u %u"
 
 # hw/qxl-render.c
 qxl_render_blit_guest_primary_initialized(void) ""
-- 
1.7.1

Re: [Qemu-devel] [PULL 00/10] spice patch queue

[Michael Tokarev]

On 13.09.2012 12:45, Gerd Hoffmann wrote:
>   Hi,
> 
> This pull brings monitor configuration support for qxl which is needed
> by the upcoming kms qxl driver.  Also a nice speedup for qxl in vga mode
> and as usual some bugfixes.

Again, anything worth to apply/backport to -stable?  For example,
"qxl: dont update invalid area" by Dunrong Huang or a fix by
Hans de Goede maybe?

Thanks,

/mjt

> Alon Levy (3):
>       hw/qxl: tracing fixes
>       qxl: add trace-event for QXL_IO_LOG
>       hw/qxl: support client monitor configuration via device
> 
> Dunrong Huang (1):
>       qxl: dont update invalid area
> 
> Gerd Hoffmann (4):
>       spice: switch to queue for vga mode updates
>       spice: split qemu_spice_create_update
>       spice: add screen mirror
>       spice: send updates only for changed screen content
> 
> Hans de Goede (1):
>       qxl: Ignore set_client_capabilities pre/post migrate
> 
> Uri Lublin (1):
>       qxl: better cleanup for surface destroy

Re: [Qemu-devel] [PULL 00/10] spice patch queue

[Gerd Hoffmann]

On 09/14/12 10:01, Michael Tokarev wrote:
> On 13.09.2012 12:45, Gerd Hoffmann wrote:
>>   Hi,
>>
>> This pull brings monitor configuration support for qxl which is needed
>> by the upcoming kms qxl driver.  Also a nice speedup for qxl in vga mode
>> and as usual some bugfixes.
> 
> Again, anything worth to apply/backport to -stable?  For example,
> "qxl: dont update invalid area" by Dunrong Huang

Yes.

> or a fix by
> Hans de Goede maybe?

No, the other fixes are for new (post 1.2) bugs and wouldn't apply to
stable anyway.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 02/10] spice: split qemu_spice_create_update

[Blue Swirl]

On Thu, Sep 13, 2012 at 8:45 AM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Creating one function which creates a single update for a given
> rectangle.  And one (for now) pretty simple wrapper around it to
> queue up screen updates for the dirty region.
>
> [ v2: also update bounding box ]
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  ui/spice-display.c |   31 ++++++++++++++++++-------------
>  1 files changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 59c5fd7..6f68f28 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -164,7 +164,8 @@ int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
>  #endif
>  }
>
> -static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
> +static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
> +                                         QXLRect *rect)
>  {
>      SimpleSpiceUpdate *update;
>      QXLDrawable *drawable;
> @@ -174,24 +175,20 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>      int by, bw, bh;
>      struct timespec time_space;
>
> -    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> -        return;
> -    };
> -
>      trace_qemu_spice_create_update(
> -           ssd->dirty.left, ssd->dirty.right,
> -           ssd->dirty.top, ssd->dirty.bottom);
> +           rect->left, rect->right,
> +           rect->top, rect->bottom);
>
>      update   = g_malloc0(sizeof(*update));
>      drawable = &update->drawable;
>      image    = &update->image;
>      cmd      = &update->ext.cmd;
>
> -    bw       = ssd->dirty.right - ssd->dirty.left;
> -    bh       = ssd->dirty.bottom - ssd->dirty.top;
> +    bw       = rect->right - rect->left;
> +    bh       = rect->bottom - rect->top;
>      update->bitmap = g_malloc(bw * bh * 4);
>
> -    drawable->bbox            = ssd->dirty;
> +    drawable->bbox            = *rect;
>      drawable->clip.type       = SPICE_CLIP_TYPE_NONE;
>      drawable->effect          = QXL_EFFECT_OPAQUE;
>      drawable->release_info.id = (uintptr_t)update;
> @@ -226,8 +223,8 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>      }
>
>      src = ds_get_data(ssd->ds) +
> -        ssd->dirty.top * ds_get_linesize(ssd->ds) +
> -        ssd->dirty.left * ds_get_bytes_per_pixel(ssd->ds);
> +        rect->top * ds_get_linesize(ssd->ds) +
> +        rect->left * ds_get_bytes_per_pixel(ssd->ds);
>      dst = update->bitmap;
>      for (by = 0; by < bh; by++) {
>          qemu_pf_conv_run(ssd->conv, dst, src, bw);
> @@ -238,10 +235,18 @@ static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
>      cmd->type = QXL_CMD_DRAW;
>      cmd->data = (uintptr_t)drawable;
>
> -    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
>      QTAILQ_INSERT_TAIL(&ssd->updates, update, next);
>  }
>
> +static void qemu_spice_create_update(SimpleSpiceDisplay *ssd)
> +{
> +    if (qemu_spice_rect_is_empty(&ssd->dirty)) {
> +        return;
> +    };

Minor nitpick: useless semicolon after brace.

> +    qemu_spice_create_one_update(ssd, &ssd->dirty);
> +    memset(&ssd->dirty, 0, sizeof(ssd->dirty));
> +}
> +
>  /*
>   * Called from spice server thread context (via interface_release_ressource)
>   * We do *not* hold the global qemu mutex here, so extra care is needed
> --
> 1.7.1
>
>

Re: [Qemu-devel] [PULL 00/10] spice patch queue

[Anthony Liguori]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> This pull brings monitor configuration support for qxl which is needed
> by the upcoming kms qxl driver.  Also a nice speedup for qxl in vga mode
> and as usual some bugfixes.
>
> please pull,
>   Gerd
>
> The following changes since commit e0a1e32dbc41e6b2aabb436a9417dfd32177a3dc:
>
>   Merge branch 'usb.64' of git://git.kraxel.org/qemu (2012-09-11 18:06:56 +0200)
>
> are available in the git repository at:
>
>   git://anongit.freedesktop.org/spice/qemu spice.v60
>

Pulled. Thanks.

Regards,

Anthony Liguori

> Alon Levy (3):
>       hw/qxl: tracing fixes
>       qxl: add trace-event for QXL_IO_LOG
>       hw/qxl: support client monitor configuration via device
>
> Dunrong Huang (1):
>       qxl: dont update invalid area
>
> Gerd Hoffmann (4):
>       spice: switch to queue for vga mode updates
>       spice: split qemu_spice_create_update
>       spice: add screen mirror
>       spice: send updates only for changed screen content
>
> Hans de Goede (1):
>       qxl: Ignore set_client_capabilities pre/post migrate
>
> Uri Lublin (1):
>       qxl: better cleanup for surface destroy
>
>  configure          |    7 +++
>  hw/qxl.c           |  107 ++++++++++++++++++++++++++++++++++++++--
>  trace-events       |   11 ++++-
>  ui/spice-display.c |  136 ++++++++++++++++++++++++++++++++++++++++------------
>  ui/spice-display.h |    4 +-
>  5 files changed, 225 insertions(+), 40 deletions(-)

Re: [Qemu-devel] [PATCH 05/10] qxl: dont update invalid area

[Michael Tokarev]

On 13.09.2012 12:45, Gerd Hoffmann wrote:
> From: Dunrong Huang <riegamaths@gmail.com>
> 
> This patch fixes the following error:
> 
> $ ~/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -spice port=5900,disable-ticketing -vga qxl -cdrom ~/Images/linuxmint-13-mate-dvd-32bit.iso
> (/home/mathslinux/usr/bin/qemu-system-x86_64:10068): SpiceWorker-CRITICAL **: red_worker.c:4599:red_update_area: condition `area->left >= 0 && area->top >= 0 && area->left < area->right && area->top < area->bottom' failed
> Aborted
> 
> spice server terminates QEMU process if we pass invalid area to it,
> so dont update those invalid areas.
> 
> Signed-off-by: Dunrong Huang <riegamaths@gmail.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/qxl.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/qxl.c b/hw/qxl.c
> index 257a37d..0176b1a 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -1470,6 +1470,13 @@ async_common:
>              return;
>          }
>  
> +        if (update.left < 0 || update.top < 0 || update.left >= update.right ||
> +            update.top >= update.bottom) {
> +            qxl_set_guest_bug(d, "QXL_IO_UPDATE_AREA: "
> +                              "invalid area(%d,%d,%d,%d)\n", update.left,
> +                              update.right, update.top, update.bottom);
> +            break;
> +        }

Please take a look at the previous chunk of code, which was
added in 511b13e2c9b426b3c56060909693de5097f0b496
"qxl/update_area_io: guest_bug on invalid parameters" by alevy:

+        if (d->ram->update_surface > NUM_SURFACES) {
+            qxl_set_guest_bug(d, "QXL_IO_UPDATE_AREA: invalid surface id %d\n",
+                              d->ram->update_surface);
+            return;
+        }
+        if (update.left >= update.right || update.top >= update.bottom) {
+            qxl_set_guest_bug(d,
+                    "QXL_IO_UPDATE_AREA: invalid area (%ux%u)x(%ux%u)\n",
+                    update.left, update.top, update.right, update.bottom);
+            return;
+        }
+

Now, this place looks.. well.. funny.

A (trivial) cleanup patch is on the way.

Thanks,

/mjt