From e0c64d08d11736dcea7c5a6373e3e7f62db51d9e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 27 Apr 2011 15:21:51 +0200 Subject: spice: don't create updates in spice server context. This patch moves the creation of spice screen updates from the spice server context to qemu iothread context (display refresh timer to be exact). This way we avoid accessing qemu internals (display surface) from spice thread context which in turn allows us to simplify locking. Signed-off-by: Gerd Hoffmann --- ui/spice-display.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) (limited to 'ui/spice-display.c') diff --git a/ui/spice-display.c b/ui/spice-display.c index 020b423bd6..d56dcfc7d3 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -62,14 +62,7 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r) dest->right = MAX(dest->right, r->right); } -/* - * Called from spice server thread context (via interface_get_command). - * - * We must aquire the global qemu mutex here to make sure the - * DisplayState (+DisplaySurface) we are accessing doesn't change - * underneath us. - */ -SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) +static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) { SimpleSpiceUpdate *update; QXLDrawable *drawable; @@ -78,9 +71,7 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) uint8_t *src, *dst; int by, bw, bh; - qemu_mutex_lock_iothread(); if (qemu_spice_rect_is_empty(&ssd->dirty)) { - qemu_mutex_unlock_iothread(); return NULL; }; @@ -141,7 +132,6 @@ SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd) cmd->data = (intptr_t)drawable; memset(&ssd->dirty, 0, sizeof(ssd->dirty)); - qemu_mutex_unlock_iothread(); return update; } @@ -241,6 +231,12 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd) qemu_pf_conv_put(ssd->conv); ssd->conv = NULL; + qemu_mutex_lock(&ssd->lock); + if (ssd->update != NULL) { + qemu_spice_destroy_update(ssd, ssd->update); + ssd->update = NULL; + } + qemu_mutex_unlock(&ssd->lock); qemu_spice_destroy_host_primary(ssd); qemu_spice_create_host_primary(ssd); @@ -252,6 +248,14 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) { dprint(3, "%s:\n", __FUNCTION__); vga_hw_update(); + + qemu_mutex_lock(&ssd->lock); + if (ssd->update == NULL) { + ssd->update = qemu_spice_create_update(ssd); + ssd->notify++; + } + qemu_mutex_unlock(&ssd->lock); + if (ssd->notify) { ssd->notify = 0; ssd->worker->wakeup(ssd->worker); @@ -298,14 +302,20 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) { SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl); SimpleSpiceUpdate *update; + int ret = false; dprint(3, "%s:\n", __FUNCTION__); - update = qemu_spice_create_update(ssd); - if (update == NULL) { - return false; + + qemu_mutex_lock(&ssd->lock); + if (ssd->update != NULL) { + update = ssd->update; + ssd->update = NULL; + *ext = update->ext; + ret = true; } - *ext = update->ext; - return true; + qemu_mutex_unlock(&ssd->lock); + + return ret; } static int interface_req_cmd_notification(QXLInstance *sin) @@ -398,6 +408,7 @@ void qemu_spice_display_init(DisplayState *ds) { assert(sdpy.ds == NULL); sdpy.ds = ds; + qemu_mutex_init(&sdpy.lock); sdpy.bufsize = (16 * 1024 * 1024); sdpy.buf = qemu_malloc(sdpy.bufsize); register_displaychangelistener(ds, &display_listener); -- cgit 1.4.1 From 075360945860ad9bdd491921954b383bf762b0e5 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 27 Apr 2011 15:50:32 +0200 Subject: spice: don't call displaystate callbacks from spice server context. This patch moves the displaystate callback calls for setting the cursor and the mouse pointer from spice server to qemu (iothread) context. This allows us to simplify locking. Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 25 ++++++++++++------------- hw/qxl.c | 2 ++ ui/spice-display.c | 12 ++++++++++++ ui/spice-display.h | 3 +++ 4 files changed, 29 insertions(+), 13 deletions(-) (limited to 'ui/spice-display.c') diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 58965e0179..1316066599 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -185,7 +185,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id); QXLCursor *cursor; QEMUCursor *c; - int x = -1, y = -1; if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) { return; @@ -198,8 +197,6 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) } switch (cmd->type) { case QXL_CURSOR_SET: - x = cmd->u.set.position.x; - y = cmd->u.set.position.y; cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id); if (cursor->chunk.data_size != cursor->data_size) { fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__); @@ -209,18 +206,20 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext) if (c == NULL) { c = cursor_builtin_left_ptr(); } - qemu_mutex_lock_iothread(); - qxl->ssd.ds->cursor_define(c); - qxl->ssd.ds->mouse_set(x, y, 1); - qemu_mutex_unlock_iothread(); - cursor_put(c); + qemu_mutex_lock(&qxl->ssd.lock); + if (qxl->ssd.cursor) { + cursor_put(qxl->ssd.cursor); + } + qxl->ssd.cursor = c; + qxl->ssd.mouse_x = cmd->u.set.position.x; + qxl->ssd.mouse_y = cmd->u.set.position.y; + qemu_mutex_unlock(&qxl->ssd.lock); break; case QXL_CURSOR_MOVE: - x = cmd->u.position.x; - y = cmd->u.position.y; - qemu_mutex_lock_iothread(); - qxl->ssd.ds->mouse_set(x, y, 1); - qemu_mutex_unlock_iothread(); + qemu_mutex_lock(&qxl->ssd.lock); + qxl->ssd.mouse_x = cmd->u.position.x; + qxl->ssd.mouse_y = cmd->u.position.y; + qemu_mutex_unlock(&qxl->ssd.lock); break; } } diff --git a/hw/qxl.c b/hw/qxl.c index bd250db885..4dfddf0dce 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1309,6 +1309,8 @@ static int qxl_init_primary(PCIDevice *dev) 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); diff --git a/ui/spice-display.c b/ui/spice-display.c index d56dcfc7d3..8579bfd5b2 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -254,6 +254,16 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) ssd->update = qemu_spice_create_update(ssd); ssd->notify++; } + if (ssd->cursor) { + ssd->ds->cursor_define(ssd->cursor); + cursor_put(ssd->cursor); + ssd->cursor = NULL; + } + if (ssd->mouse_x != -1 && ssd->mouse_y != -1) { + ssd->ds->mouse_set(ssd->mouse_x, ssd->mouse_y, 1); + ssd->mouse_x = -1; + ssd->mouse_y = -1; + } qemu_mutex_unlock(&ssd->lock); if (ssd->notify) { @@ -409,6 +419,8 @@ 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); register_displaychangelistener(ds, &display_listener); diff --git a/ui/spice-display.h b/ui/spice-display.h index e0cc46ef4c..2f95f68aad 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -20,6 +20,7 @@ #include #include "qemu-thread.h" +#include "console.h" #include "pflib.h" #define NUM_MEMSLOTS 8 @@ -55,6 +56,8 @@ struct SimpleSpiceDisplay { */ QemuMutex lock; SimpleSpiceUpdate *update; + QEMUCursor *cursor; + int mouse_x, mouse_y; }; struct SimpleSpiceUpdate { -- cgit 1.4.1 From 196a778428989217b82de042725dc8eb29c8f8d8 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 27 Apr 2011 16:06:10 +0200 Subject: spice: drop obsolete iothread locking We don't use qemu internals from spice server context any more. Thus we don't also need to grab the iothread mutex from spice server context. And we don't have to temporarely release the lock to avoid deadlocks. Drop all the calls. Signed-off-by: Gerd Hoffmann --- hw/qxl.c | 8 -------- ui/spice-display.c | 6 ------ 2 files changed, 14 deletions(-) (limited to 'ui/spice-display.c') diff --git a/hw/qxl.c b/hw/qxl.c index 4dfddf0dce..2bb36c660f 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -666,10 +666,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm) dprint(d, 1, "%s: start%s\n", __FUNCTION__, loadvm ? " (loadvm)" : ""); - qemu_mutex_unlock_iothread(); d->ssd.worker->reset_cursor(d->ssd.worker); d->ssd.worker->reset_image_cache(d->ssd.worker); - qemu_mutex_lock_iothread(); qxl_reset_surfaces(d); qxl_reset_memslots(d); @@ -799,9 +797,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d) { dprint(d, 1, "%s:\n", __FUNCTION__); d->mode = QXL_MODE_UNDEFINED; - qemu_mutex_unlock_iothread(); d->ssd.worker->destroy_surfaces(d->ssd.worker); - qemu_mutex_lock_iothread(); memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds)); } @@ -870,9 +866,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d) dprint(d, 1, "%s\n", __FUNCTION__); d->mode = QXL_MODE_UNDEFINED; - qemu_mutex_unlock_iothread(); d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0); - qemu_mutex_lock_iothread(); } static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm) @@ -942,10 +936,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_mutex_unlock_iothread(); d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface, &update, NULL, 0, 0); - qemu_mutex_lock_iothread(); break; } case QXL_IO_NOTIFY_CMD: diff --git a/ui/spice-display.c b/ui/spice-display.c index 8579bfd5b2..15f0704eaf 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -176,18 +176,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd) surface.mem = (intptr_t)ssd->buf; surface.group_id = MEMSLOT_GROUP_HOST; - qemu_mutex_unlock_iothread(); ssd->worker->create_primary_surface(ssd->worker, 0, &surface); - qemu_mutex_lock_iothread(); } void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd) { dprint(1, "%s:\n", __FUNCTION__); - qemu_mutex_unlock_iothread(); ssd->worker->destroy_primary_surface(ssd->worker, 0); - qemu_mutex_lock_iothread(); } void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason) @@ -197,9 +193,7 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason) if (running) { ssd->worker->start(ssd->worker); } else { - qemu_mutex_unlock_iothread(); ssd->worker->stop(ssd->worker); - qemu_mutex_lock_iothread(); } ssd->running = running; } -- cgit 1.4.1