From bb5a8cd5b0aec0286c5e749803083de22c2b0be6 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:25 +0200 Subject: [PATCH 01/11] qxl: fix spice+sdl no cursor regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit regression introduced by 075360945860ad9bdd491921954b383bf762b0e5, v2: lock around qemu_spice_cursor_refresh_unlocked Reported-by: Fabiano FidĂȘncio Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl.c | 4 ++++ ui/spice-display.c | 23 ++++++++++++++--------- ui/spice-display.h | 1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index f643667205..0bbf0e8c16 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1545,6 +1545,10 @@ static void display_refresh(struct DisplayState *ds) { if (qxl0->mode == QXL_MODE_VGA) { qemu_spice_display_refresh(&qxl0->ssd); + } else { + qemu_mutex_lock(&qxl0->ssd.lock); + qemu_spice_cursor_refresh_unlocked(&qxl0->ssd); + qemu_mutex_unlock(&qxl0->ssd.lock); } } diff --git a/ui/spice-display.c b/ui/spice-display.c index 6c302a3909..c6e61d84ea 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -317,16 +317,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd) ssd->notify++; } -void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +void qemu_spice_cursor_refresh_unlocked(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++; - } if (ssd->cursor) { ssd->ds->cursor_define(ssd->cursor); cursor_put(ssd->cursor); @@ -337,6 +329,19 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) ssd->mouse_x = -1; ssd->mouse_y = -1; } +} + +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +{ + dprint(3, "%s:\n", __func__); + vga_hw_update(); + + qemu_mutex_lock(&ssd->lock); + if (ssd->update == NULL) { + ssd->update = qemu_spice_create_update(ssd); + ssd->notify++; + } + qemu_spice_cursor_refresh_unlocked(ssd); qemu_mutex_unlock(&ssd->lock); if (ssd->notify) { diff --git a/ui/spice-display.h b/ui/spice-display.h index 5e52df99be..a23bfc8502 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -97,6 +97,7 @@ 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_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd); void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, qxl_async_io async); From 63ea491d4efc1e02cda3d335db3a46c81adf14ee Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:26 +0200 Subject: [PATCH 02/11] sdl: remove NULL check, g_malloc0 can't fail Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- ui/sdl.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ui/sdl.c b/ui/sdl.c index 6f8091c725..f6f711c1bb 100644 --- a/ui/sdl.c +++ b/ui/sdl.c @@ -167,10 +167,6 @@ static PixelFormat sdl_to_qemu_pixelformat(SDL_PixelFormat *sdl_pf) static DisplaySurface* sdl_create_displaysurface(int width, int height) { DisplaySurface *surface = (DisplaySurface*) g_malloc0(sizeof(DisplaySurface)); - if (surface == NULL) { - fprintf(stderr, "sdl_create_displaysurface: malloc failed\n"); - exit(1); - } surface->width = width; surface->height = height; From 45a4b48528aa20b602eb8c764d511fb1c4d6cab7 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:27 +0200 Subject: [PATCH 03/11] qxl: drop qxl_spice_update_area_async definition It was never used. Introduced in 5ff4e36c804157bd84af43c139f8cd3a59722db9 qxl: async io support using new spice api But not used even then. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/hw/qxl.h b/hw/qxl.h index d0629916ad..a615ecaebb 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -138,9 +138,3 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); void qxl_render_resize(PCIQXLDevice *qxl); void qxl_render_update(PCIQXLDevice *qxl); void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext); -#if SPICE_INTERFACE_QXL_MINOR >= 1 -void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id, - struct QXLRect *area, - uint32_t clear_dirty_region, - int is_vga); -#endif From 4295e15aa730a95003a3639d6dad2eb1e65a59e2 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:28 +0200 Subject: [PATCH 04/11] qxl: require spice >= 0.8.2 drop all ifdefs on SPICE_INTERFACE_QXL_MINOR >= 1 as a result, any check for SPICE_SERVER_VERSION that is now always satisfied, and SPICE_INTERFACE_CORE_MINOR >= 3 tests, because 0.8.2 has SPICE_INTERFACE_QXL_MINOR == 1 and SPICE_INTERFACE_CORE_MINOR == 3. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- configure | 2 +- hw/qxl.c | 40 ---------------------------------------- hw/qxl.h | 4 ---- ui/spice-core.c | 17 ----------------- ui/spice-display.c | 12 ------------ 5 files changed, 1 insertion(+), 74 deletions(-) diff --git a/configure b/configure index f9d533004b..9535f66940 100755 --- a/configure +++ b/configure @@ -2547,7 +2547,7 @@ int main(void) { spice_server_new(); return 0; } EOF spice_cflags=$($pkg_config --cflags spice-protocol spice-server 2>/dev/null) spice_libs=$($pkg_config --libs spice-protocol spice-server 2>/dev/null) - if $pkg_config --atleast-version=0.6.0 spice-server >/dev/null 2>&1 && \ + if $pkg_config --atleast-version=0.8.2 spice-server >/dev/null 2>&1 && \ compile_prog "$spice_cflags" "$spice_libs" ; then spice="yes" libs_softmmu="$libs_softmmu $spice_libs" diff --git a/hw/qxl.c b/hw/qxl.c index 0bbf0e8c16..52df97879e 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -125,9 +125,7 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl); void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...) { -#if SPICE_INTERFACE_QXL_MINOR >= 1 qxl_send_events(qxl, QXL_INTERRUPT_ERROR); -#endif if (qxl->guestdebug) { va_list ap; va_start(ap, msg); @@ -149,12 +147,8 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id, qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects, num_dirty_rects, clear_dirty_region); } else { -#if SPICE_INTERFACE_QXL_MINOR >= 1 spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area, clear_dirty_region, 0); -#else - abort(); -#endif } } @@ -171,24 +165,18 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id, qxl_async_io async) { if (async) { -#if SPICE_INTERFACE_QXL_MINOR < 1 - abort(); -#else spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)id); -#endif } else { qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id); qxl_spice_destroy_surface_wait_complete(qxl, id); } } -#if SPICE_INTERFACE_QXL_MINOR >= 1 static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl) { spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0); } -#endif void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext, uint32_t count) @@ -217,11 +205,7 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl) static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async) { if (async) { -#if SPICE_INTERFACE_QXL_MINOR < 1 - abort(); -#else spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0); -#endif } else { qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker); qxl_spice_destroy_surfaces_complete(qxl); @@ -490,7 +474,6 @@ static const char *io_port_to_string(uint32_t io_port) [QXL_IO_DESTROY_PRIMARY] = "QXL_IO_DESTROY_PRIMARY", [QXL_IO_DESTROY_SURFACE_WAIT] = "QXL_IO_DESTROY_SURFACE_WAIT", [QXL_IO_DESTROY_ALL_SURFACES] = "QXL_IO_DESTROY_ALL_SURFACES", -#if SPICE_INTERFACE_QXL_MINOR >= 1 [QXL_IO_UPDATE_AREA_ASYNC] = "QXL_IO_UPDATE_AREA_ASYNC", [QXL_IO_MEMSLOT_ADD_ASYNC] = "QXL_IO_MEMSLOT_ADD_ASYNC", [QXL_IO_CREATE_PRIMARY_ASYNC] = "QXL_IO_CREATE_PRIMARY_ASYNC", @@ -500,7 +483,6 @@ static const char *io_port_to_string(uint32_t io_port) = "QXL_IO_DESTROY_ALL_SURFACES_ASYNC", [QXL_IO_FLUSH_SURFACES_ASYNC] = "QXL_IO_FLUSH_SURFACES_ASYNC", [QXL_IO_FLUSH_RELEASE] = "QXL_IO_FLUSH_RELEASE", -#endif }; return io_port_to_string[io_port]; } @@ -735,8 +717,6 @@ static int interface_flush_resources(QXLInstance *sin) static void qxl_create_guest_primary_complete(PCIQXLDevice *d); -#if SPICE_INTERFACE_QXL_MINOR >= 1 - /* called from spice server thread context only */ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) { @@ -764,8 +744,6 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); } -#endif - static const QXLInterface qxl_interface = { .base.type = SPICE_INTERFACE_QXL, .base.description = "qxl gpu", @@ -785,9 +763,7 @@ static const QXLInterface qxl_interface = { .req_cursor_notification = interface_req_cursor_notification, .notify_update = interface_notify_update, .flush_resources = interface_flush_resources, -#if SPICE_INTERFACE_QXL_MINOR >= 1 .async_complete = interface_async_complete, -#endif }; static void qxl_enter_vga_mode(PCIQXLDevice *d) @@ -1137,9 +1113,7 @@ static void ioport_write(void *opaque, target_phys_addr_t addr, PCIQXLDevice *d = opaque; uint32_t io_port = addr; qxl_async_io async = QXL_SYNC; -#if SPICE_INTERFACE_QXL_MINOR >= 1 uint32_t orig_io_port = io_port; -#endif switch (io_port) { case QXL_IO_RESET: @@ -1149,10 +1123,8 @@ static void ioport_write(void *opaque, target_phys_addr_t addr, case QXL_IO_CREATE_PRIMARY: case QXL_IO_UPDATE_IRQ: case QXL_IO_LOG: -#if SPICE_INTERFACE_QXL_MINOR >= 1 case QXL_IO_MEMSLOT_ADD_ASYNC: case QXL_IO_CREATE_PRIMARY_ASYNC: -#endif break; default: if (d->mode != QXL_MODE_VGA) { @@ -1160,17 +1132,14 @@ static void ioport_write(void *opaque, target_phys_addr_t addr, } dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n", __func__, io_port, io_port_to_string(io_port)); -#if SPICE_INTERFACE_QXL_MINOR >= 1 /* be nice to buggy guest drivers */ if (io_port >= QXL_IO_UPDATE_AREA_ASYNC && io_port <= QXL_IO_DESTROY_ALL_SURFACES_ASYNC) { qxl_send_events(d, QXL_INTERRUPT_IO_CMD); } -#endif return; } -#if SPICE_INTERFACE_QXL_MINOR >= 1 /* we change the io_port to avoid ifdeffery in the main switch */ orig_io_port = io_port; switch (io_port) { @@ -1209,7 +1178,6 @@ async_common: default: break; } -#endif switch (io_port) { case QXL_IO_UPDATE_AREA: @@ -1301,7 +1269,6 @@ async_common: } qxl_spice_destroy_surface_wait(d, val, async); break; -#if SPICE_INTERFACE_QXL_MINOR >= 1 case QXL_IO_FLUSH_RELEASE: { QXLReleaseRing *ring = &d->ram->release_ring; if (ring->prod - ring->cons + 1 == ring->num_items) { @@ -1322,7 +1289,6 @@ async_common: d->num_free_res); qxl_spice_flush_surfaces_async(d); break; -#endif case QXL_IO_DESTROY_ALL_SURFACES: d->mode = QXL_MODE_UNDEFINED; qxl_spice_destroy_surfaces(d, async); @@ -1333,16 +1299,12 @@ async_common: } return; cancel_async: -#if SPICE_INTERFACE_QXL_MINOR >= 1 if (async) { qxl_send_events(d, QXL_INTERRUPT_IO_CMD); qemu_mutex_lock(&d->async_lock); d->current_async = QXL_UNDEFINED_IO; qemu_mutex_unlock(&d->async_lock); } -#else - return; -#endif } static uint64_t ioport_read(void *opaque, target_phys_addr_t addr, @@ -1604,9 +1566,7 @@ static int qxl_init_common(PCIQXLDevice *qxl) case 2: /* spice 0.6 -- qxl-2 */ pci_device_rev = QXL_REVISION_STABLE_V06; break; -#if SPICE_INTERFACE_QXL_MINOR >= 1 case 3: /* qxl-3 */ -#endif default: pci_device_rev = QXL_DEFAULT_REVISION; break; diff --git a/hw/qxl.h b/hw/qxl.h index a615ecaebb..9288e4603d 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -108,11 +108,7 @@ typedef struct PCIQXLDevice { } \ } while (0) -#if SPICE_INTERFACE_QXL_MINOR >= 1 #define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10 -#else -#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V06 -#endif /* qxl.c */ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id); diff --git a/ui/spice-core.c b/ui/spice-core.c index 1308a3d886..30d2c6264c 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -139,8 +139,6 @@ static void watch_remove(SpiceWatch *watch) g_free(watch); } -#if SPICE_INTERFACE_CORE_MINOR >= 3 - typedef struct ChannelList ChannelList; struct ChannelList { SpiceChannelEventInfo *info; @@ -257,15 +255,6 @@ static void channel_event(int event, SpiceChannelEventInfo *info) } } -#else /* SPICE_INTERFACE_CORE_MINOR >= 3 */ - -static QList *channel_list_get(void) -{ - return NULL; -} - -#endif /* SPICE_INTERFACE_CORE_MINOR >= 3 */ - static SpiceCoreInterface core_interface = { .base.type = SPICE_INTERFACE_CORE, .base.description = "qemu core services", @@ -281,9 +270,7 @@ static SpiceCoreInterface core_interface = { .watch_update_mask = watch_update_mask, .watch_remove = watch_remove, -#if SPICE_INTERFACE_CORE_MINOR >= 3 .channel_event = channel_event, -#endif }; #ifdef SPICE_INTERFACE_MIGRATION @@ -490,14 +477,12 @@ static void migration_state_notifier(Notifier *notifier, void *data) spice_server_migrate_start(spice_server); #endif } else if (migration_has_finished(s)) { -#if SPICE_SERVER_VERSION >= 0x000701 /* 0.7.1 */ #ifndef SPICE_INTERFACE_MIGRATION spice_server_migrate_switch(spice_server); #else spice_server_migrate_end(spice_server, true); } else if (migration_has_failed(s)) { spice_server_migrate_end(spice_server, false); -#endif #endif } } @@ -659,11 +644,9 @@ void qemu_spice_init(void) spice_server_set_noauth(spice_server); } -#if SPICE_SERVER_VERSION >= 0x000801 if (qemu_opt_get_bool(opts, "disable-copy-paste", 0)) { spice_server_set_agent_copypaste(spice_server, false); } -#endif compression = SPICE_IMAGE_COMPRESS_AUTO_GLZ; str = qemu_opt_get(opts, "image-compression"); diff --git a/ui/spice-display.c b/ui/spice-display.c index c6e61d84ea..ad76bae82c 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -64,11 +64,7 @@ void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, qxl_async_io async) { if (async != QXL_SYNC) { -#if SPICE_INTERFACE_QXL_MINOR >= 1 spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0); -#else - abort(); -#endif } else { ssd->worker->add_memslot(ssd->worker, memslot); } @@ -84,11 +80,7 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id, qxl_async_io async) { if (async != QXL_SYNC) { -#if SPICE_INTERFACE_QXL_MINOR >= 1 spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0); -#else - abort(); -#endif } else { ssd->worker->create_primary_surface(ssd->worker, id, surface); } @@ -99,11 +91,7 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id, qxl_async_io async) { if (async != QXL_SYNC) { -#if SPICE_INTERFACE_QXL_MINOR >= 1 spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0); -#else - abort(); -#endif } else { ssd->worker->destroy_primary_surface(ssd->worker, id); } From 4c19ebb51dc0a59ff12d60844512816562a25047 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:29 +0200 Subject: [PATCH 05/11] qxl: remove flipped Tested on linux and windows guests. For negative stride, qxl_flip copies directly to vga->ds->surface->data, for positive it's reallocated to share qxl->guest_primary.data Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 66 ++++++++++++++++++++++--------------------------- hw/qxl.h | 2 +- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 708414376e..c32d2fd4a9 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -23,10 +23,21 @@ static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect) { - uint8_t *src = qxl->guest_primary.data; - uint8_t *dst = qxl->guest_primary.flipped; + uint8_t *src; + uint8_t *dst = qxl->vga.ds->surface->data; int len, i; + if (qxl->guest_primary.qxl_stride > 0) { + return; + } + if (!qxl->guest_primary.data) { + dprint(qxl, 1, "%s: initializing guest_primary.data\n", __func__); + qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram); + } + dprint(qxl, 1, "%s: stride %d, [%d, %d, %d, %d]\n", __func__, + qxl->guest_primary.qxl_stride, + rect->left, rect->right, rect->top, rect->bottom); + src = qxl->guest_primary.data; src += (qxl->guest_primary.surface.height - rect->top - 1) * qxl->guest_primary.abs_stride; dst += rect->top * qxl->guest_primary.abs_stride; @@ -75,52 +86,38 @@ void qxl_render_update(PCIQXLDevice *qxl) { VGACommonState *vga = &qxl->vga; QXLRect dirty[32], update; - void *ptr; int i, redraw = 0; - - if (!is_buffer_shared(vga->ds->surface)) { - dprint(qxl, 1, "%s: restoring shared displaysurface\n", __func__); - qxl->guest_primary.resized++; - qxl->guest_primary.commands++; - redraw = 1; - } + DisplaySurface *surface = vga->ds->surface; if (qxl->guest_primary.resized) { qxl->guest_primary.resized = 0; - if (qxl->guest_primary.flipped) { - g_free(qxl->guest_primary.flipped); - qxl->guest_primary.flipped = NULL; - } - qemu_free_displaysurface(vga->ds); - qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram); - if (qxl->guest_primary.qxl_stride < 0) { - /* spice surface is upside down -> need extra buffer to flip */ - qxl->guest_primary.flipped = - g_malloc(qxl->guest_primary.surface.width * - qxl->guest_primary.abs_stride); - ptr = qxl->guest_primary.flipped; - } else { - ptr = qxl->guest_primary.data; - } - dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d, flip %s\n", + dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n", __FUNCTION__, qxl->guest_primary.surface.width, qxl->guest_primary.surface.height, qxl->guest_primary.qxl_stride, qxl->guest_primary.bytes_pp, - qxl->guest_primary.bits_pp, - qxl->guest_primary.flipped ? "yes" : "no"); - vga->ds->surface = + qxl->guest_primary.bits_pp); + } + if (surface->width != qxl->guest_primary.surface.width || + surface->height != qxl->guest_primary.surface.height) { + dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n", + __func__); + if (qxl->guest_primary.qxl_stride > 0) { + qemu_free_displaysurface(vga->ds); qemu_create_displaysurface_from(qxl->guest_primary.surface.width, qxl->guest_primary.surface.height, qxl->guest_primary.bits_pp, qxl->guest_primary.abs_stride, - ptr); - dpy_resize(vga->ds); + qxl->guest_primary.data); + } else { + qemu_resize_displaysurface(vga->ds, + qxl->guest_primary.surface.width, + qxl->guest_primary.surface.height); + } } - update.left = 0; update.right = qxl->guest_primary.surface.width; update.top = 0; @@ -136,14 +133,11 @@ void qxl_render_update(PCIQXLDevice *qxl) memset(dirty, 0, sizeof(dirty)); dirty[0] = update; } - for (i = 0; i < ARRAY_SIZE(dirty); i++) { if (qemu_spice_rect_is_empty(dirty+i)) { break; } - if (qxl->guest_primary.flipped) { - qxl_flip(qxl, dirty+i); - } + qxl_flip(qxl, dirty+i); dpy_update(vga->ds, dirty[i].left, dirty[i].top, dirty[i].right - dirty[i].left, diff --git a/hw/qxl.h b/hw/qxl.h index 9288e4603d..53a3acea37 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -52,7 +52,7 @@ typedef struct PCIQXLDevice { uint32_t abs_stride; uint32_t bits_pp; uint32_t bytes_pp; - uint8_t *data, *flipped; + uint8_t *data; } guest_primary; struct surfaces { From 2e1a98c9c1b90ca093278c6b43244dc46604d7b7 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:30 +0200 Subject: [PATCH 06/11] qxl: introduce QXLCookie Will be used in the next patch. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 2 +- hw/qxl.c | 61 +++++++++++++++++++++++++++++++++++----------- hw/qxl.h | 2 +- ui/spice-display.c | 22 ++++++++++++++--- ui/spice-display.h | 14 +++++++++++ 5 files changed, 82 insertions(+), 19 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index c32d2fd4a9..c45dc30ab3 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -127,7 +127,7 @@ void qxl_render_update(PCIQXLDevice *qxl) if (runstate_is_running() && qxl->guest_primary.commands) { qxl->guest_primary.commands = 0; qxl_spice_update_area(qxl, 0, &update, - dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC); + dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL); } if (redraw) { memset(dirty, 0, sizeof(dirty)); diff --git a/hw/qxl.c b/hw/qxl.c index 52df97879e..3bc0e5c762 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -141,14 +141,15 @@ 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_async_io async) + qxl_async_io async, struct QXLCookie *cookie) { if (async == QXL_SYNC) { qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects, num_dirty_rects, clear_dirty_region); } else { + assert(cookie != NULL); spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area, - clear_dirty_region, 0); + clear_dirty_region, (uint64_t)cookie); } } @@ -164,9 +165,13 @@ static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl, static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id, qxl_async_io async) { + QXLCookie *cookie; + if (async) { - spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, - (uint64_t)id); + cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_DESTROY_SURFACE_ASYNC); + cookie->u.surface_id = id; + spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)cookie); } else { qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id); qxl_spice_destroy_surface_wait_complete(qxl, id); @@ -175,7 +180,9 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id, static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl) { - spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0); + spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, + (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_FLUSH_SURFACES_ASYNC)); } void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext, @@ -205,7 +212,9 @@ static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl) static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async) { if (async) { - spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0); + spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, + (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_DESTROY_ALL_SURFACES_ASYNC)); } else { qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker); qxl_spice_destroy_surfaces_complete(qxl); @@ -718,9 +727,8 @@ static int interface_flush_resources(QXLInstance *sin) 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) +static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie) { - PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); uint32_t current_async; qemu_mutex_lock(&qxl->async_lock); @@ -728,8 +736,16 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) qxl->current_async = QXL_UNDEFINED_IO; qemu_mutex_unlock(&qxl->async_lock); - dprint(qxl, 2, "async_complete: %d (%" PRId64 ") done\n", - current_async, cookie); + dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie); + if (!cookie) { + fprintf(stderr, "qxl: %s: error, cookie is NULL\n", __func__); + return; + } + if (cookie && current_async != cookie->io) { + fprintf(stderr, + "qxl: %s: error: current_async = %d != %ld = cookie->io\n", + __func__, current_async, cookie->io); + } switch (current_async) { case QXL_IO_CREATE_PRIMARY_ASYNC: qxl_create_guest_primary_complete(qxl); @@ -738,12 +754,29 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) qxl_spice_destroy_surfaces_complete(qxl); break; case QXL_IO_DESTROY_SURFACE_ASYNC: - qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie); + qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id); break; } qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); } +/* called from spice server thread context only */ +static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token) +{ + PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); + QXLCookie *cookie = (QXLCookie *)cookie_token; + + switch (cookie->type) { + case QXL_COOKIE_TYPE_IO: + interface_async_complete_io(qxl, cookie); + break; + default: + fprintf(stderr, "qxl: %s: unexpected cookie type %d\n", + __func__, cookie->type); + } + g_free(cookie); +} + static const QXLInterface qxl_interface = { .base.type = SPICE_INTERFACE_QXL, .base.description = "qxl gpu", @@ -1054,9 +1087,7 @@ static int qxl_destroy_primary(PCIQXLDevice *d, qxl_async_io async) if (d->mode == QXL_MODE_UNDEFINED) { return 0; } - dprint(d, 1, "%s\n", __FUNCTION__); - d->mode = QXL_MODE_UNDEFINED; qemu_spice_destroy_primary_surface(&d->ssd, 0, async); qxl_spice_reset_cursor(d); @@ -1184,7 +1215,9 @@ async_common: { QXLRect update = d->ram->update_area; qxl_spice_update_area(d, d->ram->update_surface, - &update, NULL, 0, 0, async); + &update, NULL, 0, 0, async, + qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_UPDATE_AREA_ASYNC)); break; } case QXL_IO_NOTIFY_CMD: diff --git a/hw/qxl.h b/hw/qxl.h index 53a3acea37..1443925d0b 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -118,7 +118,7 @@ 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_async_io async); + qxl_async_io async, QXLCookie *cookie); void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext, uint32_t count); void qxl_spice_oom(PCIQXLDevice *qxl); diff --git a/ui/spice-display.c b/ui/spice-display.c index ad76bae82c..ab266aedc1 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -60,11 +60,23 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r) dest->right = MAX(dest->right, r->right); } +QXLCookie *qxl_cookie_new(int type, uint64_t io) +{ + QXLCookie *cookie; + + cookie = g_malloc0(sizeof(*cookie)); + cookie->type = type; + cookie->io = io; + return cookie; +} + void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, qxl_async_io async) { if (async != QXL_SYNC) { - spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0); + spice_qxl_add_memslot_async(&ssd->qxl, memslot, + (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_MEMSLOT_ADD_ASYNC)); } else { ssd->worker->add_memslot(ssd->worker, memslot); } @@ -80,7 +92,9 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id, qxl_async_io async) { if (async != QXL_SYNC) { - spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0); + spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, + (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_CREATE_PRIMARY_ASYNC)); } else { ssd->worker->create_primary_surface(ssd->worker, id, surface); } @@ -91,7 +105,9 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id, qxl_async_io async) { if (async != QXL_SYNC) { - spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0); + spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, + (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_DESTROY_PRIMARY_ASYNC)); } else { ssd->worker->destroy_primary_surface(ssd->worker, id); } diff --git a/ui/spice-display.h b/ui/spice-display.h index a23bfc8502..8a010cbe83 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -48,6 +48,20 @@ typedef enum qxl_async_io { QXL_ASYNC, } qxl_async_io; +enum { + QXL_COOKIE_TYPE_IO, +}; + +typedef struct QXLCookie { + int type; + uint64_t io; + union { + uint32_t surface_id; + } u; +} QXLCookie; + +QXLCookie *qxl_cookie_new(int type, uint64_t io); + typedef struct SimpleSpiceDisplay SimpleSpiceDisplay; typedef struct SimpleSpiceUpdate SimpleSpiceUpdate; From 81fb6f1504fb9ef71f2382f44af34756668296e8 Mon Sep 17 00:00:00 2001 From: Alon Levy Date: Fri, 24 Feb 2012 23:19:31 +0200 Subject: [PATCH 07/11] qxl: make qxl_render_update async RHBZ# 747011 Removes the last user of QXL_SYNC when using update drivers that use the _ASYNC io ports. The last user is qxl_render_update, it is called both by qxl_hw_update which is the vga_hw_update_ptr passed to graphic_console_init, and by qxl_hw_screen_dump. At the same time the QXLRect area being passed to the red_worker thread is passed as a copy, as part of the QXLCookie. The implementation uses interface_update_area_complete with a bh to make sure dpy_update and qxl_flip are called from the io thread, otherwise the vga->ds->surface.data can change under our feet. With this patch sdl+spice works fine. But spice by itself doesn't produce the expected screendumps unless repeated a few times, due to ppm_save being called before update_area (rendering done in spice server thread) having a chance to complete. Fixed by next patch, but see commit message for problem introduced by it. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 96 +++++++++++++++++++++++++++++++++------------- hw/qxl.c | 69 +++++++++++++++++++++++++++++++-- hw/qxl.h | 10 +++++ ui/spice-display.h | 6 +++ 4 files changed, 150 insertions(+), 31 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index c45dc30ab3..f323a4df68 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -82,17 +82,25 @@ void qxl_render_resize(PCIQXLDevice *qxl) } } -void qxl_render_update(PCIQXLDevice *qxl) +static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area) +{ + area->left = 0; + area->right = qxl->guest_primary.surface.width; + area->top = 0; + area->bottom = qxl->guest_primary.surface.height; +} + +static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) { VGACommonState *vga = &qxl->vga; - QXLRect dirty[32], update; - int i, redraw = 0; + int i; DisplaySurface *surface = vga->ds->surface; if (qxl->guest_primary.resized) { qxl->guest_primary.resized = 0; - qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram); + qxl_set_rect_to_surface(qxl, &qxl->dirty[0]); + qxl->num_dirty_rects = 1; dprint(qxl, 1, "%s: %dx%d, stride %d, bpp %d, depth %d\n", __FUNCTION__, qxl->guest_primary.surface.width, @@ -103,9 +111,9 @@ void qxl_render_update(PCIQXLDevice *qxl) } if (surface->width != qxl->guest_primary.surface.width || surface->height != qxl->guest_primary.surface.height) { - dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n", - __func__); if (qxl->guest_primary.qxl_stride > 0) { + dprint(qxl, 1, "%s: using guest_primary for displaysurface\n", + __func__); qemu_free_displaysurface(vga->ds); qemu_create_displaysurface_from(qxl->guest_primary.surface.width, qxl->guest_primary.surface.height, @@ -113,36 +121,70 @@ void qxl_render_update(PCIQXLDevice *qxl) qxl->guest_primary.abs_stride, qxl->guest_primary.data); } else { + dprint(qxl, 1, "%s: resizing displaysurface to guest_primary\n", + __func__); qemu_resize_displaysurface(vga->ds, qxl->guest_primary.surface.width, qxl->guest_primary.surface.height); } } - update.left = 0; - update.right = qxl->guest_primary.surface.width; - update.top = 0; - update.bottom = qxl->guest_primary.surface.height; - - memset(dirty, 0, sizeof(dirty)); - if (runstate_is_running() && qxl->guest_primary.commands) { - qxl->guest_primary.commands = 0; - qxl_spice_update_area(qxl, 0, &update, - dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL); - } - if (redraw) { - memset(dirty, 0, sizeof(dirty)); - dirty[0] = update; - } - for (i = 0; i < ARRAY_SIZE(dirty); i++) { - if (qemu_spice_rect_is_empty(dirty+i)) { + for (i = 0; i < qxl->num_dirty_rects; i++) { + if (qemu_spice_rect_is_empty(qxl->dirty+i)) { break; } - qxl_flip(qxl, dirty+i); + qxl_flip(qxl, qxl->dirty+i); dpy_update(vga->ds, - dirty[i].left, dirty[i].top, - dirty[i].right - dirty[i].left, - dirty[i].bottom - dirty[i].top); + qxl->dirty[i].left, qxl->dirty[i].top, + qxl->dirty[i].right - qxl->dirty[i].left, + qxl->dirty[i].bottom - qxl->dirty[i].top); } + qxl->num_dirty_rects = 0; +} + +/* + * use ssd.lock to protect render_update_cookie_num. + * qxl_render_update is called by io thread or vcpu thread, and the completion + * callbacks are called by spice_server thread, defering to bh called from the + * io thread. + */ +void qxl_render_update(PCIQXLDevice *qxl) +{ + QXLCookie *cookie; + + qemu_mutex_lock(&qxl->ssd.lock); + + if (!runstate_is_running() || !qxl->guest_primary.commands) { + qxl_render_update_area_unlocked(qxl); + qemu_mutex_unlock(&qxl->ssd.lock); + return; + } + + qxl->guest_primary.commands = 0; + qxl->render_update_cookie_num++; + qemu_mutex_unlock(&qxl->ssd.lock); + cookie = qxl_cookie_new(QXL_COOKIE_TYPE_RENDER_UPDATE_AREA, + 0); + qxl_set_rect_to_surface(qxl, &cookie->u.render.area); + qxl_spice_update_area(qxl, 0, &cookie->u.render.area, NULL, + 0, 1 /* clear_dirty_region */, QXL_ASYNC, cookie); +} + +void qxl_render_update_area_bh(void *opaque) +{ + PCIQXLDevice *qxl = opaque; + + qemu_mutex_lock(&qxl->ssd.lock); + qxl_render_update_area_unlocked(qxl); + qemu_mutex_unlock(&qxl->ssd.lock); +} + +void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie) +{ + qemu_mutex_lock(&qxl->ssd.lock); + qemu_bh_schedule(qxl->update_area_bh); + qxl->render_update_cookie_num--; + qemu_mutex_unlock(&qxl->ssd.lock); + g_free(cookie); } static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor) diff --git a/hw/qxl.c b/hw/qxl.c index 3bc0e5c762..d83d245685 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -747,6 +747,11 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie) __func__, current_async, cookie->io); } switch (current_async) { + case QXL_IO_MEMSLOT_ADD_ASYNC: + case QXL_IO_DESTROY_PRIMARY_ASYNC: + case QXL_IO_UPDATE_AREA_ASYNC: + case QXL_IO_FLUSH_SURFACES_ASYNC: + break; case QXL_IO_CREATE_PRIMARY_ASYNC: qxl_create_guest_primary_complete(qxl); break; @@ -756,10 +761,53 @@ static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie) case QXL_IO_DESTROY_SURFACE_ASYNC: qxl_spice_destroy_surface_wait_complete(qxl, cookie->u.surface_id); break; + default: + fprintf(stderr, "qxl: %s: unexpected current_async %d\n", __func__, + current_async); } qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); } +/* called from spice server thread context only */ +static void interface_update_area_complete(QXLInstance *sin, + uint32_t surface_id, + QXLRect *dirty, uint32_t num_updated_rects) +{ + PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); + int i; + int qxl_i; + + qemu_mutex_lock(&qxl->ssd.lock); + if (surface_id != 0 || !qxl->render_update_cookie_num) { + qemu_mutex_unlock(&qxl->ssd.lock); + return; + } + if (qxl->num_dirty_rects + num_updated_rects > QXL_NUM_DIRTY_RECTS) { + /* + * overflow - treat this as a full update. Not expected to be common. + */ + dprint(qxl, 1, "%s: overflow of dirty rects\n", __func__); + qxl->guest_primary.resized = 1; + } + if (qxl->guest_primary.resized) { + /* + * Don't bother copying or scheduling the bh since we will flip + * the whole area anyway on completion of the update_area async call + */ + qemu_mutex_unlock(&qxl->ssd.lock); + return; + } + qxl_i = qxl->num_dirty_rects; + for (i = 0; i < num_updated_rects; i++) { + qxl->dirty[qxl_i++] = dirty[i]; + } + qxl->num_dirty_rects += num_updated_rects; + dprint(qxl, 1, "%s: scheduling update_area_bh, #dirty %d\n", + __func__, qxl->num_dirty_rects); + qemu_bh_schedule(qxl->update_area_bh); + qemu_mutex_unlock(&qxl->ssd.lock); +} + /* called from spice server thread context only */ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token) { @@ -769,12 +817,16 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token) switch (cookie->type) { case QXL_COOKIE_TYPE_IO: interface_async_complete_io(qxl, cookie); + g_free(cookie); + break; + case QXL_COOKIE_TYPE_RENDER_UPDATE_AREA: + qxl_render_update_area_done(qxl, cookie); break; default: fprintf(stderr, "qxl: %s: unexpected cookie type %d\n", __func__, cookie->type); + g_free(cookie); } - g_free(cookie); } static const QXLInterface qxl_interface = { @@ -797,6 +849,7 @@ static const QXLInterface qxl_interface = { .notify_update = interface_notify_update, .flush_resources = interface_flush_resources, .async_complete = interface_async_complete, + .update_area_complete = interface_update_area_complete, }; static void qxl_enter_vga_mode(PCIQXLDevice *d) @@ -1213,11 +1266,17 @@ async_common: switch (io_port) { case QXL_IO_UPDATE_AREA: { + QXLCookie *cookie = NULL; QXLRect update = d->ram->update_area; + + if (async == QXL_ASYNC) { + cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_UPDATE_AREA_ASYNC); + cookie->u.area = update; + } qxl_spice_update_area(d, d->ram->update_surface, - &update, NULL, 0, 0, async, - qxl_cookie_new(QXL_COOKIE_TYPE_IO, - QXL_IO_UPDATE_AREA_ASYNC)); + cookie ? &cookie->u.area : &update, + NULL, 0, 0, async, cookie); break; } case QXL_IO_NOTIFY_CMD: @@ -1649,6 +1708,8 @@ static int qxl_init_common(PCIQXLDevice *qxl) init_pipe_signaling(qxl); qxl_reset_state(qxl); + qxl->update_area_bh = qemu_bh_new(qxl_render_update_area_bh, qxl); + return 0; } diff --git a/hw/qxl.h b/hw/qxl.h index 1443925d0b..86e415b5f4 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -18,6 +18,8 @@ enum qxl_mode { #define QXL_UNDEFINED_IO UINT32_MAX +#define QXL_NUM_DIRTY_RECTS 64 + typedef struct PCIQXLDevice { PCIDevice pci; SimpleSpiceDisplay ssd; @@ -93,6 +95,12 @@ typedef struct PCIQXLDevice { /* user-friendly properties (in megabytes) */ uint32_t ram_size_mb; uint32_t vram_size_mb; + + /* qxl_render_update state */ + int render_update_cookie_num; + int num_dirty_rects; + QXLRect dirty[QXL_NUM_DIRTY_RECTS]; + QEMUBH *update_area_bh; } PCIQXLDevice; #define PANIC_ON(x) if ((x)) { \ @@ -134,3 +142,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); void qxl_render_resize(PCIQXLDevice *qxl); void qxl_render_update(PCIQXLDevice *qxl); void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext); +void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie); +void qxl_render_update_area_bh(void *opaque); diff --git a/ui/spice-display.h b/ui/spice-display.h index 8a010cbe83..12e50b6efc 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -50,6 +50,7 @@ typedef enum qxl_async_io { enum { QXL_COOKIE_TYPE_IO, + QXL_COOKIE_TYPE_RENDER_UPDATE_AREA, }; typedef struct QXLCookie { @@ -57,6 +58,11 @@ typedef struct QXLCookie { uint64_t io; union { uint32_t surface_id; + QXLRect area; + struct { + QXLRect area; + int redraw; + } render; } u; } QXLCookie; From 6f2b175a090f367c3aab2226c4741b439671307a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 14 Oct 2011 18:05:48 +0200 Subject: [PATCH 08/11] qxl: add optinal 64bit vram bar This patch adds an 64bit pci bar for vram. It is turned off by default. It can be enabled by setting the size of the 64bit bar to be larger than the 32bit bar. Both 32bit and 64bit bar refer to the same memory. Only the first part of the memory is available via 32bit bar. The intention is to allow large vram sizes for 64bit guests, by allowing the vram bar being mapped above 4G, so we don't have to squeeze it into the pci I/O window below 4G. With vram_size_mb=16 and vram64_size_mb=256 it looks like this: 00:02.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 02) (prog-if 00 [VGA controller]) Subsystem: Red Hat, Inc Device 1100 Physical Slot: 2 Flags: fast devsel, IRQ 10 Memory at f8000000 (32-bit, non-prefetchable) [size=64M] Memory at fc000000 (32-bit, non-prefetchable) [size=16M] Memory at fd020000 (32-bit, non-prefetchable) [size=8K] I/O ports at c5a0 [size=32] Memory at ffe0000000 (64-bit, prefetchable) [size=256M] Expansion ROM at fd000000 [disabled] [size=64K] [ mapping above 4G needs patched seabios: http://www.kraxel.org/cgit/seabios/commit/?h=pci64 ] --- hw/qxl.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++--------- hw/qxl.h | 7 +++++++ 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index d83d245685..e17b0e31af 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -976,6 +976,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta, static const int regions[] = { QXL_RAM_RANGE_INDEX, QXL_VRAM_RANGE_INDEX, + QXL_VRAM64_RANGE_INDEX, }; uint64_t guest_start; uint64_t guest_end; @@ -1022,6 +1023,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta, virt_start = (intptr_t)memory_region_get_ram_ptr(&d->vga.vram); break; case QXL_VRAM_RANGE_INDEX: + case 4 /* vram 64bit */: virt_start = (intptr_t)memory_region_get_ram_ptr(&d->vram_bar); break; default: @@ -1622,18 +1624,28 @@ static void qxl_init_ramsize(PCIQXLDevice *qxl, uint32_t ram_min_mb) qxl->vga.vram_size = ram_min_mb * 1024 * 1024; } - /* vram (surfaces, bar 1) */ + /* vram32 (surfaces, 32bit, bar 1) */ + if (qxl->vram32_size_mb != -1) { + qxl->vram32_size = qxl->vram32_size_mb * 1024 * 1024; + } + if (qxl->vram32_size < 4096) { + qxl->vram32_size = 4096; + } + + /* vram (surfaces, 64bit, bar 4+5) */ if (qxl->vram_size_mb != -1) { qxl->vram_size = qxl->vram_size_mb * 1024 * 1024; } - if (qxl->vram_size < 4096) { - qxl->vram_size = 4096; - } - if (qxl->revision == 1) { - qxl->vram_size = 4096; + if (qxl->vram_size < qxl->vram32_size) { + qxl->vram_size = qxl->vram32_size; } + if (qxl->revision == 1) { + qxl->vram32_size = 4096; + qxl->vram_size = 4096; + } qxl->vga.vram_size = msb_mask(qxl->vga.vram_size * 2 - 1); + qxl->vram32_size = msb_mask(qxl->vram32_size * 2 - 1); qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1); } @@ -1675,6 +1687,8 @@ static int qxl_init_common(PCIQXLDevice *qxl) memory_region_init_ram(&qxl->vram_bar, "qxl.vram", qxl->vram_size); vmstate_register_ram(&qxl->vram_bar, &qxl->pci.qdev); + memory_region_init_alias(&qxl->vram32_bar, "qxl.vram32", &qxl->vram_bar, + 0, qxl->vram32_size); io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1); if (qxl->revision == 1) { @@ -1698,7 +1712,29 @@ static int qxl_init_common(PCIQXLDevice *qxl) PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vga.vram); pci_register_bar(&qxl->pci, QXL_VRAM_RANGE_INDEX, - PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vram_bar); + PCI_BASE_ADDRESS_SPACE_MEMORY, &qxl->vram32_bar); + + if (qxl->vram32_size < qxl->vram_size) { + /* + * Make the 64bit vram bar show up only in case it is + * configured to be larger than the 32bit vram bar. + */ + pci_register_bar(&qxl->pci, QXL_VRAM64_RANGE_INDEX, + PCI_BASE_ADDRESS_SPACE_MEMORY | + PCI_BASE_ADDRESS_MEM_TYPE_64 | + PCI_BASE_ADDRESS_MEM_PREFETCH, + &qxl->vram_bar); + } + + /* print pci bar details */ + dprint(qxl, 1, "ram/%s: %d MB [region 0]\n", + qxl->id == 0 ? "pri" : "sec", + qxl->vga.vram_size / (1024*1024)); + dprint(qxl, 1, "vram/32: %d MB [region 1]\n", + qxl->vram32_size / (1024*1024)); + dprint(qxl, 1, "vram/64: %d MB %s\n", + qxl->vram_size / (1024*1024), + qxl->vram32_size < qxl->vram_size ? "[region 4]" : "[unmapped]"); qxl->ssd.qxl.base.sif = &qxl_interface.base; qxl->ssd.qxl.id = qxl->id; @@ -1917,7 +1953,7 @@ static VMStateDescription qxl_vmstate = { static Property qxl_properties[] = { DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 * 1024), - DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, + DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram32_size, 64 * 1024 * 1024), DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, QXL_DEFAULT_REVISION), @@ -1925,7 +1961,8 @@ static Property qxl_properties[] = { DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0), DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0), DEFINE_PROP_UINT32("ram_size_mb", PCIQXLDevice, ram_size_mb, -1), - DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram_size_mb, -1), + DEFINE_PROP_UINT32("vram_size_mb", PCIQXLDevice, vram32_size_mb, 0), + DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/qxl.h b/hw/qxl.h index 86e415b5f4..11a0db3f7d 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -16,6 +16,10 @@ enum qxl_mode { QXL_MODE_NATIVE, }; +#ifndef QXL_VRAM64_RANGE_INDEX +#define QXL_VRAM64_RANGE_INDEX 4 +#endif + #define QXL_UNDEFINED_IO UINT32_MAX #define QXL_NUM_DIRTY_RECTS 64 @@ -88,6 +92,8 @@ typedef struct PCIQXLDevice { /* vram pci bar */ uint32_t vram_size; MemoryRegion vram_bar; + uint32_t vram32_size; + MemoryRegion vram32_bar; /* io bar */ MemoryRegion io_bar; @@ -95,6 +101,7 @@ typedef struct PCIQXLDevice { /* user-friendly properties (in megabytes) */ uint32_t ram_size_mb; uint32_t vram_size_mb; + uint32_t vram32_size_mb; /* qxl_render_update state */ int render_update_cookie_num; From 339a475f50ae0df8d2e222e47f1264811d5f6bee Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Fri, 24 Feb 2012 18:13:12 +0100 Subject: [PATCH 09/11] spice: use error_report to report errors Error message reporting during spice startup wasn't consistent, it was done with fprintf(stderr, "") but sometimes the message didn't have a trailing \n. Using error_report make the intent of the message clearer and deal with the final \n for us. Signed-off-by: Gerd Hoffmann --- ui/spice-core.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index 30d2c6264c..e7618139a5 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -227,8 +227,8 @@ static void channel_event(int event, SpiceChannelEventInfo *info) add_addr_info(server, (struct sockaddr *)&info->laddr_ext, info->llen_ext); } else { - fprintf(stderr, "spice: %s, extended address is expected\n", - __func__); + error_report("spice: %s, extended address is expected", + __func__); #endif add_addr_info(client, &info->paddr, info->plen); add_addr_info(server, &info->laddr, info->llen); @@ -333,7 +333,7 @@ static int parse_name(const char *string, const char *optname, if (value != -1) { return value; } - fprintf(stderr, "spice: invalid %s: %s\n", optname, string); + error_report("spice: invalid %s: %s", optname, string); exit(1); } @@ -525,7 +525,7 @@ static int add_channel(const char *name, const char *value, void *opaque) rc = spice_server_set_channel_security(spice_server, value, security); } if (rc != 0) { - fprintf(stderr, "spice: failed to set channel security for %s\n", value); + error_report("spice: failed to set channel security for %s", value); exit(1); } return 0; @@ -553,15 +553,15 @@ void qemu_spice_init(void) port = qemu_opt_get_number(opts, "port", 0); tls_port = qemu_opt_get_number(opts, "tls-port", 0); if (!port && !tls_port) { - fprintf(stderr, "neither port nor tls-port specified for spice."); + error_report("neither port nor tls-port specified for spice"); exit(1); } if (port < 0 || port > 65535) { - fprintf(stderr, "spice port is out of range"); + error_report("spice port is out of range"); exit(1); } if (tls_port < 0 || tls_port > 65535) { - fprintf(stderr, "spice tls-port is out of range"); + error_report("spice tls-port is out of range"); exit(1); } password = qemu_opt_get(opts, "password"); @@ -631,11 +631,11 @@ void qemu_spice_init(void) #if SPICE_SERVER_VERSION >= 0x000900 /* 0.9.0 */ if (spice_server_set_sasl_appname(spice_server, "qemu") == -1 || spice_server_set_sasl(spice_server, 1) == -1) { - fprintf(stderr, "spice: failed to enable sasl\n"); + error_report("spice: failed to enable sasl"); exit(1); } #else - fprintf(stderr, "spice: sasl is not available (spice >= 0.9 required)\n"); + error_report("spice: sasl is not available (spice >= 0.9 required)"); exit(1); #endif } @@ -683,7 +683,7 @@ void qemu_spice_init(void) qemu_opt_foreach(opts, add_channel, NULL, 0); if (0 != spice_server_init(spice_server, &core_interface)) { - fprintf(stderr, "failed to initialize spice server"); + error_report("failed to initialize spice server"); exit(1); }; using_spice = 1; @@ -708,7 +708,7 @@ int qemu_spice_add_interface(SpiceBaseInstance *sin) { if (!spice_server) { if (QTAILQ_FIRST(&qemu_spice_opts.head) != NULL) { - fprintf(stderr, "Oops: spice configured but not active\n"); + error_report("Oops: spice configured but not active"); exit(1); } /* From 35c633291439185d3bbf84da23db1572498e0d5d Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Fri, 24 Feb 2012 18:28:32 +0100 Subject: [PATCH 10/11] Error out when tls-channel option is used without TLS It's currently possible to setup spice channels using TLS when no TLS port has been specified (ie TLS is disabled). This cannot work, so better to error out in such a situation. Signed-off-by: Gerd Hoffmann --- ui/spice-core.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ui/spice-core.c b/ui/spice-core.c index e7618139a5..c1091e1602 100644 --- a/ui/spice-core.c +++ b/ui/spice-core.c @@ -511,6 +511,12 @@ static int add_channel(const char *name, const char *value, void *opaque) int rc; if (strcmp(name, "tls-channel") == 0) { + int *tls_port = opaque; + if (!*tls_port) { + error_report("spice: tried to setup tls-channel" + " without specifying a TLS port"); + exit(1); + } security = SPICE_CHANNEL_SECURITY_SSL; } if (strcmp(name, "plaintext-channel") == 0) { @@ -680,7 +686,7 @@ void qemu_spice_init(void) spice_server_set_playback_compression (spice_server, qemu_opt_get_bool(opts, "playback-compression", 1)); - qemu_opt_foreach(opts, add_channel, NULL, 0); + qemu_opt_foreach(opts, add_channel, &tls_port, 0); if (0 != spice_server_init(spice_server, &core_interface)) { error_report("failed to initialize spice server"); From e2efc0a3267811a2b9459116b2310325ef011f6e Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 27 Feb 2012 11:05:09 +0100 Subject: [PATCH 11/11] qxl: properly handle upright and non-shared surfaces Although qxl creates a shared displaysurface when the qxl surface is upright and doesn't need to be flipped there is no guarantee that the surface doesn't become unshared for some reason. Rename qxl_flip to qxl_blit and fix it to handle both flip and non-flip cases. Signed-off-by: Gerd Hoffmann --- hw/qxl-render.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index f323a4df68..25857f6a20 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -21,25 +21,31 @@ #include "qxl.h" -static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect) +static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect) { uint8_t *src; uint8_t *dst = qxl->vga.ds->surface->data; int len, i; - if (qxl->guest_primary.qxl_stride > 0) { + if (is_buffer_shared(qxl->vga.ds->surface)) { return; } if (!qxl->guest_primary.data) { dprint(qxl, 1, "%s: initializing guest_primary.data\n", __func__); qxl->guest_primary.data = memory_region_get_ram_ptr(&qxl->vga.vram); } - dprint(qxl, 1, "%s: stride %d, [%d, %d, %d, %d]\n", __func__, + dprint(qxl, 2, "%s: stride %d, [%d, %d, %d, %d]\n", __func__, qxl->guest_primary.qxl_stride, rect->left, rect->right, rect->top, rect->bottom); src = qxl->guest_primary.data; - src += (qxl->guest_primary.surface.height - rect->top - 1) * - qxl->guest_primary.abs_stride; + if (qxl->guest_primary.qxl_stride < 0) { + /* qxl surface is upside down, walk src scanlines + * in reverse order to flip it */ + src += (qxl->guest_primary.surface.height - rect->top - 1) * + qxl->guest_primary.abs_stride; + } else { + src += rect->top * qxl->guest_primary.abs_stride; + } dst += rect->top * qxl->guest_primary.abs_stride; src += rect->left * qxl->guest_primary.bytes_pp; dst += rect->left * qxl->guest_primary.bytes_pp; @@ -48,7 +54,7 @@ static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect) for (i = rect->top; i < rect->bottom; i++) { memcpy(dst, src, len); dst += qxl->guest_primary.abs_stride; - src -= qxl->guest_primary.abs_stride; + src += qxl->guest_primary.qxl_stride; } } @@ -132,7 +138,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) if (qemu_spice_rect_is_empty(qxl->dirty+i)) { break; } - qxl_flip(qxl, qxl->dirty+i); + qxl_blit(qxl, qxl->dirty+i); dpy_update(vga->ds, qxl->dirty[i].left, qxl->dirty[i].top, qxl->dirty[i].right - qxl->dirty[i].left,