|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [QEMU PATCH v5 10/13] virtio-gpu: Resource UUID
Hi
On Fri, Sep 15, 2023 at 3:14 PM Huang Rui <ray.huang@xxxxxxx> wrote:
>
> From: Antonio Caggiano <antonio.caggiano@xxxxxxxxxxxxx>
>
> Enable resource UUID feature and implement command resource assign UUID.
> This is done by introducing a hash table to map resource IDs to their
> UUIDs.
>
> Signed-off-by: Antonio Caggiano <antonio.caggiano@xxxxxxxxxxxxx>
> Signed-off-by: Huang Rui <ray.huang@xxxxxxx>
> ---
>
> V4 -> V5:
> - Add virtio migration handling for uuid (Akihiko)
> - Adjust sequence to allocate gpu resource before virglrender resource
> creation (Akihiko)
> - Clean up (Akihiko)
>
> hw/display/trace-events | 1 +
> hw/display/virtio-gpu-base.c | 2 ++
> hw/display/virtio-gpu-virgl.c | 21 ++++++++++++
> hw/display/virtio-gpu.c | 58 ++++++++++++++++++++++++++++++++++
> include/hw/virtio/virtio-gpu.h | 6 ++++
> 5 files changed, 88 insertions(+)
>
> diff --git a/hw/display/trace-events b/hw/display/trace-events
> index 2336a0ca15..54d6894c59 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 4f2b0ba1f3..f44388715c 100644
> --- a/hw/display/virtio-gpu-base.c
> +++ b/hw/display/virtio-gpu-base.c
> @@ -236,6 +236,8 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t
> features,
> features |= (1 << VIRTIO_GPU_F_CONTEXT_INIT);
> }
>
> + features |= (1 << VIRTIO_GPU_F_RESOURCE_UUID);
This changes what is exposed to the guest. We should have an option
for it, and set it to false on older machine types (similar to what is
done for edid).
> +
> return features;
> }
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 563a6f2f58..8a017dbeb4 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -36,11 +36,20 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
> {
> struct virtio_gpu_resource_create_2d c2d;
> struct virgl_renderer_resource_create_args args;
> + struct virtio_gpu_simple_resource *res;
>
> VIRTIO_GPU_FILL_CMD(c2d);
> trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
> c2d.width, c2d.height);
>
> + res = g_new0(struct virtio_gpu_simple_resource, 1);
> + if (!res) {
needless OOM here too
> + cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
> + return;
> + }
> + res->resource_id = c2d.resource_id;
> + QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> +
> args.handle = c2d.resource_id;
> args.target = 2;
> args.format = c2d.format;
> @@ -60,11 +69,20 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
> {
> struct virtio_gpu_resource_create_3d c3d;
> struct virgl_renderer_resource_create_args args;
> + struct virtio_gpu_simple_resource *res;
>
> VIRTIO_GPU_FILL_CMD(c3d);
> trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
> c3d.width, c3d.height, c3d.depth);
>
> + res = g_new0(struct virtio_gpu_simple_resource, 1);
> + if (!res) {
same
> + cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
> + return;
> + }
> + res->resource_id = c3d.resource_id;
> + QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> +
> args.handle = c3d.resource_id;
> args.target = c3d.target;
> args.format = c3d.format;
> @@ -682,6 +700,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:
> + virtio_gpu_resource_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 cc4c1f81bb..44414c1c5e 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -966,6 +966,38 @@ virtio_gpu_resource_detach_backing(VirtIOGPU *g,
> virtio_gpu_cleanup_mapping(g, res);
> }
>
> +void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_simple_resource *res;
> + struct virtio_gpu_resource_assign_uuid assign;
> + struct virtio_gpu_resp_resource_uuid resp;
> + QemuUUID *uuid;
> +
> + 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_check_resource(g, assign.resource_id, false,
> __func__, &cmd->error);
> + if (!res) {
> + return;
> + }
> +
> + memset(&resp, 0, sizeof(resp));
> + resp.hdr.type = VIRTIO_GPU_RESP_OK_RESOURCE_UUID;
> +
> + uuid = g_hash_table_lookup(g->resource_uuids,
> GUINT_TO_POINTER(assign.resource_id));
> + if (!uuid) {
> + uuid = g_new(QemuUUID, 1);
> + qemu_uuid_generate(uuid);
> + g_hash_table_insert(g->resource_uuids,
> GUINT_TO_POINTER(assign.resource_id), uuid);
> + res->has_uuid = true;
> + }
> +
> + memcpy(resp.uuid, uuid, sizeof(QemuUUID));
> + virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
> +}
> +
> void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
> struct virtio_gpu_ctrl_command *cmd)
> {
> @@ -1014,6 +1046,9 @@ void virtio_gpu_simple_process_cmd(VirtIOGPU *g,
> case VIRTIO_GPU_CMD_RESOURCE_DETACH_BACKING:
> virtio_gpu_resource_detach_backing(g, cmd);
> break;
> + case VIRTIO_GPU_CMD_RESOURCE_ASSIGN_UUID:
> + virtio_gpu_resource_assign_uuid(g, cmd);
> + break;
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> @@ -1208,6 +1243,7 @@ static int virtio_gpu_save(QEMUFile *f, void *opaque,
> size_t size,
> VirtIOGPU *g = opaque;
> struct virtio_gpu_simple_resource *res;
> int i;
> + QemuUUID *uuid;
>
> /* in 2d mode we should never find unprocessed commands here */
> assert(QTAILQ_EMPTY(&g->cmdq));
> @@ -1224,9 +1260,17 @@ 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_byte(f, res->has_uuid);
> + if (res->has_uuid) {
> + uuid = g_hash_table_lookup(g->resource_uuids,
> GUINT_TO_POINTER(res->resource_id));
> + qemu_put_buffer(f, (void *)uuid, sizeof(QemuUUID));
> + }
This is breaking the migration format. It will need to be handled with
an optional subsection instead.
> }
> qemu_put_be32(f, 0); /* end of list */
>
> + g_hash_table_destroy(g->resource_uuids);
> +
> return vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL);
> }
>
> @@ -1239,9 +1283,12 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,
> size_t size,
> uint32_t resource_id, pformat;
> void *bits = NULL;
> int i;
> + QemuUUID *uuid = NULL;
>
> g->hostmem = 0;
>
> + g->resource_uuids = g_hash_table_new_full(NULL, NULL, NULL, g_free);
Since it maps int -> uuid, it should probably use g_int_hash() /
g_int_qual() instead of the default (g_direct_hash()).
> +
> resource_id = qemu_get_be32(f);
> while (resource_id != 0) {
> res = virtio_gpu_find_resource(g, resource_id);
> @@ -1292,6 +1339,12 @@ 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);
>
> + res->has_uuid = qemu_get_byte(f);
> + if (res->has_uuid) {
> + qemu_get_buffer(f, (void *)uuid, sizeof(QemuUUID));
> + g_hash_table_insert(g->resource_uuids,
> GUINT_TO_POINTER(res->resource_id), uuid);
> + }
> +
> /* restore mapping */
> for (i = 0; i < res->iov_cnt; i++) {
> hwaddr len = res->iov[i].iov_len;
> @@ -1393,12 +1446,15 @@ void virtio_gpu_device_realize(DeviceState *qdev,
> Error **errp)
> QTAILQ_INIT(&g->reslist);
> QTAILQ_INIT(&g->cmdq);
> QTAILQ_INIT(&g->fenceq);
> +
> + g->resource_uuids = g_hash_table_new_full(NULL, NULL, NULL, g_free);
> }
>
> static void virtio_gpu_device_unrealize(DeviceState *qdev)
> {
> VirtIOGPU *g = VIRTIO_GPU(qdev);
>
> + g_hash_table_destroy(g->resource_uuids);
> g_clear_pointer(&g->ctrl_bh, qemu_bh_delete);
> g_clear_pointer(&g->cursor_bh, qemu_bh_delete);
> g_clear_pointer(&g->reset_bh, qemu_bh_delete);
> @@ -1452,6 +1508,8 @@ void virtio_gpu_reset(VirtIODevice *vdev)
> g_free(cmd);
> }
>
> + g_hash_table_remove_all(g->resource_uuids);
> +
> virtio_gpu_base_reset(VIRTIO_GPU_BASE(vdev));
> }
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index b9adc28071..67b39fccec 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -63,6 +63,8 @@ struct virtio_gpu_simple_resource {
> MemoryRegion *region;
> #endif
>
> + bool has_uuid;
> +
> QTAILQ_ENTRY(virtio_gpu_simple_resource) next;
> };
>
> @@ -208,6 +210,8 @@ struct VirtIOGPU {
> QTAILQ_HEAD(, VGPUDMABuf) bufs;
> VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
> } dmabuf;
> +
> + GHashTable *resource_uuids;
> };
>
> struct VirtIOGPUClass {
> @@ -285,6 +289,8 @@ void virtio_gpu_cleanup_mapping_iov(VirtIOGPU *g,
> struct iovec *iov, uint32_t count);
> void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res);
> +void virtio_gpu_resource_assign_uuid(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd);
> void virtio_gpu_process_cmdq(VirtIOGPU *g);
> void virtio_gpu_device_realize(DeviceState *qdev, Error **errp);
> void virtio_gpu_reset(VirtIODevice *vdev);
> --
> 2.34.1
>
>
--
Marc-André Lureau
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |