From: Uri Lublin<address@hidden>
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 | 118 +++++++++++++++++++++++++++++++++++++++++++--------
ui/spice-display.c | 10 +----
ui/spice-display.h | 8 +++-
3 files changed, 108 insertions(+), 28 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index d1d3c9c..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,13 +1160,15 @@ 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);
}
}
-void qxl_create_vcpu_to_iothread_pipe(SimpleSpiceDisplay *ssd)
+void qxl_create_server_to_iothread_pipe(SimpleSpiceDisplay *ssd)
{
if (pipe(ssd->pipe)< 0) {
fprintf(stderr, "%s: pipe creation failed\n", __FUNCTION__);
@@ -1110,7 +1190,7 @@ void qxl_create_vcpu_to_iothread_pipe(SimpleSpiceDisplay
*ssd)
static void init_pipe_signaling(PCIQXLDevice *d)
{
- qxl_create_vcpu_to_iothread_pipe(&d->ssd);
+ qxl_create_server_to_iothread_pipe(&d->ssd);
}
/* graphics console */
diff --git a/ui/spice-display.c b/ui/spice-display.c
index e0ac616..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)
@@ -406,7 +400,7 @@ void qemu_spice_display_init(DisplayState *ds)
qemu_spice_add_interface(&sdpy.qxl.base);
assert(sdpy.worker);
- qxl_create_vcpu_to_iothread_pipe(&sdpy);
+ qxl_create_server_to_iothread_pipe(&sdpy);
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 a708d59..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 {
@@ -75,4 +81,4 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
int qxl_vga_mode_get_command(
SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext);
/* used by both qxl and spice-display */
-void qxl_create_vcpu_to_iothread_pipe(SimpleSpiceDisplay *ssd);
+void qxl_create_server_to_iothread_pipe(SimpleSpiceDisplay *ssd);