From patchwork Mon Oct 24 12:02:19 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alon Levy X-Patchwork-Id: 121339 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 28DF21007D1 for ; Mon, 24 Oct 2011 23:43:59 +1100 (EST) Received: from localhost ([::1]:58682 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIJHt-0001Kw-NW for incoming@patchwork.ozlabs.org; Mon, 24 Oct 2011 08:05:53 -0400 Received: from eggs.gnu.org ([140.186.70.92]:53801) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIJHM-0000YJ-JI for qemu-devel@nongnu.org; Mon, 24 Oct 2011 08:05:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIJHJ-0002AD-Pj for qemu-devel@nongnu.org; Mon, 24 Oct 2011 08:05:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14128) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIJHJ-00029t-4D for qemu-devel@nongnu.org; Mon, 24 Oct 2011 08:05:17 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9OC5GsB000624 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 24 Oct 2011 08:05:16 -0400 Received: from bow.tlv.redhat.com (dhcp-3-73.tlv.redhat.com [10.35.3.73]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p9OC52vi027291; Mon, 24 Oct 2011 08:05:14 -0400 From: Alon Levy To: lcapitulino@redhat.com, armbru@redhat.com, kraxel@redhat.com Date: Mon, 24 Oct 2011 14:02:19 +0200 Message-Id: <1319457739-14562-6-git-send-email-alevy@redhat.com> In-Reply-To: <1319457739-14562-1-git-send-email-alevy@redhat.com> References: <1319457739-14562-1-git-send-email-alevy@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.25 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 209.132.183.28 Cc: mlureau@redhat.com, qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 5/5] qxl: support async monitor screen dump X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Split qxl_spice_update_area_complete from qxl_render_update, use SPICE_INTERFACE_QXL_MINOR 2 introduced spice_qxl_update_area_dirty_async to retrive the dirty rectangles asyncronously (the previous spice_qxl_update_area_async did not accept a dirty rectangles array). Introduce SpiceAsyncMonitorScreenDump for a screen_dump. vga mode screen dumps are still synchronous. Signed-off-by: Alon Levy --- patchcheck gives one false positive, identifying a point function argument as a multiplication: ERROR: need consistent spacing around '*' (ctx:WxV) #135: FILE: hw/qxl.c:148: + SpiceAsyncCommand *async_command) ^ --- hw/qxl-render.c | 67 +++++++++++++++++++++++++++++--------- hw/qxl.c | 95 +++++++++++++++++++++++++++++++++++++++++++----------- hw/qxl.h | 38 ++++++++++++++++++++-- 3 files changed, 161 insertions(+), 39 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index fd3c016..7dfd67f 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -27,6 +27,10 @@ static void qxl_flip(PCIQXLDevice *qxl, QXLRect *rect) uint8_t *dst = qxl->guest_primary.flipped; int len, i; + assert(rect->bottom >= 0); + assert(rect->bottom <= qxl->guest_primary.surface.height); + assert(rect->top >= 0); + assert(rect->top <= qxl->guest_primary.surface.height); src += (qxl->guest_primary.surface.height - rect->top - 1) * qxl->guest_primary.stride; dst += rect->top * qxl->guest_primary.stride; @@ -110,17 +114,39 @@ static void qxl_render_display_resized(PCIQXLDevice *qxl) dpy_resize(vga->ds); } -void qxl_render_update(PCIQXLDevice *qxl) +void qxl_spice_update_area_complete(PCIQXLDevice *qxl, QXLRect *dirty, + int n_dirty) { VGACommonState *vga = &qxl->vga; - QXLRect dirty[32], update; int i; + for (i = 0; i < n_dirty; i++) { + if (qemu_spice_rect_is_empty(dirty + i)) { + break; + } + if (qxl->guest_primary.flipped) { + qxl_flip(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); + } +} + +void qxl_render_update(PCIQXLDevice *qxl, + SpiceAsyncMonitorScreenDump *async_screen_dump) +{ + QXLRect dirty[32], update; + if (qxl->guest_primary.resized) { qxl_render_display_resized(qxl); } - if (!qxl->guest_primary.commands) { + if (async_screen_dump) { + dprint(qxl, 2, "%s: no update required\n", __func__); + complete_spice_async_command(qxl, &async_screen_dump->base); + } return; } qxl->guest_primary.commands = 0; @@ -131,20 +157,29 @@ void qxl_render_update(PCIQXLDevice *qxl) update.bottom = qxl->guest_primary.surface.height; memset(dirty, 0, sizeof(dirty)); + if (async_screen_dump) { +#if SPICE_INTERFACE_QXL_MINOR >= 2 + dprint(qxl, 2, "A-SYNC update_area 0 (%d,%d,%d,%d)\n", + update.top, update.left, update.bottom, update.right); + async_screen_dump->n_dirty = 32; + async_screen_dump->dirty = g_malloc0(sizeof(QXLRect) * + async_screen_dump->n_dirty); + qxl_spice_update_area(qxl, 0, &update, + async_screen_dump->dirty, + async_screen_dump->n_dirty, 1, + &async_screen_dump->base); + return; +#else + fprintf(stderr, "%s: warning: fallback to sync, spice-server < 0.8.4\n", + __func__); +#endif + } + dprint(qxl, 2, "SYNC update_area\n"); qxl_spice_update_area(qxl, 0, &update, - dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, 0); - - 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); - } - dpy_update(vga->ds, - dirty[i].left, dirty[i].top, - dirty[i].right - dirty[i].left, - dirty[i].bottom - dirty[i].top); + dirty, ARRAY_SIZE(dirty), 1, NULL); + qxl_spice_update_area_complete(qxl, dirty, ARRAY_SIZE(dirty)); + if (async_screen_dump) { + complete_spice_async_command(qxl, &async_screen_dump->base); } } diff --git a/hw/qxl.c b/hw/qxl.c index 008f5f7..a3e89cd 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -145,18 +145,32 @@ 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, uint64_t cookie) + SpiceAsyncCommand *async_command) { - if (async == QXL_SYNC) { + if (async_command == NULL) { 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 >= 2 + if (num_dirty_rects > 0) { + spice_qxl_update_area_dirty_async(&qxl->ssd.qxl, surface_id, area, + dirty_rects, num_dirty_rects, + clear_dirty_region, + async_command->cookie); + } else { + spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area, + clear_dirty_region, + async_command->cookie); + } +#else #if SPICE_INTERFACE_QXL_MINOR >= 1 spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area, - clear_dirty_region, cookie); + clear_dirty_region, + async_command->cookie); #else abort(); -#endif +#endif /* >= 1 */ +#endif /* >= 2 */ } } @@ -742,9 +756,11 @@ SpiceAsyncCommand *push_spice_async_command(PCIQXLDevice *qxl, assert(size >= sizeof(SpiceAsyncCommand)); SpiceAsyncCommand *spice_async_command = g_malloc0(size); + qemu_mutex_lock(&qxl->async_lock); spice_async_command->cookie = qxl->async_next_cookie++; - spice_async_command->async_io = async_io; + spice_async_command->io = async_io; QLIST_INSERT_HEAD(&qxl->async_commands, spice_async_command, next); + qemu_mutex_unlock(&qxl->async_lock); dprint(qxl, 2, "allocated async cookie %"PRId64"\n", qxl->async_next_cookie - 1); return spice_async_command; @@ -781,7 +797,7 @@ void complete_spice_async_command(PCIQXLDevice *qxl, static void interface_async_complete(QXLInstance *sin, uint64_t cookie) { PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); - uint32_t current_async; + uint32_t io; SpiceAsyncCommand *spice_async_command; spice_async_command = pop_spice_async_command(qxl, cookie); @@ -790,9 +806,9 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) "pending operation, cookie=%ld\n", cookie); return; } - current_async = spice_async_command->async_io; - dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie); - switch (current_async) { + io = spice_async_command->io; + dprint(qxl, 2, "async_complete: %d (%ld) done\n", io, cookie); + switch (io) { case QXL_IO_CREATE_PRIMARY_ASYNC: qxl_create_guest_primary_complete(qxl); break; @@ -806,7 +822,11 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie) } complete_spice_async_command(qxl, spice_async_command); g_free(spice_async_command); - qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); + if (io <= QXL_IO_RANGE_SIZE) { + dprint(qxl, 2, "%s: calling qxl_send_events (cookie %"PRIu64")\n", + __func__, cookie); + qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD); + } } #endif @@ -1162,17 +1182,27 @@ static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm) qxl_rom_set_dirty(d); } +typedef SpiceAsyncCommand *(*SpiceAsyncCommandCreator)(PCIQXLDevice *qxl, + uint32_t io, uint32_t val); + +static SpiceAsyncCommand *create_async_command(PCIQXLDevice *qxl, uint32_t io, + uint32_t val) +{ + return push_spice_async_command(qxl, io, sizeof(SpiceAsyncCommand)); +} + static void ioport_write(void *opaque, target_phys_addr_t addr, uint64_t val, unsigned size) { PCIQXLDevice *d = opaque; uint32_t io_port = addr; qxl_async_io async = QXL_SYNC; + SpiceAsyncCommand *spice_async_command = NULL; #if SPICE_INTERFACE_QXL_MINOR >= 1 uint32_t orig_io_port = io_port; - SpiceAsyncCommand *spice_async_command = NULL; #endif uint64_t cookie = 0; + SpiceAsyncCommandCreator creator = create_async_command; switch (io_port) { case QXL_IO_RESET: @@ -1228,10 +1258,7 @@ static void ioport_write(void *opaque, target_phys_addr_t addr, case QXL_IO_FLUSH_SURFACES_ASYNC: async_common: async = QXL_ASYNC; - qemu_mutex_lock(&d->async_lock); - spice_async_command = push_spice_async_command(d, orig_io_port, - sizeof(SpiceAsyncCommand)); - qemu_mutex_unlock(&d->async_lock); + spice_async_command = creator(d, orig_io_port, val); cookie = spice_async_command->cookie; dprint(d, 2, "start async %d (%"PRId64") cookie %"PRId64"\n", io_port, val, cookie); @@ -1246,7 +1273,7 @@ async_common: { QXLRect update = d->ram->update_area; qxl_spice_update_area(d, d->ram->update_surface, - &update, NULL, 0, 0, async, cookie); + &update, NULL, 0, 0, spice_async_command); break; } case QXL_IO_NOTIFY_CMD: @@ -1457,7 +1484,7 @@ static void qxl_hw_update(void *opaque) break; case QXL_MODE_COMPAT: case QXL_MODE_NATIVE: - qxl_render_update(qxl); + qxl_render_update(qxl, NULL); break; default: break; @@ -1472,6 +1499,36 @@ static void qxl_hw_invalidate(void *opaque) vga->invalidate(vga); } +static void screen_dump_complete(PCIQXLDevice *qxl, + SpiceAsyncCommand *spice_async_command) +{ + SpiceAsyncMonitorScreenDump *async_screen_dump = + (SpiceAsyncMonitorScreenDump *)spice_async_command; + + dprint(qxl, 2, "%s: calling screen_dump_cb >%s<\n", __func__, + async_screen_dump->filename); + qxl_spice_update_area_complete(qxl, async_screen_dump->dirty, + async_screen_dump->n_dirty); + g_free(async_screen_dump->dirty); + ppm_save(async_screen_dump->filename, qxl->ssd.ds->surface); + g_free((void *)async_screen_dump->filename); + async_screen_dump->cb(async_screen_dump->cb_opaque, NULL); +} + +static SpiceAsyncMonitorScreenDump *push_screen_dump(PCIQXLDevice *qxl, + const char *filename, MonitorCompletion *cb, void *cb_opaque) +{ + SpiceAsyncMonitorScreenDump *async_command = + (SpiceAsyncMonitorScreenDump *) + push_spice_async_command(qxl, MONITOR_UPDATE_AREA_ASYNC, + sizeof(SpiceAsyncMonitorScreenDump)); + async_command->cb = cb; + async_command->cb_opaque = cb_opaque; + async_command->filename = g_strdup(filename); + async_command->base.completion = screen_dump_complete; + return async_command; +} + static void qxl_hw_screen_dump(void *opaque, const char *filename, MonitorCompletion *cb, void *cb_opaque) { @@ -1481,9 +1538,7 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename, switch (qxl->mode) { case QXL_MODE_COMPAT: case QXL_MODE_NATIVE: - qxl_render_update(qxl); - ppm_save(filename, qxl->ssd.ds->surface); - cb(cb_opaque, NULL); + qxl_render_update(qxl, push_screen_dump(qxl, filename, cb, cb_opaque)); break; case QXL_MODE_VGA: vga->screen_dump(vga, filename, cb, cb_opaque); diff --git a/hw/qxl.h b/hw/qxl.h index 4c89e14..2375c03 100644 --- a/hw/qxl.h +++ b/hw/qxl.h @@ -17,6 +17,26 @@ enum qxl_mode { #define QXL_UNDEFINED_IO UINT32_MAX +/* The monitor's screen_dump triggers a update_area + * command which must be done asynchroniously to prevent the following + * deadlock scenario when the spice client is also a monitor client and + * is running single threaded: + * + * io thread worker thread client + * + * <---------------------------------------------------- screendump + * do_screen_dump-> read-------> + * flush, read + * client socket-------------> + * + * To avoid adding these to the spice-protocol/spice/qxl_dev.h, they + * are defined here. */ +enum { + /* Must start larger then QXL_IO_RANGE_SIZE, but that is a moving + * target, so split in half. */ + MONITOR_UPDATE_AREA_ASYNC = 0x80000000, +}; + typedef struct PCIQXLDevice PCIQXLDevice; typedef struct SpiceAsyncCommand SpiceAsyncCommand; typedef void (*SpiceAsyncCommandCompletion)(PCIQXLDevice *, @@ -27,11 +47,20 @@ typedef void (*SpiceAsyncCommandCompletion)(PCIQXLDevice *, struct SpiceAsyncCommand { uint64_t cookie; void *opaque; - uint32_t async_io; + uint32_t io; SpiceAsyncCommandCompletion completion; QLIST_ENTRY(SpiceAsyncCommand) next; }; +typedef struct SpiceAsyncMonitorScreenDump { + SpiceAsyncCommand base; + MonitorCompletion *cb; + void *cb_opaque; + const char *filename; + QXLRect *dirty; + int n_dirty; +} SpiceAsyncMonitorScreenDump; + struct PCIQXLDevice { PCIDevice pci; SimpleSpiceDisplay ssd; @@ -132,7 +161,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, uint64_t cookie); + SpiceAsyncCommand *async_command); void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext, uint32_t count); void qxl_spice_oom(PCIQXLDevice *qxl); @@ -151,11 +180,14 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext); /* qxl-render.c */ void qxl_render_resize(PCIQXLDevice *qxl); -void qxl_render_update(PCIQXLDevice *qxl); +void qxl_render_update(PCIQXLDevice *qxl, + SpiceAsyncMonitorScreenDump *async_screen_dump); 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); +void qxl_spice_update_area_complete(PCIQXLDevice *qxl, QXLRect *dirty, + int n_dirty); #endif