* [Qemu-devel] [PATCHv3] async + suspend reworked
@ 2011-07-12 13:55 Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add worker wrapper functions Alon Levy
` (20 more replies)
0 siblings, 21 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
v2->v3:
builds correctly with older and newer spice, and runs with older and newer qxl driver.
fixed update_area_async to not use QXLRect on stack
qxl-render updated to work with update_area_async correctly
reverted change to update_area api - update_area still returns dirty
rects array
Git trees:
git://anongit.freedesktop.org/~alon/qemu async_and_s3.v3
git://anongit.freedesktop.org/~alon/spice async_and_s3.v4
git://anongit.freedesktop.org/~alon/spice-protocol s3.v2 (unchanged)
git://anongit.freedesktop.org/~alon/qxl s3.v3.async.v3 (unchanged)
Alon Levy (12):
qxl: add io_port_to_string
qxl: make qxl_guest_bug take variable arguments
qxl: use QXL_REVISION_*
qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to
update_area
qxl: async io support using new spice api
qxl-render/qxl: split out qxl_save_ppm
qxl-render: split out qxl_render_update_dirty_rectangles
qxl-render: qxl_render_update: nop if \!ssd.running
qxl-render: use update_area_async and update_area_complete
qxl: qxl_send_events: ignore if stopped (instead of abort)
qxl: only disallow specific io's in vga mode
qxl: add QXL_IO_FLUSH_{SURFACES,RELEASE} for guest S3&S4 support
Gerd Hoffmann (7):
spice: add worker wrapper functions.
spice: add qemu_spice_display_init_common
qxl: remove qxl_destroy_primary()
spice/qxl: move worker wrappers
qxl: fix surface tracking & locking
qxl: error handling fixes and cleanups.
qxl: bump pci rev
hw/qxl-render.c | 97 +++++++++--
hw/qxl.c | 490 ++++++++++++++++++++++++++++++++++++++++++++--------
hw/qxl.h | 38 ++++-
ui/spice-display.c | 94 +++++++++--
ui/spice-display.h | 33 ++++
5 files changed, 652 insertions(+), 100 deletions(-)
--
1.7.6
^ permalink raw reply [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] spice: add worker wrapper functions.
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add qemu_spice_display_init_common Alon Levy
` (19 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Add wrapper functions for all spice worker calls.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl-render.c | 4 +-
hw/qxl.c | 32 +++++++++---------
ui/spice-display.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++---
ui/spice-display.h | 20 +++++++++++
4 files changed, 126 insertions(+), 24 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 1316066..bef5f14 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -124,8 +124,8 @@ void qxl_render_update(PCIQXLDevice *qxl)
update.bottom = qxl->guest_primary.surface.height;
memset(dirty, 0, sizeof(dirty));
- qxl->ssd.worker->update_area(qxl->ssd.worker, 0, &update,
- dirty, ARRAY_SIZE(dirty), 1);
+ qemu_spice_update_area(&qxl->ssd, 0, &update,
+ dirty, ARRAY_SIZE(dirty), 1);
for (i = 0; i < ARRAY_SIZE(dirty); i++) {
if (qemu_spice_rect_is_empty(dirty+i)) {
diff --git a/hw/qxl.c b/hw/qxl.c
index 919ec91..545074d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -690,8 +690,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
dprint(d, 1, "%s: start%s\n", __FUNCTION__,
loadvm ? " (loadvm)" : "");
- d->ssd.worker->reset_cursor(d->ssd.worker);
- d->ssd.worker->reset_image_cache(d->ssd.worker);
+ qemu_spice_reset_cursor(&d->ssd);
+ qemu_spice_reset_image_cache(&d->ssd);
qxl_reset_surfaces(d);
qxl_reset_memslots(d);
@@ -796,7 +796,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
__FUNCTION__, memslot.slot_id,
memslot.virt_start, memslot.virt_end);
- d->ssd.worker->add_memslot(d->ssd.worker, &memslot);
+ qemu_spice_add_memslot(&d->ssd, &memslot);
d->guest_slots[slot_id].ptr = (void*)memslot.virt_start;
d->guest_slots[slot_id].size = memslot.virt_end - memslot.virt_start;
d->guest_slots[slot_id].delta = delta;
@@ -806,14 +806,14 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
{
dprint(d, 1, "%s: slot %d\n", __FUNCTION__, slot_id);
- d->ssd.worker->del_memslot(d->ssd.worker, MEMSLOT_GROUP_HOST, slot_id);
+ qemu_spice_del_memslot(&d->ssd, MEMSLOT_GROUP_HOST, slot_id);
d->guest_slots[slot_id].active = 0;
}
static void qxl_reset_memslots(PCIQXLDevice *d)
{
dprint(d, 1, "%s:\n", __FUNCTION__);
- d->ssd.worker->reset_memslots(d->ssd.worker);
+ qemu_spice_reset_memslots(&d->ssd);
memset(&d->guest_slots, 0, sizeof(d->guest_slots));
}
@@ -821,7 +821,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
{
dprint(d, 1, "%s:\n", __FUNCTION__);
d->mode = QXL_MODE_UNDEFINED;
- d->ssd.worker->destroy_surfaces(d->ssd.worker);
+ qemu_spice_destroy_surfaces(&d->ssd);
memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
}
@@ -875,7 +875,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
qxl->mode = QXL_MODE_NATIVE;
qxl->cmdflags = 0;
- qxl->ssd.worker->create_primary_surface(qxl->ssd.worker, 0, &surface);
+ qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface);
/* for local rendering */
qxl_render_resize(qxl);
@@ -890,7 +890,7 @@ static void qxl_destroy_primary(PCIQXLDevice *d)
dprint(d, 1, "%s\n", __FUNCTION__);
d->mode = QXL_MODE_UNDEFINED;
- d->ssd.worker->destroy_primary_surface(d->ssd.worker, 0);
+ qemu_spice_destroy_primary_surface(&d->ssd, 0);
}
static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -962,15 +962,15 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
case QXL_IO_UPDATE_AREA:
{
QXLRect update = d->ram->update_area;
- d->ssd.worker->update_area(d->ssd.worker, d->ram->update_surface,
- &update, NULL, 0, 0);
+ qemu_spice_update_area(&d->ssd, d->ram->update_surface,
+ &update, NULL, 0, 0);
break;
}
case QXL_IO_NOTIFY_CMD:
- d->ssd.worker->wakeup(d->ssd.worker);
+ qemu_spice_wakeup(&d->ssd);
break;
case QXL_IO_NOTIFY_CURSOR:
- d->ssd.worker->wakeup(d->ssd.worker);
+ qemu_spice_wakeup(&d->ssd);
break;
case QXL_IO_UPDATE_IRQ:
qxl_set_irq(d);
@@ -984,7 +984,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
}
d->oom_running = 1;
- d->ssd.worker->oom(d->ssd.worker);
+ qemu_spice_oom(&d->ssd);
d->oom_running = 0;
break;
case QXL_IO_SET_MODE:
@@ -1022,10 +1022,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
qxl_destroy_primary(d);
break;
case QXL_IO_DESTROY_SURFACE_WAIT:
- d->ssd.worker->destroy_surface_wait(d->ssd.worker, val);
+ qemu_spice_destroy_surface_wait(&d->ssd, val);
break;
case QXL_IO_DESTROY_ALL_SURFACES:
- d->ssd.worker->destroy_surfaces(d->ssd.worker);
+ qemu_spice_destroy_surfaces(&d->ssd);
break;
default:
fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
@@ -1430,7 +1430,7 @@ static int qxl_post_load(void *opaque, int version)
cmds[out].cmd.type = QXL_CMD_CURSOR;
cmds[out].group_id = MEMSLOT_GROUP_GUEST;
out++;
- d->ssd.worker->loadvm_commands(d->ssd.worker, cmds, out);
+ qemu_spice_loadvm_commands(&d->ssd, cmds, out);
qemu_free(cmds);
break;
diff --git a/ui/spice-display.c b/ui/spice-display.c
index feeee73..0433ea8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,6 +62,88 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
dest->right = MAX(dest->right, r->right);
}
+
+void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
+ struct QXLRect *area, struct QXLRect *dirty_rects,
+ uint32_t num_dirty_rects, uint32_t clear_dirty_region)
+{
+ ssd->worker->update_area(ssd->worker, surface_id, area, dirty_rects,
+ num_dirty_rects, clear_dirty_region);
+}
+
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
+{
+ ssd->worker->add_memslot(ssd->worker, memslot);
+}
+
+void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid)
+{
+ ssd->worker->del_memslot(ssd->worker, gid, sid);
+}
+
+void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
+ QXLDevSurfaceCreate *surface)
+{
+ ssd->worker->create_primary_surface(ssd->worker, id, surface);
+}
+
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
+{
+ ssd->worker->destroy_primary_surface(ssd->worker, id);
+}
+
+void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id)
+{
+ ssd->worker->destroy_surface_wait(ssd->worker, id);
+}
+
+void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext,
+ uint32_t count)
+{
+ ssd->worker->loadvm_commands(ssd->worker, ext, count);
+}
+
+void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->wakeup(ssd->worker);
+}
+
+void qemu_spice_oom(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->oom(ssd->worker);
+}
+
+void qemu_spice_start(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->start(ssd->worker);
+}
+
+void qemu_spice_stop(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->stop(ssd->worker);
+}
+
+void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->reset_memslots(ssd->worker);
+}
+
+void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->destroy_surfaces(ssd->worker);
+}
+
+void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->reset_image_cache(ssd->worker);
+}
+
+void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd)
+{
+ ssd->worker->reset_cursor(ssd->worker);
+}
+
+
static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
SimpleSpiceUpdate *update;
@@ -161,7 +243,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
memset(&memslot, 0, sizeof(memslot));
memslot.slot_group_id = MEMSLOT_GROUP_HOST;
memslot.virt_end = ~0;
- ssd->worker->add_memslot(ssd->worker, &memslot);
+ qemu_spice_add_memslot(ssd, &memslot);
}
void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
@@ -181,14 +263,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
surface.mem = (intptr_t)ssd->buf;
surface.group_id = MEMSLOT_GROUP_HOST;
- ssd->worker->create_primary_surface(ssd->worker, 0, &surface);
+ qemu_spice_create_primary_surface(ssd, 0, &surface);
}
void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
{
dprint(1, "%s:\n", __FUNCTION__);
- ssd->worker->destroy_primary_surface(ssd->worker, 0);
+ qemu_spice_destroy_primary_surface(ssd, 0);
}
void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
@@ -196,9 +278,9 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
SimpleSpiceDisplay *ssd = opaque;
if (running) {
- ssd->worker->start(ssd->worker);
+ qemu_spice_start(ssd);
} else {
- ssd->worker->stop(ssd->worker);
+ qemu_spice_stop(ssd);
}
ssd->running = running;
}
@@ -267,7 +349,7 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd)
if (ssd->notify) {
ssd->notify = 0;
- ssd->worker->wakeup(ssd->worker);
+ qemu_spice_wakeup(ssd);
dprint(2, "%s: notify\n", __FUNCTION__);
}
}
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 2f95f68..0effdfa 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -80,3 +80,23 @@ 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_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
+ struct QXLRect *area, struct QXLRect *dirty_rects,
+ uint32_t num_dirty_rects, uint32_t clear_dirty_region);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
+void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid);
+void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
+ QXLDevSurfaceCreate *surface);
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
+void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id);
+void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext,
+ uint32_t count);
+void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
+void qemu_spice_oom(SimpleSpiceDisplay *ssd);
+void qemu_spice_start(SimpleSpiceDisplay *ssd);
+void qemu_spice_stop(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd);
+void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd);
+void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] spice: add qemu_spice_display_init_common
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add worker wrapper functions Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: remove qxl_destroy_primary() Alon Levy
` (18 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Factor out SimpleSpiceDisplay initialization into
qemu_spice_display_init_common() and call it from
both qxl.c (for vga mode) and spice-display.c
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl.c | 7 +------
ui/spice-display.c | 17 +++++++++++------
ui/spice-display.h | 1 +
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 545074d..2d46814 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1321,12 +1321,7 @@ static int qxl_init_primary(PCIDevice *dev)
vga->ds = graphic_console_init(qxl_hw_update, qxl_hw_invalidate,
qxl_hw_screen_dump, qxl_hw_text_update, qxl);
- qxl->ssd.ds = vga->ds;
- qemu_mutex_init(&qxl->ssd.lock);
- qxl->ssd.mouse_x = -1;
- qxl->ssd.mouse_y = -1;
- qxl->ssd.bufsize = (16 * 1024 * 1024);
- qxl->ssd.buf = qemu_malloc(qxl->ssd.bufsize);
+ qemu_spice_display_init_common(&qxl->ssd, vga->ds);
qxl0 = qxl;
register_displaychangelistener(vga->ds, &display_listener);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 0433ea8..fef1758 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -285,6 +285,16 @@ void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
ssd->running = running;
}
+void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds)
+{
+ ssd->ds = ds;
+ qemu_mutex_init(&ssd->lock);
+ ssd->mouse_x = -1;
+ ssd->mouse_y = -1;
+ ssd->bufsize = (16 * 1024 * 1024);
+ ssd->buf = qemu_malloc(ssd->bufsize);
+}
+
/* display listener callbacks */
void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
@@ -498,12 +508,7 @@ static DisplayChangeListener display_listener = {
void qemu_spice_display_init(DisplayState *ds)
{
assert(sdpy.ds == NULL);
- sdpy.ds = ds;
- qemu_mutex_init(&sdpy.lock);
- sdpy.mouse_x = -1;
- sdpy.mouse_y = -1;
- sdpy.bufsize = (16 * 1024 * 1024);
- sdpy.buf = qemu_malloc(sdpy.bufsize);
+ qemu_spice_display_init_common(&sdpy, ds);
register_displaychangelistener(ds, &display_listener);
sdpy.qxl.base.sif = &dpy_interface.base;
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 0effdfa..a39b19d 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -75,6 +75,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd);
void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd);
void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd);
void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason);
+void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd, DisplayState *ds);
void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
int x, int y, int w, int h);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: remove qxl_destroy_primary()
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add worker wrapper functions Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add qemu_spice_display_init_common Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice/qxl: move worker wrappers Alon Levy
` (17 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
We'll have to move qemu_spice_destroy_primary_surface() out of
qxl_destroy_primary(). That makes the function pretty pointless,
so zap it and open code the two lines instead.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl.c | 28 ++++++++++++----------------
1 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 2d46814..0c5ed65 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -120,7 +120,6 @@ static QXLMode qxl_modes[] = {
static PCIQXLDevice *qxl0;
static void qxl_send_events(PCIQXLDevice *d, uint32_t events);
-static void qxl_destroy_primary(PCIQXLDevice *d);
static void qxl_reset_memslots(PCIQXLDevice *d);
static void qxl_reset_surfaces(PCIQXLDevice *d);
static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
@@ -617,7 +616,10 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
return;
}
dprint(d, 1, "%s\n", __FUNCTION__);
- qxl_destroy_primary(d);
+ if (d->mode != QXL_MODE_UNDEFINED) {
+ d->mode = QXL_MODE_UNDEFINED;
+ qemu_spice_destroy_primary_surface(&d->ssd, 0);
+ }
}
static void qxl_set_irq(PCIQXLDevice *d)
@@ -720,7 +722,10 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
if (qxl->mode != QXL_MODE_VGA) {
dprint(qxl, 1, "%s\n", __FUNCTION__);
- qxl_destroy_primary(qxl);
+ if (qxl->mode != QXL_MODE_UNDEFINED) {
+ qxl->mode = QXL_MODE_UNDEFINED;
+ qemu_spice_destroy_primary_surface(&qxl->ssd, 0);
+ }
qxl_soft_reset(qxl);
}
vga_ioport_write(opaque, addr, val);
@@ -881,18 +886,6 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
qxl_render_resize(qxl);
}
-static void qxl_destroy_primary(PCIQXLDevice *d)
-{
- if (d->mode == QXL_MODE_UNDEFINED) {
- return;
- }
-
- dprint(d, 1, "%s\n", __FUNCTION__);
-
- d->mode = QXL_MODE_UNDEFINED;
- qemu_spice_destroy_primary_surface(&d->ssd, 0);
-}
-
static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
{
pcibus_t start = d->pci.io_regions[QXL_RAM_RANGE_INDEX].addr;
@@ -1019,7 +1012,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
case QXL_IO_DESTROY_PRIMARY:
PANIC_ON(val != 0);
dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
- qxl_destroy_primary(d);
+ if (d->mode != QXL_MODE_UNDEFINED) {
+ d->mode = QXL_MODE_UNDEFINED;
+ qemu_spice_destroy_primary_surface(&d->ssd, 0);
+ }
break;
case QXL_IO_DESTROY_SURFACE_WAIT:
qemu_spice_destroy_surface_wait(&d->ssd, val);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] spice/qxl: move worker wrappers
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (2 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: remove qxl_destroy_primary() Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: fix surface tracking & locking Alon Levy
` (16 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Move the wrapper functions which are used by qxl only to qxl.c.
Rename them from qemu_spice_* to qxl_spice_*. Also pass in a
qxl state pointer instead of a SimpleSpiceDisplay pointer.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl-render.c | 4 +-
hw/qxl.c | 66 ++++++++++++++++++++++++++++++++++++++++++++--------
hw/qxl.h | 12 +++++++++
ui/spice-display.c | 45 -----------------------------------
ui/spice-display.h | 11 --------
5 files changed, 70 insertions(+), 68 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index bef5f14..60b822d 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -124,8 +124,8 @@ void qxl_render_update(PCIQXLDevice *qxl)
update.bottom = qxl->guest_primary.surface.height;
memset(dirty, 0, sizeof(dirty));
- qemu_spice_update_area(&qxl->ssd, 0, &update,
- dirty, ARRAY_SIZE(dirty), 1);
+ qxl_spice_update_area(qxl, 0, &update,
+ dirty, ARRAY_SIZE(dirty), 1);
for (i = 0; i < ARRAY_SIZE(dirty); i++) {
if (qemu_spice_rect_is_empty(dirty+i)) {
diff --git a/hw/qxl.c b/hw/qxl.c
index 0c5ed65..c1508a5 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,6 +124,52 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
static void qxl_reset_surfaces(PCIQXLDevice *d);
static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
+
+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->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
+ num_dirty_rects, clear_dirty_region);
+}
+
+void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
+{
+ qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+}
+
+void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
+ uint32_t count)
+{
+ qxl->ssd.worker->loadvm_commands(qxl->ssd.worker, ext, count);
+}
+
+void qxl_spice_oom(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->oom(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->reset_memslots(qxl->ssd.worker);
+}
+
+void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
+}
+
+void qxl_spice_reset_cursor(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->reset_cursor(qxl->ssd.worker);
+}
+
+
static inline uint32_t msb_mask(uint32_t val)
{
uint32_t mask;
@@ -692,8 +738,8 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
dprint(d, 1, "%s: start%s\n", __FUNCTION__,
loadvm ? " (loadvm)" : "");
- qemu_spice_reset_cursor(&d->ssd);
- qemu_spice_reset_image_cache(&d->ssd);
+ qxl_spice_reset_cursor(d);
+ qxl_spice_reset_image_cache(d);
qxl_reset_surfaces(d);
qxl_reset_memslots(d);
@@ -818,7 +864,7 @@ static void qxl_del_memslot(PCIQXLDevice *d, uint32_t slot_id)
static void qxl_reset_memslots(PCIQXLDevice *d)
{
dprint(d, 1, "%s:\n", __FUNCTION__);
- qemu_spice_reset_memslots(&d->ssd);
+ qxl_spice_reset_memslots(d);
memset(&d->guest_slots, 0, sizeof(d->guest_slots));
}
@@ -826,7 +872,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
{
dprint(d, 1, "%s:\n", __FUNCTION__);
d->mode = QXL_MODE_UNDEFINED;
- qemu_spice_destroy_surfaces(&d->ssd);
+ qxl_spice_destroy_surfaces(d);
memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
}
@@ -955,8 +1001,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
case QXL_IO_UPDATE_AREA:
{
QXLRect update = d->ram->update_area;
- qemu_spice_update_area(&d->ssd, d->ram->update_surface,
- &update, NULL, 0, 0);
+ qxl_spice_update_area(d, d->ram->update_surface,
+ &update, NULL, 0, 0);
break;
}
case QXL_IO_NOTIFY_CMD:
@@ -977,7 +1023,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
}
d->oom_running = 1;
- qemu_spice_oom(&d->ssd);
+ qxl_spice_oom(d);
d->oom_running = 0;
break;
case QXL_IO_SET_MODE:
@@ -1018,10 +1064,10 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
break;
case QXL_IO_DESTROY_SURFACE_WAIT:
- qemu_spice_destroy_surface_wait(&d->ssd, val);
+ qxl_spice_destroy_surface_wait(d, val);
break;
case QXL_IO_DESTROY_ALL_SURFACES:
- qemu_spice_destroy_surfaces(&d->ssd);
+ qxl_spice_destroy_surfaces(d);
break;
default:
fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
@@ -1421,7 +1467,7 @@ static int qxl_post_load(void *opaque, int version)
cmds[out].cmd.type = QXL_CMD_CURSOR;
cmds[out].group_id = MEMSLOT_GROUP_GUEST;
out++;
- qemu_spice_loadvm_commands(&d->ssd, cmds, out);
+ qxl_spice_loadvm_commands(d, cmds, out);
qemu_free(cmds);
break;
diff --git a/hw/qxl.h b/hw/qxl.h
index f6c450d..489d518 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -98,6 +98,18 @@ typedef struct PCIQXLDevice {
/* qxl.c */
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
+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);
+void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id);
+void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
+ uint32_t count);
+void qxl_spice_oom(PCIQXLDevice *qxl);
+void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
+void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl);
+void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
+void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
+
/* qxl-logger.c */
void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index fef1758..af10ae8 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -63,14 +63,6 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
}
-void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
- struct QXLRect *area, struct QXLRect *dirty_rects,
- uint32_t num_dirty_rects, uint32_t clear_dirty_region)
-{
- ssd->worker->update_area(ssd->worker, surface_id, area, dirty_rects,
- num_dirty_rects, clear_dirty_region);
-}
-
void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
{
ssd->worker->add_memslot(ssd->worker, memslot);
@@ -92,27 +84,11 @@ void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
ssd->worker->destroy_primary_surface(ssd->worker, id);
}
-void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id)
-{
- ssd->worker->destroy_surface_wait(ssd->worker, id);
-}
-
-void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext,
- uint32_t count)
-{
- ssd->worker->loadvm_commands(ssd->worker, ext, count);
-}
-
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
{
ssd->worker->wakeup(ssd->worker);
}
-void qemu_spice_oom(SimpleSpiceDisplay *ssd)
-{
- ssd->worker->oom(ssd->worker);
-}
-
void qemu_spice_start(SimpleSpiceDisplay *ssd)
{
ssd->worker->start(ssd->worker);
@@ -123,27 +99,6 @@ void qemu_spice_stop(SimpleSpiceDisplay *ssd)
ssd->worker->stop(ssd->worker);
}
-void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd)
-{
- ssd->worker->reset_memslots(ssd->worker);
-}
-
-void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd)
-{
- ssd->worker->destroy_surfaces(ssd->worker);
-}
-
-void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd)
-{
- ssd->worker->reset_image_cache(ssd->worker);
-}
-
-void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd)
-{
- ssd->worker->reset_cursor(ssd->worker);
-}
-
-
static SimpleSpiceUpdate *qemu_spice_create_update(SimpleSpiceDisplay *ssd)
{
SimpleSpiceUpdate *update;
diff --git a/ui/spice-display.h b/ui/spice-display.h
index a39b19d..d32dc9e 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -82,22 +82,11 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
-void qemu_spice_update_area(SimpleSpiceDisplay *ssd, uint32_t surface_id,
- struct QXLRect *area, struct QXLRect *dirty_rects,
- uint32_t num_dirty_rects, uint32_t clear_dirty_region);
void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid);
void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
QXLDevSurfaceCreate *surface);
void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
-void qemu_spice_destroy_surface_wait(SimpleSpiceDisplay *ssd, uint32_t id);
-void qemu_spice_loadvm_commands(SimpleSpiceDisplay *ssd, struct QXLCommandExt *ext,
- uint32_t count);
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
-void qemu_spice_oom(SimpleSpiceDisplay *ssd);
void qemu_spice_start(SimpleSpiceDisplay *ssd);
void qemu_spice_stop(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_memslots(SimpleSpiceDisplay *ssd);
-void qemu_spice_destroy_surfaces(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_image_cache(SimpleSpiceDisplay *ssd);
-void qemu_spice_reset_cursor(SimpleSpiceDisplay *ssd);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: fix surface tracking & locking
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (3 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice/qxl: move worker wrappers Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add io_port_to_string Alon Levy
` (15 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Surface tracking needs proper locking since it is used from vcpu and spice
worker threads, add it. Also reset the surface counter when zapping all
surfaces.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl.c | 13 ++++++++++++-
hw/qxl.h | 2 ++
2 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index c1508a5..6862bc8 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -135,7 +135,12 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
{
+ qemu_mutex_lock(&qxl->track_lock);
+ PANIC_ON(id >= NUM_SURFACES);
qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+ qxl->guest_surfaces.cmds[id] = 0;
+ qxl->guest_surfaces.count--;
+ qemu_mutex_unlock(&qxl->track_lock);
}
void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
@@ -156,7 +161,11 @@ void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
{
+ qemu_mutex_lock(&qxl->track_lock);
qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+ memset(&qxl->guest_surfaces.cmds, 0, sizeof(qxl->guest_surfaces.cmds));
+ qxl->guest_surfaces.count = 0;
+ qemu_mutex_unlock(&qxl->track_lock);
}
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
@@ -315,6 +324,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
uint32_t id = le32_to_cpu(cmd->surface_id);
PANIC_ON(id >= NUM_SURFACES);
+ qemu_mutex_lock(&qxl->track_lock);
if (cmd->type == QXL_SURFACE_CMD_CREATE) {
qxl->guest_surfaces.cmds[id] = ext->cmd.data;
qxl->guest_surfaces.count++;
@@ -325,6 +335,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
qxl->guest_surfaces.cmds[id] = 0;
qxl->guest_surfaces.count--;
}
+ qemu_mutex_unlock(&qxl->track_lock);
break;
}
case QXL_CMD_CURSOR:
@@ -873,7 +884,6 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
dprint(d, 1, "%s:\n", __FUNCTION__);
d->mode = QXL_MODE_UNDEFINED;
qxl_spice_destroy_surfaces(d);
- memset(&d->guest_surfaces.cmds, 0, sizeof(d->guest_surfaces.cmds));
}
/* called from spice server thread context only */
@@ -1284,6 +1294,7 @@ static int qxl_init_common(PCIQXLDevice *qxl)
qxl->generation = 1;
qxl->num_memslots = NUM_MEMSLOTS;
qxl->num_surfaces = NUM_SURFACES;
+ qemu_mutex_init(&qxl->track_lock);
switch (qxl->revision) {
case 1: /* spice 0.4 -- qxl-1 */
diff --git a/hw/qxl.h b/hw/qxl.h
index 489d518..087ef6b 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -55,6 +55,8 @@ typedef struct PCIQXLDevice {
} guest_surfaces;
QXLPHYSICAL guest_cursor;
+ QemuMutex track_lock;
+
/* thread signaling */
pthread_t main;
int pipe[2];
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: add io_port_to_string
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (4 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: fix surface tracking & locking Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: error handling fixes and cleanups Alon Levy
` (14 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
---
hw/qxl.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 61 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 6862bc8..7be7ae1 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -407,6 +407,65 @@ static const char *qxl_mode_to_string(int mode)
return "INVALID";
}
+static const char *io_port_to_string(uint32_t io_port)
+{
+ if (io_port >= QXL_IO_RANGE_SIZE) {
+ return "out of range";
+ }
+ switch(io_port) {
+ case QXL_IO_NOTIFY_CMD:
+ return "QXL_IO_NOTIFY_CMD";
+ case QXL_IO_NOTIFY_CURSOR:
+ return "QXL_IO_NOTIFY_CURSOR";
+ case QXL_IO_UPDATE_AREA:
+ return "QXL_IO_UPDATE_AREA";
+ case QXL_IO_UPDATE_IRQ:
+ return "QXL_IO_UPDATE_IRQ";
+ case QXL_IO_NOTIFY_OOM:
+ return "QXL_IO_NOTIFY_OOM";
+ case QXL_IO_RESET:
+ return "QXL_IO_RESET";
+ case QXL_IO_SET_MODE:
+ return "QXL_IO_SET_MODE";
+ case QXL_IO_LOG:
+ return "QXL_IO_LOG";
+ case QXL_IO_MEMSLOT_ADD:
+ return "QXL_IO_MEMSLOT_ADD";
+ case QXL_IO_MEMSLOT_DEL:
+ return "QXL_IO_MEMSLOT_DEL";
+ case QXL_IO_DETACH_PRIMARY:
+ return "QXL_IO_DETACH_PRIMARY";
+ case QXL_IO_ATTACH_PRIMARY:
+ return "QXL_IO_ATTACH_PRIMARY";
+ case QXL_IO_CREATE_PRIMARY:
+ return "QXL_IO_CREATE_PRIMARY";
+ case QXL_IO_DESTROY_PRIMARY:
+ return "QXL_IO_DESTROY_PRIMARY";
+ case QXL_IO_DESTROY_SURFACE_WAIT:
+ return "QXL_IO_DESTROY_SURFACE_WAIT";
+ case QXL_IO_DESTROY_ALL_SURFACES:
+ return "QXL_IO_DESTROY_ALL_SURFACES";
+ case QXL_IO_UPDATE_AREA_ASYNC:
+ return "QXL_IO_UPDATE_AREA_ASYNC";
+ case QXL_IO_MEMSLOT_ADD_ASYNC:
+ return "QXL_IO_MEMSLOT_ADD_ASYNC";
+ case QXL_IO_CREATE_PRIMARY_ASYNC:
+ return "QXL_IO_CREATE_PRIMARY_ASYNC";
+ case QXL_IO_DESTROY_PRIMARY_ASYNC:
+ return "QXL_IO_DESTROY_PRIMARY_ASYNC";
+ case QXL_IO_DESTROY_SURFACE_ASYNC:
+ return "QXL_IO_DESTROY_SURFACE_ASYNC";
+ case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+ return "QXL_IO_DESTROY_ALL_SURFACES_ASYNC";
+ case QXL_IO_FLUSH_SURFACES_ASYNC:
+ return "QXL_IO_FLUSH_SURFACES_ASYNC";
+ case QXL_IO_FLUSH_RELEASE:
+ return "QXL_IO_FLUSH_RELEASE";
+ }
+ // not reached?
+ return "error in io_port_to_string";
+}
+
/* called from spice server thread context only */
static int interface_get_command(QXLInstance *sin, struct QXLCommandExt *ext)
{
@@ -1003,7 +1062,8 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
default:
if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
break;
- dprint(d, 1, "%s: unexpected port 0x%x in vga mode\n", __FUNCTION__, io_port);
+ dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
+ __FUNCTION__, io_port, io_port_to_string(io_port));
return;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: error handling fixes and cleanups.
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (5 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add io_port_to_string Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: make qxl_guest_bug take variable arguments Alon Levy
` (13 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Add qxl_guest_bug() function which is supposed to be called in case
sanity checks of guest requests fail. It raises an error IRQ and
logs a message in case guest debugging is enabled.
Make PANIC_ON() abort instead of exit. That macro should be used
for qemu bugs only, any guest-triggerable stuff should use the new
qxl_guest_bug() function instead.
Convert a few easy cases from PANIC_ON() to qxl_guest_bug() to
show intended usage.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl.c | 32 ++++++++++++++++++++++++++++----
hw/qxl.h | 3 ++-
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 7be7ae1..91bc98d 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,6 +124,14 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
static void qxl_reset_surfaces(PCIQXLDevice *d);
static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg)
+{
+ qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
+ if (qxl->guestdebug) {
+ fprintf(stderr, "qxl-%d: guest bug: %s\n", qxl->id, msg);
+ }
+}
+
void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
struct QXLRect *area, struct QXLRect *dirty_rects,
@@ -1111,22 +1119,38 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
qxl_hard_reset(d, 0);
break;
case QXL_IO_MEMSLOT_ADD:
- PANIC_ON(val >= NUM_MEMSLOTS);
- PANIC_ON(d->guest_slots[val].active);
+ if (val >= NUM_MEMSLOTS) {
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range");
+ break;
+ }
+ if (d->guest_slots[val].active) {
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active");
+ break;
+ }
d->guest_slots[val].slot = d->ram->mem_slot;
qxl_add_memslot(d, val, 0);
break;
case QXL_IO_MEMSLOT_DEL:
+ if (val >= NUM_MEMSLOTS) {
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range");
+ break;
+ }
qxl_del_memslot(d, val);
break;
case QXL_IO_CREATE_PRIMARY:
- PANIC_ON(val != 0);
+ if (val != 0) {
+ qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0");
+ break;
+ }
dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
d->guest_primary.surface = d->ram->create_surface;
qxl_create_guest_primary(d, 0);
break;
case QXL_IO_DESTROY_PRIMARY:
- PANIC_ON(val != 0);
+ if (val != 0) {
+ qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
+ break;
+ }
dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
if (d->mode != QXL_MODE_UNDEFINED) {
d->mode = QXL_MODE_UNDEFINED;
diff --git a/hw/qxl.h b/hw/qxl.h
index 087ef6b..88393c2 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -86,7 +86,7 @@ typedef struct PCIQXLDevice {
#define PANIC_ON(x) if ((x)) { \
printf("%s: PANIC %s failed\n", __FUNCTION__, #x); \
- exit(-1); \
+ abort(); \
}
#define dprint(_qxl, _level, _fmt, ...) \
@@ -99,6 +99,7 @@ typedef struct PCIQXLDevice {
/* qxl.c */
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg);
void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
struct QXLRect *area, struct QXLRect *dirty_rects,
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: make qxl_guest_bug take variable arguments
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (6 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: error handling fixes and cleanups Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: bump pci rev Alon Levy
` (12 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
---
hw/qxl.c | 18 +++++++++++-------
hw/qxl.h | 2 +-
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 91bc98d..ae1d0de 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -124,11 +124,15 @@ static void qxl_reset_memslots(PCIQXLDevice *d);
static void qxl_reset_surfaces(PCIQXLDevice *d);
static void qxl_ring_set_dirty(PCIQXLDevice *qxl);
-void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg)
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
{
qxl_send_events(qxl, QXL_INTERRUPT_ERROR);
if (qxl->guestdebug) {
- fprintf(stderr, "qxl-%d: guest bug: %s\n", qxl->id, msg);
+ va_list ap;
+ va_start(ap, msg);
+ fprintf(stderr, "qxl-%d: guest bug: ", qxl->id);
+ vfprintf(stderr, msg, ap);
+ va_end(ap);
}
}
@@ -1120,11 +1124,11 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
case QXL_IO_MEMSLOT_ADD:
if (val >= NUM_MEMSLOTS) {
- qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range");
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range\n");
break;
}
if (d->guest_slots[val].active) {
- qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active");
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: memory slot already active\n");
break;
}
d->guest_slots[val].slot = d->ram->mem_slot;
@@ -1132,14 +1136,14 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
case QXL_IO_MEMSLOT_DEL:
if (val >= NUM_MEMSLOTS) {
- qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range");
+ qxl_guest_bug(d, "QXL_IO_MEMSLOT_DEL: val out of range\n");
break;
}
qxl_del_memslot(d, val);
break;
case QXL_IO_CREATE_PRIMARY:
if (val != 0) {
- qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0");
+ qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0\n");
break;
}
dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
@@ -1148,7 +1152,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
case QXL_IO_DESTROY_PRIMARY:
if (val != 0) {
- qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
+ qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0\n");
break;
}
dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
diff --git a/hw/qxl.h b/hw/qxl.h
index 88393c2..e361bc6 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -99,7 +99,7 @@ typedef struct PCIQXLDevice {
/* qxl.c */
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
-void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg);
+void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
struct QXLRect *area, struct QXLRect *dirty_rects,
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: bump pci rev
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (7 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: make qxl_guest_bug take variable arguments Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: use QXL_REVISION_* Alon Levy
` (11 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
From: Gerd Hoffmann <kraxel@redhat.com>
Inform guest drivers about the new features I/O commands we have
now (async commands, S3 support) if building with newer spice, i.e.
if SPICE_INTERFACE_QXL_MINOR >= 1.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/qxl.c | 11 ++++++++---
hw/qxl.h | 6 ++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index ae1d0de..d3b1581 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1389,9 +1389,14 @@ static int qxl_init_common(PCIQXLDevice *qxl)
pci_device_rev = QXL_REVISION_STABLE_V04;
break;
case 2: /* spice 0.6 -- qxl-2 */
- default:
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;
}
pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
@@ -1655,7 +1660,7 @@ static PCIDeviceInfo qxl_info_primary = {
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 * 1024),
DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, 64 * 1024 * 1024),
- DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
+ DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, QXL_DEFAULT_REVISION),
DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
@@ -1676,7 +1681,7 @@ static PCIDeviceInfo qxl_info_secondary = {
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32("ram_size", PCIQXLDevice, vga.vram_size, 64 * 1024 * 1024),
DEFINE_PROP_UINT32("vram_size", PCIQXLDevice, vram_size, 64 * 1024 * 1024),
- DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, 2),
+ DEFINE_PROP_UINT32("revision", PCIQXLDevice, revision, QXL_DEFAULT_REVISION),
DEFINE_PROP_UINT32("debug", PCIQXLDevice, debug, 0),
DEFINE_PROP_UINT32("guestdebug", PCIQXLDevice, guestdebug, 0),
DEFINE_PROP_UINT32("cmdlog", PCIQXLDevice, cmdlog, 0),
diff --git a/hw/qxl.h b/hw/qxl.h
index e361bc6..85d37be 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -97,6 +97,12 @@ 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);
void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: use QXL_REVISION_*
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (8 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: bump pci rev Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area Alon Levy
` (10 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
---
hw/qxl.c | 22 ++++++++++------------
1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index d3b1581..17b5b39 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1375,7 +1375,6 @@ static DisplayChangeListener display_listener = {
static int qxl_init_common(PCIQXLDevice *qxl)
{
uint8_t* config = qxl->pci.config;
- uint32_t pci_device_rev;
uint32_t io_size;
qxl->mode = QXL_MODE_UNDEFINED;
@@ -1385,21 +1384,20 @@ static int qxl_init_common(PCIQXLDevice *qxl)
qemu_mutex_init(&qxl->track_lock);
switch (qxl->revision) {
- case 1: /* spice 0.4 -- qxl-1 */
- pci_device_rev = QXL_REVISION_STABLE_V04;
- break;
- case 2: /* spice 0.6 -- qxl-2 */
- pci_device_rev = QXL_REVISION_STABLE_V06;
- break;
+ case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
+ case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
#if SPICE_INTERFACE_QXL_MINOR >= 1
- case 3: /* qxl-3 */
+ case QXL_REVISION_STABLE_V10: /* spice 0.10? -- qxl-3 */
+ break;
#endif
default:
- pci_device_rev = QXL_DEFAULT_REVISION;
+ fprintf(stderr, "invalid revision %d, resetting to %d\n", qxl->revision,
+ QXL_DEFAULT_REVISION);
+ qxl->revision = QXL_DEFAULT_REVISION;
break;
}
- pci_set_byte(&config[PCI_REVISION_ID], pci_device_rev);
+ pci_set_byte(&config[PCI_REVISION_ID], qxl->revision);
pci_set_byte(&config[PCI_INTERRUPT_PIN], 1);
qxl->rom_size = qxl_rom_size();
@@ -1410,14 +1408,14 @@ static int qxl_init_common(PCIQXLDevice *qxl)
if (qxl->vram_size < 16 * 1024 * 1024) {
qxl->vram_size = 16 * 1024 * 1024;
}
- if (qxl->revision == 1) {
+ if (qxl->revision == QXL_REVISION_STABLE_V04) {
qxl->vram_size = 4096;
}
qxl->vram_size = msb_mask(qxl->vram_size * 2 - 1);
qxl->vram_offset = qemu_ram_alloc(&qxl->pci.qdev, "qxl.vram", qxl->vram_size);
io_size = msb_mask(QXL_IO_RANGE_SIZE * 2 - 1);
- if (qxl->revision == 1) {
+ if (qxl->revision == QXL_REVISION_STABLE_V04) {
io_size = 8;
}
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (9 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: use QXL_REVISION_* Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-13 6:56 ` Gerd Hoffmann
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: async io support using new spice api Alon Levy
` (9 subsequent siblings)
20 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
---
hw/qxl.c | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 17b5b39..6094b38 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -136,7 +136,6 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
}
}
-
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)
@@ -1081,12 +1080,9 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
switch (io_port) {
case QXL_IO_UPDATE_AREA:
- {
- QXLRect update = d->ram->update_area;
qxl_spice_update_area(d, d->ram->update_surface,
- &update, NULL, 0, 0);
+ &d->ram->update_area, NULL, 0, 1);
break;
- }
case QXL_IO_NOTIFY_CMD:
qemu_spice_wakeup(&d->ssd);
break;
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: async io support using new spice api
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (10 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm Alon Levy
` (8 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Some of the QXL port i/o commands are waiting for the spice server to
complete certain actions. Add async versions for these commands, so we
don't block the vcpu while the spice server processses the command.
Instead the qxl device will raise an IRQ when done.
The async command processing relies on an added QXLInterface::async_complete
and added QXLWorker::*_async additions, in spice server qxl >= 3.1
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Alon Levy <alevy@redhat.com>
---
hw/qxl.c | 229 ++++++++++++++++++++++++++++++++++++++++++++-------
hw/qxl.h | 15 +++-
ui/spice-display.c | 48 +++++++++---
ui/spice-display.h | 25 +++++-
4 files changed, 270 insertions(+), 47 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 6094b38..bd540c0 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -138,22 +138,49 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
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)
+ uint32_t num_dirty_rects,
+ uint32_t clear_dirty_region)
{
qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, dirty_rects,
num_dirty_rects, clear_dirty_region);
}
-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id)
+#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)
+{
+ qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area,
+ clear_dirty_region,
+ is_vga ? QXL_COOKIE_VGA : 0);
+}
+#endif
+
+static void qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl,
+ uint32_t id)
{
qemu_mutex_lock(&qxl->track_lock);
- PANIC_ON(id >= NUM_SURFACES);
- qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
qxl->guest_surfaces.cmds[id] = 0;
qxl->guest_surfaces.count--;
qemu_mutex_unlock(&qxl->track_lock);
}
+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
+ qxl->ssd.worker->destroy_surface_wait_async(qxl->ssd.worker, id,
+ (uint64_t)id);
+#endif
+ } else {
+ qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
+ qxl_spice_destroy_surface_wait_complete(qxl, id);
+ }
+}
+
void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
uint32_t count)
{
@@ -170,15 +197,28 @@ void qxl_spice_reset_memslots(PCIQXLDevice *qxl)
qxl->ssd.worker->reset_memslots(qxl->ssd.worker);
}
-void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl)
+static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
{
qemu_mutex_lock(&qxl->track_lock);
- qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
memset(&qxl->guest_surfaces.cmds, 0, sizeof(qxl->guest_surfaces.cmds));
qxl->guest_surfaces.count = 0;
qemu_mutex_unlock(&qxl->track_lock);
}
+static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
+{
+ if (async) {
+#if SPICE_INTERFACE_QXL_MINOR < 1
+ abort();
+#else
+ qxl->ssd.worker->destroy_surfaces_async(qxl->ssd.worker, 0);
+#endif
+ } else {
+ qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
+ qxl_spice_destroy_surfaces_complete(qxl);
+ }
+}
+
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl)
{
qxl->ssd.worker->reset_image_cache(qxl->ssd.worker);
@@ -705,6 +745,43 @@ static int interface_flush_resources(QXLInstance *sin)
return ret;
}
+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)
+{
+ PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
+ uint32_t current_async;
+
+ if (cookie == QXL_COOKIE_VGA) {
+ dprint(qxl, 3, "ignoring async from vga update\n");
+ return;
+ }
+
+ qemu_mutex_lock(&qxl->async_lock);
+ current_async = qxl->current_async;
+ qxl->current_async = QXL_UNDEFINED_IO;
+ qemu_mutex_unlock(&qxl->async_lock);
+
+ dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
+ switch (current_async) {
+ case QXL_IO_CREATE_PRIMARY_ASYNC:
+ qxl_create_guest_primary_complete(qxl);
+ break;
+ case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+ qxl_spice_destroy_surfaces_complete(qxl);
+ break;
+ case QXL_IO_DESTROY_SURFACE_ASYNC:
+ qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie);
+ break;
+ }
+ qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
+}
+
+#endif
+
static const QXLInterface qxl_interface = {
.base.type = SPICE_INTERFACE_QXL,
.base.description = "qxl gpu",
@@ -724,6 +801,9 @@ 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)
@@ -745,7 +825,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
dprint(d, 1, "%s\n", __FUNCTION__);
if (d->mode != QXL_MODE_UNDEFINED) {
d->mode = QXL_MODE_UNDEFINED;
- qemu_spice_destroy_primary_surface(&d->ssd, 0);
+ qemu_spice_destroy_primary_surface(&d->ssd, 0, QXL_SYNC);
}
}
@@ -851,14 +931,15 @@ static void qxl_vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
dprint(qxl, 1, "%s\n", __FUNCTION__);
if (qxl->mode != QXL_MODE_UNDEFINED) {
qxl->mode = QXL_MODE_UNDEFINED;
- qemu_spice_destroy_primary_surface(&qxl->ssd, 0);
+ qemu_spice_destroy_primary_surface(&qxl->ssd, 0, QXL_SYNC);
}
qxl_soft_reset(qxl);
}
vga_ioport_write(opaque, addr, val);
}
-static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
+static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta,
+ qxl_async_io async)
{
static const int regions[] = {
QXL_RAM_RANGE_INDEX,
@@ -928,7 +1009,7 @@ static void qxl_add_memslot(PCIQXLDevice *d, uint32_t slot_id, uint64_t delta)
__FUNCTION__, memslot.slot_id,
memslot.virt_start, memslot.virt_end);
- qemu_spice_add_memslot(&d->ssd, &memslot);
+ qemu_spice_add_memslot(&d->ssd, &memslot, async);
d->guest_slots[slot_id].ptr = (void*)memslot.virt_start;
d->guest_slots[slot_id].size = memslot.virt_end - memslot.virt_start;
d->guest_slots[slot_id].delta = delta;
@@ -953,7 +1034,7 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
{
dprint(d, 1, "%s:\n", __FUNCTION__);
d->mode = QXL_MODE_UNDEFINED;
- qxl_spice_destroy_surfaces(d);
+ qxl_spice_destroy_surfaces(d, QXL_SYNC);
}
/* called from spice server thread context only */
@@ -978,7 +1059,14 @@ void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
}
}
-static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
+static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
+{
+ /* for local rendering */
+ qxl_render_resize(qxl);
+}
+
+static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
+ qxl_async_io async)
{
QXLDevSurfaceCreate surface;
QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
@@ -1006,10 +1094,11 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm)
qxl->mode = QXL_MODE_NATIVE;
qxl->cmdflags = 0;
- qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface);
+ qemu_spice_create_primary_surface(&qxl->ssd, 0, &surface, async);
- /* for local rendering */
- qxl_render_resize(qxl);
+ if (async == QXL_SYNC) {
+ qxl_create_guest_primary_complete(qxl);
+ }
}
static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
@@ -1039,10 +1128,10 @@ static void qxl_set_mode(PCIQXLDevice *d, int modenr, int loadvm)
}
d->guest_slots[0].slot = slot;
- qxl_add_memslot(d, 0, devmem);
+ qxl_add_memslot(d, 0, devmem, QXL_SYNC);
d->guest_primary.surface = surface;
- qxl_create_guest_primary(d, 0);
+ qxl_create_guest_primary(d, 0, QXL_SYNC);
d->mode = QXL_MODE_COMPAT;
d->cmdflags = QXL_COMMAND_FLAG_COMPAT;
@@ -1060,13 +1149,16 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
PCIQXLDevice *d = opaque;
uint32_t io_port = addr - d->io_base;
+ qxl_async_io async = QXL_SYNC;
switch (io_port) {
case QXL_IO_RESET:
case QXL_IO_SET_MODE:
case QXL_IO_MEMSLOT_ADD:
+ case QXL_IO_MEMSLOT_ADD_ASYNC:
case QXL_IO_MEMSLOT_DEL:
case QXL_IO_CREATE_PRIMARY:
+ case QXL_IO_CREATE_PRIMARY_ASYNC:
case QXL_IO_UPDATE_IRQ:
case QXL_IO_LOG:
break;
@@ -1075,13 +1167,52 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
__FUNCTION__, io_port, io_port_to_string(io_port));
+ /* 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);
+ }
+ return;
+ }
+
+ switch (io_port) {
+ case QXL_IO_UPDATE_AREA_ASYNC:
+ case QXL_IO_MEMSLOT_ADD_ASYNC:
+ case QXL_IO_CREATE_PRIMARY_ASYNC:
+ case QXL_IO_DESTROY_PRIMARY_ASYNC:
+ case QXL_IO_DESTROY_SURFACE_ASYNC:
+ case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+#if SPICE_INTERFACE_QXL_MINOR < 1
+ fprintf(stderr, "qxl: error: async not supported by libspice but guest driver used it\n");
return;
+#else
+ async = QXL_ASYNC;
+ qemu_mutex_lock(&d->async_lock);
+ if (d->current_async != QXL_UNDEFINED_IO) {
+ qxl_guest_bug(d, "%d async started before last (%d) complete\n",
+ io_port, d->current_async);
+ qemu_mutex_unlock(&d->async_lock);
+ return;
+ }
+ d->current_async = io_port;
+ qemu_mutex_unlock(&d->async_lock);
+ dprint(d, 2, "start async %d (%d)\n", io_port, val);
+ break;
+#endif
+ default:
+ break;
}
switch (io_port) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ case QXL_IO_UPDATE_AREA_ASYNC:
+ qxl_spice_update_area_async(d, d->ram->update_surface,
+ &d->ram->update_area, 0, 0);
+ break;
+#endif
case QXL_IO_UPDATE_AREA:
qxl_spice_update_area(d, d->ram->update_surface,
- &d->ram->update_area, NULL, 0, 1);
+ &d->ram->update_area, NULL, 0, 0);
break;
case QXL_IO_NOTIFY_CMD:
qemu_spice_wakeup(&d->ssd);
@@ -1118,6 +1249,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
dprint(d, 1, "QXL_IO_RESET\n");
qxl_hard_reset(d, 0);
break;
+ case QXL_IO_MEMSLOT_ADD_ASYNC:
case QXL_IO_MEMSLOT_ADD:
if (val >= NUM_MEMSLOTS) {
qxl_guest_bug(d, "QXL_IO_MEMSLOT_ADD: val out of range\n");
@@ -1128,7 +1260,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
break;
}
d->guest_slots[val].slot = d->ram->mem_slot;
- qxl_add_memslot(d, val, 0);
+ qxl_add_memslot(d, val, 0, async);
break;
case QXL_IO_MEMSLOT_DEL:
if (val >= NUM_MEMSLOTS) {
@@ -1137,36 +1269,66 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
qxl_del_memslot(d, val);
break;
+ case QXL_IO_CREATE_PRIMARY_ASYNC:
case QXL_IO_CREATE_PRIMARY:
if (val != 0) {
- qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY: val != 0\n");
- break;
+ qxl_guest_bug(d, "QXL_IO_CREATE_PRIMARY (async=%d): val != 0\n", async);
+ goto cancel_async;
}
- dprint(d, 1, "QXL_IO_CREATE_PRIMARY\n");
+ dprint(d, 1, "QXL_IO_CREATE_PRIMARY async=%d\n", async);
d->guest_primary.surface = d->ram->create_surface;
- qxl_create_guest_primary(d, 0);
+ qxl_create_guest_primary(d, 0, async);
break;
+ case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_DESTROY_PRIMARY:
if (val != 0) {
- qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0\n");
- break;
+ qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0\n", async);
+ goto cancel_async;
}
- dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (%s)\n", qxl_mode_to_string(d->mode));
+ dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
+ qxl_mode_to_string(d->mode));
if (d->mode != QXL_MODE_UNDEFINED) {
d->mode = QXL_MODE_UNDEFINED;
- qemu_spice_destroy_primary_surface(&d->ssd, 0);
+ qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
+ } else {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ if (async) {
+ dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
+ qxl_mode_to_string(d->mode));
+ qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
+ goto cancel_async;
+ }
+#endif
}
break;
+ case QXL_IO_DESTROY_SURFACE_ASYNC:
case QXL_IO_DESTROY_SURFACE_WAIT:
- qxl_spice_destroy_surface_wait(d, val);
+ if (val >= NUM_SURFACES) {
+ qxl_guest_bug(d, "QXL_IO_DESTROY_SURFACE (async=%d): %d >= NUM_SURFACES", async, val);
+ goto cancel_async;
+ }
+ qxl_spice_destroy_surface_wait(d, val, async);
break;
+ case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
case QXL_IO_DESTROY_ALL_SURFACES:
- qxl_spice_destroy_surfaces(d);
+ d->mode = QXL_MODE_UNDEFINED;
+ qxl_spice_destroy_surfaces(d, async);
break;
default:
fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
abort();
}
+ return;
+cancel_async:
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ if (async) {
+ qemu_mutex_lock(&d->async_lock);
+ d->current_async = QXL_UNDEFINED_IO;
+ qemu_mutex_unlock(&d->async_lock);
+ }
+#else
+ return;
+#endif
}
static uint32_t ioport_read(void *opaque, uint32_t addr)
@@ -1378,7 +1540,10 @@ static int qxl_init_common(PCIQXLDevice *qxl)
qxl->num_memslots = NUM_MEMSLOTS;
qxl->num_surfaces = NUM_SURFACES;
qemu_mutex_init(&qxl->track_lock);
-
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ qemu_mutex_init(&qxl->async_lock);
+ qxl->current_async = QXL_UNDEFINED_IO;
+#endif
switch (qxl->revision) {
case QXL_REVISION_STABLE_V04: /* spice 0.4 -- qxl-1 */
case QXL_REVISION_STABLE_V06: /* spice 0.6 -- qxl-2 */
@@ -1546,9 +1711,9 @@ static int qxl_post_load(void *opaque, int version)
if (!d->guest_slots[i].active) {
continue;
}
- qxl_add_memslot(d, i, 0);
+ qxl_add_memslot(d, i, 0, QXL_SYNC);
}
- qxl_create_guest_primary(d, 1);
+ qxl_create_guest_primary(d, 1, QXL_SYNC);
/* replay surface-create and cursor-set commands */
cmds = qemu_mallocz(sizeof(QXLCommandExt) * (NUM_SURFACES + 1));
diff --git a/hw/qxl.h b/hw/qxl.h
index 85d37be..064311a 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -15,6 +15,8 @@ enum qxl_mode {
QXL_MODE_NATIVE,
};
+#define QXL_UNDEFINED_IO UINT32_MAX
+
typedef struct PCIQXLDevice {
PCIDevice pci;
SimpleSpiceDisplay ssd;
@@ -30,6 +32,11 @@ typedef struct PCIQXLDevice {
int32_t num_memslots;
int32_t num_surfaces;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ uint32_t current_async;
+ QemuMutex async_lock;
+#endif
+
struct guest_slots {
QXLMemSlot slot;
void *ptr;
@@ -110,12 +117,10 @@ void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
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);
-void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id);
void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
uint32_t count);
void qxl_spice_oom(PCIQXLDevice *qxl);
void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
-void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl);
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
@@ -127,3 +132,9 @@ 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
diff --git a/ui/spice-display.c b/ui/spice-display.c
index af10ae8..059184f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -62,26 +62,54 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
dest->right = MAX(dest->right, r->right);
}
-
-void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot)
-{
- ssd->worker->add_memslot(ssd->worker, memslot);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
+ qxl_async_io async)
+{
+ if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ ssd->worker->add_memslot_async(ssd->worker, memslot, 0);
+#else
+ abort();
+#endif
+ } else {
+ ssd->worker->add_memslot(ssd->worker, memslot);
+ }
}
+
void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid)
{
ssd->worker->del_memslot(ssd->worker, gid, sid);
}
void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
- QXLDevSurfaceCreate *surface)
-{
- ssd->worker->create_primary_surface(ssd->worker, id, surface);
+ QXLDevSurfaceCreate *surface,
+ qxl_async_io async)
+{
+ if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ ssd->worker->create_primary_surface_async(ssd->worker, id, surface, 0);
+#else
+ abort();
+#endif
+ } else {
+ ssd->worker->create_primary_surface(ssd->worker, id, surface);
+ }
}
-void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id)
+
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
+ uint32_t id, qxl_async_io async)
{
- ssd->worker->destroy_primary_surface(ssd->worker, id);
+ if (async != QXL_SYNC) {
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ ssd->worker->destroy_primary_surface_async(ssd->worker, id, 0);
+#else
+ abort();
+#endif
+ } else {
+ ssd->worker->destroy_primary_surface(ssd->worker, id);
+ }
}
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
@@ -198,7 +226,7 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
memset(&memslot, 0, sizeof(memslot));
memslot.slot_group_id = MEMSLOT_GROUP_HOST;
memslot.virt_end = ~0;
- qemu_spice_add_memslot(ssd, &memslot);
+ qemu_spice_add_memslot(ssd, &memslot, QXL_SYNC);
}
void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
@@ -218,14 +246,14 @@ void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
surface.mem = (intptr_t)ssd->buf;
surface.group_id = MEMSLOT_GROUP_HOST;
- qemu_spice_create_primary_surface(ssd, 0, &surface);
+ qemu_spice_create_primary_surface(ssd, 0, &surface, QXL_SYNC);
}
void qemu_spice_destroy_host_primary(SimpleSpiceDisplay *ssd)
{
dprint(1, "%s:\n", __FUNCTION__);
- qemu_spice_destroy_primary_surface(ssd, 0);
+ qemu_spice_destroy_primary_surface(ssd, 0, QXL_SYNC);
}
void qemu_spice_vm_change_state_handler(void *opaque, int running, int reason)
diff --git a/ui/spice-display.h b/ui/spice-display.h
index d32dc9e..d24cca9 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -33,6 +33,22 @@
#define NUM_SURFACES 1024
+/*
+ * Internal enum to differenciate between options for
+ * io calls that have a sync (old) version and an _async (new)
+ * version:
+ * QXL_SYNC: use the old version
+ * QXL_ASYNC: use the new version and make sure there are no two
+ * happening at the same time. This is used for guest initiated
+ * calls
+ */
+typedef enum qxl_async_io {
+ QXL_SYNC,
+ QXL_ASYNC,
+} qxl_async_io;
+
+#define QXL_COOKIE_VGA ((uint64_t)-1)
+
typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
@@ -82,11 +98,14 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd,
void qemu_spice_display_resize(SimpleSpiceDisplay *ssd);
void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd);
-void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot);
+void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
+ qxl_async_io async);
void qemu_spice_del_memslot(SimpleSpiceDisplay *ssd, uint32_t gid, uint32_t sid);
void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
- QXLDevSurfaceCreate *surface);
-void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id);
+ QXLDevSurfaceCreate *surface,
+ qxl_async_io async);
+void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
+ uint32_t id, qxl_async_io async);
void qemu_spice_wakeup(SimpleSpiceDisplay *ssd);
void qemu_spice_start(SimpleSpiceDisplay *ssd);
void qemu_spice_stop(SimpleSpiceDisplay *ssd);
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (11 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: async io support using new spice api Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-13 7:10 ` Gerd Hoffmann
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: split out qxl_render_update_dirty_rectangles Alon Levy
` (7 subsequent siblings)
20 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Later the save will happen asynchronously on surface_updated callback.
---
hw/qxl-render.c | 10 ++++++++++
hw/qxl.c | 2 +-
hw/qxl.h | 2 ++
3 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index 60b822d..e64b646 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -70,6 +70,15 @@ void qxl_render_resize(PCIQXLDevice *qxl)
}
}
+static void qxl_save_ppm(PCIQXLDevice *qxl)
+{
+ if (qxl->ppm_save_filename) {
+ ppm_save(qxl->ppm_save_filename, qxl->ssd.ds->surface);
+ free(qxl->ppm_save_filename);
+ qxl->ppm_save_filename = NULL;
+ }
+}
+
void qxl_render_update(PCIQXLDevice *qxl)
{
VGACommonState *vga = &qxl->vga;
@@ -139,6 +148,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
dirty[i].right - dirty[i].left,
dirty[i].bottom - dirty[i].top);
}
+ qxl_save_ppm(qxl);
}
static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
diff --git a/hw/qxl.c b/hw/qxl.c
index bd540c0..de93efa 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1462,8 +1462,8 @@ static void qxl_hw_screen_dump(void *opaque, const char *filename)
switch (qxl->mode) {
case QXL_MODE_COMPAT:
case QXL_MODE_NATIVE:
+ qxl->ppm_save_filename = qemu_strdup(filename);
qxl_render_update(qxl);
- ppm_save(filename, qxl->ssd.ds->surface);
break;
case QXL_MODE_VGA:
vga->screen_dump(vga, filename);
diff --git a/hw/qxl.h b/hw/qxl.h
index 064311a..2c7f94a 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -32,6 +32,8 @@ typedef struct PCIQXLDevice {
int32_t num_memslots;
int32_t num_surfaces;
+ char *ppm_save_filename;
+
#if SPICE_INTERFACE_QXL_MINOR >= 1
uint32_t current_async;
QemuMutex async_lock;
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl-render: split out qxl_render_update_dirty_rectangles
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (12 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: qxl_render_update: nop if \!ssd.running Alon Levy
` (6 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
will later be reused from surface_updated callback when compiling against a
newer spice-server.
---
hw/qxl-render.c | 37 ++++++++++++++++++++++---------------
1 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index e64b646..d70373d 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -79,12 +79,32 @@ static void qxl_save_ppm(PCIQXLDevice *qxl)
}
}
+static void qxl_render_update_dirty_rectangles(PCIQXLDevice *qxl, QXLRect *dirty, uint32_t num_dirty)
+{
+ VGACommonState *vga = &qxl->vga;
+ int i;
+
+ dprint(qxl, 3, "%s: %d\n", __FUNCTION__, num_dirty);
+ for (i = 0; i < num_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);
+ }
+ qxl_save_ppm(qxl);
+}
+
void qxl_render_update(PCIQXLDevice *qxl)
{
VGACommonState *vga = &qxl->vga;
QXLRect dirty[32], update;
void *ptr;
- int i;
if (qxl->guest_primary.resized) {
qxl->guest_primary.resized = 0;
@@ -135,20 +155,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
memset(dirty, 0, sizeof(dirty));
qxl_spice_update_area(qxl, 0, &update,
dirty, ARRAY_SIZE(dirty), 1);
-
- 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);
- }
- qxl_save_ppm(qxl);
+ qxl_render_update_dirty_rectangles(qxl, dirty, ARRAY_SIZE(dirty));
}
static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl-render: qxl_render_update: nop if \!ssd.running
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (13 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: split out qxl_render_update_dirty_rectangles Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete Alon Levy
` (5 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
---
hw/qxl-render.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index d70373d..f440fe8 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -106,6 +106,10 @@ void qxl_render_update(PCIQXLDevice *qxl)
QXLRect dirty[32], update;
void *ptr;
+ if (!qxl->ssd.running) {
+ return;
+ }
+
if (qxl->guest_primary.resized) {
qxl->guest_primary.resized = 0;
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (14 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: qxl_render_update: nop if \!ssd.running Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-13 7:51 ` Gerd Hoffmann
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort) Alon Levy
` (4 subsequent siblings)
20 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
So now there are two implementations chosen based on QXL_INTERFACE_MINOR:
* old (spice qxl minor == 0) - use update_area, no change.
* new:
1. keep an array of updated rectangles (ssd.dirty_rects)
2. update it on callback (realloc)
3. render the current one before issuing a new update_area_async
---
hw/qxl-render.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
hw/qxl.c | 12 ++++++++++++
hw/qxl.h | 2 ++
ui/spice-display.h | 4 ++++
4 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index f440fe8..4862c35 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -103,8 +103,11 @@ static void qxl_render_update_dirty_rectangles(PCIQXLDevice *qxl, QXLRect *dirty
void qxl_render_update(PCIQXLDevice *qxl)
{
VGACommonState *vga = &qxl->vga;
- QXLRect dirty[32], update;
+ QXLRect update;
void *ptr;
+#if SPICE_INTERFACE_QXL_MINOR < 1
+ QXLRect dirty[32];
+#endif
if (!qxl->ssd.running) {
return;
@@ -112,7 +115,13 @@ void qxl_render_update(PCIQXLDevice *qxl)
if (qxl->guest_primary.resized) {
qxl->guest_primary.resized = 0;
-
+ qemu_mutex_lock(&qxl->ssd.lock);
+ if (qxl->ssd.num_dirty_rects > 0) {
+ free(qxl->ssd.dirty_rects);
+ qxl->ssd.dirty_rects = NULL;
+ qxl->ssd.num_dirty_rects = 0;
+ }
+ qemu_mutex_unlock(&qxl->ssd.lock);
if (qxl->guest_primary.flipped) {
qemu_free(qxl->guest_primary.flipped);
qxl->guest_primary.flipped = NULL;
@@ -146,6 +155,19 @@ void qxl_render_update(PCIQXLDevice *qxl)
dpy_resize(vga->ds);
}
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ /* render rectangles from last update_area_async */
+ qemu_mutex_lock(&qxl->ssd.lock);
+ if (qxl->ssd.num_dirty_rects > 0) {
+ qxl_render_update_dirty_rectangles(qxl, qxl->ssd.dirty_rects,
+ qxl->ssd.num_dirty_rects);
+ free(qxl->ssd.dirty_rects);
+ qxl->ssd.dirty_rects = NULL;
+ qxl->ssd.num_dirty_rects = 0;
+ }
+ qemu_mutex_unlock(&qxl->ssd.lock);
+#endif
+
if (!qxl->guest_primary.commands) {
return;
}
@@ -156,11 +178,33 @@ void qxl_render_update(PCIQXLDevice *qxl)
update.top = 0;
update.bottom = qxl->guest_primary.surface.height;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ /* do a new update_area */
+ qxl_spice_update_area_async(qxl, 0, &update, 1, 1);
+#else
memset(dirty, 0, sizeof(dirty));
qxl_spice_update_area(qxl, 0, &update,
dirty, ARRAY_SIZE(dirty), 1);
qxl_render_update_dirty_rectangles(qxl, dirty, ARRAY_SIZE(dirty));
+#endif
+}
+
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+void qxl_render_primary_updated(PCIQXLDevice *qxl, QXLRect *dirty,
+ uint32_t num_dirty)
+{
+ if (num_dirty == 0) {
+ return;
+ }
+ qemu_mutex_lock(&qxl->ssd.lock);
+ qxl->ssd.num_dirty_rects += num_dirty;
+ qxl->ssd.dirty_rects = qemu_realloc(qxl->ssd.dirty_rects,
+ sizeof(QXLRect) * qxl->ssd.num_dirty_rects);
+ memcpy(&qxl->ssd.dirty_rects[qxl->ssd.num_dirty_rects - num_dirty],
+ dirty, num_dirty * sizeof(QXLRect));
+ qemu_mutex_unlock(&qxl->ssd.lock);
}
+#endif /* SPICE_INTERFACE_QXL_MINOR >= 1 */
static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor)
{
diff --git a/hw/qxl.c b/hw/qxl.c
index de93efa..8a9463e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -780,6 +780,17 @@ static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
}
+static void interface_update_area_complete(QXLInstance *sin,
+ uint32_t surface_id, struct QXLRect *updated_rects,
+ uint32_t num_updated_rects)
+{
+ PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
+
+ dprint(qxl, 3, "%s: %d\n", __FUNCTION__, surface_id);
+ if (surface_id == 0) {
+ qxl_render_primary_updated(qxl, updated_rects, num_updated_rects);
+ }
+}
#endif
static const QXLInterface qxl_interface = {
@@ -803,6 +814,7 @@ static const QXLInterface qxl_interface = {
.flush_resources = interface_flush_resources,
#if SPICE_INTERFACE_QXL_MINOR >= 1
.async_complete = interface_async_complete,
+ .update_area_complete = interface_update_area_complete,
#endif
};
diff --git a/hw/qxl.h b/hw/qxl.h
index 2c7f94a..fad01c6 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -139,4 +139,6 @@ 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_render_primary_updated(PCIQXLDevice *qxl, QXLRect *dirty,
+ uint32_t num_dirty);
#endif
diff --git a/ui/spice-display.h b/ui/spice-display.h
index d24cca9..115aae5 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -65,6 +65,10 @@ struct SimpleSpiceDisplay {
int notify;
int running;
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ QXLRect *dirty_rects;
+ uint32_t num_dirty_rects;
+#endif
/*
* All struct members below this comment can be accessed from
* both spice server and qemu (iothread) context and any access
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort)
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (15 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-13 7:54 ` Gerd Hoffmann
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: only disallow specific io's in vga mode Alon Levy
` (3 subsequent siblings)
20 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
This can happen if there is an interface_get_command issued when
the server has been stopped. easy to trigger - do stop/cont a few
times (three seem to be enough).
The "solution" of ignoring the request is bad, but better then aborting
and a real solution would probably be in spice to not call get_command
in the first place.
---
hw/qxl.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 8a9463e..0585f02 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1406,7 +1406,10 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
uint32_t old_pending;
uint32_t le_events = cpu_to_le32(events);
- assert(d->ssd.running);
+ if (!d->ssd.running) {
+ fprintf(stderr, "qxl: not sending interrupt %d while stopped\n", events);
+ return;
+ }
old_pending = __sync_fetch_and_or(&d->ram->int_pending, le_events);
if ((old_pending & le_events) == le_events) {
return;
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: only disallow specific io's in vga mode
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (16 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort) Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
` (2 subsequent siblings)
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Since the driver is still in operation even after moving to UNDEFINED, i.e.
by destroying primary in any way.
---
hw/qxl.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 0585f02..1d6acce 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1175,8 +1175,9 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
case QXL_IO_LOG:
break;
default:
- if (d->mode == QXL_MODE_NATIVE || d->mode == QXL_MODE_COMPAT)
+ if (d->mode != QXL_MODE_VGA) {
break;
+ }
dprint(d, 1, "%s: unexpected port 0x%x (%s) in vga mode\n",
__FUNCTION__, io_port, io_port_to_string(io_port));
/* be nice to buggy guest drivers */
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (17 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: only disallow specific io's in vga mode Alon Levy
@ 2011-07-12 13:55 ` Alon Levy
2011-07-13 6:43 ` [Qemu-devel] [PATCHv3] async + suspend reworked Gerd Hoffmann
2011-07-13 7:11 ` Gerd Hoffmann
20 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-12 13:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel
Add two new IOs.
QXL_IO_FLUSH_SURFACES - equivalent to update area for all surfaces, used
to reduce vmexits from NumSurfaces to 1 on guest S3, S4 and resolution change (windows
driver implementation is such that this is done on each of those occasions).
QXL_IO_FLUSH_RELEASE - used to ensure anything on last_release is put on the release ring
for the client to free.
Cc: Yonit Halperin <yhalperi@redhat.com>
---
hw/qxl.c | 32 ++++++++++++++++++++++++++++++++
1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/hw/qxl.c b/hw/qxl.c
index 1d6acce..a9cc1a3 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -181,6 +181,13 @@ static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
}
}
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
+{
+ qxl->ssd.worker->flush_surfaces_async(qxl->ssd.worker, 0);
+}
+#endif
+
void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
uint32_t count)
{
@@ -1195,6 +1202,7 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
case QXL_IO_DESTROY_PRIMARY_ASYNC:
case QXL_IO_DESTROY_SURFACE_ASYNC:
case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
+ case QXL_IO_FLUSH_SURFACES_ASYNC:
#if SPICE_INTERFACE_QXL_MINOR < 1
fprintf(stderr, "qxl: error: async not supported by libspice but guest driver used it\n");
return;
@@ -1322,6 +1330,30 @@ static void ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
qxl_spice_destroy_surface_wait(d, val, async);
break;
+ case QXL_IO_FLUSH_RELEASE: {
+ QXLReleaseRing *ring = &d->ram->release_ring;
+ if (ring->prod - ring->cons + 1 == ring->num_items) {
+ fprintf(stderr,
+ "ERROR: no flush, full release ring [p%d,%dc]\n",
+ ring->prod, ring->cons);
+ }
+ qxl_push_free_res(d, 1 /* flush */);
+ dprint(d, 1, "QXL_IO_FLUSH_RELEASE exit (%s, s#=%d, res#=%d,%p)\n",
+ qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+ d->num_free_res, d->last_release);
+ break;
+ }
+ case QXL_IO_FLUSH_SURFACES_ASYNC:
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+ dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) (%s, s#=%d, res#=%d)\n",
+ val, qxl_mode_to_string(d->mode), d->guest_surfaces.count,
+ d->num_free_res);
+ qxl_spice_flush_surfaces_async(d);
+#else
+ dprint(d, 1, "QXL_IO_FLUSH_SURFACES_ASYNC (%d) ignored, too old spice server\n",
+ val);
+#endif
+ break;
case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
case QXL_IO_DESTROY_ALL_SURFACES:
d->mode = QXL_MODE_UNDEFINED;
--
1.7.6
^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (18 preceding siblings ...)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
@ 2011-07-13 6:43 ` Gerd Hoffmann
2011-07-13 8:51 ` Alon Levy
2011-07-13 7:11 ` Gerd Hoffmann
20 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 6:43 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
On 07/12/11 15:55, Alon Levy wrote:
> v2->v3:
> builds correctly with older and newer spice, and runs with older and newer qxl driver.
> fixed update_area_async to not use QXLRect on stack
> qxl-render updated to work with update_area_async correctly
> reverted change to update_area api - update_area still returns dirty
> rects array
Hmm? No patch numbers in the subject? That makes the patch order
unclear ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area Alon Levy
@ 2011-07-13 6:56 ` Gerd Hoffmann
2011-07-13 9:29 ` Alon Levy
0 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 6:56 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
Hi,
> - QXLRect update = d->ram->update_area;
> qxl_spice_update_area(d, d->ram->update_surface,
> -&update, NULL, 0, 0);
> +&d->ram->update_area, NULL, 0, 1);
No, -ESECURITY.
With this in place the guest can change the update rect while
spice-server is working with it.
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm Alon Levy
@ 2011-07-13 7:10 ` Gerd Hoffmann
2011-07-13 9:29 ` Alon Levy
2011-07-13 10:50 ` Daniel P. Berrange
0 siblings, 2 replies; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 7:10 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
On 07/12/11 15:55, Alon Levy wrote:
> Later the save will happen asynchronously on surface_updated callback.
Hmm. I can see why you are doing that. It makes the file being written
*after* the monitor command finishes though, which I think we should avoid.
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
` (19 preceding siblings ...)
2011-07-13 6:43 ` [Qemu-devel] [PATCHv3] async + suspend reworked Gerd Hoffmann
@ 2011-07-13 7:11 ` Gerd Hoffmann
2011-07-13 9:05 ` Alon Levy
20 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 7:11 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
On 07/12/11 15:55, Alon Levy wrote:
> v2->v3:
> builds correctly with older and newer spice, and runs with older and newer qxl driver.
> fixed update_area_async to not use QXLRect on stack
> qxl-render updated to work with update_area_async correctly
> reverted change to update_area api - update_area still returns dirty
> rects array
Please run scripts/checkpatch.pl on the patches (and fix the style
stuff), it has a bunch of complains ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete Alon Levy
@ 2011-07-13 7:51 ` Gerd Hoffmann
2011-07-13 9:30 ` Alon Levy
0 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 7:51 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
Hi,
> +void qxl_render_primary_updated(PCIQXLDevice *qxl, QXLRect *dirty,
> + uint32_t num_dirty);
> @@ -65,6 +65,10 @@ struct SimpleSpiceDisplay {
> int notify;
> int running;
>
> +#if SPICE_INTERFACE_QXL_MINOR>= 1
> + QXLRect *dirty_rects;
> + uint32_t num_dirty_rects;
> +#endif
Why do you put this into SimpleSpiceDisplay instead of PCIQXLState?
I also wouldn't #ifdef the struct elements to reduce the #ifdef clutter.
#ifdefs should only be there in case the code wouldn't compile without
them.
Additionally you can fill these struct elements in sync mode too and
have qxl_render_primary_updated pick up the rectangles from the struct
instead of getting them passed in as arguments, thereby reducing the
code differences between sync and async mode.
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort)
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort) Alon Levy
@ 2011-07-13 7:54 ` Gerd Hoffmann
2011-07-13 9:17 ` Alon Levy
0 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 7:54 UTC (permalink / raw)
To: Alon Levy; +Cc: qemu-devel
Hi,
> The "solution" of ignoring the request is bad, but better then aborting
> and a real solution would probably be in spice to not call get_command
> in the first place.
Isn't the plan to fix spice-server this way?
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-13 6:43 ` [Qemu-devel] [PATCHv3] async + suspend reworked Gerd Hoffmann
@ 2011-07-13 8:51 ` Alon Levy
2011-07-13 10:45 ` Gerd Hoffmann
0 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-13 8:51 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 08:43:57AM +0200, Gerd Hoffmann wrote:
> On 07/12/11 15:55, Alon Levy wrote:
> >v2->v3:
> > builds correctly with older and newer spice, and runs with older and newer qxl driver.
> > fixed update_area_async to not use QXLRect on stack
> > qxl-render updated to work with update_area_async correctly
> > reverted change to update_area api - update_area still returns dirty
> > rects array
>
> Hmm? No patch numbers in the subject? That makes the patch order
> unclear ...
>
Sorry, I don't understand what you mean? the comments are general, they don't
apply to a specific patch out of the series. I basically do a diff against the last
series (the tip) and document.
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-13 7:11 ` Gerd Hoffmann
@ 2011-07-13 9:05 ` Alon Levy
0 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 9:05 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 09:11:35AM +0200, Gerd Hoffmann wrote:
> On 07/12/11 15:55, Alon Levy wrote:
> >v2->v3:
> > builds correctly with older and newer spice, and runs with older and newer qxl driver.
> > fixed update_area_async to not use QXLRect on stack
> > qxl-render updated to work with update_area_async correctly
> > reverted change to update_area api - update_area still returns dirty
> > rects array
>
> Please run scripts/checkpatch.pl on the patches (and fix the style
> stuff), it has a bunch of complains ...
ok, my bad.
>
> cheers,
> Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort)
2011-07-13 7:54 ` Gerd Hoffmann
@ 2011-07-13 9:17 ` Alon Levy
0 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 9:17 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 09:54:55AM +0200, Gerd Hoffmann wrote:
> Hi,
>
> >The "solution" of ignoring the request is bad, but better then aborting
> >and a real solution would probably be in spice to not call get_command
> >in the first place.
>
> Isn't the plan to fix spice-server this way?
I can't reproduce this right now, I'll drop the patch. I do see spice-server
is fixed in red_process_commands to not call get_command on stopped, so I probably
used an unpatched server when this happened.
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 7:10 ` Gerd Hoffmann
@ 2011-07-13 9:29 ` Alon Levy
2011-07-13 10:41 ` Gerd Hoffmann
2011-07-13 10:50 ` Daniel P. Berrange
1 sibling, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-13 9:29 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> On 07/12/11 15:55, Alon Levy wrote:
> >Later the save will happen asynchronously on surface_updated callback.
>
> Hmm. I can see why you are doing that. It makes the file being
> written *after* the monitor command finishes though, which I think
> we should avoid.
I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area
2011-07-13 6:56 ` Gerd Hoffmann
@ 2011-07-13 9:29 ` Alon Levy
0 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 9:29 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 08:56:27AM +0200, Gerd Hoffmann wrote:
> Hi,
>
> >- QXLRect update = d->ram->update_area;
> > qxl_spice_update_area(d, d->ram->update_surface,
> >-&update, NULL, 0, 0);
> >+&d->ram->update_area, NULL, 0, 1);
>
> No, -ESECURITY.
>
> With this in place the guest can change the update rect while
> spice-server is working with it.
>
ok, will drop.
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete
2011-07-13 7:51 ` Gerd Hoffmann
@ 2011-07-13 9:30 ` Alon Levy
0 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 9:30 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 09:51:14AM +0200, Gerd Hoffmann wrote:
> Hi,
>
> >+void qxl_render_primary_updated(PCIQXLDevice *qxl, QXLRect *dirty,
> >+ uint32_t num_dirty);
>
> >@@ -65,6 +65,10 @@ struct SimpleSpiceDisplay {
> > int notify;
> > int running;
> >
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+ QXLRect *dirty_rects;
> >+ uint32_t num_dirty_rects;
> >+#endif
>
> Why do you put this into SimpleSpiceDisplay instead of PCIQXLState?
>
> I also wouldn't #ifdef the struct elements to reduce the #ifdef
> clutter. #ifdefs should only be there in case the code wouldn't
> compile without them.
>
> Additionally you can fill these struct elements in sync mode too and
> have qxl_render_primary_updated pick up the rectangles from the
> struct instead of getting them passed in as arguments, thereby
> reducing the code differences between sync and async mode.
>
ok, will do.
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 9:29 ` Alon Levy
@ 2011-07-13 10:41 ` Gerd Hoffmann
2011-07-13 10:54 ` Daniel P. Berrange
` (2 more replies)
0 siblings, 3 replies; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 10:41 UTC (permalink / raw)
To: qemu-devel, Luiz Capitulino
On 07/13/11 11:29, Alon Levy wrote:
> On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
>> On 07/12/11 15:55, Alon Levy wrote:
>>> Later the save will happen asynchronously on surface_updated callback.
>>
>> Hmm. I can see why you are doing that. It makes the file being
>> written *after* the monitor command finishes though, which I think
>> we should avoid.
>
> I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
Not sure. Luiz, do we have async monitor commands meanwhile?
Background: screendump for qxl vga can take a while as the spice-server
might have to render everything first ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-13 8:51 ` Alon Levy
@ 2011-07-13 10:45 ` Gerd Hoffmann
2011-07-13 11:24 ` Alon Levy
0 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 10:45 UTC (permalink / raw)
To: qemu-devel
On 07/13/11 10:51, Alon Levy wrote:
> On Wed, Jul 13, 2011 at 08:43:57AM +0200, Gerd Hoffmann wrote:
>> On 07/12/11 15:55, Alon Levy wrote:
>>> v2->v3:
>>> builds correctly with older and newer spice, and runs with older and newer qxl driver.
>>> fixed update_area_async to not use QXLRect on stack
>>> qxl-render updated to work with update_area_async correctly
>>> reverted change to update_area api - update_area still returns dirty
>>> rects array
>>
>> Hmm? No patch numbers in the subject? That makes the patch order
>> unclear ...
>>
>
> Sorry, I don't understand what you mean?
The subject line of the patches says just [patch] not [patch
index/total] which makes it hard to figure the patch ordering ...
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 7:10 ` Gerd Hoffmann
2011-07-13 9:29 ` Alon Levy
@ 2011-07-13 10:50 ` Daniel P. Berrange
1 sibling, 0 replies; 47+ messages in thread
From: Daniel P. Berrange @ 2011-07-13 10:50 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: Alon Levy, qemu-devel
On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> On 07/12/11 15:55, Alon Levy wrote:
> >Later the save will happen asynchronously on surface_updated callback.
>
> Hmm. I can see why you are doing that. It makes the file being
> written *after* the monitor command finishes though, which I think
> we should avoid.
Yes, that will very likely break applications using the screenshot
command which expect to be able to load/read the whole image
immediately after the screnshot command returns from the monitor
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 10:41 ` Gerd Hoffmann
@ 2011-07-13 10:54 ` Daniel P. Berrange
2011-07-13 11:29 ` Alon Levy
2011-07-13 12:32 ` Luiz Capitulino
2 siblings, 0 replies; 47+ messages in thread
From: Daniel P. Berrange @ 2011-07-13 10:54 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Luiz Capitulino
On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> On 07/13/11 11:29, Alon Levy wrote:
> >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> >>On 07/12/11 15:55, Alon Levy wrote:
> >>>Later the save will happen asynchronously on surface_updated callback.
> >>
> >>Hmm. I can see why you are doing that. It makes the file being
> >>written *after* the monitor command finishes though, which I think
> >>we should avoid.
> >
> >I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
>
> Not sure. Luiz, do we have async monitor commands meanwhile?
>
> Background: screendump for qxl vga can take a while as the
> spice-server might have to render everything first ...
Another option would be for the screenshot command to allow a
pre-opened FD to be passed in.
So libvirt would create a pipe(2) pass the write end of the
pipe to the 'screenshot' command. The screenshot command
can return from the monitor the moment it has validated that
it can perform the screenshot. QEMU can then write data to the
pipe in the background. libvirt (or equiv app) can likewise
safely read data out of the pipe as it becomes available without
any race condition.
This kind of approach would actually fit better with what libvirt
wants from a screenshot command, because we don't really want to
have the screenshot saved to disk at all. Our API just provides
applications with a stream to data the screenshot data from.
Currently we create a temporary file, save the screenshot to it,
and then immediately unlink it and stream data back to the app
from the deleted file. If we could just use a anonymous pipe it
would be much nicer
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] async + suspend reworked
2011-07-13 10:45 ` Gerd Hoffmann
@ 2011-07-13 11:24 ` Alon Levy
0 siblings, 0 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 11:24 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, Jul 13, 2011 at 12:45:24PM +0200, Gerd Hoffmann wrote:
> On 07/13/11 10:51, Alon Levy wrote:
> >On Wed, Jul 13, 2011 at 08:43:57AM +0200, Gerd Hoffmann wrote:
> >>On 07/12/11 15:55, Alon Levy wrote:
> >>>v2->v3:
> >>> builds correctly with older and newer spice, and runs with older and newer qxl driver.
> >>> fixed update_area_async to not use QXLRect on stack
> >>> qxl-render updated to work with update_area_async correctly
> >>> reverted change to update_area api - update_area still returns dirty
> >>> rects array
> >>
> >>Hmm? No patch numbers in the subject? That makes the patch order
> >>unclear ...
> >>
> >
> >Sorry, I don't understand what you mean?
>
> The subject line of the patches says just [patch] not [patch
> index/total] which makes it hard to figure the patch ordering ...
oh, right, forgot -N.
>
> cheers,
> Gerd
>
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 10:41 ` Gerd Hoffmann
2011-07-13 10:54 ` Daniel P. Berrange
@ 2011-07-13 11:29 ` Alon Levy
2011-07-13 11:46 ` Gerd Hoffmann
2011-07-13 12:33 ` Luiz Capitulino
2011-07-13 12:32 ` Luiz Capitulino
2 siblings, 2 replies; 47+ messages in thread
From: Alon Levy @ 2011-07-13 11:29 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Luiz Capitulino
On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> On 07/13/11 11:29, Alon Levy wrote:
> >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> >>On 07/12/11 15:55, Alon Levy wrote:
> >>>Later the save will happen asynchronously on surface_updated callback.
> >>
> >>Hmm. I can see why you are doing that. It makes the file being
> >>written *after* the monitor command finishes though, which I think
> >>we should avoid.
> >
> >I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
>
> Not sure. Luiz, do we have async monitor commands meanwhile?
>
> Background: screendump for qxl vga can take a while as the
> spice-server might have to render everything first ...
I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
ugly. Also I guess what Daniel described is possible, but it changes the usage of screendump
even more. Is turning do_screen_dump to async viable? I think I'll work on it.
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 11:29 ` Alon Levy
@ 2011-07-13 11:46 ` Gerd Hoffmann
2011-07-13 12:39 ` Luiz Capitulino
2011-07-13 12:33 ` Luiz Capitulino
1 sibling, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 11:46 UTC (permalink / raw)
To: qemu-devel, Luiz Capitulino
Hi,
> I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
> ugly. Also I guess what Daniel described is possible, but it changes the usage of screendump
> even more. Is turning do_screen_dump to async viable? I think I'll work on it.
Daniel's suggestion is a nice option which goes on top ;)
The filename-based screendump has to continue working for backward
compatibility reasons.
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 10:41 ` Gerd Hoffmann
2011-07-13 10:54 ` Daniel P. Berrange
2011-07-13 11:29 ` Alon Levy
@ 2011-07-13 12:32 ` Luiz Capitulino
2011-07-13 13:45 ` Gerd Hoffmann
2 siblings, 1 reply; 47+ messages in thread
From: Luiz Capitulino @ 2011-07-13 12:32 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, 13 Jul 2011 12:41:48 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 07/13/11 11:29, Alon Levy wrote:
> > On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> >> On 07/12/11 15:55, Alon Levy wrote:
> >>> Later the save will happen asynchronously on surface_updated callback.
> >>
> >> Hmm. I can see why you are doing that. It makes the file being
> >> written *after* the monitor command finishes though, which I think
> >> we should avoid.
> >
> > I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
>
> Not sure. Luiz, do we have async monitor commands meanwhile?
Not yet, this is a QAPI feature that should land soon, but it's not
available yet.
>
> Background: screendump for qxl vga can take a while as the spice-server
> might have to render everything first ...
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 11:29 ` Alon Levy
2011-07-13 11:46 ` Gerd Hoffmann
@ 2011-07-13 12:33 ` Luiz Capitulino
2011-07-13 12:56 ` Alon Levy
1 sibling, 1 reply; 47+ messages in thread
From: Luiz Capitulino @ 2011-07-13 12:33 UTC (permalink / raw)
To: Alon Levy; +Cc: Gerd Hoffmann, qemu-devel
On Wed, 13 Jul 2011 14:29:16 +0300
Alon Levy <alevy@redhat.com> wrote:
> On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> > On 07/13/11 11:29, Alon Levy wrote:
> > >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> > >>On 07/12/11 15:55, Alon Levy wrote:
> > >>>Later the save will happen asynchronously on surface_updated callback.
> > >>
> > >>Hmm. I can see why you are doing that. It makes the file being
> > >>written *after* the monitor command finishes though, which I think
> > >>we should avoid.
> > >
> > >I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
> >
> > Not sure. Luiz, do we have async monitor commands meanwhile?
> >
> > Background: screendump for qxl vga can take a while as the
> > spice-server might have to render everything first ...
>
> I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
> ugly.
IIRC, that interface doesn't work as expected and is going to be replaced
by the QAPI's one.
> Also I guess what Daniel described is possible, but it changes the usage of screendump
> even more. Is turning do_screen_dump to async viable? I think I'll work on it.
>
> >
> > cheers,
> > Gerd
> >
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 11:46 ` Gerd Hoffmann
@ 2011-07-13 12:39 ` Luiz Capitulino
0 siblings, 0 replies; 47+ messages in thread
From: Luiz Capitulino @ 2011-07-13 12:39 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel
On Wed, 13 Jul 2011 13:46:50 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Hi,
>
> > I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
> > ugly. Also I guess what Daniel described is possible, but it changes the usage of screendump
> > even more. Is turning do_screen_dump to async viable? I think I'll work on it.
>
> Daniel's suggestion is a nice option which goes on top ;)
Some months ago it was suggested that we returned the screenshot as a base64
string. But this might have issues if the screenshot is/becomes too large,
as we're imposing some limits on the json token size (I think it's 64MB today).
Daniel's idea seems to be a good one though.
>
> The filename-based screendump has to continue working for backward
> compatibility reasons.
>
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 12:33 ` Luiz Capitulino
@ 2011-07-13 12:56 ` Alon Levy
2011-07-13 13:15 ` Luiz Capitulino
0 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-13 12:56 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: Gerd Hoffmann, qemu-devel
On Wed, Jul 13, 2011 at 09:33:26AM -0300, Luiz Capitulino wrote:
> On Wed, 13 Jul 2011 14:29:16 +0300
> Alon Levy <alevy@redhat.com> wrote:
>
> > On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> > > On 07/13/11 11:29, Alon Levy wrote:
> > > >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> > > >>On 07/12/11 15:55, Alon Levy wrote:
> > > >>>Later the save will happen asynchronously on surface_updated callback.
> > > >>
> > > >>Hmm. I can see why you are doing that. It makes the file being
> > > >>written *after* the monitor command finishes though, which I think
> > > >>we should avoid.
> > > >
> > > >I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
> > >
> > > Not sure. Luiz, do we have async monitor commands meanwhile?
> > >
> > > Background: screendump for qxl vga can take a while as the
> > > spice-server might have to render everything first ...
> >
> > I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
> > ugly.
>
> IIRC, that interface doesn't work as expected and is going to be replaced
> by the QAPI's one.
In what way is in wrong? it seems to work fine here - monitor is blocked until
I call the callback, after which it returns. Didn't test the qmp part though.
>
> > Also I guess what Daniel described is possible, but it changes the usage of screendump
> > even more. Is turning do_screen_dump to async viable? I think I'll work on it.
> >
> > >
> > > cheers,
> > > Gerd
> > >
> >
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 12:56 ` Alon Levy
@ 2011-07-13 13:15 ` Luiz Capitulino
0 siblings, 0 replies; 47+ messages in thread
From: Luiz Capitulino @ 2011-07-13 13:15 UTC (permalink / raw)
To: Alon Levy; +Cc: Gerd Hoffmann, qemu-devel
On Wed, 13 Jul 2011 15:56:55 +0300
Alon Levy <alevy@redhat.com> wrote:
> On Wed, Jul 13, 2011 at 09:33:26AM -0300, Luiz Capitulino wrote:
> > On Wed, 13 Jul 2011 14:29:16 +0300
> > Alon Levy <alevy@redhat.com> wrote:
> >
> > > On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> > > > On 07/13/11 11:29, Alon Levy wrote:
> > > > >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> > > > >>On 07/12/11 15:55, Alon Levy wrote:
> > > > >>>Later the save will happen asynchronously on surface_updated callback.
> > > > >>
> > > > >>Hmm. I can see why you are doing that. It makes the file being
> > > > >>written *after* the monitor command finishes though, which I think
> > > > >>we should avoid.
> > > > >
> > > > >I think the simplest thing would be to add a specific cond for this - ppm_save_filename_cond. ok?
> > > >
> > > > Not sure. Luiz, do we have async monitor commands meanwhile?
> > > >
> > > > Background: screendump for qxl vga can take a while as the
> > > > spice-server might have to render everything first ...
> > >
> > > I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's becoming pretty
> > > ugly.
> >
> > IIRC, that interface doesn't work as expected and is going to be replaced
> > by the QAPI's one.
>
> In what way is in wrong? it seems to work fine here - monitor is blocked until
> I call the callback, after which it returns. Didn't test the qmp part though.
One problem is that the error is global and could be overwritten by other
handlers. Another problem is that there's no way for a client to kill an async
handler that got stuck, this could cause a client to wait for the handler
forever.
>
> >
> > > Also I guess what Daniel described is possible, but it changes the usage of screendump
> > > even more. Is turning do_screen_dump to async viable? I think I'll work on it.
> > >
> > > >
> > > > cheers,
> > > > Gerd
> > > >
> > >
> >
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 12:32 ` Luiz Capitulino
@ 2011-07-13 13:45 ` Gerd Hoffmann
2011-07-13 14:10 ` Alon Levy
0 siblings, 1 reply; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 13:45 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel
On 07/13/11 14:32, Luiz Capitulino wrote:
>> Not sure. Luiz, do we have async monitor commands meanwhile?
>
> Not yet, this is a QAPI feature that should land soon, but it's not
> available yet.
Hmm. Alon, is it an option to just leave the whole qxl-render stuff in
sync mode for now and convert it later? Or will that have bad
interactions with QXL_IO_UPDATE_AREA_ASYNC being used by the guest?
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 13:45 ` Gerd Hoffmann
@ 2011-07-13 14:10 ` Alon Levy
2011-07-13 14:25 ` Gerd Hoffmann
0 siblings, 1 reply; 47+ messages in thread
From: Alon Levy @ 2011-07-13 14:10 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, Luiz Capitulino
On Wed, Jul 13, 2011 at 03:45:16PM +0200, Gerd Hoffmann wrote:
> On 07/13/11 14:32, Luiz Capitulino wrote:
> >>Not sure. Luiz, do we have async monitor commands meanwhile?
> >
> >Not yet, this is a QAPI feature that should land soon, but it's not
> >available yet.
>
> Hmm. Alon, is it an option to just leave the whole qxl-render stuff
> in sync mode for now and convert it later? Or will that have bad
> interactions with QXL_IO_UPDATE_AREA_ASYNC being used by the guest?
>
It's not a problem. I do have a working version using async monitor command, but
I will gladly put it off if the monitor async is considered harmful.
> cheers,
> Gerd
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm
2011-07-13 14:10 ` Alon Levy
@ 2011-07-13 14:25 ` Gerd Hoffmann
0 siblings, 0 replies; 47+ messages in thread
From: Gerd Hoffmann @ 2011-07-13 14:25 UTC (permalink / raw)
To: Luiz Capitulino, qemu-devel
Hi,
>> Hmm. Alon, is it an option to just leave the whole qxl-render stuff
>> in sync mode for now and convert it later? Or will that have bad
>> interactions with QXL_IO_UPDATE_AREA_ASYNC being used by the guest?
>>
> It's not a problem. I do have a working version using async monitor command, but
> I will gladly put it off if the monitor async is considered harmful.
Good, so we know the libspice-server API is sane. Lets put it off for
now and resume later when QAPI and the other stuff is sorted, async i/o
commands and s3/s4 support are more important now.
cheers,
Gerd
^ permalink raw reply [flat|nested] 47+ messages in thread
end of thread, other threads:[~2011-07-13 14:25 UTC | newest]
Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-12 13:55 [Qemu-devel] [PATCHv3] async + suspend reworked Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add worker wrapper functions Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice: add qemu_spice_display_init_common Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: remove qxl_destroy_primary() Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] spice/qxl: move worker wrappers Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: fix surface tracking & locking Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add io_port_to_string Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: error handling fixes and cleanups Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: make qxl_guest_bug take variable arguments Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: bump pci rev Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: use QXL_REVISION_* Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: QXL_IO_UPDATE_AREA: pass ram->update_area directly to update_area Alon Levy
2011-07-13 6:56 ` Gerd Hoffmann
2011-07-13 9:29 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: async io support using new spice api Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm Alon Levy
2011-07-13 7:10 ` Gerd Hoffmann
2011-07-13 9:29 ` Alon Levy
2011-07-13 10:41 ` Gerd Hoffmann
2011-07-13 10:54 ` Daniel P. Berrange
2011-07-13 11:29 ` Alon Levy
2011-07-13 11:46 ` Gerd Hoffmann
2011-07-13 12:39 ` Luiz Capitulino
2011-07-13 12:33 ` Luiz Capitulino
2011-07-13 12:56 ` Alon Levy
2011-07-13 13:15 ` Luiz Capitulino
2011-07-13 12:32 ` Luiz Capitulino
2011-07-13 13:45 ` Gerd Hoffmann
2011-07-13 14:10 ` Alon Levy
2011-07-13 14:25 ` Gerd Hoffmann
2011-07-13 10:50 ` Daniel P. Berrange
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: split out qxl_render_update_dirty_rectangles Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: qxl_render_update: nop if \!ssd.running Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl-render: use update_area_async and update_area_complete Alon Levy
2011-07-13 7:51 ` Gerd Hoffmann
2011-07-13 9:30 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: qxl_send_events: ignore if stopped (instead of abort) Alon Levy
2011-07-13 7:54 ` Gerd Hoffmann
2011-07-13 9:17 ` Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: only disallow specific io's in vga mode Alon Levy
2011-07-12 13:55 ` [Qemu-devel] [PATCHv3] qxl: add QXL_IO_FLUSH_{SURFACES, RELEASE} for guest S3&S4 support Alon Levy
2011-07-13 6:43 ` [Qemu-devel] [PATCHv3] async + suspend reworked Gerd Hoffmann
2011-07-13 8:51 ` Alon Levy
2011-07-13 10:45 ` Gerd Hoffmann
2011-07-13 11:24 ` Alon Levy
2011-07-13 7:11 ` Gerd Hoffmann
2011-07-13 9:05 ` Alon Levy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.