[Qemu-devel] [PATCH 0/6] virtio-gpu: fixes and spice preparation

[Qemu-devel] [PATCH 0/6] virtio-gpu: fixes and spice preparation

[Gerd Hoffmann]

  Hi,

This patch series brings some minor opengl bug fixes and also
some virtio-gpu preparations for spice support.

please review,
  Gerd

Gerd Hoffmann (6):
  zap qemu_egl_has_ext in include/ui/egl-helpers.h
  console: block rendering until client is done
  virtio-gpu: fix memory leak in error path
  virtio-gpu: maintain command queue
  virtio-gpu: add support to enable/disable command processing
  virtio-gpu: block any rendering until client (ui) is done

 hw/display/virtio-gpu-3d.c     | 11 +++++--
 hw/display/virtio-gpu.c        | 71 +++++++++++++++++++++++++++++-------------
 hw/display/virtio-vga.c        | 10 ++++++
 include/hw/virtio/virtio-gpu.h |  4 +++
 include/ui/console.h           |  2 ++
 include/ui/egl-helpers.h       |  1 -
 ui/console.c                   | 10 ++++++
 7 files changed, 84 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/6] zap qemu_egl_has_ext in include/ui/egl-helpers.h

[Gerd Hoffmann]

Drop leftover prototype which sneaked in by mistake

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/egl-helpers.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
index 5ad5dc3..8c84398 100644
--- a/include/ui/egl-helpers.h
+++ b/include/ui/egl-helpers.h
@@ -11,6 +11,5 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, Window win);
 
 int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug);
 EGLContext qemu_egl_init_ctx(void);
-bool qemu_egl_has_ext(const char *haystack, const char *needle);
 
 #endif /* EGL_HELPERS_H */
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/6] console: block rendering until client is done

[Gerd Hoffmann]

Allow gl user interfaces to block display device gl rendering.
The ui code might want to do that in case it takes a little
longer to bring things to screen, for example because we'll
hand over a dma-buf to another process (spice will do that).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/console.h |  2 ++
 ui/console.c         | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/ui/console.h b/include/ui/console.h
index adac36d..12ad627 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -362,6 +362,7 @@ typedef struct GraphicHwOps {
     void (*text_update)(void *opaque, console_ch_t *text);
     void (*update_interval)(void *opaque, uint64_t interval);
     int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info);
+    void (*gl_block)(void *opaque, bool block);
 } GraphicHwOps;
 
 QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
@@ -374,6 +375,7 @@ void graphic_console_set_hwops(QemuConsole *con,
 void graphic_hw_update(QemuConsole *con);
 void graphic_hw_invalidate(QemuConsole *con);
 void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
+void graphic_hw_gl_block(QemuConsole *con, bool block);
 
 QemuConsole *qemu_console_lookup_by_index(unsigned int index);
 QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
diff --git a/ui/console.c b/ui/console.c
index 4b65c34..f0a4bd8 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -261,6 +261,16 @@ void graphic_hw_update(QemuConsole *con)
     }
 }
 
+void graphic_hw_gl_block(QemuConsole *con, bool block)
+{
+    if (!con) {
+        con = active_console;
+    }
+    if (con && con->hw_ops->gl_block) {
+        con->hw_ops->gl_block(con->hw, block);
+    }
+}
+
 void graphic_hw_invalidate(QemuConsole *con)
 {
     if (!con) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/6] virtio-gpu: fix memory leak in error path

[Gerd Hoffmann]

Found by Coverity Scan, buf not freed on error.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu-3d.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 28dccfd..d2af327 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -197,7 +197,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
         qemu_log_mask(LOG_GUEST_ERROR, "%s: size mismatch (%zd/%d)",
                       __func__, s, cs.size);
         cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
-        return;
+        goto out;
     }
 
     if (virtio_gpu_stats_enabled(g->conf)) {
@@ -207,6 +207,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
 
     virgl_renderer_submit_cmd(buf, cs.hdr.ctx_id, cs.size / 4);
 
+out:
     g_free(buf);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/6] virtio-gpu: maintain command queue

[Gerd Hoffmann]

We'll go take out the commands we receive out of the virt queue and put
them into a linked list, to decouple virtio queue handling from actual
command processing.

Also move cmd processing to new virtio_gpu_handle_ctrl func, so we can
easily kick it from different places.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu.c        | 60 ++++++++++++++++++++++++++----------------
 include/hw/virtio/virtio-gpu.h |  1 +
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 7e79a9c..4433c12 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -754,33 +754,21 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev, VirtQueue *vq)
     qemu_bh_schedule(g->cursor_bh);
 }
 
-static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+static void virtio_cpu_process_cmdq(VirtIOGPU *g)
 {
-    VirtIOGPU *g = VIRTIO_GPU(vdev);
     struct virtio_gpu_ctrl_command *cmd;
 
-    if (!virtio_queue_ready(vq)) {
-        return;
-    }
-
-#ifdef CONFIG_VIRGL
-    if (!g->renderer_inited && g->use_virgl_renderer) {
-        virtio_gpu_virgl_init(g);
-        g->renderer_inited = true;
-    }
-#endif
-
-    cmd = g_new(struct virtio_gpu_ctrl_command, 1);
-    while (virtqueue_pop(vq, &cmd->elem)) {
-        cmd->vq = vq;
-        cmd->error = 0;
-        cmd->finished = false;
-        if (virtio_gpu_stats_enabled(g->conf)) {
-            g->stats.requests++;
-        }
+    while (!QTAILQ_EMPTY(&g->cmdq)) {
+        cmd = QTAILQ_FIRST(&g->cmdq);
 
+        /* process command */
         VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
               g, cmd);
+        QTAILQ_REMOVE(&g->cmdq, cmd, next);
+        if (virtio_gpu_stats_enabled(g->conf)) {
+            g->stats.requests++;
+        }
+
         if (!cmd->finished) {
             QTAILQ_INSERT_TAIL(&g->fenceq, cmd, next);
             g->inflight++;
@@ -790,11 +778,38 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
                 }
                 fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
             }
-            cmd = g_new(struct virtio_gpu_ctrl_command, 1);
         }
     }
+}
+
+static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+    VirtIOGPU *g = VIRTIO_GPU(vdev);
+    struct virtio_gpu_ctrl_command *cmd;
+
+    if (!virtio_queue_ready(vq)) {
+        return;
+    }
+
+#ifdef CONFIG_VIRGL
+    if (!g->renderer_inited && g->use_virgl_renderer) {
+        virtio_gpu_virgl_init(g);
+        g->renderer_inited = true;
+    }
+#endif
+
+    cmd = g_new0(struct virtio_gpu_ctrl_command, 1);
+    while (virtqueue_pop(vq, &cmd->elem)) {
+        cmd->vq = vq;
+        cmd->error = 0;
+        cmd->finished = false;
+        QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
+        cmd = g_new0(struct virtio_gpu_ctrl_command, 1);
+    }
     g_free(cmd);
 
+    virtio_cpu_process_cmdq(g);
+
 #ifdef CONFIG_VIRGL
     if (g->use_virgl_renderer) {
         virtio_gpu_virgl_fence_poll(g);
@@ -920,6 +935,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
     g->ctrl_bh = qemu_bh_new(virtio_gpu_ctrl_bh, g);
     g->cursor_bh = qemu_bh_new(virtio_gpu_cursor_bh, g);
     QTAILQ_INIT(&g->reslist);
+    QTAILQ_INIT(&g->cmdq);
     QTAILQ_INIT(&g->fenceq);
 
     g->enabled_output_bitmask = 1;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 9b279d7..f7e7a52 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -94,6 +94,7 @@ typedef struct VirtIOGPU {
     DeviceState *qdev;
 
     QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
+    QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
     QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq;
 
     struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUT];
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/6] virtio-gpu: add support to enable/disable command processing

[Gerd Hoffmann]

So we can stop rendering for a while in case we have to.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu-3d.c     | 3 ++-
 hw/display/virtio-gpu.c        | 5 ++++-
 include/hw/virtio/virtio-gpu.h | 2 ++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index d2af327..65926b1 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -553,7 +553,8 @@ static void virtio_gpu_fence_poll(void *opaque)
     VirtIOGPU *g = opaque;
 
     virgl_renderer_poll();
-    if (g->inflight) {
+    virtio_cpu_process_cmdq(g);
+    if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
         timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
     }
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 4433c12..9087e63 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -754,7 +754,7 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev, VirtQueue *vq)
     qemu_bh_schedule(g->cursor_bh);
 }
 
-static void virtio_cpu_process_cmdq(VirtIOGPU *g)
+void virtio_cpu_process_cmdq(VirtIOGPU *g)
 {
     struct virtio_gpu_ctrl_command *cmd;
 
@@ -764,6 +764,9 @@ static void virtio_cpu_process_cmdq(VirtIOGPU *g)
         /* process command */
         VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
               g, cmd);
+        if (cmd->waiting) {
+            break;
+        }
         QTAILQ_REMOVE(&g->cmdq, cmd, next);
         if (virtio_gpu_stats_enabled(g->conf)) {
             g->stats.requests++;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f7e7a52..fdf0091 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -76,6 +76,7 @@ struct virtio_gpu_ctrl_command {
     VirtQueue *vq;
     struct virtio_gpu_ctrl_hdr cmd_hdr;
     uint32_t error;
+    bool waiting;
     bool finished;
     QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
 };
@@ -152,6 +153,7 @@ int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab,
                                   struct virtio_gpu_ctrl_command *cmd,
                                   struct iovec **iov);
 void virtio_gpu_cleanup_mapping_iov(struct iovec *iov, uint32_t count);
+void virtio_cpu_process_cmdq(VirtIOGPU *g);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/6] virtio-gpu: block any rendering until client (ui) is done

[Gerd Hoffmann]

Wire up gl_block callback, so ui code can request to stop
virtio-gpu rendering.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu-3d.c     |  5 +++++
 hw/display/virtio-gpu.c        |  8 ++++++++
 hw/display/virtio-vga.c        | 10 ++++++++++
 include/hw/virtio/virtio-gpu.h |  1 +
 4 files changed, 24 insertions(+)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 65926b1..321a959 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -382,6 +382,11 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
 {
     VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
 
+    cmd->waiting = g->renderer_blocked;
+    if (cmd->waiting) {
+        return;
+    }
+
     virgl_renderer_force_ctx_0();
     switch (cmd->cmd_hdr.type) {
     case VIRTIO_GPU_CMD_CTX_CREATE:
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 9087e63..09936c9 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -893,11 +893,19 @@ static int virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
     return 0;
 }
 
+static void virtio_gpu_gl_block(void *opaque, bool block)
+{
+    VirtIOGPU *g = opaque;
+
+    g->renderer_blocked = block;
+}
+
 const GraphicHwOps virtio_gpu_ops = {
     .invalidate = virtio_gpu_invalidate_display,
     .gfx_update = virtio_gpu_update_display,
     .text_update = virtio_gpu_text_update,
     .ui_info = virtio_gpu_ui_info,
+    .gl_block = virtio_gpu_gl_block,
 };
 
 static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index f7e539f..255d419 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -65,11 +65,21 @@ static int virtio_vga_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
     return -1;
 }
 
+static void virtio_vga_gl_block(void *opaque, bool block)
+{
+    VirtIOVGA *vvga = opaque;
+
+    if (virtio_gpu_ops.gl_block) {
+        virtio_gpu_ops.gl_block(&vvga->vdev, block);
+    }
+}
+
 static const GraphicHwOps virtio_vga_ops = {
     .invalidate = virtio_vga_invalidate_display,
     .gfx_update = virtio_vga_update_display,
     .text_update = virtio_vga_text_update,
     .ui_info = virtio_vga_ui_info,
+    .gl_block = virtio_vga_gl_block,
 };
 
 /* VGA device wrapper around PCI device around virtio GPU */
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index fdf0091..7249fcd 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -107,6 +107,7 @@ typedef struct VirtIOGPU {
 
     bool use_virgl_renderer;
     bool renderer_inited;
+    bool renderer_blocked;
     QEMUTimer *fence_poll;
     QEMUTimer *print_stats;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 6/6] virtio-gpu: block any rendering until client (ui) is done

[Marc-André Lureau]

Hi

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Wire up gl_block callback, so ui code can request to stop
> virtio-gpu rendering.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/virtio-gpu-3d.c     |  5 +++++
>  hw/display/virtio-gpu.c        |  8 ++++++++
>  hw/display/virtio-vga.c        | 10 ++++++++++
>  include/hw/virtio/virtio-gpu.h |  1 +
>  4 files changed, 24 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
> index 65926b1..321a959 100644
> --- a/hw/display/virtio-gpu-3d.c
> +++ b/hw/display/virtio-gpu-3d.c
> @@ -382,6 +382,11 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
>  {
>      VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
>
> +    cmd->waiting = g->renderer_blocked;
> +    if (cmd->waiting) {
> +        return;
> +    }
> +
>      virgl_renderer_force_ctx_0();
>      switch (cmd->cmd_hdr.type) {
>      case VIRTIO_GPU_CMD_CTX_CREATE:
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 9087e63..09936c9 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -893,11 +893,19 @@ static int virtio_gpu_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
>      return 0;
>  }
>
> +static void virtio_gpu_gl_block(void *opaque, bool block)
> +{
> +    VirtIOGPU *g = opaque;
> +
> +    g->renderer_blocked = block;
> +}

Why not add here?:

+    if (!block) {
+        virtio_cpu_process_cmdq(g);
+    }


> +
>  const GraphicHwOps virtio_gpu_ops = {
>      .invalidate = virtio_gpu_invalidate_display,
>      .gfx_update = virtio_gpu_update_display,
>      .text_update = virtio_gpu_text_update,
>      .ui_info = virtio_gpu_ui_info,
> +    .gl_block = virtio_gpu_gl_block,
>  };
>
>  static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index f7e539f..255d419 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -65,11 +65,21 @@ static int virtio_vga_ui_info(void *opaque, uint32_t idx, QemuUIInfo *info)
>      return -1;
>  }
>
> +static void virtio_vga_gl_block(void *opaque, bool block)
> +{
> +    VirtIOVGA *vvga = opaque;
> +
> +    if (virtio_gpu_ops.gl_block) {
> +        virtio_gpu_ops.gl_block(&vvga->vdev, block);
> +    }
> +}
> +
>  static const GraphicHwOps virtio_vga_ops = {
>      .invalidate = virtio_vga_invalidate_display,
>      .gfx_update = virtio_vga_update_display,
>      .text_update = virtio_vga_text_update,
>      .ui_info = virtio_vga_ui_info,
> +    .gl_block = virtio_vga_gl_block,
>  };
>
>  /* VGA device wrapper around PCI device around virtio GPU */
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index fdf0091..7249fcd 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -107,6 +107,7 @@ typedef struct VirtIOGPU {
>
>      bool use_virgl_renderer;
>      bool renderer_inited;
> +    bool renderer_blocked;
>      QEMUTimer *fence_poll;
>      QEMUTimer *print_stats;
>
> --
> 1.8.3.1
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 1/6] zap qemu_egl_has_ext in include/ui/egl-helpers.h

[Marc-André Lureau]

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Drop leftover prototype which sneaked in by mistake
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/ui/egl-helpers.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h
> index 5ad5dc3..8c84398 100644
> --- a/include/ui/egl-helpers.h
> +++ b/include/ui/egl-helpers.h
> @@ -11,6 +11,5 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, Window win);
>
>  int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug);
>  EGLContext qemu_egl_init_ctx(void);
> -bool qemu_egl_has_ext(const char *haystack, const char *needle);
>
>  #endif /* EGL_HELPERS_H */
> --
> 1.8.3.1
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 2/6] console: block rendering until client is done

[Marc-André Lureau]

Hi

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Allow gl user interfaces to block display device gl rendering.
> The ui code might want to do that in case it takes a little
> longer to bring things to screen, for example because we'll
> hand over a dma-buf to another process (spice will do that).
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  include/ui/console.h |  2 ++
>  ui/console.c         | 10 ++++++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index adac36d..12ad627 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -362,6 +362,7 @@ typedef struct GraphicHwOps {
>      void (*text_update)(void *opaque, console_ch_t *text);
>      void (*update_interval)(void *opaque, uint64_t interval);
>      int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info);
> +    void (*gl_block)(void *opaque, bool block);
>  } GraphicHwOps;
>
>  QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
> @@ -374,6 +375,7 @@ void graphic_console_set_hwops(QemuConsole *con,
>  void graphic_hw_update(QemuConsole *con);
>  void graphic_hw_invalidate(QemuConsole *con);
>  void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
> +void graphic_hw_gl_block(QemuConsole *con, bool block);
>
>  QemuConsole *qemu_console_lookup_by_index(unsigned int index);
>  QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
> diff --git a/ui/console.c b/ui/console.c
> index 4b65c34..f0a4bd8 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -261,6 +261,16 @@ void graphic_hw_update(QemuConsole *con)
>      }
>  }
>
> +void graphic_hw_gl_block(QemuConsole *con, bool block)
> +{
> +    if (!con) {
> +        con = active_console;
> +    }
> +    if (con && con->hw_ops->gl_block) {
> +        con->hw_ops->gl_block(con->hw, block);
> +    }
> +}
> +
>  void graphic_hw_invalidate(QemuConsole *con)
>  {
>      if (!con) {
> --
> 1.8.3.1
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 3/6] virtio-gpu: fix memory leak in error path

[Marc-André Lureau]

Hi

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> Found by Coverity Scan, buf not freed on error.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

>  hw/display/virtio-gpu-3d.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
> index 28dccfd..d2af327 100644
> --- a/hw/display/virtio-gpu-3d.c
> +++ b/hw/display/virtio-gpu-3d.c
> @@ -197,7 +197,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: size mismatch (%zd/%d)",
>                        __func__, s, cs.size);
>          cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> -        return;
> +        goto out;
>      }
>
>      if (virtio_gpu_stats_enabled(g->conf)) {
> @@ -207,6 +207,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
>
>      virgl_renderer_submit_cmd(buf, cs.hdr.ctx_id, cs.size / 4);
>
> +out:
>      g_free(buf);
>  }
>
> --
> 1.8.3.1
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 4/6] virtio-gpu: maintain command queue

[Marc-André Lureau]

Hi

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> We'll go take out the commands we receive out of the virt queue and put
> them into a linked list, to decouple virtio queue handling from actual
> command processing.
>
> Also move cmd processing to new virtio_gpu_handle_ctrl func, so we can
> easily kick it from different places.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/virtio-gpu.c        | 60 ++++++++++++++++++++++++++----------------
>  include/hw/virtio/virtio-gpu.h |  1 +
>  2 files changed, 39 insertions(+), 22 deletions(-)
>
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 7e79a9c..4433c12 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -754,33 +754,21 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev, VirtQueue *vq)
>      qemu_bh_schedule(g->cursor_bh);
>  }
>
> -static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +static void virtio_cpu_process_cmdq(VirtIOGPU *g)

cpu/gpu

>  {
> -    VirtIOGPU *g = VIRTIO_GPU(vdev);
>      struct virtio_gpu_ctrl_command *cmd;
>
> -    if (!virtio_queue_ready(vq)) {
> -        return;
> -    }
> -
> -#ifdef CONFIG_VIRGL
> -    if (!g->renderer_inited && g->use_virgl_renderer) {
> -        virtio_gpu_virgl_init(g);
> -        g->renderer_inited = true;
> -    }
> -#endif
> -
> -    cmd = g_new(struct virtio_gpu_ctrl_command, 1);
> -    while (virtqueue_pop(vq, &cmd->elem)) {
> -        cmd->vq = vq;
> -        cmd->error = 0;
> -        cmd->finished = false;
> -        if (virtio_gpu_stats_enabled(g->conf)) {
> -            g->stats.requests++;
> -        }
> +    while (!QTAILQ_EMPTY(&g->cmdq)) {
> +        cmd = QTAILQ_FIRST(&g->cmdq);
>
> +        /* process command */
>          VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
>                g, cmd);
> +        QTAILQ_REMOVE(&g->cmdq, cmd, next);
> +        if (virtio_gpu_stats_enabled(g->conf)) {
> +            g->stats.requests++;
> +        }
> +
>          if (!cmd->finished) {
>              QTAILQ_INSERT_TAIL(&g->fenceq, cmd, next);
>              g->inflight++;
> @@ -790,11 +778,38 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
>                  }
>                  fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
>              }
> -            cmd = g_new(struct virtio_gpu_ctrl_command, 1);
>          }

else g_free(cmd)

>      }
> +}
> +
> +static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtIOGPU *g = VIRTIO_GPU(vdev);
> +    struct virtio_gpu_ctrl_command *cmd;
> +
> +    if (!virtio_queue_ready(vq)) {
> +        return;
> +    }
> +
> +#ifdef CONFIG_VIRGL
> +    if (!g->renderer_inited && g->use_virgl_renderer) {
> +        virtio_gpu_virgl_init(g);
> +        g->renderer_inited = true;
> +    }
> +#endif
> +
> +    cmd = g_new0(struct virtio_gpu_ctrl_command, 1);

g_new0 is costly (the struct is big) and quite unnecessary here:

> +    while (virtqueue_pop(vq, &cmd->elem)) {
> +        cmd->vq = vq;
> +        cmd->error = 0;
> +        cmd->finished = false;

Add:
+ cmd->waiting = false;

and you can switch to g_new.

> +        QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
> +        cmd = g_new0(struct virtio_gpu_ctrl_command, 1);

same

> +    }
>      g_free(cmd);
>
> +    virtio_cpu_process_cmdq(g);
> +
>  #ifdef CONFIG_VIRGL
>      if (g->use_virgl_renderer) {
>          virtio_gpu_virgl_fence_poll(g);
> @@ -920,6 +935,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
>      g->ctrl_bh = qemu_bh_new(virtio_gpu_ctrl_bh, g);
>      g->cursor_bh = qemu_bh_new(virtio_gpu_cursor_bh, g);
>      QTAILQ_INIT(&g->reslist);
> +    QTAILQ_INIT(&g->cmdq);
>      QTAILQ_INIT(&g->fenceq);
>
>      g->enabled_output_bitmask = 1;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9b279d7..f7e7a52 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -94,6 +94,7 @@ typedef struct VirtIOGPU {
>      DeviceState *qdev;
>
>      QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
> +    QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
>      QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq;
>
>      struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUT];
> --
> 1.8.3.1
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH 5/6] virtio-gpu: add support to enable/disable command processing

[Marc-André Lureau]

Hi

On Wed, Jan 13, 2016 at 1:02 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
> So we can stop rendering for a while in case we have to.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  hw/display/virtio-gpu-3d.c     | 3 ++-
>  hw/display/virtio-gpu.c        | 5 ++++-
>  include/hw/virtio/virtio-gpu.h | 2 ++
>  3 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
> index d2af327..65926b1 100644
> --- a/hw/display/virtio-gpu-3d.c
> +++ b/hw/display/virtio-gpu-3d.c
> @@ -553,7 +553,8 @@ static void virtio_gpu_fence_poll(void *opaque)
>      VirtIOGPU *g = opaque;
>
>      virgl_renderer_poll();
> -    if (g->inflight) {
> +    virtio_cpu_process_cmdq(g);
> +    if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
>          timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
>      }
>  }
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 4433c12..9087e63 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -754,7 +754,7 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev, VirtQueue *vq)
>      qemu_bh_schedule(g->cursor_bh);
>  }
>
> -static void virtio_cpu_process_cmdq(VirtIOGPU *g)
> +void virtio_cpu_process_cmdq(VirtIOGPU *g)
>  {
>      struct virtio_gpu_ctrl_command *cmd;
>
> @@ -764,6 +764,9 @@ static void virtio_cpu_process_cmdq(VirtIOGPU *g)
>          /* process command */
>          VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
>                g, cmd);
> +        if (cmd->waiting) {
> +            break;
> +        }
>          QTAILQ_REMOVE(&g->cmdq, cmd, next);
>          if (virtio_gpu_stats_enabled(g->conf)) {
>              g->stats.requests++;
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index f7e7a52..fdf0091 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -76,6 +76,7 @@ struct virtio_gpu_ctrl_command {
>      VirtQueue *vq;
>      struct virtio_gpu_ctrl_hdr cmd_hdr;
>      uint32_t error;
> +    bool waiting;
>      bool finished;
>      QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>  };
> @@ -152,6 +153,7 @@ int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab,
>                                    struct virtio_gpu_ctrl_command *cmd,
>                                    struct iovec **iov);
>  void virtio_gpu_cleanup_mapping_iov(struct iovec *iov, uint32_t count);
> +void virtio_cpu_process_cmdq(VirtIOGPU *g);
>
>  /* virtio-gpu-3d.c */
>  void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> --
> 1.8.3.1
>



-- 
Marc-André Lureau