diff mbox series

[v8,09/11] virtio-gpu: Resource UUID

Message ID 20240418190040.1110210-10-dmitry.osipenko@collabora.com
State New
Headers show
Series Support blob memory and venus on qemu | expand

Commit Message

Dmitry Osipenko April 18, 2024, 7 p.m. UTC 
From: Antonio Caggiano <antonio.caggiano@collabora.com>

Enable resource UUID feature and implement command resource assign UUID.
UUID feature availability is mandatory for Vulkan Venus context.

UUID is intended for sharing dmabufs between virtio devices on host. Qemu
doesn't have second virtio device for sharing, thus a simple stub UUID
implementation is enough. More complete implementation using global UUID
resource table might become interesting for a multi-gpu cases.

Signed-off-by: Antonio Caggiano <antonio.caggiano@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 hw/display/trace-events        |  1 +
 hw/display/virtio-gpu-base.c   |  1 +
 hw/display/virtio-gpu-virgl.c  | 31 +++++++++++++++++++++++++++++++
 hw/display/virtio-gpu.c        |  5 +++++
 include/hw/virtio/virtio-gpu.h |  3 +++
 5 files changed, 41 insertions(+)

Comments

Akihiko Odaki April 19, 2024, 9:29 a.m. UTC | #1 
On 2024/04/19 4:00, Dmitry Osipenko wrote:
> From: Antonio Caggiano <antonio.caggiano@collabora.com>
> 
> Enable resource UUID feature and implement command resource assign UUID.
> UUID feature availability is mandatory for Vulkan Venus context.
> 
> UUID is intended for sharing dmabufs between virtio devices on host. Qemu
> doesn't have second virtio device for sharing, thus a simple stub UUID
> implementation is enough. More complete implementation using global UUID
> resource table might become interesting for a multi-gpu cases.

Isn't it possible to add two virtio-gpu devices even now?

A new subsection should also be added for migration compatibility; see: 
docs/devel/migration/main.rst
Dmitry Osipenko April 23, 2024, 5:43 p.m. UTC | #2 
On 4/19/24 12:29, Akihiko Odaki wrote:
> On 2024/04/19 4:00, Dmitry Osipenko wrote:
>> From: Antonio Caggiano <antonio.caggiano@collabora.com>
>>
>> Enable resource UUID feature and implement command resource assign UUID.
>> UUID feature availability is mandatory for Vulkan Venus context.
>>
>> UUID is intended for sharing dmabufs between virtio devices on host. Qemu
>> doesn't have second virtio device for sharing, thus a simple stub UUID
>> implementation is enough. More complete implementation using global UUID
>> resource table might become interesting for a multi-gpu cases.
> 
> Isn't it possible to add two virtio-gpu devices even now?

We can add two virtio-gpu devices, but these devices can't interact with
each other efficiently. They won't be able to share host blob resources
without proper UUID implementation.

> A new subsection should also be added for migration compatibility; see:
> docs/devel/migration/main.rst

Will update the docs, thanks.
Dmitry Osipenko April 24, 2024, 12:52 p.m. UTC | #3 
On 4/18/24 22:00, Dmitry Osipenko wrote:
> @@ -1405,6 +1408,8 @@ static int virtio_gpu_blob_load(QEMUFile *f, void *opaque, size_t size,
>              res->iov[i].iov_len = qemu_get_be32(f);
>          }
>  
> +        qemu_get_buffer(f, res->uuid.data, sizeof(res->uuid.data));

Save/loading uuid without changing vm version was a bad idea. Will drop
it in v9, we don't need to save/load uuid for virgl anyways.
diff mbox series

Patch

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 2336a0ca1570..54d6894c59f4 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -41,6 +41,7 @@  virtio_gpu_cmd_res_create_blob(uint32_t res, uint64_t size) "res 0x%x, size %" P
 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"
+virtio_gpu_cmd_res_assign_uuid(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_2d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_toh_3d(uint32_t res) "res 0x%x"
 virtio_gpu_cmd_res_xfer_fromh_3d(uint32_t res) "res 0x%x"
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index 4fc7ef8896c1..610926348bd9 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -225,6 +225,7 @@  virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t features,
     if (virtio_gpu_virgl_enabled(g->conf) ||
         virtio_gpu_rutabaga_enabled(g->conf)) {
         features |= (1 << VIRTIO_GPU_F_VIRGL);
+        features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
     }
     if (virtio_gpu_edid_enabled(g->conf)) {
         features |= (1 << VIRTIO_GPU_F_EDID);
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index de132b22f554..eee3816b987f 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -160,6 +160,7 @@  static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
     res->format = c2d.format;
     res->resource_id = c2d.resource_id;
     res->dmabuf_fd = -1;
+    qemu_uuid_generate(&res->uuid);
     QTAILQ_INSERT_HEAD(&g->reslist, res, next);
 
     args.handle = c2d.resource_id;
@@ -208,6 +209,7 @@  static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
     res->format = c3d.format;
     res->resource_id = c3d.resource_id;
     res->dmabuf_fd = -1;
+    qemu_uuid_generate(&res->uuid);
     QTAILQ_INSERT_HEAD(&g->reslist, res, next);
 
     args.handle = c3d.resource_id;
@@ -635,6 +637,7 @@  static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
     res->resource_id = cblob.resource_id;
     res->blob_size = cblob.size;
     res->dmabuf_fd = -1;
+    qemu_uuid_generate(&res->uuid);
 
     if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) {
         ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob),
@@ -833,6 +836,31 @@  static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
 }
 #endif /* HAVE_VIRGL_RESOURCE_BLOB */
 
+static void virgl_cmd_assign_uuid(VirtIOGPU *g,
+                                  struct virtio_gpu_ctrl_command *cmd)
+{
+    struct virtio_gpu_resource_assign_uuid assign;
+    struct virtio_gpu_resp_resource_uuid resp;
+    struct virtio_gpu_simple_resource *res;
+
+    VIRTIO_GPU_FILL_CMD(assign);
+    virtio_gpu_bswap_32(&assign, sizeof(assign));
+    trace_virtio_gpu_cmd_res_assign_uuid(assign.resource_id);
+
+    res = virtio_gpu_find_resource(g, assign.resource_id);
+    if (!res) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+                      __func__, assign.resource_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+        return;
+    }
+
+    memset(&resp, 0, sizeof(resp));
+    resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
+    memcpy(resp.uuid, res->uuid.data, sizeof(resp.uuid));
+    virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
+}
+
 void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
                                       struct virtio_gpu_ctrl_command *cmd)
 {
@@ -887,6 +915,9 @@  void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
         /* TODO add security */
         virgl_cmd_ctx_detach_resource(g, cmd);
         break;
+    case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
+        virgl_cmd_assign_uuid(g, cmd);
+        break;
     case VIRTIO_GPU_CMD_GET_CAPSET_INFO:
         virgl_cmd_get_capset_info(g, cmd);
         break;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 45c1f2006712..fbf5c0e6b8b7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1236,6 +1236,7 @@  static int virtio_gpu_save(QEMUFile *f, void *opaque, size_t size,
         }
         qemu_put_buffer(f, (void *)pixman_image_get_data(res->image),
                         pixman_image_get_stride(res->image) * res->height);
+        qemu_put_buffer(f, res->uuid.data, sizeof(res->uuid.data));
     }
     qemu_put_be32(f, 0); /* end of list */
 
@@ -1333,6 +1334,7 @@  static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
         }
         qemu_get_buffer(f, (void *)pixman_image_get_data(res->image),
                         pixman_image_get_stride(res->image) * res->height);
+        qemu_get_buffer(f, res->uuid.data, sizeof(res->uuid.data));
 
         if (!virtio_gpu_load_restore_mapping(g, res)) {
             pixman_image_unref(res->image);
@@ -1371,6 +1373,7 @@  static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
             qemu_put_be64(f, res->addrs[i]);
             qemu_put_be32(f, res->iov[i].iov_len);
         }
+        qemu_put_buffer(f, res->uuid.data, sizeof(res->uuid.data));
     }
     qemu_put_be32(f, 0); /* end of list */
 
@@ -1405,6 +1408,8 @@  static int virtio_gpu_blob_load(QEMUFile *f, void *opaque, size_t size,
             res->iov[i].iov_len = qemu_get_be32(f);
         }
 
+        qemu_get_buffer(f, res->uuid.data, sizeof(res->uuid.data));
+
         if (!virtio_gpu_load_restore_mapping(g, res)) {
             g_free(res);
             return -EINVAL;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index b9d5e106f3c5..d2a0d542fbb3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -19,6 +19,7 @@ 
 #include "ui/console.h"
 #include "hw/virtio/virtio.h"
 #include "qemu/log.h"
+#include "qemu/uuid.h"
 #include "sysemu/vhost-user-backend.h"
 
 #include "standard-headers/linux/virtio_gpu.h"
@@ -65,6 +66,8 @@  struct virtio_gpu_simple_resource {
     bool async_unmap_completed;
     bool async_unmap_in_progress;
 
+    QemuUUID uuid;
+
     QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
 };