From: Antonio Caggiano <antonio.caggiano@collabora.com>
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@collabora.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
---
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);
+
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) {
+ 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) {
+ 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));
+ }
}
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);
+
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
Hi On Fri, Sep 15, 2023 at 3:14 PM Huang Rui <ray.huang@amd.com> wrote: > > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > 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@collabora.com> > Signed-off-by: Huang Rui <ray.huang@amd.com> > --- > > 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
On Tue, Sep 19, 2023 at 05:00:05PM +0800, Marc-André Lureau wrote: > Hi > > On Fri, Sep 15, 2023 at 3:14 PM Huang Rui <ray.huang@amd.com> wrote: > > > > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > > > 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@collabora.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > > > 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). > Did you mean we can add "resource_uuid" in virtio_gpu_properties[] like below, user can set it to false on older types with the param? DEFINE_PROP_BIT("resource_uuid", VirtIOGPU, parent_obj.conf.flags, VIRTIO_GPU_FLAG_RESOURCE_UUID, true); > > + > > 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. I am using one byte to store res->has_uuid after pixman data, if check this field is true, then have UUID data. Would you mind to elaborate the optional subsection? > > > } > > 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()). > Thanks to reminder, will update it in V6. > > + > > 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
On 2023/09/15 20:11, Huang Rui wrote: > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > 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@collabora.com> > Signed-off-by: Huang Rui <ray.huang@amd.com> > --- > > 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); > + > 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) { > + cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY; > + return; > + } > + res->resource_id = c2d.resource_id; > + QTAILQ_INSERT_HEAD(&g->reslist, res, next); > + The struct virtio_gpu_simple_resource for a resource created with virgl_cmd_create_resource_2d() and virgl_resource_attach_backing() will not have iov and iov_cnt set, which is inconsistent with virgl_cmd_resource_create_blob(). > 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) { > + 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; > + } What about embedding uuid into struct virtio_gpu_simple_resource? You will not need to maintain another table in this way.
On Sat, Sep 16, 2023 at 12:48:14AM +0800, Akihiko Odaki wrote: > On 2023/09/15 20:11, Huang Rui wrote: > > From: Antonio Caggiano <antonio.caggiano@collabora.com> > > > > 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@collabora.com> > > Signed-off-by: Huang Rui <ray.huang@amd.com> > > --- > > > > 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); > > + > > 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) { > > + cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY; > > + return; > > + } > > + res->resource_id = c2d.resource_id; > > + QTAILQ_INSERT_HEAD(&g->reslist, res, next); > > + > > The struct virtio_gpu_simple_resource for a resource created with > virgl_cmd_create_resource_2d() and virgl_resource_attach_backing() will > not have iov and iov_cnt set, which is inconsistent with > virgl_cmd_resource_create_blob(). > OK, so we should find out the resource in virgl_resource_attach_backing(), and set the related iov and iov_cnt for this resource to align with blob memory. > > 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) { > > + 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; > > + } > > What about embedding uuid into struct virtio_gpu_simple_resource? You > will not need to maintain another table in this way. Yes, agree. Will update it in V6. Thanks, Ray
© 2016 - 2024 Red Hat, Inc.