drm/xe: Add staging tree for VM binds

Concurrent VM bind staging and zapping of PTEs from a userptr notifier
do not work because the view of PTEs is not stable. VM binds cannot
acquire the notifier lock during staging, as memory allocations are
required. To resolve this race condition, use a staging tree for VM
binds that is committed only under the userptr notifier lock during the
final step of the bind. This ensures a consistent view of the PTEs in
the userptr notifier.

A follow up may only use staging for VM in fault mode as this is the
only mode in which the above race exists.

v3:
 - Drop zap PTE change (Thomas)
 - s/xe_pt_entry/xe_pt_entry_staging (Thomas)

Suggested-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Cc: <stable@vger.kernel.org>
Fixes: e8babb280b5e ("drm/xe: Convert multiple bind ops into single job")
Fixes: a708f6501c69 ("drm/xe: Update PT layer with better error handling")
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250228073058.59510-5-thomas.hellstrom@linux.intel.com
Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
(cherry picked from commit 6f39b0c5ef0385eae586760d10b9767168037aa5)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
index 12a627a23eb45152f1c4d3d24a537e87672426d3..dc24baa840924429d1dc9a3b69806bf0c678bc2e 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pt.c
+++ b/drivers/gpu/drm/xe/xe_pt.c
@@ -28,6 +28,8 @@ struct xe_pt_dir {
        struct xe_pt pt;
        /** @children: Array of page-table child nodes */
        struct xe_ptw *children[XE_PDES];
+       /** @staging: Array of page-table staging nodes */
+       struct xe_ptw *staging[XE_PDES];
 };
 
 #if IS_ENABLED(CONFIG_DRM_XE_DEBUG_VM)
@@ -48,9 +50,10 @@ static struct xe_pt_dir *as_xe_pt_dir(struct xe_pt *pt)
        return container_of(pt, struct xe_pt_dir, pt);
 }
 
-static struct xe_pt *xe_pt_entry(struct xe_pt_dir *pt_dir, unsigned int index)
+static struct xe_pt *
+xe_pt_entry_staging(struct xe_pt_dir *pt_dir, unsigned int index)
 {
-       return container_of(pt_dir->children[index], struct xe_pt, base);
+       return container_of(pt_dir->staging[index], struct xe_pt, base);
 }
 
 static u64 __xe_pt_empty_pte(struct xe_tile *tile, struct xe_vm *vm,
@@ -125,6 +128,7 @@ struct xe_pt *xe_pt_create(struct xe_vm *vm, struct xe_tile *tile,
        }
        pt->bo = bo;
        pt->base.children = level ? as_xe_pt_dir(pt)->children : NULL;
+       pt->base.staging = level ? as_xe_pt_dir(pt)->staging : NULL;
 
        if (vm->xef)
                xe_drm_client_add_bo(vm->xef->client, pt->bo);
@@ -206,8 +210,8 @@ void xe_pt_destroy(struct xe_pt *pt, u32 flags, struct llist_head *deferred)
                struct xe_pt_dir *pt_dir = as_xe_pt_dir(pt);
 
                for (i = 0; i < XE_PDES; i++) {
-                       if (xe_pt_entry(pt_dir, i))
-                               xe_pt_destroy(xe_pt_entry(pt_dir, i), flags,
+                       if (xe_pt_entry_staging(pt_dir, i))
+                               xe_pt_destroy(xe_pt_entry_staging(pt_dir, i), flags,
                                              deferred);
                }
        }
@@ -376,8 +380,10 @@ xe_pt_insert_entry(struct xe_pt_stage_bind_walk *xe_walk, struct xe_pt *parent,
                /* Continue building a non-connected subtree. */
                struct iosys_map *map = &parent->bo->vmap;
 
-               if (unlikely(xe_child))
+               if (unlikely(xe_child)) {
                        parent->base.children[offset] = &xe_child->base;
+                       parent->base.staging[offset] = &xe_child->base;
+               }
 
                xe_pt_write(xe_walk->vm->xe, map, offset, pte);
                parent->num_live++;
@@ -614,6 +620,7 @@ xe_pt_stage_bind(struct xe_tile *tile, struct xe_vma *vma,
                        .ops = &xe_pt_stage_bind_ops,
                        .shifts = xe_normal_pt_shifts,
                        .max_level = XE_PT_HIGHEST_LEVEL,
+                       .staging = true,
                },
                .vm = xe_vma_vm(vma),
                .tile = tile,
@@ -873,7 +880,7 @@ static void xe_pt_cancel_bind(struct xe_vma *vma,
        }
 }
 
-static void xe_pt_commit_locks_assert(struct xe_vma *vma)
+static void xe_pt_commit_prepare_locks_assert(struct xe_vma *vma)
 {
        struct xe_vm *vm = xe_vma_vm(vma);
 
@@ -885,6 +892,16 @@ static void xe_pt_commit_locks_assert(struct xe_vma *vma)
        xe_vm_assert_held(vm);
 }
 
+static void xe_pt_commit_locks_assert(struct xe_vma *vma)
+{
+       struct xe_vm *vm = xe_vma_vm(vma);
+
+       xe_pt_commit_prepare_locks_assert(vma);
+
+       if (xe_vma_is_userptr(vma))
+               lockdep_assert_held_read(&vm->userptr.notifier_lock);
+}
+
 static void xe_pt_commit(struct xe_vma *vma,
                         struct xe_vm_pgtable_update *entries,
                         u32 num_entries, struct llist_head *deferred)
@@ -895,13 +912,17 @@ static void xe_pt_commit(struct xe_vma *vma,
 
        for (i = 0; i < num_entries; i++) {
                struct xe_pt *pt = entries[i].pt;
+               struct xe_pt_dir *pt_dir;
 
                if (!pt->level)
                        continue;
 
+               pt_dir = as_xe_pt_dir(pt);
                for (j = 0; j < entries[i].qwords; j++) {
                        struct xe_pt *oldpte = entries[i].pt_entries[j].pt;
+                       int j_ = j + entries[i].ofs;
 
+                       pt_dir->children[j_] = pt_dir->staging[j_];
                        xe_pt_destroy(oldpte, xe_vma_vm(vma)->flags, deferred);
                }
        }
@@ -913,7 +934,7 @@ static void xe_pt_abort_bind(struct xe_vma *vma,
 {
        int i, j;
 
-       xe_pt_commit_locks_assert(vma);
+       xe_pt_commit_prepare_locks_assert(vma);
 
        for (i = num_entries - 1; i >= 0; --i) {
                struct xe_pt *pt = entries[i].pt;
@@ -928,10 +949,10 @@ static void xe_pt_abort_bind(struct xe_vma *vma,
                pt_dir = as_xe_pt_dir(pt);
                for (j = 0; j < entries[i].qwords; j++) {
                        u32 j_ = j + entries[i].ofs;
-                       struct xe_pt *newpte = xe_pt_entry(pt_dir, j_);
+                       struct xe_pt *newpte = xe_pt_entry_staging(pt_dir, j_);
                        struct xe_pt *oldpte = entries[i].pt_entries[j].pt;
 
-                       pt_dir->children[j_] = oldpte ? &oldpte->base : 0;
+                       pt_dir->staging[j_] = oldpte ? &oldpte->base : 0;
                        xe_pt_destroy(newpte, xe_vma_vm(vma)->flags, NULL);
                }
        }
@@ -943,7 +964,7 @@ static void xe_pt_commit_prepare_bind(struct xe_vma *vma,
 {
        u32 i, j;
 
-       xe_pt_commit_locks_assert(vma);
+       xe_pt_commit_prepare_locks_assert(vma);
 
        for (i = 0; i < num_entries; i++) {
                struct xe_pt *pt = entries[i].pt;
@@ -961,10 +982,10 @@ static void xe_pt_commit_prepare_bind(struct xe_vma *vma,
                        struct xe_pt *newpte = entries[i].pt_entries[j].pt;
                        struct xe_pt *oldpte = NULL;
 
-                       if (xe_pt_entry(pt_dir, j_))
-                               oldpte = xe_pt_entry(pt_dir, j_);
+                       if (xe_pt_entry_staging(pt_dir, j_))
+                               oldpte = xe_pt_entry_staging(pt_dir, j_);
 
-                       pt_dir->children[j_] = &newpte->base;
+                       pt_dir->staging[j_] = &newpte->base;
                        entries[i].pt_entries[j].pt = oldpte;
                }
        }
@@ -1494,6 +1515,7 @@ static unsigned int xe_pt_stage_unbind(struct xe_tile *tile, struct xe_vma *vma,
                        .ops = &xe_pt_stage_unbind_ops,
                        .shifts = xe_normal_pt_shifts,
                        .max_level = XE_PT_HIGHEST_LEVEL,
+                       .staging = true,
                },
                .tile = tile,
                .modified_start = xe_vma_start(vma),
@@ -1535,7 +1557,7 @@ static void xe_pt_abort_unbind(struct xe_vma *vma,
 {
        int i, j;
 
-       xe_pt_commit_locks_assert(vma);
+       xe_pt_commit_prepare_locks_assert(vma);
 
        for (i = num_entries - 1; i >= 0; --i) {
                struct xe_vm_pgtable_update *entry = &entries[i];
@@ -1548,7 +1570,7 @@ static void xe_pt_abort_unbind(struct xe_vma *vma,
                        continue;
 
                for (j = entry->ofs; j < entry->ofs + entry->qwords; j++)
-                       pt_dir->children[j] =
+                       pt_dir->staging[j] =
                                entries[i].pt_entries[j - entry->ofs].pt ?
                                &entries[i].pt_entries[j - entry->ofs].pt->base : NULL;
        }
@@ -1561,7 +1583,7 @@ xe_pt_commit_prepare_unbind(struct xe_vma *vma,
 {
        int i, j;
 
-       xe_pt_commit_locks_assert(vma);
+       xe_pt_commit_prepare_locks_assert(vma);
 
        for (i = 0; i < num_entries; ++i) {
                struct xe_vm_pgtable_update *entry = &entries[i];
@@ -1575,8 +1597,8 @@ xe_pt_commit_prepare_unbind(struct xe_vma *vma,
                pt_dir = as_xe_pt_dir(pt);
                for (j = entry->ofs; j < entry->ofs + entry->qwords; j++) {
                        entry->pt_entries[j - entry->ofs].pt =
-                               xe_pt_entry(pt_dir, j);
-                       pt_dir->children[j] = NULL;
+                               xe_pt_entry_staging(pt_dir, j);
+                       pt_dir->staging[j] = NULL;
                }
        }
 }

diff --git a/drivers/gpu/drm/xe/xe_pt_walk.c b/drivers/gpu/drm/xe/xe_pt_walk.c
index b8b3d2aea4923d0ac087f6a2c972652aba8efc6f..be602a763ff32b75c96891c82230af3a3a560354 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pt_walk.c
+++ b/drivers/gpu/drm/xe/xe_pt_walk.c
@@ -74,7 +74,8 @@ int xe_pt_walk_range(struct xe_ptw *parent, unsigned int level,
                     u64 addr, u64 end, struct xe_pt_walk *walk)
 {
        pgoff_t offset = xe_pt_offset(addr, level, walk);
-       struct xe_ptw **entries = parent->children ? parent->children : NULL;
+       struct xe_ptw **entries = walk->staging ? (parent->staging ?: NULL) :
+               (parent->children ?: NULL);
        const struct xe_pt_walk_ops *ops = walk->ops;
        enum page_walk_action action;
        struct xe_ptw *child;

diff --git a/drivers/gpu/drm/xe/xe_pt_walk.h b/drivers/gpu/drm/xe/xe_pt_walk.h
index 5ecc4d2f0f6536b7ec79033f80f556ce1f00edc5..5c02c244f7de35ae2d083744f1c4afe1121941b6 100644 (file)
--- a/drivers/gpu/drm/xe/xe_pt_walk.h
+++ b/drivers/gpu/drm/xe/xe_pt_walk.h
@@ -11,12 +11,14 @@
 /**
  * struct xe_ptw - base class for driver pagetable subclassing.
  * @children: Pointer to an array of children if any.
+ * @staging: Pointer to an array of staging if any.
  *
  * Drivers could subclass this, and if it's a page-directory, typically
  * embed an array of xe_ptw pointers.
  */
 struct xe_ptw {
        struct xe_ptw **children;
+       struct xe_ptw **staging;
 };
 
 /**
@@ -41,6 +43,8 @@ struct xe_pt_walk {
         * as shared pagetables.
         */
        bool shared_pt_mode;
+       /** @staging: Walk staging PT structure */
+       bool staging;
 };
 
 /**