From cd7ebf6bf2ca367fde49d73f662e700fd355741f Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:02:59 +0300 Subject: [PATCH 01/13] virtio-gpu: Use trace events for tracking number of in-flight fences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace printf's used for tracking of in-flight fence inc/dec events with tracing, for consistency with the rest of virtio-gpu code that uses tracing. Suggested-by: Marc-André Lureau Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-2-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/trace-events | 2 ++ hw/display/virtio-gpu-virgl.c | 2 +- hw/display/virtio-gpu.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/display/trace-events b/hw/display/trace-events index 781f8a3320..e212710284 100644 --- a/hw/display/trace-events +++ b/hw/display/trace-events @@ -53,6 +53,8 @@ virtio_gpu_cmd_ctx_submit(uint32_t ctx, uint32_t size) "ctx 0x%x, size %d" virtio_gpu_update_cursor(uint32_t scanout, uint32_t x, uint32_t y, const char *type, uint32_t res) "scanout %d, x %d, y %d, %s, res 0x%x" virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type 0x%x" virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64 +virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u" +virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u" # qxl.c disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u" diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 9f34d0e661..14091b191e 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -525,7 +525,7 @@ static void virgl_write_fence(void *opaque, uint32_t fence) g_free(cmd); g->inflight--; if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { - fprintf(stderr, "inflight: %3d (-)\r", g->inflight); + trace_virtio_gpu_dec_inflight_fences(g->inflight); } } } diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 49fd803393..3fcc434732 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1046,7 +1046,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g) if (g->stats.max_inflight < g->inflight) { g->stats.max_inflight = g->inflight; } - fprintf(stderr, "inflight: %3d (+)\r", g->inflight); + trace_virtio_gpu_inc_inflight_fences(g->inflight); } } else { g_free(cmd); @@ -1066,7 +1066,7 @@ static void virtio_gpu_process_fenceq(VirtIOGPU *g) g_free(cmd); g->inflight--; if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { - fprintf(stderr, "inflight: %3d (-)\r", g->inflight); + trace_virtio_gpu_dec_inflight_fences(g->inflight); } } } From a723d2eaf0d73749525dc91610d52845e0505b04 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:00 +0300 Subject: [PATCH 02/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh that are used only by GL device. Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-3-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-virgl.c | 8 +++++--- include/hw/virtio/virtio-gpu.h | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 14091b191e..91dce90f91 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -594,11 +594,12 @@ static void virtio_gpu_print_stats(void *opaque) static void virtio_gpu_fence_poll(void *opaque) { VirtIOGPU *g = opaque; + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g); virgl_renderer_poll(); virtio_gpu_process_cmdq(g); if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) { - timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10); + timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10); } } @@ -626,6 +627,7 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) { int ret; uint32_t flags = 0; + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g); #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4 if (qemu_egl_display) { @@ -645,8 +647,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) return ret; } - g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, - virtio_gpu_fence_poll, g); + gl->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL, + virtio_gpu_fence_poll, g); if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL, diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index e343110e23..48a67d662d 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -194,7 +194,6 @@ struct VirtIOGPU { uint64_t hostmem; bool processing_cmdq; - QEMUTimer *fence_poll; QEMUTimer *print_stats; uint32_t inflight; @@ -229,6 +228,8 @@ struct VirtIOGPUGL { bool renderer_inited; bool renderer_reset; + + QEMUTimer *fence_poll; }; struct VhostUserGPU { From a0a8f47fd0d5dc11487bb65e005700d7b984a207 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:01 +0300 Subject: [PATCH 03/13] virtio-gpu: Move print_stats timer to VirtIOGPUGL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move print_stats timer to VirtIOGPUGL for consistency with cmdq_resume_bh and fence_poll that are used only by GL device. Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-4-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-virgl.c | 10 ++++++---- include/hw/virtio/virtio-gpu.h | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 91dce90f91..a63d1f540f 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -574,6 +574,7 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs = { static void virtio_gpu_print_stats(void *opaque) { VirtIOGPU *g = opaque; + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g); if (g->stats.requests) { fprintf(stderr, "stats: vq req %4d, %3d -- 3D %4d (%5d)\n", @@ -588,7 +589,7 @@ static void virtio_gpu_print_stats(void *opaque) } else { fprintf(stderr, "stats: idle\r"); } - timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); + timer_mod(gl->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); } static void virtio_gpu_fence_poll(void *opaque) @@ -651,9 +652,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) virtio_gpu_fence_poll, g); if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { - g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL, - virtio_gpu_print_stats, g); - timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); + gl->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL, + virtio_gpu_print_stats, g); + timer_mod(gl->print_stats, + qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); } return 0; } diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 48a67d662d..18b6c3b3a2 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -194,7 +194,6 @@ struct VirtIOGPU { uint64_t hostmem; bool processing_cmdq; - QEMUTimer *print_stats; uint32_t inflight; struct { @@ -230,6 +229,7 @@ struct VirtIOGPUGL { bool renderer_reset; QEMUTimer *fence_poll; + QEMUTimer *print_stats; }; struct VhostUserGPU { From 7e688d1bf515316e1e334eb5dcb3dd1577810fe3 Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:02 +0300 Subject: [PATCH 04/13] virtio-gpu: Handle virtio_gpu_virgl_init() failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit virtio_gpu_virgl_init() may fail, leading to a further Qemu crash because Qemu assumes it never fails. Check virtio_gpu_virgl_init() return code and don't execute virtio commands on error. Failed virtio_gpu_virgl_init() will result in a timed out virtio commands for a guest OS. Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-5-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-gl.c | 30 ++++++++++++++++++++++-------- include/hw/virtio/virtio-gpu.h | 11 +++++++++-- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index 29d20b0132..ea3413aa56 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -29,9 +29,14 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g, struct virtio_gpu_scanout *s, uint32_t resource_id) { + VirtIOGPUGL *gl = VIRTIO_GPU_GL(g); uint32_t width, height; uint32_t pixels, *data; + if (gl->renderer_state != RS_INITED) { + return; + } + data = virgl_renderer_get_cursor_data(resource_id, &width, &height); if (!data) { return; @@ -65,13 +70,22 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) return; } - if (!gl->renderer_inited) { - virtio_gpu_virgl_init(g); - gl->renderer_inited = true; - } - if (gl->renderer_reset) { - gl->renderer_reset = false; + switch (gl->renderer_state) { + case RS_RESET: virtio_gpu_virgl_reset(g); + /* fallthrough */ + case RS_START: + if (virtio_gpu_virgl_init(g)) { + gl->renderer_state = RS_INIT_FAILED; + return; + } + + gl->renderer_state = RS_INITED; + break; + case RS_INIT_FAILED: + return; + case RS_INITED: + break; } cmd = virtqueue_pop(vq, sizeof(struct virtio_gpu_ctrl_command)); @@ -98,9 +112,9 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev) * GL functions must be called with the associated GL context in main * thread, and when the renderer is unblocked. */ - if (gl->renderer_inited && !gl->renderer_reset) { + if (gl->renderer_state == RS_INITED) { virtio_gpu_virgl_reset_scanout(g); - gl->renderer_reset = true; + gl->renderer_state = RS_RESET; } } diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 18b6c3b3a2..71775243e9 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -222,11 +222,18 @@ struct VirtIOGPUClass { Error **errp); }; +/* VirtIOGPUGL renderer states */ +typedef enum { + RS_START, /* starting state */ + RS_INIT_FAILED, /* failed initialisation */ + RS_INITED, /* initialised and working */ + RS_RESET, /* inited and reset pending, moves to start after reset */ +} RenderState; + struct VirtIOGPUGL { struct VirtIOGPU parent_obj; - bool renderer_inited; - bool renderer_reset; + RenderState renderer_state; QEMUTimer *fence_poll; QEMUTimer *print_stats; From b218c12ae2f7f1e35fd65d745b4b9e15003d82ed Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:03 +0300 Subject: [PATCH 05/13] virtio-gpu: Unrealize GL device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though GL GPU doesn't support hotplugging today, free virgl resources when GL device is unrealized. For consistency. Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-6-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-gl.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index ea3413aa56..753b35ed69 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -156,6 +156,22 @@ static Property virtio_gpu_gl_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) +{ + VirtIOGPU *g = VIRTIO_GPU(qdev); + VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev); + + if (gl->renderer_state >= RS_INITED) { + if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { + timer_free(gl->print_stats); + } + timer_free(gl->fence_poll); + virgl_renderer_cleanup(NULL); + } + + gl->renderer_state = RS_START; +} + static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -169,6 +185,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data; vdc->realize = virtio_gpu_gl_device_realize; + vdc->unrealize = virtio_gpu_gl_device_unrealize; vdc->reset = virtio_gpu_gl_reset; device_class_set_props(dc, virtio_gpu_gl_properties); } From ffac9641baafde399a2ef08b0573b7e15b0a8a3b Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:04 +0300 Subject: [PATCH 06/13] virtio-gpu: Use pkgconfig version to decide which virgl features are available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New virglrerenderer features were stabilized with release of v1.0.0. Presence of symbols in virglrenderer.h doesn't guarantee ABI compatibility with pre-release development versions of libvirglerender. Use virglrenderer version to decide reliably which virgl features are available. Reviewed-by: Alex Bennée Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-7-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-virgl.c | 2 +- meson.build | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index a63d1f540f..ca6f4d6cbb 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -171,7 +171,7 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g, struct virgl_renderer_resource_info info; void *d3d_tex2d = NULL; -#ifdef HAVE_VIRGL_D3D_INFO_EXT +#if VIRGL_VERSION_MAJOR >= 1 struct virgl_renderer_resource_info_ext ext; memset(&ext, 0, sizeof(ext)); ret = virgl_renderer_resource_get_info_ext(ss.resource_id, &ext); diff --git a/meson.build b/meson.build index 85594fd3f1..8dc53c12fb 100644 --- a/meson.build +++ b/meson.build @@ -2437,10 +2437,7 @@ config_host_data.set('CONFIG_VNC', vnc.found()) config_host_data.set('CONFIG_VNC_JPEG', jpeg.found()) config_host_data.set('CONFIG_VNC_SASL', sasl.found()) if virgl.found() - config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', - cc.has_member('struct virgl_renderer_resource_info_ext', 'd3d_tex2d', - prefix: '#include ', - dependencies: virgl)) + config_host_data.set('VIRGL_VERSION_MAJOR', virgl.version().split('.')[0]) endif config_host_data.set('CONFIG_VIRTFS', have_virtfs) config_host_data.set('CONFIG_VTE', vte.found()) From 2c868c7901590ae2cbc726c339325119ef0647cb Mon Sep 17 00:00:00 2001 From: Huang Rui Date: Fri, 25 Oct 2024 00:03:05 +0300 Subject: [PATCH 07/13] virtio-gpu: Support context-init feature with virglrenderer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init feature flags. Expose this feature and support creating virglrenderer context with flags using context_id if libvirglrenderer is new enough. Originally-by: Antonio Caggiano Signed-off-by: Huang Rui Reviewed-by: Antonio Caggiano Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-8-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-gl.c | 4 ++++ hw/display/virtio-gpu-virgl.c | 20 ++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index 753b35ed69..bf87ba4232 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -147,6 +147,10 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = virtio_gpu_virgl_get_num_capsets(g); +#if VIRGL_VERSION_MAJOR >= 1 + g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED; +#endif + virtio_gpu_device_realize(qdev, errp); } diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index ca6f4d6cbb..b3aa444bcf 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -106,8 +106,24 @@ static void virgl_cmd_context_create(VirtIOGPU *g, trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id, cc.debug_name); - virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, - cc.debug_name); + if (cc.context_init) { + if (!virtio_gpu_context_init_enabled(g->parent_obj.conf)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: context_init disabled", + __func__); + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + +#if VIRGL_VERSION_MAJOR >= 1 + virgl_renderer_context_create_with_flags(cc.hdr.ctx_id, + cc.context_init, + cc.nlen, + cc.debug_name); + return; +#endif + } + + virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name); } static void virgl_cmd_context_destroy(VirtIOGPU *g, From 6a3e00d8c9822433a8989bd0529e80580bf3e54f Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:06 +0300 Subject: [PATCH 08/13] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The udmabuf usage is mandatory when virgl is disabled and blobs feature enabled in the Qemu machine configuration. If virgl and blobs are enabled, then udmabuf requirement is optional. Since udmabuf isn't widely supported by a popular Linux distros today, let's relax the udmabuf requirement for blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is available to Qemu users without a need to have udmabuf available in the system. Reviewed-by: Antonio Caggiano Signed-off-by: Huang Rui Reviewed-by: Antonio Caggiano Reviewed-by: Marc-André Lureau Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-9-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 3fcc434732..3d9679c1ef 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1455,6 +1455,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) if (virtio_gpu_blob_enabled(g->parent_obj.conf)) { if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) && + !virtio_gpu_virgl_enabled(g->parent_obj.conf) && !virtio_gpu_have_udmabuf()) { error_setg(errp, "need rutabaga or udmabuf for blob resources"); return; From df4c498e02e48c4dfc13e5d961b58f890a66c832 Mon Sep 17 00:00:00 2001 From: Huang Rui Date: Fri, 25 Oct 2024 00:03:07 +0300 Subject: [PATCH 09/13] virtio-gpu: Add virgl resource management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In a preparation to adding host blobs support to virtio-gpu, add virgl resource management that allows to retrieve resource based on its ID and virgl resource wrapper on top of simple resource that will be contain fields specific to virgl. Signed-off-by: Huang Rui Reviewed-by: Antonio Caggiano Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-10-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-virgl.c | 76 +++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index b3aa444bcf..3ffea478e7 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -22,6 +22,23 @@ #include +struct virtio_gpu_virgl_resource { + struct virtio_gpu_simple_resource base; +}; + +static struct virtio_gpu_virgl_resource * +virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id) +{ + struct virtio_gpu_simple_resource *res; + + res = virtio_gpu_find_resource(g, resource_id); + if (!res) { + return NULL; + } + + return container_of(res, struct virtio_gpu_virgl_resource, base); +} + #if VIRGL_RENDERER_CALLBACKS_VERSION >= 4 static void * virgl_get_egl_display(G_GNUC_UNUSED void *cookie) @@ -35,11 +52,34 @@ 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_virgl_resource *res; VIRTIO_GPU_FILL_CMD(c2d); trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format, c2d.width, c2d.height); + if (c2d.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_virgl_find_resource(g, c2d.resource_id); + if (res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n", + __func__, c2d.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + + res = g_new0(struct virtio_gpu_virgl_resource, 1); + res->base.width = c2d.width; + res->base.height = c2d.height; + res->base.format = c2d.format; + res->base.resource_id = c2d.resource_id; + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next); + args.handle = c2d.resource_id; args.target = 2; args.format = c2d.format; @@ -59,11 +99,34 @@ 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_virgl_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); + if (c3d.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_virgl_find_resource(g, c3d.resource_id); + if (res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n", + __func__, c3d.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + + res = g_new0(struct virtio_gpu_virgl_resource, 1); + res->base.width = c3d.width; + res->base.height = c3d.height; + res->base.format = c3d.format; + res->base.resource_id = c3d.resource_id; + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next); + args.handle = c3d.resource_id; args.target = c3d.target; args.format = c3d.format; @@ -82,12 +145,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { struct virtio_gpu_resource_unref unref; + struct virtio_gpu_virgl_resource *res; struct iovec *res_iovs = NULL; int num_iovs = 0; VIRTIO_GPU_FILL_CMD(unref); trace_virtio_gpu_cmd_res_unref(unref.resource_id); + res = virtio_gpu_virgl_find_resource(g, unref.resource_id); + if (!res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n", + __func__, unref.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + virgl_renderer_resource_detach_iov(unref.resource_id, &res_iovs, &num_iovs); @@ -95,6 +167,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g, virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs); } virgl_renderer_resource_unref(unref.resource_id); + + QTAILQ_REMOVE(&g->reslist, &res->base, next); + + g_free(res); } static void virgl_cmd_context_create(VirtIOGPU *g, From 640f9149c3dcafbfeb495a10f6105c6f71f24d1d Mon Sep 17 00:00:00 2001 From: Dmitry Osipenko Date: Fri, 25 Oct 2024 00:03:08 +0300 Subject: [PATCH 10/13] virtio-gpu: Support suspension of commands processing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check whether command processing has been finished; otherwise, stop processing commands and retry the command again next time. This allows us to support asynchronous execution of non-fenced commands needed for unmapping host blobs safely. Suggested-by: Akihiko Odaki Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-11-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/trace-events | 1 + hw/display/virtio-gpu.c | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/hw/display/trace-events b/hw/display/trace-events index e212710284..d26d663f96 100644 --- a/hw/display/trace-events +++ b/hw/display/trace-events @@ -55,6 +55,7 @@ virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64 virtio_gpu_inc_inflight_fences(uint32_t inflight) "in-flight+ %u" virtio_gpu_dec_inflight_fences(uint32_t inflight) "in-flight- %u" +virtio_gpu_cmd_suspended(uint32_t cmd) "cmd 0x%x" # qxl.c disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u" diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 3d9679c1ef..180d882f0a 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1034,6 +1034,12 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g) /* process command */ vgc->process_cmd(g, cmd); + /* command suspended */ + if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) { + trace_virtio_gpu_cmd_suspended(cmd->cmd_hdr.type); + break; + } + QTAILQ_REMOVE(&g->cmdq, cmd, next); if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { g->stats.requests++; From 7c092f17cceef10258ed23006b40e19b14996471 Mon Sep 17 00:00:00 2001 From: Robert Beckett Date: Fri, 25 Oct 2024 00:03:09 +0300 Subject: [PATCH 11/13] virtio-gpu: Handle resource blob commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Support BLOB resources creation, mapping, unmapping and set-scanout by calling the new stable virglrenderer 0.10 interface. Only enabled when available and via the blob config. E.g. -device virtio-vga-gl,blob=true Signed-off-by: Antonio Caggiano Signed-off-by: Robert Beckett # added set_scanout_blob Signed-off-by: Xenia Ragiadakou Signed-off-by: Huang Rui Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-12-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-gl.c | 3 + hw/display/virtio-gpu-virgl.c | 414 ++++++++++++++++++++++++++++++++- hw/display/virtio-gpu.c | 18 +- include/hw/virtio/virtio-gpu.h | 9 + 4 files changed, 434 insertions(+), 10 deletions(-) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index bf87ba4232..f2555673a1 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -166,6 +166,9 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev); if (gl->renderer_state >= RS_INITED) { +#if VIRGL_VERSION_MAJOR >= 1 + qemu_bh_delete(gl->cmdq_resume_bh); +#endif if (virtio_gpu_stats_enabled(g->parent_obj.conf)) { timer_free(gl->print_stats); } diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 3ffea478e7..b2f4e215a7 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -17,6 +17,8 @@ #include "trace.h" #include "hw/virtio/virtio.h" #include "hw/virtio/virtio-gpu.h" +#include "hw/virtio/virtio-gpu-bswap.h" +#include "hw/virtio/virtio-gpu-pixman.h" #include "ui/egl-helpers.h" @@ -24,6 +26,7 @@ struct virtio_gpu_virgl_resource { struct virtio_gpu_simple_resource base; + MemoryRegion *mr; }; static struct virtio_gpu_virgl_resource * @@ -47,6 +50,145 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie) } #endif +#if VIRGL_VERSION_MAJOR >= 1 +struct virtio_gpu_virgl_hostmem_region { + MemoryRegion mr; + struct VirtIOGPU *g; + bool finish_unmapping; +}; + +static struct virtio_gpu_virgl_hostmem_region * +to_hostmem_region(MemoryRegion *mr) +{ + return container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr); +} + +static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque) +{ + VirtIOGPU *g = opaque; + + virtio_gpu_process_cmdq(g); +} + +static void virtio_gpu_virgl_hostmem_region_free(void *obj) +{ + MemoryRegion *mr = MEMORY_REGION(obj); + struct virtio_gpu_virgl_hostmem_region *vmr; + VirtIOGPUBase *b; + VirtIOGPUGL *gl; + + vmr = to_hostmem_region(mr); + vmr->finish_unmapping = true; + + b = VIRTIO_GPU_BASE(vmr->g); + b->renderer_blocked--; + + /* + * memory_region_unref() is executed from RCU thread context, while + * virglrenderer works only on the main-loop thread that's holding GL + * context. + */ + gl = VIRTIO_GPU_GL(vmr->g); + qemu_bh_schedule(gl->cmdq_resume_bh); +} + +static int +virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g, + struct virtio_gpu_virgl_resource *res, + uint64_t offset) +{ + struct virtio_gpu_virgl_hostmem_region *vmr; + VirtIOGPUBase *b = VIRTIO_GPU_BASE(g); + MemoryRegion *mr; + uint64_t size; + void *data; + int ret; + + if (!virtio_gpu_hostmem_enabled(b->conf)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__); + return -EOPNOTSUPP; + } + + ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size); + if (ret) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n", + __func__, strerror(-ret)); + return ret; + } + + vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1); + vmr->g = g; + + mr = &vmr->mr; + memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data); + memory_region_add_subregion(&b->hostmem, offset, mr); + memory_region_set_enabled(mr, true); + + /* + * MR could outlive the resource if MR's reference is held outside of + * virtio-gpu. In order to prevent unmapping resource while MR is alive, + * and thus, making the data pointer invalid, we will block virtio-gpu + * command processing until MR is fully unreferenced and freed. + */ + OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free; + + res->mr = mr; + + return 0; +} + +static int +virtio_gpu_virgl_unmap_resource_blob(VirtIOGPU *g, + struct virtio_gpu_virgl_resource *res, + bool *cmd_suspended) +{ + struct virtio_gpu_virgl_hostmem_region *vmr; + VirtIOGPUBase *b = VIRTIO_GPU_BASE(g); + MemoryRegion *mr = res->mr; + int ret; + + if (!mr) { + return 0; + } + + vmr = to_hostmem_region(res->mr); + + /* + * Perform async unmapping in 3 steps: + * + * 1. Begin async unmapping with memory_region_del_subregion() + * and suspend/block cmd processing. + * 2. Wait for res->mr to be freed and cmd processing resumed + * asynchronously by virtio_gpu_virgl_hostmem_region_free(). + * 3. Finish the unmapping with final virgl_renderer_resource_unmap(). + */ + if (vmr->finish_unmapping) { + res->mr = NULL; + g_free(vmr); + + ret = virgl_renderer_resource_unmap(res->base.resource_id); + if (ret) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: failed to unmap virgl resource: %s\n", + __func__, strerror(-ret)); + return ret; + } + } else { + *cmd_suspended = true; + + /* render will be unblocked once MR is freed */ + b->renderer_blocked++; + + /* memory region owns self res->mr object and frees it by itself */ + memory_region_set_enabled(mr, false); + memory_region_del_subregion(&b->hostmem, mr); + object_unparent(OBJECT(mr)); + } + + return 0; +} +#endif + static void virgl_cmd_create_resource_2d(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { @@ -78,6 +220,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g, res->base.height = c2d.height; res->base.format = c2d.format; res->base.resource_id = c2d.resource_id; + res->base.dmabuf_fd = -1; QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next); args.handle = c2d.resource_id; @@ -125,6 +268,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g, res->base.height = c3d.height; res->base.format = c3d.format; res->base.resource_id = c3d.resource_id; + res->base.dmabuf_fd = -1; QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next); args.handle = c3d.resource_id; @@ -142,7 +286,8 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g, } static void virgl_cmd_resource_unref(VirtIOGPU *g, - struct virtio_gpu_ctrl_command *cmd) + struct virtio_gpu_ctrl_command *cmd, + bool *cmd_suspended) { struct virtio_gpu_resource_unref unref; struct virtio_gpu_virgl_resource *res; @@ -160,6 +305,16 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g, return; } +#if VIRGL_VERSION_MAJOR >= 1 + if (virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended)) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + if (*cmd_suspended) { + return; + } +#endif + virgl_renderer_resource_detach_iov(unref.resource_id, &res_iovs, &num_iovs); @@ -509,9 +664,241 @@ static void virgl_cmd_get_capset(VirtIOGPU *g, g_free(resp); } +#if VIRGL_VERSION_MAJOR >= 1 +static void virgl_cmd_resource_create_blob(VirtIOGPU *g, + struct virtio_gpu_ctrl_command *cmd) +{ + struct virgl_renderer_resource_create_blob_args virgl_args = { 0 }; + g_autofree struct virtio_gpu_virgl_resource *res = NULL; + struct virtio_gpu_resource_create_blob cblob; + struct virgl_renderer_resource_info info; + int ret; + + if (!virtio_gpu_blob_enabled(g->parent_obj.conf)) { + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; + return; + } + + 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_virgl_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_virgl_resource, 1); + res->base.resource_id = cblob.resource_id; + res->base.blob_size = cblob.size; + res->base.dmabuf_fd = -1; + + if (cblob.blob_mem != VIRTIO_GPU_BLOB_MEM_HOST3D) { + ret = virtio_gpu_create_mapping_iov(g, cblob.nr_entries, sizeof(cblob), + cmd, &res->base.addrs, + &res->base.iov, &res->base.iov_cnt); + if (!ret) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + } + + virgl_args.res_handle = cblob.resource_id; + virgl_args.ctx_id = cblob.hdr.ctx_id; + virgl_args.blob_mem = cblob.blob_mem; + virgl_args.blob_id = cblob.blob_id; + virgl_args.blob_flags = cblob.blob_flags; + virgl_args.size = cblob.size; + virgl_args.iovecs = res->base.iov; + virgl_args.num_iovs = res->base.iov_cnt; + + ret = virgl_renderer_resource_create_blob(&virgl_args); + if (ret) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: virgl blob create error: %s\n", + __func__, strerror(-ret)); + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + virtio_gpu_cleanup_mapping(g, &res->base); + return; + } + + ret = virgl_renderer_resource_get_info(cblob.resource_id, &info); + if (ret) { + qemu_log_mask(LOG_GUEST_ERROR, + "%s: resource does not have info %d: %s\n", + __func__, cblob.resource_id, strerror(-ret)); + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + virtio_gpu_cleanup_mapping(g, &res->base); + virgl_renderer_resource_unref(cblob.resource_id); + return; + } + + res->base.dmabuf_fd = info.fd; + + QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next); + res = NULL; +} + +static void virgl_cmd_resource_map_blob(VirtIOGPU *g, + struct virtio_gpu_ctrl_command *cmd) +{ + struct virtio_gpu_resource_map_blob mblob; + struct virtio_gpu_virgl_resource *res; + struct virtio_gpu_resp_map_info resp; + int ret; + + VIRTIO_GPU_FILL_CMD(mblob); + virtio_gpu_map_blob_bswap(&mblob); + + res = virtio_gpu_virgl_find_resource(g, mblob.resource_id); + if (!res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n", + __func__, mblob.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + + ret = virtio_gpu_virgl_map_resource_blob(g, res, mblob.offset); + if (ret) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + + memset(&resp, 0, sizeof(resp)); + resp.hdr.type = VIRTIO_GPU_RESP_OK_MAP_INFO; + virgl_renderer_resource_get_map_info(mblob.resource_id, &resp.map_info); + virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp)); +} + +static void virgl_cmd_resource_unmap_blob(VirtIOGPU *g, + struct virtio_gpu_ctrl_command *cmd, + bool *cmd_suspended) +{ + struct virtio_gpu_resource_unmap_blob ublob; + struct virtio_gpu_virgl_resource *res; + int ret; + + VIRTIO_GPU_FILL_CMD(ublob); + virtio_gpu_unmap_blob_bswap(&ublob); + + res = virtio_gpu_virgl_find_resource(g, ublob.resource_id); + if (!res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n", + __func__, ublob.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + + ret = virtio_gpu_virgl_unmap_resource_blob(g, res, cmd_suspended); + if (ret) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } +} + +static void virgl_cmd_set_scanout_blob(VirtIOGPU *g, + struct virtio_gpu_ctrl_command *cmd) +{ + struct virtio_gpu_framebuffer fb = { 0 }; + struct virtio_gpu_virgl_resource *res; + 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.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; + } + + if (ss.resource_id == 0) { + virtio_gpu_disable_scanout(g, ss.scanout_id); + return; + } + + if (ss.width < 16 || + ss.height < 16 || + ss.r.x + ss.r.width > ss.width || + ss.r.y + ss.r.height > ss.height) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for" + " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n", + __func__, ss.scanout_id, ss.resource_id, + ss.r.x, ss.r.y, ss.r.width, ss.r.height, + ss.width, ss.height); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; + return; + } + + res = virtio_gpu_virgl_find_resource(g, ss.resource_id); + if (!res) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n", + __func__, ss.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID; + return; + } + if (res->base.dmabuf_fd < 0) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: resource not backed by dmabuf %d\n", + __func__, ss.resource_id); + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + + fb.format = virtio_gpu_get_pixman_format(ss.format); + if (!fb.format) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %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; + + fbend = fb.offset; + fbend += fb.stride * (ss.r.height - 1); + fbend += fb.bytes_pp * ss.r.width; + if (fbend > res->base.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; + } + + g->parent_obj.enable = 1; + if (virtio_gpu_update_dmabuf(g, ss.scanout_id, &res->base, &fb, &ss.r)) { + qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to update dmabuf\n", + __func__); + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; + return; + } + + virtio_gpu_update_scanout(g, ss.scanout_id, &res->base, &fb, &ss.r); +} +#endif + void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { + bool cmd_suspended = false; + VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr); virgl_renderer_force_ctx_0(); @@ -553,7 +940,7 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, virgl_cmd_resource_flush(g, cmd); break; case VIRTIO_GPU_CMD_RESOURCE_UNREF: - virgl_cmd_resource_unref(g, cmd); + virgl_cmd_resource_unref(g, cmd, &cmd_suspended); break; case VIRTIO_GPU_CMD_CTX_ATTACH_RESOURCE: /* TODO add security */ @@ -575,12 +962,26 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, case VIRTIO_GPU_CMD_GET_EDID: virtio_gpu_get_edid(g, cmd); break; +#if VIRGL_VERSION_MAJOR >= 1 + case VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB: + virgl_cmd_resource_create_blob(g, cmd); + break; + case VIRTIO_GPU_CMD_RESOURCE_MAP_BLOB: + virgl_cmd_resource_map_blob(g, cmd); + break; + case VIRTIO_GPU_CMD_RESOURCE_UNMAP_BLOB: + virgl_cmd_resource_unmap_blob(g, cmd, &cmd_suspended); + break; + case VIRTIO_GPU_CMD_SET_SCANOUT_BLOB: + virgl_cmd_set_scanout_blob(g, cmd); + break; +#endif default: cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; break; } - if (cmd->finished) { + if (cmd_suspended || cmd->finished) { return; } if (cmd->error) { @@ -749,6 +1150,13 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) timer_mod(gl->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000); } + +#if VIRGL_VERSION_MAJOR >= 1 + gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(), + virtio_gpu_virgl_resume_cmdq_bh, + g); +#endif + return 0; } diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 180d882f0a..b6cd9fe567 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -362,7 +362,7 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g, QTAILQ_INSERT_HEAD(&g->reslist, res, next); } -static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id) +void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id) { struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id]; struct virtio_gpu_simple_resource *res; @@ -579,11 +579,11 @@ static void virtio_unref_resource(pixman_image_t *image, void *data) pixman_image_unref(data); } -static void virtio_gpu_update_scanout(VirtIOGPU *g, - uint32_t scanout_id, - struct virtio_gpu_simple_resource *res, - struct virtio_gpu_framebuffer *fb, - struct virtio_gpu_rect *r) +void virtio_gpu_update_scanout(VirtIOGPU *g, + uint32_t scanout_id, + struct virtio_gpu_simple_resource *res, + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r) { struct virtio_gpu_simple_resource *ores; struct virtio_gpu_scanout *scanout; @@ -1467,10 +1467,14 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) return; } +#ifdef VIRGL_VERSION_MAJOR + #if VIRGL_VERSION_MAJOR < 1 if (virtio_gpu_virgl_enabled(g->parent_obj.conf)) { - error_setg(errp, "blobs and virgl are not compatible (yet)"); + error_setg(errp, "old virglrenderer, blob resources unsupported"); return; } + #endif +#endif } if (!virtio_gpu_base_device_realize(qdev, diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 71775243e9..f9ed81071f 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -237,6 +237,8 @@ struct VirtIOGPUGL { QEMUTimer *fence_poll; QEMUTimer *print_stats; + + QEMUBH *cmdq_resume_bh; }; struct VhostUserGPU { @@ -336,6 +338,13 @@ int virtio_gpu_update_dmabuf(VirtIOGPU *g, struct virtio_gpu_framebuffer *fb, struct virtio_gpu_rect *r); +void virtio_gpu_update_scanout(VirtIOGPU *g, + uint32_t scanout_id, + struct virtio_gpu_simple_resource *res, + struct virtio_gpu_framebuffer *fb, + struct virtio_gpu_rect *r); +void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id); + /* virtio-gpu-3d.c */ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd); From 1333fd0693a87bd509c025e6212fcfeced948a15 Mon Sep 17 00:00:00 2001 From: Pierre-Eric Pelloux-Prayer Date: Fri, 25 Oct 2024 00:03:10 +0300 Subject: [PATCH 12/13] virtio-gpu: Register capsets dynamically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't assume that capset_index 1 is always VIRGL2 once we'll support more capsets, like Venus and DRM capsets. Register capsets dynamically to avoid that problem. Reviewed-by: Manos Pitsidianakis Signed-off-by: Pierre-Eric Pelloux-Prayer Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-13-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- hw/display/virtio-gpu-gl.c | 6 ++++-- hw/display/virtio-gpu-virgl.c | 33 +++++++++++++++++++++------------ include/hw/virtio/virtio-gpu.h | 4 +++- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index f2555673a1..e859c0dff0 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -144,8 +144,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) } g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED); - VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = - virtio_gpu_virgl_get_num_capsets(g); + g->capset_ids = virtio_gpu_virgl_get_capsets(g); + VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len; #if VIRGL_VERSION_MAJOR >= 1 g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED; @@ -177,6 +177,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState *qdev) } gl->renderer_state = RS_START; + + g_array_unref(g->capset_ids); } static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data) diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index b2f4e215a7..5a881c58a1 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -622,19 +622,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g, VIRTIO_GPU_FILL_CMD(info); memset(&resp, 0, sizeof(resp)); - if (info.capset_index == 0) { - resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL; + + if (info.capset_index < g->capset_ids->len) { + resp.capset_id = g_array_index(g->capset_ids, uint32_t, + info.capset_index); virgl_renderer_get_cap_set(resp.capset_id, &resp.capset_max_version, &resp.capset_max_size); - } else if (info.capset_index == 1) { - resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2; - virgl_renderer_get_cap_set(resp.capset_id, - &resp.capset_max_version, - &resp.capset_max_size); - } else { - resp.capset_max_version = 0; - resp.capset_max_size = 0; } resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO; virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp)); @@ -1160,12 +1154,27 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) return 0; } -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g) +static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id) +{ + g_array_append_val(capset_ids, capset_id); +} + +GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g) { uint32_t capset2_max_ver, capset2_max_size; + GArray *capset_ids; + + capset_ids = g_array_new(false, false, sizeof(uint32_t)); + + /* VIRGL is always supported. */ + virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL); + virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2, &capset2_max_ver, &capset2_max_size); + if (capset2_max_ver) { + virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2); + } - return capset2_max_ver ? 2 : 1; + return capset_ids; } diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index f9ed81071f..07131d8eb9 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -207,6 +207,8 @@ struct VirtIOGPU { QTAILQ_HEAD(, VGPUDMABuf) bufs; VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS]; } dmabuf; + + GArray *capset_ids; }; struct VirtIOGPUClass { @@ -352,6 +354,6 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g); void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g); void virtio_gpu_virgl_reset(VirtIOGPU *g); int virtio_gpu_virgl_init(VirtIOGPU *g); -int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g); +GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g); #endif From 94d0ea1c19289d76ced934711fccd2269e69bb29 Mon Sep 17 00:00:00 2001 From: Antonio Caggiano Date: Fri, 25 Oct 2024 00:03:11 +0300 Subject: [PATCH 13/13] virtio-gpu: Support Venus context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Request Venus when initializing VirGL and if venus=true flag is set for virtio-gpu-gl device. Signed-off-by: Antonio Caggiano Signed-off-by: Huang Rui Signed-off-by: Dmitry Osipenko Message-Id: <20241024210311.118220-14-dmitry.osipenko@collabora.com> Signed-off-by: Alex Bennée --- docs/system/devices/virtio-gpu.rst | 11 +++++++++++ hw/display/virtio-gpu-gl.c | 2 ++ hw/display/virtio-gpu-virgl.c | 22 ++++++++++++++++++---- hw/display/virtio-gpu.c | 15 +++++++++++++++ include/hw/virtio/virtio-gpu.h | 3 +++ 5 files changed, 49 insertions(+), 4 deletions(-) diff --git a/docs/system/devices/virtio-gpu.rst b/docs/system/devices/virtio-gpu.rst index cb73dd7998..b7eb0fc0e7 100644 --- a/docs/system/devices/virtio-gpu.rst +++ b/docs/system/devices/virtio-gpu.rst @@ -71,6 +71,17 @@ representation back to OpenGL API calls. .. _Gallium3D: https://www.freedesktop.org/wiki/Software/gallium/ .. _virglrenderer: https://gitlab.freedesktop.org/virgl/virglrenderer/ +Translation of Vulkan API calls is supported since release of `virglrenderer`_ +v1.0.0 using `venus`_ protocol. ``Venus`` virtio-gpu capability set ("capset") +requires host blob support (``hostmem`` and ``blob`` fields) and should +be enabled using ``venus`` field. The ``hostmem`` field specifies the size +of virtio-gpu host memory window. This is typically between 256M and 8G. + +.. parsed-literal:: + -device virtio-gpu-gl,hostmem=8G,blob=true,venus=true + +.. _venus: https://gitlab.freedesktop.org/virgl/venus-protocol/ + virtio-gpu rutabaga ------------------- diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c index e859c0dff0..7c0e448b46 100644 --- a/hw/display/virtio-gpu-gl.c +++ b/hw/display/virtio-gpu-gl.c @@ -157,6 +157,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev, Error **errp) static Property virtio_gpu_gl_properties[] = { DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags, VIRTIO_GPU_FLAG_STATS_ENABLED, false), + DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags, + VIRTIO_GPU_FLAG_VENUS_ENABLED, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c index 5a881c58a1..eedae7357f 100644 --- a/hw/display/virtio-gpu-virgl.c +++ b/hw/display/virtio-gpu-virgl.c @@ -1128,6 +1128,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g) flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE; } #endif +#if VIRGL_VERSION_MAJOR >= 1 + if (virtio_gpu_venus_enabled(g->parent_obj.conf)) { + flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER; + } +#endif ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs); if (ret != 0) { @@ -1161,7 +1166,7 @@ static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id) GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g) { - uint32_t capset2_max_ver, capset2_max_size; + uint32_t capset_max_ver, capset_max_size; GArray *capset_ids; capset_ids = g_array_new(false, false, sizeof(uint32_t)); @@ -1170,11 +1175,20 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g) virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL); virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2, - &capset2_max_ver, - &capset2_max_size); - if (capset2_max_ver) { + &capset_max_ver, + &capset_max_size); + if (capset_max_ver) { virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2); } + if (virtio_gpu_venus_enabled(g->parent_obj.conf)) { + virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS, + &capset_max_ver, + &capset_max_size); + if (capset_max_size) { + virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VENUS); + } + } + return capset_ids; } diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index b6cd9fe567..c0570ef856 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -1477,6 +1477,21 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) #endif } + if (virtio_gpu_venus_enabled(g->parent_obj.conf)) { +#ifdef VIRGL_VERSION_MAJOR + #if VIRGL_VERSION_MAJOR >= 1 + if (!virtio_gpu_blob_enabled(g->parent_obj.conf) || + !virtio_gpu_hostmem_enabled(g->parent_obj.conf)) { + error_setg(errp, "venus requires enabled blob and hostmem options"); + return; + } + #else + error_setg(errp, "old virglrenderer, venus unsupported"); + return; + #endif +#endif + } + if (!virtio_gpu_base_device_realize(qdev, virtio_gpu_handle_ctrl_cb, virtio_gpu_handle_cursor_cb, diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 07131d8eb9..553799b8cc 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -97,6 +97,7 @@ enum virtio_gpu_base_conf_flags { VIRTIO_GPU_FLAG_BLOB_ENABLED, VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED, VIRTIO_GPU_FLAG_RUTABAGA_ENABLED, + VIRTIO_GPU_FLAG_VENUS_ENABLED, }; #define virtio_gpu_virgl_enabled(_cfg) \ @@ -115,6 +116,8 @@ enum virtio_gpu_base_conf_flags { (_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED)) #define virtio_gpu_hostmem_enabled(_cfg) \ (_cfg.hostmem > 0) +#define virtio_gpu_venus_enabled(_cfg) \ + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED)) struct virtio_gpu_base_conf { uint32_t max_outputs;