On 10/6/25 21:35, Kim, Dongwon wrote: > Hi Dmitry, > >> Subject: Re: [PATCH v5 2/3] drm/virtio: Add support for saving and restoring >> virtio_gpu_objects >> >> On 10/3/25 21:59, Kim, Dongwon wrote: >>> Hi Dmitry, >>> >>>> Subject: Re: [PATCH v5 2/3] drm/virtio: Add support for saving and >>>> restoring virtio_gpu_objects >>>> >>>> On 10/3/25 20:01, Kim, Dongwon wrote: >>>>> Hi Dmitry, >>>>> >>>>>> Subject: Re: [PATCH v5 2/3] drm/virtio: Add support for saving and >>>>>> restoring virtio_gpu_objects >>>>>> >>>>>> On 10/3/25 08:34, [email protected] wrote: >>>>>>> +int virtio_gpu_object_restore_all(struct virtio_gpu_device *vgdev) { >>>>>>> + struct virtio_gpu_object *bo, *tmp; >>>>>>> + struct virtio_gpu_mem_entry *ents; >>>>>>> + unsigned int nents; >>>>>>> + int ret = 0; >>>>>>> + >>>>>>> + spin_lock(&vgdev->obj_restore_lock); >>>>>>> + list_for_each_entry_safe(bo, tmp, &vgdev->obj_restore_list, >>>>>>> list) { >>>>>>> + ret = virtio_gpu_object_shmem_init(vgdev, bo, &ents, >>>>>>> &nents); >>>>>>> + if (ret) >>>>>>> + break; >>>>>>> + >>>>>>> + if (bo->params.blob) { >>>>>>> + virtio_gpu_cmd_resource_create_blob(vgdev, bo, >>>>>>> &bo- >>>>>>> params, >>>>>>> + ents, >>>>>>> nents); >>>>>>> + } else if (bo->params.virgl) { >>>>>>> + virtio_gpu_cmd_resource_create_3d(vgdev, bo, >>>>>>> &bo- >>>>>>> params, >>>>>>> + NULL, NULL); >>>>>>> + >>>>>>> + if (bo->attached) { >>>>>>> + bo->attached = false; >>>>>>> + virtio_gpu_object_attach(vgdev, bo, >>>>>>> ents, >>>>>> nents); >>>>>>> + } >>>>>>> + } else { >>>>>> >>>>>> No need to restore blob and 3d resources that we don't support >>>>>> restore of and that won't be in the obj_restore_list since only >>>>>> shmem objs are added to the list. >>>>>> >>>>> >>>>> We are very interested in restoration of blob as well. It is >>>>> actually the primary type of resource we want to recover because >>>>> those are used as guest frame buffer we render in QEMU. Can you >>>>> tell me why it should be excluded? Would it be because of ambiguity >>>>> of the location of >>>> backing pages for it (e.g. VRAM)? >>>>> >>>>> And 3D is not our primary interest so I don't have any issue >>>>> excluding it but it would be great if you can explain the reason for it >>>>> as well. >>>>> >>>>> Thanks! >>>> >>>> Good point, only host blobs should be excluded. We can't restore VRAM >>>> on resume as hostmem is lost on host side after entering S4. >>>> >>>> Actually, now I don't see where imported dmabuf guest blob added to >>>> the restore_list in virtgpu_dma_buf_init_obj(). Please make sure >>>> restoring guest blobs tested properly. >>> >>> Right, you got some critical point there.. I forgot about this object with >> imported dmabuf. >>> We will have to store the case virtio_gpu_objects backed by imported >>> dmabuf as well in case the backing storage is still valid. By the way, >>> so far we are not using imported buffer as a guest framebuffer. All >>> scanouts are originated from virtio_gpu itself, which would be the reason it >> has worked so far. >>> >>> Anyhow, no matter whether it's imported or created by virtio-gpu >>> driver, I guess storing only blobs with bo->attached=true would be >>> enough assuming host blob doesn't have any backing pages. What do you >> think? >>> (I will also modify the code to store objects backed by imported dmabuf.) >> >> The bo->attached only means whether pages are attached to shmem BO on >> host at the moment. >> >> The whole state should be restored on resume - all shmem BOs re-created and >> pages attached to them when bo->attached=true. > > Got it. By the way, regarding exclusion of host blob, I guess you meant only > blobs with > "!guest_blob && host3d_blob". If that is the case, it is automatically > excluded as > virtio_gpu_object_create is not even called. Or did you also mean blobs with > both > flags - host3d_blob and guest_blob set should also be excluded?
There is no need to re-create resources on host that we can't restore, i.e. "!guest_blob && host3d_blob". Restore all BOs that can be fully restored on resume. Also, reject hibernation on PM_HIBERNATION_PREPARE with a error msg if virgl is enabled. -- Best regards, Dmitry
