* [PATCH 02/10] drm/amdgpu: fix eviction fence and userq manager shutdown
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 03/10] drm/amdgpu: fix adding eviction fence Christian König
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
That is a really complicated dance and wasn't implemented fully correct.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 ++
drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c | 8 +++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h | 1 +
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 5 +++++
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h | 1 +
5 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index bc0c62c312ff..a44baa9ee78d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2953,6 +2953,8 @@ static int amdgpu_drm_release(struct inode *inode, struct file *filp)
if (fpriv && drm_dev_enter(dev, &idx)) {
amdgpu_evf_mgr_shutdown(&fpriv->evf_mgr);
+ amdgpu_userq_mgr_cancel_resume(&fpriv->userq_mgr);
+ amdgpu_evf_mgr_flush_suspend(&fpriv->evf_mgr);
amdgpu_userq_mgr_fini(&fpriv->userq_mgr);
amdgpu_evf_mgr_fini(&fpriv->evf_mgr);
drm_dev_exit(idx);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index 69698cb60778..ccbdb3068b6f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -143,13 +143,19 @@ void amdgpu_evf_mgr_init(struct amdgpu_eviction_fence_mgr *evf_mgr)
void amdgpu_evf_mgr_shutdown(struct amdgpu_eviction_fence_mgr *evf_mgr)
{
evf_mgr->shutdown = true;
+ /* Make sure that the shutdown is visible to the suspend work */
flush_work(&evf_mgr->suspend_work);
}
-void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr)
+void amdgpu_evf_mgr_flush_suspend(struct amdgpu_eviction_fence_mgr *evf_mgr)
{
dma_fence_wait(rcu_dereference_protected(evf_mgr->ev_fence, true),
false);
+ /* Make sure that we are done with the last suspend work */
flush_work(&evf_mgr->suspend_work);
+}
+
+void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr)
+{
dma_fence_put(evf_mgr->ev_fence);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
index 527de3a23583..132a13a5dc1c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
@@ -66,6 +66,7 @@ void amdgpu_evf_mgr_detach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
struct amdgpu_bo *bo);
void amdgpu_evf_mgr_init(struct amdgpu_eviction_fence_mgr *evf_mgr);
void amdgpu_evf_mgr_shutdown(struct amdgpu_eviction_fence_mgr *evf_mgr);
+void amdgpu_evf_mgr_flush_suspend(struct amdgpu_eviction_fence_mgr *evf_mgr);
void amdgpu_evf_mgr_fini(struct amdgpu_eviction_fence_mgr *evf_mgr);
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 5bc804636015..2c1b8bbbc903 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1345,6 +1345,11 @@ int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *f
return 0;
}
+void amdgpu_userq_mgr_cancel_resume(struct amdgpu_userq_mgr *userq_mgr)
+{
+ cancel_delayed_work_sync(&userq_mgr->resume_work);
+}
+
void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr)
{
struct amdgpu_usermode_queue *queue;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
index 82306d489064..f0abc16d02cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
@@ -123,6 +123,7 @@ int amdgpu_userq_ioctl(struct drm_device *dev, void *data, struct drm_file *filp
int amdgpu_userq_mgr_init(struct amdgpu_userq_mgr *userq_mgr, struct drm_file *file_priv,
struct amdgpu_device *adev);
+void amdgpu_userq_mgr_cancel_resume(struct amdgpu_userq_mgr *userq_mgr);
void amdgpu_userq_mgr_fini(struct amdgpu_userq_mgr *userq_mgr);
int amdgpu_userq_create_object(struct amdgpu_userq_mgr *uq_mgr,
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 03/10] drm/amdgpu: fix adding eviction fence
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
2026-03-17 10:54 ` [PATCH 02/10] drm/amdgpu: fix eviction fence and userq manager shutdown Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 04/10] drm/amdgpu: rework amdgpu_userq_wait_ioctl v4 Christian König
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
We can't add the eviction fence without validating the BO.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
.../drm/amd/amdgpu/amdgpu_eviction_fence.c | 19 ++++++++++++++++---
.../drm/amd/amdgpu/amdgpu_eviction_fence.h | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 9 ++++++---
3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index ccbdb3068b6f..ef7d07a134ce 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -79,14 +79,27 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
mutex_unlock(&uq_mgr->userq_mutex);
}
-void amdgpu_evf_mgr_attach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
- struct amdgpu_bo *bo)
+int amdgpu_evf_mgr_attach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
+ struct amdgpu_bo *bo)
{
struct dma_fence *ev_fence = amdgpu_evf_mgr_get_fence(evf_mgr);
+ struct ttm_operation_ctx ctx = { false, false };
struct dma_resv *resv = bo->tbo.base.resv;
+ int ret;
+
+ if (!dma_fence_is_signaled(ev_fence)) {
+
+ amdgpu_bo_placement_from_domain(bo, bo->allowed_domains);
+ ret = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
+ if (!ret)
+ dma_resv_add_fence(resv, ev_fence,
+ DMA_RESV_USAGE_BOOKKEEP);
+ } else {
+ ret = 0;
+ }
- dma_resv_add_fence(resv, ev_fence, DMA_RESV_USAGE_BOOKKEEP);
dma_fence_put(ev_fence);
+ return ret;
}
int amdgpu_evf_mgr_rearm(struct amdgpu_eviction_fence_mgr *evf_mgr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
index 132a13a5dc1c..2a750add4e7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.h
@@ -58,8 +58,8 @@ amdgpu_evf_mgr_get_fence(struct amdgpu_eviction_fence_mgr *evf_mgr)
return ev_fence;
}
-void amdgpu_evf_mgr_attach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
- struct amdgpu_bo *bo);
+int amdgpu_evf_mgr_attach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
+ struct amdgpu_bo *bo);
int amdgpu_evf_mgr_rearm(struct amdgpu_eviction_fence_mgr *evf_mgr,
struct drm_exec *exec);
void amdgpu_evf_mgr_detach_fence(struct amdgpu_eviction_fence_mgr *evf_mgr,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index e28abfd04867..88a21400ae09 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -258,12 +258,15 @@ static int amdgpu_gem_object_open(struct drm_gem_object *obj,
amdgpu_vm_bo_update_shared(abo);
bo_va = amdgpu_vm_bo_find(vm, abo);
- if (!bo_va)
+ if (!bo_va) {
bo_va = amdgpu_vm_bo_add(adev, vm, abo);
- else
+ r = amdgpu_evf_mgr_attach_fence(&fpriv->evf_mgr, abo);
+ if (r)
+ goto out_unlock;
+ } else {
++bo_va->ref_count;
+ }
- amdgpu_evf_mgr_attach_fence(&fpriv->evf_mgr, abo);
drm_exec_fini(&exec);
/* Validate and add eviction fence to DMABuf imports with dynamic
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 04/10] drm/amdgpu: rework amdgpu_userq_wait_ioctl v4
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
2026-03-17 10:54 ` [PATCH 02/10] drm/amdgpu: fix eviction fence and userq manager shutdown Christian König
2026-03-17 10:54 ` [PATCH 03/10] drm/amdgpu: fix adding eviction fence Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 05/10] drm/amdgpu: make amdgpu_user_wait_ioctl more resilent v2 Christian König
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
Lockdep was complaining about a number of issues here. Especially lock
inversion between syncobj, dma_resv and copying things into userspace.
Rework the functionality. Split it up into multiple functions,
consistenly use memdup_array_user(), fix the lock inversions and a few
more bugs in error handling.
v2: drop the dma_fence leak fix, turned out that was actually correct,
just not well documented. Apply some more cleanup suggestion from
Tvrtko.
v3: rebase on already done cleanups
v4: add missing dma_fence_put() in error path.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 582 ++++++++++--------
1 file changed, 323 insertions(+), 259 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 442c08b69f7c..0d9a13081f2f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -617,335 +617,399 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
return r;
}
-int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
- struct drm_file *filp)
+/* Count the number of expected fences so userspace can alloc a buffer */
+static int
+amdgpu_userq_wait_count_fences(struct drm_file *filp,
+ struct drm_amdgpu_userq_wait *wait_info,
+ u32 *syncobj_handles, u32 *timeline_points,
+ u32 *timeline_handles,
+ struct drm_gem_object **gobj_write,
+ struct drm_gem_object **gobj_read)
{
- struct drm_amdgpu_userq_wait *wait_info = data;
- const unsigned int num_write_bo_handles = wait_info->num_bo_write_handles;
- const unsigned int num_read_bo_handles = wait_info->num_bo_read_handles;
- struct drm_amdgpu_userq_fence_info *fence_info = NULL;
- struct amdgpu_fpriv *fpriv = filp->driver_priv;
- struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
- struct drm_gem_object **gobj_write, **gobj_read;
- u32 *timeline_points, *timeline_handles;
- struct amdgpu_usermode_queue *waitq = NULL;
- u32 *syncobj_handles, num_syncobj;
- struct dma_fence **fences = NULL;
- u16 num_points, num_fences = 0;
+ int num_read_bo_handles, num_write_bo_handles;
+ struct dma_fence_unwrap iter;
+ struct dma_fence *fence, *f;
+ unsigned int num_fences = 0;
struct drm_exec exec;
- int r, i, cnt;
-
- if (!amdgpu_userq_enabled(dev))
- return -ENOTSUPP;
-
- if (wait_info->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
- wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
- return -EINVAL;
-
- num_syncobj = wait_info->num_syncobj_handles;
- syncobj_handles = memdup_array_user(u64_to_user_ptr(wait_info->syncobj_handles),
- num_syncobj, sizeof(u32));
- if (IS_ERR(syncobj_handles))
- return PTR_ERR(syncobj_handles);
-
+ int i, r;
+
+ /*
+ * This needs to be outside of the lock provided by drm_exec for
+ * DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT to work correctly.
+ */
+
+ /* Count timeline fences */
+ for (i = 0; i < wait_info->num_syncobj_timeline_handles; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ return r;
+
+ dma_fence_unwrap_for_each(f, &iter, fence)
+ num_fences++;
- num_points = wait_info->num_syncobj_timeline_handles;
- timeline_handles = memdup_array_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
- num_points, sizeof(u32));
- if (IS_ERR(timeline_handles)) {
- r = PTR_ERR(timeline_handles);
- goto free_syncobj_handles;
+ dma_fence_put(fence);
}
- timeline_points = memdup_array_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
- num_points, sizeof(u32));
+ /* Count boolean fences */
+ for (i = 0; i < wait_info->num_syncobj_handles; i++) {
+ r = drm_syncobj_find_fence(filp, syncobj_handles[i], 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ return r;
- if (IS_ERR(timeline_points)) {
- r = PTR_ERR(timeline_points);
- goto free_timeline_handles;
+ num_fences++;
+ dma_fence_put(fence);
}
- r = drm_gem_objects_lookup(filp,
- u64_to_user_ptr(wait_info->bo_read_handles),
- num_read_bo_handles,
- &gobj_read);
- if (r)
- goto free_timeline_points;
-
- r = drm_gem_objects_lookup(filp,
- u64_to_user_ptr(wait_info->bo_write_handles),
- num_write_bo_handles,
- &gobj_write);
- if (r)
- goto put_gobj_read;
-
+ /* Lock all the GEM objects */
+ /* TODO: It is actually not necessary to lock them */
+ num_read_bo_handles = wait_info->num_bo_read_handles;
+ num_write_bo_handles = wait_info->num_bo_write_handles;
drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
- (num_read_bo_handles + num_write_bo_handles));
+ num_read_bo_handles + num_write_bo_handles);
- /* Lock all BOs with retry handling */
drm_exec_until_all_locked(&exec) {
- r = drm_exec_prepare_array(&exec, gobj_read, num_read_bo_handles, 1);
+ r = drm_exec_prepare_array(&exec, gobj_read,
+ num_read_bo_handles, 1);
drm_exec_retry_on_contention(&exec);
- if (r) {
- drm_exec_fini(&exec);
- goto put_gobj_write;
- }
+ if (r)
+ goto error_unlock;
- r = drm_exec_prepare_array(&exec, gobj_write, num_write_bo_handles, 1);
+ r = drm_exec_prepare_array(&exec, gobj_write,
+ num_write_bo_handles, 1);
drm_exec_retry_on_contention(&exec);
- if (r) {
- drm_exec_fini(&exec);
- goto put_gobj_write;
- }
+ if (r)
+ goto error_unlock;
}
- if (!wait_info->num_fences) {
- if (num_points) {
- struct dma_fence_unwrap iter;
- struct dma_fence *fence;
- struct dma_fence *f;
-
- for (i = 0; i < num_points; i++) {
- r = drm_syncobj_find_fence(filp, timeline_handles[i],
- timeline_points[i],
- DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
- &fence);
- if (r)
- goto exec_fini;
-
- dma_fence_unwrap_for_each(f, &iter, fence)
- num_fences++;
-
- dma_fence_put(fence);
- }
- }
+ /* Count read fences */
+ for (i = 0; i < num_read_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
- /* Count syncobj's fence */
- for (i = 0; i < num_syncobj; i++) {
- struct dma_fence *fence;
+ dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
+ DMA_RESV_USAGE_READ, fence)
+ num_fences++;
+ }
- r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0,
- DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
- &fence);
- if (r)
- goto exec_fini;
+ /* Count write fences */
+ for (i = 0; i < num_write_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+ dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
+ DMA_RESV_USAGE_WRITE, fence)
num_fences++;
- dma_fence_put(fence);
- }
+ }
- /* Count GEM objects fence */
- for (i = 0; i < num_read_bo_handles; i++) {
- struct dma_resv_iter resv_cursor;
- struct dma_fence *fence;
+ wait_info->num_fences = num_fences;
+ r = 0;
- dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
- DMA_RESV_USAGE_READ, fence)
- num_fences++;
- }
+error_unlock:
+ /* Unlock all the GEM objects */
+ drm_exec_fini(&exec);
+ return r;
+}
- for (i = 0; i < num_write_bo_handles; i++) {
- struct dma_resv_iter resv_cursor;
- struct dma_fence *fence;
+static int
+amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
+ struct drm_amdgpu_userq_wait *wait_info,
+ u32 *syncobj_handles, u32 *timeline_points,
+ u32 *timeline_handles,
+ struct drm_gem_object **gobj_write,
+ struct drm_gem_object **gobj_read)
+{
+ struct amdgpu_fpriv *fpriv = filp->driver_priv;
+ struct amdgpu_userq_mgr *userq_mgr = &fpriv->userq_mgr;
+ struct drm_amdgpu_userq_fence_info *fence_info;
+ int num_read_bo_handles, num_write_bo_handles;
+ struct amdgpu_usermode_queue *waitq;
+ struct dma_fence **fences, *fence, *f;
+ struct dma_fence_unwrap iter;
+ int num_points, num_syncobj;
+ unsigned int num_fences = 0;
+ struct drm_exec exec;
+ int i, cnt, r;
- dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
- DMA_RESV_USAGE_WRITE, fence)
- num_fences++;
- }
+ fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info),
+ GFP_KERNEL);
+ if (!fence_info)
+ return -ENOMEM;
- /*
- * Passing num_fences = 0 means that userspace doesn't want to
- * retrieve userq_fence_info. If num_fences = 0 we skip filling
- * userq_fence_info and return the actual number of fences on
- * args->num_fences.
- */
- wait_info->num_fences = num_fences;
- } else {
- /* Array of fence info */
- fence_info = kmalloc_array(wait_info->num_fences, sizeof(*fence_info), GFP_KERNEL);
- if (!fence_info) {
- r = -ENOMEM;
- goto exec_fini;
- }
+ fences = kmalloc_array(wait_info->num_fences, sizeof(*fences),
+ GFP_KERNEL);
+ if (!fences) {
+ r = -ENOMEM;
+ goto free_fence_info;
+ }
+
+ /* Retrieve timeline fences */
+ num_points = wait_info->num_syncobj_timeline_handles;
+ for (i = 0; i < num_points; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ goto free_fences;
+
+ dma_fence_unwrap_for_each(f, &iter, fence) {
+ if (num_fences >= wait_info->num_fences) {
+ r = -EINVAL;
+ dma_fence_put(fence);
+ goto free_fences;
+ }
- /* Array of fences */
- fences = kmalloc_array(wait_info->num_fences, sizeof(*fences), GFP_KERNEL);
- if (!fences) {
- r = -ENOMEM;
- goto free_fence_info;
+ fences[num_fences++] = dma_fence_get(f);
}
- /* Retrieve GEM read objects fence */
- for (i = 0; i < num_read_bo_handles; i++) {
- struct dma_resv_iter resv_cursor;
- struct dma_fence *fence;
+ dma_fence_put(fence);
+ }
- dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
- DMA_RESV_USAGE_READ, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
- goto free_fences;
- }
+ /* Retrieve boolean fences */
+ num_syncobj = wait_info->num_syncobj_handles;
+ for (i = 0; i < num_syncobj; i++) {
+ struct dma_fence *fence;
- fences[num_fences++] = fence;
- dma_fence_get(fence);
- }
+ r = drm_syncobj_find_fence(filp, syncobj_handles[i], 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r)
+ goto free_fences;
+
+ if (num_fences >= wait_info->num_fences) {
+ dma_fence_put(fence);
+ r = -EINVAL;
+ goto free_fences;
}
- /* Retrieve GEM write objects fence */
- for (i = 0; i < num_write_bo_handles; i++) {
- struct dma_resv_iter resv_cursor;
- struct dma_fence *fence;
+ /* Give the reference to the fence array */
+ fences[num_fences++] = fence;
+ }
+
+ /* Lock all the GEM objects */
+ num_read_bo_handles = wait_info->num_bo_read_handles;
+ num_write_bo_handles = wait_info->num_bo_write_handles;
+ drm_exec_init(&exec, DRM_EXEC_INTERRUPTIBLE_WAIT,
+ num_read_bo_handles + num_write_bo_handles);
- dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
- DMA_RESV_USAGE_WRITE, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
- goto free_fences;
- }
+ drm_exec_until_all_locked(&exec) {
+ r = drm_exec_prepare_array(&exec, gobj_read,
+ num_read_bo_handles, 1);
+ drm_exec_retry_on_contention(&exec);
+ if (r)
+ goto error_unlock;
- fences[num_fences++] = fence;
- dma_fence_get(fence);
- }
- }
+ r = drm_exec_prepare_array(&exec, gobj_write,
+ num_write_bo_handles, 1);
+ drm_exec_retry_on_contention(&exec);
+ if (r)
+ goto error_unlock;
+ }
- if (num_points) {
- struct dma_fence_unwrap iter;
- struct dma_fence *fence;
- struct dma_fence *f;
-
- for (i = 0; i < num_points; i++) {
- r = drm_syncobj_find_fence(filp, timeline_handles[i],
- timeline_points[i],
- DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
- &fence);
- if (r)
- goto free_fences;
-
- dma_fence_unwrap_for_each(f, &iter, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
- dma_fence_put(fence);
- goto free_fences;
- }
-
- dma_fence_get(f);
- fences[num_fences++] = f;
- }
+ /* Retrieve GEM read objects fence */
+ for (i = 0; i < num_read_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
- dma_fence_put(fence);
+ dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
+ DMA_RESV_USAGE_READ, fence) {
+ if (num_fences >= wait_info->num_fences) {
+ r = -EINVAL;
+ goto error_unlock;
}
- }
- /* Retrieve syncobj's fence */
- for (i = 0; i < num_syncobj; i++) {
- struct dma_fence *fence;
+ fences[num_fences++] = dma_fence_get(fence);
+ }
+ }
- r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0,
- DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
- &fence);
- if (r)
- goto free_fences;
+ /* Retrieve GEM write objects fence */
+ for (i = 0; i < num_write_bo_handles; i++) {
+ struct dma_resv_iter resv_cursor;
+ struct dma_fence *fence;
+ dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
+ DMA_RESV_USAGE_WRITE, fence) {
if (num_fences >= wait_info->num_fences) {
r = -EINVAL;
- dma_fence_put(fence);
- goto free_fences;
+ goto error_unlock;
}
- fences[num_fences++] = fence;
+ fences[num_fences++] = dma_fence_get(fence);
}
+ }
- /*
- * Keep only the latest fences to reduce the number of values
- * given back to userspace.
- */
- num_fences = dma_fence_dedup_array(fences, num_fences);
+ drm_exec_fini(&exec);
- waitq = amdgpu_userq_get(userq_mgr, wait_info->waitq_id);
- if (!waitq) {
- r = -EINVAL;
- goto free_fences;
- }
+ /*
+ * Keep only the latest fences to reduce the number of values
+ * given back to userspace.
+ */
+ num_fences = dma_fence_dedup_array(fences, num_fences);
- for (i = 0, cnt = 0; i < num_fences; i++) {
- struct amdgpu_userq_fence_driver *fence_drv;
- struct amdgpu_userq_fence *userq_fence;
- u32 index;
-
- userq_fence = to_amdgpu_userq_fence(fences[i]);
- if (!userq_fence) {
- /*
- * Just waiting on other driver fences should
- * be good for now
- */
- r = dma_fence_wait(fences[i], true);
- if (r)
- goto free_fences;
-
- continue;
- }
+ waitq = amdgpu_userq_get(userq_mgr, wait_info->waitq_id);
+ if (!waitq) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ for (i = 0, cnt = 0; i < num_fences; i++) {
+ struct amdgpu_userq_fence_driver *fence_drv;
+ struct amdgpu_userq_fence *userq_fence;
+ u32 index;
- fence_drv = userq_fence->fence_drv;
+ userq_fence = to_amdgpu_userq_fence(fences[i]);
+ if (!userq_fence) {
/*
- * We need to make sure the user queue release their reference
- * to the fence drivers at some point before queue destruction.
- * Otherwise, we would gather those references until we don't
- * have any more space left and crash.
+ * Just waiting on other driver fences should
+ * be good for now
*/
- r = xa_alloc(&waitq->fence_drv_xa, &index, fence_drv,
- xa_limit_32b, GFP_KERNEL);
+ r = dma_fence_wait(fences[i], true);
if (r)
- goto free_fences;
+ goto put_waitq;
- amdgpu_userq_fence_driver_get(fence_drv);
+ continue;
+ }
- /* Store drm syncobj's gpu va address and value */
- fence_info[cnt].va = fence_drv->va;
- fence_info[cnt].value = fences[i]->seqno;
+ fence_drv = userq_fence->fence_drv;
+ /*
+ * We need to make sure the user queue release their reference
+ * to the fence drivers at some point before queue destruction.
+ * Otherwise, we would gather those references until we don't
+ * have any more space left and crash.
+ */
+ r = xa_alloc(&waitq->fence_drv_xa, &index, fence_drv,
+ xa_limit_32b, GFP_KERNEL);
+ if (r)
+ goto put_waitq;
- /* Increment the actual userq fence count */
- cnt++;
- }
+ amdgpu_userq_fence_driver_get(fence_drv);
- wait_info->num_fences = cnt;
- /* Copy userq fence info to user space */
- if (copy_to_user(u64_to_user_ptr(wait_info->out_fences),
- fence_info, wait_info->num_fences * sizeof(*fence_info))) {
- r = -EFAULT;
- goto free_fences;
- }
+ /* Store drm syncobj's gpu va address and value */
+ fence_info[cnt].va = fence_drv->va;
+ fence_info[cnt].value = fences[i]->seqno;
+
+ /* Increment the actual userq fence count */
+ cnt++;
}
+ wait_info->num_fences = cnt;
+
+ /* Copy userq fence info to user space */
+ if (copy_to_user(u64_to_user_ptr(wait_info->out_fences),
+ fence_info, cnt * sizeof(*fence_info)))
+ r = -EFAULT;
+ else
+ r = 0;
+
+put_waitq:
+ amdgpu_userq_put(waitq);
free_fences:
- if (fences) {
- while (num_fences-- > 0)
- dma_fence_put(fences[num_fences]);
- kfree(fences);
- }
+ while (num_fences--)
+ dma_fence_put(fences[num_fences]);
+ kfree(fences);
+
free_fence_info:
kfree(fence_info);
-exec_fini:
+ return r;
+
+error_unlock:
drm_exec_fini(&exec);
-put_gobj_write:
- for (i = 0; i < num_write_bo_handles; i++)
- drm_gem_object_put(gobj_write[i]);
+ goto free_fences;
+}
+
+int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *filp)
+{
+ int num_points, num_syncobj, num_read_bo_handles, num_write_bo_handles;
+ u32 *syncobj_handles, *timeline_points, *timeline_handles;
+ struct drm_amdgpu_userq_wait *wait_info = data;
+ struct drm_gem_object **gobj_write;
+ struct drm_gem_object **gobj_read;
+ void __user *ptr;
+ int r;
+
+ if (!amdgpu_userq_enabled(dev))
+ return -ENOTSUPP;
+
+ if (wait_info->num_bo_write_handles > AMDGPU_USERQ_MAX_HANDLES ||
+ wait_info->num_bo_read_handles > AMDGPU_USERQ_MAX_HANDLES)
+ return -EINVAL;
+
+ num_syncobj = wait_info->num_syncobj_handles;
+ ptr = u64_to_user_ptr(wait_info->syncobj_handles);
+ syncobj_handles = memdup_array_user(ptr, num_syncobj, sizeof(u32));
+ if (IS_ERR(syncobj_handles))
+ return PTR_ERR(syncobj_handles);
+
+ num_points = wait_info->num_syncobj_timeline_handles;
+ ptr = u64_to_user_ptr(wait_info->syncobj_timeline_handles);
+ timeline_handles = memdup_array_user(ptr, num_points, sizeof(u32));
+ if (IS_ERR(timeline_handles)) {
+ r = PTR_ERR(timeline_handles);
+ goto free_syncobj_handles;
+ }
+
+ ptr = u64_to_user_ptr(wait_info->syncobj_timeline_points);
+ timeline_points = memdup_array_user(ptr, num_points, sizeof(u32));
+ if (IS_ERR(timeline_points)) {
+ r = PTR_ERR(timeline_points);
+ goto free_timeline_handles;
+ }
+
+ num_read_bo_handles = wait_info->num_bo_read_handles;
+ ptr = u64_to_user_ptr(wait_info->bo_read_handles),
+ r = drm_gem_objects_lookup(filp, ptr, num_read_bo_handles, &gobj_read);
+ if (r)
+ goto free_timeline_points;
+
+ num_write_bo_handles = wait_info->num_bo_write_handles;
+ ptr = u64_to_user_ptr(wait_info->bo_write_handles),
+ r = drm_gem_objects_lookup(filp, ptr, num_write_bo_handles,
+ &gobj_write);
+ if (r)
+ goto put_gobj_read;
+
+ /*
+ * Passing num_fences = 0 means that userspace doesn't want to
+ * retrieve userq_fence_info. If num_fences = 0 we skip filling
+ * userq_fence_info and return the actual number of fences on
+ * args->num_fences.
+ */
+ if (!wait_info->num_fences) {
+ r = amdgpu_userq_wait_count_fences(filp, wait_info,
+ syncobj_handles,
+ timeline_points,
+ timeline_handles,
+ gobj_write,
+ gobj_read);
+ } else {
+ r = amdgpu_userq_wait_return_fence_info(filp, wait_info,
+ syncobj_handles,
+ timeline_points,
+ timeline_handles,
+ gobj_write,
+ gobj_read);
+ }
+
+ while (num_write_bo_handles--)
+ drm_gem_object_put(gobj_write[num_write_bo_handles]);
kvfree(gobj_write);
+
put_gobj_read:
- for (i = 0; i < num_read_bo_handles; i++)
- drm_gem_object_put(gobj_read[i]);
+ while (num_read_bo_handles--)
+ drm_gem_object_put(gobj_read[num_read_bo_handles]);
kvfree(gobj_read);
+
free_timeline_points:
kfree(timeline_points);
free_timeline_handles:
kfree(timeline_handles);
free_syncobj_handles:
kfree(syncobj_handles);
-
- if (waitq)
- amdgpu_userq_put(waitq);
-
return r;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 05/10] drm/amdgpu: make amdgpu_user_wait_ioctl more resilent v2
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (2 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 04/10] drm/amdgpu: rework amdgpu_userq_wait_ioctl v4 Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 06/10] drm/amdgpu: annotate eviction fence signaling path Christian König
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
When the memory allocated by userspace isn't sufficient for all the
fences then just wait on them instead of returning an error.
v2: use correct variable as pointed out by Sunil
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 48 +++++++++++--------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 0d9a13081f2f..f93da45cfa7e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -706,7 +706,7 @@ amdgpu_userq_wait_count_fences(struct drm_file *filp,
num_fences++;
}
- wait_info->num_fences = num_fences;
+ wait_info->num_fences = min(num_fences, USHRT_MAX);
r = 0;
error_unlock:
@@ -715,6 +715,19 @@ amdgpu_userq_wait_count_fences(struct drm_file *filp,
return r;
}
+static int
+amdgpu_userq_wait_add_fence(struct drm_amdgpu_userq_wait *wait_info,
+ struct dma_fence **fences, unsigned int *num_fences,
+ struct dma_fence *fence)
+{
+ /* As fallback shouldn't userspace allocate enough space */
+ if (*num_fences >= wait_info->num_fences)
+ return dma_fence_wait(fence, true);
+
+ fences[(*num_fences)++] = dma_fence_get(fence);
+ return 0;
+}
+
static int
amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
struct drm_amdgpu_userq_wait *wait_info,
@@ -758,13 +771,12 @@ amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
goto free_fences;
dma_fence_unwrap_for_each(f, &iter, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
+ r = amdgpu_userq_wait_add_fence(wait_info, fences,
+ &num_fences, f);
+ if (r) {
dma_fence_put(fence);
goto free_fences;
}
-
- fences[num_fences++] = dma_fence_get(f);
}
dma_fence_put(fence);
@@ -781,14 +793,12 @@ amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
if (r)
goto free_fences;
- if (num_fences >= wait_info->num_fences) {
- dma_fence_put(fence);
- r = -EINVAL;
+ r = amdgpu_userq_wait_add_fence(wait_info, fences,
+ &num_fences, fence);
+ dma_fence_put(fence);
+ if (r)
goto free_fences;
- }
- /* Give the reference to the fence array */
- fences[num_fences++] = fence;
}
/* Lock all the GEM objects */
@@ -818,12 +828,10 @@ amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
dma_resv_for_each_fence(&resv_cursor, gobj_read[i]->resv,
DMA_RESV_USAGE_READ, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
+ r = amdgpu_userq_wait_add_fence(wait_info, fences,
+ &num_fences, fence);
+ if (r)
goto error_unlock;
- }
-
- fences[num_fences++] = dma_fence_get(fence);
}
}
@@ -834,12 +842,10 @@ amdgpu_userq_wait_return_fence_info(struct drm_file *filp,
dma_resv_for_each_fence(&resv_cursor, gobj_write[i]->resv,
DMA_RESV_USAGE_WRITE, fence) {
- if (num_fences >= wait_info->num_fences) {
- r = -EINVAL;
+ r = amdgpu_userq_wait_add_fence(wait_info, fences,
+ &num_fences, fence);
+ if (r)
goto error_unlock;
- }
-
- fences[num_fences++] = dma_fence_get(fence);
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 06/10] drm/amdgpu: annotate eviction fence signaling path
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (3 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 05/10] drm/amdgpu: make amdgpu_user_wait_ioctl more resilent v2 Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 07/10] drm/amdgpu: fix some more bug in amdgpu_gem_va_ioctl Christian König
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
Make sure lockdep sees the dependencies here.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
index ef7d07a134ce..641d03ef8608 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eviction_fence.c
@@ -64,8 +64,17 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
container_of(evf_mgr, struct amdgpu_fpriv, evf_mgr);
struct amdgpu_userq_mgr *uq_mgr = &fpriv->userq_mgr;
struct dma_fence *ev_fence;
+ bool cookie;
mutex_lock(&uq_mgr->userq_mutex);
+
+ /*
+ * This is intentionally after taking the userq_mutex since we do
+ * allocate memory while holding this lock, but only after ensuring that
+ * the eviction fence is signaled.
+ */
+ cookie = dma_fence_begin_signalling();
+
ev_fence = amdgpu_evf_mgr_get_fence(evf_mgr);
amdgpu_userq_evict(uq_mgr, !evf_mgr->shutdown);
@@ -75,6 +84,7 @@ amdgpu_eviction_fence_suspend_worker(struct work_struct *work)
* next fence.
*/
dma_fence_signal(ev_fence);
+ dma_fence_end_signalling(cookie);
dma_fence_put(ev_fence);
mutex_unlock(&uq_mgr->userq_mutex);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 07/10] drm/amdgpu: fix some more bug in amdgpu_gem_va_ioctl
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (4 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 06/10] drm/amdgpu: annotate eviction fence signaling path Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 08/10] drm/amdgpu: revert to old status lock handling v4 Christian König
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
Some illegal combination of input flags were not checked and we need to
take the PDEs into account when returning the fence as well.
Signed-off-by: Christian König <christian.koenig@amd.com>
Acked-by: Sunil Khatri <sunil.khatri@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 76 +++++++++++--------------
1 file changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 88a21400ae09..98276b55ad3c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -30,6 +30,7 @@
#include <linux/pagemap.h>
#include <linux/pci.h>
#include <linux/dma-buf.h>
+#include <linux/dma-fence-unwrap.h>
#include <drm/amdgpu_drm.h>
#include <drm/drm_drv.h>
@@ -741,11 +742,10 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
struct dma_fence *fence;
int r = 0;
- /* Always start from the VM's existing last update fence. */
- fence = dma_fence_get(vm->last_update);
-
+ /* If the VM is not ready return only a stub. */
if (!amdgpu_vm_ready(vm))
- return fence;
+ return dma_fence_get_stub();
+
/*
* First clean up any freed mappings in the VM.
@@ -754,7 +754,7 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
* schedules GPU work. If nothing needs clearing, @fence can remain as
* the original vm->last_update.
*/
- r = amdgpu_vm_clear_freed(adev, vm, &fence);
+ r = amdgpu_vm_clear_freed(adev, vm, &vm->last_update);
if (r)
goto error;
@@ -771,47 +771,34 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
if (r)
goto error;
- /*
- * Decide which fence best represents the last update:
- *
- * MAP/REPLACE:
- * - For always-valid mappings, use vm->last_update.
- * - Otherwise, export bo_va->last_pt_update.
- *
- * UNMAP/CLEAR:
- * Keep the fence returned by amdgpu_vm_clear_freed(). If no work was
- * needed, it can remain as vm->last_pt_update.
- *
- * The VM and BO update fences are always initialized to a valid value.
- * vm->last_update and bo_va->last_pt_update always start as valid fences.
- * and are never expected to be NULL.
- */
- switch (operation) {
- case AMDGPU_VA_OP_MAP:
- case AMDGPU_VA_OP_REPLACE:
+ if ((operation == AMDGPU_VA_OP_MAP ||
+ operation == AMDGPU_VA_OP_REPLACE) &&
+ !amdgpu_vm_is_bo_always_valid(vm, bo_va->base.bo)) {
+
/*
- * For MAP/REPLACE, return the page table update fence for the
- * mapping we just modified. bo_va is expected to be valid here.
+ * For MAP/REPLACE of non per-VM BOs we need to sync to both the
+ * bo_va->last_pt_update and vm->last_update or otherwise we
+ * potentially miss the PDE updates.
*/
- dma_fence_put(fence);
-
- if (amdgpu_vm_is_bo_always_valid(vm, bo_va->base.bo))
- fence = dma_fence_get(vm->last_update);
- else
- fence = dma_fence_get(bo_va->last_pt_update);
- break;
- case AMDGPU_VA_OP_UNMAP:
- case AMDGPU_VA_OP_CLEAR:
- default:
- /* keep @fence as returned by amdgpu_vm_clear_freed() */
- break;
+ fence = dma_fence_unwrap_merge(vm->last_update,
+ bo_va->last_pt_update);
+ if (!fence) {
+ /* As fallback in OOM situations */
+ dma_fence_wait(vm->last_update, false);
+ dma_fence_wait(bo_va->last_pt_update, false);
+ fence = dma_fence_get_stub();
+ }
+ } else {
+ fence = dma_fence_get(vm->last_update);
}
+ return fence;
+
error:
if (r && r != -ERESTARTSYS)
DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
- return fence;
+ return dma_fence_get(vm->last_update);
}
int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
@@ -832,7 +819,6 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
struct amdgpu_bo_va *bo_va;
struct drm_syncobj *timeline_syncobj = NULL;
struct dma_fence_chain *timeline_chain = NULL;
- struct dma_fence *fence;
struct drm_exec exec;
uint64_t vm_size;
int r = 0;
@@ -884,6 +870,10 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
return -EINVAL;
}
+ if (args->flags & AMDGPU_VM_DELAY_UPDATE &&
+ args->vm_timeline_syncobj_out)
+ return -EINVAL;
+
if ((args->operation != AMDGPU_VA_OP_CLEAR) &&
!(args->flags & AMDGPU_VM_PAGE_PRT)) {
gobj = drm_gem_object_lookup(filp, args->handle);
@@ -973,11 +963,13 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
* that represents the last relevant update for this mapping. This
* fence can then be exported to the user-visible VM timeline.
*/
- if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !adev->debug_vm) {
+ if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) &&
+ (!adev->debug_vm || timeline_syncobj)) {
+ struct dma_fence *fence;
+
fence = amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va,
args->operation);
-
- if (timeline_syncobj && fence) {
+ if (timeline_syncobj) {
if (!args->vm_timeline_point) {
/* Replace the existing fence when no point is given. */
drm_syncobj_replace_fence(timeline_syncobj,
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 08/10] drm/amdgpu: revert to old status lock handling v4
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (5 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 07/10] drm/amdgpu: fix some more bug in amdgpu_gem_va_ioctl Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 10:54 ` [PATCH 09/10] drm/amdgpu: restructure VM state machine v2 Christian König
2026-03-17 10:55 ` [PATCH 10/10] drm/amdgpu: WIP sync amdgpu_ttm_fill_mem only to kernel fences Christian König
8 siblings, 0 replies; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
It turned out that protecting the status of each bo_va with a
spinlock was just hiding problems instead of solving them.
Revert the whole approach, add a separate stats_lock and lockdep
assertions that the correct reservation lock is held all over the place.
This not only allows for better checks if a state transition is properly
protected by a lock, but also switching back to using list macros to
iterate over the state of lists protected by the dma_resv lock of the
root PD.
v2: re-add missing check
v3: split into two patches
v4: re-apply by fixing holding the VM lock at the right places.
Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 146 ++++++++--------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 15 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 4 -
4 files changed, 68 insertions(+), 105 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 2c1b8bbbc903..c50a484d22e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1041,12 +1041,12 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
struct amdgpu_bo *bo;
int ret;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
while (!list_empty(&vm->invalidated)) {
bo_va = list_first_entry(&vm->invalidated,
struct amdgpu_bo_va,
base.vm_status);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
bo = bo_va->base.bo;
ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 2);
@@ -1063,9 +1063,9 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
if (ret)
return ret;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
}
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
return 0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 01fef0e4f408..b89013a6aa0b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -153,12 +153,10 @@ static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
vm_bo->moved = true;
amdgpu_vm_assert_locked(vm);
- spin_lock(&vm_bo->vm->status_lock);
if (bo->tbo.type == ttm_bo_type_kernel)
list_move(&vm_bo->vm_status, &vm->evicted);
else
list_move_tail(&vm_bo->vm_status, &vm->evicted);
- spin_unlock(&vm_bo->vm->status_lock);
}
/**
* amdgpu_vm_bo_moved - vm_bo is moved
@@ -171,9 +169,7 @@ static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
{
amdgpu_vm_assert_locked(vm_bo->vm);
- spin_lock(&vm_bo->vm->status_lock);
list_move(&vm_bo->vm_status, &vm_bo->vm->moved);
- spin_unlock(&vm_bo->vm->status_lock);
}
/**
@@ -187,9 +183,7 @@ static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
{
amdgpu_vm_assert_locked(vm_bo->vm);
- spin_lock(&vm_bo->vm->status_lock);
list_move(&vm_bo->vm_status, &vm_bo->vm->idle);
- spin_unlock(&vm_bo->vm->status_lock);
vm_bo->moved = false;
}
@@ -203,9 +197,9 @@ static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
*/
static void amdgpu_vm_bo_invalidated(struct amdgpu_vm_bo_base *vm_bo)
{
- spin_lock(&vm_bo->vm->status_lock);
+ spin_lock(&vm_bo->vm->invalidated_lock);
list_move(&vm_bo->vm_status, &vm_bo->vm->invalidated);
- spin_unlock(&vm_bo->vm->status_lock);
+ spin_unlock(&vm_bo->vm->invalidated_lock);
}
/**
@@ -218,10 +212,9 @@ static void amdgpu_vm_bo_invalidated(struct amdgpu_vm_bo_base *vm_bo)
*/
static void amdgpu_vm_bo_evicted_user(struct amdgpu_vm_bo_base *vm_bo)
{
+ amdgpu_vm_assert_locked(vm_bo->vm);
vm_bo->moved = true;
- spin_lock(&vm_bo->vm->status_lock);
list_move(&vm_bo->vm_status, &vm_bo->vm->evicted_user);
- spin_unlock(&vm_bo->vm->status_lock);
}
/**
@@ -235,13 +228,10 @@ static void amdgpu_vm_bo_evicted_user(struct amdgpu_vm_bo_base *vm_bo)
static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base *vm_bo)
{
amdgpu_vm_assert_locked(vm_bo->vm);
- if (vm_bo->bo->parent) {
- spin_lock(&vm_bo->vm->status_lock);
+ if (vm_bo->bo->parent)
list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
- spin_unlock(&vm_bo->vm->status_lock);
- } else {
+ else
amdgpu_vm_bo_idle(vm_bo);
- }
}
/**
@@ -255,9 +245,7 @@ static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base *vm_bo)
static void amdgpu_vm_bo_done(struct amdgpu_vm_bo_base *vm_bo)
{
amdgpu_vm_assert_locked(vm_bo->vm);
- spin_lock(&vm_bo->vm->status_lock);
list_move(&vm_bo->vm_status, &vm_bo->vm->done);
- spin_unlock(&vm_bo->vm->status_lock);
}
/**
@@ -271,13 +259,13 @@ static void amdgpu_vm_bo_reset_state_machine(struct amdgpu_vm *vm)
{
struct amdgpu_vm_bo_base *vm_bo, *tmp;
- amdgpu_vm_assert_locked(vm);
-
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
list_splice_init(&vm->done, &vm->invalidated);
list_for_each_entry(vm_bo, &vm->invalidated, vm_status)
vm_bo->moved = true;
+ spin_unlock(&vm->invalidated_lock);
+ amdgpu_vm_assert_locked(vm);
list_for_each_entry_safe(vm_bo, tmp, &vm->idle, vm_status) {
struct amdgpu_bo *bo = vm_bo->bo;
@@ -287,14 +275,13 @@ static void amdgpu_vm_bo_reset_state_machine(struct amdgpu_vm *vm)
else if (bo->parent)
list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
}
- spin_unlock(&vm->status_lock);
}
/**
* amdgpu_vm_update_shared - helper to update shared memory stat
* @base: base structure for tracking BO usage in a VM
*
- * Takes the vm status_lock and updates the shared memory stat. If the basic
+ * Takes the vm stats_lock and updates the shared memory stat. If the basic
* stat changed (e.g. buffer was moved) amdgpu_vm_update_stats need to be called
* as well.
*/
@@ -307,7 +294,7 @@ static void amdgpu_vm_update_shared(struct amdgpu_vm_bo_base *base)
bool shared;
dma_resv_assert_held(bo->tbo.base.resv);
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->stats_lock);
shared = drm_gem_object_is_shared_for_memory_stats(&bo->tbo.base);
if (base->shared != shared) {
base->shared = shared;
@@ -319,7 +306,7 @@ static void amdgpu_vm_update_shared(struct amdgpu_vm_bo_base *base)
vm->stats[bo_memtype].drm.private += size;
}
}
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->stats_lock);
}
/**
@@ -344,11 +331,11 @@ void amdgpu_vm_bo_update_shared(struct amdgpu_bo *bo)
* be bo->tbo.resource
* @sign: if we should add (+1) or subtract (-1) from the stat
*
- * Caller need to have the vm status_lock held. Useful for when multiple update
+ * Caller need to have the vm stats_lock held. Useful for when multiple update
* need to happen at the same time.
*/
static void amdgpu_vm_update_stats_locked(struct amdgpu_vm_bo_base *base,
- struct ttm_resource *res, int sign)
+ struct ttm_resource *res, int sign)
{
struct amdgpu_vm *vm = base->vm;
struct amdgpu_bo *bo = base->bo;
@@ -372,7 +359,8 @@ static void amdgpu_vm_update_stats_locked(struct amdgpu_vm_bo_base *base,
*/
if (bo->flags & AMDGPU_GEM_CREATE_DISCARDABLE)
vm->stats[res_memtype].drm.purgeable += size;
- if (!(bo->preferred_domains & amdgpu_mem_type_to_domain(res_memtype)))
+ if (!(bo->preferred_domains &
+ amdgpu_mem_type_to_domain(res_memtype)))
vm->stats[bo_memtype].evicted += size;
}
}
@@ -391,9 +379,9 @@ void amdgpu_vm_update_stats(struct amdgpu_vm_bo_base *base,
{
struct amdgpu_vm *vm = base->vm;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->stats_lock);
amdgpu_vm_update_stats_locked(base, res, sign);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->stats_lock);
}
/**
@@ -419,10 +407,10 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
base->next = bo->vm_bo;
bo->vm_bo = base;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->stats_lock);
base->shared = drm_gem_object_is_shared_for_memory_stats(&bo->tbo.base);
amdgpu_vm_update_stats_locked(base, bo->tbo.resource, +1);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->stats_lock);
if (!amdgpu_vm_is_bo_always_valid(vm, bo))
return;
@@ -481,25 +469,25 @@ int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
int ret;
/* We can only trust prev->next while holding the lock */
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
while (!list_is_head(prev->next, &vm->done)) {
bo_va = list_entry(prev->next, typeof(*bo_va), base.vm_status);
bo = bo_va->base.bo;
if (bo) {
amdgpu_bo_ref(bo);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 1);
amdgpu_bo_unref(&bo);
if (unlikely(ret))
return ret;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
}
prev = prev->next;
}
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
return 0;
}
@@ -595,7 +583,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
void *param)
{
uint64_t new_vm_generation = amdgpu_vm_generation(adev, vm);
- struct amdgpu_vm_bo_base *bo_base;
+ struct amdgpu_vm_bo_base *bo_base, *tmp;
struct amdgpu_bo *bo;
int r;
@@ -608,13 +596,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
return r;
}
- spin_lock(&vm->status_lock);
- while (!list_empty(&vm->evicted)) {
- bo_base = list_first_entry(&vm->evicted,
- struct amdgpu_vm_bo_base,
- vm_status);
- spin_unlock(&vm->status_lock);
-
+ list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
bo = bo_base->bo;
r = validate(param, bo);
@@ -627,26 +609,21 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
vm->update_funcs->map_table(to_amdgpu_bo_vm(bo));
amdgpu_vm_bo_relocated(bo_base);
}
- spin_lock(&vm->status_lock);
}
- while (ticket && !list_empty(&vm->evicted_user)) {
- bo_base = list_first_entry(&vm->evicted_user,
- struct amdgpu_vm_bo_base,
- vm_status);
- spin_unlock(&vm->status_lock);
- bo = bo_base->bo;
- dma_resv_assert_held(bo->tbo.base.resv);
+ if (ticket) {
+ list_for_each_entry_safe(bo_base, tmp, &vm->evicted_user,
+ vm_status) {
+ bo = bo_base->bo;
+ dma_resv_assert_held(bo->tbo.base.resv);
- r = validate(param, bo);
- if (r)
- return r;
-
- amdgpu_vm_bo_invalidated(bo_base);
+ r = validate(param, bo);
+ if (r)
+ return r;
- spin_lock(&vm->status_lock);
+ amdgpu_vm_bo_invalidated(bo_base);
+ }
}
- spin_unlock(&vm->status_lock);
amdgpu_vm_eviction_lock(vm);
vm->evicting = false;
@@ -675,9 +652,7 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
ret = !vm->evicting;
amdgpu_vm_eviction_unlock(vm);
- spin_lock(&vm->status_lock);
ret &= list_empty(&vm->evicted);
- spin_unlock(&vm->status_lock);
spin_lock(&vm->immediate.lock);
ret &= !vm->immediate.stopped;
@@ -971,18 +946,13 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
struct amdgpu_vm *vm, bool immediate)
{
struct amdgpu_vm_update_params params;
- struct amdgpu_vm_bo_base *entry;
+ struct amdgpu_vm_bo_base *entry, *tmp;
bool flush_tlb_needed = false;
- LIST_HEAD(relocated);
int r, idx;
amdgpu_vm_assert_locked(vm);
- spin_lock(&vm->status_lock);
- list_splice_init(&vm->relocated, &relocated);
- spin_unlock(&vm->status_lock);
-
- if (list_empty(&relocated))
+ if (list_empty(&vm->relocated))
return 0;
if (!drm_dev_enter(adev_to_drm(adev), &idx))
@@ -998,7 +968,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (r)
goto error;
- list_for_each_entry(entry, &relocated, vm_status) {
+ list_for_each_entry(entry, &vm->relocated, vm_status) {
/* vm_flush_needed after updating moved PDEs */
flush_tlb_needed |= entry->moved;
@@ -1014,9 +984,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (flush_tlb_needed)
atomic64_inc(&vm->tlb_seq);
- while (!list_empty(&relocated)) {
- entry = list_first_entry(&relocated, struct amdgpu_vm_bo_base,
- vm_status);
+ list_for_each_entry_safe(entry, tmp, &vm->relocated, vm_status) {
amdgpu_vm_bo_idle(entry);
}
@@ -1243,9 +1211,9 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM])
{
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->stats_lock);
memcpy(stats, vm->stats, sizeof(*stats) * __AMDGPU_PL_NUM);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->stats_lock);
}
/**
@@ -1612,29 +1580,24 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
struct ww_acquire_ctx *ticket)
{
- struct amdgpu_bo_va *bo_va;
+ struct amdgpu_bo_va *bo_va, *tmp;
struct dma_resv *resv;
bool clear, unlock;
int r;
- spin_lock(&vm->status_lock);
- while (!list_empty(&vm->moved)) {
- bo_va = list_first_entry(&vm->moved, struct amdgpu_bo_va,
- base.vm_status);
- spin_unlock(&vm->status_lock);
-
+ list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
/* Per VM BOs never need to bo cleared in the page tables */
r = amdgpu_vm_bo_update(adev, bo_va, false);
if (r)
return r;
- spin_lock(&vm->status_lock);
}
+ spin_lock(&vm->invalidated_lock);
while (!list_empty(&vm->invalidated)) {
bo_va = list_first_entry(&vm->invalidated, struct amdgpu_bo_va,
base.vm_status);
resv = bo_va->base.bo->tbo.base.resv;
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
/* Try to reserve the BO to avoid clearing its ptes */
if (!adev->debug_vm && dma_resv_trylock(resv)) {
@@ -1666,9 +1629,9 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
bo_va->base.bo->tbo.resource->mem_type == TTM_PL_SYSTEM))
amdgpu_vm_bo_evicted_user(&bo_va->base);
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
}
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
return 0;
}
@@ -2211,9 +2174,9 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
}
}
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->invalidated_lock);
list_del(&bo_va->base.vm_status);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->invalidated_lock);
list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
list_del(&mapping->list);
@@ -2321,10 +2284,10 @@ void amdgpu_vm_bo_move(struct amdgpu_bo *bo, struct ttm_resource *new_mem,
for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
struct amdgpu_vm *vm = bo_base->vm;
- spin_lock(&vm->status_lock);
+ spin_lock(&vm->stats_lock);
amdgpu_vm_update_stats_locked(bo_base, bo->tbo.resource, -1);
amdgpu_vm_update_stats_locked(bo_base, new_mem, +1);
- spin_unlock(&vm->status_lock);
+ spin_unlock(&vm->stats_lock);
}
amdgpu_vm_bo_invalidate(bo, evicted);
@@ -2596,11 +2559,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
INIT_LIST_HEAD(&vm->relocated);
INIT_LIST_HEAD(&vm->moved);
INIT_LIST_HEAD(&vm->idle);
+ spin_lock_init(&vm->invalidated_lock);
INIT_LIST_HEAD(&vm->invalidated);
- spin_lock_init(&vm->status_lock);
INIT_LIST_HEAD(&vm->freed);
INIT_LIST_HEAD(&vm->done);
INIT_KFIFO(vm->faults);
+ spin_lock_init(&vm->stats_lock);
r = amdgpu_vm_init_entities(adev, vm);
if (r)
@@ -3068,7 +3032,6 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
amdgpu_vm_assert_locked(vm);
- spin_lock(&vm->status_lock);
seq_puts(m, "\tIdle BOs:\n");
list_for_each_entry_safe(bo_va, tmp, &vm->idle, base.vm_status) {
if (!bo_va->base.bo)
@@ -3106,11 +3069,13 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
id = 0;
seq_puts(m, "\tInvalidated BOs:\n");
+ spin_lock(&vm->invalidated_lock);
list_for_each_entry_safe(bo_va, tmp, &vm->invalidated, base.vm_status) {
if (!bo_va->base.bo)
continue;
total_invalidated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
}
+ spin_unlock(&vm->invalidated_lock);
total_invalidated_objs = id;
id = 0;
@@ -3120,7 +3085,6 @@ void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
continue;
total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
}
- spin_unlock(&vm->status_lock);
total_done_objs = id;
seq_printf(m, "\tTotal idle size: %12lld\tobjs:\t%d\n", total_idle,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index dc4b0ec672ec..ae9449d5b00c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -205,11 +205,11 @@ struct amdgpu_vm_bo_base {
/* protected by bo being reserved */
struct amdgpu_vm_bo_base *next;
- /* protected by vm status_lock */
+ /* protected by vm reservation and invalidated_lock */
struct list_head vm_status;
/* if the bo is counted as shared in mem stats
- * protected by vm status_lock */
+ * protected by vm BO being reserved */
bool shared;
/* protected by the BO being reserved */
@@ -345,10 +345,8 @@ struct amdgpu_vm {
bool evicting;
unsigned int saved_flags;
- /* Lock to protect vm_bo add/del/move on all lists of vm */
- spinlock_t status_lock;
-
- /* Memory statistics for this vm, protected by status_lock */
+ /* Memory statistics for this vm, protected by stats_lock */
+ spinlock_t stats_lock;
struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
/*
@@ -356,6 +354,8 @@ struct amdgpu_vm {
* PDs, PTs or per VM BOs. The state transits are:
*
* evicted -> relocated (PDs, PTs) or moved (per VM BOs) -> idle
+ *
+ * Lists are protected by the root PD dma_resv lock.
*/
/* Per-VM and PT BOs who needs a validation */
@@ -376,7 +376,10 @@ struct amdgpu_vm {
* state transits are:
*
* evicted_user or invalidated -> done
+ *
+ * Lists are protected by the invalidated_lock.
*/
+ spinlock_t invalidated_lock;
/* BOs for user mode queues that need a validation */
struct list_head evicted_user;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 31a437ce9570..7bdd664f0770 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -544,9 +544,7 @@ static void amdgpu_vm_pt_free(struct amdgpu_vm_bo_base *entry)
entry->bo->vm_bo = NULL;
ttm_bo_set_bulk_move(&entry->bo->tbo, NULL);
- spin_lock(&entry->vm->status_lock);
list_del(&entry->vm_status);
- spin_unlock(&entry->vm->status_lock);
amdgpu_bo_unref(&entry->bo);
}
@@ -590,7 +588,6 @@ static void amdgpu_vm_pt_add_list(struct amdgpu_vm_update_params *params,
struct amdgpu_vm_pt_cursor seek;
struct amdgpu_vm_bo_base *entry;
- spin_lock(¶ms->vm->status_lock);
for_each_amdgpu_vm_pt_dfs_safe(params->adev, params->vm, cursor, seek, entry) {
if (entry && entry->bo)
list_move(&entry->vm_status, ¶ms->tlb_flush_waitlist);
@@ -598,7 +595,6 @@ static void amdgpu_vm_pt_add_list(struct amdgpu_vm_update_params *params,
/* enter start node now */
list_move(&cursor->entry->vm_status, ¶ms->tlb_flush_waitlist);
- spin_unlock(¶ms->vm->status_lock);
}
/**
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 09/10] drm/amdgpu: restructure VM state machine v2
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (6 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 08/10] drm/amdgpu: revert to old status lock handling v4 Christian König
@ 2026-03-17 10:54 ` Christian König
2026-03-17 12:03 ` Khatri, Sunil
2026-03-17 10:55 ` [PATCH 10/10] drm/amdgpu: WIP sync amdgpu_ttm_fill_mem only to kernel fences Christian König
8 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2026-03-17 10:54 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
Instead of coming up with more sophisticated names for states a VM BO
can be in, group them by the type of BO first and then by the state.
So we end with BO type kernel, always_valid and individual and then states
evicted, moved and idle.
Not much functional change, except that evicted_user is moved back
together with the other BOs again which makes the handling in
amdgpu_vm_validate() a bit more complex.
Also fixes a problem with user queues and amdgpu_vm_ready(). We didn't
considered the VM ready when user BOs were not ideally placed, harmless
performance impact for kernel queues but a complete show stopper for
userqueues.
v2: fix a few typos in comments, rename the BO types to make them more
descriptive, fix a couple of bugs found during testing
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 22 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 466 ++++++++++------------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 64 ++-
3 files changed, 251 insertions(+), 301 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index c50a484d22e0..9b2dfeead299 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1041,12 +1041,12 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
struct amdgpu_bo *bo;
int ret;
- spin_lock(&vm->invalidated_lock);
- while (!list_empty(&vm->invalidated)) {
- bo_va = list_first_entry(&vm->invalidated,
+ spin_lock(&vm->individual_lock);
+ while (!list_empty(&vm->always_valid.evicted)) {
+ bo_va = list_first_entry(&vm->always_valid.evicted,
struct amdgpu_bo_va,
base.vm_status);
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
bo = bo_va->base.bo;
ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 2);
@@ -1058,14 +1058,14 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
if (ret)
return ret;
- /* This moves the bo_va to the done list */
+ /* This moves the bo_va to the idle list */
ret = amdgpu_vm_bo_update(adev, bo_va, false);
if (ret)
return ret;
- spin_lock(&vm->invalidated_lock);
+ spin_lock(&vm->individual_lock);
}
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
return 0;
}
@@ -1097,7 +1097,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
if (unlikely(ret))
goto unlock_all;
- ret = amdgpu_vm_lock_done_list(vm, &exec, 1);
+ ret = amdgpu_vm_lock_individual(vm, &exec, 1);
drm_exec_retry_on_contention(&exec);
if (unlikely(ret))
goto unlock_all;
@@ -1140,7 +1140,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
key = 0;
/* Validate User Ptr BOs */
- list_for_each_entry(bo_va, &vm->done, base.vm_status) {
+ list_for_each_entry(bo_va, &vm->always_valid.idle, base.vm_status) {
bo = bo_va->base.bo;
if (!bo)
continue;
@@ -1190,10 +1190,10 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
/*
* We need to wait for all VM updates to finish before restarting the
- * queues. Using the done list like that is now ok since everything is
+ * queues. Using the idle list like that is now ok since everything is
* locked in place.
*/
- list_for_each_entry(bo_va, &vm->done, base.vm_status)
+ list_for_each_entry(bo_va, &vm->always_valid.idle, base.vm_status)
dma_fence_wait(bo_va->last_pt_update, false);
dma_fence_wait(vm->last_update, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b89013a6aa0b..1223d1e86afa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -138,143 +138,127 @@ static void amdgpu_vm_assert_locked(struct amdgpu_vm *vm)
dma_resv_assert_held(vm->root.bo->tbo.base.resv);
}
-/**
- * amdgpu_vm_bo_evicted - vm_bo is evicted
- *
- * @vm_bo: vm_bo which is evicted
- *
- * State for PDs/PTs and per VM BOs which are not at the location they should
- * be.
+/* Initialize the amdgpu_vm_bo_status object */
+static void amdgpu_vm_bo_status_init(struct amdgpu_vm_bo_status *lists)
+{
+ INIT_LIST_HEAD(&lists->evicted);
+ INIT_LIST_HEAD(&lists->moved);
+ INIT_LIST_HEAD(&lists->idle);
+}
+
+/*
+ * Make sure we have the lock to modify the vm_bo status and return the object
+ * with the status lists.
*/
-static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
+static struct amdgpu_vm_bo_status *
+amdgpu_vm_bo_lock_lists(struct amdgpu_vm_bo_base *vm_bo)
{
struct amdgpu_vm *vm = vm_bo->vm;
struct amdgpu_bo *bo = vm_bo->bo;
- vm_bo->moved = true;
- amdgpu_vm_assert_locked(vm);
- if (bo->tbo.type == ttm_bo_type_kernel)
- list_move(&vm_bo->vm_status, &vm->evicted);
- else
- list_move_tail(&vm_bo->vm_status, &vm->evicted);
-}
-/**
- * amdgpu_vm_bo_moved - vm_bo is moved
- *
- * @vm_bo: vm_bo which is moved
- *
- * State for per VM BOs which are moved, but that change is not yet reflected
- * in the page tables.
- */
-static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
-{
- amdgpu_vm_assert_locked(vm_bo->vm);
- list_move(&vm_bo->vm_status, &vm_bo->vm->moved);
-}
+ if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
+ /* No extra locking needed, protected by the root PD resv lock */
+ amdgpu_vm_assert_locked(vm);
-/**
- * amdgpu_vm_bo_idle - vm_bo is idle
- *
- * @vm_bo: vm_bo which is now idle
- *
- * State for PDs/PTs and per VM BOs which have gone through the state machine
- * and are now idle.
- */
-static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
-{
- amdgpu_vm_assert_locked(vm_bo->vm);
- list_move(&vm_bo->vm_status, &vm_bo->vm->idle);
- vm_bo->moved = false;
+ if (bo->tbo.type == ttm_bo_type_kernel)
+ return &vm->kernel;
+
+ return &vm->always_valid;
+ }
+
+ spin_lock(&vm_bo->vm->individual_lock);
+ return &vm->individual;
}
-/**
- * amdgpu_vm_bo_invalidated - vm_bo is invalidated
- *
- * @vm_bo: vm_bo which is now invalidated
- *
- * State for normal BOs which are invalidated and that change not yet reflected
- * in the PTs.
- */
-static void amdgpu_vm_bo_invalidated(struct amdgpu_vm_bo_base *vm_bo)
+/* Eventually unlock the status list lock again */
+static void amdgpu_vm_bo_unlock_lists(struct amdgpu_vm_bo_base *vm_bo)
{
- spin_lock(&vm_bo->vm->invalidated_lock);
- list_move(&vm_bo->vm_status, &vm_bo->vm->invalidated);
- spin_unlock(&vm_bo->vm->invalidated_lock);
+ if (amdgpu_vm_is_bo_always_valid(vm_bo->vm, vm_bo->bo))
+ amdgpu_vm_assert_locked(vm_bo->vm);
+ else
+ spin_unlock(&vm_bo->vm->individual_lock);
}
/**
- * amdgpu_vm_bo_evicted_user - vm_bo is evicted
+ * amdgpu_vm_bo_evicted - vm_bo is evicted
*
* @vm_bo: vm_bo which is evicted
*
- * State for BOs used by user mode queues which are not at the location they
- * should be.
+ * State for vm_bo objects meaning the underlying BO was evicted and need to
+ * move in place again.
*/
-static void amdgpu_vm_bo_evicted_user(struct amdgpu_vm_bo_base *vm_bo)
+static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
{
- amdgpu_vm_assert_locked(vm_bo->vm);
+ struct amdgpu_vm_bo_status *lists;
+
+ lists = amdgpu_vm_bo_lock_lists(vm_bo);
vm_bo->moved = true;
- list_move(&vm_bo->vm_status, &vm_bo->vm->evicted_user);
+ list_move(&vm_bo->vm_status, &lists->evicted);
+ amdgpu_vm_bo_unlock_lists(vm_bo);
}
-
/**
- * amdgpu_vm_bo_relocated - vm_bo is reloacted
+ * amdgpu_vm_bo_moved - vm_bo is moved
*
- * @vm_bo: vm_bo which is relocated
+ * @vm_bo: vm_bo which is moved
*
- * State for PDs/PTs which needs to update their parent PD.
- * For the root PD, just move to idle state.
+ * State for vm_bo objects meaning the underlying BO was moved but the new
+ * location not yet reflected in the page tables.
*/
-static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base *vm_bo)
+static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
{
- amdgpu_vm_assert_locked(vm_bo->vm);
- if (vm_bo->bo->parent)
- list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
- else
- amdgpu_vm_bo_idle(vm_bo);
+ struct amdgpu_vm_bo_status *lists;
+ struct amdgpu_bo *bo = vm_bo->bo;
+
+ /*
+ * The root PD doesn't have a parent PDE and goes directly into the
+ * idle state.
+ */
+ lists = amdgpu_vm_bo_lock_lists(vm_bo);
+ if (bo && bo->tbo.type == ttm_bo_type_kernel && !bo->parent) {
+ vm_bo->moved = false;
+ list_move(&vm_bo->vm_status, &lists->idle);
+ } else {
+ vm_bo->moved = true;
+ list_move(&vm_bo->vm_status, &lists->moved);
+ }
+ amdgpu_vm_bo_unlock_lists(vm_bo);
}
/**
- * amdgpu_vm_bo_done - vm_bo is done
+ * amdgpu_vm_bo_idle - vm_bo is idle
*
- * @vm_bo: vm_bo which is now done
+ * @vm_bo: vm_bo which is now idle
*
- * State for normal BOs which are invalidated and that change has been updated
- * in the PTs.
+ * State for vm_bo objects meaning we are done with the state machine and no
+ * further action is necessary.
*/
-static void amdgpu_vm_bo_done(struct amdgpu_vm_bo_base *vm_bo)
+static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
{
- amdgpu_vm_assert_locked(vm_bo->vm);
- list_move(&vm_bo->vm_status, &vm_bo->vm->done);
+ struct amdgpu_vm_bo_status *lists;
+
+ lists = amdgpu_vm_bo_lock_lists(vm_bo);
+ if (!amdgpu_vm_is_bo_always_valid(vm_bo->vm, vm_bo->bo))
+ vm_bo->moved = false;
+ list_move(&vm_bo->vm_status, &lists->idle);
+ amdgpu_vm_bo_unlock_lists(vm_bo);
}
/**
* amdgpu_vm_bo_reset_state_machine - reset the vm_bo state machine
* @vm: the VM which state machine to reset
*
- * Move all vm_bo object in the VM into a state where they will be updated
- * again during validation.
+ * Move all vm_bo object in the VM into a state where their location will be
+ * updated in the page tables again.
*/
static void amdgpu_vm_bo_reset_state_machine(struct amdgpu_vm *vm)
{
- struct amdgpu_vm_bo_base *vm_bo, *tmp;
-
- spin_lock(&vm->invalidated_lock);
- list_splice_init(&vm->done, &vm->invalidated);
- list_for_each_entry(vm_bo, &vm->invalidated, vm_status)
- vm_bo->moved = true;
- spin_unlock(&vm->invalidated_lock);
-
amdgpu_vm_assert_locked(vm);
- list_for_each_entry_safe(vm_bo, tmp, &vm->idle, vm_status) {
- struct amdgpu_bo *bo = vm_bo->bo;
+ list_splice_init(&vm->kernel.idle, &vm->kernel.moved);
+ list_splice_init(&vm->always_valid.idle, &vm->always_valid.moved);
- vm_bo->moved = true;
- if (!bo || bo->tbo.type != ttm_bo_type_kernel)
- list_move(&vm_bo->vm_status, &vm_bo->vm->moved);
- else if (bo->parent)
- list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
- }
+ spin_lock(&vm->individual_lock);
+ list_splice_init(&vm->individual.idle, &vm->individual.moved);
+ spin_unlock(&vm->individual_lock);
}
/**
@@ -402,8 +386,10 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
base->next = NULL;
INIT_LIST_HEAD(&base->vm_status);
+ dma_resv_assert_held(vm->root.bo->tbo.base.resv);
if (!bo)
return;
+
base->next = bo->vm_bo;
bo->vm_bo = base;
@@ -412,27 +398,22 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
amdgpu_vm_update_stats_locked(base, bo->tbo.resource, +1);
spin_unlock(&vm->stats_lock);
- if (!amdgpu_vm_is_bo_always_valid(vm, bo))
+ if (!amdgpu_vm_is_bo_always_valid(vm, bo)) {
+ amdgpu_vm_bo_idle(base);
return;
-
- dma_resv_assert_held(vm->root.bo->tbo.base.resv);
+ }
ttm_bo_set_bulk_move(&bo->tbo, &vm->lru_bulk_move);
- if (bo->tbo.type == ttm_bo_type_kernel && bo->parent)
- amdgpu_vm_bo_relocated(base);
- else
- amdgpu_vm_bo_idle(base);
+ /*
+ * When a per VM isn't in the desired domain put it into the evicted
+ * state to make sure that it gets validated on the next best occasion.
+ */
if (bo->preferred_domains &
amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type))
- return;
-
- /*
- * we checked all the prerequisites, but it looks like this per vm bo
- * is currently evicted. add the bo to the evicted list to make sure it
- * is validated on next vm use to avoid fault.
- * */
- amdgpu_vm_bo_evicted(base);
+ amdgpu_vm_bo_moved(base);
+ else
+ amdgpu_vm_bo_evicted(base);
}
/**
@@ -453,41 +434,41 @@ int amdgpu_vm_lock_pd(struct amdgpu_vm *vm, struct drm_exec *exec,
}
/**
- * amdgpu_vm_lock_done_list - lock all BOs on the done list
+ * amdgpu_vm_lock_individual - lock all BOs on the individual idle list
* @vm: vm providing the BOs
* @exec: drm execution context
* @num_fences: number of extra fences to reserve
*
- * Lock the BOs on the done list in the DRM execution context.
+ * Lock the BOs on the individual idle list in the DRM execution context.
*/
-int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
- unsigned int num_fences)
+int amdgpu_vm_lock_individual(struct amdgpu_vm *vm, struct drm_exec *exec,
+ unsigned int num_fences)
{
- struct list_head *prev = &vm->done;
+ struct list_head *prev = &vm->individual.idle;
struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *bo;
int ret;
/* We can only trust prev->next while holding the lock */
- spin_lock(&vm->invalidated_lock);
- while (!list_is_head(prev->next, &vm->done)) {
+ spin_lock(&vm->individual_lock);
+ while (!list_is_head(prev->next, &vm->individual.idle)) {
bo_va = list_entry(prev->next, typeof(*bo_va), base.vm_status);
bo = bo_va->base.bo;
if (bo) {
amdgpu_bo_ref(bo);
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 1);
amdgpu_bo_unref(&bo);
if (unlikely(ret))
return ret;
- spin_lock(&vm->invalidated_lock);
+ spin_lock(&vm->individual_lock);
}
prev = prev->next;
}
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
return 0;
}
@@ -584,9 +565,9 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
{
uint64_t new_vm_generation = amdgpu_vm_generation(adev, vm);
struct amdgpu_vm_bo_base *bo_base, *tmp;
- struct amdgpu_bo *bo;
int r;
+ dma_resv_assert_held(vm->root.bo->tbo.base.resv);
if (vm->generation != new_vm_generation) {
vm->generation = new_vm_generation;
amdgpu_vm_bo_reset_state_machine(vm);
@@ -596,38 +577,59 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
return r;
}
- list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
- bo = bo_base->bo;
-
- r = validate(param, bo);
+ list_for_each_entry_safe(bo_base, tmp, &vm->kernel.evicted, vm_status) {
+ r = validate(param, bo_base->bo);
if (r)
return r;
- if (bo->tbo.type != ttm_bo_type_kernel) {
- amdgpu_vm_bo_moved(bo_base);
- } else {
- vm->update_funcs->map_table(to_amdgpu_bo_vm(bo));
- amdgpu_vm_bo_relocated(bo_base);
- }
+ vm->update_funcs->map_table(to_amdgpu_bo_vm(bo_base->bo));
+ amdgpu_vm_bo_moved(bo_base);
}
- if (ticket) {
- list_for_each_entry_safe(bo_base, tmp, &vm->evicted_user,
- vm_status) {
- bo = bo_base->bo;
- dma_resv_assert_held(bo->tbo.base.resv);
+ /*
+ * As soon as all page tables are in place we can start updating them
+ * again.
+ */
+ amdgpu_vm_eviction_lock(vm);
+ vm->evicting = false;
+ amdgpu_vm_eviction_unlock(vm);
- r = validate(param, bo);
- if (r)
- return r;
+ list_for_each_entry_safe(bo_base, tmp, &vm->always_valid.evicted,
+ vm_status) {
+ r = validate(param, bo_base->bo);
+ if (r)
+ return r;
- amdgpu_vm_bo_invalidated(bo_base);
- }
+ amdgpu_vm_bo_moved(bo_base);
}
- amdgpu_vm_eviction_lock(vm);
- vm->evicting = false;
- amdgpu_vm_eviction_unlock(vm);
+ if (!ticket)
+ return 0;
+
+ spin_lock(&vm->individual_lock);
+restart:
+ list_for_each_entry(bo_base, &vm->individual.evicted, vm_status) {
+ struct amdgpu_bo *bo = bo_base->bo;
+
+ if (dma_resv_locking_ctx(bo->tbo.base.resv) != ticket)
+ continue;
+
+ spin_unlock(&vm->individual_lock);
+
+ r = validate(param, bo);
+ if (r)
+ return r;
+
+ amdgpu_vm_bo_moved(bo_base);
+
+ /* It's a bit inefficient to always jump back to the start, but
+ * we would need to re-structure the KFD for properly fixing
+ * that.
+ */
+ spin_lock(&vm->individual_lock);
+ goto restart;
+ }
+ spin_unlock(&vm->individual_lock);
return 0;
}
@@ -652,7 +654,7 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
ret = !vm->evicting;
amdgpu_vm_eviction_unlock(vm);
- ret &= list_empty(&vm->evicted);
+ ret &= list_empty(&vm->kernel.evicted);
spin_lock(&vm->immediate.lock);
ret &= !vm->immediate.stopped;
@@ -952,7 +954,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
amdgpu_vm_assert_locked(vm);
- if (list_empty(&vm->relocated))
+ if (list_empty(&vm->kernel.moved))
return 0;
if (!drm_dev_enter(adev_to_drm(adev), &idx))
@@ -968,7 +970,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (r)
goto error;
- list_for_each_entry(entry, &vm->relocated, vm_status) {
+ list_for_each_entry(entry, &vm->kernel.moved, vm_status) {
/* vm_flush_needed after updating moved PDEs */
flush_tlb_needed |= entry->moved;
@@ -984,9 +986,8 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
if (flush_tlb_needed)
atomic64_inc(&vm->tlb_seq);
- list_for_each_entry_safe(entry, tmp, &vm->relocated, vm_status) {
+ list_for_each_entry_safe(entry, tmp, &vm->kernel.moved, vm_status)
amdgpu_vm_bo_idle(entry);
- }
error:
drm_dev_exit(idx);
@@ -1357,7 +1358,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
else
amdgpu_vm_bo_idle(&bo_va->base);
} else {
- amdgpu_vm_bo_done(&bo_va->base);
+ amdgpu_vm_bo_idle(&bo_va->base);
}
list_splice_init(&bo_va->invalids, &bo_va->valids);
@@ -1585,19 +1586,20 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
bool clear, unlock;
int r;
- list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
+ list_for_each_entry_safe(bo_va, tmp, &vm->always_valid.moved,
+ base.vm_status) {
/* Per VM BOs never need to bo cleared in the page tables */
r = amdgpu_vm_bo_update(adev, bo_va, false);
if (r)
return r;
}
- spin_lock(&vm->invalidated_lock);
- while (!list_empty(&vm->invalidated)) {
- bo_va = list_first_entry(&vm->invalidated, struct amdgpu_bo_va,
- base.vm_status);
+ spin_lock(&vm->individual_lock);
+ while (!list_empty(&vm->individual.moved)) {
+ bo_va = list_first_entry(&vm->individual.moved,
+ typeof(*bo_va), base.vm_status);
resv = bo_va->base.bo->tbo.base.resv;
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
/* Try to reserve the BO to avoid clearing its ptes */
if (!adev->debug_vm && dma_resv_trylock(resv)) {
@@ -1627,11 +1629,11 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
drm_gem_is_imported(&bo_va->base.bo->tbo.base) &&
(!bo_va->base.bo->tbo.resource ||
bo_va->base.bo->tbo.resource->mem_type == TTM_PL_SYSTEM))
- amdgpu_vm_bo_evicted_user(&bo_va->base);
+ amdgpu_vm_bo_evicted(&bo_va->base);
- spin_lock(&vm->invalidated_lock);
+ spin_lock(&vm->individual_lock);
}
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
return 0;
}
@@ -2174,9 +2176,9 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
}
}
- spin_lock(&vm->invalidated_lock);
+ spin_lock(&vm->individual_lock);
list_del(&bo_va->base.vm_status);
- spin_unlock(&vm->invalidated_lock);
+ spin_unlock(&vm->individual_lock);
list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
list_del(&mapping->list);
@@ -2256,14 +2258,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_bo *bo, bool evicted)
if (bo_base->moved)
continue;
- bo_base->moved = true;
-
- if (bo->tbo.type == ttm_bo_type_kernel)
- amdgpu_vm_bo_relocated(bo_base);
- else if (amdgpu_vm_is_bo_always_valid(vm, bo))
- amdgpu_vm_bo_moved(bo_base);
- else
- amdgpu_vm_bo_invalidated(bo_base);
+ amdgpu_vm_bo_moved(bo_base);
}
}
@@ -2554,15 +2549,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
vm->va = RB_ROOT_CACHED;
for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
vm->reserved_vmid[i] = NULL;
- INIT_LIST_HEAD(&vm->evicted);
- INIT_LIST_HEAD(&vm->evicted_user);
- INIT_LIST_HEAD(&vm->relocated);
- INIT_LIST_HEAD(&vm->moved);
- INIT_LIST_HEAD(&vm->idle);
- spin_lock_init(&vm->invalidated_lock);
- INIT_LIST_HEAD(&vm->invalidated);
+
+ amdgpu_vm_bo_status_init(&vm->kernel);
+ amdgpu_vm_bo_status_init(&vm->always_valid);
+ spin_lock_init(&vm->individual_lock);
+ amdgpu_vm_bo_status_init(&vm->individual);
INIT_LIST_HEAD(&vm->freed);
- INIT_LIST_HEAD(&vm->done);
INIT_KFIFO(vm->faults);
spin_lock_init(&vm->stats_lock);
@@ -3005,100 +2997,62 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
}
#if defined(CONFIG_DEBUG_FS)
-/**
- * amdgpu_debugfs_vm_bo_info - print BO info for the VM
- *
- * @vm: Requested VM for printing BO info
- * @m: debugfs file
- *
- * Print BO information in debugfs file for the VM
- */
-void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
-{
- struct amdgpu_bo_va *bo_va, *tmp;
- u64 total_idle = 0;
- u64 total_evicted = 0;
- u64 total_relocated = 0;
- u64 total_moved = 0;
- u64 total_invalidated = 0;
- u64 total_done = 0;
- unsigned int total_idle_objs = 0;
- unsigned int total_evicted_objs = 0;
- unsigned int total_relocated_objs = 0;
- unsigned int total_moved_objs = 0;
- unsigned int total_invalidated_objs = 0;
- unsigned int total_done_objs = 0;
- unsigned int id = 0;
- amdgpu_vm_assert_locked(vm);
+/* print the debug info for a specific set of status lists */
+static void amdgpu_debugfs_vm_bo_status_info(struct seq_file *m,
+ struct amdgpu_vm_bo_status *lists)
+{
+ struct amdgpu_vm_bo_base *base;
+ unsigned int id;
- seq_puts(m, "\tIdle BOs:\n");
- list_for_each_entry_safe(bo_va, tmp, &vm->idle, base.vm_status) {
- if (!bo_va->base.bo)
- continue;
- total_idle += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
- }
- total_idle_objs = id;
id = 0;
-
seq_puts(m, "\tEvicted BOs:\n");
- list_for_each_entry_safe(bo_va, tmp, &vm->evicted, base.vm_status) {
- if (!bo_va->base.bo)
+ list_for_each_entry(base, &lists->evicted, vm_status) {
+ if (!base->bo)
continue;
- total_evicted += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
- }
- total_evicted_objs = id;
- id = 0;
- seq_puts(m, "\tRelocated BOs:\n");
- list_for_each_entry_safe(bo_va, tmp, &vm->relocated, base.vm_status) {
- if (!bo_va->base.bo)
- continue;
- total_relocated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
+ amdgpu_bo_print_info(id++, base->bo, m);
}
- total_relocated_objs = id;
- id = 0;
+ id = 0;
seq_puts(m, "\tMoved BOs:\n");
- list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
- if (!bo_va->base.bo)
+ list_for_each_entry(base, &lists->moved, vm_status) {
+ if (!base->bo)
continue;
- total_moved += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
+
+ amdgpu_bo_print_info(id++, base->bo, m);
}
- total_moved_objs = id;
- id = 0;
- seq_puts(m, "\tInvalidated BOs:\n");
- spin_lock(&vm->invalidated_lock);
- list_for_each_entry_safe(bo_va, tmp, &vm->invalidated, base.vm_status) {
- if (!bo_va->base.bo)
+ id = 0;
+ seq_puts(m, "\tIdle BOs:\n");
+ list_for_each_entry(base, &lists->moved, vm_status) {
+ if (!base->bo)
continue;
- total_invalidated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
+
+ amdgpu_bo_print_info(id++, base->bo, m);
}
- spin_unlock(&vm->invalidated_lock);
- total_invalidated_objs = id;
- id = 0;
+}
- seq_puts(m, "\tDone BOs:\n");
- list_for_each_entry_safe(bo_va, tmp, &vm->done, base.vm_status) {
- if (!bo_va->base.bo)
- continue;
- total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
- }
- total_done_objs = id;
-
- seq_printf(m, "\tTotal idle size: %12lld\tobjs:\t%d\n", total_idle,
- total_idle_objs);
- seq_printf(m, "\tTotal evicted size: %12lld\tobjs:\t%d\n", total_evicted,
- total_evicted_objs);
- seq_printf(m, "\tTotal relocated size: %12lld\tobjs:\t%d\n", total_relocated,
- total_relocated_objs);
- seq_printf(m, "\tTotal moved size: %12lld\tobjs:\t%d\n", total_moved,
- total_moved_objs);
- seq_printf(m, "\tTotal invalidated size: %12lld\tobjs:\t%d\n", total_invalidated,
- total_invalidated_objs);
- seq_printf(m, "\tTotal done size: %12lld\tobjs:\t%d\n", total_done,
- total_done_objs);
+/**
+ * amdgpu_debugfs_vm_bo_info - print BO info for the VM
+ *
+ * @vm: Requested VM for printing BO info
+ * @m: debugfs file
+ *
+ * Print BO information in debugfs file for the VM
+ */
+void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
+{
+ amdgpu_vm_assert_locked(vm);
+
+ seq_puts(m, "\tKernel PT/PDs:\n");
+ amdgpu_debugfs_vm_bo_status_info(m, &vm->kernel);
+
+ seq_puts(m, "\tPer VM BOs:\n");
+ amdgpu_debugfs_vm_bo_status_info(m, &vm->always_valid);
+
+ seq_puts(m, "\tIndividual BOs:\n");
+ amdgpu_debugfs_vm_bo_status_info(m, &vm->individual);
}
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ae9449d5b00c..9b4a4e5c24f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -216,6 +216,23 @@ struct amdgpu_vm_bo_base {
bool moved;
};
+/*
+ * The following status lists contain amdgpu_vm_bo_base objects for
+ * either PD/PTs, per VM BOs or BOs with individual resv object.
+ *
+ * The state transits are: evicted -> moved -> idle
+ */
+struct amdgpu_vm_bo_status {
+ /* BOs evicted which need to move into place again */
+ struct list_head evicted;
+
+ /* BOs which moved but new location hasn't been updated in the PDs/PTs */
+ struct list_head moved;
+
+ /* BOs done with the state machine and need no further action */
+ struct list_head idle;
+};
+
/* provided by hw blocks that can write ptes, e.g., sdma */
struct amdgpu_vm_pte_funcs {
/* number of dw to reserve per operation */
@@ -349,46 +366,25 @@ struct amdgpu_vm {
spinlock_t stats_lock;
struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
+ /* BO's belonging to PD/PT which are internal to the kernel. */
+ struct amdgpu_vm_bo_status kernel;
+
/*
- * The following lists contain amdgpu_vm_bo_base objects for either
- * PDs, PTs or per VM BOs. The state transits are:
- *
- * evicted -> relocated (PDs, PTs) or moved (per VM BOs) -> idle
- *
- * Lists are protected by the root PD dma_resv lock.
+ * BOs allocated by userspace where the dma_resv is shared with the
+ * root PD
*/
-
- /* Per-VM and PT BOs who needs a validation */
- struct list_head evicted;
-
- /* PT BOs which relocated and their parent need an update */
- struct list_head relocated;
-
- /* per VM BOs moved, but not yet updated in the PT */
- struct list_head moved;
-
- /* All BOs of this VM not currently in the state machine */
- struct list_head idle;
+ struct amdgpu_vm_bo_status always_valid;
/*
* The following lists contain amdgpu_vm_bo_base objects for BOs which
- * have their own dma_resv object and not depend on the root PD. Their
- * state transits are:
- *
- * evicted_user or invalidated -> done
+ * have their own dma_resv object and not depend on the root PD.
*
- * Lists are protected by the invalidated_lock.
+ * Lists are protected by the individual_lock.
*/
- spinlock_t invalidated_lock;
-
- /* BOs for user mode queues that need a validation */
- struct list_head evicted_user;
-
- /* regular invalidated BOs, but not yet updated in the PT */
- struct list_head invalidated;
+ spinlock_t individual_lock;
- /* BOs which are invalidated, has been updated in the PTs */
- struct list_head done;
+ /* Userspace BOs with individual resv object */
+ struct amdgpu_vm_bo_status individual;
/*
* This list contains amdgpu_bo_va_mapping objects which have been freed
@@ -508,8 +504,8 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
int amdgpu_vm_lock_pd(struct amdgpu_vm *vm, struct drm_exec *exec,
unsigned int num_fences);
-int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
- unsigned int num_fences);
+int amdgpu_vm_lock_individual(struct amdgpu_vm *vm, struct drm_exec *exec,
+ unsigned int num_fences);
bool amdgpu_vm_ready(struct amdgpu_vm *vm);
uint64_t amdgpu_vm_generation(struct amdgpu_device *adev, struct amdgpu_vm *vm);
int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 09/10] drm/amdgpu: restructure VM state machine v2
2026-03-17 10:54 ` [PATCH 09/10] drm/amdgpu: restructure VM state machine v2 Christian König
@ 2026-03-17 12:03 ` Khatri, Sunil
0 siblings, 0 replies; 12+ messages in thread
From: Khatri, Sunil @ 2026-03-17 12:03 UTC (permalink / raw)
To: Christian König, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
[-- Attachment #1: Type: text/plain, Size: 30645 bytes --]
Reviewed-by: Sunil Khatri <sunil.khatri@amd.com>
On 17-03-2026 04:24 pm, Christian König wrote:
> Instead of coming up with more sophisticated names for states a VM BO
> can be in, group them by the type of BO first and then by the state.
>
> So we end with BO type kernel, always_valid and individual and then states
> evicted, moved and idle.
>
> Not much functional change, except that evicted_user is moved back
> together with the other BOs again which makes the handling in
> amdgpu_vm_validate() a bit more complex.
>
> Also fixes a problem with user queues and amdgpu_vm_ready(). We didn't
> considered the VM ready when user BOs were not ideally placed, harmless
> performance impact for kernel queues but a complete show stopper for
> userqueues.
>
> v2: fix a few typos in comments, rename the BO types to make them more
> descriptive, fix a couple of bugs found during testing
>
> Signed-off-by: Christian König<christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 22 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 466 ++++++++++------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 64 ++-
> 3 files changed, 251 insertions(+), 301 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index c50a484d22e0..9b2dfeead299 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1041,12 +1041,12 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
> struct amdgpu_bo *bo;
> int ret;
>
> - spin_lock(&vm->invalidated_lock);
> - while (!list_empty(&vm->invalidated)) {
> - bo_va = list_first_entry(&vm->invalidated,
> + spin_lock(&vm->individual_lock);
> + while (!list_empty(&vm->always_valid.evicted)) {
> + bo_va = list_first_entry(&vm->always_valid.evicted,
> struct amdgpu_bo_va,
> base.vm_status);
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> bo = bo_va->base.bo;
> ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 2);
> @@ -1058,14 +1058,14 @@ amdgpu_userq_bo_validate(struct amdgpu_device *adev, struct drm_exec *exec,
> if (ret)
> return ret;
>
> - /* This moves the bo_va to the done list */
> + /* This moves the bo_va to the idle list */
> ret = amdgpu_vm_bo_update(adev, bo_va, false);
> if (ret)
> return ret;
>
> - spin_lock(&vm->invalidated_lock);
> + spin_lock(&vm->individual_lock);
> }
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> return 0;
> }
> @@ -1097,7 +1097,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
> if (unlikely(ret))
> goto unlock_all;
>
> - ret = amdgpu_vm_lock_done_list(vm, &exec, 1);
> + ret = amdgpu_vm_lock_individual(vm, &exec, 1);
> drm_exec_retry_on_contention(&exec);
> if (unlikely(ret))
> goto unlock_all;
> @@ -1140,7 +1140,7 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
>
> key = 0;
> /* Validate User Ptr BOs */
> - list_for_each_entry(bo_va, &vm->done, base.vm_status) {
> + list_for_each_entry(bo_va, &vm->always_valid.idle, base.vm_status) {
> bo = bo_va->base.bo;
> if (!bo)
> continue;
> @@ -1190,10 +1190,10 @@ amdgpu_userq_vm_validate(struct amdgpu_userq_mgr *uq_mgr)
>
> /*
> * We need to wait for all VM updates to finish before restarting the
> - * queues. Using the done list like that is now ok since everything is
> + * queues. Using the idle list like that is now ok since everything is
> * locked in place.
> */
> - list_for_each_entry(bo_va, &vm->done, base.vm_status)
> + list_for_each_entry(bo_va, &vm->always_valid.idle, base.vm_status)
> dma_fence_wait(bo_va->last_pt_update, false);
> dma_fence_wait(vm->last_update, false);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index b89013a6aa0b..1223d1e86afa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -138,143 +138,127 @@ static void amdgpu_vm_assert_locked(struct amdgpu_vm *vm)
> dma_resv_assert_held(vm->root.bo->tbo.base.resv);
> }
>
> -/**
> - * amdgpu_vm_bo_evicted - vm_bo is evicted
> - *
> - * @vm_bo: vm_bo which is evicted
> - *
> - * State for PDs/PTs and per VM BOs which are not at the location they should
> - * be.
> +/* Initialize the amdgpu_vm_bo_status object */
> +static void amdgpu_vm_bo_status_init(struct amdgpu_vm_bo_status *lists)
> +{
> + INIT_LIST_HEAD(&lists->evicted);
> + INIT_LIST_HEAD(&lists->moved);
> + INIT_LIST_HEAD(&lists->idle);
> +}
> +
> +/*
> + * Make sure we have the lock to modify the vm_bo status and return the object
> + * with the status lists.
> */
> -static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
> +static struct amdgpu_vm_bo_status *
> +amdgpu_vm_bo_lock_lists(struct amdgpu_vm_bo_base *vm_bo)
> {
> struct amdgpu_vm *vm = vm_bo->vm;
> struct amdgpu_bo *bo = vm_bo->bo;
>
> - vm_bo->moved = true;
> - amdgpu_vm_assert_locked(vm);
> - if (bo->tbo.type == ttm_bo_type_kernel)
> - list_move(&vm_bo->vm_status, &vm->evicted);
> - else
> - list_move_tail(&vm_bo->vm_status, &vm->evicted);
> -}
> -/**
> - * amdgpu_vm_bo_moved - vm_bo is moved
> - *
> - * @vm_bo: vm_bo which is moved
> - *
> - * State for per VM BOs which are moved, but that change is not yet reflected
> - * in the page tables.
> - */
> -static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
> -{
> - amdgpu_vm_assert_locked(vm_bo->vm);
> - list_move(&vm_bo->vm_status, &vm_bo->vm->moved);
> -}
> + if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
> + /* No extra locking needed, protected by the root PD resv lock */
> + amdgpu_vm_assert_locked(vm);
>
> -/**
> - * amdgpu_vm_bo_idle - vm_bo is idle
> - *
> - * @vm_bo: vm_bo which is now idle
> - *
> - * State for PDs/PTs and per VM BOs which have gone through the state machine
> - * and are now idle.
> - */
> -static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
> -{
> - amdgpu_vm_assert_locked(vm_bo->vm);
> - list_move(&vm_bo->vm_status, &vm_bo->vm->idle);
> - vm_bo->moved = false;
> + if (bo->tbo.type == ttm_bo_type_kernel)
> + return &vm->kernel;
> +
> + return &vm->always_valid;
> + }
> +
> + spin_lock(&vm_bo->vm->individual_lock);
> + return &vm->individual;
> }
>
> -/**
> - * amdgpu_vm_bo_invalidated - vm_bo is invalidated
> - *
> - * @vm_bo: vm_bo which is now invalidated
> - *
> - * State for normal BOs which are invalidated and that change not yet reflected
> - * in the PTs.
> - */
> -static void amdgpu_vm_bo_invalidated(struct amdgpu_vm_bo_base *vm_bo)
> +/* Eventually unlock the status list lock again */
> +static void amdgpu_vm_bo_unlock_lists(struct amdgpu_vm_bo_base *vm_bo)
> {
> - spin_lock(&vm_bo->vm->invalidated_lock);
> - list_move(&vm_bo->vm_status, &vm_bo->vm->invalidated);
> - spin_unlock(&vm_bo->vm->invalidated_lock);
> + if (amdgpu_vm_is_bo_always_valid(vm_bo->vm, vm_bo->bo))
> + amdgpu_vm_assert_locked(vm_bo->vm);
> + else
> + spin_unlock(&vm_bo->vm->individual_lock);
> }
>
> /**
> - * amdgpu_vm_bo_evicted_user - vm_bo is evicted
> + * amdgpu_vm_bo_evicted - vm_bo is evicted
> *
> * @vm_bo: vm_bo which is evicted
> *
> - * State for BOs used by user mode queues which are not at the location they
> - * should be.
> + * State for vm_bo objects meaning the underlying BO was evicted and need to
> + * move in place again.
> */
> -static void amdgpu_vm_bo_evicted_user(struct amdgpu_vm_bo_base *vm_bo)
> +static void amdgpu_vm_bo_evicted(struct amdgpu_vm_bo_base *vm_bo)
> {
> - amdgpu_vm_assert_locked(vm_bo->vm);
> + struct amdgpu_vm_bo_status *lists;
> +
> + lists = amdgpu_vm_bo_lock_lists(vm_bo);
> vm_bo->moved = true;
> - list_move(&vm_bo->vm_status, &vm_bo->vm->evicted_user);
> + list_move(&vm_bo->vm_status, &lists->evicted);
> + amdgpu_vm_bo_unlock_lists(vm_bo);
> }
> -
> /**
> - * amdgpu_vm_bo_relocated - vm_bo is reloacted
> + * amdgpu_vm_bo_moved - vm_bo is moved
> *
> - * @vm_bo: vm_bo which is relocated
> + * @vm_bo: vm_bo which is moved
> *
> - * State for PDs/PTs which needs to update their parent PD.
> - * For the root PD, just move to idle state.
> + * State for vm_bo objects meaning the underlying BO was moved but the new
> + * location not yet reflected in the page tables.
> */
> -static void amdgpu_vm_bo_relocated(struct amdgpu_vm_bo_base *vm_bo)
> +static void amdgpu_vm_bo_moved(struct amdgpu_vm_bo_base *vm_bo)
> {
> - amdgpu_vm_assert_locked(vm_bo->vm);
> - if (vm_bo->bo->parent)
> - list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
> - else
> - amdgpu_vm_bo_idle(vm_bo);
> + struct amdgpu_vm_bo_status *lists;
> + struct amdgpu_bo *bo = vm_bo->bo;
> +
> + /*
> + * The root PD doesn't have a parent PDE and goes directly into the
> + * idle state.
> + */
> + lists = amdgpu_vm_bo_lock_lists(vm_bo);
> + if (bo && bo->tbo.type == ttm_bo_type_kernel && !bo->parent) {
> + vm_bo->moved = false;
> + list_move(&vm_bo->vm_status, &lists->idle);
> + } else {
> + vm_bo->moved = true;
> + list_move(&vm_bo->vm_status, &lists->moved);
> + }
> + amdgpu_vm_bo_unlock_lists(vm_bo);
> }
>
> /**
> - * amdgpu_vm_bo_done - vm_bo is done
> + * amdgpu_vm_bo_idle - vm_bo is idle
> *
> - * @vm_bo: vm_bo which is now done
> + * @vm_bo: vm_bo which is now idle
> *
> - * State for normal BOs which are invalidated and that change has been updated
> - * in the PTs.
> + * State for vm_bo objects meaning we are done with the state machine and no
> + * further action is necessary.
> */
> -static void amdgpu_vm_bo_done(struct amdgpu_vm_bo_base *vm_bo)
> +static void amdgpu_vm_bo_idle(struct amdgpu_vm_bo_base *vm_bo)
> {
> - amdgpu_vm_assert_locked(vm_bo->vm);
> - list_move(&vm_bo->vm_status, &vm_bo->vm->done);
> + struct amdgpu_vm_bo_status *lists;
> +
> + lists = amdgpu_vm_bo_lock_lists(vm_bo);
> + if (!amdgpu_vm_is_bo_always_valid(vm_bo->vm, vm_bo->bo))
> + vm_bo->moved = false;
> + list_move(&vm_bo->vm_status, &lists->idle);
> + amdgpu_vm_bo_unlock_lists(vm_bo);
> }
>
> /**
> * amdgpu_vm_bo_reset_state_machine - reset the vm_bo state machine
> * @vm: the VM which state machine to reset
> *
> - * Move all vm_bo object in the VM into a state where they will be updated
> - * again during validation.
> + * Move all vm_bo object in the VM into a state where their location will be
> + * updated in the page tables again.
> */
> static void amdgpu_vm_bo_reset_state_machine(struct amdgpu_vm *vm)
> {
> - struct amdgpu_vm_bo_base *vm_bo, *tmp;
> -
> - spin_lock(&vm->invalidated_lock);
> - list_splice_init(&vm->done, &vm->invalidated);
> - list_for_each_entry(vm_bo, &vm->invalidated, vm_status)
> - vm_bo->moved = true;
> - spin_unlock(&vm->invalidated_lock);
> -
> amdgpu_vm_assert_locked(vm);
> - list_for_each_entry_safe(vm_bo, tmp, &vm->idle, vm_status) {
> - struct amdgpu_bo *bo = vm_bo->bo;
> + list_splice_init(&vm->kernel.idle, &vm->kernel.moved);
> + list_splice_init(&vm->always_valid.idle, &vm->always_valid.moved);
>
> - vm_bo->moved = true;
> - if (!bo || bo->tbo.type != ttm_bo_type_kernel)
> - list_move(&vm_bo->vm_status, &vm_bo->vm->moved);
> - else if (bo->parent)
> - list_move(&vm_bo->vm_status, &vm_bo->vm->relocated);
> - }
> + spin_lock(&vm->individual_lock);
> + list_splice_init(&vm->individual.idle, &vm->individual.moved);
> + spin_unlock(&vm->individual_lock);
> }
>
> /**
> @@ -402,8 +386,10 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> base->next = NULL;
> INIT_LIST_HEAD(&base->vm_status);
>
> + dma_resv_assert_held(vm->root.bo->tbo.base.resv);
> if (!bo)
> return;
> +
> base->next = bo->vm_bo;
> bo->vm_bo = base;
>
> @@ -412,27 +398,22 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
> amdgpu_vm_update_stats_locked(base, bo->tbo.resource, +1);
> spin_unlock(&vm->stats_lock);
>
> - if (!amdgpu_vm_is_bo_always_valid(vm, bo))
> + if (!amdgpu_vm_is_bo_always_valid(vm, bo)) {
> + amdgpu_vm_bo_idle(base);
> return;
> -
> - dma_resv_assert_held(vm->root.bo->tbo.base.resv);
> + }
>
> ttm_bo_set_bulk_move(&bo->tbo, &vm->lru_bulk_move);
> - if (bo->tbo.type == ttm_bo_type_kernel && bo->parent)
> - amdgpu_vm_bo_relocated(base);
> - else
> - amdgpu_vm_bo_idle(base);
>
> + /*
> + * When a per VM isn't in the desired domain put it into the evicted
> + * state to make sure that it gets validated on the next best occasion.
> + */
> if (bo->preferred_domains &
> amdgpu_mem_type_to_domain(bo->tbo.resource->mem_type))
> - return;
> -
> - /*
> - * we checked all the prerequisites, but it looks like this per vm bo
> - * is currently evicted. add the bo to the evicted list to make sure it
> - * is validated on next vm use to avoid fault.
> - * */
> - amdgpu_vm_bo_evicted(base);
> + amdgpu_vm_bo_moved(base);
> + else
> + amdgpu_vm_bo_evicted(base);
> }
>
> /**
> @@ -453,41 +434,41 @@ int amdgpu_vm_lock_pd(struct amdgpu_vm *vm, struct drm_exec *exec,
> }
>
> /**
> - * amdgpu_vm_lock_done_list - lock all BOs on the done list
> + * amdgpu_vm_lock_individual - lock all BOs on the individual idle list
> * @vm: vm providing the BOs
> * @exec: drm execution context
> * @num_fences: number of extra fences to reserve
> *
> - * Lock the BOs on the done list in the DRM execution context.
> + * Lock the BOs on the individual idle list in the DRM execution context.
> */
> -int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
> - unsigned int num_fences)
> +int amdgpu_vm_lock_individual(struct amdgpu_vm *vm, struct drm_exec *exec,
> + unsigned int num_fences)
> {
> - struct list_head *prev = &vm->done;
> + struct list_head *prev = &vm->individual.idle;
> struct amdgpu_bo_va *bo_va;
> struct amdgpu_bo *bo;
> int ret;
>
> /* We can only trust prev->next while holding the lock */
> - spin_lock(&vm->invalidated_lock);
> - while (!list_is_head(prev->next, &vm->done)) {
> + spin_lock(&vm->individual_lock);
> + while (!list_is_head(prev->next, &vm->individual.idle)) {
> bo_va = list_entry(prev->next, typeof(*bo_va), base.vm_status);
>
> bo = bo_va->base.bo;
> if (bo) {
> amdgpu_bo_ref(bo);
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> ret = drm_exec_prepare_obj(exec, &bo->tbo.base, 1);
> amdgpu_bo_unref(&bo);
> if (unlikely(ret))
> return ret;
>
> - spin_lock(&vm->invalidated_lock);
> + spin_lock(&vm->individual_lock);
> }
> prev = prev->next;
> }
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> return 0;
> }
> @@ -584,9 +565,9 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> {
> uint64_t new_vm_generation = amdgpu_vm_generation(adev, vm);
> struct amdgpu_vm_bo_base *bo_base, *tmp;
> - struct amdgpu_bo *bo;
> int r;
>
> + dma_resv_assert_held(vm->root.bo->tbo.base.resv);
> if (vm->generation != new_vm_generation) {
> vm->generation = new_vm_generation;
> amdgpu_vm_bo_reset_state_machine(vm);
> @@ -596,38 +577,59 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> return r;
> }
>
> - list_for_each_entry_safe(bo_base, tmp, &vm->evicted, vm_status) {
> - bo = bo_base->bo;
> -
> - r = validate(param, bo);
> + list_for_each_entry_safe(bo_base, tmp, &vm->kernel.evicted, vm_status) {
> + r = validate(param, bo_base->bo);
> if (r)
> return r;
>
> - if (bo->tbo.type != ttm_bo_type_kernel) {
> - amdgpu_vm_bo_moved(bo_base);
> - } else {
> - vm->update_funcs->map_table(to_amdgpu_bo_vm(bo));
> - amdgpu_vm_bo_relocated(bo_base);
> - }
> + vm->update_funcs->map_table(to_amdgpu_bo_vm(bo_base->bo));
> + amdgpu_vm_bo_moved(bo_base);
> }
>
> - if (ticket) {
> - list_for_each_entry_safe(bo_base, tmp, &vm->evicted_user,
> - vm_status) {
> - bo = bo_base->bo;
> - dma_resv_assert_held(bo->tbo.base.resv);
> + /*
> + * As soon as all page tables are in place we can start updating them
> + * again.
> + */
> + amdgpu_vm_eviction_lock(vm);
> + vm->evicting = false;
> + amdgpu_vm_eviction_unlock(vm);
>
> - r = validate(param, bo);
> - if (r)
> - return r;
> + list_for_each_entry_safe(bo_base, tmp, &vm->always_valid.evicted,
> + vm_status) {
> + r = validate(param, bo_base->bo);
> + if (r)
> + return r;
>
> - amdgpu_vm_bo_invalidated(bo_base);
> - }
> + amdgpu_vm_bo_moved(bo_base);
> }
>
> - amdgpu_vm_eviction_lock(vm);
> - vm->evicting = false;
> - amdgpu_vm_eviction_unlock(vm);
> + if (!ticket)
> + return 0;
> +
> + spin_lock(&vm->individual_lock);
> +restart:
> + list_for_each_entry(bo_base, &vm->individual.evicted, vm_status) {
> + struct amdgpu_bo *bo = bo_base->bo;
> +
> + if (dma_resv_locking_ctx(bo->tbo.base.resv) != ticket)
> + continue;
> +
> + spin_unlock(&vm->individual_lock);
> +
> + r = validate(param, bo);
> + if (r)
> + return r;
> +
> + amdgpu_vm_bo_moved(bo_base);
> +
> + /* It's a bit inefficient to always jump back to the start, but
> + * we would need to re-structure the KFD for properly fixing
> + * that.
> + */
> + spin_lock(&vm->individual_lock);
> + goto restart;
> + }
> + spin_unlock(&vm->individual_lock);
>
> return 0;
> }
> @@ -652,7 +654,7 @@ bool amdgpu_vm_ready(struct amdgpu_vm *vm)
> ret = !vm->evicting;
> amdgpu_vm_eviction_unlock(vm);
>
> - ret &= list_empty(&vm->evicted);
> + ret &= list_empty(&vm->kernel.evicted);
>
> spin_lock(&vm->immediate.lock);
> ret &= !vm->immediate.stopped;
> @@ -952,7 +954,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>
> amdgpu_vm_assert_locked(vm);
>
> - if (list_empty(&vm->relocated))
> + if (list_empty(&vm->kernel.moved))
> return 0;
>
> if (!drm_dev_enter(adev_to_drm(adev), &idx))
> @@ -968,7 +970,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
> if (r)
> goto error;
>
> - list_for_each_entry(entry, &vm->relocated, vm_status) {
> + list_for_each_entry(entry, &vm->kernel.moved, vm_status) {
> /* vm_flush_needed after updating moved PDEs */
> flush_tlb_needed |= entry->moved;
>
> @@ -984,9 +986,8 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
> if (flush_tlb_needed)
> atomic64_inc(&vm->tlb_seq);
>
> - list_for_each_entry_safe(entry, tmp, &vm->relocated, vm_status) {
> + list_for_each_entry_safe(entry, tmp, &vm->kernel.moved, vm_status)
> amdgpu_vm_bo_idle(entry);
> - }
>
> error:
> drm_dev_exit(idx);
> @@ -1357,7 +1358,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
> else
> amdgpu_vm_bo_idle(&bo_va->base);
> } else {
> - amdgpu_vm_bo_done(&bo_va->base);
> + amdgpu_vm_bo_idle(&bo_va->base);
> }
>
> list_splice_init(&bo_va->invalids, &bo_va->valids);
> @@ -1585,19 +1586,20 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> bool clear, unlock;
> int r;
>
> - list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
> + list_for_each_entry_safe(bo_va, tmp, &vm->always_valid.moved,
> + base.vm_status) {
> /* Per VM BOs never need to bo cleared in the page tables */
> r = amdgpu_vm_bo_update(adev, bo_va, false);
> if (r)
> return r;
> }
>
> - spin_lock(&vm->invalidated_lock);
> - while (!list_empty(&vm->invalidated)) {
> - bo_va = list_first_entry(&vm->invalidated, struct amdgpu_bo_va,
> - base.vm_status);
> + spin_lock(&vm->individual_lock);
> + while (!list_empty(&vm->individual.moved)) {
> + bo_va = list_first_entry(&vm->individual.moved,
> + typeof(*bo_va), base.vm_status);
> resv = bo_va->base.bo->tbo.base.resv;
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> /* Try to reserve the BO to avoid clearing its ptes */
> if (!adev->debug_vm && dma_resv_trylock(resv)) {
> @@ -1627,11 +1629,11 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
> drm_gem_is_imported(&bo_va->base.bo->tbo.base) &&
> (!bo_va->base.bo->tbo.resource ||
> bo_va->base.bo->tbo.resource->mem_type == TTM_PL_SYSTEM))
> - amdgpu_vm_bo_evicted_user(&bo_va->base);
> + amdgpu_vm_bo_evicted(&bo_va->base);
>
> - spin_lock(&vm->invalidated_lock);
> + spin_lock(&vm->individual_lock);
> }
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> return 0;
> }
> @@ -2174,9 +2176,9 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
> }
> }
>
> - spin_lock(&vm->invalidated_lock);
> + spin_lock(&vm->individual_lock);
> list_del(&bo_va->base.vm_status);
> - spin_unlock(&vm->invalidated_lock);
> + spin_unlock(&vm->individual_lock);
>
> list_for_each_entry_safe(mapping, next, &bo_va->valids, list) {
> list_del(&mapping->list);
> @@ -2256,14 +2258,7 @@ void amdgpu_vm_bo_invalidate(struct amdgpu_bo *bo, bool evicted)
>
> if (bo_base->moved)
> continue;
> - bo_base->moved = true;
> -
> - if (bo->tbo.type == ttm_bo_type_kernel)
> - amdgpu_vm_bo_relocated(bo_base);
> - else if (amdgpu_vm_is_bo_always_valid(vm, bo))
> - amdgpu_vm_bo_moved(bo_base);
> - else
> - amdgpu_vm_bo_invalidated(bo_base);
> + amdgpu_vm_bo_moved(bo_base);
> }
> }
>
> @@ -2554,15 +2549,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> vm->va = RB_ROOT_CACHED;
> for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
> vm->reserved_vmid[i] = NULL;
> - INIT_LIST_HEAD(&vm->evicted);
> - INIT_LIST_HEAD(&vm->evicted_user);
> - INIT_LIST_HEAD(&vm->relocated);
> - INIT_LIST_HEAD(&vm->moved);
> - INIT_LIST_HEAD(&vm->idle);
> - spin_lock_init(&vm->invalidated_lock);
> - INIT_LIST_HEAD(&vm->invalidated);
> +
> + amdgpu_vm_bo_status_init(&vm->kernel);
> + amdgpu_vm_bo_status_init(&vm->always_valid);
> + spin_lock_init(&vm->individual_lock);
> + amdgpu_vm_bo_status_init(&vm->individual);
> INIT_LIST_HEAD(&vm->freed);
> - INIT_LIST_HEAD(&vm->done);
> INIT_KFIFO(vm->faults);
> spin_lock_init(&vm->stats_lock);
>
> @@ -3005,100 +2997,62 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
> }
>
> #if defined(CONFIG_DEBUG_FS)
> -/**
> - * amdgpu_debugfs_vm_bo_info - print BO info for the VM
> - *
> - * @vm: Requested VM for printing BO info
> - * @m: debugfs file
> - *
> - * Print BO information in debugfs file for the VM
> - */
> -void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
> -{
> - struct amdgpu_bo_va *bo_va, *tmp;
> - u64 total_idle = 0;
> - u64 total_evicted = 0;
> - u64 total_relocated = 0;
> - u64 total_moved = 0;
> - u64 total_invalidated = 0;
> - u64 total_done = 0;
> - unsigned int total_idle_objs = 0;
> - unsigned int total_evicted_objs = 0;
> - unsigned int total_relocated_objs = 0;
> - unsigned int total_moved_objs = 0;
> - unsigned int total_invalidated_objs = 0;
> - unsigned int total_done_objs = 0;
> - unsigned int id = 0;
>
> - amdgpu_vm_assert_locked(vm);
> +/* print the debug info for a specific set of status lists */
> +static void amdgpu_debugfs_vm_bo_status_info(struct seq_file *m,
> + struct amdgpu_vm_bo_status *lists)
> +{
> + struct amdgpu_vm_bo_base *base;
> + unsigned int id;
>
> - seq_puts(m, "\tIdle BOs:\n");
> - list_for_each_entry_safe(bo_va, tmp, &vm->idle, base.vm_status) {
> - if (!bo_va->base.bo)
> - continue;
> - total_idle += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> - }
> - total_idle_objs = id;
> id = 0;
> -
> seq_puts(m, "\tEvicted BOs:\n");
> - list_for_each_entry_safe(bo_va, tmp, &vm->evicted, base.vm_status) {
> - if (!bo_va->base.bo)
> + list_for_each_entry(base, &lists->evicted, vm_status) {
> + if (!base->bo)
> continue;
> - total_evicted += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> - }
> - total_evicted_objs = id;
> - id = 0;
>
> - seq_puts(m, "\tRelocated BOs:\n");
> - list_for_each_entry_safe(bo_va, tmp, &vm->relocated, base.vm_status) {
> - if (!bo_va->base.bo)
> - continue;
> - total_relocated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> + amdgpu_bo_print_info(id++, base->bo, m);
> }
> - total_relocated_objs = id;
> - id = 0;
>
> + id = 0;
> seq_puts(m, "\tMoved BOs:\n");
> - list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
> - if (!bo_va->base.bo)
> + list_for_each_entry(base, &lists->moved, vm_status) {
> + if (!base->bo)
> continue;
> - total_moved += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> +
> + amdgpu_bo_print_info(id++, base->bo, m);
> }
> - total_moved_objs = id;
> - id = 0;
>
> - seq_puts(m, "\tInvalidated BOs:\n");
> - spin_lock(&vm->invalidated_lock);
> - list_for_each_entry_safe(bo_va, tmp, &vm->invalidated, base.vm_status) {
> - if (!bo_va->base.bo)
> + id = 0;
> + seq_puts(m, "\tIdle BOs:\n");
> + list_for_each_entry(base, &lists->moved, vm_status) {
> + if (!base->bo)
> continue;
> - total_invalidated += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> +
> + amdgpu_bo_print_info(id++, base->bo, m);
> }
> - spin_unlock(&vm->invalidated_lock);
> - total_invalidated_objs = id;
> - id = 0;
> +}
>
> - seq_puts(m, "\tDone BOs:\n");
> - list_for_each_entry_safe(bo_va, tmp, &vm->done, base.vm_status) {
> - if (!bo_va->base.bo)
> - continue;
> - total_done += amdgpu_bo_print_info(id++, bo_va->base.bo, m);
> - }
> - total_done_objs = id;
> -
> - seq_printf(m, "\tTotal idle size: %12lld\tobjs:\t%d\n", total_idle,
> - total_idle_objs);
> - seq_printf(m, "\tTotal evicted size: %12lld\tobjs:\t%d\n", total_evicted,
> - total_evicted_objs);
> - seq_printf(m, "\tTotal relocated size: %12lld\tobjs:\t%d\n", total_relocated,
> - total_relocated_objs);
> - seq_printf(m, "\tTotal moved size: %12lld\tobjs:\t%d\n", total_moved,
> - total_moved_objs);
> - seq_printf(m, "\tTotal invalidated size: %12lld\tobjs:\t%d\n", total_invalidated,
> - total_invalidated_objs);
> - seq_printf(m, "\tTotal done size: %12lld\tobjs:\t%d\n", total_done,
> - total_done_objs);
> +/**
> + * amdgpu_debugfs_vm_bo_info - print BO info for the VM
> + *
> + * @vm: Requested VM for printing BO info
> + * @m: debugfs file
> + *
> + * Print BO information in debugfs file for the VM
> + */
> +void amdgpu_debugfs_vm_bo_info(struct amdgpu_vm *vm, struct seq_file *m)
> +{
> + amdgpu_vm_assert_locked(vm);
> +
> + seq_puts(m, "\tKernel PT/PDs:\n");
> + amdgpu_debugfs_vm_bo_status_info(m, &vm->kernel);
> +
> + seq_puts(m, "\tPer VM BOs:\n");
> + amdgpu_debugfs_vm_bo_status_info(m, &vm->always_valid);
> +
> + seq_puts(m, "\tIndividual BOs:\n");
> + amdgpu_debugfs_vm_bo_status_info(m, &vm->individual);
> }
> #endif
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index ae9449d5b00c..9b4a4e5c24f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -216,6 +216,23 @@ struct amdgpu_vm_bo_base {
> bool moved;
> };
>
> +/*
> + * The following status lists contain amdgpu_vm_bo_base objects for
> + * either PD/PTs, per VM BOs or BOs with individual resv object.
> + *
> + * The state transits are: evicted -> moved -> idle
> + */
> +struct amdgpu_vm_bo_status {
> + /* BOs evicted which need to move into place again */
> + struct list_head evicted;
> +
> + /* BOs which moved but new location hasn't been updated in the PDs/PTs */
> + struct list_head moved;
> +
> + /* BOs done with the state machine and need no further action */
> + struct list_head idle;
> +};
> +
> /* provided by hw blocks that can write ptes, e.g., sdma */
> struct amdgpu_vm_pte_funcs {
> /* number of dw to reserve per operation */
> @@ -349,46 +366,25 @@ struct amdgpu_vm {
> spinlock_t stats_lock;
> struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM];
>
> + /* BO's belonging to PD/PT which are internal to the kernel. */
> + struct amdgpu_vm_bo_status kernel;
> +
> /*
> - * The following lists contain amdgpu_vm_bo_base objects for either
> - * PDs, PTs or per VM BOs. The state transits are:
> - *
> - * evicted -> relocated (PDs, PTs) or moved (per VM BOs) -> idle
> - *
> - * Lists are protected by the root PD dma_resv lock.
> + * BOs allocated by userspace where the dma_resv is shared with the
> + * root PD
> */
> -
> - /* Per-VM and PT BOs who needs a validation */
> - struct list_head evicted;
> -
> - /* PT BOs which relocated and their parent need an update */
> - struct list_head relocated;
> -
> - /* per VM BOs moved, but not yet updated in the PT */
> - struct list_head moved;
> -
> - /* All BOs of this VM not currently in the state machine */
> - struct list_head idle;
> + struct amdgpu_vm_bo_status always_valid;
>
> /*
> * The following lists contain amdgpu_vm_bo_base objects for BOs which
> - * have their own dma_resv object and not depend on the root PD. Their
> - * state transits are:
> - *
> - * evicted_user or invalidated -> done
> + * have their own dma_resv object and not depend on the root PD.
> *
> - * Lists are protected by the invalidated_lock.
> + * Lists are protected by the individual_lock.
> */
> - spinlock_t invalidated_lock;
> -
> - /* BOs for user mode queues that need a validation */
> - struct list_head evicted_user;
> -
> - /* regular invalidated BOs, but not yet updated in the PT */
> - struct list_head invalidated;
> + spinlock_t individual_lock;
>
> - /* BOs which are invalidated, has been updated in the PTs */
> - struct list_head done;
> + /* Userspace BOs with individual resv object */
> + struct amdgpu_vm_bo_status individual;
>
> /*
> * This list contains amdgpu_bo_va_mapping objects which have been freed
> @@ -508,8 +504,8 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> int amdgpu_vm_lock_pd(struct amdgpu_vm *vm, struct drm_exec *exec,
> unsigned int num_fences);
> -int amdgpu_vm_lock_done_list(struct amdgpu_vm *vm, struct drm_exec *exec,
> - unsigned int num_fences);
> +int amdgpu_vm_lock_individual(struct amdgpu_vm *vm, struct drm_exec *exec,
> + unsigned int num_fences);
> bool amdgpu_vm_ready(struct amdgpu_vm *vm);
> uint64_t amdgpu_vm_generation(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
[-- Attachment #2: Type: text/html, Size: 30953 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 10/10] drm/amdgpu: WIP sync amdgpu_ttm_fill_mem only to kernel fences
2026-03-17 10:54 [PATCH 01/10] drm/amdgpu: completely rework eviction fence handling v2 Christian König
` (7 preceding siblings ...)
2026-03-17 10:54 ` [PATCH 09/10] drm/amdgpu: restructure VM state machine v2 Christian König
@ 2026-03-17 10:55 ` Christian König
2026-03-17 12:03 ` Khatri, Sunil
8 siblings, 1 reply; 12+ messages in thread
From: Christian König @ 2026-03-17 10:55 UTC (permalink / raw)
To: sukhatri, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
That's not even remotely correct, but should unblock testing for now.
Signed-off-by: Christian König <christian.koenig@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 714fd8d12ca5..69f52a078022 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2428,12 +2428,14 @@ static int amdgpu_ttm_prepare_job(struct amdgpu_device *adev,
struct amdgpu_ttm_buffer_entity *entity,
unsigned int num_dw,
struct dma_resv *resv,
+ enum dma_resv_usage usage,
bool vm_needs_flush,
struct amdgpu_job **job,
u64 k_job_id)
{
enum amdgpu_ib_pool_type pool = AMDGPU_IB_POOL_DELAYED;
int r;
+
r = amdgpu_job_alloc_with_ib(adev, &entity->base,
AMDGPU_FENCE_OWNER_UNDEFINED,
num_dw * 4, pool, job, k_job_id);
@@ -2449,8 +2451,7 @@ static int amdgpu_ttm_prepare_job(struct amdgpu_device *adev,
if (!resv)
return 0;
- return drm_sched_job_add_resv_dependencies(&(*job)->base, resv,
- DMA_RESV_USAGE_BOOKKEEP);
+ return drm_sched_job_add_resv_dependencies(&(*job)->base, resv, usage);
}
int amdgpu_copy_buffer(struct amdgpu_device *adev,
@@ -2479,9 +2480,9 @@ int amdgpu_copy_buffer(struct amdgpu_device *adev,
max_bytes = adev->mman.buffer_funcs->copy_max_bytes;
num_loops = DIV_ROUND_UP(byte_count, max_bytes);
num_dw = ALIGN(num_loops * adev->mman.buffer_funcs->copy_num_dw, 8);
- r = amdgpu_ttm_prepare_job(adev, entity, num_dw,
- resv, vm_needs_flush, &job,
- AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER);
+ r = amdgpu_ttm_prepare_job(adev, entity, num_dw, resv,
+ DMA_RESV_USAGE_BOOKKEEP, vm_needs_flush,
+ &job, AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER);
if (r)
goto error_free;
@@ -2524,6 +2525,7 @@ static int amdgpu_ttm_fill_mem(struct amdgpu_device *adev,
num_loops = DIV_ROUND_UP_ULL(byte_count, max_bytes);
num_dw = ALIGN(num_loops * adev->mman.buffer_funcs->fill_num_dw, 8);
r = amdgpu_ttm_prepare_job(adev, entity, num_dw, resv,
+ DMA_RESV_USAGE_KERNEL,
vm_needs_flush, &job, k_job_id);
if (r)
return r;
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 10/10] drm/amdgpu: WIP sync amdgpu_ttm_fill_mem only to kernel fences
2026-03-17 10:55 ` [PATCH 10/10] drm/amdgpu: WIP sync amdgpu_ttm_fill_mem only to kernel fences Christian König
@ 2026-03-17 12:03 ` Khatri, Sunil
0 siblings, 0 replies; 12+ messages in thread
From: Khatri, Sunil @ 2026-03-17 12:03 UTC (permalink / raw)
To: Christian König, tursulin, Alexander.Deucher, Prike.Liang,
SRINIVASAN.SHANMUGAM, christian.koenig
Cc: amd-gfx
[-- Attachment #1: Type: text/plain, Size: 2500 bytes --]
Acked-by: Sunil Khatri <sunil.khatri@amd.com>
On 17-03-2026 04:25 pm, Christian König wrote:
> That's not even remotely correct, but should unblock testing for now.
>
> Signed-off-by: Christian König<christian.koenig@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 714fd8d12ca5..69f52a078022 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -2428,12 +2428,14 @@ static int amdgpu_ttm_prepare_job(struct amdgpu_device *adev,
> struct amdgpu_ttm_buffer_entity *entity,
> unsigned int num_dw,
> struct dma_resv *resv,
> + enum dma_resv_usage usage,
> bool vm_needs_flush,
> struct amdgpu_job **job,
> u64 k_job_id)
> {
> enum amdgpu_ib_pool_type pool = AMDGPU_IB_POOL_DELAYED;
> int r;
> +
> r = amdgpu_job_alloc_with_ib(adev, &entity->base,
> AMDGPU_FENCE_OWNER_UNDEFINED,
> num_dw * 4, pool, job, k_job_id);
> @@ -2449,8 +2451,7 @@ static int amdgpu_ttm_prepare_job(struct amdgpu_device *adev,
> if (!resv)
> return 0;
>
> - return drm_sched_job_add_resv_dependencies(&(*job)->base, resv,
> - DMA_RESV_USAGE_BOOKKEEP);
> + return drm_sched_job_add_resv_dependencies(&(*job)->base, resv, usage);
> }
>
> int amdgpu_copy_buffer(struct amdgpu_device *adev,
> @@ -2479,9 +2480,9 @@ int amdgpu_copy_buffer(struct amdgpu_device *adev,
> max_bytes = adev->mman.buffer_funcs->copy_max_bytes;
> num_loops = DIV_ROUND_UP(byte_count, max_bytes);
> num_dw = ALIGN(num_loops * adev->mman.buffer_funcs->copy_num_dw, 8);
> - r = amdgpu_ttm_prepare_job(adev, entity, num_dw,
> - resv, vm_needs_flush, &job,
> - AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER);
> + r = amdgpu_ttm_prepare_job(adev, entity, num_dw, resv,
> + DMA_RESV_USAGE_BOOKKEEP, vm_needs_flush,
> + &job, AMDGPU_KERNEL_JOB_ID_TTM_COPY_BUFFER);
> if (r)
> goto error_free;
>
> @@ -2524,6 +2525,7 @@ static int amdgpu_ttm_fill_mem(struct amdgpu_device *adev,
> num_loops = DIV_ROUND_UP_ULL(byte_count, max_bytes);
> num_dw = ALIGN(num_loops * adev->mman.buffer_funcs->fill_num_dw, 8);
> r = amdgpu_ttm_prepare_job(adev, entity, num_dw, resv,
> + DMA_RESV_USAGE_KERNEL,
> vm_needs_flush, &job, k_job_id);
> if (r)
> return r;
[-- Attachment #2: Type: text/html, Size: 3026 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread