diff mbox series

[5/5] drm/vmwgfx: Fix the lifetime of the bo cursor memory

Message ID 20240126200804.732454-6-zack.rusin@broadcom.com (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: Various kms related fixes | expand

Commit Message

Zack Rusin Jan. 26, 2024, 8:08 p.m. UTC
The cleanup can be dispatched while the atomic update is still active,
which means that the memory acquired in the atomic update needs to
not be invalidated by the cleanup. The buffer objects in vmw_plane_state
instead of using the builtin map_and_cache were trying to handle
the lifetime of the mapped memory themselves, leading to crashes.

Use the map_and_cache instead of trying to manage the lifetime of the
buffer objects held by the vmw_plane_state.

Fixes kernel oops'es in IGT's kms_cursor_legacy forked-bo.

Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
Fixes: bb6780aa5a1d ("drm/vmwgfx: Diff cursors when using cmds")
Cc: <stable@vger.kernel.org> # v6.2+
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

Comments

Martin Krastev Jan. 27, 2024, 9:56 a.m. UTC | #1
On Fri, Jan 26, 2024 at 10:08 PM Zack Rusin <zack.rusin@broadcom.com> wrote:
>
> The cleanup can be dispatched while the atomic update is still active,
> which means that the memory acquired in the atomic update needs to
> not be invalidated by the cleanup. The buffer objects in vmw_plane_state
> instead of using the builtin map_and_cache were trying to handle
> the lifetime of the mapped memory themselves, leading to crashes.
>
> Use the map_and_cache instead of trying to manage the lifetime of the
> buffer objects held by the vmw_plane_state.
>
> Fixes kernel oops'es in IGT's kms_cursor_legacy forked-bo.
>
> Signed-off-by: Zack Rusin <zack.rusin@broadcom.com>
> Fixes: bb6780aa5a1d ("drm/vmwgfx: Diff cursors when using cmds")
> Cc: <stable@vger.kernel.org> # v6.2+
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index e2bfaf4522a6..cd4925346ed4 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -185,13 +185,12 @@ static u32 vmw_du_cursor_mob_size(u32 w, u32 h)
>   */
>  static u32 *vmw_du_cursor_plane_acquire_image(struct vmw_plane_state *vps)
>  {
> -       bool is_iomem;
>         if (vps->surf) {
>                 if (vps->surf_mapped)
>                         return vmw_bo_map_and_cache(vps->surf->res.guest_memory_bo);
>                 return vps->surf->snooper.image;
>         } else if (vps->bo)
> -               return ttm_kmap_obj_virtual(&vps->bo->map, &is_iomem);
> +               return vmw_bo_map_and_cache(vps->bo);
>         return NULL;
>  }
>
> @@ -653,22 +652,12 @@ vmw_du_cursor_plane_cleanup_fb(struct drm_plane *plane,
>  {
>         struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
>         struct vmw_plane_state *vps = vmw_plane_state_to_vps(old_state);
> -       bool is_iomem;
>
>         if (vps->surf_mapped) {
>                 vmw_bo_unmap(vps->surf->res.guest_memory_bo);
>                 vps->surf_mapped = false;
>         }
>
> -       if (vps->bo && ttm_kmap_obj_virtual(&vps->bo->map, &is_iomem)) {
> -               const int ret = ttm_bo_reserve(&vps->bo->tbo, true, false, NULL);
> -
> -               if (likely(ret == 0)) {
> -                       ttm_bo_kunmap(&vps->bo->map);
> -                       ttm_bo_unreserve(&vps->bo->tbo);
> -               }
> -       }
> -
>         vmw_du_cursor_plane_unmap_cm(vps);
>         vmw_du_put_cursor_mob(vcp, vps);
>
> --
> 2.40.1
>

LGTM!

Reviewed-by: Martin Krastev <martin.krastev@broadcom.com>

Regards,
Martin
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index e2bfaf4522a6..cd4925346ed4 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -185,13 +185,12 @@  static u32 vmw_du_cursor_mob_size(u32 w, u32 h)
  */
 static u32 *vmw_du_cursor_plane_acquire_image(struct vmw_plane_state *vps)
 {
-	bool is_iomem;
 	if (vps->surf) {
 		if (vps->surf_mapped)
 			return vmw_bo_map_and_cache(vps->surf->res.guest_memory_bo);
 		return vps->surf->snooper.image;
 	} else if (vps->bo)
-		return ttm_kmap_obj_virtual(&vps->bo->map, &is_iomem);
+		return vmw_bo_map_and_cache(vps->bo);
 	return NULL;
 }
 
@@ -653,22 +652,12 @@  vmw_du_cursor_plane_cleanup_fb(struct drm_plane *plane,
 {
 	struct vmw_cursor_plane *vcp = vmw_plane_to_vcp(plane);
 	struct vmw_plane_state *vps = vmw_plane_state_to_vps(old_state);
-	bool is_iomem;
 
 	if (vps->surf_mapped) {
 		vmw_bo_unmap(vps->surf->res.guest_memory_bo);
 		vps->surf_mapped = false;
 	}
 
-	if (vps->bo && ttm_kmap_obj_virtual(&vps->bo->map, &is_iomem)) {
-		const int ret = ttm_bo_reserve(&vps->bo->tbo, true, false, NULL);
-
-		if (likely(ret == 0)) {
-			ttm_bo_kunmap(&vps->bo->map);
-			ttm_bo_unreserve(&vps->bo->tbo);
-		}
-	}
-
 	vmw_du_cursor_plane_unmap_cm(vps);
 	vmw_du_put_cursor_mob(vcp, vps);