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, dongwon....@intel.com 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.
--
Best regards,
Dmitry
