similar to: [PATCH] drm/virtio: fix memory leak in virtio_gpu_cleanup_object()

From: Xin He <hexin.op at bytedance.com> [ Upstream commit 836b194d65782aaec4485a07d2aab52d3f698505 ] Before setting shmem->pages to NULL, kfree() should be called. Signed-off-by: Xin He <hexin.op at bytedance.com> Reviewed-by: Qi Liu <liuqi.16 at bytedance.com> Link: http://patchwork.freedesktop.org/patch/msgid/20200722051851.72662-1-hexin.op at bytedance.com

From: Xin He <hexin.op at bytedance.com> [ Upstream commit 836b194d65782aaec4485a07d2aab52d3f698505 ] Before setting shmem->pages to NULL, kfree() should be called. Signed-off-by: Xin He <hexin.op at bytedance.com> Reviewed-by: Qi Liu <liuqi.16 at bytedance.com> Link: http://patchwork.freedesktop.org/patch/msgid/20200722051851.72662-1-hexin.op at bytedance.com

Stop sending DETACH_BACKING commands, that will happening anyway when releasing resources via UNREF. Handle guest-side cleanup in virtio_gpu_cleanup_object(), called when the host finished processing the UNREF command. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 -- drivers/gpu/drm/virtio/virtgpu_object.c | 14 ++++++--

Stop sending DETACH_BACKING commands, that will happening anyway when releasing resources via UNREF. Handle guest-side cleanup in virtio_gpu_cleanup_object(), called when the host finished processing the UNREF command. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 -- drivers/gpu/drm/virtio/virtgpu_object.c | 14 ++++++--

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c |

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c |

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> Acked-by: Rob Herring <robh at kernel.org> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c |

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> Acked-by: Rob Herring <robh at kernel.org> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c |

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> Acked-by: Rob Herring <robh at kernel.org> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/tiny/gm12u320.c |

DEFINE_DRM_GEM_SHMEM_FOPS is identical to DEFINE_DRM_GEM_FOPS now, drop it. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- include/drm/drm_gem_shmem_helper.h | 26 ------------------------- drivers/gpu/drm/cirrus/cirrus.c | 2 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 2 +- drivers/gpu/drm/v3d/v3d_drv.c | 2 +- 4 files changed, 3 insertions(+), 29

drm_gem_shmem_get_sg_table is meant to implement obj->funcs->get_sg_table, for prime exporting. The one we want is drm_gem_shmem_get_pages_sgt, which also handles imported dma-buf, not just native objects. v2: Rebase, this stuff moved around in commit 2f2aa13724d56829d910b2fa8e80c502d388f106 Author: Gerd Hoffmann <kraxel at redhat.com> Date: Fri Feb 7 08:46:38 2020 +0100

. On Wed, Jun 19, 2019 at 11:08 PM Gerd Hoffmann <kraxel at redhat.com> wrote: > > virtio-gpu basically needs a sg_table for the bo, to tell the host where > the backing pages for the object are. So the gem shmem helpers are a > perfect fit. Some drm_gem_object_funcs need thin wrappers to update the > host state, but otherwise the helpers handle everything just fine. >

virtio-gpu basically needs a sg_table for the bo, to tell the host where the backing pages for the object are. So the gem shmem helpers are a perfect fit. Some drm_gem_object_funcs need thin wrappers to update the host state, but otherwise the helpers handle everything just fine. Once the fencing was sorted the switch was surprisingly easy and for the most part just removing the ttm code.

These days dma ops can be overridden per device, and the virtio core uses that to handle the dma quirks transparently for the rest of the kernel. So we can drop the virtio_has_dma_quirk() checks, just use the dma api unconditionally and depend on the virtio core having setup dma_ops as needed. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_object.c

These days dma ops can be overridden per device, and the virtio core uses that to handle the dma quirks transparently for the rest of the kernel. So we can drop the virtio_has_dma_quirk() checks, just use the dma api unconditionally and depend on the virtio core having setup dma_ops as needed. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/virtio/virtgpu_object.c

On Tue, Sep 08, 2020 at 08:47:41AM +0200, Gerd Hoffmann wrote: > These days dma ops can be overridden per device, and the virtio core "can be overridden" or "are"? The comment above vring_use_dma_api() suggests that's not yet done. If that's wrong then I think updating the comment would be really good. -Daniel > uses that to handle the dma quirks transparently

virtio-gpu basically needs a sg_table for the bo, to tell the host where the backing pages for the object are. So the gem shmem helpers are a perfect fit. Some drm_gem_object_funcs need thin wrappers to update the host state, but otherwise the helpers handle everything just fine. Once the fencing was sorted the switch was surprisingly easy and for the most part just removing the ttm code. v4:

virtio-gpu basically needs a sg_table for the bo, to tell the host where the backing pages for the object are. So the gem shmem helpers are a perfect fit. Some drm_gem_object_funcs need thin wrappers to update the host state, but otherwise the helpers handle everything just fine. Once the fencing was sorted the switch was surprisingly easy and for the most part just removing the ttm code.

virtio-gpu basically needs a sg_table for the bo, to tell the host where the backing pages for the object are. So the gem shmem helpers are a perfect fit. Some drm_gem_object_funcs need thin wrappers to update the host state, but otherwise the helpers handle everything just fine. Once the fencing was sorted the switch was surprisingly easy and for the most part just removing the ttm code. v5:

On Wed, Feb 5, 2020 at 3:00 AM Gerd Hoffmann <kraxel at redhat.com> wrote: > > Add new virtio_gpu_cleanup_object() helper function for object cleanup. > Wire up callback function for resource unref, do cleanup from callback > when we know the host stopped using the resource. > > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> > --- >