diff mbox series

[5/6] drm/xe: Remove vunmap calls object-freeing code

Message ID 20240614133556.11378-6-tzimmermann@suse.de (mailing list archive)
State New, archived
Headers show
Series drm/{ttm,xe}: Improve ttm_bo_vmap() and update xe | expand

Commit Message

Thomas Zimmermann June 14, 2024, 1:21 p.m. UTC
Move calls to unmap the buffer-object memory from the object-release
code in xe_gem_object_free() to the caller of the release.

Doing an unmap for a BO requires holding the reservation lock, which
is not allowed while releasing a GEM object. Without the reservation
lock, TTM can concurrently evict the buffer object that is to be
released; making the mapping invalid.

Pushing the unmap calls, namely xe_bo_vunmap(), releases the mapped
pages before the buffer object.

While at it, add a warning about buffer mappings to the GEM-object
release code. The warning message and unmap code can be removed after
the driver has been audited to not release mapped buffer objects.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/xe/display/intel_fb_bo.c | 12 +++++++-----
 drivers/gpu/drm/xe/xe_bo.c               | 14 +++++++++++++-
 drivers/gpu/drm/xe/xe_bo.h               | 23 ++++++++++++-----------
 drivers/gpu/drm/xe/xe_lrc.c              |  1 +
 4 files changed, 33 insertions(+), 17 deletions(-)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/xe/display/intel_fb_bo.c b/drivers/gpu/drm/xe/display/intel_fb_bo.c
index f835492f73fb4..cf2720dbdd51f 100644
--- a/drivers/gpu/drm/xe/display/intel_fb_bo.c
+++ b/drivers/gpu/drm/xe/display/intel_fb_bo.c
@@ -12,12 +12,14 @@ 
 
 void intel_fb_bo_framebuffer_fini(struct xe_bo *bo)
 {
-	if (bo->flags & XE_BO_FLAG_PINNED) {
-		/* Unpin our kernel fb first */
-		xe_bo_lock(bo, false);
+	xe_bo_lock(bo, false);
+	xe_bo_vunmap(bo);
+
+	/* Unpin our kernel fb first */
+	if (bo->flags & XE_BO_FLAG_PINNED)
 		xe_bo_unpin(bo);
-		xe_bo_unlock(bo);
-	}
+
+	xe_bo_unlock(bo);
 	xe_bo_put(bo);
 }
 
diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
index 2bae01ce4e5b9..a98d857ff1e5a 100644
--- a/drivers/gpu/drm/xe/xe_bo.c
+++ b/drivers/gpu/drm/xe/xe_bo.c
@@ -1093,6 +1093,19 @@  static void xe_ttm_bo_destroy(struct ttm_buffer_object *ttm_bo)
 
 static void xe_gem_object_free(struct drm_gem_object *obj)
 {
+	struct xe_bo *bo = gem_to_xe_bo(obj);
+
+	/*
+	 * Trying to free the object with a mapping in place. Resolve
+	 * this warning by calling xe_bo_vunmap() in the code that leads
+	 * ot this object release.
+	 *
+	 * TODO: Audit the driver to not release mapped buffer objects and
+	 *       then remove this block.
+	 */
+	if (drm_WARN_ON(obj->dev, !iosys_map_is_null(&bo->vmap)))
+		__xe_bo_vunmap(bo);
+
 	/* Our BO reference counting scheme works as follows:
 	 *
 	 * The gem object kref is typically used throughout the driver,
@@ -1106,7 +1119,6 @@  static void xe_gem_object_free(struct drm_gem_object *obj)
 	 * driver ttm callbacks is allowed to use the ttm_buffer_object
 	 * refcount directly if needed.
 	 */
-	__xe_bo_vunmap(gem_to_xe_bo(obj));
 	ttm_bo_put(container_of(obj, struct ttm_buffer_object, base));
 }
 
diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
index 6de894c728f54..5d1f8f13fbf13 100644
--- a/drivers/gpu/drm/xe/xe_bo.h
+++ b/drivers/gpu/drm/xe/xe_bo.h
@@ -170,17 +170,6 @@  static inline bool xe_bo_is_pinned(struct xe_bo *bo)
 	return bo->ttm.pin_count;
 }
 
-static inline void xe_bo_unpin_map_no_vm(struct xe_bo *bo)
-{
-	if (likely(bo)) {
-		xe_bo_lock(bo, false);
-		xe_bo_unpin(bo);
-		xe_bo_unlock(bo);
-
-		xe_bo_put(bo);
-	}
-}
-
 bool xe_bo_is_xe_bo(struct ttm_buffer_object *bo);
 dma_addr_t __xe_bo_addr(struct xe_bo *bo, u64 offset, size_t page_size);
 dma_addr_t xe_bo_addr(struct xe_bo *bo, u64 offset, size_t page_size);
@@ -202,6 +191,18 @@  xe_bo_ggtt_addr(struct xe_bo *bo)
 int xe_bo_vmap(struct xe_bo *bo);
 void xe_bo_vunmap(struct xe_bo *bo);
 
+static inline void xe_bo_unpin_map_no_vm(struct xe_bo *bo)
+{
+	if (likely(bo)) {
+		xe_bo_lock(bo, false);
+		xe_bo_vunmap(bo);
+		xe_bo_unpin(bo);
+		xe_bo_unlock(bo);
+
+		xe_bo_put(bo);
+	}
+}
+
 bool mem_type_is_vram(u32 mem_type);
 bool xe_bo_is_vram(struct xe_bo *bo);
 bool xe_bo_is_stolen(struct xe_bo *bo);
diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
index 3a68fe6d592ed..8a52797fe79bb 100644
--- a/drivers/gpu/drm/xe/xe_lrc.c
+++ b/drivers/gpu/drm/xe/xe_lrc.c
@@ -812,6 +812,7 @@  static void xe_lrc_finish(struct xe_lrc *lrc)
 {
 	xe_hw_fence_ctx_finish(&lrc->fence_ctx);
 	xe_bo_lock(lrc->bo, false);
+	xe_bo_vunmap(lrc->bo);
 	xe_bo_unpin(lrc->bo);
 	xe_bo_unlock(lrc->bo);
 	xe_bo_put(lrc->bo);