[PATCH v2 1/2] drm/virtio: Don't attach GEM to a non-created context in gem_object_open()

[PATCH v2 1/2] drm/virtio: Don't attach GEM to a non-created context in gem_object_open()

[Dmitry Osipenko]

The vfpriv->ctx_id is always initialized to a non-zero value. Check whether
context was created before attaching GEM to this context ID. This left
unnoticed previously because host silently skips attachment if context
doesn't exist, still we shouldn't do that for consistency.

Fixes: 086b9f27f0ab ("drm/virtio: Don't create a context with default param if context_init is supported")
Cc: <stable@vger.kernel.org> # v6.14+
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---
 drivers/gpu/drm/virtio/virtgpu_gem.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
index 5aab588fc400..3d6aa26fdb53 100644
--- a/drivers/gpu/drm/virtio/virtgpu_gem.c
+++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
@@ -115,13 +115,14 @@ int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
 	if (!vgdev->has_context_init)
 		virtio_gpu_create_context(obj->dev, file);
 
-	objs = virtio_gpu_array_alloc(1);
-	if (!objs)
-		return -ENOMEM;
-	virtio_gpu_array_add_obj(objs, obj);
+	if (vfpriv->context_created) {
+		objs = virtio_gpu_array_alloc(1);
+		if (!objs)
+			return -ENOMEM;
+		virtio_gpu_array_add_obj(objs, obj);
 
-	if (vfpriv->ctx_id)
 		virtio_gpu_cmd_context_attach_resource(vgdev, vfpriv->ctx_id, objs);
+	}
 
 out_notify:
 	virtio_gpu_notify(vgdev);
-- 
2.49.0

[PATCH v2 2/2] drm/virtio: Fix missed dmabuf unpinning in error path of prepare_fb()

[Dmitry Osipenko]

Correct error handling in prepare_fb() to fix leaking resources when
error happens.

Fixes: 4a696a2ee646 ("drm/virtio: Add prepare and cleanup routines for imported dmabuf obj")
Cc: <stable@vger.kernel.org> # v6.14+
Acked-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
---

v2: Change order of prep_obj/fence_alloc, add ack from Vivek

 drivers/gpu/drm/virtio/virtgpu_plane.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c
index 42aa554eca9f..26abe3d1b122 100644
--- a/drivers/gpu/drm/virtio/virtgpu_plane.c
+++ b/drivers/gpu/drm/virtio/virtgpu_plane.c
@@ -322,12 +322,6 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
 		return 0;
 
 	obj = new_state->fb->obj[0];
-	if (obj->import_attach) {
-		ret = virtio_gpu_prepare_imported_obj(plane, new_state, obj);
-		if (ret)
-			return ret;
-	}
-
 	if (bo->dumb || obj->import_attach) {
 		vgplane_st->fence = virtio_gpu_fence_alloc(vgdev,
 						     vgdev->fence_drv.context,
@@ -336,7 +330,21 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane *plane,
 			return -ENOMEM;
 	}
 
+	if (obj->import_attach) {
+		ret = virtio_gpu_prepare_imported_obj(plane, new_state, obj);
+		if (ret)
+			goto err_fence;
+	}
+
 	return 0;
+
+err_fence:
+	if (vgplane_st->fence) {
+		dma_fence_put(&vgplane_st->fence->f);
+		vgplane_st->fence = NULL;
+	}
+
+	return ret;
 }
 
 static void virtio_gpu_cleanup_imported_obj(struct drm_gem_object *obj)
-- 
2.49.0

Re: [PATCH v2 1/2] drm/virtio: Don't attach GEM to a non-created context in gem_object_open()

[Rob Clark]

On Tue, Apr 1, 2025 at 5:39 AM Dmitry Osipenko
<dmitry.osipenko@collabora.com> wrote:
>
> The vfpriv->ctx_id is always initialized to a non-zero value. Check whether
> context was created before attaching GEM to this context ID. This left
> unnoticed previously because host silently skips attachment if context
> doesn't exist, still we shouldn't do that for consistency.
>
> Fixes: 086b9f27f0ab ("drm/virtio: Don't create a context with default param if context_init is supported")
> Cc: <stable@vger.kernel.org> # v6.14+
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>  drivers/gpu/drm/virtio/virtgpu_gem.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_gem.c b/drivers/gpu/drm/virtio/virtgpu_gem.c
> index 5aab588fc400..3d6aa26fdb53 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_gem.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_gem.c
> @@ -115,13 +115,14 @@ int virtio_gpu_gem_object_open(struct drm_gem_object *obj,
>         if (!vgdev->has_context_init)
>                 virtio_gpu_create_context(obj->dev, file);
>
> -       objs = virtio_gpu_array_alloc(1);
> -       if (!objs)
> -               return -ENOMEM;
> -       virtio_gpu_array_add_obj(objs, obj);
> +       if (vfpriv->context_created) {
> +               objs = virtio_gpu_array_alloc(1);
> +               if (!objs)
> +                       return -ENOMEM;
> +               virtio_gpu_array_add_obj(objs, obj);
>
> -       if (vfpriv->ctx_id)
>                 virtio_gpu_cmd_context_attach_resource(vgdev, vfpriv->ctx_id, objs);
> +       }
>
>  out_notify:
>         virtio_gpu_notify(vgdev);
> --
> 2.49.0
>

Re: [PATCH v2 1/2] drm/virtio: Don't attach GEM to a non-created context in gem_object_open()

[Dmitry Osipenko]

On 4/1/25 22:53, Rob Clark wrote:
> On Tue, Apr 1, 2025 at 5:39 AM Dmitry Osipenko
> <dmitry.osipenko@collabora.com> wrote:
>>
>> The vfpriv->ctx_id is always initialized to a non-zero value. Check whether
>> context was created before attaching GEM to this context ID. This left
>> unnoticed previously because host silently skips attachment if context
>> doesn't exist, still we shouldn't do that for consistency.
>>
>> Fixes: 086b9f27f0ab ("drm/virtio: Don't create a context with default param if context_init is supported")
>> Cc: <stable@vger.kernel.org> # v6.14+
>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> 
> Reviewed-by: Rob Clark <robdclark@gmail.com>

Thanks, applied both patches to misc-fixes

-- 
Best regards,
Dmitry