[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspen
From: |
Chen, Jiqian |
Subject: |
Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend |
Date: |
Fri, 30 Jun 2023 07:14:31 +0000 |
Hi Dongwon,
On 2023/6/30 00:53, Kim, Dongwon wrote:
> This method - letting QEMU not remove resources would work on S3 case but
> with S4, the QEMU would lose all the resources anyway as the process will be
> terminated. So objects restoring was only option for us as
My patch is for S3 function on Xen. I haven't tried S4 before, I will try S4
later.
>
> in [RFC PATCH 2/2] drm/virtio: restore virtio_gpu_objects upon suspend and
> resume (lists.freedesktop.org)
> <https://lists.freedesktop.org/archives/dri-devel/2022-September/373894.html>
>
> But I only considered and tested cases with scanout blob resources, so this
> may not cover other resource types...
I tried your patch, but I can't success to resume guest and guest crashed.
>
> On 6/7/2023 7:56 PM, 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.
>>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>> ---
>> hw/display/virtio-gpu-gl.c | 9 ++++++-
>> hw/display/virtio-gpu-virgl.c | 3 +++
>> hw/display/virtio-gpu.c | 26 +++++++++++++++++++--
>> include/hw/virtio/virtio-gpu.h | 3 +++
>> include/standard-headers/linux/virtio_gpu.h | 9 +++++++
>> 5 files changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
>> index e06be60dfb..e11ad233eb 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 (!g->freezing) {
>> + gl->renderer_reset = true;
>> + }
>> }
>> }
>> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
>> index 73cb92c8d5..183ec92d53 100644
>> --- a/hw/display/virtio-gpu-virgl.c
>> +++ b/hw/display/virtio-gpu-virgl.c
>> @@ -464,6 +464,9 @@ 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:
>> + virtio_gpu_cmd_status_freezing(g, cmd);
>> + 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..8f235d7848 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,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_STATUS_FREEZING:
>> + virtio_gpu_cmd_status_freezing(g, cmd);
>> + break;
>> default:
>> cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
>> break;
>> @@ -1344,6 +1357,8 @@ void virtio_gpu_device_realize(DeviceState *qdev,
>> Error **errp)
>> QTAILQ_INIT(&g->reslist);
>> QTAILQ_INIT(&g->cmdq);
>> QTAILQ_INIT(&g->fenceq);
>> +
>> + g->freezing = false;
>> }
>> void virtio_gpu_reset(VirtIODevice *vdev)
>> @@ -1352,8 +1367,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 (!g->freezing) {
>> + QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
>> + virtio_gpu_resource_destroy(g, res);
>> + }
>> }
>> while (!QTAILQ_EMPTY(&g->cmdq)) {
>> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
>> index 2e28507efe..c21c2990fb 100644
>> --- a/include/hw/virtio/virtio-gpu.h
>> +++ b/include/hw/virtio/virtio-gpu.h
>> @@ -173,6 +173,7 @@ struct VirtIOGPU {
>> uint64_t hostmem;
>> + bool freezing;
>> bool processing_cmdq;
>> QEMUTimer *fence_poll;
>> QEMUTimer *print_stats;
>> @@ -284,5 +285,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..aefffbd751 100644
>> --- a/include/standard-headers/linux/virtio_gpu.h
>> +++ b/include/standard-headers/linux/virtio_gpu.h
>> @@ -116,6 +116,9 @@ 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,
>> +
>> + /* status */
>> + VIRTIO_GPU_CMD_STATUS_FREEZING = 0x1300,
>> };
>> enum virtio_gpu_shm_id {
>> @@ -453,4 +456,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;
>> +};
>> +
>> #endif
--
Best regards,
Jiqian Chen.
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, (continued)
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Gerd Hoffmann, 2023/06/19
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Chen, Jiqian, 2023/06/20
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Gerd Hoffmann, 2023/06/20
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Robert Beckett, 2023/06/20
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Kim, Dongwon, 2023/06/20
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Gerd Hoffmann, 2023/06/21
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Robert Beckett, 2023/06/21
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Gerd Hoffmann, 2023/06/21
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Kim, Dongwon, 2023/06/29
Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend, Kim, Dongwon, 2023/06/29
- Re: [QEMU PATCH 1/1] virtgpu: do not destroy resources when guest suspend,
Chen, Jiqian <=
Re: [QEMU PATCH 0/1], Chen, Jiqian, 2023/06/08