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 <tzimmerm...@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 --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);
-- 
2.45.2

Reply via email to