|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [QEMU PATCH v2 1/1] virtgpu: do not destroy resources when guest suspend
On Fri, Jun 30, 2023 at 03:00:16PM +0800, Jiqian Chen wrote:
> After suspending and resuming guest VM, you will get
> a black screen, and the display can't come back.
>
> This is because when guest did suspending, it called
> into qemu to call virtio_gpu_gl_reset. In function
> virtio_gpu_gl_reset, it destroyed resources and reset
> renderer, which were used for display. As a result,
> guest's screen can't come back to the time when it was
> suspended and only showed black.
>
> So, this patch adds a new ctrl message
> VIRTIO_GPU_CMD_STATUS_FREEZING to get notification from
> guest. If guest is during suspending, it sets freezing
> status of virtgpu to true, this will prevent destroying
> resources and resetting renderer when guest calls into
> virtio_gpu_gl_reset. If guest is during resuming, it sets
> freezing to false, and then virtio_gpu_gl_reset will keep
> its origin actions and has no other impaction.
>
> Due to this implemention needs cooperation with guest, so
> it added a new feature flag VIRTIO_GPU_F_FREEZING, so
> that guest and host can negotiate whenever freezing is
> supported or not.
>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@xxxxxxx>
> ---
> hw/display/virtio-gpu-base.c | 3 ++
> hw/display/virtio-gpu-gl.c | 9 +++-
> hw/display/virtio-gpu-virgl.c | 7 +++
> hw/display/virtio-gpu.c | 52 ++++++++++++++++++++-
> hw/virtio/virtio.c | 3 ++
> include/hw/virtio/virtio-gpu.h | 6 +++
> include/standard-headers/linux/virtio_gpu.h | 15 ++++++
> 7 files changed, 92 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
> index a29f191aa8..d55dc8fdfc 100644
> --- a/hw/display/virtio-gpu-base.c
> +++ b/hw/display/virtio-gpu-base.c
> @@ -215,6 +215,9 @@ virtio_gpu_base_get_features(VirtIODevice *vdev, uint64_t
> features,
> if (virtio_gpu_blob_enabled(g->conf)) {
> features |= (1 << VIRTIO_GPU_F_RESOURCE_BLOB);
> }
> + if (virtio_gpu_freezing_enabled(g->conf)) {
> + features |= (1 << VIRTIO_GPU_F_FREEZING);
> + }
>
> return features;
> }
> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
> index e06be60dfb..de108f1502 100644
> --- a/hw/display/virtio-gpu-gl.c
> +++ b/hw/display/virtio-gpu-gl.c
> @@ -100,7 +100,14 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
> */
> if (gl->renderer_inited && !gl->renderer_reset) {
> virtio_gpu_virgl_reset_scanout(g);
> - gl->renderer_reset = true;
> + /*
> + * If guest is suspending, we shouldn't reset renderer,
> + * otherwise, the display can't come back to the time when
> + * it was suspended after guest resumed.
> + */
> + if (!virtio_gpu_freezing_enabled(g->parent_obj.conf) ||
> !g->freezing) {
> + gl->renderer_reset = true;
> + }
> }
> }
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 73cb92c8d5..547c4d98ad 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -464,6 +464,13 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
> case VIRTIO_GPU_CMD_GET_EDID:
> virtio_gpu_get_edid(g, cmd);
> break;
> + case VIRTIO_GPU_CMD_STATUS_FREEZING:
> + if (virtio_gpu_freezing_enabled(g->parent_obj.conf)) {
> + virtio_gpu_cmd_status_freezing(g, cmd);
> + } else {
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> + }
> + break;
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5e15c79b94..54a5e2e57c 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -373,6 +373,16 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
> QTAILQ_INSERT_HEAD(&g->reslist, res, next);
> }
>
> +void virtio_gpu_cmd_status_freezing(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd)
> +{
> + struct virtio_gpu_status_freezing sf;
> +
> + VIRTIO_GPU_FILL_CMD(sf);
> + virtio_gpu_bswap_32(&sf, sizeof(sf));
> + g->freezing = sf.freezing;
> +}
> +
> static void virtio_gpu_disable_scanout(VirtIOGPU *g, int scanout_id)
> {
> struct virtio_gpu_scanout *scanout = &g->parent_obj.scanout[scanout_id];
> @@ -986,6 +996,13 @@ 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_STATUS_FREEZING:
> + if (virtio_gpu_freezing_enabled(g->parent_obj.conf)) {
> + virtio_gpu_cmd_status_freezing(g, cmd);
> + } else {
> + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> + }
> + break;
> default:
> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> break;
> @@ -1344,6 +1361,27 @@ void virtio_gpu_device_realize(DeviceState *qdev,
> Error **errp)
> QTAILQ_INIT(&g->reslist);
> QTAILQ_INIT(&g->cmdq);
> QTAILQ_INIT(&g->fenceq);
> +
> + g->freezing = false;
> +}
> +
> +static void virtio_gpu_device_unrealize(DeviceState *qdev)
> +{
> + VirtIOGPU *g = VIRTIO_GPU(qdev);
> + struct virtio_gpu_simple_resource *res, *tmp;
> +
> + /*
> + * This is to prevent memory leak in the situation that qemu is
> + * destroyed when guest is suspended. This also need hot-plug
> + * support.
> + */
> + if (virtio_gpu_freezing_enabled(g->parent_obj.conf) && g->freezing) {
> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
> + virtio_gpu_resource_destroy(g, res);
> + }
> + virtio_gpu_virgl_reset(g);
> + }
> +
> }
>
> void virtio_gpu_reset(VirtIODevice *vdev)
> @@ -1352,8 +1390,15 @@ void virtio_gpu_reset(VirtIODevice *vdev)
> struct virtio_gpu_simple_resource *res, *tmp;
> struct virtio_gpu_ctrl_command *cmd;
>
> - QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
> - virtio_gpu_resource_destroy(g, res);
> + /*
> + * If guest is suspending, we shouldn't destroy resources,
> + * otherwise, the display can't come back to the time when
> + * it was suspended after guest resumed.
> + */
> + if (!virtio_gpu_freezing_enabled(g->parent_obj.conf) || !g->freezing) {
> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
> + virtio_gpu_resource_destroy(g, res);
> + }
> }
>
> while (!QTAILQ_EMPTY(&g->cmdq)) {
> @@ -1425,6 +1470,8 @@ static Property virtio_gpu_properties[] = {
> 256 * MiB),
> DEFINE_PROP_BIT("blob", VirtIOGPU, parent_obj.conf.flags,
> VIRTIO_GPU_FLAG_BLOB_ENABLED, false),
> + DEFINE_PROP_BIT("freezing", VirtIOGPU, parent_obj.conf.flags,
> + VIRTIO_GPU_FLAG_FREEZING_ENABLED, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -1441,6 +1488,7 @@ static void virtio_gpu_class_init(ObjectClass *klass,
> void *data)
> vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
>
> vdc->realize = virtio_gpu_device_realize;
> + vdc->unrealize = virtio_gpu_device_unrealize;
> vdc->reset = virtio_gpu_reset;
> vdc->get_config = virtio_gpu_get_config;
> vdc->set_config = virtio_gpu_set_config;
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index eb6347ab5d..e2ccf50a9e 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -240,6 +240,9 @@ qmp_virtio_feature_map_t virtio_gpu_feature_map[] = {
> FEATURE_ENTRY(VIRTIO_GPU_F_CONTEXT_INIT, \
> "VIRTIO_GPU_F_CONTEXT_INIT: Context types and synchronization "
> "timelines supported"),
> + FEATURE_ENTRY(VIRTIO_GPU_F_FREEZING, \
> + "VIRTIO_GPU_F_FREEZING: Freezing virtio-gpu and keeping
> resources"
> + "alive is supported."),
> FEATURE_ENTRY(VHOST_F_LOG_ALL, \
> "VHOST_F_LOG_ALL: Logging write descriptors supported"),
> FEATURE_ENTRY(VHOST_USER_F_PROTOCOL_FEATURES, \
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 2e28507efe..53e06e47cb 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -90,6 +90,7 @@ enum virtio_gpu_base_conf_flags {
> VIRTIO_GPU_FLAG_EDID_ENABLED,
> VIRTIO_GPU_FLAG_DMABUF_ENABLED,
> VIRTIO_GPU_FLAG_BLOB_ENABLED,
> + VIRTIO_GPU_FLAG_FREEZING_ENABLED,
> };
>
> #define virtio_gpu_virgl_enabled(_cfg) \
> @@ -102,6 +103,8 @@ enum virtio_gpu_base_conf_flags {
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_DMABUF_ENABLED))
> #define virtio_gpu_blob_enabled(_cfg) \
> (_cfg.flags & (1 << VIRTIO_GPU_FLAG_BLOB_ENABLED))
> +#define virtio_gpu_freezing_enabled(_cfg) \
> + (_cfg.flags & (1 << VIRTIO_GPU_FLAG_FREEZING_ENABLED))
>
> struct virtio_gpu_base_conf {
> uint32_t max_outputs;
> @@ -173,6 +176,7 @@ struct VirtIOGPU {
>
> uint64_t hostmem;
>
> + bool freezing;
> bool processing_cmdq;
> QEMUTimer *fence_poll;
> QEMUTimer *print_stats;
> @@ -284,5 +288,7 @@ 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);
> +void virtio_gpu_cmd_status_freezing(VirtIOGPU *g,
> + struct virtio_gpu_ctrl_command *cmd);
>
> #endif
> diff --git a/include/standard-headers/linux/virtio_gpu.h
> b/include/standard-headers/linux/virtio_gpu.h
> index 2da48d3d4c..cc9286b30e 100644
> --- a/include/standard-headers/linux/virtio_gpu.h
> +++ b/include/standard-headers/linux/virtio_gpu.h
> @@ -65,6 +65,11 @@
> */
> #define VIRTIO_GPU_F_CONTEXT_INIT 4
>
> +/*
> + * VIRTIO_GPU_CMD_STATUS_FREEZING
> + */
> +#define VIRTIO_GPU_F_FREEZING 5
> +
> enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_UNDEFINED = 0,
>
> @@ -100,6 +105,9 @@ enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_CMD_UPDATE_CURSOR = 0x0300,
> VIRTIO_GPU_CMD_MOVE_CURSOR,
>
> + /* status */
> + VIRTIO_GPU_CMD_STATUS_FREEZING = 0x0400,
> +
> /* success responses */
> VIRTIO_GPU_RESP_OK_NODATA = 0x1100,
> VIRTIO_GPU_RESP_OK_DISPLAY_INFO,
> @@ -116,6 +124,7 @@ enum virtio_gpu_ctrl_type {
> VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID,
> VIRTIO_GPU_RESP_ERR_INVALID_CONTEXT_ID,
> VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER,
> +
> };
>
???
> enum virtio_gpu_shm_id {
> @@ -453,4 +462,10 @@ struct virtio_gpu_resource_unmap_blob {
> uint32_t padding;
> };
>
> +/* VIRTIO_GPU_CMD_STATUS_FREEZING */
> +struct virtio_gpu_status_freezing {
> + struct virtio_gpu_ctrl_hdr hdr;
> + __u32 freezing;
> +};
> +
Fails build on mingw:
/scm/qemu/include/standard-headers/linux/virtio_gpu.h:468:9: error: unknown
type name '__u32'
468 | __u32 freezing;
| ^~~~~
Reason is you are not supposed to edit this directly: it has to be edited
in linux then we import it.
> #endif
> --
> 2.34.1
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |