qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
  2011-06-24 13:02 [Qemu-devel] [PATCH v2] S3&S4 support Alon Levy
@ 2011-06-24 13:02 ` Alon Levy
  0 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-06-24 13:02 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>
---
 hw/qxl.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 969a984..fab0208 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1167,6 +1167,30 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_DESTROY_ALL_SURFACES:
         qemu_spice_destroy_surfaces(&d->ssd);
         break;
+    case QXL_IO_FLUSH_SURFACES:
+        dprint(d, 1, "QXL_IO_FLUSH_SURFACES (%d) entry (%s, s#=%d, res#=%d)\n",
+            val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+            d->num_free_res);
+        qemu_spice_stop(&d->ssd);
+        qemu_spice_start(&d->ssd);
+        dprint(d, 1, "QXL_IO_FLUSH_SURFACES 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_RELEASE: {
+        QXLReleaseRing *ring = &d->ram->release_ring;
+        if (ring->prod - ring->cons + 1 == ring->num_items) {
+            // TODO - "return" a value to the guest and let it loop?
+            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_MEMSLOT_ADD_ASYNC:
         PANIC_ON(val >= NUM_MEMSLOTS);
         PANIC_ON(d->guest_slots[val].active);
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] async + suspend reworked
@ 2011-07-07 16:50 Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add worker wrapper functions Alon Levy
                   ` (13 more replies)
  0 siblings, 14 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

Everything is based on spice.v38 from git://anongit.freedesktop.org/spice/qemu

v1->v2 changes:
 dropped wlock
 dropped oom_async
 update_area_async used in qxl-render
 added async_lock
 async_complete handles completion of io, not at dispatcher call time

Git trees:
 git://anongit.freedesktop.org/~alon/qemu            s3.v4.async.api.v2

 git://anongit.freedesktop.org/~alon/spice           s3.v3.async.v3
 git://anongit.freedesktop.org/~alon/spice-protocol  s3.v2
 git://anongit.freedesktop.org/~alon/qxl             s3.v3.async.v3



Alon Levy (7):
  qxl: add io_port_to_string
  qxl: make qxl_guest_bug take variable arguments
  qxl: async I/O
  qxl: only disallow specific io's in vga mode
  qxl: add QXL_IO_FLUSH_{SURFACES,RELEASE} for guest S3&S4 support
  qxl: use QXL_REVISION_*
  qxl: use update_area_async in qxl-render

Gerd Hoffmann (7):
  spice: add worker wrapper functions.
  spice: add qemu_spice_display_init_common
  qxl: remove qxl_destroy_primary()
  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           |  467 +++++++++++++++++++++++++++++++++++++++++++---------
 hw/qxl.h           |   32 ++++-
 ui/spice-display.c |   99 ++++++++++--
 ui/spice-display.h |   18 ++
 5 files changed, 530 insertions(+), 90 deletions(-)

-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] spice: add worker wrapper functions.
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add qemu_spice_display_init_common Alon Levy
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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 |   94 ++++++++++++++++++++++++++++++++++++++++++++++++---
 ui/spice-display.h |   20 +++++++++++
 4 files changed, 126 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 0b9a4c7..19240da 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -684,8 +684,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);
 
@@ -790,7 +790,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;
@@ -800,14 +800,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));
 }
 
@@ -815,7 +815,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));
 }
 
@@ -869,7 +869,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);
@@ -884,7 +884,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)
@@ -956,15 +956,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);
@@ -978,7 +978,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:
@@ -1016,10 +1016,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);
@@ -1424,7 +1424,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..0433ea8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,6 +62,88 @@ 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 +243,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 +263,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 +278,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 +349,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..0effdfa 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -80,3 +80,23 @@ 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.5.4

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

* [Qemu-devel] [PATCH v2] spice: add qemu_spice_display_init_common
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add worker wrapper functions Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: remove qxl_destroy_primary() Alon Levy
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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 19240da..d92d5b2 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1315,12 +1315,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 0433ea8..fef1758 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -285,6 +285,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,
@@ -498,12 +508,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 0effdfa..a39b19d 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.5.4

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

* [Qemu-devel] [PATCH v2] qxl: remove qxl_destroy_primary()
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add worker wrapper functions Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add qemu_spice_display_init_common Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice/qxl: move worker wrappers Alon Levy
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

From: Gerd Hoffmann <kraxel@redhat.com>

We'll have to move qemu_spice_destroy_primary_surface() out of
qxl_destroy_primary().  That makes the function pretty pointless,
so zap it and open code the two lines instead.

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

diff --git a/hw/qxl.c b/hw/qxl.c
index d92d5b2..a405e50 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -120,7 +120,6 @@ 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_reset_memslots(PCIQXLDevice *d);
 static void qxl_reset_surfaces(PCIQXLDevice *d);
 static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
@@ -617,7 +616,10 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
         return;
     }
     dprint(d, 1, "%s\n", __FUNCTION__);
-    qxl_destroy_primary(d);
+    if (d->mode != QXL_MODE_UNDEFINED) {
+        d->mode = QXL_MODE_UNDEFINED;
+        qemu_spice_destroy_primary_surface(&d->ssd, 0);
+    }
 }
 
 static void qxl_set_irq(PCIQXLDevice *d)
@@ -714,7 +716,10 @@ 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);
+        if (qxl->mode != QXL_MODE_UNDEFINED) {
+            qxl->mode = QXL_MODE_UNDEFINED;
+            qemu_spice_destroy_primary_surface(&qxl->ssd, 0);
+        }
         qxl_soft_reset(qxl);
     }
     vga_ioport_write(opaque, addr, val);
@@ -875,18 +880,6 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
     qxl_render_resize(qxl);
 }
 
-static void qxl_destroy_primary(PCIQXLDevice *d)
-{
-    if (d->mode == QXL_MODE_UNDEFINED) {
-        return;
-    }
-
-    dprint(d, 1, "%s\n", __FUNCTION__);
-
-    d->mode = QXL_MODE_UNDEFINED;
-    qemu_spice_destroy_primary_surface(&d->ssd, 0);
-}
-
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
 {
     pcibus_t start = d->pci.io_regions[QXL_RAM_RANGE_INDEX].addr;
@@ -1013,7 +1006,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_DESTROY_PRIMARY:
         PANIC_ON(val != 0);
         dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
-        qxl_destroy_primary(d);
+        if (d->mode != QXL_MODE_UNDEFINED) {
+            d->mode = QXL_MODE_UNDEFINED;
+            qemu_spice_destroy_primary_surface(&d->ssd, 0);
+        }
         break;
     case QXL_IO_DESTROY_SURFACE_WAIT:
         qemu_spice_destroy_surface_wait(&d->ssd, val);
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] spice/qxl: move worker wrappers
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (2 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: remove qxl_destroy_primary() Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: fix surface tracking & locking Alon Levy
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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           |   66 ++++++++++++++++++++++++++++++++++++++++++++--------
 hw/qxl.h           |   12 +++++++++
 ui/spice-display.c |   45 -----------------------------------
 ui/spice-display.h |   11 --------
 5 files changed, 70 insertions(+), 68 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 a405e50..def128d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,6 +124,52 @@ 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;
@@ -686,8 +732,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);
 
@@ -812,7 +858,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));
 }
 
@@ -820,7 +866,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));
 }
 
@@ -949,8 +995,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:
@@ -971,7 +1017,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:
@@ -1012,10 +1058,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
         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);
@@ -1415,7 +1461,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..489d518 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -98,6 +98,18 @@ 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 fef1758..af10ae8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -63,14 +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);
@@ -92,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);
@@ -123,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 a39b19d..d32dc9e 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -82,22 +82,11 @@ 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.5.4

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

* [Qemu-devel] [PATCH v2] qxl: fix surface tracking & locking
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (3 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice/qxl: move worker wrappers Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add io_port_to_string Alon Levy
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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 def128d..9116c99 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -135,7 +135,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,
@@ -156,7 +161,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)
@@ -315,6 +324,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++;
@@ -325,6 +335,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:
@@ -867,7 +878,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 */
@@ -1278,6 +1288,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 489d518..087ef6b 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.5.4

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

* [Qemu-devel] [PATCH v2] qxl: add io_port_to_string
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (4 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: fix surface tracking & locking Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: error handling fixes and cleanups Alon Levy
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

---
 hw/qxl.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 9116c99..f3312f0 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -407,6 +407,65 @@ 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";
+    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";
+    }
+    // not reached?
+    return "error in io_port_to_string";
+}
+
 /* called from spice server thread context only */
 static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
 {
@@ -997,7 +1056,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",
+            __FUNCTION__, io_port, io_port_to_string(io_port));
         return;
     }
 
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: error handling fixes and cleanups.
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (5 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add io_port_to_string Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: make qxl_guest_bug take variable arguments Alon Levy
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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 |   32 ++++++++++++++++++++++++++++----
 hw/qxl.h |    3 ++-
 2 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index f3312f0..0d62036 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,6 +124,14 @@ 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)
+{
+    qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
+    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,
@@ -1105,22 +1113,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));
         if (d->mode != QXL_MODE_UNDEFINED) {
             d->mode = QXL_MODE_UNDEFINED;
diff --git a/hw/qxl.h b/hw/qxl.h
index 087ef6b..88393c2 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.5.4

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

* [Qemu-devel] [PATCH v2] qxl: make qxl_guest_bug take variable arguments
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (6 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: error handling fixes and cleanups Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: async I/O Alon Levy
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

---
 hw/qxl.c |   18 +++++++++++-------
 hw/qxl.h |    2 +-
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 0d62036..935bac0 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,11 +124,15 @@ 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, ...)
 {
     qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
     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);
+        va_end(ap);
     }
 }
 
@@ -1114,11 +1118,11 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         break;
     case QXL_IO_MEMSLOT_ADD:
         if (val >= NUM_MEMSLOTS) {
-            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range");
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range\n");
             break;
         }
         if (d->guest_slots[val].active) {
-            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active");
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active\n");
             break;
         }
         d->guest_slots[val].slot = d->ram->mem_slot;
@@ -1126,14 +1130,14 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         break;
     case QXL_IO_MEMSLOT_DEL:
         if (val >= NUM_MEMSLOTS) {
-            qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range");
+            qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range\n");
             break;
         }
         qxl_del_memslot(d, val);
         break;
     case QXL_IO_CREATE_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0");
+            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0\n");
             break;
         }
         dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
@@ -1142,7 +1146,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         break;
     case QXL_IO_DESTROY_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0\n");
             break;
         }
         dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
diff --git a/hw/qxl.h b/hw/qxl.h
index 88393c2..e361bc6 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.5.4

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

* [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (7 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: make qxl_guest_bug take variable arguments Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-08  7:17   ` Gerd Hoffmann
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: bump pci rev Alon Levy
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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.c           |  244 ++++++++++++++++++++++++++++++++++++++++++---------
 hw/qxl.h           |   21 ++++-
 ui/spice-display.c |   33 +++++++
 ui/spice-display.h |    8 ++
 4 files changed, 261 insertions(+), 45 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 935bac0..f72d5b8 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -136,25 +136,47 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
     }
 }
 
+void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
+                           struct QXLRect *area, struct QXLRect *dirty_rects,
+                           uint32_t num_dirty_rects, uint32_t clear_dirty_region,
+                           int async)
+{
+    if (async) {
+        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area, dirty_rects,
+                                 num_dirty_rects, clear_dirty_region, 0);
+    } else {
+        qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
+                                 num_dirty_rects, clear_dirty_region);
+    }
+}
 
 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);
+    qxl_spice_update_area_async(qxl, surface_id, area, dirty_rects,
+                                num_dirty_rects, clear_dirty_region, 0);
 }
 
-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
+static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl)
 {
     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.cmds[qxl->io_data.surface_id] = 0;
     qxl->guest_surfaces.count--;
     qemu_mutex_unlock(&qxl->track_lock);
 }
 
+static void qxl_spice_destroy_surface_wait_async(PCIQXLDevice *qxl, uint32_t id, int async)
+{
+    qxl->io_data.surface_id = id;
+    if (async) {
+        qxl->ssd.worker->destroy_surface_wait_async(qxl->ssd.worker, id, 0);
+    } else {
+        qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+        qxl_spice_destroy_surface_wait_complete(qxl);
+    }
+}
+
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count)
 {
@@ -171,15 +193,29 @@ 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->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+    qxl_spice_destroy_surfaces_complete(qxl);
+}
+
+static void qxl_spice_destroy_surfaces_async(PCIQXLDevice *qxl, int async)
+{
+    if (async) {
+        qxl->ssd.worker->destroy_surfaces_async(qxl->ssd.worker, 0);
+    } else {
+        qxl_spice_destroy_surfaces(qxl);
+    }
+}
+
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
 {
     qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
@@ -706,6 +742,38 @@ static int interface_flush_resources(QXLInstance *sin)
     return ret;
 }
 
+static void qxl_add_memslot_complete(PCIQXLDevice *d);
+static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
+
+/* 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, 1, "async_complete: %d (%ld) done\n", current_async, cookie);
+    switch (current_async) {
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+        qxl_add_memslot_complete(qxl);
+        break;
+    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);
+        break;
+    }
+    qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
+}
+
 static const QXLInterface qxl_interface = {
     .base.type               = SPICE_INTERFACE_QXL,
     .base.description        = "qxl gpu",
@@ -725,6 +793,7 @@ static const QXLInterface qxl_interface = {
     .req_cursor_notification = interface_req_cursor_notification,
     .notify_update           = interface_notify_update,
     .flush_resources         = interface_flush_resources,
+    .async_complete          = interface_async_complete,
 };
 
 static void qxl_enter_vga_mode(PCIQXLDevice *d)
@@ -853,7 +922,18 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
     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_complete(PCIQXLDevice *d)
+{
+    QXLDevMemSlot *memslot = &d->io_data.memslot;
+    uint32_t slot_id = d->io_data.memslot.slot_id;
+
+    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 = d->io_data.delta;
+    d->guest_slots[slot_id].active = 1;
+}
+
+static void qxl_add_memslot_async(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta, int async)
 {
     static const int regions[] = {
         QXL_RAM_RANGE_INDEX,
@@ -865,9 +945,11 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
     pcibus_t pci_start;
     pcibus_t pci_end;
     intptr_t virt_start;
-    QXLDevMemSlot memslot;
+    QXLDevMemSlot *memslot = &d->io_data.memslot;
     int i;
 
+    d->io_data.delta = delta;
+
     guest_start = le64_to_cpu(d->guest_slots[slot_id].slot.mem_start);
     guest_end   = le64_to_cpu(d->guest_slots[slot_id].slot.mem_end);
 
@@ -911,23 +993,27 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
         abort();
     }
 
-    memslot.slot_id = slot_id;
-    memslot.slot_group_id = MEMSLOT_GROUP_GUEST; /* guest group */
-    memslot.virt_start = virt_start + (guest_start - pci_start);
-    memslot.virt_end   = virt_start + (guest_end   - pci_start);
-    memslot.addr_delta = memslot.virt_start - delta;
-    memslot.generation = d->rom->slot_generation = 0;
+    memslot->slot_id = slot_id;
+    memslot->slot_group_id = MEMSLOT_GROUP_GUEST; /* guest group */
+    memslot->virt_start = virt_start + (guest_start - pci_start);
+    memslot->virt_end   = virt_start + (guest_end   - pci_start);
+    memslot->addr_delta = memslot->virt_start - delta;
+    memslot->generation = d->rom->slot_generation = 0;
     qxl_rom_set_dirty(d);
 
     dprint(d, 1, "%s: slot %d: host virt 0x%" PRIx64 " - 0x%" PRIx64 "\n",
-           __FUNCTION__, memslot.slot_id,
-           memslot.virt_start, memslot.virt_end);
+           __FUNCTION__, memslot->slot_id,
+           memslot->virt_start, memslot->virt_end);
 
-    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;
-    d->guest_slots[slot_id].active = 1;
+    qemu_spice_add_memslot_async(&d->ssd, memslot, async);
+    if (!async) {
+        qxl_add_memslot_complete(d);
+    }
+}
+
+static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
+{
+    qxl_add_memslot_async(d, slot_id, delta, 0);
 }
 
 static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
@@ -973,7 +1059,13 @@ 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_async(PCIQXLDevice *qxl, int loadvm, int async)
 {
     QXLDevSurfaceCreate surface;
     QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
@@ -1001,10 +1093,16 @@ 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_async(&qxl->ssd, 0, &surface, async);
 
-    /* for local rendering */
-    qxl_render_resize(qxl);
+    if (!async) {
+        qxl_create_guest_primary_complete(qxl);
+    }
+}
+
+static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
+{
+    qxl_create_guest_primary_async(qxl, loadvm, 0);
 }
 
 static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -1055,13 +1153,16 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
 {
     PCIQXLDevice *d = opaque;
     uint32_t io_port = addr - d->io_base;
+    int async = 0;
 
     switch (io_port) {
     case QXL_IO_RESET:
     case QXL_IO_SET_MODE:
     case QXL_IO_MEMSLOT_ADD:
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
     case QXL_IO_MEMSLOT_DEL:
     case QXL_IO_CREATE_PRIMARY:
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
     case QXL_IO_UPDATE_IRQ:
     case QXL_IO_LOG:
         break;
@@ -1070,17 +1171,47 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
             break;
         dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
             __FUNCTION__, io_port, io_port_to_string(io_port));
+        /* 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);
+        }
         return;
     }
 
     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);
+    case QXL_IO_UPDATE_AREA_ASYNC:
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
+    case QXL_IO_DESTROY_SURFACE_ASYNC:
+    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+        if (!qemu_spice_supports_async(&d->ssd)) {
+            fprintf(stderr, "qxl: error: async not supported by libspice but guest driver used it\n");
+            return;
+        }
+        async = 1;
+        qemu_mutex_lock(&d->async_lock);
+        if (d->current_async != QXL_UNDEFINED_IO) {
+            qxl_guest_bug(d, "%d async started before last (%d) complete\n",
+                io_port, d->current_async);
+            qemu_mutex_unlock(&d->async_lock);
+            return;
+        }
+        d->current_async = io_port;
+        qemu_mutex_unlock(&d->async_lock);
+        dprint(d, 1, "start async %d\n", d->current_async);
+        break;
+    default:
         break;
     }
+
+    switch (io_port) {
+    case QXL_IO_UPDATE_AREA_ASYNC:
+    case QXL_IO_UPDATE_AREA:
+        qxl_spice_update_area_async(d, d->ram->update_surface,
+                              &d->ram->update_area, NULL, 0, 0, async);
+        break;
     case QXL_IO_NOTIFY_CMD:
         qemu_spice_wakeup(&d->ssd);
         break;
@@ -1116,6 +1247,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         dprint(d, 1, "QXL_IO_RESET\n");
         qxl_hard_reset(d, 0);
         break;
+    case QXL_IO_MEMSLOT_ADD_ASYNC:
     case QXL_IO_MEMSLOT_ADD:
         if (val >= NUM_MEMSLOTS) {
             qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range\n");
@@ -1126,7 +1258,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_async(d, val, 0, async);
         break;
     case QXL_IO_MEMSLOT_DEL:
         if (val >= NUM_MEMSLOTS) {
@@ -1135,36 +1267,60 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
         qxl_del_memslot(d, val);
         break;
+    case QXL_IO_CREATE_PRIMARY_ASYNC:
     case QXL_IO_CREATE_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0\n");
-            break;
+            qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY (async=%d): val != 0\n", 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_async(d, 0, async);
         break;
+    case QXL_IO_DESTROY_PRIMARY_ASYNC:
     case QXL_IO_DESTROY_PRIMARY:
         if (val != 0) {
-            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0\n");
-            break;
+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0\n", async);
+            goto cancel_async;
         }
-        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
+        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
+               qxl_mode_to_string(d->mode));
         if (d->mode != QXL_MODE_UNDEFINED) {
             d->mode = QXL_MODE_UNDEFINED;
-            qemu_spice_destroy_primary_surface(&d->ssd, 0);
+            qemu_spice_destroy_primary_surface_async(&d->ssd, 0, async);
+        } else {
+            if (async) {
+                dprint(d, 1, "QXL_IO_DESTROY_PRIMARY in %s, ignored\n",
+                        qxl_mode_to_string(d->mode));
+                qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
+                goto cancel_async;
+            }
         }
         break;
+    case QXL_IO_DESTROY_SURFACE_ASYNC:
     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_async(d, val, async);
         break;
+    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
     case QXL_IO_DESTROY_ALL_SURFACES:
-        qxl_spice_destroy_surfaces(d);
+        d->mode = QXL_MODE_UNDEFINED;
+        qxl_spice_destroy_surfaces_async(d, async);
         break;
     default:
         fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
         abort();
     }
+    return;
+cancel_async:
+    if (async) {
+        qemu_mutex_lock(&d->async_lock);
+        d->current_async = QXL_UNDEFINED_IO;
+        qemu_mutex_unlock(&d->async_lock);
+    }
 }
 
 static uint32_t ioport_read(void *opaque, uint32_t addr)
@@ -1377,6 +1533,8 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     qxl->num_memslots = NUM_MEMSLOTS;
     qxl->num_surfaces = NUM_SURFACES;
     qemu_mutex_init(&qxl->track_lock);
+    qemu_mutex_init(&qxl->async_lock);
+    qxl->current_async = QXL_UNDEFINED_IO;
 
     switch (qxl->revision) {
     case 1: /* spice 0.4 -- qxl-1 */
diff --git a/hw/qxl.h b/hw/qxl.h
index e361bc6..16639ce 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,8 @@ typedef struct PCIQXLDevice {
     int32_t            num_memslots;
     int32_t            num_surfaces;
 
+    uint32_t           current_async;
+
     struct guest_slots {
         QXLMemSlot     slot;
         void           *ptr;
@@ -56,6 +60,7 @@ typedef struct PCIQXLDevice {
     QXLPHYSICAL        guest_cursor;
 
     QemuMutex          track_lock;
+    QemuMutex          async_lock;
 
     /* thread signaling */
     pthread_t          main;
@@ -82,6 +87,15 @@ typedef struct PCIQXLDevice {
 
     /* io bar */
     uint32_t           io_base;
+
+    /* io data for asyncable calls */
+    struct {
+        /* QXL_IO_MEMSLOT_ADD{,ASYNC} */
+        QXLDevMemSlot memslot;
+        uint64_t delta;
+        /* QXL_IO_DESTROY_SURFACE_{WAIT,ASYNC} */
+        uint32_t surface_id;
+    } io_data;
 } PCIQXLDevice;
 
 #define PANIC_ON(x) if ((x)) {                         \
@@ -104,12 +118,15 @@ 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);
+void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
+                           struct QXLRect *area, struct QXLRect *dirty_rects,
+                           uint32_t num_dirty_rects, uint32_t clear_dirty_region,
+                           int async);
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count);
 void qxl_spice_oom(PCIQXLDevice *qxl);
+void qxl_spice_oom_async(PCIQXLDevice *qxl, int async);
 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);
 
diff --git a/ui/spice-display.c b/ui/spice-display.c
index af10ae8..b7bc0de 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,6 +62,20 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
     dest->right = MAX(dest->right, r->right);
 }
 
+int qemu_spice_supports_async(SimpleSpiceDisplay *ssd)
+{
+    return (ssd->worker->major_version > 3 ||
+            (ssd->worker->major_version == 3 && ssd->worker->minor_version >= 1));
+}
+
+void qemu_spice_add_memslot_async(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, int async)
+{
+    if (async) {
+        ssd->worker->add_memslot_async(ssd->worker, memslot, 0);
+    } else {
+        qemu_spice_add_memslot(ssd, memslot);
+    }
+}
 
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
 {
@@ -73,12 +87,31 @@ 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_async(SimpleSpiceDisplay *ssd, uint32_t id,
+                                       QXLDevSurfaceCreate *surface, int async)
+{
+    if (async) {
+        ssd->worker->create_primary_surface_async(ssd->worker, id, surface, 0);
+    } else {
+        qemu_spice_create_primary_surface(ssd, id, surface);
+    }
+}
+
 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_async(SimpleSpiceDisplay *ssd, uint32_t id, int async)
+{
+    if (async) {
+        ssd->worker->destroy_primary_surface_async(ssd->worker, id, 0);
+    } else {
+        qemu_spice_destroy_primary_surface(ssd, id);
+    }
+}
+
 void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
 {
     ssd->worker->destroy_primary_surface(ssd->worker, id);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index d32dc9e..ebfa7ca 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -90,3 +90,11 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
 void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
 void qemu_spice_start(SimpleSpiceDisplay *ssd);
 void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+
+int qemu_spice_supports_async(SimpleSpiceDisplay *ssd);
+
+/* calls async version if async == 1, otherwise calls the sync version */
+void qemu_spice_add_memslot_async(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, int async);
+void qemu_spice_create_primary_surface_async(SimpleSpiceDisplay *ssd, uint32_t id,
+                                       QXLDevSurfaceCreate *surface, int async);
+void qemu_spice_destroy_primary_surface_async(SimpleSpiceDisplay *ssd, uint32_t id, int async);
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: bump pci rev
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (8 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: async I/O Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-08  7:19   ` Gerd Hoffmann
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: only disallow specific io's in vga mode Alon Levy
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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).

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

diff --git a/hw/qxl.c b/hw/qxl.c
index f72d5b8..395d994 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1541,9 +1541,12 @@ 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;
+    case 3: /* qxl-3 */
+    default:
+        pci_device_rev = 3;
+        break;
     }
 
     pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
@@ -1807,7 +1810,7 @@ static PCIDeviceInfo qxl_info_primary = {
     .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("revision", PCIQXLDevice, revision, 3),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
@@ -1828,7 +1831,7 @@ static PCIDeviceInfo qxl_info_secondary = {
     .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("revision", PCIQXLDevice, revision, 3),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: only disallow specific io's in vga mode
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (9 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: bump pci rev Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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.
---
 hw/qxl.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 395d994..e57ccf2 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1167,8 +1167,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",
             __FUNCTION__, io_port, io_port_to_string(io_port));
         /* be nice to buggy guest drivers */
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (10 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: only disallow specific io's in vga mode Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use QXL_REVISION_* Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use update_area_async in qxl-render Alon Levy
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 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>
---
 hw/qxl.c |   25 +++++++++++++++++++++++++
 hw/qxl.h |    1 +
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index e57ccf2..4007847 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -177,6 +177,11 @@ static void qxl_spice_destroy_surface_wait_async(PCIQXLDevice *qxl, uint32_t id,
     }
 }
 
+void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
+{
+    qxl->ssd.worker->flush_surfaces_async(qxl->ssd.worker, 0);
+}
+
 void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
                                uint32_t count)
 {
@@ -1187,6 +1192,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
     case QXL_IO_DESTROY_PRIMARY_ASYNC:
     case QXL_IO_DESTROY_SURFACE_ASYNC:
     case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+    case QXL_IO_FLUSH_SURFACES_ASYNC:
         if (!qemu_spice_supports_async(&d->ssd)) {
             fprintf(stderr, "qxl: error: async not supported by libspice but guest driver used it\n");
             return;
@@ -1306,6 +1312,25 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
         }
         qxl_spice_destroy_surface_wait_async(d, val, async);
         break;
+    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);
+        break;
     case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
     case QXL_IO_DESTROY_ALL_SURFACES:
         d->mode = QXL_MODE_UNDEFINED;
diff --git a/hw/qxl.h b/hw/qxl.h
index 16639ce..23d6215 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -129,6 +129,7 @@ void qxl_spice_oom_async(PCIQXLDevice *qxl, int async);
 void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
+void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl);
 
 /* qxl-logger.c */
 void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: use QXL_REVISION_*
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (11 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use update_area_async in qxl-render Alon Levy
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

---
 hw/qxl.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 4007847..54ee5f5 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1551,7 +1551,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;
@@ -1563,19 +1562,18 @@ static int qxl_init_common(PCIQXLDevice *qxl)
     qxl->current_async = QXL_UNDEFINED_IO;
 
     switch (qxl->revision) {
-    case 1: /* spice 0.4 -- qxl-1 */
-        pci_device_rev = QXL_REVISION_STABLE_V04;
+    case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
+    case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
+    case QXL_REVISION_STABLE_V10: /* spice 0.10? -- qxl-3 */
         break;
-    case 2: /* spice 0.6 -- qxl-2 */
-        pci_device_rev = QXL_REVISION_STABLE_V06;
-        break;
-    case 3: /* qxl-3 */
     default:
-        pci_device_rev = 3;
+        fprintf(stderr, "invalid revision %d, resetting to %d\n", qxl->revision,
+            QXL_REVISION_STABLE_V10);
+        qxl->revision = QXL_REVISION_STABLE_V10;
         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();
@@ -1586,14 +1584,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;
     }
 
@@ -1836,7 +1834,7 @@ static PCIDeviceInfo qxl_info_primary = {
     .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, 3),
+        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, QXL_REVISION_STABLE_V10),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
@@ -1857,7 +1855,7 @@ static PCIDeviceInfo qxl_info_secondary = {
     .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, 3),
+        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, QXL_REVISION_STABLE_V10),
         DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
         DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
         DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
-- 
1.7.5.4

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

* [Qemu-devel] [PATCH v2] qxl: use update_area_async in qxl-render
  2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
                   ` (12 preceding siblings ...)
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use QXL_REVISION_* Alon Levy
@ 2011-07-07 16:50 ` Alon Levy
  13 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-07 16:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: yhalperi, kraxel

---
 hw/qxl-render.c |    4 ++--
 hw/qxl.c        |   14 +++++---------
 hw/qxl.h        |    3 ---
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 60b822d..8d847d0 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_spice_update_area(qxl, 0, &update,
-                          dirty, ARRAY_SIZE(dirty), 1);
+    qxl_spice_update_area_async(qxl, 0, &update,
+                          dirty, ARRAY_SIZE(dirty), 1, 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 54ee5f5..c447e89 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -150,14 +150,6 @@ void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_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)
-{
-    qxl_spice_update_area_async(qxl, surface_id, area, dirty_rects,
-                                num_dirty_rects, clear_dirty_region, 0);
-}
-
 static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl)
 {
     qemu_mutex_lock(&qxl->track_lock);
@@ -776,7 +768,11 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
         qxl_spice_destroy_surface_wait_complete(qxl);
         break;
     }
-    qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
+    if (current_async != QXL_IO_UPDATE_AREA_ASYNC || qxl->mode != QXL_MODE_VGA) {
+        qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
+    } else {
+        dprint(qxl, 1, "in vga mode; update_area_async interrupt not sent\n");
+    }
 }
 
 static const QXLInterface qxl_interface = {
diff --git a/hw/qxl.h b/hw/qxl.h
index 23d6215..f02c28f 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -115,9 +115,6 @@ typedef struct PCIQXLDevice {
 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,
-                           uint32_t num_dirty_rects, uint32_t clear_dirty_region);
 void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
                            struct QXLRect *area, struct QXLRect *dirty_rects,
                            uint32_t num_dirty_rects, uint32_t clear_dirty_region,
-- 
1.7.5.4

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

* Re: [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: async I/O Alon Levy
@ 2011-07-08  7:17   ` Gerd Hoffmann
  2011-07-08  8:00     ` Alon Levy
  2011-07-08  8:12     ` Alon Levy
  0 siblings, 2 replies; 23+ messages in thread
From: Gerd Hoffmann @ 2011-07-08  7:17 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

> +void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
> +                           struct QXLRect *area, struct QXLRect *dirty_rects,
> +                           uint32_t num_dirty_rects, uint32_t clear_dirty_region,
> +                           int async)
> +{
> +    if (async) {
> +        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area, dirty_rects,
> +                                 num_dirty_rects, clear_dirty_region, 0);

Fails to build with older libspice.

> +    } else {
> +        qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
> +                                 num_dirty_rects, clear_dirty_region);
> +    }
> +}
>
>   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);
> +    qxl_spice_update_area_async(qxl, surface_id, area, dirty_rects,
> +                                num_dirty_rects, clear_dirty_region, 0);
>   }

Pretty pointless wrapper IMHO.

> -void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
> +static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl)
>   {
>       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.cmds[qxl->io_data.surface_id] = 0;

I'd suggest to pass in the surface id as argument instead.

>       qxl->guest_surfaces.count--;
>       qemu_mutex_unlock(&qxl->track_lock);
>   }
>
> +static void qxl_spice_destroy_surface_wait_async(PCIQXLDevice *qxl, uint32_t id, int async)
> +{
> +    qxl->io_data.surface_id = id;
> +    if (async) {
> +        qxl->ssd.worker->destroy_surface_wait_async(qxl->ssd.worker, id, 0);
> +    } else {
> +        qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
> +        qxl_spice_destroy_surface_wait_complete(qxl);

qxl_spice_destroy_surface_wait_complete(qxl, id);

> +    }
> +}
> +
>   void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
>                                  uint32_t count)
>   {
> @@ -171,15 +193,29 @@ 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->ssd.worker->destroy_surfaces(qxl->ssd.worker);
> +    qxl_spice_destroy_surfaces_complete(qxl);
> +}
> +
> +static void qxl_spice_destroy_surfaces_async(PCIQXLDevice *qxl, int async)
> +{
> +    if (async) {
> +        qxl->ssd.worker->destroy_surfaces_async(qxl->ssd.worker, 0);
> +    } else {
> +        qxl_spice_destroy_surfaces(qxl);
> +    }
> +}

I'd combine those into one function simliar to 
qxl_spice_destroy_surface_wait_async (and we don't need the _async 
suffix if we have a single version only which gets passed in async as 
argument).

> +
>   void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
>   {
>       qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
> @@ -706,6 +742,38 @@ static int interface_flush_resources(QXLInstance *sin)
>       return ret;
>   }
>
> +static void qxl_add_memslot_complete(PCIQXLDevice *d);
> +static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
> +
> +/* 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);

I'd tend to use the cookie to pass that information (also the stuff in 
io_data).

> -static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
> +static void qxl_add_memslot_complete(PCIQXLDevice *d)

I think it isn't needed to move that to the completion callback.  Memory 
slots can be created and destroyed with I/O commands only, so there is 
no need to care about the ordering like we have to with surfaces.

>       qemu_mutex_init(&qxl->track_lock);
> +    qemu_mutex_init(&qxl->async_lock);

Do we really need two locks?
When passing info via cookie, doesn't the need for the async lock go 
away completely?

> index af10ae8..b7bc0de 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -62,6 +62,20 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
>       dest->right = MAX(dest->right, r->right);
>   }
>
> +int qemu_spice_supports_async(SimpleSpiceDisplay *ssd)
> +{
> +    return (ssd->worker->major_version>  3 ||
> +            (ssd->worker->major_version == 3&&  ssd->worker->minor_version>= 1));
> +}

Doing a runtime check here is pointless, just use
#if SPICE_INTERFACE_QXL_MINOR >= 1
...
#endif

>   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_async(SimpleSpiceDisplay *ssd, uint32_t id, int async)
> +{
> +    if (async) {
> +        ssd->worker->destroy_primary_surface_async(ssd->worker, id, 0);
> +    } else {
> +        qemu_spice_destroy_primary_surface(ssd, id);
> +    }
> +}

Like for all others: one only please.

cheers,
   Gerd

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

* Re: [Qemu-devel] [PATCH v2] qxl: bump pci rev
  2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: bump pci rev Alon Levy
@ 2011-07-08  7:19   ` Gerd Hoffmann
  2011-07-08  8:02     ` Alon Levy
  0 siblings, 1 reply; 23+ messages in thread
From: Gerd Hoffmann @ 2011-07-08  7:19 UTC (permalink / raw)
  To: Alon Levy; +Cc: yhalperi, qemu-devel

   Hi,

> -        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
> +        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 3),

Can't be done unconditionally.  With an older libspice we can't support 
the rev3 features.

cheers,
   Gerd

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

* Re: [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-08  7:17   ` Gerd Hoffmann
@ 2011-07-08  8:00     ` Alon Levy
  2011-07-08  8:10       ` Gerd Hoffmann
  2011-07-08  8:12     ` Alon Levy
  1 sibling, 1 reply; 23+ messages in thread
From: Alon Levy @ 2011-07-08  8:00 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 08, 2011 at 09:17:50AM +0200, Gerd Hoffmann wrote:
> >+void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
> >+                           struct QXLRect *area, struct QXLRect *dirty_rects,
> >+                           uint32_t num_dirty_rects, uint32_t clear_dirty_region,
> >+                           int async)
> >+{
> >+    if (async) {
> >+        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area, dirty_rects,
> >+                                 num_dirty_rects, clear_dirty_region, 0);
> 
> Fails to build with older libspice.

> 
> >+    } else {
> >+        qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
> >+                                 num_dirty_rects, clear_dirty_region);
> >+    }
> >+}
> >
> >  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);
> >+    qxl_spice_update_area_async(qxl, surface_id, area, dirty_rects,
> >+                                num_dirty_rects, clear_dirty_region, 0);
> >  }
> 
> Pretty pointless wrapper IMHO.

The above two lines change was a mistake. What about:

qxl_spice_update_area_async(...)
{
#ifdef ..
 if (async) {
    qxl->ssd.worker->update_area_async(...)
 } else {
    qxl_spice_update_area(...)
 }
#else
 qxl_spice_update_area(...)
#endif
}

> 
> >-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
> >+static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl)
> >  {
> >      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.cmds[qxl->io_data.surface_id] = 0;
> 
> I'd suggest to pass in the surface id as argument instead.

I can use the cookie if that's what you mean (which I guess means it will make
more sense to define it as a void pointer).
> 
> >      qxl->guest_surfaces.count--;
> >      qemu_mutex_unlock(&qxl->track_lock);
> >  }
> >
> >+static void qxl_spice_destroy_surface_wait_async(PCIQXLDevice *qxl, uint32_t id, int async)
> >+{
> >+    qxl->io_data.surface_id = id;
> >+    if (async) {
> >+        qxl->ssd.worker->destroy_surface_wait_async(qxl->ssd.worker, id, 0);
> >+    } else {
> >+        qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
> >+        qxl_spice_destroy_surface_wait_complete(qxl);
> 
> qxl_spice_destroy_surface_wait_complete(qxl, id);
and use the cookie on the async_complete with appropriate casting and free, got it.

> 
> >+    }
> >+}
> >+
> >  void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
> >                                 uint32_t count)
> >  {
> >@@ -171,15 +193,29 @@ 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->ssd.worker->destroy_surfaces(qxl->ssd.worker);
> >+    qxl_spice_destroy_surfaces_complete(qxl);
> >+}
> >+
> >+static void qxl_spice_destroy_surfaces_async(PCIQXLDevice *qxl, int async)
> >+{
> >+    if (async) {
> >+        qxl->ssd.worker->destroy_surfaces_async(qxl->ssd.worker, 0);
> >+    } else {
> >+        qxl_spice_destroy_surfaces(qxl);
> >+    }
> >+}
> 
> I'd combine those into one function simliar to
> qxl_spice_destroy_surface_wait_async (and we don't need the _async
> suffix if we have a single version only which gets passed in async
> as argument).
ok, I'll ditch the suffix.

> 
> >+
> >  void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
> >  {
> >      qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
> >@@ -706,6 +742,38 @@ static int interface_flush_resources(QXLInstance *sin)
> >      return ret;
> >  }
> >
> >+static void qxl_add_memslot_complete(PCIQXLDevice *d);
> >+static void qxl_create_guest_primary_complete(PCIQXLDevice *d);
> >+
> >+/* 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);
> 
> I'd tend to use the cookie to pass that information (also the stuff
> in io_data).

yeah, I'll throw that, malloc something, cast to cookie, pass it, cast back, free.

> 
> >-static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
> >+static void qxl_add_memslot_complete(PCIQXLDevice *d)
> 
> I think it isn't needed to move that to the completion callback.
> Memory slots can be created and destroyed with I/O commands only, so
> there is no need to care about the ordering like we have to with
> surfaces.
ok.

> 
> >      qemu_mutex_init(&qxl->track_lock);
> >+    qemu_mutex_init(&qxl->async_lock);
> 
> Do we really need two locks?
> When passing info via cookie, doesn't the need for the async lock go
> away completely?
the current_async still gets changed from two threads (on [vcpu]ioport_write
and [worker]async_complete)

> 
> >index af10ae8..b7bc0de 100644
> >--- a/ui/spice-display.c
> >+++ b/ui/spice-display.c
> >@@ -62,6 +62,20 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
> >      dest->right = MAX(dest->right, r->right);
> >  }
> >
> >+int qemu_spice_supports_async(SimpleSpiceDisplay *ssd)
> >+{
> >+    return (ssd->worker->major_version>  3 ||
> >+            (ssd->worker->major_version == 3&&  ssd->worker->minor_version>= 1));
> >+}
> 
> Doing a runtime check here is pointless, just use
> #if SPICE_INTERFACE_QXL_MINOR >= 1
> ...
> #endif
this is a runtime check - what's preventing someone from compiling with 3.1 and running with 3.0?
that we will require a newer library version? (which I am yet to send a patch for)

> 
> >  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_async(SimpleSpiceDisplay *ssd, uint32_t id, int async)
> >+{
> >+    if (async) {
> >+        ssd->worker->destroy_primary_surface_async(ssd->worker, id, 0);
> >+    } else {
> >+        qemu_spice_destroy_primary_surface(ssd, id);
> >+    }
> >+}
> 
> Like for all others: one only please.
ok

> 
> cheers,
>   Gerd

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

* Re: [Qemu-devel] [PATCH v2] qxl: bump pci rev
  2011-07-08  7:19   ` Gerd Hoffmann
@ 2011-07-08  8:02     ` Alon Levy
  0 siblings, 0 replies; 23+ messages in thread
From: Alon Levy @ 2011-07-08  8:02 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 08, 2011 at 09:19:10AM +0200, Gerd Hoffmann wrote:
>   Hi,
> 
> >-        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
> >+        DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 3),
> 
> Can't be done unconditionally.  With an older libspice we can't
> support the rev3 features.

ok. so this needs a runtime check for major_version.minor_version >= 3.1, no? a compile
time won't work because someone can change the shared object, no? (maybe I should just
check this scenario).

> 
> cheers,
>   Gerd

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

* Re: [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-08  8:00     ` Alon Levy
@ 2011-07-08  8:10       ` Gerd Hoffmann
  0 siblings, 0 replies; 23+ messages in thread
From: Gerd Hoffmann @ 2011-07-08  8:10 UTC (permalink / raw)
  To: qemu-devel, yhalperi

> The above two lines change was a mistake. What about:
>
> qxl_spice_update_area_async(...)
> {
> #ifdef ..
>   if (async) {
>      qxl->ssd.worker->update_area_async(...)
>   } else {
>      qxl_spice_update_area(...)
>   }
> #else
>   qxl_spice_update_area(...)
> #endif
> }

I would do

if (async) {
#if ...
   worker->foo_async()
#else
   abort() /* should hot happen */
#endif
} else {
   worker->foo
}

> yeah, I'll throw that, malloc something, cast to cookie, pass it, cast back, free.

cookie should be big enougth to store the info directly.  malloc works 
too though.

>> Doing a runtime check here is pointless, just use
>> #if SPICE_INTERFACE_QXL_MINOR>= 1
>> ...
>> #endif
> this is a runtime check - what's preventing someone from compiling with 3.1 and running with 3.0?
> that we will require a newer library version? (which I am yet to send a patch for)

Yes, thats why the minor version of the shared library needs to be raised.

cheers,
   Gerd

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

* Re: [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-08  7:17   ` Gerd Hoffmann
  2011-07-08  8:00     ` Alon Levy
@ 2011-07-08  8:12     ` Alon Levy
  2011-07-08  8:16       ` Gerd Hoffmann
  1 sibling, 1 reply; 23+ messages in thread
From: Alon Levy @ 2011-07-08  8:12 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: yhalperi, qemu-devel

On Fri, Jul 08, 2011 at 09:17:50AM +0200, Gerd Hoffmann wrote:
> >+void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
> >+                           struct QXLRect *area, struct QXLRect *dirty_rects,
> >+                           uint32_t num_dirty_rects, uint32_t clear_dirty_region,
> >+                           int async)
> >+{
> >+    if (async) {
> >+        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area, dirty_rects,
> >+                                 num_dirty_rects, clear_dirty_region, 0);
> 
> Fails to build with older libspice.

btw, I'm looking at "#if.*MINOR" code like

 #if SPICE_INTERFACE_CORE_MINOR >= 3

(ui/spice-core.c)

Shouldn't that be checking the MAJOR as well?

[snip]

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

* Re: [Qemu-devel] [PATCH v2] qxl: async I/O
  2011-07-08  8:12     ` Alon Levy
@ 2011-07-08  8:16       ` Gerd Hoffmann
  0 siblings, 0 replies; 23+ messages in thread
From: Gerd Hoffmann @ 2011-07-08  8:16 UTC (permalink / raw)
  To: qemu-devel, yhalperi

> btw, I'm looking at "#if.*MINOR" code like
>
>   #if SPICE_INTERFACE_CORE_MINOR>= 3
>
> (ui/spice-core.c)
>
> Shouldn't that be checking the MAJOR as well?

major changing means a incompatible change.  I doubt we ever will do 
that.  But if you feel better checking that it probably should just be a

#if SPICE_INTERFACE_CORE_MAJOR != 1
#error incompatible spice core interface
#endif

at the top of the file.

cheers,
   Gerd

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

end of thread, other threads:[~2011-07-08  8:16 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-07 16:50 [Qemu-devel] [PATCH v2] async + suspend reworked Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add worker wrapper functions Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice: add qemu_spice_display_init_common Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: remove qxl_destroy_primary() Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] spice/qxl: move worker wrappers Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: fix surface tracking & locking Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add io_port_to_string Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: error handling fixes and cleanups Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: make qxl_guest_bug take variable arguments Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: async I/O Alon Levy
2011-07-08  7:17   ` Gerd Hoffmann
2011-07-08  8:00     ` Alon Levy
2011-07-08  8:10       ` Gerd Hoffmann
2011-07-08  8:12     ` Alon Levy
2011-07-08  8:16       ` Gerd Hoffmann
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: bump pci rev Alon Levy
2011-07-08  7:19   ` Gerd Hoffmann
2011-07-08  8:02     ` Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: only disallow specific io's in vga mode Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use QXL_REVISION_* Alon Levy
2011-07-07 16:50 ` [Qemu-devel] [PATCH v2] qxl: use update_area_async in qxl-render Alon Levy
  -- strict thread matches above, loose matches on Subject: below --
2011-06-24 13:02 [Qemu-devel] [PATCH v2] S3&S4 support Alon Levy
2011-06-24 13:02 ` [Qemu-devel] [PATCH v2] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest " Alon Levy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).