All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Add support for Blob resources feature
@ 2021-03-31  3:09 Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 01/11] ui: Get the fd associated with udmabuf driver Vivek Kasireddy
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Marc-André Lureau, Dongwon Kim, Tina Zhang, Vivek Kasireddy,
	Gerd Hoffmann

Enabling this feature would eliminate data copies from the resource
object in the Guest to the shadow resource in Qemu. This patch series
however adds support only for Blobs of type
VIRTIO_GPU_BLOB_MEM_GUEST with property VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE.

Most of the patches in this series are a rebased, refactored and bugfixed 
versions of Gerd Hoffmann's patches located here:
https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next

TODO:
- Enable the combination virgl + blob resources
- Add support for VIRTGPU_BLOB_MEM_HOST3D blobs

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Dongwon Kim <dongwon.kim@intel.com>
Cc: Tina Zhang <tina.zhang@intel.com>

Vivek Kasireddy (11):
  ui: Get the fd associated with udmabuf driver
  ui/pixman: Add qemu_pixman_to_drm_format()
  virtio-gpu: Add udmabuf helpers
  virtio-gpu: Add virtio_gpu_find_check_resource
  virtio-gpu: Refactor virtio_gpu_set_scanout
  virtio-gpu: Refactor virtio_gpu_create_mapping_iov
  virtio-gpu: Add initial definitions for blob resources
  virtio-gpu: Add virtio_gpu_resource_create_blob
  virtio-gpu: Add helpers to create and destroy dmabuf objects
  virtio-gpu: Add virtio_gpu_set_scanout_blob
  virtio-gpu: Update cursor data using blob

 hw/display/meson.build                      |   2 +-
 hw/display/trace-events                     |   2 +
 hw/display/virtio-gpu-3d.c                  |   3 +-
 hw/display/virtio-gpu-base.c                |   3 +
 hw/display/virtio-gpu-udmabuf.c             | 276 +++++++++++++
 hw/display/virtio-gpu.c                     | 423 +++++++++++++++-----
 include/hw/virtio/virtio-gpu-bswap.h        |  16 +
 include/hw/virtio/virtio-gpu.h              |  41 +-
 include/standard-headers/linux/udmabuf.h    |  32 ++
 include/standard-headers/linux/virtio_gpu.h |   1 +
 include/ui/console.h                        |   3 +
 include/ui/qemu-pixman.h                    |   1 +
 scripts/update-linux-headers.sh             |   3 +
 ui/meson.build                              |   1 +
 ui/qemu-pixman.c                            |  35 +-
 ui/udmabuf.c                                |  40 ++
 16 files changed, 772 insertions(+), 110 deletions(-)
 create mode 100644 hw/display/virtio-gpu-udmabuf.c
 create mode 100644 include/standard-headers/linux/udmabuf.h
 create mode 100644 ui/udmabuf.c

-- 
2.26.2



^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/11] ui: Get the fd associated with udmabuf driver
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 02/11] ui/pixman: Add qemu_pixman_to_drm_format() Vivek Kasireddy
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

Try to open the udmabuf dev node for the first time or return the
fd if the device was previously opened.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/console.h |  3 +++
 ui/meson.build       |  1 +
 ui/udmabuf.c         | 40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+)
 create mode 100644 ui/udmabuf.c

diff --git a/include/ui/console.h b/include/ui/console.h
index ca3c7af6a6..b30b63976a 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -471,4 +471,7 @@ bool vnc_display_reload_certs(const char *id,  Error **errp);
 /* input.c */
 int index_from_key(const char *key, size_t key_length);
 
+/* udmabuf.c */
+int udmabuf_fd(void);
+
 #endif
diff --git a/ui/meson.build b/ui/meson.build
index e8d3ff41b9..7a709ff548 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -11,6 +11,7 @@ softmmu_ss.add(files(
   'kbd-state.c',
   'keymaps.c',
   'qemu-pixman.c',
+  'udmabuf.c',
 ))
 softmmu_ss.add([spice_headers, files('spice-module.c')])
 
diff --git a/ui/udmabuf.c b/ui/udmabuf.c
new file mode 100644
index 0000000000..e6234fd86f
--- /dev/null
+++ b/ui/udmabuf.c
@@ -0,0 +1,40 @@
+/*
+ * udmabuf helper functions.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "ui/console.h"
+
+#ifdef CONFIG_LINUX
+
+#include <sys/fcntl.h>
+#include <sys/ioctl.h>
+
+int udmabuf_fd(void)
+{
+    static bool first = true;
+    static int udmabuf;
+
+    if (!first) {
+        return udmabuf;
+    }
+    first = false;
+
+    udmabuf = open("/dev/udmabuf", O_RDWR);
+    if (udmabuf < 0) {
+        warn_report("open /dev/udmabuf: %s", strerror(errno));
+    }
+    return udmabuf;
+}
+
+#else
+
+int udmabuf_fd(void)
+{
+    return -1;
+}
+
+#endif
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/11] ui/pixman: Add qemu_pixman_to_drm_format()
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 01/11] ui: Get the fd associated with udmabuf driver Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 03/11] virtio-gpu: Add udmabuf helpers Vivek Kasireddy
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

This new function to get the drm_format associated with a pixman
format will be useful while creating a dmabuf.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 include/ui/qemu-pixman.h |  1 +
 ui/qemu-pixman.c         | 35 ++++++++++++++++++++++++-----------
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 87737a6f16..806ddcd7cd 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -62,6 +62,7 @@ typedef struct PixelFormat {
 PixelFormat qemu_pixelformat_from_pixman(pixman_format_code_t format);
 pixman_format_code_t qemu_default_pixman_format(int bpp, bool native_endian);
 pixman_format_code_t qemu_drm_format_to_pixman(uint32_t drm_format);
+uint32_t qemu_pixman_to_drm_format(pixman_format_code_t pixman);
 int qemu_pixman_get_type(int rshift, int gshift, int bshift);
 pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf);
 bool qemu_pixman_check_format(DisplayChangeListener *dcl,
diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
index 85f2945e88..3ab7e2e958 100644
--- a/ui/qemu-pixman.c
+++ b/ui/qemu-pixman.c
@@ -89,21 +89,34 @@ pixman_format_code_t qemu_default_pixman_format(int bpp, bool native_endian)
 }
 
 /* Note: drm is little endian, pixman is native endian */
+static const struct {
+    uint32_t drm_format;
+    pixman_format_code_t pixman_format;
+} drm_format_pixman_map[] = {
+    { DRM_FORMAT_RGB888,   PIXMAN_LE_r8g8b8   },
+    { DRM_FORMAT_ARGB8888, PIXMAN_LE_a8r8g8b8 },
+    { DRM_FORMAT_XRGB8888, PIXMAN_LE_x8r8g8b8 }
+};
+
 pixman_format_code_t qemu_drm_format_to_pixman(uint32_t drm_format)
 {
-    static const struct {
-        uint32_t drm_format;
-        pixman_format_code_t pixman;
-    } map[] = {
-        { DRM_FORMAT_RGB888,   PIXMAN_LE_r8g8b8   },
-        { DRM_FORMAT_ARGB8888, PIXMAN_LE_a8r8g8b8 },
-        { DRM_FORMAT_XRGB8888, PIXMAN_LE_x8r8g8b8 }
-    };
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(map); i++) {
-        if (drm_format == map[i].drm_format) {
-            return map[i].pixman;
+    for (i = 0; i < ARRAY_SIZE(drm_format_pixman_map); i++) {
+        if (drm_format == drm_format_pixman_map[i].drm_format) {
+            return drm_format_pixman_map[i].pixman_format;
+        }
+    }
+    return 0;
+}
+
+uint32_t qemu_pixman_to_drm_format(pixman_format_code_t pixman_format)
+{
+    int i;
+
+    for (i = 0; i < ARRAY_SIZE(drm_format_pixman_map); i++) {
+        if (pixman_format == drm_format_pixman_map[i].pixman_format) {
+            return drm_format_pixman_map[i].drm_format;
         }
     }
     return 0;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/11] virtio-gpu: Add udmabuf helpers
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 01/11] ui: Get the fd associated with udmabuf driver Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 02/11] ui/pixman: Add qemu_pixman_to_drm_format() Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 04/11] virtio-gpu: Add virtio_gpu_find_check_resource Vivek Kasireddy
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

Add helper functions to create a dmabuf for a resource and mmap it.
To be able to create a dmabuf using the udmabuf driver, Qemu needs
to be lauched with the memfd memory backend like this:

qemu-system-x86_64 -m 8192m -object memory-backend-memfd,id=mem1,size=8192M
-machine memory-backend=mem1

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/meson.build                   |   2 +-
 hw/display/virtio-gpu-udmabuf.c          | 182 +++++++++++++++++++++++
 include/hw/virtio/virtio-gpu.h           |  10 ++
 include/standard-headers/linux/udmabuf.h |  32 ++++
 scripts/update-linux-headers.sh          |   3 +
 5 files changed, 228 insertions(+), 1 deletion(-)
 create mode 100644 hw/display/virtio-gpu-udmabuf.c
 create mode 100644 include/standard-headers/linux/udmabuf.h

diff --git a/hw/display/meson.build b/hw/display/meson.build
index 9d79e3951d..e0fa88d62f 100644
--- a/hw/display/meson.build
+++ b/hw/display/meson.build
@@ -56,7 +56,7 @@ softmmu_ss.add(when: [pixman, 'CONFIG_ATI_VGA'], if_true: files('ati.c', 'ati_2d
 if config_all_devices.has_key('CONFIG_VIRTIO_GPU')
   virtio_gpu_ss = ss.source_set()
   virtio_gpu_ss.add(when: 'CONFIG_VIRTIO_GPU',
-                    if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c'), pixman, virgl])
+                    if_true: [files('virtio-gpu-base.c', 'virtio-gpu.c', 'virtio-gpu-udmabuf.c'), pixman, virgl])
   virtio_gpu_ss.add(when: ['CONFIG_VIRTIO_GPU', 'CONFIG_VIRGL'],
                     if_true: [files('virtio-gpu-3d.c'), pixman, virgl])
   virtio_gpu_ss.add(when: 'CONFIG_VHOST_USER_GPU', if_true: files('vhost-user-gpu.c'))
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
new file mode 100644
index 0000000000..8d0fe1ce4a
--- /dev/null
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -0,0 +1,182 @@
+/*
+ * Virtio GPU Device
+ *
+ * Copyright Red Hat, Inc. 2013-2014
+ *
+ * Authors:
+ *     Dave Airlie <airlied@redhat.com>
+ *     Gerd Hoffmann <kraxel@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu-common.h"
+#include "qemu/iov.h"
+#include "ui/console.h"
+#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
+#include "trace.h"
+
+#ifdef CONFIG_LINUX
+
+#include "exec/ramblock.h"
+#include "sysemu/hostmem.h"
+#include <sys/fcntl.h>
+#include <sys/ioctl.h>
+#include <linux/memfd.h>
+#include "standard-headers/linux/udmabuf.h"
+
+static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    struct udmabuf_create_list *list;
+    RAMBlock *rb;
+    ram_addr_t offset;
+    int udmabuf, i;
+
+    udmabuf = udmabuf_fd();
+    if (udmabuf < 0) {
+        return;
+    }
+
+    list = g_malloc0(sizeof(struct udmabuf_create_list) +
+                     sizeof(struct udmabuf_create_item) * res->iov_cnt);
+
+    for (i = 0; i < res->iov_cnt; i++) {
+        rcu_read_lock();
+        rb = qemu_ram_block_from_host(res->iov[i].iov_base, false, &offset);
+        rcu_read_unlock();
+
+        if (!rb || rb->fd < 0) {
+            g_free(list);
+            return;
+        }
+
+        list->list[i].memfd  = rb->fd;
+        list->list[i].offset = offset;
+        list->list[i].size   = res->iov[i].iov_len;
+    }
+
+    list->count = res->iov_cnt;
+    list->flags = UDMABUF_FLAGS_CLOEXEC;
+
+    res->dmabuf_fd = ioctl(udmabuf, UDMABUF_CREATE_LIST, list);
+    if (res->dmabuf_fd < 0) {
+        warn_report("%s: UDMABUF_CREATE_LIST: %s", __func__,
+                    strerror(errno));
+    }
+    g_free(list);
+}
+
+static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    res->remapsz = res->width * res->height * 4;
+    res->remapsz = QEMU_ALIGN_UP(res->remapsz, getpagesize());
+
+    res->remapped = mmap(NULL, res->remapsz, PROT_READ,
+                         MAP_SHARED, res->dmabuf_fd, 0);
+    if (res->remapped == MAP_FAILED) {
+        warn_report("%s: dmabuf mmap failed: %s", __func__,
+                    strerror(errno));
+        res->remapped = NULL;
+    }
+}
+
+static void virtio_gpu_destroy_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    if (res->remapped) {
+        munmap(res->remapped, res->remapsz);
+        res->remapped = NULL;
+    }
+    if (res->dmabuf_fd >= 0) {
+        close(res->dmabuf_fd);
+        res->dmabuf_fd = -1;
+    }
+}
+
+static int find_memory_backend_type(Object *obj, void *opaque)
+{
+    bool *memfd_backend = opaque;
+    int ret;
+
+    if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
+        HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+        RAMBlock *rb = backend->mr.ram_block;
+
+        if (rb && rb->fd > 0) {
+            ret = fcntl(rb->fd, F_GET_SEALS);
+            if (ret > 0) {
+                *memfd_backend = true;
+            }
+        }
+    }
+
+    return 0;
+}
+
+bool virtio_gpu_have_udmabuf(void)
+{
+    Object *memdev_root;
+    int udmabuf;
+    bool memfd_backend = false;
+
+    udmabuf = udmabuf_fd();
+    if (udmabuf < 0)
+        return false;
+
+    memdev_root = object_resolve_path("/objects", NULL);
+    object_child_foreach(memdev_root, find_memory_backend_type, &memfd_backend);
+
+    return memfd_backend;
+}
+
+void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    void *pdata = NULL;
+
+    res->dmabuf_fd = -1;
+    if (res->iov_cnt == 1) {
+        pdata = res->iov[0].iov_base;
+    } else {
+        virtio_gpu_create_udmabuf(res);
+        if (res->dmabuf_fd < 0) {
+            return;
+        }
+        virtio_gpu_remap_udmabuf(res);
+        if (!res->remapped) {
+            return;
+        }
+        pdata = res->remapped;
+    }
+
+    (void) pdata;
+}
+
+void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    if (res->remapped) {
+        virtio_gpu_destroy_udmabuf(res);
+    }
+}
+
+#else
+
+bool virtio_gpu_have_udmabuf(void)
+{
+    /* nothing (stub) */
+    return false;
+}
+
+void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    /* nothing (stub) */
+}
+
+void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
+{
+    /* nothing (stub) */
+}
+
+#endif
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index fae149235c..a4b7738d8a 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -47,6 +47,11 @@ struct virtio_gpu_simple_resource {
     uint32_t scanout_bitmask;
     pixman_image_t *image;
     uint64_t hostmem;
+
+    int dmabuf_fd;
+    size_t remapsz;
+    uint8_t *remapped;
+
     QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
 };
 
@@ -214,6 +219,11 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
                                     struct iovec *iov, uint32_t count);
 void virtio_gpu_process_cmdq(VirtIOGPU *g);
 
+/* virtio-gpu-udmabuf.c */
+bool virtio_gpu_have_udmabuf(void);
+void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
+void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
+
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
                                   struct virtio_gpu_ctrl_command *cmd);
diff --git a/include/standard-headers/linux/udmabuf.h b/include/standard-headers/linux/udmabuf.h
new file mode 100644
index 0000000000..e19eb5b5ce
--- /dev/null
+++ b/include/standard-headers/linux/udmabuf.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _LINUX_UDMABUF_H
+#define _LINUX_UDMABUF_H
+
+#include "standard-headers/linux/types.h"
+
+#define UDMABUF_FLAGS_CLOEXEC	0x01
+
+struct udmabuf_create {
+	uint32_t memfd;
+	uint32_t flags;
+	uint64_t offset;
+	uint64_t size;
+};
+
+struct udmabuf_create_item {
+	uint32_t memfd;
+	uint32_t __pad;
+	uint64_t offset;
+	uint64_t size;
+};
+
+struct udmabuf_create_list {
+	uint32_t flags;
+	uint32_t count;
+	struct udmabuf_create_item list[];
+};
+
+#define UDMABUF_CREATE       _IOW('u', 0x42, struct udmabuf_create)
+#define UDMABUF_CREATE_LIST  _IOW('u', 0x43, struct udmabuf_create_list)
+
+#endif /* _LINUX_UDMABUF_H */
diff --git a/scripts/update-linux-headers.sh b/scripts/update-linux-headers.sh
index 1050e36169..fea4d6eb65 100755
--- a/scripts/update-linux-headers.sh
+++ b/scripts/update-linux-headers.sh
@@ -34,6 +34,7 @@ cp_portable() {
     if
         grep '#include' "$f" | grep -v -e 'linux/virtio' \
                                      -e 'linux/types' \
+                                     -e 'linux/ioctl' \
                                      -e 'stdint' \
                                      -e 'linux/if_ether' \
                                      -e 'input-event-codes' \
@@ -66,6 +67,7 @@ cp_portable() {
         -e 's/__BITS_PER_LONG/HOST_LONG_BITS/' \
         -e '/\"drm.h\"/d' \
         -e '/sys\/ioctl.h/d' \
+        -e '/linux\/ioctl.h/d' \
         -e 's/SW_MAX/SW_MAX_/' \
         -e 's/atomic_t/int/' \
         -e 's/__kernel_long_t/long/' \
@@ -190,6 +192,7 @@ for i in "$tmpdir"/include/linux/*virtio*.h \
          "$tmpdir/include/linux/fuse.h" \
          "$tmpdir/include/linux/input.h" \
          "$tmpdir/include/linux/input-event-codes.h" \
+         "$tmpdir/include/linux/udmabuf.h" \
          "$tmpdir/include/linux/pci_regs.h" \
          "$tmpdir/include/linux/ethtool.h" \
          "$tmpdir/include/linux/const.h" \
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/11] virtio-gpu: Add virtio_gpu_find_check_resource
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (2 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 03/11] virtio-gpu: Add udmabuf helpers Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 05/11] virtio-gpu: Refactor virtio_gpu_set_scanout Vivek Kasireddy
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

Move finding the resource and validating its backing storage into one
function.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 66 +++++++++++++++++++++++++++++------------
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index c9f5e36fd0..de7462a515 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -35,6 +35,10 @@
 
 static struct virtio_gpu_simple_resource*
 virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id);
+static struct virtio_gpu_simple_resource *
+virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
+                               bool require_backing,
+                               const char *caller, uint32_t *error);
 
 static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
                                        struct virtio_gpu_simple_resource *res);
@@ -63,7 +67,8 @@ static void update_cursor_data_simple(VirtIOGPU *g,
     struct virtio_gpu_simple_resource *res;
     uint32_t pixels;
 
-    res = virtio_gpu_find_resource(g, resource_id);
+    res = virtio_gpu_find_check_resource(g, resource_id, false,
+                                         __func__, NULL);
     if (!res) {
         return;
     }
@@ -158,6 +163,37 @@ virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id)
     return NULL;
 }
 
+static struct virtio_gpu_simple_resource *
+virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
+                               bool require_backing,
+                               const char *caller, uint32_t *error)
+{
+    struct virtio_gpu_simple_resource *res;
+
+    res = virtio_gpu_find_resource(g, resource_id);
+    if (!res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid resource specified %d\n",
+                      caller, resource_id);
+        if (error) {
+            *error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        }
+        return NULL;
+    }
+
+    if (require_backing) {
+        if (!res->iov || !res->image) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
+                          caller, resource_id);
+            if (error) {
+                *error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+            }
+            return NULL;
+        }
+    }
+
+    return res;
+}
+
 void virtio_gpu_ctrl_response(VirtIOGPU *g,
                               struct virtio_gpu_ctrl_command *cmd,
                               struct virtio_gpu_ctrl_hdr *resp,
@@ -396,11 +432,9 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
     virtio_gpu_t2d_bswap(&t2d);
     trace_virtio_gpu_cmd_res_xfer_toh_2d(t2d.resource_id);
 
-    res = virtio_gpu_find_resource(g, t2d.resource_id);
-    if (!res || !res->iov) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal resource specified %d\n",
-                      __func__, t2d.resource_id);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+    res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
+                                         __func__, &cmd->error);
+    if (!res) {
         return;
     }
 
@@ -454,11 +488,9 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
     trace_virtio_gpu_cmd_res_flush(rf.resource_id,
                                    rf.r.width, rf.r.height, rf.r.x, rf.r.y);
 
-    res = virtio_gpu_find_resource(g, rf.resource_id);
+    res = virtio_gpu_find_check_resource(g, rf.resource_id, false,
+                                         __func__, &cmd->error);
     if (!res) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal resource specified %d\n",
-                      __func__, rf.resource_id);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
         return;
     }
 
@@ -541,11 +573,9 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
     }
 
     /* create a surface for this scanout */
-    res = virtio_gpu_find_resource(g, ss.resource_id);
+    res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
+                                         __func__, &cmd->error);
     if (!res) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal resource specified %d\n",
-                      __func__, ss.resource_id);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
         return;
     }
 
@@ -737,11 +767,9 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
     virtio_gpu_bswap_32(&detach, sizeof(detach));
     trace_virtio_gpu_cmd_res_back_detach(detach.resource_id);
 
-    res = virtio_gpu_find_resource(g, detach.resource_id);
-    if (!res || !res->iov) {
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal resource specified %d\n",
-                      __func__, detach.resource_id);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+    res = virtio_gpu_find_check_resource(g, detach.resource_id, true,
+                                         __func__, &cmd->error);
+    if (!res) {
         return;
     }
     virtio_gpu_cleanup_mapping(g, res);
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/11] virtio-gpu: Refactor virtio_gpu_set_scanout
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (3 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 04/11] virtio-gpu: Add virtio_gpu_find_check_resource Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 06/11] virtio-gpu: Refactor virtio_gpu_create_mapping_iov Vivek Kasireddy
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

Store the meta-data associated with a FB in a new object
(struct virtio_gpu_framebuffer) and pass the object to set_scanout.
Also move code in set_scanout into a do_set_scanout function.
This will be helpful when adding set_scanout_blob API.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c        | 149 +++++++++++++++++++--------------
 include/hw/virtio/virtio-gpu.h |   8 ++
 2 files changed, 94 insertions(+), 63 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index de7462a515..5e1152aa2a 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -544,95 +544,118 @@ static void virtio_unref_resource(pixman_image_t *image, void *data)
     pixman_image_unref(data);
 }
 
-static void virtio_gpu_set_scanout(VirtIOGPU *g,
-                                   struct virtio_gpu_ctrl_command *cmd)
+static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
+                                      uint32_t scanout_id,
+                                      struct virtio_gpu_framebuffer *fb,
+                                      struct virtio_gpu_simple_resource *res,
+                                      struct virtio_gpu_rect *r,
+                                      uint32_t *error)
 {
-    struct virtio_gpu_simple_resource *res, *ores;
+    struct virtio_gpu_simple_resource *ores;
     struct virtio_gpu_scanout *scanout;
-    pixman_format_code_t format;
-    uint32_t offset;
-    int bpp;
-    struct virtio_gpu_set_scanout ss;
-
-    VIRTIO_GPU_FILL_CMD(ss);
-    virtio_gpu_bswap_32(&ss, sizeof(ss));
-    trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
-                                     ss.r.width, ss.r.height, ss.r.x, ss.r.y);
+    uint8_t *data;
 
-    if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+    if (scanout_id >= g->parent_obj.conf.max_outputs) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
-                      __func__, ss.scanout_id);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
-        return;
-    }
-
-    g->parent_obj.enable = 1;
-    if (ss.resource_id == 0) {
-        virtio_gpu_disable_scanout(g, ss.scanout_id);
-        return;
-    }
-
-    /* create a surface for this scanout */
-    res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
-                                         __func__, &cmd->error);
-    if (!res) {
+                      __func__, scanout_id);
+        *error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
         return;
     }
+    scanout = &g->parent_obj.scanout[scanout_id];
 
-    if (ss.r.x > res->width ||
-        ss.r.y > res->height ||
-        ss.r.width < 16 ||
-        ss.r.height < 16 ||
-        ss.r.width > res->width ||
-        ss.r.height > res->height ||
-        ss.r.x + ss.r.width > res->width ||
-        ss.r.y + ss.r.height > res->height) {
+    if (r->x > fb->width ||
+        r->y > fb->height ||
+        r->width < 16 ||
+        r->height < 16 ||
+        r->width > fb->width ||
+        r->height > fb->height ||
+        r->x + r->width > fb->width ||
+        r->y + r->height > fb->height) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
-                      " resource %d, (%d,%d)+%d,%d vs %d %d\n",
-                      __func__, ss.scanout_id, ss.resource_id, ss.r.x, ss.r.y,
-                      ss.r.width, ss.r.height, res->width, res->height);
-        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+                      " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+                      __func__, scanout_id, res->resource_id,
+                      r->x, r->y, r->width, r->height,
+                      fb->width, fb->height);
+        *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
         return;
     }
 
-    scanout = &g->parent_obj.scanout[ss.scanout_id];
+    g->parent_obj.enable = 1;
+    data = (uint8_t *)pixman_image_get_data(res->image);
 
-    format = pixman_image_get_format(res->image);
-    bpp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
-    offset = (ss.r.x * bpp) + ss.r.y * pixman_image_get_stride(res->image);
-    if (!scanout->ds || surface_data(scanout->ds)
-        != ((uint8_t *)pixman_image_get_data(res->image) + offset) ||
-        scanout->width != ss.r.width ||
-        scanout->height != ss.r.height) {
+    /* create a surface for this scanout */
+    if (!scanout->ds ||
+        surface_data(scanout->ds) != data + fb->offset ||
+        scanout->width != r->width ||
+        scanout->height != r->height) {
         pixman_image_t *rect;
-        void *ptr = (uint8_t *)pixman_image_get_data(res->image) + offset;
-        rect = pixman_image_create_bits(format, ss.r.width, ss.r.height, ptr,
-                                        pixman_image_get_stride(res->image));
-        pixman_image_ref(res->image);
-        pixman_image_set_destroy_function(rect, virtio_unref_resource,
-                                          res->image);
+        void *ptr = data + fb->offset;
+        rect = pixman_image_create_bits(fb->format, r->width, r->height,
+                                        ptr, fb->stride);
+
+        if (res->image) {
+            pixman_image_ref(res->image);
+            pixman_image_set_destroy_function(rect, virtio_unref_resource,
+                                              res->image);
+        }
+
         /* realloc the surface ptr */
         scanout->ds = qemu_create_displaysurface_pixman(rect);
         if (!scanout->ds) {
-            cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+            *error = VIRTIO_GPU_RESP_ERR_UNSPEC;
             return;
         }
+
         pixman_image_unref(rect);
-        dpy_gfx_replace_surface(g->parent_obj.scanout[ss.scanout_id].con,
+        dpy_gfx_replace_surface(g->parent_obj.scanout[scanout_id].con,
                                 scanout->ds);
     }
 
     ores = virtio_gpu_find_resource(g, scanout->resource_id);
     if (ores) {
-        ores->scanout_bitmask &= ~(1 << ss.scanout_id);
+        ores->scanout_bitmask &= ~(1 << scanout_id);
+    }
+
+    res->scanout_bitmask |= (1 << scanout_id);
+    scanout->resource_id = res->resource_id;
+    scanout->x = r->x;
+    scanout->y = r->y;
+    scanout->width = r->width;
+    scanout->height = r->height;
+}
+
+static void virtio_gpu_set_scanout(VirtIOGPU *g,
+                                   struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_framebuffer fb = { 0 };
+    struct virtio_gpu_set_scanout ss;
+
+    VIRTIO_GPU_FILL_CMD(ss);
+    virtio_gpu_bswap_32(&ss, sizeof(ss));
+    trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
+                                     ss.r.width, ss.r.height, ss.r.x, ss.r.y);
+
+    if (ss.resource_id == 0) {
+        virtio_gpu_disable_scanout(g, ss.scanout_id);
+        return;
     }
 
-    res->scanout_bitmask |= (1 << ss.scanout_id);
-    scanout->resource_id = ss.resource_id;
-    scanout->x = ss.r.x;
-    scanout->y = ss.r.y;
-    scanout->width = ss.r.width;
-    scanout->height = ss.r.height;
+    res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
+                                         __func__, &cmd->error);
+    if (!res) {
+        return;
+    }
+
+    fb.format = pixman_image_get_format(res->image);
+    fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+    fb.width  = pixman_image_get_width(res->image);
+    fb.height = pixman_image_get_height(res->image);
+    fb.stride = pixman_image_get_stride(res->image);
+    fb.offset = ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+
+    virtio_gpu_do_set_scanout(g, ss.scanout_id,
+                              &fb, res, &ss.r, &cmd->error);
 }
 
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a4b7738d8a..6fd86d6b92 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -55,6 +55,14 @@ struct virtio_gpu_simple_resource {
     QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
 };
 
+struct virtio_gpu_framebuffer {
+    pixman_format_code_t format;
+    uint32_t bytes_pp;
+    uint32_t width, height;
+    uint32_t stride;
+    uint32_t offset;
+};
+
 struct virtio_gpu_scanout {
     QemuConsole *con;
     DisplaySurface *ds;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/11] virtio-gpu: Refactor virtio_gpu_create_mapping_iov
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (4 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 05/11] virtio-gpu: Refactor virtio_gpu_set_scanout Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 07/11] virtio-gpu: Add initial definitions for blob resources Vivek Kasireddy
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy

Instead of passing the attach_backing object to extract nr_entries
and offset, explicitly pass these as arguments to this function.
This will be helpful when adding create_blob API.

Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-3d.c     |  3 ++-
 hw/display/virtio-gpu.c        | 22 +++++++++++-----------
 include/hw/virtio/virtio-gpu.h |  2 +-
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index d98964858e..87ca661a26 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -288,7 +288,8 @@ static void virgl_resource_attach_backing(VirtIOGPU *g,
     VIRTIO_GPU_FILL_CMD(att_rb);
     trace_virtio_gpu_cmd_res_back_attach(att_rb.resource_id);
 
-    ret = virtio_gpu_create_mapping_iov(g, &att_rb, cmd, NULL, &res_iovs);
+    ret = virtio_gpu_create_mapping_iov(g, att_rb.nr_entries, sizeof(att_rb),
+                                        cmd, NULL, &res_iovs);
     if (ret != 0) {
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         return;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5e1152aa2a..85c8eb749d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -659,7 +659,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
 }
 
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
-                                  struct virtio_gpu_resource_attach_backing *ab,
+                                  uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
                                   uint64_t **addr, struct iovec **iov)
 {
@@ -667,17 +667,17 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
     size_t esize, s;
     int i;
 
-    if (ab->nr_entries > 16384) {
+    if (nr_entries > 16384) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: nr_entries is too big (%d > 16384)\n",
-                      __func__, ab->nr_entries);
+                      __func__, nr_entries);
         return -1;
     }
 
-    esize = sizeof(*ents) * ab->nr_entries;
+    esize = sizeof(*ents) * nr_entries;
     ents = g_malloc(esize);
     s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
-                   sizeof(*ab), ents, esize);
+                   offset, ents, esize);
     if (s != esize) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: command data size incorrect %zu vs %zu\n",
@@ -686,11 +686,11 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
         return -1;
     }
 
-    *iov = g_malloc0(sizeof(struct iovec) * ab->nr_entries);
+    *iov = g_malloc0(sizeof(struct iovec) * nr_entries);
     if (addr) {
-        *addr = g_malloc0(sizeof(uint64_t) * ab->nr_entries);
+        *addr = g_malloc0(sizeof(uint64_t) * nr_entries);
     }
-    for (i = 0; i < ab->nr_entries; i++) {
+    for (i = 0; i < nr_entries; i++) {
         uint64_t a = le64_to_cpu(ents[i].addr);
         uint32_t l = le32_to_cpu(ents[i].length);
         hwaddr len = l;
@@ -702,8 +702,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
         }
         if (!(*iov)[i].iov_base || len != l) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
-                          " resource %d element %d\n",
-                          __func__, ab->resource_id, i);
+                          " element %d\n", __func__, i);
             if ((*iov)[i].iov_base) {
                 i++; /* cleanup the 'i'th map */
             }
@@ -770,7 +769,8 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
         return;
     }
 
-    ret = virtio_gpu_create_mapping_iov(g, &ab, cmd, &res->addrs, &res->iov);
+    ret = virtio_gpu_create_mapping_iov(g, ab.nr_entries, sizeof(ab),
+                                        cmd, &res->addrs, &res->iov);
     if (ret != 0) {
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
         return;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 6fd86d6b92..879f4cfff6 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -220,7 +220,7 @@ void virtio_gpu_get_display_info(VirtIOGPU *g,
 void virtio_gpu_get_edid(VirtIOGPU *g,
                          struct virtio_gpu_ctrl_command *cmd);
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
-                                  struct virtio_gpu_resource_attach_backing *ab,
+                                  uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
                                   uint64_t **addr, struct iovec **iov);
 void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/11] virtio-gpu: Add initial definitions for blob resources
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (5 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 06/11] virtio-gpu: Refactor virtio_gpu_create_mapping_iov Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 08/11] virtio-gpu: Add virtio_gpu_resource_create_blob Vivek Kasireddy
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

Add the property bit, configuration flag and other relevant
macros and definitions associated with this feature.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-base.c   |  3 +++
 hw/display/virtio-gpu.c        | 27 +++++++++++++++++++++++++++
 include/hw/virtio/virtio-gpu.h |  3 +++
 3 files changed, 33 insertions(+)

diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 25f8920fdb..70e4d53d26 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -210,6 +210,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
     if (virtio_gpu_edid_enabled(g->conf)) {
         features |= (1 << VIRTIO_GPU_F_EDID);
     }
+    if (virtio_gpu_blob_enabled(g->conf)) {
+        features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
+    }
 
     return features;
 }
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 85c8eb749d..30d3e38fa9 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -20,6 +20,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/virtio/virtio.h"
 #include "migration/qemu-file-types.h"
+#include "migration/blocker.h"
 #include "hw/virtio/virtio-gpu.h"
 #include "hw/virtio/virtio-gpu-bswap.h"
 #include "hw/virtio/virtio-gpu-pixman.h"
@@ -1160,6 +1161,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
     VirtIOGPU *g = VIRTIO_GPU(qdev);
+    Error *local_err = NULL;
     bool have_virgl;
 
 #if !defined(CONFIG_VIRGL) || defined(HOST_WORDS_BIGENDIAN)
@@ -1176,6 +1178,29 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
 #endif
     }
 
+    if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
+        if (!virtio_gpu_have_udmabuf()) {
+            error_setg(errp, "cannot enable blob resources without udmabuf");
+            return;
+        }
+
+        /* FIXME: to be investigated ... */
+        if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) {
+            error_setg(errp, "blobs and virgl are not compatible (yet)");
+            return;
+        }
+
+        /* FIXME: must xfer resource type somehow */
+        error_setg(&g->parent_obj.migration_blocker,
+                   "blobs are not migratable (yet)");
+        migrate_add_blocker(g->parent_obj.migration_blocker, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            error_free(g->parent_obj.migration_blocker);
+            return;
+        }
+    }
+
     if (!virtio_gpu_base_device_realize(qdev,
                                         virtio_gpu_handle_ctrl_cb,
                                         virtio_gpu_handle_cursor_cb,
@@ -1292,6 +1317,8 @@ static Property virtio_gpu_properties[] = {
     DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
                     VIRTIO_GPU_FLAG_STATS_ENABLED, false),
 #endif
+    DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
+                    VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 879f4cfff6..415aeddb07 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -85,6 +85,7 @@ enum virtio_gpu_base_conf_flags {
     VIRTIO_GPU_FLAG_STATS_ENABLED,
     VIRTIO_GPU_FLAG_EDID_ENABLED,
     VIRTIO_GPU_FLAG_DMABUF_ENABLED,
+    VIRTIO_GPU_FLAG_BLOB_ENABLED,
 };
 
 #define virtio_gpu_virgl_enabled(_cfg) \
@@ -95,6 +96,8 @@ enum virtio_gpu_base_conf_flags {
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_EDID_ENABLED))
 #define virtio_gpu_dmabuf_enabled(_cfg) \
     (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
+#define virtio_gpu_blob_enabled(_cfg) \
+    (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
 
 struct virtio_gpu_base_conf {
     uint32_t max_outputs;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/11] virtio-gpu: Add virtio_gpu_resource_create_blob
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (6 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 07/11] virtio-gpu: Add initial definitions for blob resources Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:09 ` [PATCH 09/11] virtio-gpu: Add helpers to create and destroy dmabuf objects Vivek Kasireddy
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

This API allows Qemu to register the blob allocated by the Guest
as a new resource and map its backing storage.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/trace-events              |  1 +
 hw/display/virtio-gpu-udmabuf.c      |  9 +++-
 hw/display/virtio-gpu.c              | 74 ++++++++++++++++++++++++++--
 include/hw/virtio/virtio-gpu-bswap.h |  9 ++++
 include/hw/virtio/virtio-gpu.h       |  2 +
 5 files changed, 90 insertions(+), 5 deletions(-)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 957b8ba994..99e5256aac 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -42,6 +42,7 @@ virtio_gpu_cmd_get_edid(uint32_t scanout) "scanout %d"
 virtio_gpu_cmd_set_scanout(uint32_t id, uint32_t res, uint32_t w, uint32_t h, uint32_t x, uint32_t y) "id %d, res 0x%x, w %d, h %d, x %d, y %d"
 virtio_gpu_cmd_res_create_2d(uint32_t res, uint32_t fmt, uint32_t w, uint32_t h) "res 0x%x, fmt 0x%x, w %d, h %d"
 virtio_gpu_cmd_res_create_3d(uint32_t res, uint32_t fmt, uint32_t w, uint32_t h, uint32_t d) "res 0x%x, fmt 0x%x, w %d, h %d, d %d"
+virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) "res 0x%x, size %" PRId64
 virtio_gpu_cmd_res_unref(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_attach(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_back_detach(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index 8d0fe1ce4a..bbcaf27b56 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -72,7 +72,10 @@ static void virtio_gpu_create_udmabuf(struct virtio_gpu_simple_resource *res)
 
 static void virtio_gpu_remap_udmabuf(struct virtio_gpu_simple_resource *res)
 {
-    res->remapsz = res->width * res->height * 4;
+    if (res->blob_size) {
+        res->remapsz = res->blob_size;
+    }
+
     res->remapsz = QEMU_ALIGN_UP(res->remapsz, getpagesize());
 
     res->remapped = mmap(NULL, res->remapsz, PROT_READ,
@@ -151,7 +154,9 @@ void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res)
         pdata = res->remapped;
     }
 
-    (void) pdata;
+    if (pdata) {
+        res->blob = pdata;
+    }
 }
 
 void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 30d3e38fa9..8a8cc7f181 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -182,7 +182,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
     }
 
     if (require_backing) {
-        if (!res->iov || !res->image) {
+        if (!res->iov || (!res->image && !res->blob)) {
             qemu_log_mask(LOG_GUEST_ERROR, "%s: no backing storage %d\n",
                           caller, resource_id);
             if (error) {
@@ -358,6 +358,63 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
     g->hostmem += res->hostmem;
 }
 
+static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
+                                            struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_resource_create_blob cblob;
+    int ret;
+
+    VIRTIO_GPU_FILL_CMD(cblob);
+    virtio_gpu_create_blob_bswap(&cblob);
+    trace_virtio_gpu_cmd_res_create_blob(cblob.resource_id, cblob.size);
+
+    if (cblob.resource_id == 0) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+                      __func__);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = virtio_gpu_find_resource(g, cblob.resource_id);
+    if (res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+                      __func__, cblob.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    res = g_new0(struct virtio_gpu_simple_resource, 1);
+    res->resource_id = cblob.resource_id;
+    res->blob_size = cblob.size;
+
+    if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_GUEST &&
+        cblob.blob_flags != VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid memory type\n",
+                      __func__);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        g_free(res);
+        return;
+    }
+
+    if (res->iov) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+
+    ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
+                                        cmd, &res->addrs, &res->iov);
+    if (ret != 0) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+
+    res->iov_cnt = cblob.nr_entries;
+    virtio_gpu_init_udmabuf(res);
+
+    QTAILQ_INSERT_HEAD(&g->reslist, res, next);
+}
+
 static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
 {
     struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
@@ -435,7 +492,7 @@ static void virtio_gpu_transfer_to_host_2d(VirtIOGPU *g,
 
     res = virtio_gpu_find_check_resource(g, t2d.resource_id, true,
                                          __func__, &cmd->error);
-    if (!res) {
+    if (!res || res->blob) {
         return;
     }
 
@@ -491,7 +548,7 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g,
 
     res = virtio_gpu_find_check_resource(g, rf.resource_id, false,
                                          __func__, &cmd->error);
-    if (!res) {
+    if (!res || res->blob) {
         return;
     }
 
@@ -743,6 +800,10 @@ static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
     res->iov_cnt = 0;
     g_free(res->addrs);
     res->addrs = NULL;
+
+    if (res->blob) {
+        virtio_gpu_fini_udmabuf(res);
+    }
 }
 
 static void
@@ -815,6 +876,13 @@ static void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
     case VIRTIO_GPU_CMD_RESOURCE_CREATE_2D:
         virtio_gpu_resource_create_2d(g, cmd);
         break;
+  case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
+        if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) {
+            cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+            break;
+        }
+        virtio_gpu_resource_create_blob(g, cmd);
+        break;
     case VIRTIO_GPU_CMD_RESOURCE_UNREF:
         virtio_gpu_resource_unref(g, cmd);
         break;
diff --git a/include/hw/virtio/virtio-gpu-bswap.h b/include/hw/virtio/virtio-gpu-bswap.h
index 203f9e1718..d23ac5cc4a 100644
--- a/include/hw/virtio/virtio-gpu-bswap.h
+++ b/include/hw/virtio/virtio-gpu-bswap.h
@@ -59,4 +59,13 @@ virtio_gpu_t2d_bswap(struct virtio_gpu_transfer_to_host_2d *t2d)
     le32_to_cpus(&t2d->padding);
 }
 
+static inline void
+virtio_gpu_create_blob_bswap(struct virtio_gpu_resource_create_blob *cblob)
+{
+    virtio_gpu_ctrl_hdr_bswap(&cblob->hdr);
+    le32_to_cpus(&cblob->resource_id);
+    le32_to_cpus(&cblob->blob_flags);
+    le64_to_cpus(&cblob->size);
+}
+
 #endif
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 415aeddb07..a65215dc52 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -48,6 +48,8 @@ struct virtio_gpu_simple_resource {
     pixman_image_t *image;
     uint64_t hostmem;
 
+    uint64_t blob_size;
+    void *blob;
     int dmabuf_fd;
     size_t remapsz;
     uint8_t *remapped;
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/11] virtio-gpu: Add helpers to create and destroy dmabuf objects
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (7 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 08/11] virtio-gpu: Add virtio_gpu_resource_create_blob Vivek Kasireddy
@ 2021-03-31  3:09 ` Vivek Kasireddy
  2021-03-31  3:10 ` [PATCH 10/11] virtio-gpu: Add virtio_gpu_set_scanout_blob Vivek Kasireddy
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:09 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy

These helpers can be useful for creating dmabuf objects from blobs
and submitting them to the UI.

Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu-udmabuf.c | 88 +++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-gpu.h  | 15 ++++++
 2 files changed, 103 insertions(+)

diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index bbcaf27b56..bda8af5458 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -166,6 +166,85 @@ void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
     }
 }
 
+static void virtio_gpu_free_one_dmabuf(VirtIOGPU *g, VGPUDMABuf *dmabuf,
+                                       uint32_t scanout_id)
+{
+    struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
+
+    QTAILQ_REMOVE(&g->dmabuf.bufs, dmabuf, next);
+    dpy_gl_release_dmabuf(scanout->con, &dmabuf->buf);
+    g_free(dmabuf);
+}
+
+static void virtio_gpu_free_dmabufs(VirtIOGPU *g,
+                                    uint32_t scanout_id)
+{
+    VGPUDMABuf *dmabuf, *tmp;
+
+    QTAILQ_FOREACH_SAFE(dmabuf, &g->dmabuf.bufs, next, tmp) {
+        if (dmabuf != g->dmabuf.primary &&
+            dmabuf->scanout_id == scanout_id) {
+            virtio_gpu_free_one_dmabuf(g, dmabuf, scanout_id);
+            break;
+        }
+    }
+}
+
+static VGPUDMABuf *virtio_gpu_create_dmabuf(VirtIOGPU *g,
+                                            uint32_t scanout_id,
+                                            struct virtio_gpu_simple_resource *res,
+                                            struct virtio_gpu_framebuffer *fb)
+{
+    VGPUDMABuf *dmabuf;
+
+    if (res->dmabuf_fd < 0) {
+        return NULL;
+    }
+
+    dmabuf = g_new0(VGPUDMABuf, 1);
+    dmabuf->buf.width = fb->width;
+    dmabuf->buf.height = fb->height;
+    dmabuf->buf.stride = fb->stride;
+    dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
+    dmabuf->buf.fd = res->dmabuf_fd;
+
+    dmabuf->scanout_id = scanout_id;
+    QTAILQ_INSERT_HEAD(&g->dmabuf.bufs, dmabuf, next);
+
+    return dmabuf;
+}
+
+int virtio_gpu_dmabuf_update(VirtIOGPU *g,
+                             uint32_t scanout_id,
+                             struct virtio_gpu_simple_resource *res,
+                             struct virtio_gpu_framebuffer *fb)
+{
+    struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
+    VGPUDMABuf *primary;
+    bool free_bufs = false;
+
+    primary = virtio_gpu_create_dmabuf(g, scanout_id, res, fb);
+    if (!primary) {
+        return -EINVAL;
+    }
+
+    if (g->dmabuf.primary != primary) {
+        g->dmabuf.primary = primary;
+        qemu_console_resize(scanout->con,
+                            primary->buf.width, primary->buf.height);
+        dpy_gl_scanout_dmabuf(scanout->con, &primary->buf);
+        free_bufs = true;
+    }
+
+    dpy_gl_update(scanout->con, 0, 0, primary->buf.width, primary->buf.height);
+
+    if (free_bufs) {
+        virtio_gpu_free_dmabufs(g, scanout_id);
+    }
+
+    return 0;
+}
+
 #else
 
 bool virtio_gpu_have_udmabuf(void)
@@ -184,4 +263,13 @@ void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res)
     /* nothing (stub) */
 }
 
+int virtio_gpu_dmabuf_update(VirtIOGPU *g,
+                             uint32_t scanout_id,
+                             struct virtio_gpu_simple_resource *res,
+                             struct virtio_gpu_framebuffer *fb)
+{
+    /* nothing (stub) */
+    return 0;
+}
+
 #endif
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index a65215dc52..8b6bf851da 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -149,6 +149,12 @@ struct VirtIOGPUBaseClass {
     DEFINE_PROP_UINT32("xres", _state, _conf.xres, 1024), \
     DEFINE_PROP_UINT32("yres", _state, _conf.yres, 768)
 
+typedef struct VGPUDMABuf {
+    QemuDmaBuf buf;
+    uint32_t scanout_id;
+    QTAILQ_ENTRY(VGPUDMABuf) next;
+} VGPUDMABuf;
+
 struct VirtIOGPU {
     VirtIOGPUBase parent_obj;
 
@@ -179,6 +185,11 @@ struct VirtIOGPU {
         uint32_t req_3d;
         uint32_t bytes_3d;
     } stats;
+
+    struct {
+        QTAILQ_HEAD(, VGPUDMABuf) bufs;
+        VGPUDMABuf *primary;
+    } dmabuf;
 };
 
 struct VhostUserGPU {
@@ -236,6 +247,10 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g);
 bool virtio_gpu_have_udmabuf(void);
 void virtio_gpu_init_udmabuf(struct virtio_gpu_simple_resource *res);
 void virtio_gpu_fini_udmabuf(struct virtio_gpu_simple_resource *res);
+int virtio_gpu_dmabuf_update(VirtIOGPU *g,
+                             uint32_t scanout_id,
+                             struct virtio_gpu_simple_resource *res,
+                             struct virtio_gpu_framebuffer *fb);
 
 /* virtio-gpu-3d.c */
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/11] virtio-gpu: Add virtio_gpu_set_scanout_blob
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (8 preceding siblings ...)
  2021-03-31  3:09 ` [PATCH 09/11] virtio-gpu: Add helpers to create and destroy dmabuf objects Vivek Kasireddy
@ 2021-03-31  3:10 ` Vivek Kasireddy
  2021-03-31  3:10 ` [PATCH 11/11] virtio-gpu: Update cursor data using blob Vivek Kasireddy
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

This API allows Qemu to set the blob allocated by the Guest as
the scanout buffer. If Opengl support is available, then the
scanout buffer would be submitted as a dmabuf to the UI; if not,
a pixman image is created from the scanout buffer and is
submitted to the UI via the display surface.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/trace-events                     |  1 +
 hw/display/virtio-gpu-udmabuf.c             |  1 +
 hw/display/virtio-gpu.c                     | 84 ++++++++++++++++++++-
 include/hw/virtio/virtio-gpu-bswap.h        |  7 ++
 include/hw/virtio/virtio-gpu.h              |  1 +
 include/standard-headers/linux/virtio_gpu.h |  1 +
 6 files changed, 91 insertions(+), 4 deletions(-)

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 99e5256aac..96fe1ea3de 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -40,6 +40,7 @@ virtio_gpu_features(bool virgl) "virgl %d"
 virtio_gpu_cmd_get_display_info(void) ""
 virtio_gpu_cmd_get_edid(uint32_t scanout) "scanout %d"
 virtio_gpu_cmd_set_scanout(uint32_t id, uint32_t res, uint32_t w, uint32_t h, uint32_t x, uint32_t y) "id %d, res 0x%x, w %d, h %d, x %d, y %d"
+virtio_gpu_cmd_set_scanout_blob(uint32_t id, uint32_t res, uint32_t w, uint32_t h, uint32_t x, uint32_t y) "id %d, res 0x%x, w %d, h %d, x %d, y %d"
 virtio_gpu_cmd_res_create_2d(uint32_t res, uint32_t fmt, uint32_t w, uint32_t h) "res 0x%x, fmt 0x%x, w %d, h %d"
 virtio_gpu_cmd_res_create_3d(uint32_t res, uint32_t fmt, uint32_t w, uint32_t h, uint32_t d) "res 0x%x, fmt 0x%x, w %d, h %d, d %d"
 virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) "res 0x%x, size %" PRId64
diff --git a/hw/display/virtio-gpu-udmabuf.c b/hw/display/virtio-gpu-udmabuf.c
index bda8af5458..6fbe709811 100644
--- a/hw/display/virtio-gpu-udmabuf.c
+++ b/hw/display/virtio-gpu-udmabuf.c
@@ -206,6 +206,7 @@ static VGPUDMABuf *virtio_gpu_create_dmabuf(VirtIOGPU *g,
     dmabuf->buf.height = fb->height;
     dmabuf->buf.stride = fb->stride;
     dmabuf->buf.fourcc = qemu_pixman_to_drm_format(fb->format);
+    dmabuf->buf.modifier = fb->modifier;
     dmabuf->buf.fd = res->dmabuf_fd;
 
     dmabuf->scanout_id = scanout_id;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 8a8cc7f181..642f95a269 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -449,7 +449,9 @@ static void virtio_gpu_resource_destroy(VirtIOGPU *g,
         }
     }
 
-    pixman_image_unref(res->image);
+    if (res->image) {
+        pixman_image_unref(res->image);
+    }
     virtio_gpu_cleanup_mapping(g, res);
     QTAILQ_REMOVE(&g->reslist, res, next);
     g->hostmem -= res->hostmem;
@@ -639,10 +641,22 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g,
     }
 
     g->parent_obj.enable = 1;
-    data = (uint8_t *)pixman_image_get_data(res->image);
+
+    if (res->blob) {
+        if (display_opengl) {
+            if (!virtio_gpu_dmabuf_update(g, scanout_id, res, fb)) {
+                return;
+            }
+        }
+
+        data = res->blob;
+    } else {
+        data = (uint8_t *)pixman_image_get_data(res->image);
+    }
 
     /* create a surface for this scanout */
-    if (!scanout->ds ||
+    if ((res->blob && !display_opengl) ||
+        !scanout->ds ||
         surface_data(scanout->ds) != data + fb->offset ||
         scanout->width != r->width ||
         scanout->height != r->height) {
@@ -716,6 +730,61 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
                               &fb, res, &ss.r, &cmd->error);
 }
 
+static void virtio_gpu_set_scanout_blob(VirtIOGPU *g,
+                                        struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_simple_resource *res;
+    struct virtio_gpu_framebuffer fb = { 0 };
+    struct virtio_gpu_set_scanout_blob ss;
+    uint64_t fbend;
+
+    VIRTIO_GPU_FILL_CMD(ss);
+    virtio_gpu_scanout_blob_bswap(&ss);
+    trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+                                          ss.r.width, ss.r.height, ss.r.x, ss.r.y);
+
+    if (ss.resource_id == 0) {
+        virtio_gpu_disable_scanout(g, ss.scanout_id);
+        return;
+    }
+
+    res = virtio_gpu_find_check_resource(g, ss.resource_id, true,
+                                         __func__, &cmd->error);
+    if (!res) {
+        return;
+    }
+
+    fb.format = virtio_gpu_get_pixman_format(ss.format);
+    if (!fb.format) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: host couldn't handle guest format %d\n",
+                      __func__, ss.format);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        return;
+    }
+
+    fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+    fb.width = ss.width;
+    fb.height = ss.height;
+    fb.stride = ss.strides[0];
+    fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+    fb.modifier = ss.modifier;
+
+    fbend = fb.offset;
+    fbend += fb.stride * (ss.r.height - 1);
+    fbend += fb.bytes_pp * ss.r.width;
+    if (fbend > res->blob_size) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: fb end out of range\n",
+                      __func__);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        return;
+    }
+
+    virtio_gpu_do_set_scanout(g, ss.scanout_id,
+                              &fb, res, &ss.r, &cmd->error);
+}
+
 int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
                                   uint32_t nr_entries, uint32_t offset,
                                   struct virtio_gpu_ctrl_command *cmd,
@@ -876,7 +945,7 @@ static void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
     case VIRTIO_GPU_CMD_RESOURCE_CREATE_2D:
         virtio_gpu_resource_create_2d(g, cmd);
         break;
-  case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
+    case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB:
         if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) {
             cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
             break;
@@ -895,6 +964,13 @@ static void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
     case VIRTIO_GPU_CMD_SET_SCANOUT:
         virtio_gpu_set_scanout(g, cmd);
         break;
+    case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB:
+        if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) {
+            cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+            break;
+        }
+        virtio_gpu_set_scanout_blob(g, cmd);
+        break;
     case VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING:
         virtio_gpu_resource_attach_backing(g, cmd);
         break;
diff --git a/include/hw/virtio/virtio-gpu-bswap.h b/include/hw/virtio/virtio-gpu-bswap.h
index d23ac5cc4a..e2bee8f595 100644
--- a/include/hw/virtio/virtio-gpu-bswap.h
+++ b/include/hw/virtio/virtio-gpu-bswap.h
@@ -68,4 +68,11 @@ virtio_gpu_create_blob_bswap(struct virtio_gpu_resource_create_blob *cblob)
     le64_to_cpus(&cblob->size);
 }
 
+static inline void
+virtio_gpu_scanout_blob_bswap(struct virtio_gpu_set_scanout_blob *ssb)
+{
+    virtio_gpu_bswap_32(ssb, sizeof(*ssb) - sizeof(ssb->offsets[3]));
+    le32_to_cpus(&ssb->offsets[3]);
+}
+
 #endif
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 8b6bf851da..2ad90ae632 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -63,6 +63,7 @@ struct virtio_gpu_framebuffer {
     uint32_t width, height;
     uint32_t stride;
     uint32_t offset;
+    uint64_t modifier;
 };
 
 struct virtio_gpu_scanout {
diff --git a/include/standard-headers/linux/virtio_gpu.h b/include/standard-headers/linux/virtio_gpu.h
index 1357e4774e..87992ca7ee 100644
--- a/include/standard-headers/linux/virtio_gpu.h
+++ b/include/standard-headers/linux/virtio_gpu.h
@@ -409,6 +409,7 @@ struct virtio_gpu_set_scanout_blob {
 	uint32_t width;
 	uint32_t height;
 	uint32_t format;
+	uint64_t modifier;
 	uint32_t padding;
 	uint32_t strides[4];
 	uint32_t offsets[4];
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/11] virtio-gpu: Update cursor data using blob
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (9 preceding siblings ...)
  2021-03-31  3:10 ` [PATCH 10/11] virtio-gpu: Add virtio_gpu_set_scanout_blob Vivek Kasireddy
@ 2021-03-31  3:10 ` Vivek Kasireddy
  2021-03-31  3:45 ` [PATCH 00/11] Add support for Blob resources feature no-reply
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Vivek Kasireddy @ 2021-03-31  3:10 UTC (permalink / raw)
  To: qemu-devel; +Cc: Vivek Kasireddy, Gerd Hoffmann

If a blob is available for the cursor, copy the data from the blob.

Based-on-patch-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 hw/display/virtio-gpu.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 642f95a269..c2f5290ffe 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -67,6 +67,7 @@ static void update_cursor_data_simple(VirtIOGPU *g,
 {
     struct virtio_gpu_simple_resource *res;
     uint32_t pixels;
+    void *data;
 
     res = virtio_gpu_find_check_resource(g, resource_id, false,
                                          __func__, NULL);
@@ -74,14 +75,22 @@ static void update_cursor_data_simple(VirtIOGPU *g,
         return;
     }
 
-    if (pixman_image_get_width(res->image)  != s->current_cursor->width ||
-        pixman_image_get_height(res->image) != s->current_cursor->height) {
-        return;
+    if (res->blob_size) {
+        if (res->blob_size < (s->current_cursor->width *
+                              s->current_cursor->height * 4)) {
+            return;
+        }
+        data = res->blob;
+    } else {
+        if (pixman_image_get_width(res->image)  != s->current_cursor->width ||
+            pixman_image_get_height(res->image) != s->current_cursor->height) {
+            return;
+        }
+        data = pixman_image_get_data(res->image);
     }
 
     pixels = s->current_cursor->width * s->current_cursor->height;
-    memcpy(s->current_cursor->data,
-           pixman_image_get_data(res->image),
+    memcpy(s->current_cursor->data, data,
            pixels * sizeof(uint32_t));
 }
 
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] Add support for Blob resources feature
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (10 preceding siblings ...)
  2021-03-31  3:10 ` [PATCH 11/11] virtio-gpu: Update cursor data using blob Vivek Kasireddy
@ 2021-03-31  3:45 ` no-reply
  2021-03-31  5:18 ` Zhang, Tina
  2021-04-13  6:10 ` Kasireddy, Vivek
  13 siblings, 0 replies; 20+ messages in thread
From: no-reply @ 2021-03-31  3:45 UTC (permalink / raw)
  To: vivek.kasireddy
  Cc: dongwon.kim, qemu-devel, vivek.kasireddy, tina.zhang, kraxel,
	marcandre.lureau

Patchew URL: https://patchew.org/QEMU/20210331031001.1564125-1-vivek.kasireddy@intel.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20210331031001.1564125-1-vivek.kasireddy@intel.com
Subject: [PATCH 00/11] Add support for Blob resources feature

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20210331031001.1564125-1-vivek.kasireddy@intel.com -> patchew/20210331031001.1564125-1-vivek.kasireddy@intel.com
Switched to a new branch 'test'
b446fd9 virtio-gpu: Update cursor data using blob
3239ca7 virtio-gpu: Add virtio_gpu_set_scanout_blob
f52199e virtio-gpu: Add helpers to create and destroy dmabuf objects
1dcd341 virtio-gpu: Add virtio_gpu_resource_create_blob
6550dc7 virtio-gpu: Add initial definitions for blob resources
d394612 virtio-gpu: Refactor virtio_gpu_create_mapping_iov
372d98c virtio-gpu: Refactor virtio_gpu_set_scanout
43aa8d5 virtio-gpu: Add virtio_gpu_find_check_resource
4ebddba virtio-gpu: Add udmabuf helpers
969da42 ui/pixman: Add qemu_pixman_to_drm_format()
d33ad11 ui: Get the fd associated with udmabuf driver

=== OUTPUT BEGIN ===
1/11 Checking commit d33ad11ce69f (ui: Get the fd associated with udmabuf driver)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#44: 
new file mode 100644

total: 0 errors, 1 warnings, 54 lines checked

Patch 1/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/11 Checking commit 969da423dd95 (ui/pixman: Add qemu_pixman_to_drm_format())
3/11 Checking commit 4ebddba7f74b (virtio-gpu: Add udmabuf helpers)
Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#39: 
new file mode 100644

ERROR: use qemu_real_host_page_size instead of getpagesize()
#119: FILE: hw/display/virtio-gpu-udmabuf.c:76:
+    res->remapsz = QEMU_ALIGN_UP(res->remapsz, getpagesize());

ERROR: braces {} are necessary for all arms of this statement
#169: FILE: hw/display/virtio-gpu-udmabuf.c:126:
+    if (udmabuf < 0)
[...]

total: 2 errors, 1 warnings, 265 lines checked

Patch 3/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/11 Checking commit 43aa8d5eb5dd (virtio-gpu: Add virtio_gpu_find_check_resource)
5/11 Checking commit 372d98ca604a (virtio-gpu: Refactor virtio_gpu_set_scanout)
6/11 Checking commit d3946125fd9b (virtio-gpu: Refactor virtio_gpu_create_mapping_iov)
7/11 Checking commit 6550dc790eef (virtio-gpu: Add initial definitions for blob resources)
8/11 Checking commit 1dcd3417ed0f (virtio-gpu: Add virtio_gpu_resource_create_blob)
9/11 Checking commit f52199e64d16 (virtio-gpu: Add helpers to create and destroy dmabuf objects)
WARNING: line over 80 characters
#51: FILE: hw/display/virtio-gpu-udmabuf.c:195:
+                                            struct virtio_gpu_simple_resource *res,

total: 0 errors, 1 warnings, 131 lines checked

Patch 9/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/11 Checking commit 3239ca755628 (virtio-gpu: Add virtio_gpu_set_scanout_blob)
WARNING: line over 80 characters
#104: FILE: hw/display/virtio-gpu.c:744:
+                                          ss.r.width, ss.r.height, ss.r.x, ss.r.y);

total: 0 errors, 1 warnings, 155 lines checked

Patch 10/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
11/11 Checking commit b446fd91fd53 (virtio-gpu: Update cursor data using blob)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20210331031001.1564125-1-vivek.kasireddy@intel.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 00/11] Add support for Blob resources feature
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (11 preceding siblings ...)
  2021-03-31  3:45 ` [PATCH 00/11] Add support for Blob resources feature no-reply
@ 2021-03-31  5:18 ` Zhang, Tina
  2021-04-13  6:10 ` Kasireddy, Vivek
  13 siblings, 0 replies; 20+ messages in thread
From: Zhang, Tina @ 2021-03-31  5:18 UTC (permalink / raw)
  To: Kasireddy, Vivek, qemu-devel, Gerd Hoffmann
  Cc: Marc-André Lureau, Kim,  Dongwon

Hi Gerd,

Since the udmabuf and blob supports are on their way, can we reconsider the display-drm ui support which was list in https://git.kraxel.org/cgit/qemu/refs/heads as a branch before.

I did some rebasing work and can help to send them out for review if possible.

BR,
Tina

> -----Original Message-----
> From: Kasireddy, Vivek <vivek.kasireddy@intel.com>
> Sent: Wednesday, March 31, 2021 11:10 AM
> To: qemu-devel@nongnu.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Marc-André Lureau
> <marcandre.lureau@redhat.com>; Kim, Dongwon
> <dongwon.kim@intel.com>; Zhang, Tina <tina.zhang@intel.com>
> Subject: [PATCH 00/11] Add support for Blob resources feature
> 
> Enabling this feature would eliminate data copies from the resource object in
> the Guest to the shadow resource in Qemu. This patch series however adds
> support only for Blobs of type VIRTIO_GPU_BLOB_MEM_GUEST with
> property VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE.
> 
> Most of the patches in this series are a rebased, refactored and bugfixed
> versions of Gerd Hoffmann's patches located here:
> https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next
> 
> TODO:
> - Enable the combination virgl + blob resources
> - Add support for VIRTGPU_BLOB_MEM_HOST3D blobs
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Cc: Tina Zhang <tina.zhang@intel.com>
> 
> Vivek Kasireddy (11):
>   ui: Get the fd associated with udmabuf driver
>   ui/pixman: Add qemu_pixman_to_drm_format()
>   virtio-gpu: Add udmabuf helpers
>   virtio-gpu: Add virtio_gpu_find_check_resource
>   virtio-gpu: Refactor virtio_gpu_set_scanout
>   virtio-gpu: Refactor virtio_gpu_create_mapping_iov
>   virtio-gpu: Add initial definitions for blob resources
>   virtio-gpu: Add virtio_gpu_resource_create_blob
>   virtio-gpu: Add helpers to create and destroy dmabuf objects
>   virtio-gpu: Add virtio_gpu_set_scanout_blob
>   virtio-gpu: Update cursor data using blob
> 
>  hw/display/meson.build                      |   2 +-
>  hw/display/trace-events                     |   2 +
>  hw/display/virtio-gpu-3d.c                  |   3 +-
>  hw/display/virtio-gpu-base.c                |   3 +
>  hw/display/virtio-gpu-udmabuf.c             | 276 +++++++++++++
>  hw/display/virtio-gpu.c                     | 423 +++++++++++++++-----
>  include/hw/virtio/virtio-gpu-bswap.h        |  16 +
>  include/hw/virtio/virtio-gpu.h              |  41 +-
>  include/standard-headers/linux/udmabuf.h    |  32 ++
>  include/standard-headers/linux/virtio_gpu.h |   1 +
>  include/ui/console.h                        |   3 +
>  include/ui/qemu-pixman.h                    |   1 +
>  scripts/update-linux-headers.sh             |   3 +
>  ui/meson.build                              |   1 +
>  ui/qemu-pixman.c                            |  35 +-
>  ui/udmabuf.c                                |  40 ++
>  16 files changed, 772 insertions(+), 110 deletions(-)  create mode 100644
> hw/display/virtio-gpu-udmabuf.c  create mode 100644 include/standard-
> headers/linux/udmabuf.h
>  create mode 100644 ui/udmabuf.c
> 
> --
> 2.26.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 00/11] Add support for Blob resources feature
  2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
                   ` (12 preceding siblings ...)
  2021-03-31  5:18 ` Zhang, Tina
@ 2021-04-13  6:10 ` Kasireddy, Vivek
  2021-04-14  9:45   ` Gerd Hoffmann
  13 siblings, 1 reply; 20+ messages in thread
From: Kasireddy, Vivek @ 2021-04-13  6:10 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel
  Cc: Marc-André Lureau, Kim,  Dongwon, Zhang, Tina

Hi Gerd,
While looking at the Qemu UI code, I noticed that there is a Blit operation
performed to copy the Guest FB (texture) into a Host buffer before it is presented
to the Host compositor. I was wondering if there are any elegant ways to
eliminate this Blit to further the goal of absolute zero-copy. One idea that I am 
familiar with involves the usage of this extension:
https://www.khronos.org/registry/EGL/extensions/WL/EGL_WL_create_wayland_buffer_from_image.txt

However, I do realize that this is very Wayland specific but we are also going to need
to add explicit sync support which is currently only available in upstream Weston. 
I did try adding explicit sync support to GTK but the maintainers are not particularly
thrilled about it:
https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3410

Any other ideas as to how to eliminate that Blit cleanly?

Thanks,
Vivek

> -----Original Message-----
> From: Kasireddy, Vivek <vivek.kasireddy@intel.com>
> Sent: Tuesday, March 30, 2021 8:10 PM
> To: qemu-devel@nongnu.org
> Cc: Kasireddy, Vivek <vivek.kasireddy@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Marc-André Lureau <marcandre.lureau@redhat.com>; Kim,
> Dongwon <dongwon.kim@intel.com>; Zhang, Tina <tina.zhang@intel.com>
> Subject: [PATCH 00/11] Add support for Blob resources feature
> 
> Enabling this feature would eliminate data copies from the resource object in the Guest to
> the shadow resource in Qemu. This patch series however adds support only for Blobs of
> type VIRTIO_GPU_BLOB_MEM_GUEST with property
> VIRTIO_GPU_BLOB_FLAG_USE_SHAREABLE.
> 
> Most of the patches in this series are a rebased, refactored and bugfixed versions of Gerd
> Hoffmann's patches located here:
> https://gitlab.freedesktop.org/virgl/qemu/-/commits/virtio-gpu-next
> 
> TODO:
> - Enable the combination virgl + blob resources
> - Add support for VIRTGPU_BLOB_MEM_HOST3D blobs
> 
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Cc: Dongwon Kim <dongwon.kim@intel.com>
> Cc: Tina Zhang <tina.zhang@intel.com>
> 
> Vivek Kasireddy (11):
>   ui: Get the fd associated with udmabuf driver
>   ui/pixman: Add qemu_pixman_to_drm_format()
>   virtio-gpu: Add udmabuf helpers
>   virtio-gpu: Add virtio_gpu_find_check_resource
>   virtio-gpu: Refactor virtio_gpu_set_scanout
>   virtio-gpu: Refactor virtio_gpu_create_mapping_iov
>   virtio-gpu: Add initial definitions for blob resources
>   virtio-gpu: Add virtio_gpu_resource_create_blob
>   virtio-gpu: Add helpers to create and destroy dmabuf objects
>   virtio-gpu: Add virtio_gpu_set_scanout_blob
>   virtio-gpu: Update cursor data using blob
> 
>  hw/display/meson.build                      |   2 +-
>  hw/display/trace-events                     |   2 +
>  hw/display/virtio-gpu-3d.c                  |   3 +-
>  hw/display/virtio-gpu-base.c                |   3 +
>  hw/display/virtio-gpu-udmabuf.c             | 276 +++++++++++++
>  hw/display/virtio-gpu.c                     | 423 +++++++++++++++-----
>  include/hw/virtio/virtio-gpu-bswap.h        |  16 +
>  include/hw/virtio/virtio-gpu.h              |  41 +-
>  include/standard-headers/linux/udmabuf.h    |  32 ++
>  include/standard-headers/linux/virtio_gpu.h |   1 +
>  include/ui/console.h                        |   3 +
>  include/ui/qemu-pixman.h                    |   1 +
>  scripts/update-linux-headers.sh             |   3 +
>  ui/meson.build                              |   1 +
>  ui/qemu-pixman.c                            |  35 +-
>  ui/udmabuf.c                                |  40 ++
>  16 files changed, 772 insertions(+), 110 deletions(-)  create mode 100644
> hw/display/virtio-gpu-udmabuf.c  create mode 100644 include/standard-
> headers/linux/udmabuf.h
>  create mode 100644 ui/udmabuf.c
> 
> --
> 2.26.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] Add support for Blob resources feature
  2021-04-13  6:10 ` Kasireddy, Vivek
@ 2021-04-14  9:45   ` Gerd Hoffmann
  2021-04-14 21:26     ` Kasireddy, Vivek
  0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2021-04-14  9:45 UTC (permalink / raw)
  To: Kasireddy, Vivek
  Cc: Marc-André Lureau, Zhang, Tina, qemu-devel, Kim, Dongwon

  Hi,

> Any other ideas as to how to eliminate that Blit cleanly?

Well, "cleanly" pretty much implies "supported by toolkit".

gtk glarea for example sets up a framebuffer and expects the application
render to that framebuffer.  So qemu glarea code does a fb-to-fb blit.

Other reasons are scaling and cursor rendering.  Not all reasons apply
to all UIs.  I think when using spice qemu doesn't blit (not fully sure
what happens inside spice-server), but it could very well be that the
spice-client does the blit instead, i.e. we just shift the issue to
another place ...

take care,
  Gerd



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 00/11] Add support for Blob resources feature
  2021-04-14  9:45   ` Gerd Hoffmann
@ 2021-04-14 21:26     ` Kasireddy, Vivek
  2021-04-15  7:43       ` Gerd Hoffmann
  0 siblings, 1 reply; 20+ messages in thread
From: Kasireddy, Vivek @ 2021-04-14 21:26 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Marc-André Lureau, Zhang, Tina, qemu-devel, Kim, Dongwon

Hi Gerd,

> 
> > Any other ideas as to how to eliminate that Blit cleanly?
> 
> Well, "cleanly" pretty much implies "supported by toolkit".
[Kasireddy, Vivek] I was kind of hoping you'd not draw that implication :)
> 
> gtk glarea for example sets up a framebuffer and expects the application render to that
> framebuffer.  So qemu glarea code does a fb-to-fb blit.
[Kasireddy, Vivek] Right, that is how it works today but we'd like to not have that
blit and instead submit the Qemu application buffer (i.e Guest FB) directly to the
compositor  -- so that it can be placed on a hardware plane should the compositor
decide to do so. Only then it'd be absolute zero-copy but the compositor may decide
to composite it which would mean another blit that we cannot avoid. 

I am trying to make a concerted effort to accomplish this using GTK/GLArea:
https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3410

But I get a feeling that it is inadequate as GTK/GLArea does not manage the wl_buffers
submitted to the compositor -- EGL does. I suspect we either need to use a new GTK
mechanism -- that perhaps does not exist yet -- or not use GTK at all for this.

I do understand that adding a new purely Wayland backend would make it redundant given
that GTK, SDL, Spice, etc already support Wayland; however, I do not see any good options
available for eliminating that blit.

Thanks,
Vivek

> 
> Other reasons are scaling and cursor rendering.  Not all reasons apply to all UIs.  I think
> when using spice qemu doesn't blit (not fully sure what happens inside spice-server), but it
> could very well be that the spice-client does the blit instead, i.e. we just shift the issue to
> another place ...
> 
> take care,
>   Gerd



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] Add support for Blob resources feature
  2021-04-14 21:26     ` Kasireddy, Vivek
@ 2021-04-15  7:43       ` Gerd Hoffmann
  2021-04-16  6:33         ` Kasireddy, Vivek
  0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2021-04-15  7:43 UTC (permalink / raw)
  To: Kasireddy, Vivek
  Cc: Marc-André Lureau, Zhang, Tina, qemu-devel, Kim, Dongwon

  Hi,

> But I get a feeling that it is inadequate as GTK/GLArea does not manage the wl_buffers
> submitted to the compositor -- EGL does. I suspect we either need to use a new GTK
> mechanism -- that perhaps does not exist yet -- or not use GTK at all for this.
> 
> I do understand that adding a new purely Wayland backend would make it redundant given
> that GTK, SDL, Spice, etc already support Wayland; however, I do not see any good options
> available for eliminating that blit.

Well, one idea is using dbus (discovery/setup) and pipewire (data
streams) to allow other applications access the guest display (also
audio, input, ...).  Simliar to gnome exporting the wayland display
that way for remote access and screen sharing.

pipewire supports using dmabufs, so that should allow to decouple user
interface code from qemu without making compromises on performance,
which is probably quite useful for a number of use cases, like a
zero-copy wayland client, or a client using drm directly.

Right now pipewire support is at "idea" level, there isn't even a
proof-of-concept for that.  So it surely doesn't help short-term, but
IMHO this makes alot of sense medium/long-term.

Marc-André has experimental code for a dbus-based UI for qemu.  It
doesn't use pipewire as data transport though.  At least the first
version posted a while ago @ qemu-devel doesn't.

take care,
  Gerd



^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: [PATCH 00/11] Add support for Blob resources feature
  2021-04-15  7:43       ` Gerd Hoffmann
@ 2021-04-16  6:33         ` Kasireddy, Vivek
  2021-04-16  7:56           ` Gerd Hoffmann
  0 siblings, 1 reply; 20+ messages in thread
From: Kasireddy, Vivek @ 2021-04-16  6:33 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Marc-André Lureau, Zhang, Tina, qemu-devel, Kim, Dongwon

Hi Gerd,

> > I do understand that adding a new purely Wayland backend would make it
> > redundant given that GTK, SDL, Spice, etc already support Wayland;
> > however, I do not see any good options available for eliminating that blit.
> 
> Well, one idea is using dbus (discovery/setup) and pipewire (data
> streams) to allow other applications access the guest display (also audio, input, ...).
> Simliar to gnome exporting the wayland display that way for remote access and screen
> sharing.
> 
> pipewire supports using dmabufs, so that should allow to decouple user interface code
> from qemu without making compromises on performance, which is probably quite useful
> for a number of use cases, like a zero-copy wayland client, or a client using drm directly.
[Kasireddy, Vivek] We considered having a separate client but decided that adding the
relevant pieces to Qemu UI would be sufficient. We also felt that the interaction between
the client and Qemu for sharing the dmabuf (guest FB) would add to the overhead and
exacerbate synchronization issues. And, maintaining and distributing such a client would 
probably not be prudent for us right now.

> 
> Right now pipewire support is at "idea" level, there isn't even a proof-of-concept for that.
> So it surely doesn't help short-term, but IMHO this makes alot of sense medium/long-
> term.
[Kasireddy, Vivek] Ok, we'll explore this idea further to see if it would work for our use-case. 

> 
> Marc-André has experimental code for a dbus-based UI for qemu.  It doesn't use pipewire
> as data transport though.  At least the first version posted a while ago @ qemu-devel
> doesn't.
[Kasireddy, Vivek] What is the main motivation for a new dbus-based UI for Qemu?
Also, would you be able to review the patches in this series soon?

Thanks,
Vivek

> 
> take care,
>   Gerd



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/11] Add support for Blob resources feature
  2021-04-16  6:33         ` Kasireddy, Vivek
@ 2021-04-16  7:56           ` Gerd Hoffmann
  0 siblings, 0 replies; 20+ messages in thread
From: Gerd Hoffmann @ 2021-04-16  7:56 UTC (permalink / raw)
  To: Kasireddy, Vivek
  Cc: Marc-André Lureau, Zhang, Tina, qemu-devel, Kim, Dongwon

  Hi,

> > Marc-André has experimental code for a dbus-based UI for qemu.  It doesn't use pipewire
> > as data transport though.  At least the first version posted a while ago @ qemu-devel
> > doesn't.
> [Kasireddy, Vivek] What is the main motivation for a new dbus-based UI for Qemu?

Having UI and qemu run in separate processes has serveral advantages,
for starters the VM is independent from the desktop session.  Today you
can use vnc or spice, the dbus UI should allow better performance and
integration than that.  If it works out well we maybe we can even drop
the sdl/gtk UIs.

> Also, would you be able to review the patches in this series soon?

I expect there will be a v2 anyway due to the kernel-side changes?

take care,
  Gerd



^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-04-16  7:57 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-31  3:09 [PATCH 00/11] Add support for Blob resources feature Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 01/11] ui: Get the fd associated with udmabuf driver Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 02/11] ui/pixman: Add qemu_pixman_to_drm_format() Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 03/11] virtio-gpu: Add udmabuf helpers Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 04/11] virtio-gpu: Add virtio_gpu_find_check_resource Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 05/11] virtio-gpu: Refactor virtio_gpu_set_scanout Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 06/11] virtio-gpu: Refactor virtio_gpu_create_mapping_iov Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 07/11] virtio-gpu: Add initial definitions for blob resources Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 08/11] virtio-gpu: Add virtio_gpu_resource_create_blob Vivek Kasireddy
2021-03-31  3:09 ` [PATCH 09/11] virtio-gpu: Add helpers to create and destroy dmabuf objects Vivek Kasireddy
2021-03-31  3:10 ` [PATCH 10/11] virtio-gpu: Add virtio_gpu_set_scanout_blob Vivek Kasireddy
2021-03-31  3:10 ` [PATCH 11/11] virtio-gpu: Update cursor data using blob Vivek Kasireddy
2021-03-31  3:45 ` [PATCH 00/11] Add support for Blob resources feature no-reply
2021-03-31  5:18 ` Zhang, Tina
2021-04-13  6:10 ` Kasireddy, Vivek
2021-04-14  9:45   ` Gerd Hoffmann
2021-04-14 21:26     ` Kasireddy, Vivek
2021-04-15  7:43       ` Gerd Hoffmann
2021-04-16  6:33         ` Kasireddy, Vivek
2021-04-16  7:56           ` Gerd Hoffmann

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.