From patchwork Wed Mar 16 12:48:32 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alon Levy X-Patchwork-Id: 87248 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 1DC86B6FA8 for ; Thu, 17 Mar 2011 00:07:05 +1100 (EST) Received: from localhost ([127.0.0.1]:49895 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzqRJ-0000cc-Jp for incoming@patchwork.ozlabs.org; Wed, 16 Mar 2011 09:07:01 -0400 Received: from [140.186.70.92] (port=42901 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PzqOI-0007B1-9q for qemu-devel@nongnu.org; Wed, 16 Mar 2011 09:03:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pzq9v-0006zH-Hx for qemu-devel@nongnu.org; Wed, 16 Mar 2011 08:49:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23299) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pzq9v-0006zD-80 for qemu-devel@nongnu.org; Wed, 16 Mar 2011 08:49:03 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p2GCn2wj018281 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 16 Mar 2011 08:49:02 -0400 Received: from playa.tlv.redhat.com (dhcp-3-110.tlv.redhat.com [10.35.3.110]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id p2GCmusi026509; Wed, 16 Mar 2011 08:49:01 -0400 From: Alon Levy To: qemu-devel@nongnu.org Date: Wed, 16 Mar 2011 14:48:32 +0200 Message-Id: <1300279714-24721-3-git-send-email-alevy@redhat.com> In-Reply-To: <1300279714-24721-1-git-send-email-alevy@redhat.com> References: <1300279714-24721-1-git-send-email-alevy@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: hdegoede@redhat.com, uril@redhat.com, gleb@redhat.com Subject: [Qemu-devel] [PATCH v2 2/4] qxl: implement get_command in vga mode without locks X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Uri Lublin This patch and the next drop the requirement to lose the global qemu mutex during dispatcher calls. This patch enables it, the next drops the unlock/lock pairs around dispatcher calls. The current solution of dropping the locks is buggy: * it allows multiple dispatcher calls from two vcpu threads, the dispatcher doesn't handle that by design (single fd, not locked, can't handle writes from two threads) * it requires us to keep track of cpu_single_env, which is magic. The solution implemented in this patch and the next (the next just drops the locks, this patch allows that to work): * the only operation that needed locking was qemu_create_simple_update, * it required locking because it was called from the spice-server thread. * do it in the iothread by reusing the existing pipe used for set_irq. The current flow implemented is now: spice-server thread: qxl.c:interface_get_command (called either by polling or from wakeup) if update!=NULL: waiting_for_update=0 update=NULL return update else: if not waiting_for_update: waiting_for_update=1 write to pipe, which is read by iothread (main thread) iothread: wakeup from select, qxl.c:pipe_read update=qemu_create_simple_update() wakeup spice-server thread by calling d.worker->wakeup(d.worker) --- hw/qxl.c | 114 ++++++++++++++++++++++++++++++++++++++++++++-------- ui/spice-display.c | 8 +--- ui/spice-display.h | 9 ++++ 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index f1ee0ae..2419236 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -28,6 +28,8 @@ #include "qxl.h" +#define EMPTY_UPDATE ((void *)-1) + #undef SPICE_RING_PROD_ITEM #define SPICE_RING_PROD_ITEM(r, ret) { \ typeof(r) start = r; \ @@ -117,6 +119,15 @@ static QXLMode qxl_modes[] = { #endif }; + +/* + * commands/requests from server thread to iothread. + */ +enum { + QXL_SERVER_SET_IRQ = 1, + QXL_SERVER_CREATE_UPDATE, +}; + static PCIQXLDevice *qxl0; static void qxl_send_events(PCIQXLDevice *d, uint32_t events); @@ -336,11 +347,36 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info) info->n_surfaces = NUM_SURFACES; } +int qxl_vga_mode_get_command( + SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext) +{ + SimpleSpiceUpdate *update; + unsigned char req; + int r; + + update = ssd->update; + if (update != NULL) { + ssd->waiting_for_update = 0; + ssd->update = NULL; + if (update != EMPTY_UPDATE) { + *ext = update->ext; + return true; + } + } else { + if (!ssd->waiting_for_update) { + ssd->waiting_for_update = 1; + req = QXL_SERVER_CREATE_UPDATE; + r = write(ssd->pipe[1], &req , 1); + assert(r == 1); + } + } + return false; +} + /* called from spice server thread context only */ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) { PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); - SimpleSpiceUpdate *update; QXLCommandRing *ring; QXLCommand *cmd; int notify; @@ -348,16 +384,25 @@ static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) switch (qxl->mode) { case QXL_MODE_VGA: dprint(qxl, 2, "%s: vga\n", __FUNCTION__); - update = qemu_spice_create_update(&qxl->ssd); - if (update == NULL) { - return false; + if (qxl_vga_mode_get_command(&qxl->ssd, ext)) { + qxl_log_command(qxl, "vga", ext); + return true; } - *ext = update->ext; - qxl_log_command(qxl, "vga", ext); - return true; + return false; case QXL_MODE_COMPAT: case QXL_MODE_NATIVE: case QXL_MODE_UNDEFINED: + /* flush any existing updates that we didn't send to the guest. + * since update != NULL it means the server didn't get it, and + * because we changed mode to != QXL_MODE_VGA, it won't. */ + if (qxl->ssd.update != NULL) { + if (qxl->ssd.update != EMPTY_UPDATE) { + qemu_spice_destroy_update(&qxl->ssd, qxl->ssd.update); + } + qxl->ssd.update = NULL; + qxl->ssd.waiting_for_update = 0; + } + /* */ dprint(qxl, 2, "%s: %s\n", __FUNCTION__, qxl->cmdflags ? "compat" : "native"); ring = &qxl->ram->cmd_ring; @@ -1057,17 +1102,50 @@ static void qxl_map(PCIDevice *pci, int region_num, static void pipe_read(void *opaque) { - PCIQXLDevice *d = opaque; - char dummy; - int len; + SimpleSpiceDisplay *ssd = opaque; + unsigned char cmd; + int len, set_irq = 0; + int create_update = 0; do { - len = read(d->ssd.pipe[0], &dummy, sizeof(dummy)); - } while (len == sizeof(dummy)); - qxl_set_irq(d); + cmd = 0; + len = read(ssd->pipe[0], &cmd, sizeof(cmd)); + if (len < 0) { + if (errno == EINTR) { + continue; + } + if (errno == EAGAIN) { + break; + } + perror("qxl: pipe_read: read failed"); + break; + } + switch (cmd) { + case QXL_SERVER_SET_IRQ: + set_irq = 1; + break; + case QXL_SERVER_CREATE_UPDATE: + create_update = 1; + break; + default: + fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd); + abort(); + } + } while (1); + /* no need to do either operation more than once */ + if (create_update) { + assert(ssd->update == NULL); + ssd->update = qemu_spice_create_update(ssd); + if (ssd->update == NULL) { + ssd->update = EMPTY_UPDATE; + } + ssd->worker->wakeup(ssd->worker); + } + if (set_irq) { + qxl_set_irq(container_of(ssd, PCIQXLDevice, ssd)); + } } -/* called from spice server thread context only */ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) { uint32_t old_pending; @@ -1082,9 +1160,11 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) /* running in io_thread thread */ qxl_set_irq(d); } else { - if (write(d->ssd.pipe[1], d, 1) != 1) { - dprint(d, 1, "%s: write to pipe failed\n", __FUNCTION__); - } + /* called from spice-server thread */ + int ret; + unsigned char ack = QXL_SERVER_SET_IRQ; + ret = write(d->ssd.pipe[1], &ack, 1); + assert(ret == 1); } } diff --git a/ui/spice-display.c b/ui/spice-display.c index d60e7a9..a9ecee0 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -297,15 +297,9 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info) static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext) { SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl); - SimpleSpiceUpdate *update; dprint(3, "%s:\n", __FUNCTION__); - update = qemu_spice_create_update(ssd); - if (update == NULL) { - return false; - } - *ext = update->ext; - return true; + return qxl_vga_mode_get_command(ssd, ext); } static int interface_req_cmd_notification(QXLInstance *sin) diff --git a/ui/spice-display.h b/ui/spice-display.h index de584d9..210b0b5 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -31,6 +31,8 @@ #define NUM_SURFACES 1024 +struct SimpleSpiceUpdate; + typedef struct SimpleSpiceDisplay { DisplayState *ds; void *buf; @@ -48,6 +50,10 @@ typedef struct SimpleSpiceDisplay { * and in native mode) and without qxl */ pthread_t main; int pipe[2]; /* to iothread */ + + /* ssd updates (one request/command at a time) */ + struct SimpleSpiceUpdate *update; + int waiting_for_update; } SimpleSpiceDisplay; typedef struct SimpleSpiceUpdate { @@ -71,5 +77,8 @@ 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); +/* shared with qxl.c in vga mode and ui/spice-display (no qxl mode) */ +int qxl_vga_mode_get_command( + SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext); /* used by both qxl and spice-display */ void qxl_create_server_to_iothread_pipe(SimpleSpiceDisplay *ssd);