From 523423436e83b01a83c60bc44bd08471992aaff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Mon, 15 Jan 2024 16:34:33 +0400 Subject: [PATCH 1/4] virtio-gpu: fix scanout migration post-load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RH-Author: Marc-André Lureau RH-MergeRequest: 377: virtio-gpu: fix scanout migration post-load RH-Jira: RHEL-36181 RH-Acked-by: Peter Xu RH-Acked-by: Miroslav Rezanina RH-Commit: [1/3] 05e9dbf7f602d62ec2f336a855daf57e6ee8f3f3 The current post-loading code for scanout has a FIXME: it doesn't take the resource region/rect into account. But there is more, when adding blob migration support in commit f66767f75c9, I didn't realize that blob resources could be used for scanouts. This situationn leads to a crash during post-load, as they don't have an associated res->image. virtio_gpu_do_set_scanout() handle all cases, but requires the associated virtio_gpu_framebuffer, which is currently not saved during migration. Add a v2 of "virtio-gpu-one-scanout" with the framebuffer fields, so we can restore blob scanouts, as well as fixing the existing FIXME. Signed-off-by: Marc-André Lureau Reviewed-by: Sebastian Ott (cherry picked from commit dfcf74fa68c88233209aafc5f82728d0b9a1af5c) --- hw/display/virtio-gpu.c | 55 +++++++++++++++++++++++++++------- include/hw/virtio/virtio-gpu.h | 1 + 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 1702190ead..bc485d60ea 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -598,6 +598,7 @@ static void virtio_unref_resource(pixman_image_t *image, void *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) { struct virtio_gpu_simple_resource *ores; @@ -615,9 +616,10 @@ static void virtio_gpu_update_scanout(VirtIOGPU *g, scanout->y = r->y; scanout->width = r->width; scanout->height = r->height; + scanout->fb = *fb; } -static void virtio_gpu_do_set_scanout(VirtIOGPU *g, +static bool virtio_gpu_do_set_scanout(VirtIOGPU *g, uint32_t scanout_id, struct virtio_gpu_framebuffer *fb, struct virtio_gpu_simple_resource *res, @@ -643,7 +645,7 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, r->x, r->y, r->width, r->height, fb->width, fb->height); *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; - return; + return false; } g->parent_obj.enable = 1; @@ -651,11 +653,12 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, if (res->blob) { if (console_has_gl(scanout->con)) { if (!virtio_gpu_update_dmabuf(g, scanout_id, res, fb, r)) { - virtio_gpu_update_scanout(g, scanout_id, res, r); + virtio_gpu_update_scanout(g, scanout_id, res, fb, r); } else { *error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY; + return false; } - return; + return true; } data = res->blob; @@ -684,7 +687,7 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, scanout->ds = qemu_create_displaysurface_pixman(rect); if (!scanout->ds) { *error = VIRTIO_GPU_RESP_ERR_UNSPEC; - return; + return false; } #ifdef WIN32 qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, fb->offset); @@ -695,7 +698,8 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, scanout->ds); } - virtio_gpu_update_scanout(g, scanout_id, res, r); + virtio_gpu_update_scanout(g, scanout_id, res, fb, r); + return true; } static void virtio_gpu_set_scanout(VirtIOGPU *g, @@ -1166,8 +1170,9 @@ static void virtio_gpu_cursor_bh(void *opaque) static const VMStateDescription vmstate_virtio_gpu_scanout = { .name = "virtio-gpu-one-scanout", - .version_id = 1, - .fields = (VMStateField[]) { + .version_id = 2, + .minimum_version_id = 1, + .fields = (const VMStateField[]) { VMSTATE_UINT32(resource_id, struct virtio_gpu_scanout), VMSTATE_UINT32(width, struct virtio_gpu_scanout), VMSTATE_UINT32(height, struct virtio_gpu_scanout), @@ -1178,6 +1183,12 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = { VMSTATE_UINT32(cursor.hot_y, struct virtio_gpu_scanout), VMSTATE_UINT32(cursor.pos.x, struct virtio_gpu_scanout), VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout), + VMSTATE_UINT32_V(fb.format, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_V(fb.bytes_pp, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_V(fb.width, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_V(fb.height, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_V(fb.stride, struct virtio_gpu_scanout, 2), + VMSTATE_UINT32_V(fb.offset, struct virtio_gpu_scanout, 2), VMSTATE_END_OF_LIST() }, }; @@ -1349,6 +1360,7 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size, if (!res->blob_size) { continue; } + assert(!res->image); qemu_put_be32(f, res->resource_id); qemu_put_be32(f, res->blob_size); qemu_put_be32(f, res->iov_cnt); @@ -1411,11 +1423,11 @@ static int virtio_gpu_post_load(void *opaque, int version_id) int i; for (i = 0; i < g->parent_obj.conf.max_outputs; i++) { - /* FIXME: should take scanout.r.{x,y} into account */ scanout = &g->parent_obj.scanout[i]; if (!scanout->resource_id) { continue; } + res = virtio_gpu_find_resource(g, scanout->resource_id); if (!res) { return -EINVAL; @@ -1428,7 +1440,30 @@ static int virtio_gpu_post_load(void *opaque, int version_id) qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, 0); #endif - dpy_gfx_replace_surface(scanout->con, scanout->ds); + if (scanout->fb.format != 0) { + uint32_t error = 0; + struct virtio_gpu_rect r = { + .x = scanout->x, + .y = scanout->y, + .width = scanout->width, + .height = scanout->height + }; + + if (!virtio_gpu_do_set_scanout(g, i, &scanout->fb, res, &r, &error)) { + return -EINVAL; + } + } else { + /* legacy v1 migration support */ + if (!res->image) { + return -EINVAL; + } + scanout->ds = qemu_create_displaysurface_pixman(res->image); +#ifdef WIN32 + qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, 0); +#endif + dpy_gfx_replace_surface(scanout->con, scanout->ds); + } + dpy_gfx_update_full(scanout->con); if (scanout->cursor.resource_id) { update_cursor(g, &scanout->cursor); diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 584ba2ed73..9a13afb34e 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -81,6 +81,7 @@ struct virtio_gpu_scanout { uint32_t resource_id; struct virtio_gpu_update_cursor cursor; QEMUCursor *current_cursor; + struct virtio_gpu_framebuffer fb; }; struct virtio_gpu_requested_state { -- 2.39.3