All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCHv5 00/12] async + suspend reworked
@ 2011-07-14 19:13 Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 01/12] spice: add worker wrapper functions Alon Levy
                   ` (11 more replies)
  0 siblings, 12 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

v4->v5:
 * build with SPICE_INTERFACE_QXL_MINOR in {0,1}
  * particularily, I've taken Gerd's suggestion:
   * spice 0.8.2 (or the next with QXL_MINOR 1) will depend
    on spice-protocol 0.8.1 (or the next with the IO_*_ASYNC)
   * qemu continues to check only spice version, and still >= 0.6.0
   * spice-devel will drag in the right spice-protocol-devel,
    hence assuring qemu (which actually depends on qxl_dev.h
    having been updated between 0.8.0 and 0.8.1 in spice-protocol)
    will build with the right combo:

     SPICE_INTERFACE_ | spice-protocol  | qxl_dev.h with
     QXL_MINOR        |                 | QXL_IO_*_ASYNC &
                      |                 | new INTERRUPT_
     -----------------------------------------------------
     0                  current           no
     1                  current+0.1       yes

  - this unfortunately resulted in a lot of code churn.
 * dropped patch removing qxl_destroy_primary
 * returned implicit "\n" to qxl_guest_bug
 * rewrote add io_port_to_string
 * dropped QXL_COOKIE_VGA since we don't use async in qxl-render right now

Git trees:
 git://anongit.freedesktop.org/~alon/qemu            async_and_s3.v5

 git://anongit.freedesktop.org/~alon/spice           async_and_s3.v5
  * +version bump 0.8.1
  * still need to fix reversed order of dispatcher locals change on async calls (yonit)

 git://anongit.freedesktop.org/~alon/spice-protocol  s3.v3
  * +version bump 0.8.1
 git://anongit.freedesktop.org/~alon/qxl             s3.v3.async.v4
  * some problems here. but they seem to be driver only. wip.. see last patch.
 

Alon Levy (6):
  qxl: add io_port_to_string
  qxl: make qxl_guest_bug take variable arguments
  qxl: only disallow specific io's in vga mode
  qxl: async io support using new spice api
  qxl: add QXL_IO_FLUSH_{SURFACES,RELEASE} for guest S3&S4 support
  qxl: use QXL_REVISION_*

Gerd Hoffmann (6):
  spice: add worker wrapper functions.
  spice: add qemu_spice_display_init_common
  spice/qxl: move worker wrappers
  qxl: fix surface tracking & locking
  qxl: error handling fixes and cleanups.
  qxl: bump pci rev

 hw/qxl-render.c    |    4 +-
 hw/qxl.c           |  474 +++++++++++++++++++++++++++++++++++++++++++++-------
 hw/qxl.h           |   36 ++++-
 ui/spice-display.c |   93 +++++++++--
 ui/spice-display.h |   28 +++
 5 files changed, 559 insertions(+), 76 deletions(-)

-- 
1.7.6

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 01/12] spice: add worker wrapper functions.
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 02/12] spice: add qemu_spice_display_init_common Alon Levy
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Add wrapper functions for all spice worker calls.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl-render.c    |    4 +-
 hw/qxl.c           |   32 +++++++++---------
 ui/spice-display.c |   95 ++++++++++++++++++++++++++++++++++++++++++++++++---
 ui/spice-display.h |   22 ++++++++++++
 4 files changed, 129 insertions(+), 24 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 1316066..bef5f14 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -124,8 +124,8 @@ void qxl_render_update(PCIQXLDevice *qxl)
     update.bottom = qxl->guest_primary.surface.height;
 
     memset(dirty, 0, sizeof(dirty));
-    qxl->ssd.worker->update_area(qxl->ssd.worker, 0, &update,
-                                 dirty, ARRAY_SIZE(dirty), 1);
+    qemu_spice_update_area(&qxl->ssd, 0, &update,
+                           dirty, ARRAY_SIZE(dirty), 1);
 
     for (i = 0; i < ARRAY_SIZE(dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
diff --git a/hw/qxl.c b/hw/qxl.c
index 919ec91..545074d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -690,8 +690,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
     dprint(d, 1, "%s: start%s\n", __FUNCTION__,
            loadvm ? " (loadvm)" : "");
 
-    d->ssd.worker->reset_cursor(d->ssd.worker);
-    d->ssd.worker->reset_image_cache(d->ssd.worker);
+    qemu_spice_reset_cursor(&d->ssd);
+    qemu_spice_reset_image_cache(&d->ssd);
     qxl_reset_surfaces(d);
     qxl_reset_memslots(d);
 
@@ -796,7 +796,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
            __FUNCTION__, memslot.slot_id,
            memslot.virt_start, memslot.virt_end);
 
-    d->ssd.worker->add_memslot(d->ssd.worker, &memslot);
+    qemu_spice_add_memslot(&d->ssd, &memslot);
     d->guest_slots[slot_id].ptr = (void*)memslot.virt_start;
     d->guest_slots[slot_id].size = memslot.virt_end - memslot.virt_start;
     d->guest_slots[slot_id].delta = delta;
@@ -806,14 +806,14 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
 static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
 {
     dprint(d, 1, "%s: slot %d\n", __FUNCTION__, slot_id);
-    d->ssd.worker->del_memslot(d->ssd.worker, MEMSLOT_GROUP_HOST, slot_id);
+    qemu_spice_del_memslot(&d->ssd, MEMSLOT_GROUP_HOST, slot_id);
     d->guest_slots[slot_id].active = 0;
 }
 
 static void qxl_reset_memslots(PCIQXLDevice *d)
 {
     dprint(d, 1, "%s:\n", __FUNCTION__);
-    d->ssd.worker->reset_memslots(d->ssd.worker);
+    qemu_spice_reset_memslots(&d->ssd);
     memset(&d->guest_slots, 0, sizeof(d->guest_slots));
 }
 
@@ -821,7 +821,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
 {
     dprint(d, 1, "%s:\n", __FUNCTION__);
     d->mode = QXL_MODE_UNDEFINED;
-    d->ssd.worker->destroy_surfaces(d->ssd.worker);
+    qemu_spice_destroy_surfaces(&d->ssd);
     memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
 }
 
@@ -875,7 +875,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
 
     qxl->mode = QXL_MODE_NATIVE;
     qxl->cmdflags = 0;
-    qxl->ssd.worker->create_primary_surface(qxl->ssd.worker, 0, &surface);
+    qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface);
 
     /* for local rendering */
     qxl_render_resize(qxl);
@@ -890,7 +890,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
     dprint(d, 1, "%s\n", __FUNCTION__);
 
     d->mode = QXL_MODE_UNDEFINED;
-    d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0);
+    qemu_spice_destroy_primary_surface(&d->ssd, 0);
 }
 
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -962,15 +962,15 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_UPDATE_AREA:
     {
         QXLRect update = d->ram->update_area;
-        d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface,
-                                   &update, NULL, 0, 0);
+        qemu_spice_update_area(&d->ssd, d->ram->update_surface,
+                               &update, NULL, 0, 0);
         break;
     }
     case QXL_IO_NOTIFY_CMD:
-        d->ssd.worker->wakeup(d->ssd.worker);
+        qemu_spice_wakeup(&d->ssd);
         break;
     case QXL_IO_NOTIFY_CURSOR:
-        d->ssd.worker->wakeup(d->ssd.worker);
+        qemu_spice_wakeup(&d->ssd);
         break;
     case QXL_IO_UPDATE_IRQ:
         qxl_set_irq(d);
@@ -984,7 +984,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         }
         d->oom_running = 1;
-        d->ssd.worker->oom(d->ssd.worker);
+        qemu_spice_oom(&d->ssd);
         d->oom_running = 0;
         break;
     case QXL_IO_SET_MODE:
@@ -1022,10 +1022,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         qxl_destroy_primary(d);
         break;
     case QXL_IO_DESTROY_SURFACE_WAIT:
-        d->ssd.worker->destroy_surface_wait(d->ssd.worker, val);
+        qemu_spice_destroy_surface_wait(&d->ssd, val);
         break;
     case QXL_IO_DESTROY_ALL_SURFACES:
-        d->ssd.worker->destroy_surfaces(d->ssd.worker);
+        qemu_spice_destroy_surfaces(&d->ssd);
         break;
     default:
         fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
@@ -1430,7 +1430,7 @@ static int qxl_post_load(void *opaque, int version)
         cmds[out].cmd.type = QXL_CMD_CURSOR;
         cmds[out].group_id = MEMSLOT_GROUP_GUEST;
         out++;
-        d->ssd.worker->loadvm_commands(d->ssd.worker, cmds, out);
+        qemu_spice_loadvm_commands(&d->ssd, cmds, out);
         qemu_free(cmds);
 
         break;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index feeee73..1e6a38f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,6 +62,89 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
     dest->right = MAX(dest->right, r->right);
 }
 
+
+void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
+                            struct QXLRect *area, struct QXLRect *dirty_rects,
+                            uint32_t num_dirty_rects,
+                            uint32_t clear_dirty_region)
+{
+    ssd->worker->update_area(ssd->worker, surface_id, area, dirty_rects,
+                             num_dirty_rects, clear_dirty_region);
+}
+
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
+{
+    ssd->worker->add_memslot(ssd->worker, memslot);
+}
+
+void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid)
+{
+    ssd->worker->del_memslot(ssd->worker, gid, sid);
+}
+
+void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
+                                       QXLDevSurfaceCreate *surface)
+{
+    ssd->worker->create_primary_surface(ssd->worker, id, surface);
+}
+
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
+{
+    ssd->worker->destroy_primary_surface(ssd->worker, id);
+}
+
+void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id)
+{
+    ssd->worker->destroy_surface_wait(ssd->worker, id);
+}
+
+void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd,
+                                struct QXLCommandExt *ext, uint32_t count)
+{
+    ssd->worker->loadvm_commands(ssd->worker, ext, count);
+}
+
+void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->wakeup(ssd->worker);
+}
+
+void qemu_spice_oom(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->oom(ssd->worker);
+}
+
+void qemu_spice_start(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->start(ssd->worker);
+}
+
+void qemu_spice_stop(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->stop(ssd->worker);
+}
+
+void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->reset_memslots(ssd->worker);
+}
+
+void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->destroy_surfaces(ssd->worker);
+}
+
+void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->reset_image_cache(ssd->worker);
+}
+
+void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd)
+{
+    ssd->worker->reset_cursor(ssd->worker);
+}
+
+
 static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
     SimpleSpiceUpdate *update;
@@ -161,7 +244,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
     memset(&memslot, 0, sizeof(memslot));
     memslot.slot_group_id = MEMSLOT_GROUP_HOST;
     memslot.virt_end = ~0;
-    ssd->worker->add_memslot(ssd->worker, &memslot);
+    qemu_spice_add_memslot(ssd, &memslot);
 }
 
 void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
@@ -181,14 +264,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
     surface.mem        = (intptr_t)ssd->buf;
     surface.group_id   = MEMSLOT_GROUP_HOST;
 
-    ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
+    qemu_spice_create_primary_surface(ssd, 0, &surface);
 }
 
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 {
     dprint(1, "%s:\n", __FUNCTION__);
 
-    ssd->worker->destroy_primary_surface(ssd->worker, 0);
+    qemu_spice_destroy_primary_surface(ssd, 0);
 }
 
 void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
@@ -196,9 +279,9 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
     SimpleSpiceDisplay *ssd = opaque;
 
     if (running) {
-        ssd->worker->start(ssd->worker);
+        qemu_spice_start(ssd);
     } else {
-        ssd->worker->stop(ssd->worker);
+        qemu_spice_stop(ssd);
     }
     ssd->running = running;
 }
@@ -267,7 +350,7 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
 
     if (ssd->notify) {
         ssd->notify = 0;
-        ssd->worker->wakeup(ssd->worker);
+        qemu_spice_wakeup(ssd);
         dprint(2, "%s: notify\n", __FUNCTION__);
     }
 }
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 2f95f68..5b06b11 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -80,3 +80,25 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
                                int x, int y, int w, int h);
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
+
+void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
+                            struct QXLRect *area, struct QXLRect *dirty_rects,
+                            uint32_t num_dirty_rects,
+                            uint32_t clear_dirty_region);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
+void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid,
+                            uint32_t sid);
+void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
+                                       QXLDevSurfaceCreate *surface);
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
+void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id);
+void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd,
+                                struct QXLCommandExt *ext, uint32_t count);
+void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
+void qemu_spice_oom(SimpleSpiceDisplay *ssd);
+void qemu_spice_start(SimpleSpiceDisplay *ssd);
+void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd);
+void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 02/12] spice: add qemu_spice_display_init_common
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 01/12] spice: add worker wrapper functions Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 03/12] spice/qxl: move worker wrappers Alon Levy
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Factor out SimpleSpiceDisplay initialization into
qemu_spice_display_init_common() and call it from
both qxl.c (for vga mode) and spice-display.c

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c           |    7 +------
 ui/spice-display.c |   17 +++++++++++------
 ui/spice-display.h |    1 +
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 545074d..2d46814 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1321,12 +1321,7 @@ static int qxl_init_primary(PCIDevice *dev)
 
     vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
                                    qxl_hw_screen_dump, qxl_hw_text_update, qxl);
-    qxl->ssd.ds = vga->ds;
-    qemu_mutex_init(&qxl->ssd.lock);
-    qxl->ssd.mouse_x = -1;
-    qxl->ssd.mouse_y = -1;
-    qxl->ssd.bufsize = (16 * 1024 * 1024);
-    qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
+    qemu_spice_display_init_common(&qxl->ssd, vga->ds);
 
     qxl0 = qxl;
     register_displaychangelistener(vga->ds, &display_listener);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 1e6a38f..93e25bf 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -286,6 +286,16 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
     ssd->running = running;
 }
 
+void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
+{
+    ssd->ds = ds;
+    qemu_mutex_init(&ssd->lock);
+    ssd->mouse_x = -1;
+    ssd->mouse_y = -1;
+    ssd->bufsize = (16 * 1024 * 1024);
+    ssd->buf = qemu_malloc(ssd->bufsize);
+}
+
 /* display listener callbacks */
 
 void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
@@ -499,12 +509,7 @@ static DisplayChangeListener display_listener = {
 void qemu_spice_display_init(DisplayState *ds)
 {
     assert(sdpy.ds == NULL);
-    sdpy.ds = ds;
-    qemu_mutex_init(&sdpy.lock);
-    sdpy.mouse_x = -1;
-    sdpy.mouse_y = -1;
-    sdpy.bufsize = (16 * 1024 * 1024);
-    sdpy.buf = qemu_malloc(sdpy.bufsize);
+    qemu_spice_display_init_common(&sdpy, ds);
     register_displaychangelistener(ds, &display_listener);
 
     sdpy.qxl.base.sif = &dpy_interface.base;
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 5b06b11..eb7a573 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -75,6 +75,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd);
 void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd);
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd);
 void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason);
+void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds);
 
 void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
                                int x, int y, int w, int h);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 03/12] spice/qxl: move worker wrappers
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 01/12] spice: add worker wrapper functions Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 02/12] spice: add qemu_spice_display_init_common Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 04/12] qxl: fix surface tracking & locking Alon Levy
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Move the wrapper functions which are used by qxl only to qxl.c.
Rename them from qemu_spice_* to qxl_spice_*.  Also pass in a
qxl state pointer instead of a SimpleSpiceDisplay pointer.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl-render.c    |    4 +-
 hw/qxl.c           |   67 ++++++++++++++++++++++++++++++++++++++++++++-------
 hw/qxl.h           |   13 ++++++++++
 ui/spice-display.c |   46 -----------------------------------
 ui/spice-display.h |   12 ---------
 5 files changed, 72 insertions(+), 70 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index bef5f14..60b822d 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -124,8 +124,8 @@ void qxl_render_update(PCIQXLDevice *qxl)
     update.bottom = qxl->guest_primary.surface.height;
 
     memset(dirty, 0, sizeof(dirty));
-    qemu_spice_update_area(&qxl->ssd, 0, &update,
-                           dirty, ARRAY_SIZE(dirty), 1);
+    qxl_spice_update_area(qxl, 0, &update,
+                          dirty, ARRAY_SIZE(dirty), 1);
 
     for (i = 0; i < ARRAY_SIZE(dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
diff --git a/hw/qxl.c b/hw/qxl.c
index 2d46814..29094a9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -125,6 +125,53 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
 static void qxl_reset_surfaces(PCIQXLDevice *d);
 static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
 
+
+void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
+                           struct QXLRect *area, struct QXLRect *dirty_rects,
+                           uint32_t num_dirty_rects,
+                           uint32_t clear_dirty_region)
+{
+    qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
+                             num_dirty_rects, clear_dirty_region);
+}
+
+void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
+{
+    qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+}
+
+void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
+                               uint32_t count)
+{
+    qxl->ssd.worker->loadvm_commands(qxl->ssd.worker, ext, count);
+}
+
+void qxl_spice_oom(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->oom(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->reset_memslots(qxl->ssd.worker);
+}
+
+void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->reset_cursor(qxl->ssd.worker);
+}
+
+
 static inline uint32_t msb_mask(uint32_t val)
 {
     uint32_t mask;
@@ -690,8 +737,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
     dprint(d, 1, "%s: start%s\n", __FUNCTION__,
            loadvm ? " (loadvm)" : "");
 
-    qemu_spice_reset_cursor(&d->ssd);
-    qemu_spice_reset_image_cache(&d->ssd);
+    qxl_spice_reset_cursor(d);
+    qxl_spice_reset_image_cache(d);
     qxl_reset_surfaces(d);
     qxl_reset_memslots(d);
 
@@ -813,7 +860,7 @@ static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
 static void qxl_reset_memslots(PCIQXLDevice *d)
 {
     dprint(d, 1, "%s:\n", __FUNCTION__);
-    qemu_spice_reset_memslots(&d->ssd);
+    qxl_spice_reset_memslots(d);
     memset(&d->guest_slots, 0, sizeof(d->guest_slots));
 }
 
@@ -821,7 +868,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
 {
     dprint(d, 1, "%s:\n", __FUNCTION__);
     d->mode = QXL_MODE_UNDEFINED;
-    qemu_spice_destroy_surfaces(&d->ssd);
+    qxl_spice_destroy_surfaces(d);
     memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
 }
 
@@ -962,8 +1009,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_UPDATE_AREA:
     {
         QXLRect update = d->ram->update_area;
-        qemu_spice_update_area(&d->ssd, d->ram->update_surface,
-                               &update, NULL, 0, 0);
+        qxl_spice_update_area(d, d->ram->update_surface,
+                              &update, NULL, 0, 0);
         break;
     }
     case QXL_IO_NOTIFY_CMD:
@@ -984,7 +1031,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         }
         d->oom_running = 1;
-        qemu_spice_oom(&d->ssd);
+        qxl_spice_oom(d);
         d->oom_running = 0;
         break;
     case QXL_IO_SET_MODE:
@@ -1022,10 +1069,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         qxl_destroy_primary(d);
         break;
     case QXL_IO_DESTROY_SURFACE_WAIT:
-        qemu_spice_destroy_surface_wait(&d->ssd, val);
+        qxl_spice_destroy_surface_wait(d, val);
         break;
     case QXL_IO_DESTROY_ALL_SURFACES:
-        qemu_spice_destroy_surfaces(&d->ssd);
+        qxl_spice_destroy_surfaces(d);
         break;
     default:
         fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
@@ -1425,7 +1472,7 @@ static int qxl_post_load(void *opaque, int version)
         cmds[out].cmd.type = QXL_CMD_CURSOR;
         cmds[out].group_id = MEMSLOT_GROUP_GUEST;
         out++;
-        qemu_spice_loadvm_commands(&d->ssd, cmds, out);
+        qxl_spice_loadvm_commands(d, cmds, out);
         qemu_free(cmds);
 
         break;
diff --git a/hw/qxl.h b/hw/qxl.h
index f6c450d..e62b9d0 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -98,6 +98,19 @@ typedef struct PCIQXLDevice {
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
 
+void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
+                           struct QXLRect *area, struct QXLRect *dirty_rects,
+                           uint32_t num_dirty_rects,
+                           uint32_t clear_dirty_region);
+void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id);
+void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
+                               uint32_t count);
+void qxl_spice_oom(PCIQXLDevice *qxl);
+void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
+void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl);
+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);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 93e25bf..af10ae8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -63,15 +63,6 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
 }
 
 
-void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
-                            struct QXLRect *area, struct QXLRect *dirty_rects,
-                            uint32_t num_dirty_rects,
-                            uint32_t clear_dirty_region)
-{
-    ssd->worker->update_area(ssd->worker, surface_id, area, dirty_rects,
-                             num_dirty_rects, clear_dirty_region);
-}
-
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
 {
     ssd->worker->add_memslot(ssd->worker, memslot);
@@ -93,27 +84,11 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
     ssd->worker->destroy_primary_surface(ssd->worker, id);
 }
 
-void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id)
-{
-    ssd->worker->destroy_surface_wait(ssd->worker, id);
-}
-
-void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd,
-                                struct QXLCommandExt *ext, uint32_t count)
-{
-    ssd->worker->loadvm_commands(ssd->worker, ext, count);
-}
-
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
 {
     ssd->worker->wakeup(ssd->worker);
 }
 
-void qemu_spice_oom(SimpleSpiceDisplay *ssd)
-{
-    ssd->worker->oom(ssd->worker);
-}
-
 void qemu_spice_start(SimpleSpiceDisplay *ssd)
 {
     ssd->worker->start(ssd->worker);
@@ -124,27 +99,6 @@ void qemu_spice_stop(SimpleSpiceDisplay *ssd)
     ssd->worker->stop(ssd->worker);
 }
 
-void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd)
-{
-    ssd->worker->reset_memslots(ssd->worker);
-}
-
-void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd)
-{
-    ssd->worker->destroy_surfaces(ssd->worker);
-}
-
-void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd)
-{
-    ssd->worker->reset_image_cache(ssd->worker);
-}
-
-void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd)
-{
-    ssd->worker->reset_cursor(ssd->worker);
-}
-
-
 static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
 {
     SimpleSpiceUpdate *update;
diff --git a/ui/spice-display.h b/ui/spice-display.h
index eb7a573..abe99c7 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -82,24 +82,12 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
 
-void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
-                            struct QXLRect *area, struct QXLRect *dirty_rects,
-                            uint32_t num_dirty_rects,
-                            uint32_t clear_dirty_region);
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
 void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid,
                             uint32_t sid);
 void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
                                        QXLDevSurfaceCreate *surface);
 void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
-void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id);
-void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd,
-                                struct QXLCommandExt *ext, uint32_t count);
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
-void qemu_spice_oom(SimpleSpiceDisplay *ssd);
 void qemu_spice_start(SimpleSpiceDisplay *ssd);
 void qemu_spice_stop(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd);
-void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 04/12] qxl: fix surface tracking & locking
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (2 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 03/12] spice/qxl: move worker wrappers Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string Alon Levy
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Surface tracking needs proper locking since it is used from vcpu and spice
worker threads, add it.  Also reset the surface counter when zapping all
surfaces.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |   13 ++++++++++++-
 hw/qxl.h |    2 ++
 2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 29094a9..e832d00 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -137,7 +137,12 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
 
 void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
 {
+    qemu_mutex_lock(&qxl->track_lock);
+    PANIC_ON(id >= NUM_SURFACES);
     qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+    qxl->guest_surfaces.cmds[id] = 0;
+    qxl->guest_surfaces.count--;
+    qemu_mutex_unlock(&qxl->track_lock);
 }
 
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
@@ -158,7 +163,11 @@ void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
 
 void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
 {
+    qemu_mutex_lock(&qxl->track_lock);
     qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+    memset(&qxl->guest_surfaces.cmds, 0, sizeof(qxl->guest_surfaces.cmds));
+    qxl->guest_surfaces.count = 0;
+    qemu_mutex_unlock(&qxl->track_lock);
 }
 
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
@@ -317,6 +326,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
         QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
         uint32_t id = le32_to_cpu(cmd->surface_id);
         PANIC_ON(id >= NUM_SURFACES);
+        qemu_mutex_lock(&qxl->track_lock);
         if (cmd->type == QXL_SURFACE_CMD_CREATE) {
             qxl->guest_surfaces.cmds[id] = ext->cmd.data;
             qxl->guest_surfaces.count++;
@@ -327,6 +337,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
             qxl->guest_surfaces.cmds[id] = 0;
             qxl->guest_surfaces.count--;
         }
+        qemu_mutex_unlock(&qxl->track_lock);
         break;
     }
     case QXL_CMD_CURSOR:
@@ -869,7 +880,6 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
     dprint(d, 1, "%s:\n", __FUNCTION__);
     d->mode = QXL_MODE_UNDEFINED;
     qxl_spice_destroy_surfaces(d);
-    memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
 }
 
 /* called from spice server thread context only */
@@ -1289,6 +1299,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     qxl->generation = 1;
     qxl->num_memslots = NUM_MEMSLOTS;
     qxl->num_surfaces = NUM_SURFACES;
+    qemu_mutex_init(&qxl->track_lock);
 
     switch (qxl->revision) {
     case 1: /* spice 0.4 -- qxl-1 */
diff --git a/hw/qxl.h b/hw/qxl.h
index e62b9d0..5d0e85e 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -55,6 +55,8 @@ typedef struct PCIQXLDevice {
     } guest_surfaces;
     QXLPHYSICAL        guest_cursor;
 
+    QemuMutex          track_lock;
+
     /* thread signaling */
     pthread_t          main;
     int                pipe[2];
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (3 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 04/12] qxl: fix surface tracking & locking Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-15  7:59   ` Gerd Hoffmann
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 06/12] qxl: error handling fixes and cleanups Alon Levy
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e832d00..c998e9b 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -409,6 +409,66 @@ static const char *qxl_mode_to_string(int mode)
     return "INVALID";
 }
 
+static const char *io_port_to_string(uint32_t io_port)
+{
+    if (io_port >= QXL_IO_RANGE_SIZE) {
+        return "out of range";
+    }
+    switch (io_port) {
+    case QXL_IO_NOTIFY_CMD:
+        return "QXL_IO_NOTIFY_CMD";
+    case QXL_IO_NOTIFY_CURSOR:
+        return "QXL_IO_NOTIFY_CURSOR";
+    case QXL_IO_UPDATE_AREA:
+        return "QXL_IO_UPDATE_AREA";
+    case QXL_IO_UPDATE_IRQ:
+        return "QXL_IO_UPDATE_IRQ";
+    case QXL_IO_NOTIFY_OOM:
+        return "QXL_IO_NOTIFY_OOM";
+    case QXL_IO_RESET:
+        return "QXL_IO_RESET";
+    case QXL_IO_SET_MODE:
+        return "QXL_IO_SET_MODE";
+    case QXL_IO_LOG:
+        return "QXL_IO_LOG";
+    case QXL_IO_MEMSLOT_ADD:
+        return "QXL_IO_MEMSLOT_ADD";
+    case QXL_IO_MEMSLOT_DEL:
+        return "QXL_IO_MEMSLOT_DEL";
+    case QXL_IO_DETACH_PRIMARY:
+        return "QXL_IO_DETACH_PRIMARY";
+    case QXL_IO_ATTACH_PRIMARY:
+        return "QXL_IO_ATTACH_PRIMARY";
+    case QXL_IO_CREATE_PRIMARY:
+        return "QXL_IO_CREATE_PRIMARY";
+    case QXL_IO_DESTROY_PRIMARY:
+        return "QXL_IO_DESTROY_PRIMARY";
+    case QXL_IO_DESTROY_SURFACE_WAIT:
+        return "QXL_IO_DESTROY_SURFACE_WAIT";
+    case QXL_IO_DESTROY_ALL_SURFACES:
+        return "QXL_IO_DESTROY_ALL_SURFACES";
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    case QXL_IO_UPDATE_AREA_ASYNC:
+        return "QXL_IO_UPDATE_AREA_ASYNC";
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+        return "QXL_IO_MEMSLOT_ADD_ASYNC";
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
+        return "QXL_IO_CREATE_PRIMARY_ASYNC";
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
+        return "QXL_IO_DESTROY_PRIMARY_ASYNC";
+    case QXL_IO_DESTROY_SURFACE_ASYNC:
+        return "QXL_IO_DESTROY_SURFACE_ASYNC";
+    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+        return "QXL_IO_DESTROY_ALL_SURFACES_ASYNC";
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
+        return "QXL_IO_FLUSH_SURFACES_ASYNC";
+    case QXL_IO_FLUSH_RELEASE:
+        return "QXL_IO_FLUSH_RELEASE";
+#endif
+    }
+    return "error in io_port_to_string";
+}
+
 /* called from spice server thread context only */
 static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
 {
@@ -1011,7 +1071,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     default:
         if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
             break;
-        dprint(d, 1, "%s: unexpected port 0x%x in vga mode\n", __FUNCTION__, io_port);
+        dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
+            __func__, io_port, io_port_to_string(io_port));
         return;
     }
 
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 06/12] qxl: error handling fixes and cleanups.
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (4 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 07/12] qxl: make qxl_guest_bug take variable arguments Alon Levy
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Add qxl_guest_bug() function which is supposed to be called in case
sanity checks of guest requests fail.  It raises an error IRQ and
logs a message in case guest debugging is enabled.

Make PANIC_ON() abort instead of exit.  That macro should be used
for qemu bugs only, any guest-triggerable stuff should use the new
qxl_guest_bug() function instead.

Convert a few easy cases from PANIC_ON() to qxl_guest_bug() to
show intended usage.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |   34 ++++++++++++++++++++++++++++++----
 hw/qxl.h |    3 ++-
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index c998e9b..e51851a5 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -125,6 +125,16 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
 static void qxl_reset_surfaces(PCIQXLDevice *d);
 static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
 
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg)
+{
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
+#endif
+    if (qxl->guestdebug) {
+        fprintf(stderr, "qxl-%d: guest bug: %s\n", qxl->id, msg);
+    }
+}
+
 
 void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
@@ -1120,22 +1130,38 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         qxl_hard_reset(d, 0);
         break;
     case QXL_IO_MEMSLOT_ADD:
-        PANIC_ON(val >= NUM_MEMSLOTS);
-        PANIC_ON(d->guest_slots[val].active);
+        if (val >= NUM_MEMSLOTS) {
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range");
+            break;
+        }
+        if (d->guest_slots[val].active) {
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active");
+            break;
+        }
         d->guest_slots[val].slot = d->ram->mem_slot;
         qxl_add_memslot(d, val, 0);
         break;
     case QXL_IO_MEMSLOT_DEL:
+        if (val >= NUM_MEMSLOTS) {
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range");
+            break;
+        }
         qxl_del_memslot(d, val);
         break;
     case QXL_IO_CREATE_PRIMARY:
-        PANIC_ON(val != 0);
+        if (val != 0) {
+            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0");
+            break;
+        }
         dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
         d->guest_primary.surface = d->ram->create_surface;
         qxl_create_guest_primary(d, 0);
         break;
     case QXL_IO_DESTROY_PRIMARY:
-        PANIC_ON(val != 0);
+        if (val != 0) {
+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
+            break;
+        }
         dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
         qxl_destroy_primary(d);
         break;
diff --git a/hw/qxl.h b/hw/qxl.h
index 5d0e85e..5db9aae 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -86,7 +86,7 @@ typedef struct PCIQXLDevice {
 
 #define PANIC_ON(x) if ((x)) {                         \
     printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
-    exit(-1);                                          \
+    abort();                                           \
 }
 
 #define dprint(_qxl, _level, _fmt, ...)                                 \
@@ -99,6 +99,7 @@ typedef struct PCIQXLDevice {
 
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg);
 
 void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 07/12] qxl: make qxl_guest_bug take variable arguments
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (5 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 06/12] qxl: error handling fixes and cleanups Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 08/12] qxl: only disallow specific io's in vga mode Alon Levy
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |    9 +++++++--
 hw/qxl.h |    2 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e51851a5..09382f5 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -125,13 +125,18 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
 static void qxl_reset_surfaces(PCIQXLDevice *d);
 static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
 
-void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg)
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
 {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
     qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
 #endif
     if (qxl->guestdebug) {
-        fprintf(stderr, "qxl-%d: guest bug: %s\n", qxl->id, msg);
+        va_list ap;
+        va_start(ap, msg);
+        fprintf(stderr, "qxl-%d: guest bug: ", qxl->id);
+        vfprintf(stderr, msg, ap);
+        fprintf(stderr, "\n");
+        va_end(ap);
     }
 }
 
diff --git a/hw/qxl.h b/hw/qxl.h
index 5db9aae..32ca5a0 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -99,7 +99,7 @@ typedef struct PCIQXLDevice {
 
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
-void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg);
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
 
 void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 08/12] qxl: only disallow specific io's in vga mode
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (6 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 07/12] qxl: make qxl_guest_bug take variable arguments Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api Alon Levy
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Since the driver is still in operation even after moving to UNDEFINED, i.e.
by destroying primary in any way.

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

diff --git a/hw/qxl.c b/hw/qxl.c
index 09382f5..27eee4b 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1084,8 +1084,9 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_LOG:
         break;
     default:
-        if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
+        if (d->mode != QXL_MODE_VGA) {
             break;
+        }
         dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
             __func__, io_port, io_port_to_string(io_port));
         return;
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (7 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 08/12] qxl: only disallow specific io's in vga mode Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-15  8:12   ` Gerd Hoffmann
  2011-07-15  8:19   ` Gerd Hoffmann
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
                   ` (2 subsequent siblings)
  11 siblings, 2 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Some of the QXL port i/o commands are waiting for the spice server to
complete certain actions.  Add async versions for these commands, so we
don't block the vcpu while the spice server processses the command.
Instead the qxl device will raise an IRQ when done.

The async command processing relies on an added QXLInterface::async_complete
and added QXLWorker::*_async additions, in spice server qxl >= 3.1

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Alon Levy     <alevy@redhat.com>
---
 hw/qxl-render.c    |    2 +-
 hw/qxl.c           |  236 ++++++++++++++++++++++++++++++++++++++++++++--------
 hw/qxl.h           |   18 ++++-
 ui/spice-display.c |   47 ++++++++--
 ui/spice-display.h |   23 +++++-
 5 files changed, 273 insertions(+), 53 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 60b822d..643ff2d 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -125,7 +125,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
 
     memset(dirty, 0, sizeof(dirty));
     qxl_spice_update_area(qxl, 0, &update,
-                          dirty, ARRAY_SIZE(dirty), 1);
+                          dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC);
 
     for (i = 0; i < ARRAY_SIZE(dirty); i++) {
         if (qemu_spice_rect_is_empty(dirty+i)) {
diff --git a/hw/qxl.c b/hw/qxl.c
index 27eee4b..6f784e6 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -120,7 +120,7 @@ static QXLMode qxl_modes[] = {
 static PCIQXLDevice *qxl0;
 
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events);
-static void qxl_destroy_primary(PCIQXLDevice *d);
+static void qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async);
 static void qxl_reset_memslots(PCIQXLDevice *d);
 static void qxl_reset_surfaces(PCIQXLDevice *d);
 static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
@@ -144,22 +144,47 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
 void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
                            uint32_t num_dirty_rects,
-                           uint32_t clear_dirty_region)
+                           uint32_t clear_dirty_region,
+                           qxl_async_io async)
 {
-    qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
-                             num_dirty_rects, clear_dirty_region);
+    if (async == QXL_SYNC) {
+        qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area,
+                        dirty_rects, num_dirty_rects, clear_dirty_region);
+    } else {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area,
+                                           clear_dirty_region, 0);
+#else
+        abort();
+#endif
+    }
 }
 
-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
+static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl,
+                                                    uint32_t id)
 {
     qemu_mutex_lock(&qxl->track_lock);
-    PANIC_ON(id >= NUM_SURFACES);
-    qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
     qxl->guest_surfaces.cmds[id] = 0;
     qxl->guest_surfaces.count--;
     qemu_mutex_unlock(&qxl->track_lock);
 }
 
+static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
+                                           qxl_async_io async)
+{
+    if (async) {
+#if SPICE_INTERFACE_QXL_MINOR < 1
+        abort();
+#else
+        qxl->ssd.worker->destroy_surface_wait_async(qxl->ssd.worker, id,
+                                                    (uint64_t)id);
+#endif
+    } else {
+        qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+        qxl_spice_destroy_surface_wait_complete(qxl, id);
+    }
+}
+
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count)
 {
@@ -176,15 +201,28 @@ void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
     qxl->ssd.worker->reset_memslots(qxl->ssd.worker);
 }
 
-void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
+static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
 {
     qemu_mutex_lock(&qxl->track_lock);
-    qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
     memset(&qxl->guest_surfaces.cmds, 0, sizeof(qxl->guest_surfaces.cmds));
     qxl->guest_surfaces.count = 0;
     qemu_mutex_unlock(&qxl->track_lock);
 }
 
+static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
+{
+    if (async) {
+#if SPICE_INTERFACE_QXL_MINOR < 1
+        abort();
+#else
+        qxl->ssd.worker->destroy_surfaces_async(qxl->ssd.worker, 0);
+#endif
+    } else {
+        qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+        qxl_spice_destroy_surfaces_complete(qxl);
+    }
+}
+
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
 {
     qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
@@ -712,6 +750,38 @@ static int interface_flush_resources(QXLInstance *sin)
     return ret;
 }
 
+static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
+
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+
+/* called from spice server thread context only */
+static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
+{
+    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
+    uint32_t current_async;
+
+    qemu_mutex_lock(&qxl->async_lock);
+    current_async = qxl->current_async;
+    qxl->current_async = QXL_UNDEFINED_IO;
+    qemu_mutex_unlock(&qxl->async_lock);
+
+    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
+    switch (current_async) {
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
+        qxl_create_guest_primary_complete(qxl);
+        break;
+    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+        qxl_spice_destroy_surfaces_complete(qxl);
+        break;
+    case QXL_IO_DESTROY_SURFACE_ASYNC:
+        qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie);
+        break;
+    }
+    qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
+}
+
+#endif
+
 static const QXLInterface qxl_interface = {
     .base.type               = SPICE_INTERFACE_QXL,
     .base.description        = "qxl gpu",
@@ -731,6 +801,9 @@ static const QXLInterface qxl_interface = {
     .req_cursor_notification = interface_req_cursor_notification,
     .notify_update           = interface_notify_update,
     .flush_resources         = interface_flush_resources,
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    .async_complete          = interface_async_complete,
+#endif
 };
 
 static void qxl_enter_vga_mode(PCIQXLDevice *d)
@@ -750,7 +823,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
         return;
     }
     dprint(d, 1, "%s\n", __FUNCTION__);
-    qxl_destroy_primary(d);
+    qxl_destroy_primary(d, QXL_SYNC);
 }
 
 static void qxl_set_irq(PCIQXLDevice *d)
@@ -853,13 +926,14 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
 
     if (qxl->mode != QXL_MODE_VGA) {
         dprint(qxl, 1, "%s\n", __FUNCTION__);
-        qxl_destroy_primary(qxl);
+        qxl_destroy_primary(qxl, QXL_SYNC);
         qxl_soft_reset(qxl);
     }
     vga_ioport_write(opaque, addr, val);
 }
 
-static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
+static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
+                            qxl_async_io async)
 {
     static const int regions[] = {
         QXL_RAM_RANGE_INDEX,
@@ -929,7 +1003,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
            __FUNCTION__, memslot.slot_id,
            memslot.virt_start, memslot.virt_end);
 
-    qemu_spice_add_memslot(&d->ssd, &memslot);
+    qemu_spice_add_memslot(&d->ssd, &memslot, async);
     d->guest_slots[slot_id].ptr = (void*)memslot.virt_start;
     d->guest_slots[slot_id].size = memslot.virt_end - memslot.virt_start;
     d->guest_slots[slot_id].delta = delta;
@@ -954,7 +1028,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
 {
     dprint(d, 1, "%s:\n", __FUNCTION__);
     d->mode = QXL_MODE_UNDEFINED;
-    qxl_spice_destroy_surfaces(d);
+    qxl_spice_destroy_surfaces(d, QXL_SYNC);
 }
 
 /* called from spice server thread context only */
@@ -979,7 +1053,14 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
     }
 }
 
-static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
+static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
+{
+    /* for local rendering */
+    qxl_render_resize(qxl);
+}
+
+static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
+                                     qxl_async_io async)
 {
     QXLDevSurfaceCreate surface;
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
@@ -1007,13 +1088,14 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
 
     qxl->mode = QXL_MODE_NATIVE;
     qxl->cmdflags = 0;
-    qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface);
+    qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface, async);
 
-    /* for local rendering */
-    qxl_render_resize(qxl);
+    if (async == QXL_SYNC) {
+        qxl_create_guest_primary_complete(qxl);
+    }
 }
 
-static void qxl_destroy_primary(PCIQXLDevice *d)
+static void qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async)
 {
     if (d->mode == QXL_MODE_UNDEFINED) {
         return;
@@ -1022,7 +1104,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
     dprint(d, 1, "%s\n", __FUNCTION__);
 
     d->mode = QXL_MODE_UNDEFINED;
-    qemu_spice_destroy_primary_surface(&d->ssd, 0);
+    qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
 }
 
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -1052,10 +1134,10 @@ static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
     }
 
     d->guest_slots[0].slot = slot;
-    qxl_add_memslot(d, 0, devmem);
+    qxl_add_memslot(d, 0, devmem, QXL_SYNC);
 
     d->guest_primary.surface = surface;
-    qxl_create_guest_primary(d, 0);
+    qxl_create_guest_primary(d, 0, QXL_SYNC);
 
     d->mode = QXL_MODE_COMPAT;
     d->cmdflags = QXL_COMMAND_FLAG_COMPAT;
@@ -1073,6 +1155,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     PCIQXLDevice *d = opaque;
     uint32_t io_port = addr - d->io_base;
+    qxl_async_io async = QXL_SYNC;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    uint32_t orig_io_port = io_port;
+#endif
 
     switch (io_port) {
     case QXL_IO_RESET:
@@ -1082,6 +1168,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_CREATE_PRIMARY:
     case QXL_IO_UPDATE_IRQ:
     case QXL_IO_LOG:
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
+#endif
         break;
     default:
         if (d->mode != QXL_MODE_VGA) {
@@ -1089,15 +1179,61 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
         dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
             __func__, io_port, io_port_to_string(io_port));
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        /* be nice to buggy guest drivers */
+        if (io_port >= QXL_IO_UPDATE_AREA_ASYNC &&
+            io_port <= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) {
+            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
+        }
+#endif
         return;
     }
 
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    /* we change the io_port to avoid ifdeffery in the main switch */
+    orig_io_port = io_port;
+    switch (io_port) {
+    case QXL_IO_UPDATE_AREA_ASYNC:
+        io_port = QXL_IO_UPDATE_AREA;
+        goto async_common;
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+        io_port = QXL_IO_MEMSLOT_ADD;
+        goto async_common;
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
+        io_port = QXL_IO_CREATE_PRIMARY;
+        goto async_common;
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
+        io_port = QXL_IO_DESTROY_PRIMARY;
+        goto async_common;
+    case QXL_IO_DESTROY_SURFACE_ASYNC:
+        io_port = QXL_IO_DESTROY_SURFACE_WAIT;
+        goto async_common;
+    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+        io_port = QXL_IO_DESTROY_ALL_SURFACES;
+    async_common:
+        async = QXL_ASYNC;
+        qemu_mutex_lock(&d->async_lock);
+        if (d->current_async != QXL_UNDEFINED_IO) {
+            qxl_guest_bug(d, "%d async started before last (%d) complete",
+                io_port, d->current_async);
+            qemu_mutex_unlock(&d->async_lock);
+            return;
+        }
+        d->current_async = orig_io_port;
+        qemu_mutex_unlock(&d->async_lock);
+        dprint(d, 2, "start async %d (%d)\n", io_port, val);
+        break;
+    default:
+        break;
+    }
+#endif
+
     switch (io_port) {
     case QXL_IO_UPDATE_AREA:
     {
         QXLRect update = d->ram->update_area;
         qxl_spice_update_area(d, d->ram->update_surface,
-                              &update, NULL, 0, 0);
+                              &update, NULL, 0, 0, async);
         break;
     }
     case QXL_IO_NOTIFY_CMD:
@@ -1145,7 +1281,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         }
         d->guest_slots[val].slot = d->ram->mem_slot;
-        qxl_add_memslot(d, val, 0);
+        qxl_add_memslot(d, val, 0, async);
         break;
     case QXL_IO_MEMSLOT_DEL:
         if (val >= NUM_MEMSLOTS) {
@@ -1156,31 +1292,59 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         break;
     case QXL_IO_CREATE_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0");
-            break;
+            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY (async=%d): val != 0",
+                          async);
+            goto cancel_async;
         }
-        dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
+        dprint(d, 1, "QXL_IO_CREATE_PRIMARY async=%d\n", async);
         d->guest_primary.surface = d->ram->create_surface;
-        qxl_create_guest_primary(d, 0);
+        qxl_create_guest_primary(d, 0, async);
         break;
     case QXL_IO_DESTROY_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
-            break;
+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0",
+                          async);
+            goto cancel_async;
+        }
+        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
+               qxl_mode_to_string(d->mode));
+        qxl_destroy_primary(d, async);
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        if (d->mode == QXL_MODE_UNDEFINED && async == QXL_ASYNC) {
+            dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
+                    qxl_mode_to_string(d->mode));
+            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
+            goto cancel_async;
         }
-        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
-        qxl_destroy_primary(d);
+#endif
         break;
     case QXL_IO_DESTROY_SURFACE_WAIT:
-        qxl_spice_destroy_surface_wait(d, val);
+        if (val >= NUM_SURFACES) {
+            qxl_guest_bug(d, "QXL_IO_DESTROY_SURFACE (async=%d):"
+                             "%d >= NUM_SURFACES", async, val);
+            goto cancel_async;
+        }
+        qxl_spice_destroy_surface_wait(d, val, async);
         break;
     case QXL_IO_DESTROY_ALL_SURFACES:
-        qxl_spice_destroy_surfaces(d);
+        d->mode = QXL_MODE_UNDEFINED;
+        qxl_spice_destroy_surfaces(d, async);
         break;
     default:
         fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
         abort();
     }
+    return;
+cancel_async:
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    if (async) {
+        qemu_mutex_lock(&d->async_lock);
+        d->current_async = QXL_UNDEFINED_IO;
+        qemu_mutex_unlock(&d->async_lock);
+    }
+#else
+    return;
+#endif
 }
 
 static uint32_t ioport_read(void *opaque, uint32_t addr)
@@ -1557,9 +1721,9 @@ static int qxl_post_load(void *opaque, int version)
             if (!d->guest_slots[i].active) {
                 continue;
             }
-            qxl_add_memslot(d, i, 0);
+            qxl_add_memslot(d, i, 0, QXL_SYNC);
         }
-        qxl_create_guest_primary(d, 1);
+        qxl_create_guest_primary(d, 1, QXL_SYNC);
 
         /* replay surface-create and cursor-set commands */
         cmds = qemu_mallocz(sizeof(QXLCommandExt) * (NUM_SURFACES + 1));
diff --git a/hw/qxl.h b/hw/qxl.h
index 32ca5a0..41f0311 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -15,6 +15,8 @@ enum qxl_mode {
     QXL_MODE_NATIVE,
 };
 
+#define QXL_UNDEFINED_IO UINT32_MAX
+
 typedef struct PCIQXLDevice {
     PCIDevice          pci;
     SimpleSpiceDisplay ssd;
@@ -30,6 +32,11 @@ typedef struct PCIQXLDevice {
     int32_t            num_memslots;
     int32_t            num_surfaces;
 
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    uint32_t           current_async;
+    QemuMutex          async_lock;
+#endif
+
     struct guest_slots {
         QXLMemSlot     slot;
         void           *ptr;
@@ -104,13 +111,12 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
 void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
                            uint32_t num_dirty_rects,
-                           uint32_t clear_dirty_region);
-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id);
+                           uint32_t clear_dirty_region,
+                           qxl_async_io async);
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count);
 void qxl_spice_oom(PCIQXLDevice *qxl);
 void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
-void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl);
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
 
@@ -122,3 +128,9 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
 void qxl_render_resize(PCIQXLDevice *qxl);
 void qxl_render_update(PCIQXLDevice *qxl);
 void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
+                                 struct QXLRect *area,
+                                 uint32_t clear_dirty_region,
+                                 int is_vga);
+#endif
diff --git a/ui/spice-display.c b/ui/spice-display.c
index af10ae8..fb1436e 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,10 +62,18 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
     dest->right = MAX(dest->right, r->right);
 }
 
-
-void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
-{
-    ssd->worker->add_memslot(ssd->worker, memslot);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
+                            qxl_async_io async)
+{
+    if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        ssd->worker->add_memslot_async(ssd->worker, memslot, 0);
+#else
+        abort();
+#endif
+    } else {
+        ssd->worker->add_memslot(ssd->worker, memslot);
+    }
 }
 
 void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid)
@@ -74,14 +82,33 @@ void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid)
 }
 
 void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
-                                       QXLDevSurfaceCreate *surface)
-{
-    ssd->worker->create_primary_surface(ssd->worker, id, surface);
+                                       QXLDevSurfaceCreate *surface,
+                                       qxl_async_io async)
+{
+    if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        ssd->worker->create_primary_surface_async(ssd->worker, id, surface, 0);
+#else
+        abort();
+#endif
+    } else {
+        ssd->worker->create_primary_surface(ssd->worker, id, surface);
+    }
 }
 
-void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
+
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
+                                        uint32_t id, qxl_async_io async)
 {
-    ssd->worker->destroy_primary_surface(ssd->worker, id);
+    if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        ssd->worker->destroy_primary_surface_async(ssd->worker, id, 0);
+#else
+        abort();
+#endif
+    } else {
+        ssd->worker->destroy_primary_surface(ssd->worker, id);
+    }
 }
 
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
@@ -198,7 +225,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
     memset(&memslot, 0, sizeof(memslot));
     memslot.slot_group_id = MEMSLOT_GROUP_HOST;
     memslot.virt_end = ~0;
-    qemu_spice_add_memslot(ssd, &memslot);
+    qemu_spice_add_memslot(ssd, &memslot, QXL_SYNC);
 }
 
 void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
@@ -218,14 +245,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
     surface.mem        = (intptr_t)ssd->buf;
     surface.group_id   = MEMSLOT_GROUP_HOST;
 
-    qemu_spice_create_primary_surface(ssd, 0, &surface);
+    qemu_spice_create_primary_surface(ssd, 0, &surface, QXL_SYNC);
 }
 
 void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
 {
     dprint(1, "%s:\n", __FUNCTION__);
 
-    qemu_spice_destroy_primary_surface(ssd, 0);
+    qemu_spice_destroy_primary_surface(ssd, 0, QXL_SYNC);
 }
 
 void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
diff --git a/ui/spice-display.h b/ui/spice-display.h
index abe99c7..1388641 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -33,6 +33,20 @@
 
 #define NUM_SURFACES 1024
 
+/*
+ * Internal enum to differenciate between options for
+ * io calls that have a sync (old) version and an _async (new)
+ * version:
+ *  QXL_SYNC: use the old version
+ *  QXL_ASYNC: use the new version and make sure there are no two
+ *   happening at the same time. This is used for guest initiated
+ *   calls
+ */
+typedef enum qxl_async_io {
+    QXL_SYNC,
+    QXL_ASYNC,
+} qxl_async_io;
+
 typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
 typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
 
@@ -82,12 +96,15 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
 void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
 void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
 
-void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
+                            qxl_async_io async);
 void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid,
                             uint32_t sid);
 void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
-                                       QXLDevSurfaceCreate *surface);
-void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
+                                       QXLDevSurfaceCreate *surface,
+                                       qxl_async_io async);
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
+                                        uint32_t id, qxl_async_io async);
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
 void qemu_spice_start(SimpleSpiceDisplay *ssd);
 void qemu_spice_stop(SimpleSpiceDisplay *ssd);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (8 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-15  8:15   ` Gerd Hoffmann
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev Alon Levy
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_* Alon Levy
  11 siblings, 1 reply; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Add two new IOs.
 QXL_IO_FLUSH_SURFACES - equivalent to update area for all surfaces, used
  to reduce vmexits from NumSurfaces to 1 on guest S3, S4 and resolution change (windows
  driver implementation is such that this is done on each of those occasions).
 QXL_IO_FLUSH_RELEASE - used to ensure anything on last_release is put on the release ring
  for the client to free.

Cc: Yonit Halperin <yhalperi@redhat.com>
Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 6f784e6..77f9ec4 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -185,6 +185,13 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
     }
 }
 
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->flush_surfaces_async(qxl->ssd.worker, 0);
+}
+#endif
+
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count)
 {
@@ -1210,6 +1217,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         goto async_common;
     case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
         io_port = QXL_IO_DESTROY_ALL_SURFACES;
+        goto async_common;
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
     async_common:
         async = QXL_ASYNC;
         qemu_mutex_lock(&d->async_lock);
@@ -1326,6 +1335,27 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
         qxl_spice_destroy_surface_wait(d, val, async);
         break;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    case QXL_IO_FLUSH_RELEASE: {
+        QXLReleaseRing *ring = &d->ram->release_ring;
+        if (ring->prod - ring->cons + 1 == ring->num_items) {
+            fprintf(stderr,
+                "ERROR: no flush, full release ring [p%d,%dc]\n",
+                ring->prod, ring->cons);
+        }
+        qxl_push_free_res(d, 1 /* flush */);
+        dprint(d, 1, "QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n",
+            qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+            d->num_free_res, d->last_release);
+        break;
+    }
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
+        dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n",
+               val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+               d->num_free_res);
+        qxl_spice_flush_surfaces_async(d);
+#endif
+        break;
     case QXL_IO_DESTROY_ALL_SURFACES:
         d->mode = QXL_MODE_UNDEFINED;
         qxl_spice_destroy_surfaces(d, async);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (9 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-15  8:20   ` Gerd Hoffmann
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_* Alon Levy
  11 siblings, 1 reply; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

Inform guest drivers about the new features I/O commands we have
now (async commands, S3 support) if building with newer spice, i.e.
if SPICE_INTERFACE_QXL_MINOR >= 1.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qxl.c |   25 ++++++++++++++++++-------
 hw/qxl.h |    6 ++++++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 77f9ec4..5052206 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1593,9 +1593,14 @@ static int qxl_init_common(PCIQXLDevice *qxl)
         pci_device_rev = QXL_REVISION_STABLE_V04;
         break;
     case 2: /* spice 0.6 -- qxl-2 */
-    default:
         pci_device_rev = QXL_REVISION_STABLE_V06;
         break;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    case 3: /* qxl-3 */
+#endif
+    default:
+        pci_device_rev = QXL_DEFAULT_REVISION;
+        break;
     }
 
     pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
@@ -1857,9 +1862,12 @@ static PCIDeviceInfo qxl_info_primary = {
     .device_id    = QXL_DEVICE_ID_STABLE,
     .class_id     = PCI_CLASS_DISPLAY_VGA,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 * 1024),
-        DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, 64 * 1024 * 1024),
-        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
+        DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size,
+                           64 * 1024 * 1024),
+        DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size,
+                           64 * 1024 * 1024),
+        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision,
+                           QXL_DEFAULT_REVISION),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
@@ -1878,9 +1886,12 @@ static PCIDeviceInfo qxl_info_secondary = {
     .device_id    = QXL_DEVICE_ID_STABLE,
     .class_id     = PCI_CLASS_DISPLAY_OTHER,
     .qdev.props = (Property[]) {
-        DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 * 1024),
-        DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, 64 * 1024 * 1024),
-        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
+        DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size,
+                           64 * 1024 * 1024),
+        DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size,
+                           64 * 1024 * 1024),
+        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision,
+                           QXL_DEFAULT_REVISION),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
diff --git a/hw/qxl.h b/hw/qxl.h
index 41f0311..b1af1d2 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -104,6 +104,12 @@ typedef struct PCIQXLDevice {
         }                                                               \
     } while (0)
 
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
+#else
+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V06
+#endif
+
 /* qxl.c */
 void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
 void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_*
  2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
                   ` (10 preceding siblings ...)
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev Alon Levy
@ 2011-07-14 19:13 ` Alon Levy
  2011-07-15  8:23   ` Gerd Hoffmann
  11 siblings, 1 reply; 26+ messages in thread
From: Alon Levy @ 2011-07-14 19:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 5052206..b9d27b9 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1579,7 +1579,6 @@ static DisplayChangeListener display_listener = {
 static int qxl_init_common(PCIQXLDevice *qxl)
 {
     uint8_t* config = qxl->pci.config;
-    uint32_t pci_device_rev;
     uint32_t io_size;
 
     qxl->mode = QXL_MODE_UNDEFINED;
@@ -1587,23 +1586,25 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     qxl->num_memslots = NUM_MEMSLOTS;
     qxl->num_surfaces = NUM_SURFACES;
     qemu_mutex_init(&qxl->track_lock);
-
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+    qemu_mutex_init(&qxl->async_lock);
+    qxl->current_async = QXL_UNDEFINED_IO;
+#endif
     switch (qxl->revision) {
-    case 1: /* spice 0.4 -- qxl-1 */
-        pci_device_rev = QXL_REVISION_STABLE_V04;
-        break;
-    case 2: /* spice 0.6 -- qxl-2 */
-        pci_device_rev = QXL_REVISION_STABLE_V06;
-        break;
+    case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
+    case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-    case 3: /* qxl-3 */
+    case QXL_REVISION_STABLE_V10: /* spice 0.10? -- qxl-3 */
+        break;
 #endif
     default:
-        pci_device_rev = QXL_DEFAULT_REVISION;
+        fprintf(stderr, "invalid revision %d, resetting to %d\n", qxl->revision,
+            QXL_DEFAULT_REVISION);
+        qxl->revision = QXL_DEFAULT_REVISION;
         break;
     }
 
-    pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
+    pci_set_byte(&config[PCI_REVISION_ID], qxl->revision);
     pci_set_byte(&config[PCI_INTERRUPT_PIN], 1);
 
     qxl->rom_size = qxl_rom_size();
@@ -1614,14 +1615,14 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     if (qxl->vram_size < 16 * 1024 * 1024) {
         qxl->vram_size = 16 * 1024 * 1024;
     }
-    if (qxl->revision == 1) {
+    if (qxl->revision == QXL_REVISION_STABLE_V04) {
         qxl->vram_size = 4096;
     }
     qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1);
     qxl->vram_offset = qemu_ram_alloc(&qxl->pci.qdev, "qxl.vram", qxl->vram_size);
 
     io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
-    if (qxl->revision == 1) {
+    if (qxl->revision == QXL_REVISION_STABLE_V04) {
         io_size = 8;
     }
 
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string Alon Levy
@ 2011-07-15  7:59   ` Gerd Hoffmann
  2011-07-15  9:02     ` Alon Levy
  0 siblings, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  7:59 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

   Hi,

> +    switch (io_port) {
> +    case QXL_IO_NOTIFY_CMD:
> +        return "QXL_IO_NOTIFY_CMD";

Wasn't the plan to make this an array?

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api Alon Levy
@ 2011-07-15  8:12   ` Gerd Hoffmann
  2011-07-15  9:05     ` Alon Levy
  2011-07-15  8:19   ` Gerd Hoffmann
  1 sibling, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  8:12 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

>       case QXL_IO_DESTROY_PRIMARY:
>           if (val != 0) {
> -            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
> -            break;
> +            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0",
> +                          async);
> +            goto cancel_async;
> +        }
> +        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
> +               qxl_mode_to_string(d->mode));
> +        qxl_destroy_primary(d, async);
> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +        if (d->mode == QXL_MODE_UNDEFINED&&  async == QXL_ASYNC) {
> +            dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
> +                    qxl_mode_to_string(d->mode));
> +            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
> +            goto cancel_async;
>           }

Hmm?  Why this is needed?

>       default:
>           fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
>           abort();
>       }
> +    return;
> +cancel_async:
> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +    if (async) {
> +        qemu_mutex_lock(&d->async_lock);
> +        d->current_async = QXL_UNDEFINED_IO;
> +        qemu_mutex_unlock(&d->async_lock);

Add "qxl_send_events(d, QXL_INTERRUPT_IO_CMD)" here?

>   typedef struct PCIQXLDevice {
>       PCIDevice          pci;
>       SimpleSpiceDisplay ssd;
> @@ -30,6 +32,11 @@ typedef struct PCIQXLDevice {
>       int32_t            num_memslots;
>       int32_t            num_surfaces;
>
> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +    uint32_t           current_async;
> +    QemuMutex          async_lock;
> +#endif

No need to ifdef this.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
@ 2011-07-15  8:15   ` Gerd Hoffmann
  2011-07-15  9:11     ` Alon Levy
  0 siblings, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  8:15 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +    case QXL_IO_FLUSH_RELEASE: {
> +        QXLReleaseRing *ring =&d->ram->release_ring;
> +        if (ring->prod - ring->cons + 1 == ring->num_items) {
> +            fprintf(stderr,
> +                "ERROR: no flush, full release ring [p%d,%dc]\n",
> +                ring->prod, ring->cons);
> +        }
> +        qxl_push_free_res(d, 1 /* flush */);
> +        dprint(d, 1, "QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n",
> +            qxl_mode_to_string(d->mode), d->guest_surfaces.count,
> +            d->num_free_res, d->last_release);
> +        break;
> +    }
> +    case QXL_IO_FLUSH_SURFACES_ASYNC:
> +        dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n",
> +               val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
> +               d->num_free_res);
> +        qxl_spice_flush_surfaces_async(d);
> +#endif
> +        break;

The last two lines should be swapped I guess.
Doesn't harm, but looks a bit odd ...

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api Alon Levy
  2011-07-15  8:12   ` Gerd Hoffmann
@ 2011-07-15  8:19   ` Gerd Hoffmann
  2011-07-15  9:06     ` Alon Levy
  1 sibling, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  8:19 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

On 07/14/11 21:13, Alon Levy wrote:
> Some of the QXL port i/o commands are waiting for the spice server to
> complete certain actions.  Add async versions for these commands, so we
> don't block the vcpu while the spice server processses the command.
> Instead the qxl device will raise an IRQ when done.
>
> The async command processing relies on an added QXLInterface::async_complete
> and added QXLWorker::*_async additions, in spice server qxl>= 3.1

Applying: qxl: async io support using new spice api
=== checkpatch complains ===
WARNING: labels should not be indented
#320: FILE: hw/qxl.c:1207:
+    async_common:

total: 0 errors, 1 warnings, 574 lines checked

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev Alon Levy
@ 2011-07-15  8:20   ` Gerd Hoffmann
  2011-07-15  9:13     ` Alon Levy
  0 siblings, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  8:20 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
> +#else
> +#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V06
> +#endif

Does that actually build with old spice-protocol?  I don't think so ...

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_*
  2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_* Alon Levy
@ 2011-07-15  8:23   ` Gerd Hoffmann
  2011-07-15  9:14     ` Alon Levy
  0 siblings, 1 reply; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15  8:23 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

   Hi,

> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> +    qemu_mutex_init(&qxl->async_lock);
> +    qxl->current_async = QXL_UNDEFINED_IO;
> +#endif

That surely belongs into another patch ...

> +    case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
> +    case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */

I think we should just not use these defines for now.

In a year or so when the new bits are in widespread use we can raise the 
minimum required spice-server+spice-protocol versions, cleanup the ifdef 
mess and also use QXL_REVISION_STABLE_V04.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string
  2011-07-15  7:59   ` Gerd Hoffmann
@ 2011-07-15  9:02     ` Alon Levy
  0 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:02 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 09:59:21AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >+    switch (io_port) {
> >+    case QXL_IO_NOTIFY_CMD:
> >+        return "QXL_IO_NOTIFY_CMD";
> 
> Wasn't the plan to make this an array?
> 

yes. Somehow I sent the wrong patch. No idea how I managed to do that.

> cheers,
>   Gerd
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-15  8:12   ` Gerd Hoffmann
@ 2011-07-15  9:05     ` Alon Levy
  2011-07-15 11:23       ` Gerd Hoffmann
  0 siblings, 1 reply; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:05 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 10:12:14AM +0200, Gerd Hoffmann wrote:
> >      case QXL_IO_DESTROY_PRIMARY:
> >          if (val != 0) {
> >-            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
> >-            break;
> >+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0",
> >+                          async);
> >+            goto cancel_async;
> >+        }
> >+        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
> >+               qxl_mode_to_string(d->mode));
> >+        qxl_destroy_primary(d, async);
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+        if (d->mode == QXL_MODE_UNDEFINED&&  async == QXL_ASYNC) {
> >+            dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
> >+                    qxl_mode_to_string(d->mode));
> >+            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
> >+            goto cancel_async;
> >          }
> 
> Hmm?  Why this is needed?
In this case we don't do any operation (i.e. qxl_destroy_primary is a nop,
it checkes d->mode == QXL_MODE_UNDEFINED too) so there will never be an async_complete,
so there will never be a qxl_send_events, so we need to send it now.

> 
> >      default:
> >          fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
> >          abort();
> >      }
> >+    return;
> >+cancel_async:
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+    if (async) {
> >+        qemu_mutex_lock(&d->async_lock);
> >+        d->current_async = QXL_UNDEFINED_IO;
> >+        qemu_mutex_unlock(&d->async_lock);
> 
> Add "qxl_send_events(d, QXL_INTERRUPT_IO_CMD)" here?

no, we want that wen the command is actually complete, on async_complete,
where it is already done.

> 
> >  typedef struct PCIQXLDevice {
> >      PCIDevice          pci;
> >      SimpleSpiceDisplay ssd;
> >@@ -30,6 +32,11 @@ typedef struct PCIQXLDevice {
> >      int32_t            num_memslots;
> >      int32_t            num_surfaces;
> >
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+    uint32_t           current_async;
> >+    QemuMutex          async_lock;
> >+#endif
> 
> No need to ifdef this.
> 

missed it.

> cheers,
>   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-15  8:19   ` Gerd Hoffmann
@ 2011-07-15  9:06     ` Alon Levy
  0 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:06 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 10:19:27AM +0200, Gerd Hoffmann wrote:
> On 07/14/11 21:13, Alon Levy wrote:
> >Some of the QXL port i/o commands are waiting for the spice server to
> >complete certain actions.  Add async versions for these commands, so we
> >don't block the vcpu while the spice server processses the command.
> >Instead the qxl device will raise an IRQ when done.
> >
> >The async command processing relies on an added QXLInterface::async_complete
> >and added QXLWorker::*_async additions, in spice server qxl>= 3.1
> 
> Applying: qxl: async io support using new spice api
> === checkpatch complains ===
> WARNING: labels should not be indented
> #320: FILE: hw/qxl.c:1207:
> +    async_common:
> 

thanks, my bad, did this after the checkpatch (obviously). I'll go add
it to the commit hook.

> total: 0 errors, 1 warnings, 574 lines checked
> 
> cheers,
>   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
  2011-07-15  8:15   ` Gerd Hoffmann
@ 2011-07-15  9:11     ` Alon Levy
  0 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:11 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 10:15:26AM +0200, Gerd Hoffmann wrote:
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+    case QXL_IO_FLUSH_RELEASE: {
> >+        QXLReleaseRing *ring =&d->ram->release_ring;
> >+        if (ring->prod - ring->cons + 1 == ring->num_items) {
> >+            fprintf(stderr,
> >+                "ERROR: no flush, full release ring [p%d,%dc]\n",
> >+                ring->prod, ring->cons);
> >+        }
> >+        qxl_push_free_res(d, 1 /* flush */);
> >+        dprint(d, 1, "QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n",
> >+            qxl_mode_to_string(d->mode), d->guest_surfaces.count,
> >+            d->num_free_res, d->last_release);
> >+        break;
> >+    }
> >+    case QXL_IO_FLUSH_SURFACES_ASYNC:
> >+        dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n",
> >+               val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
> >+               d->num_free_res);
> >+        qxl_spice_flush_surfaces_async(d);
> >+#endif
> >+        break;
> 
> The last two lines should be swapped I guess.
> Doesn't harm, but looks a bit odd ...

yep. thanks, will fix.

> 
> cheers,
>   Gerd
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev
  2011-07-15  8:20   ` Gerd Hoffmann
@ 2011-07-15  9:13     ` Alon Levy
  0 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:13 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 10:20:08AM +0200, Gerd Hoffmann wrote:
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
> >+#else
> >+#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V06
> >+#endif
> 
> Does that actually build with old spice-protocol?  I don't think so ...
> 

no, it won't. /me wonders how I did the compilations. I'll use 2 and 3.

> cheers,
>   Gerd
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_*
  2011-07-15  8:23   ` Gerd Hoffmann
@ 2011-07-15  9:14     ` Alon Levy
  0 siblings, 0 replies; 26+ messages in thread
From: Alon Levy @ 2011-07-15  9:14 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 15, 2011 at 10:23:39AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+    qemu_mutex_init(&qxl->async_lock);
> >+    qxl->current_async = QXL_UNDEFINED_IO;
> >+#endif
> 
> That surely belongs into another patch ...

yes.

> 
> >+    case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
> >+    case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
> 
> I think we should just not use these defines for now.
> 
> In a year or so when the new bits are in widespread use we can raise
> the minimum required spice-server+spice-protocol versions, cleanup
> the ifdef mess and also use QXL_REVISION_STABLE_V04.
> 

ok. I'll drop this patch (at least the bits fitting the patch header).

> cheers,
>   Gerd
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api
  2011-07-15  9:05     ` Alon Levy
@ 2011-07-15 11:23       ` Gerd Hoffmann
  0 siblings, 0 replies; 26+ messages in thread
From: Gerd Hoffmann @ 2011-07-15 11:23 UTC (permalink / raw)
  To: qemu-devel, yhalperi

   Hi,

>>> +        qxl_destroy_primary(d, async);
>>> +#if SPICE_INTERFACE_QXL_MINOR>= 1
>>> +        if (d->mode == QXL_MODE_UNDEFINED&&   async == QXL_ASYNC) {
>>> +            dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
>>> +                    qxl_mode_to_string(d->mode));
>>> +            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
>>> +            goto cancel_async;
>>>           }
>>
>> Hmm?  Why this is needed?
> In this case we don't do any operation (i.e. qxl_destroy_primary is a nop,
> it checkes d->mode == QXL_MODE_UNDEFINED too) so there will never be an async_complete,
> so there will never be a qxl_send_events, so we need to send it now.

Ok.  Maybe make qxl_destroy_primary return an error in case there is no 
primary to destroy, then catch this here?  Makes more clear what is 
going on here.

>>
>>>       default:
>>>           fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
>>>           abort();
>>>       }
>>> +    return;
>>> +cancel_async:
>>> +#if SPICE_INTERFACE_QXL_MINOR>= 1
>>> +    if (async) {
>>> +        qemu_mutex_lock(&d->async_lock);
>>> +        d->current_async = QXL_UNDEFINED_IO;
>>> +        qemu_mutex_unlock(&d->async_lock);
>>
>> Add "qxl_send_events(d, QXL_INTERRUPT_IO_CMD)" here?
>
> no, we want that wen the command is actually complete, on async_complete,
> where it is already done.

That is the cancel code path, i.e. the guest did a async request but we 
don't submit one to the spice-server because some sanity check failed, 
so we never get a complete callback.  Thats why I think we should raise 
the QXL_INTERRUPT_IO_CMD interrupt here instead.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2011-07-15 11:23 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-14 19:13 [Qemu-devel] [PATCHv5 00/12] async + suspend reworked Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 01/12] spice: add worker wrapper functions Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 02/12] spice: add qemu_spice_display_init_common Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 03/12] spice/qxl: move worker wrappers Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 04/12] qxl: fix surface tracking & locking Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 05/12] qxl: add io_port_to_string Alon Levy
2011-07-15  7:59   ` Gerd Hoffmann
2011-07-15  9:02     ` Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 06/12] qxl: error handling fixes and cleanups Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 07/12] qxl: make qxl_guest_bug take variable arguments Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 08/12] qxl: only disallow specific io's in vga mode Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api Alon Levy
2011-07-15  8:12   ` Gerd Hoffmann
2011-07-15  9:05     ` Alon Levy
2011-07-15 11:23       ` Gerd Hoffmann
2011-07-15  8:19   ` Gerd Hoffmann
2011-07-15  9:06     ` Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 10/12] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
2011-07-15  8:15   ` Gerd Hoffmann
2011-07-15  9:11     ` Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 11/12] qxl: bump pci rev Alon Levy
2011-07-15  8:20   ` Gerd Hoffmann
2011-07-15  9:13     ` Alon Levy
2011-07-14 19:13 ` [Qemu-devel] [PATCHv5 12/12] qxl: use QXL_REVISION_* Alon Levy
2011-07-15  8:23   ` Gerd Hoffmann
2011-07-15  9:14     ` Alon Levy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.