From 0f5a15e40ad1512c70d1323a73c7b073e32e17ee Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 24 Mar 2017 15:24:49 +0100 Subject: virtio-input: free event queue when finalizing VirtIOInput.queue was never freed. This commit adds an explicit g_free to virtio_input_finalize and switches the allocation function from realloc to g_realloc in virtio_input_send. Signed-off-by: Ladi Prosek Message-id: 1490365490-4854-2-git-send-email-lprosek@redhat.com Signed-off-by: Gerd Hoffmann --- hw/input/virtio-input.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c index b678ee9f20..728832aa62 100644 --- a/hw/input/virtio-input.c +++ b/hw/input/virtio-input.c @@ -32,8 +32,8 @@ void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event) /* queue up events ... */ if (vinput->qindex == vinput->qsize) { vinput->qsize++; - vinput->queue = realloc(vinput->queue, vinput->qsize * - sizeof(virtio_input_event)); + vinput->queue = g_realloc(vinput->queue, vinput->qsize * + sizeof(virtio_input_event)); } vinput->queue[vinput->qindex++] = *event; @@ -272,6 +272,8 @@ static void virtio_input_finalize(Object *obj) QTAILQ_REMOVE(&vinput->cfg_list, cfg, node); g_free(cfg); } + + g_free(vinput->queue); } static void virtio_input_device_unrealize(DeviceState *dev, Error **errp) { -- cgit v1.2.3 From 57094547dfea4ce784923b8abb53ac3ab4e3961a Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 24 Mar 2017 15:24:50 +0100 Subject: virtio-input: fix eventq batching virtio_input_send buffers input events until it sees a SYNC. Then it either sends or drops the entire batch, depending on whether eventq has enough space available. The case to avoid here is partial sends where only part of the batch would get to the guest. Using virtqueue_get_avail_bytes to check the state of eventq was not correct. The queue may have a smaller number of larger buffers available so bytes may be enough but the batch would still not be possible to send, leading to the "Huh? No vq elem available" error. Instead of checking available bytes, this patch optimistically pops buffers from the queue and puts them back in case it runs out of space and the batch needs to be dropped. Signed-off-by: Ladi Prosek Message-id: 1490365490-4854-3-git-send-email-lprosek@redhat.com Signed-off-by: Gerd Hoffmann --- hw/input/virtio-input.c | 29 ++++++++++++++--------------- include/hw/virtio/virtio-input.h | 5 ++++- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c index 728832aa62..0e42f0d02c 100644 --- a/hw/input/virtio-input.c +++ b/hw/input/virtio-input.c @@ -22,7 +22,6 @@ void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event) { VirtQueueElement *elem; - unsigned have, need; int i, len; if (!vinput->active) { @@ -33,9 +32,9 @@ void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event) if (vinput->qindex == vinput->qsize) { vinput->qsize++; vinput->queue = g_realloc(vinput->queue, vinput->qsize * - sizeof(virtio_input_event)); + sizeof(vinput->queue[0])); } - vinput->queue[vinput->qindex++] = *event; + vinput->queue[vinput->qindex++].event = *event; /* ... until we see a report sync ... */ if (event->type != cpu_to_le16(EV_SYN) || @@ -44,24 +43,24 @@ void virtio_input_send(VirtIOInput *vinput, virtio_input_event *event) } /* ... then check available space ... */ - need = sizeof(virtio_input_event) * vinput->qindex; - virtqueue_get_avail_bytes(vinput->evt, &have, NULL, need, 0); - if (have < need) { - vinput->qindex = 0; - trace_virtio_input_queue_full(); - return; - } - - /* ... and finally pass them to the guest */ for (i = 0; i < vinput->qindex; i++) { elem = virtqueue_pop(vinput->evt, sizeof(VirtQueueElement)); if (!elem) { - /* should not happen, we've checked for space beforehand */ - fprintf(stderr, "%s: Huh? No vq elem available ...\n", __func__); + while (--i >= 0) { + virtqueue_unpop(vinput->evt, vinput->queue[i].elem, 0); + } + vinput->qindex = 0; + trace_virtio_input_queue_full(); return; } + vinput->queue[i].elem = elem; + } + + /* ... and finally pass them to the guest */ + for (i = 0; i < vinput->qindex; i++) { + elem = vinput->queue[i].elem; len = iov_from_buf(elem->in_sg, elem->in_num, - 0, vinput->queue+i, sizeof(virtio_input_event)); + 0, &vinput->queue[i].event, sizeof(virtio_input_event)); virtqueue_push(vinput->evt, elem, len); g_free(elem); } diff --git a/include/hw/virtio/virtio-input.h b/include/hw/virtio/virtio-input.h index 55db31087a..91df57eca4 100644 --- a/include/hw/virtio/virtio-input.h +++ b/include/hw/virtio/virtio-input.h @@ -62,7 +62,10 @@ struct VirtIOInput { VirtQueue *evt, *sts; char *serial; - virtio_input_event *queue; + struct { + virtio_input_event event; + VirtQueueElement *elem; + } *queue; uint32_t qindex, qsize; bool active; -- cgit v1.2.3 From db6cd4c855fd7a9e397bafd3361e66509fe47de0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Wed, 22 Mar 2017 08:38:23 +0100 Subject: cirrus: fix PUTPIXEL macro Should be "c" not "col". The macro is used with "col" as third parameter everywhere, so this tyops doesn't break something. Fixes: 026aeffcb4752054830ba203020ed6eb05bcaba8 Reported-by: Dr. David Alan Gilbert Signed-off-by: Gerd Hoffmann Reviewed-by: Dr. David Alan Gilbert Message-id: 1490168303-24588-1-git-send-email-kraxel@redhat.com --- hw/display/cirrus_vga_rop2.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/cirrus_vga_rop2.h b/hw/display/cirrus_vga_rop2.h index b86bcd6e09..b208b7348a 100644 --- a/hw/display/cirrus_vga_rop2.h +++ b/hw/display/cirrus_vga_rop2.h @@ -29,8 +29,8 @@ #elif DEPTH == 24 #define PUTPIXEL(s, a, c) do { \ ROP_OP(s, a, c); \ - ROP_OP(s, a + 1, (col >> 8)); \ - ROP_OP(s, a + 2, (col >> 16)); \ + ROP_OP(s, a + 1, (c >> 8)); \ + ROP_OP(s, a + 2, (c >> 16)); \ } while (0) #elif DEPTH == 32 #define PUTPIXEL(s, a, c) ROP_OP_32(s, a, c) -- cgit v1.2.3 From 8bce03e39328f0b3a6700efb9edb8b7d7aef629b Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Mon, 20 Mar 2017 09:04:02 +0100 Subject: ui/egl-helpers: fix egl 1.5 display init Unfortunaly switching to getPlatformDisplayEXT isn't as easy as implemented by 0ea1523fb6703aa0dcd65e66b59e96fec028e60a. See the longish comment for the complete story. Cc: Frediano Ziglio Suggested-by: Hans de Goede Signed-off-by: Gerd Hoffmann Message-id: 1489997042-1824-1-git-send-email-kraxel@redhat.com --- ui/egl-helpers.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c index 584dd1b04d..b7b6b2e3cc 100644 --- a/ui/egl-helpers.c +++ b/ui/egl-helpers.c @@ -192,6 +192,56 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, Window win) /* ---------------------------------------------------------------------- */ +/* + * Taken from glamor_egl.h from the Xorg xserver, which is MIT licensed + * + * Create an EGLDisplay from a native display type. This is a little quirky + * for a few reasons. + * + * 1: GetPlatformDisplayEXT and GetPlatformDisplay are the API you want to + * use, but have different function signatures in the third argument; this + * happens not to matter for us, at the moment, but it means epoxy won't alias + * them together. + * + * 2: epoxy 1.3 and earlier don't understand EGL client extensions, which + * means you can't call "eglGetPlatformDisplayEXT" directly, as the resolver + * will crash. + * + * 3: You can't tell whether you have EGL 1.5 at this point, because + * eglQueryString(EGL_VERSION) is a property of the display, which we don't + * have yet. So you have to query for extensions no matter what. Fortunately + * epoxy_has_egl_extension _does_ let you query for client extensions, so + * we don't have to write our own extension string parsing. + * + * 4. There is no EGL_KHR_platform_base to complement the EXT one, thus one + * needs to know EGL 1.5 is supported in order to use the eglGetPlatformDisplay + * function pointer. + * We can workaround this (circular dependency) by probing for the EGL 1.5 + * platform extensions (EGL_KHR_platform_gbm and friends) yet it doesn't seem + * like mesa will be able to advertise these (even though it can do EGL 1.5). + */ +static EGLDisplay qemu_egl_get_display(void *native) +{ + EGLDisplay dpy = EGL_NO_DISPLAY; + +#ifdef EGL_MESA_platform_gbm + /* In practise any EGL 1.5 implementation would support the EXT extension */ + if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) { + PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT = + (void *) eglGetProcAddress("eglGetPlatformDisplayEXT"); + if (getPlatformDisplayEXT) { + dpy = getPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, native, NULL); + } + } +#endif + + if (dpy == EGL_NO_DISPLAY) { + /* fallback */ + dpy = eglGetDisplay(native); + } + return dpy; +} + int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug) { static const EGLint conf_att_gl[] = { @@ -222,12 +272,8 @@ int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug) setenv("LIBGL_DEBUG", "verbose", true); } - egl_dbg("eglGetDisplay (dpy %p) ...\n", dpy); -#ifdef EGL_MESA_platform_gbm - qemu_egl_display = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA, dpy, NULL); -#else - qemu_egl_display = eglGetDisplay(dpy); -#endif + egl_dbg("qemu_egl_get_display (dpy %p) ...\n", dpy); + qemu_egl_display = qemu_egl_get_display(dpy); if (qemu_egl_display == EGL_NO_DISPLAY) { error_report("egl: eglGetDisplay failed"); return -1; -- cgit v1.2.3 From e5766eb40453b1d22815fc5482802688ff184006 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 14 Mar 2017 09:26:58 +0100 Subject: vnc: fix reverse mode vnc server in reverse mode (qemu -vnc localhost:$nr,reverse) interprets $nr as display number (i.e. with 5900 offset) in recent qemu versions. Historical and documented behavior is interpreting $nr as port number though. So we should bring code and documentation in line. Given that default listening port for viewers is 5500 the 5900 offset is pretty inconvinient, because it is simply impossible to connect to port 5500. So, lets fix the code not the docs. Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel P. Berrange Message-id: 1489480018-11443-1-git-send-email-kraxel@redhat.com --- ui/vnc.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 6e93b883b5..821acdd8b0 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -3401,6 +3401,7 @@ vnc_display_create_creds(bool x509, static int vnc_display_get_address(const char *addrstr, bool websocket, + bool reverse, int displaynum, int to, bool has_ipv4, @@ -3480,21 +3481,22 @@ static int vnc_display_get_address(const char *addrstr, inet->port = g_strdup(port); } } else { + int offset = reverse ? 0 : 5900; if (parse_uint_full(port, &baseport, 10) < 0) { error_setg(errp, "can't convert to a number: %s", port); goto cleanup; } if (baseport > 65535 || - baseport + 5900 > 65535) { + baseport + offset > 65535) { error_setg(errp, "port %s out of range", port); goto cleanup; } inet->port = g_strdup_printf( - "%d", (int)baseport + 5900); + "%d", (int)baseport + offset); if (to) { inet->has_to = true; - inet->to = to + 5900; + inet->to = to + offset; } } @@ -3516,6 +3518,7 @@ static int vnc_display_get_address(const char *addrstr, } static int vnc_display_get_addresses(QemuOpts *opts, + bool reverse, SocketAddress ***retsaddr, size_t *retnsaddr, SocketAddress ***retwsaddr, @@ -3555,7 +3558,7 @@ static int vnc_display_get_addresses(QemuOpts *opts, qemu_opt_iter_init(&addriter, opts, "vnc"); while ((addr = qemu_opt_iter_next(&addriter)) != NULL) { int rv; - rv = vnc_display_get_address(addr, false, 0, to, + rv = vnc_display_get_address(addr, false, reverse, 0, to, has_ipv4, has_ipv6, ipv4, ipv6, &saddr, errp); @@ -3580,7 +3583,7 @@ static int vnc_display_get_addresses(QemuOpts *opts, qemu_opt_iter_init(&addriter, opts, "websocket"); while ((addr = qemu_opt_iter_next(&addriter)) != NULL) { - if (vnc_display_get_address(addr, true, displaynum, to, + if (vnc_display_get_address(addr, true, reverse, displaynum, to, has_ipv4, has_ipv6, ipv4, ipv6, &wsaddr, errp) < 0) { @@ -3777,7 +3780,8 @@ void vnc_display_open(const char *id, Error **errp) return; } - if (vnc_display_get_addresses(opts, &saddr, &nsaddr, + reverse = qemu_opt_get_bool(opts, "reverse", false); + if (vnc_display_get_addresses(opts, reverse, &saddr, &nsaddr, &wsaddr, &nwsaddr, errp) < 0) { goto fail; } @@ -3803,7 +3807,6 @@ void vnc_display_open(const char *id, Error **errp) } } - reverse = qemu_opt_get_bool(opts, "reverse", false); lock_key_sync = qemu_opt_get_bool(opts, "lock-key-sync", true); key_delay_ms = qemu_opt_get_number(opts, "key-delay-ms", 1); sasl = qemu_opt_get_bool(opts, "sasl", false); -- cgit v1.2.3