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

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

[Gerd Hoffmann]

  Hi,

Here comes the spice patch queue.  Nothing major.  Most patches make qxl
robust against guest bugs, by making qxl logging a message and raising
an error IRQ instead of calling abort().  And a few other bugfixes.

please pull,
  Gerd

Alon Levy (12):
  spice: require spice-protocol >= 0.8.1
  spice_info: add mouse_mode
  hw/qxl.c: qxl_phys2virt: replace panics with guest_bug
  qxl: check for NULL return from qxl_phys2virt
  qxl: replace panic with guest bug in qxl_track_command
  qxl: fix > 80 chars line
  qxl: don't abort on guest trigerrable ring indices mismatch
  qxl: cleanup s/__FUNCTION__/__func__/
  qxl: interface_notify_update: remove guest trigerrable abort
  qxl: qxl_add_memslot: remove guest trigerrable panics
  qxl: ioport_write: remove guest trigerrable abort
  qxl: don't assert on guest create_guest_primary

 configure        |    1 +
 hmp.c            |    2 +
 hw/qxl-logger.c  |   51 ++++++++++++++-----
 hw/qxl-render.c  |   14 ++++-
 hw/qxl.c         |  145 +++++++++++++++++++++++++++++++++++++++++-------------
 hw/qxl.h         |    6 +-
 qapi-schema.json |   27 ++++++++++-
 ui/spice-core.c  |    7 +++
 8 files changed, 198 insertions(+), 55 deletions(-)

The following changes since commit 58a331bab950e9a12da8b17b57e11927893eaf5e:

  Merge remote-tracking branch 'kwolf/for-anthony' into staging (2012-05-02 14:49:53 -0500)

are available in the git repository at:

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

Alon Levy (12):
      spice: require spice-protocol >= 0.8.1
      spice_info: add mouse_mode
      hw/qxl.c: qxl_phys2virt: replace panics with guest_bug
      qxl: check for NULL return from qxl_phys2virt
      qxl: replace panic with guest bug in qxl_track_command
      qxl: fix > 80 chars line
      qxl: don't abort on guest trigerrable ring indices mismatch
      qxl: cleanup s/__FUNCTION__/__func__/
      qxl: interface_notify_update: remove guest trigerrable abort
      qxl: qxl_add_memslot: remove guest trigerrable panics
      qxl: ioport_write: remove guest trigerrable abort
      qxl: don't assert on guest create_guest_primary

 configure        |    1 +
 hmp.c            |    2 +
 hw/qxl-logger.c  |   51 ++++++++++++++-----
 hw/qxl-render.c  |   14 ++++-
 hw/qxl.c         |  145 +++++++++++++++++++++++++++++++++++++++++-------------
 hw/qxl.h         |    6 +-
 qapi-schema.json |   27 ++++++++++-
 ui/spice-core.c  |    7 +++
 8 files changed, 198 insertions(+), 55 deletions(-)

[Qemu-devel] [PATCH 01/12] spice: require spice-protocol >= 0.8.1

[Gerd Hoffmann]

From: Alon Levy <alevy@redhat.com>

Requiring spice-server >= 0.8.2 is not enough since spice-server.pc
doesn't require spice-protocol (any version). Until that is fixed
upstream an explicit requirement in qemu fixes compilation broken since

commit 2e1a98c9c1b90ca093278c6b43244dc46604d7b7
Author: Alon Levy <alevy@redhat.com>
Date:   Fri Feb 24 23:19:30 2012 +0200

    qxl: introduce QXLCookie

Reported-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 configure |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 0111774..62aa760 100755
--- a/configure
+++ b/configure
@@ -2592,6 +2592,7 @@ EOF
   spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null)
   spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null)
   if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \
+     $pkg_config --atleast-version=0.8.1 spice-protocol > /dev/null 2>&1 && \
      compile_prog "$spice_cflags" "$spice_libs" ; then
     spice="yes"
     libs_softmmu="$libs_softmmu $spice_libs"
-- 
1.7.1

[Qemu-devel] [PATCH 02/12] spice_info: add mouse_mode

[Gerd Hoffmann]

From: Alon Levy <alevy@redhat.com>

Add mouse_mode, either server or mouse, to qmp and hmp commands, based
on spice_server_is_server_mouse added in spice-server 0.10.3.

Signed-off-by: Alon Levy <alevy@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp.c            |    2 ++
 qapi-schema.json |   27 ++++++++++++++++++++++++++-
 ui/spice-core.c  |    7 +++++++
 3 files changed, 35 insertions(+), 1 deletions(-)

diff --git a/hmp.c b/hmp.c
index eb96618..1f9fe0e 100644
--- a/hmp.c
+++ b/hmp.c
@@ -350,6 +350,8 @@ void hmp_info_spice(Monitor *mon)
     }
     monitor_printf(mon, "        auth: %s\n", info->auth);
     monitor_printf(mon, "    compiled: %s\n", info->compiled_version);
+    monitor_printf(mon, "  mouse-mode: %s\n",
+                   SpiceQueryMouseMode_lookup[info->mouse_mode]);
 
     if (!info->has_channels || info->channels == NULL) {
         monitor_printf(mon, "Channels: none\n");
diff --git a/qapi-schema.json b/qapi-schema.json
index 9193fb9..4279259 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -634,6 +634,25 @@
            'tls': 'bool'} }
 
 ##
+# @SpiceQueryMouseMode
+#
+# An enumation of Spice mouse states.
+#
+# @client: Mouse cursor position is determined by the client.
+#
+# @server: Mouse cursor position is determined by the server.
+#
+# @unknown: No information is available about mouse mode used by
+#           the spice server.
+#
+# Note: spice/enums.h has a SpiceMouseMode already, hence the name.
+#
+# Since: 1.1
+##
+{ 'enum': 'SpiceQueryMouseMode',
+  'data': [ 'client', 'server', 'unknown' ] }
+
+##
 # @SpiceInfo
 #
 # Information about the SPICE session.
@@ -654,6 +673,12 @@
 #        'spice' uses SASL or direct TLS authentication, depending on command
 #                line options
 #
+# @mouse-mode: The mode in which the mouse cursor is displayed currently. Can
+#              be determined by the client or the server, or unknown if spice
+#              server doesn't provide this information.
+#
+#              Since: 1.1
+#
 # @channels: a list of @SpiceChannel for each active spice channel
 #
 # Since: 0.14.0
@@ -661,7 +686,7 @@
 { 'type': 'SpiceInfo',
   'data': {'enabled': 'bool', '*host': 'str', '*port': 'int',
            '*tls-port': 'int', '*auth': 'str', '*compiled-version': 'str',
-           '*channels': ['SpiceChannel']} }
+           'mouse-mode': 'SpiceQueryMouseMode', '*channels': ['SpiceChannel']} }
 
 ##
 # @query-spice
diff --git a/ui/spice-core.c b/ui/spice-core.c
index a468524..4fc48f8 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -462,6 +462,13 @@ SpiceInfo *qmp_query_spice(Error **errp)
         info->tls_port = tls_port;
     }
 
+#if SPICE_SERVER_VERSION >= 0x000a03 /* 0.10.3 */
+    info->mouse_mode = spice_server_is_server_mouse(spice_server) ?
+                       SPICE_QUERY_MOUSE_MODE_SERVER :
+                       SPICE_QUERY_MOUSE_MODE_CLIENT;
+#else
+    info->mouse_mode = SPICE_QUERY_MOUSE_MODE_UNKNOWN;
+#endif
     /* for compatibility with the original command */
     info->has_channels = true;
     info->channels = qmp_query_spice_channels();
-- 
1.7.1

[Qemu-devel] [PATCH 03/12] hw/qxl.c: qxl_phys2virt: replace panics with guest_bug

[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 |   25 +++++++++++++++++++------
 1 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index c3540c3..9e8cdf3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1097,15 +1097,28 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
     case MEMSLOT_GROUP_HOST:
         return (void *)(intptr_t)offset;
     case MEMSLOT_GROUP_GUEST:
-        PANIC_ON(slot >= NUM_MEMSLOTS);
-        PANIC_ON(!qxl->guest_slots[slot].active);
-        PANIC_ON(offset < qxl->guest_slots[slot].delta);
+        if (slot >= NUM_MEMSLOTS) {
+            qxl_guest_bug(qxl, "slot too large %d >= %d", slot, NUM_MEMSLOTS);
+            return NULL;
+        }
+        if (!qxl->guest_slots[slot].active) {
+            qxl_guest_bug(qxl, "inactive slot %d\n", slot);
+            return NULL;
+        }
+        if (offset < qxl->guest_slots[slot].delta) {
+            qxl_guest_bug(qxl, "slot %d offset %"PRIu64" < delta %"PRIu64"\n",
+                          slot, offset, qxl->guest_slots[slot].delta);
+            return NULL;
+        }
         offset -= qxl->guest_slots[slot].delta;
-        PANIC_ON(offset > qxl->guest_slots[slot].size)
+        if (offset > qxl->guest_slots[slot].size) {
+            qxl_guest_bug(qxl, "slot %d offset %"PRIu64" > size %"PRIu64"\n",
+                          slot, offset, qxl->guest_slots[slot].size);
+            return NULL;
+        }
         return qxl->guest_slots[slot].ptr + offset;
-    default:
-        PANIC_ON(1);
     }
+    return NULL;
 }
 
 static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
-- 
1.7.1

[Qemu-devel] [PATCH 04/12] qxl: check for NULL return from qxl_phys2virt

[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-logger.c |   51 ++++++++++++++++++++++++++++++++++++++-------------
 hw/qxl-render.c |   14 +++++++++++---
 hw/qxl.c        |   13 ++++++++++++-
 hw/qxl.h        |    6 +++---
 4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/hw/qxl-logger.c b/hw/qxl-logger.c
index 367aad1..fe2878c 100644
--- a/hw/qxl-logger.c
+++ b/hw/qxl-logger.c
@@ -100,12 +100,15 @@ static const char *qxl_v2n(const char *n[], size_t l, int v)
 }
 #define qxl_name(_list, _value) qxl_v2n(_list, ARRAY_SIZE(_list), _value)
 
-static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
+static int qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
 {
     QXLImage *image;
     QXLImageDescriptor *desc;
 
     image = qxl_phys2virt(qxl, addr, group_id);
+    if (!image) {
+        return 1;
+    }
     desc = &image->descriptor;
     fprintf(stderr, " (id %" PRIx64 " type %d flags %d width %d height %d",
             desc->id, desc->type, desc->flags, desc->width, desc->height);
@@ -120,6 +123,7 @@ static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
         break;
     }
     fprintf(stderr, ")");
+    return 0;
 }
 
 static void qxl_log_rect(QXLRect *rect)
@@ -130,17 +134,24 @@ static void qxl_log_rect(QXLRect *rect)
             rect->left, rect->top);
 }
 
-static void qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy, int group_id)
+static int qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy,
+                                 int group_id)
 {
+    int ret;
+
     fprintf(stderr, " src %" PRIx64,
             copy->src_bitmap);
-    qxl_log_image(qxl, copy->src_bitmap, group_id);
+    ret = qxl_log_image(qxl, copy->src_bitmap, group_id);
+    if (ret != 0) {
+        return ret;
+    }
     fprintf(stderr, " area");
     qxl_log_rect(&copy->src_area);
     fprintf(stderr, " rop %d", copy->rop_descriptor);
+    return 0;
 }
 
-static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
+static int qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
 {
     fprintf(stderr, ": surface_id %d type %s effect %s",
             draw->surface_id,
@@ -148,13 +159,14 @@ static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
             qxl_name(qxl_draw_effect, draw->effect));
     switch (draw->type) {
     case QXL_DRAW_COPY:
-        qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
+        return qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
         break;
     }
+    return 0;
 }
 
-static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
-                                    int group_id)
+static int qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
+                                   int group_id)
 {
     fprintf(stderr, ": type %s effect %s",
             qxl_name(qxl_draw_type, draw->type),
@@ -166,9 +178,10 @@ static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
     }
     switch (draw->type) {
     case QXL_DRAW_COPY:
-        qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
+        return qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
         break;
     }
+    return 0;
 }
 
 static void qxl_log_cmd_surface(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
@@ -189,7 +202,7 @@ static void qxl_log_cmd_surface(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
     }
 }
 
-void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
+int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
 {
     QXLCursor *cursor;
 
@@ -203,6 +216,9 @@ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
                 cmd->u.set.visible ? "yes" : "no",
                 cmd->u.set.shape);
         cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id);
+        if (!cursor) {
+            return 1;
+        }
         fprintf(stderr, " type %s size %dx%d hot-spot +%d+%d"
                 " unique 0x%" PRIx64 " data-size %d",
                 qxl_name(spice_cursor_type, cursor->header.type),
@@ -214,15 +230,17 @@ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
         fprintf(stderr, " +%d+%d", cmd->u.position.x, cmd->u.position.y);
         break;
     }
+    return 0;
 }
 
-void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
+int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
 {
     bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT;
     void *data;
+    int ret;
 
     if (!qxl->cmdlog) {
-        return;
+        return 0;
     }
     fprintf(stderr, "%" PRId64 " qxl-%d/%s:", qemu_get_clock_ns(vm_clock),
             qxl->id, ring);
@@ -231,12 +249,18 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
             compat ? "(compat)" : "");
 
     data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+    if (!data) {
+        return 1;
+    }
     switch (ext->cmd.type) {
     case QXL_CMD_DRAW:
         if (!compat) {
-            qxl_log_cmd_draw(qxl, data, ext->group_id);
+            ret = qxl_log_cmd_draw(qxl, data, ext->group_id);
         } else {
-            qxl_log_cmd_draw_compat(qxl, data, ext->group_id);
+            ret = qxl_log_cmd_draw_compat(qxl, data, ext->group_id);
+        }
+        if (ret) {
+            return ret;
         }
         break;
     case QXL_CMD_SURFACE:
@@ -247,4 +271,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
         break;
     }
     fprintf(stderr, "\n");
+    return 0;
 }
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index f7f1bfd..e2e3fe2 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -228,14 +228,18 @@ fail:
 
 
 /* called from spice server thread context only */
-void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
+int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
 {
     QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
     QXLCursor *cursor;
     QEMUCursor *c;
 
+    if (!cmd) {
+        return 1;
+    }
+
     if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) {
-        return;
+        return 0;
     }
 
     if (qxl->debug > 1 && cmd->type != QXL_CURSOR_MOVE) {
@@ -246,9 +250,12 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
     switch (cmd->type) {
     case QXL_CURSOR_SET:
         cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id);
+        if (!cursor) {
+            return 1;
+        }
         if (cursor->chunk.data_size != cursor->data_size) {
             fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__);
-            return;
+            return 1;
         }
         c = qxl_cursor(qxl, cursor);
         if (c == NULL) {
@@ -270,4 +277,5 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
         qemu_mutex_unlock(&qxl->ssd.lock);
         break;
     }
+    return 0;
 }
diff --git a/hw/qxl.c b/hw/qxl.c
index 9e8cdf3..b6a738e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -383,12 +383,16 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl)
  * keep track of some command state, for savevm/loadvm.
  * called from spice server thread context only
  */
-static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
+static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
 {
     switch (le32_to_cpu(ext->cmd.type)) {
     case QXL_CMD_SURFACE:
     {
         QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+
+        if (!cmd) {
+            return 1;
+        }
         uint32_t id = le32_to_cpu(cmd->surface_id);
         PANIC_ON(id >= NUM_SURFACES);
         qemu_mutex_lock(&qxl->track_lock);
@@ -408,6 +412,10 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
     case QXL_CMD_CURSOR:
     {
         QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+
+        if (!cmd) {
+            return 1;
+        }
         if (cmd->type == QXL_CURSOR_SET) {
             qemu_mutex_lock(&qxl->track_lock);
             qxl->guest_cursor = ext->cmd.data;
@@ -416,6 +424,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
         break;
     }
     }
+    return 0;
 }
 
 /* spice display interface callbacks */
@@ -1568,10 +1577,12 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
 
         cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i],
                             MEMSLOT_GROUP_GUEST);
+        assert(cmd);
         assert(cmd->type == QXL_SURFACE_CMD_CREATE);
         surface_offset = (intptr_t)qxl_phys2virt(qxl,
                                                  cmd->u.surface_create.data,
                                                  MEMSLOT_GROUP_GUEST);
+        assert(surface_offset);
         surface_offset -= vram_start;
         surface_size = cmd->u.surface_create.height *
                        abs(cmd->u.surface_create.stride);
diff --git a/hw/qxl.h b/hw/qxl.h
index cbb1e2d..3102950 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -142,12 +142,12 @@ void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
 
 /* qxl-logger.c */
-void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
-void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
+int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
+int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
 
 /* qxl-render.c */
 void qxl_render_resize(PCIQXLDevice *qxl);
 void qxl_render_update(PCIQXLDevice *qxl);
-void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
+int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
 void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
 void qxl_render_update_area_bh(void *opaque);
-- 
1.7.1

[Qemu-devel] [PATCH 05/12] qxl: replace panic with guest bug in qxl_track_command

[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 |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b6a738e..8b66cbb 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -394,7 +394,11 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
             return 1;
         }
         uint32_t id = le32_to_cpu(cmd->surface_id);
-        PANIC_ON(id >= NUM_SURFACES);
+
+        if (id >= NUM_SURFACES) {
+            qxl_guest_bug(qxl, "QXL_CMD_SURFACE id %d >= %d", id, NUM_SURFACES);
+            return 1;
+        }
         qemu_mutex_lock(&qxl->track_lock);
         if (cmd->type == QXL_SURFACE_CMD_CREATE) {
             qxl->guest_surfaces.cmds[id] = ext->cmd.data;
-- 
1.7.1

[Qemu-devel] [PATCH 06/12] qxl: fix > 80 chars line

[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 |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 8b66cbb..b22f86e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -777,8 +777,8 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
     }
     if (cookie && current_async != cookie->io) {
         fprintf(stderr,
-                "qxl: %s: error: current_async = %d != %" PRId64 " = cookie->io\n",
-                __func__, current_async, cookie->io);
+                "qxl: %s: error: current_async = %d != %"
+                PRId64 " = cookie->io\n", __func__, current_async, cookie->io);
     }
     switch (current_async) {
     case QXL_IO_MEMSLOT_ADD_ASYNC:
-- 
1.7.1

[Qemu-devel] [PATCH 07/12] qxl: don't abort on guest trigerrable ring indices mismatch

[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 |   51 +++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b22f86e..44a4c9b 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -27,28 +27,42 @@
 
 #include "qxl.h"
 
+/*
+ * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as
+ * such can be changed by the guest, so to avoid a guest trigerrable
+ * abort we just set qxl_guest_bug and set the return to NULL. Still
+ * it may happen as a result of emulator bug as well.
+ */
 #undef SPICE_RING_PROD_ITEM
-#define SPICE_RING_PROD_ITEM(r, ret) {                                  \
+#define SPICE_RING_PROD_ITEM(qxl, r, ret) {                             \
         typeof(r) start = r;                                            \
         typeof(r) end = r + 1;                                          \
         uint32_t prod = (r)->prod & SPICE_RING_INDEX_MASK(r);           \
         typeof(&(r)->items[prod]) m_item = &(r)->items[prod];           \
         if (!((uint8_t*)m_item >= (uint8_t*)(start) && (uint8_t*)(m_item + 1) <= (uint8_t*)(end))) { \
-            abort();                                                    \
+            qxl_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " \
+                          "! %p <= %p < %p", (uint8_t *)start,          \
+                          (uint8_t *)m_item, (uint8_t *)end);           \
+            ret = NULL;                                                 \
+        } else {                                                        \
+            ret = &m_item->el;                                          \
         }                                                               \
-        ret = &m_item->el;                                              \
     }
 
 #undef SPICE_RING_CONS_ITEM
-#define SPICE_RING_CONS_ITEM(r, ret) {                                  \
+#define SPICE_RING_CONS_ITEM(qxl, r, ret) {                             \
         typeof(r) start = r;                                            \
         typeof(r) end = r + 1;                                          \
         uint32_t cons = (r)->cons & SPICE_RING_INDEX_MASK(r);           \
         typeof(&(r)->items[cons]) m_item = &(r)->items[cons];           \
         if (!((uint8_t*)m_item >= (uint8_t*)(start) && (uint8_t*)(m_item + 1) <= (uint8_t*)(end))) { \
-            abort();                                                    \
+            qxl_guest_bug(qxl, "SPICE_RING_CONS_ITEM indices mismatch " \
+                          "! %p <= %p < %p", (uint8_t *)start,          \
+                          (uint8_t *)m_item, (uint8_t *)end);           \
+            ret = NULL;                                                 \
+        } else {                                                        \
+            ret = &m_item->el;                                          \
         }                                                               \
-        ret = &m_item->el;                                              \
     }
 
 #undef ALIGN
@@ -343,7 +357,8 @@ static void init_qxl_ram(PCIQXLDevice *d)
     SPICE_RING_INIT(&d->ram->cmd_ring);
     SPICE_RING_INIT(&d->ram->cursor_ring);
     SPICE_RING_INIT(&d->ram->release_ring);
-    SPICE_RING_PROD_ITEM(&d->ram->release_ring, item);
+    SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item);
+    assert(item);
     *item = 0;
     qxl_ring_set_dirty(d);
 }
@@ -559,8 +574,10 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
         if (SPICE_RING_IS_EMPTY(ring)) {
             return false;
         }
-        trace_qxl_ring_command_get(qxl->id, qxl_mode_to_string(qxl->mode));
-        SPICE_RING_CONS_ITEM(ring, cmd);
+        SPICE_RING_CONS_ITEM(qxl, ring, cmd);
+        if (!cmd) {
+            return false;
+        }
         ext->cmd      = *cmd;
         ext->group_id = MEMSLOT_GROUP_GUEST;
         ext->flags    = qxl->cmdflags;
@@ -572,6 +589,7 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
         qxl->guest_primary.commands++;
         qxl_track_command(qxl, ext);
         qxl_log_command(qxl, "cmd", ext);
+        trace_qxl_ring_command_get(qxl->id, qxl_mode_to_string(qxl->mode));
         return true;
     default:
         return false;
@@ -630,7 +648,10 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush)
     if (notify) {
         qxl_send_events(d, QXL_INTERRUPT_DISPLAY);
     }
-    SPICE_RING_PROD_ITEM(ring, item);
+    SPICE_RING_PROD_ITEM(d, ring, item);
+    if (!item) {
+        return;
+    }
     *item = 0;
     d->num_free_res = 0;
     d->last_release = NULL;
@@ -656,7 +677,10 @@ static void interface_release_resource(QXLInstance *sin,
      * pci bar 0, $command.release_info
      */
     ring = &qxl->ram->release_ring;
-    SPICE_RING_PROD_ITEM(ring, item);
+    SPICE_RING_PROD_ITEM(qxl, ring, item);
+    if (!item) {
+        return;
+    }
     if (*item == 0) {
         /* stick head into the ring */
         id = ext.info->id;
@@ -695,7 +719,10 @@ static int interface_get_cursor_command(QXLInstance *sin, struct QXLCommandExt *
         if (SPICE_RING_IS_EMPTY(ring)) {
             return false;
         }
-        SPICE_RING_CONS_ITEM(ring, cmd);
+        SPICE_RING_CONS_ITEM(qxl, ring, cmd);
+        if (!cmd) {
+            return false;
+        }
         ext->cmd      = *cmd;
         ext->group_id = MEMSLOT_GROUP_GUEST;
         ext->flags    = qxl->cmdflags;
-- 
1.7.1

[Qemu-devel] [PATCH 08/12] qxl: cleanup s/__FUNCTION__/__func__/

[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 |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 44a4c9b..6e7232c 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1503,7 +1503,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
         qxl_update_irq(d);
     } else {
         if (write(d->pipe[1], d, 1) != 1) {
-            dprint(d, 1, "%s: write to pipe failed\n", __FUNCTION__);
+            dprint(d, 1, "%s: write to pipe failed\n", __func__);
         }
     }
 }
-- 
1.7.1

[Qemu-devel] [PATCH 09/12] qxl: interface_notify_update: remove guest trigerrable abort

[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 |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 6e7232c..44ee495 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -768,8 +768,13 @@ static int interface_req_cursor_notification(QXLInstance *sin)
 /* called from spice server thread context */
 static void interface_notify_update(QXLInstance *sin, uint32_t update_id)
 {
-    fprintf(stderr, "%s: abort()\n", __FUNCTION__);
-    abort();
+    /*
+     * Called by spice-server as a result of a QXL_CMD_UPDATE which is not in
+     * use by xf86-video-qxl and is defined out in the qxl windows driver.
+     * Probably was at some earlier version that is prior to git start (2009),
+     * and is still guest trigerrable.
+     */
+    fprintf(stderr, "%s: deprecated\n", __func__);
 }
 
 /* called from spice server thread context only */
-- 
1.7.1

[Qemu-devel] [PATCH 10/12] qxl: qxl_add_memslot: remove guest trigerrable panics

[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 |   27 ++++++++++++++++++++-------
 1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 44ee495..44a167a 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1038,8 +1038,8 @@ static const MemoryRegionPortio qxl_vga_portio_list[] = {
     PORTIO_END_OF_LIST(),
 };
 
-static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
-                            qxl_async_io async)
+static int qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
+                           qxl_async_io async)
 {
     static const int regions[] = {
         QXL_RAM_RANGE_INDEX,
@@ -1060,8 +1060,16 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
 
     trace_qxl_memslot_add_guest(d->id, slot_id, guest_start, guest_end);
 
-    PANIC_ON(slot_id >= NUM_MEMSLOTS);
-    PANIC_ON(guest_start > guest_end);
+    if (slot_id >= NUM_MEMSLOTS) {
+        qxl_guest_bug(d, "%s: slot_id >= NUM_MEMSLOTS %d >= %d", __func__,
+                      slot_id, NUM_MEMSLOTS);
+        return 1;
+    }
+    if (guest_start > guest_end) {
+        qxl_guest_bug(d, "%s: guest_start > guest_end 0x%" PRIx64
+                         " > 0x%" PRIx64, __func__, guest_start, guest_end);
+        return 1;
+    }
 
     for (i = 0; i < ARRAY_SIZE(regions); i++) {
         pci_region = regions[i];
@@ -1082,7 +1090,10 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
         /* passed */
         break;
     }
-    PANIC_ON(i == ARRAY_SIZE(regions)); /* finished loop without match */
+    if (i == ARRAY_SIZE(regions)) {
+        qxl_guest_bug(d, "%s: finished loop without match", __func__);
+        return 1;
+    }
 
     switch (pci_region) {
     case QXL_RAM_RANGE_INDEX:
@@ -1094,7 +1105,8 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
         break;
     default:
         /* should not happen */
-        abort();
+        qxl_guest_bug(d, "%s: pci_region = %d", __func__, pci_region);
+        return 1;
     }
 
     memslot.slot_id = slot_id;
@@ -1110,6 +1122,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
     d->guest_slots[slot_id].size = memslot.virt_end - memslot.virt_start;
     d->guest_slots[slot_id].delta = delta;
     d->guest_slots[slot_id].active = 1;
+    return 0;
 }
 
 static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
@@ -1250,7 +1263,7 @@ static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
     }
 
     d->guest_slots[0].slot = slot;
-    qxl_add_memslot(d, 0, devmem, QXL_SYNC);
+    assert(qxl_add_memslot(d, 0, devmem, QXL_SYNC) == 0);
 
     d->guest_primary.surface = surface;
     qxl_create_guest_primary(d, 0, QXL_SYNC);
-- 
1.7.1

[Qemu-devel] [PATCH 11/12] qxl: ioport_write: remove guest trigerrable abort

[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 |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 44a167a..c614c91 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1464,8 +1464,7 @@ async_common:
         qxl_spice_destroy_surfaces(d, async);
         break;
     default:
-        fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
-        abort();
+        qxl_guest_bug(d, "%s: unexpected ioport=0x%x\n", __func__, io_port);
     }
     return;
 cancel_async:
-- 
1.7.1

[Qemu-devel] [PATCH 12/12] qxl: don't assert on guest create_guest_primary

[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 |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index c614c91..6c11e70 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1191,7 +1191,10 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
     QXLDevSurfaceCreate surface;
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
 
-    assert(qxl->mode != QXL_MODE_NATIVE);
+    if (qxl->mode == QXL_MODE_NATIVE) {
+        qxl_guest_bug(qxl, "%s: nop since already in QXL_MODE_NATIVE",
+                      __func__);
+    }
     qxl_exit_vga_mode(qxl);
 
     surface.format     = le32_to_cpu(sc->format);
-- 
1.7.1

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

[Anthony Liguori]

On 05/03/2012 03:53 AM, Gerd Hoffmann wrote:
>    Hi,
>
> Here comes the spice patch queue.  Nothing major.  Most patches make qxl
> robust against guest bugs, by making qxl logging a message and raising
> an error IRQ instead of calling abort().  And a few other bugfixes.

Pulled.  Thanks.

Regards,

Anthony Liguori

> please pull,
>    Gerd
>
> Alon Levy (12):
>    spice: require spice-protocol>= 0.8.1
>    spice_info: add mouse_mode
>    hw/qxl.c: qxl_phys2virt: replace panics with guest_bug
>    qxl: check for NULL return from qxl_phys2virt
>    qxl: replace panic with guest bug in qxl_track_command
>    qxl: fix>  80 chars line
>    qxl: don't abort on guest trigerrable ring indices mismatch
>    qxl: cleanup s/__FUNCTION__/__func__/
>    qxl: interface_notify_update: remove guest trigerrable abort
>    qxl: qxl_add_memslot: remove guest trigerrable panics
>    qxl: ioport_write: remove guest trigerrable abort
>    qxl: don't assert on guest create_guest_primary
>
>   configure        |    1 +
>   hmp.c            |    2 +
>   hw/qxl-logger.c  |   51 ++++++++++++++-----
>   hw/qxl-render.c  |   14 ++++-
>   hw/qxl.c         |  145 +++++++++++++++++++++++++++++++++++++++++-------------
>   hw/qxl.h         |    6 +-
>   qapi-schema.json |   27 ++++++++++-
>   ui/spice-core.c  |    7 +++
>   8 files changed, 198 insertions(+), 55 deletions(-)
>
> The following changes since commit 58a331bab950e9a12da8b17b57e11927893eaf5e:
>
>    Merge remote-tracking branch 'kwolf/for-anthony' into staging (2012-05-02 14:49:53 -0500)
>
> are available in the git repository at:
>
>    git://anongit.freedesktop.org/spice/qemu spice.v54
>
> Alon Levy (12):
>        spice: require spice-protocol>= 0.8.1
>        spice_info: add mouse_mode
>        hw/qxl.c: qxl_phys2virt: replace panics with guest_bug
>        qxl: check for NULL return from qxl_phys2virt
>        qxl: replace panic with guest bug in qxl_track_command
>        qxl: fix>  80 chars line
>        qxl: don't abort on guest trigerrable ring indices mismatch
>        qxl: cleanup s/__FUNCTION__/__func__/
>        qxl: interface_notify_update: remove guest trigerrable abort
>        qxl: qxl_add_memslot: remove guest trigerrable panics
>        qxl: ioport_write: remove guest trigerrable abort
>        qxl: don't assert on guest create_guest_primary
>
>   configure        |    1 +
>   hmp.c            |    2 +
>   hw/qxl-logger.c  |   51 ++++++++++++++-----
>   hw/qxl-render.c  |   14 ++++-
>   hw/qxl.c         |  145 +++++++++++++++++++++++++++++++++++++++++-------------
>   hw/qxl.h         |    6 +-
>   qapi-schema.json |   27 ++++++++++-
>   ui/spice-core.c  |    7 +++
>   8 files changed, 198 insertions(+), 55 deletions(-)
>
>