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? > > -- > Best regards, > Dmitry
