[Qemu-devel] [PULL for-2.9 0/5] fixes for 2.9: vga, egl, cirrus, virtio-input.

[Qemu-devel] [PULL for-2.9 0/5] fixes for 2.9: vga, egl, cirrus, virtio-input.

[Gerd Hoffmann]

  Hi,

Bugfix collection for 2.9, ui, vga, input.

please pull,
  Gerd

The following changes since commit ea2afcf5b6727a577cf561fd8fe0d8c397ecc927:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-03-24 14:14:18 +0000)

are available in the git repository at:


  git://git.kraxel.org/qemu tags/pull-fixes-20170327-1

for you to fetch changes up to e5766eb40453b1d22815fc5482802688ff184006:

  vnc: fix reverse mode (2017-03-27 12:16:02 +0200)

----------------------------------------------------------------
fixes for 2.9: vga, egl, cirrus, virtio-input.

----------------------------------------------------------------
Gerd Hoffmann (3):
      cirrus: fix PUTPIXEL macro
      ui/egl-helpers: fix egl 1.5 display init
      vnc: fix reverse mode

Ladi Prosek (2):
      virtio-input: free event queue when finalizing
      virtio-input: fix eventq batching

 hw/display/cirrus_vga_rop2.h     |  4 +--
 hw/input/virtio-input.c          | 33 ++++++++++++-----------
 include/hw/virtio/virtio-input.h |  5 +++-
 ui/egl-helpers.c                 | 58 +++++++++++++++++++++++++++++++++++-----
 ui/vnc.c                         | 17 +++++++-----
 5 files changed, 85 insertions(+), 32 deletions(-)

[Qemu-devel] [PULL for-2.9 1/5] virtio-input: free event queue when finalizing

[Gerd Hoffmann]

From: Ladi Prosek <lprosek@redhat.com>

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 <lprosek@redhat.com>
Message-id: 1490365490-4854-2-git-send-email-lprosek@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 b678ee9..728832a 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)
 {
-- 
1.8.3.1

[Qemu-devel] [PULL for-2.9 2/5] virtio-input: fix eventq batching

[Gerd Hoffmann]

From: Ladi Prosek <lprosek@redhat.com>

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 <lprosek@redhat.com>
Message-id: 1490365490-4854-3-git-send-email-lprosek@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 728832a..0e42f0d 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 55db310..91df57e 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;
-- 
1.8.3.1

[Qemu-devel] [PULL for-2.9 3/5] cirrus: fix PUTPIXEL macro

[Gerd Hoffmann]

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 <dgilbert@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
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 b86bcd6..b208b73 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)
-- 
1.8.3.1

[Qemu-devel] [PULL for-2.9 4/5] ui/egl-helpers: fix egl 1.5 display init

[Gerd Hoffmann]

Unfortunaly switching to getPlatformDisplayEXT isn't as easy as
implemented by 0ea1523fb6703aa0dcd65e66b59e96fec028e60a.  See the
longish comment for the complete story.

Cc: Frediano Ziglio <fziglio@redhat.com>
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
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 584dd1b..b7b6b2e 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;
-- 
1.8.3.1

[Qemu-devel] [PULL for-2.9 5/5] vnc: fix reverse mode

[Gerd Hoffmann]

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 <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
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 6e93b88..821acdd 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);
-- 
1.8.3.1

Re: [Qemu-devel] [PULL for-2.9 0/5] fixes for 2.9: vga, egl, cirrus, virtio-input.

[Peter Maydell]

On 27 March 2017 at 14:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Bugfix collection for 2.9, ui, vga, input.
>
> please pull,
>   Gerd
>
> The following changes since commit ea2afcf5b6727a577cf561fd8fe0d8c397ecc927:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-03-24 14:14:18 +0000)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-fixes-20170327-1
>
> for you to fetch changes up to e5766eb40453b1d22815fc5482802688ff184006:
>
>   vnc: fix reverse mode (2017-03-27 12:16:02 +0200)
>
> ----------------------------------------------------------------
> fixes for 2.9: vga, egl, cirrus, virtio-input.
>
> ----------------------------------------------------------------
> Gerd Hoffmann (3):
>       cirrus: fix PUTPIXEL macro
>       ui/egl-helpers: fix egl 1.5 display init
>       vnc: fix reverse mode
>
> Ladi Prosek (2):
>       virtio-input: free event queue when finalizing
>       virtio-input: fix eventq batching

Applied, thanks.

-- PMM