qxl: implement get_command in vga mode without locks

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)

3 files changed, 146 insertions, 26 deletions

diff --git a/hw/qxl.c b/hw/qxl.c
index 201698fad..64580f136 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -340,7 +340,6 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
 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 +347,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 != QXL_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 +1065,50 @@ static void qxl_map(PCIDevice *pci, int region_num,
 
 static void pipe_read(void *opaque)
 {
-    PCIQXLDevice *d = opaque;
-    char dummy;
-    int len;
-
-    do {
-        len = read(d->ssd.pipe[0], &dummy, sizeof(dummy));
-    } while (len == sizeof(dummy));
-    qxl_set_irq(d);
+    SimpleSpiceDisplay *ssd = opaque;
+    unsigned char cmd;
+    int len, set_irq = 0;
+    int create_update = 0;
+
+    while (1) {
+        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();
+        }
+    }
+    /* 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 = QXL_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 +1123,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 ec6e0cbed..bdd14b8cd 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -294,18 +294,39 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
     info->n_surfaces = NUM_SURFACES;
 }
 
+/* Called from spice server thread context (via interface_get_command) */
+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 != QXL_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;
+}
+
 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)
@@ -394,6 +415,45 @@ static DisplayChangeListener display_listener = {
     .dpy_refresh = display_refresh,
 };
 
+static void pipe_read(void *opaque)
+{
+    SimpleSpiceDisplay *ssd = opaque;
+    unsigned char cmd;
+    int len, create_update = 0;
+
+    while (1) {
+        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_CREATE_UPDATE:
+            create_update = 1;
+            break;
+        default:
+            fprintf(stderr, "%s: unknown cmd %u\n", __FUNCTION__, cmd);
+            abort();
+        }
+    }
+    /* no need to do this more than once */
+    if (create_update) {
+        assert(ssd->update == NULL);
+        ssd->update = qemu_spice_create_update(ssd);
+        if (ssd->update == NULL) {
+            ssd->update = QXL_EMPTY_UPDATE;
+        }
+        ssd->worker->wakeup(ssd->worker);
+    }
+}
+
 void qxl_create_server_to_iothread_pipe(SimpleSpiceDisplay *ssd,
     IOHandler *pipe_read)
 {
@@ -427,6 +487,7 @@ void qemu_spice_display_init(DisplayState *ds)
     qemu_spice_add_interface(&sdpy.qxl.base);
     assert(sdpy.worker);
 
+    qxl_create_server_to_iothread_pipe(&sdpy, pipe_read);
     qemu_add_vm_change_state_handler(qemu_spice_vm_change_state_handler, &sdpy);
     qemu_spice_create_host_memslot(&sdpy);
     qemu_spice_create_host_primary(&sdpy);
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 3e6cf7c52..2be6a345a 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -31,6 +31,15 @@
 
 #define NUM_SURFACES 1024
 
+/* For commands/requests from server thread to iothread */
+#define QXL_EMPTY_UPDATE ((void *)-1)
+enum {
+    QXL_SERVER_SET_IRQ = 1,
+    QXL_SERVER_CREATE_UPDATE,
+};
+
+struct SimpleSpiceUpdate;
+
 typedef struct SimpleSpiceDisplay {
     DisplayState *ds;
     void *buf;
@@ -48,6 +57,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,6 +84,9 @@ 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,
     IOHandler *pipe_read);