* [RFC PATCH 2/3] drm/amdgpu: recheck range validity after page table allocation
2026-08-30 15:40 [RFC PATCH 0/3] drm/amdgpu: unify SVM notifier and VM eviction locking Honglei Huang
2026-08-30 15:40 ` [RFC PATCH 1/3] drm/amdgpu: make VM eviction lock read/write to match notifier lock Honglei Huang
@ 2026-08-30 15:40 ` Honglei Huang
2026-08-30 15:40 ` [RFC PATCH 3/3] drm/amdgpu: use drm_gpusvm notifier lock for VM eviction Honglei Huang
2026-09-01 4:34 ` [RFC PATCH 0/3] drm/amdgpu: unify SVM notifier and VM eviction locking Huang, Honglei
3 siblings, 0 replies; 5+ messages in thread
From: Honglei Huang @ 2026-08-30 15:40 UTC (permalink / raw)
To: Christian.Koenig, Alexander.Deucher, Felix.Kuehling, Oak.Zeng,
Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Junhua.Shen, timur.kristof, natalie.vock
Cc: amd-gfx, honghuan, Christian König
From: Honglei Huang <honghuan@amd.com>
The notifier and eviction locks need to be the same lock. Page table
allocation can enter memory reclaim, and reclaim can invoke an MMU
notifier that takes the same lock. Release the lock before allocating
page tables to avoid a deadlock.
Dropping the lock allows an unmap operation to invalidate the range
concurrently. Recheck the HMM or GPU SVM range after reacquiring the
lock and return -EAGAIN if its sequence number has changed.
Allow callers that already hold the eviction lock to pass it to the VM
update helpers. Drop the lock around amdgpu_vm_pt_create() and reacquire
it before continuing. Callers that pass NULL retain the existing
internal locking path.
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
.../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c | 5 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 75 +++++++++++++------
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 9 ++-
.../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h | 28 +++++--
drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 29 ++++++-
drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +-
10 files changed, 116 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index 7882f13852..74a8a27a9e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -503,7 +503,7 @@ static int vm_update_pds(struct amdgpu_vm *vm, struct amdgpu_sync *sync)
struct amdgpu_device *adev = amdgpu_ttm_adev(pd->tbo.bdev);
int ret;
- ret = amdgpu_vm_update_pdes(adev, vm, false);
+ ret = amdgpu_vm_update_pdes(adev, vm, false, NULL);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index e129ec4644..30469769b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1184,7 +1184,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
if (r)
return r;
- r = amdgpu_vm_update_pdes(adev, vm, false);
+ r = amdgpu_vm_update_pdes(adev, vm, false, NULL);
if (r)
return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index f754a4a3a1..833d05a199 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -795,7 +795,7 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
}
/* Always update PDEs after we touched the mappings. */
- r = amdgpu_vm_update_pdes(adev, vm, false);
+ r = amdgpu_vm_update_pdes(adev, vm, false, NULL);
if (r)
goto error;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
index 08a10955a9..3e622ac059 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
@@ -73,7 +73,7 @@ amdgpu_svm_range_zap_ptes(struct amdgpu_svm *svm,
flags = memalloc_noreclaim_save();
ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, NULL,
- start_page, last_page, 0, &fence);
+ start_page, last_page, 0, NULL, &fence);
memalloc_noreclaim_restore(flags);
if (fence) {
@@ -222,6 +222,7 @@ amdgpu_svm_range_update_gpu_range(struct amdgpu_svm *svm,
flush_tlb && is_last_seg, true, NULL,
start_page, last_page, pte_flags,
0, entry->addr, NULL, NULL, NULL,
+ NULL, NULL,
wait_fence && is_last_seg ? fence : NULL);
if (ret)
return ret;
@@ -330,7 +331,7 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm,
dma_fence_put(fence);
if (!ret)
- ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false);
+ ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false, NULL);
if (!ret) {
if (flush_tlb)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 0a816b3c5f..568e6bb7f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1151,7 +1151,7 @@ amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr *uq_mgr)
goto retry_lock;
}
- ret = amdgpu_vm_update_pdes(adev, vm, false);
+ ret = amdgpu_vm_update_pdes(adev, vm, false, NULL);
if (ret)
goto unlock_all;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a2cc6e2017..cb62c08ec5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -986,6 +986,7 @@ uint64_t amdgpu_vm_map_gart(const dma_addr_t *pages_addr, uint64_t addr)
* @adev: amdgpu_device pointer
* @vm: requested vm
* @immediate: submit immediately to the paging queue
+ * @eviction_lock: optional caller-held eviction lock
*
* Makes sure all directories are up to date.
*
@@ -993,7 +994,8 @@ uint64_t amdgpu_vm_map_gart(const dma_addr_t *pages_addr, uint64_t addr)
* 0 for success, error for failure.
*/
int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
- struct amdgpu_vm *vm, bool immediate)
+ struct amdgpu_vm *vm, bool immediate,
+ struct rw_semaphore *eviction_lock)
{
struct amdgpu_vm_update_params params;
struct amdgpu_vm_bo_base *entry, *tmp;
@@ -1013,6 +1015,16 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
params.vm = vm;
params.immediate = immediate;
+ /*
+ * Keep PDE updates under the SVM eviction lock so MMU notifier
+ * invalidation won't happen between the PTE and PDE updates.
+ */
+ params.eviction_lock = eviction_lock;
+
+ r = amdgpu_vm_begin_critical(¶ms);
+ if (r)
+ goto error;
+
r = vm->update_funcs->prepare(¶ms, NULL,
AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
if (r)
@@ -1039,6 +1051,7 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
amdgpu_vm_bo_idle(entry);
error:
+ amdgpu_vm_end_critical(¶ms);
drm_dev_exit(idx);
return r;
}
@@ -1119,6 +1132,8 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
* @res: ttm_resource to map
* @pages_addr: DMA addresses to use for mapping
* @hmm_range: to check validity of DMA addresses
+ * @eviction_lock: optional caller-held eviction lock
+ * @gpusvm_range: optional SVM range to revalidate
* @fence: optional resulting fence
*
* Fill in the page table entries between @start and @last. Allocate and free
@@ -1134,6 +1149,8 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
uint64_t vram_base, struct ttm_resource *res,
dma_addr_t *pages_addr,
struct amdgpu_hmm_range *hmm_range,
+ struct rw_semaphore *eviction_lock,
+ struct drm_gpusvm_range *gpusvm_range,
struct dma_fence **fence)
{
struct amdgpu_vm_tlb_seq_struct *tlb_cb;
@@ -1146,10 +1163,25 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
if (!drm_dev_enter(adev_to_drm(adev), &idx))
return -ENODEV;
+ memset(¶ms, 0, sizeof(params));
+ params.adev = adev;
+ params.vm = vm;
+ params.pages_addr = pages_addr;
+ params.hmm_range = hmm_range;
+ params.eviction_lock = eviction_lock;
+ params.gpusvm_range = gpusvm_range;
+ params.override_pte = allow_override && adev->gmc.override_pte;
+ INIT_LIST_HEAD(¶ms.tlb_flush_waitlist);
+
+ /* Enter noreclaim before allocating the small tlb_cb. */
+ r = amdgpu_vm_begin_critical(¶ms);
+ if (r)
+ goto error_critical;
+
tlb_cb = kmalloc_obj(*tlb_cb);
if (!tlb_cb) {
- drm_dev_exit(idx);
- return -ENOMEM;
+ r = -ENOMEM;
+ goto error_critical;
}
/* Vega20+XGMI where PTEs get inadvertently cached in L2 texture cache,
@@ -1163,18 +1195,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
*/
flush_tlb |= amdgpu_ip_version(adev, GC_HWIP, 0) < IP_VERSION(9, 0, 0);
- memset(¶ms, 0, sizeof(params));
- params.adev = adev;
- params.vm = vm;
- params.pages_addr = pages_addr;
- params.hmm_range = hmm_range;
params.needs_flush = flush_tlb;
- params.override_pte = allow_override && adev->gmc.override_pte;
- INIT_LIST_HEAD(¶ms.tlb_flush_waitlist);
-
- r = amdgpu_vm_begin_critical(¶ms);
- if (r)
- goto error_free;
if (!dma_fence_is_signaled(vm->last_unlocked)) {
struct dma_fence *tmp = dma_fence_get_stub();
@@ -1256,6 +1277,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
error_free:
kfree(tlb_cb);
+error_critical:
amdgpu_vm_end_critical(¶ms);
drm_dev_exit(idx);
return r;
@@ -1270,6 +1292,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
* @start: start of unmapped range
* @last: last unmapped entry
* @flags: flags for the entries
+ * @eviction_lock: optional caller-held eviction lock
* @fence: optional resulting fence
*
* Fill in the page table entries between @start and @last with a fixed flags
@@ -1282,6 +1305,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_sync *sync, uint64_t start,
uint64_t last, uint64_t flags,
+ struct rw_semaphore *eviction_lock,
struct dma_fence **fence)
{
struct amdgpu_vm_tlb_seq_struct *tlb_cb;
@@ -1291,22 +1315,24 @@ int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
if (!drm_dev_enter(adev_to_drm(adev), &idx))
return -ENODEV;
- tlb_cb = kmalloc(sizeof(*tlb_cb), GFP_KERNEL);
- if (!tlb_cb) {
- drm_dev_exit(idx);
- return -ENOMEM;
- }
-
memset(¶ms, 0, sizeof(params));
params.adev = adev;
params.vm = vm;
params.needs_flush = true;
params.unlocked = true;
+ params.eviction_lock = eviction_lock;
INIT_LIST_HEAD(¶ms.tlb_flush_waitlist);
+ /* Enter noreclaim before allocating the small tlb_cb. */
r = amdgpu_vm_begin_critical(¶ms);
if (r)
- goto error_free;
+ goto error_critical;
+
+ tlb_cb = kmalloc_obj(*tlb_cb);
+ if (!tlb_cb) {
+ r = -ENOMEM;
+ goto error_critical;
+ }
r = vm->update_funcs->prepare(¶ms, sync,
AMDGPU_KERNEL_JOB_ID_VM_UNMAP_RANGE);
@@ -1325,6 +1351,7 @@ int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
error_free:
kfree(tlb_cb);
+error_critical:
amdgpu_vm_end_critical(¶ms);
drm_dev_exit(idx);
return r;
@@ -1462,7 +1489,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
mapping->start, mapping->last,
update_flags, mapping->offset,
vram_base, mem, pages_addr, NULL,
- last_update);
+ NULL, NULL, last_update);
if (r)
goto error_free;
}
@@ -1671,7 +1698,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
r = amdgpu_vm_map_range(adev, vm, true, false,
&sync, mapping->start, mapping->last,
- 0, 0, 0, NULL, NULL, NULL, &f);
+ 0, 0, 0, NULL, NULL, NULL,
+ NULL, NULL, &f);
amdgpu_vm_free_mapping(adev, vm, mapping, f);
if (r) {
dma_fence_put(f);
@@ -2851,7 +2879,6 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
amdgpu_svm_close(vm);
amdgpu_svm_fini(vm);
-
amdgpu_amdkfd_gpuvm_destroy_cb(adev, vm);
root = amdgpu_bo_ref(vm->root.bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index eac2632db0..5fdfed2f28 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -39,6 +39,7 @@
#include "amdgpu_ttm.h"
struct drm_exec;
+struct drm_gpusvm_range;
struct amdgpu_bo_va;
struct amdgpu_job;
@@ -284,7 +285,7 @@ struct amdgpu_vm {
/* tree of virtual addresses mapped */
struct rb_root_cached va;
- /* SVM can replace this pointer with its notifier lock. */
+ /* SVM uses gpusvm.notifier_lock, other VMs use default_eviction_lock. */
struct rw_semaphore default_eviction_lock;
struct rw_semaphore *eviction_lock;
bool evicting;
@@ -447,7 +448,8 @@ void amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job,
bool *need_pipe_sync, bool *emit_spm_needed,
bool *emit_gds_needed);
int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
- struct amdgpu_vm *vm, bool immediate);
+ struct amdgpu_vm *vm, bool immediate,
+ struct rw_semaphore *eviction_lock);
int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
struct dma_fence **fence);
@@ -467,10 +469,13 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
uint64_t vram_base, struct ttm_resource *res,
dma_addr_t *pages_addr,
struct amdgpu_hmm_range *hmm_range,
+ struct rw_semaphore *eviction_lock,
+ struct drm_gpusvm_range *gpusvm_range,
struct dma_fence **fence);
int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
struct amdgpu_sync *sync, uint64_t start,
uint64_t last, uint64_t flags,
+ struct rw_semaphore *eviction_lock,
struct dma_fence **fence);
int amdgpu_vm_bo_update(struct amdgpu_device *adev,
struct amdgpu_bo_va *bo_va,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index 2bd069c56c..1475b155d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -24,6 +24,7 @@
#ifndef __AMDGPU_VM_INTERNAL_H__
#define __AMDGPU_VM_INTERNAL_H__
+#include <drm/drm_gpusvm.h>
#include <linux/types.h>
#include <linux/list.h>
#include "amdgpu_hmm.h"
@@ -80,6 +81,12 @@ struct amdgpu_vm_update_params {
*/
struct amdgpu_hmm_range *hmm_range;
+ /** @eviction_lock: Optional caller-held eviction lock. */
+ struct rw_semaphore *eviction_lock;
+
+ /** @gpusvm_range: Optional SVM range to revalidate after taking the lock. */
+ struct drm_gpusvm_range *gpusvm_range;
+
/**
* @job: job to used for hw submission
*/
@@ -149,6 +156,7 @@ void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev);
*
* Serialize all updates, check parameters and make sure that memory allocations
* don't enter the reclaim path so that we don't deadlock with MMU notifiers.
+ * A non-NULL eviction_lock means the caller already holds the selected lock.
*
* Returns:
*
@@ -158,7 +166,9 @@ void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev);
static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
{
/* Use the write lock to serialize updates when the root BO is unlocked. */
- if (p->unlocked)
+ if (p->eviction_lock)
+ lockdep_assert_held(p->eviction_lock);
+ else if (p->unlocked)
down_write(p->vm->eviction_lock);
else
down_read(p->vm->eviction_lock);
@@ -168,6 +178,10 @@ static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
return -EBUSY;
if (p->hmm_range && !amdgpu_hmm_range_valid(p->hmm_range))
return -EAGAIN;
+ if (p->gpusvm_range &&
+ !drm_gpusvm_range_pages_valid(p->gpusvm_range->gpusvm,
+ p->gpusvm_range))
+ return -EAGAIN;
return 0;
}
@@ -175,15 +189,17 @@ static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
* amdgpu_vm_end_critical - end the critical section of the update
* @p: The update parameters
*
- * Restore the GFP flags and drop the lock.
+ * Restore the GFP flags.
*/
static inline void amdgpu_vm_end_critical(struct amdgpu_vm_update_params *p)
{
memalloc_noreclaim_restore(p->saved_flags);
- if (p->unlocked)
- up_write(p->vm->eviction_lock);
- else
- up_read(p->vm->eviction_lock);
+ if (!p->eviction_lock) {
+ if (p->unlocked)
+ up_write(p->vm->eviction_lock);
+ else
+ up_read(p->vm->eviction_lock);
+ }
}
#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 73786996c6..06492358d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -475,6 +475,23 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
return amdgpu_bo_create_vm(adev, &bp, vmbo);
}
+static void amdgpu_vm_update_unlock(struct amdgpu_vm_update_params *p)
+{
+ amdgpu_vm_end_critical(p);
+ if (p->eviction_lock) {
+ lockdep_assert_held_read(p->eviction_lock);
+ up_read(p->eviction_lock);
+ }
+}
+
+static int amdgpu_vm_update_relock(struct amdgpu_vm_update_params *p)
+{
+ if (p->eviction_lock)
+ down_read(p->eviction_lock);
+
+ return amdgpu_vm_begin_critical(p);
+}
+
/**
* amdgpu_vm_pt_alloc - Allocate a specific page table
*
@@ -498,19 +515,23 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
if (entry->bo)
return 0;
- amdgpu_vm_end_critical(p);
+ amdgpu_vm_update_unlock(p);
r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, &pt,
p->vm->root.bo->xcp_id);
- r2 = amdgpu_vm_begin_critical(p);
+ r2 = amdgpu_vm_update_relock(p);
if (r)
return r;
- if (r2)
+ pt_bo = &pt->bo;
+ if (r2) {
+ amdgpu_vm_update_unlock(p);
+ amdgpu_bo_unref(&pt_bo);
+ (void)amdgpu_vm_update_relock(p);
return r2;
+ }
/* Keep a reference to the root directory to avoid
* freeing them up in the wrong order.
*/
- pt_bo = &pt->bo;
pt_bo->parent = amdgpu_bo_ref(cursor->parent->bo);
amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
r = amdgpu_vm_pt_clear(p->adev, p->vm, pt);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 3a224dac7b..2a3baf7df1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1407,7 +1407,7 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
}
return amdgpu_vm_unmap_range(adev, vm, NULL, gpu_start, gpu_end,
- init_pte_value, fence);
+ init_pte_value, NULL, fence);
}
static int
@@ -1522,7 +1522,8 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
gpu_start, gpu_end, pte_flags,
(last_start - prange->start) << PAGE_SHIFT,
bo_adev ? bo_adev->vm_manager.vram_base_offset : 0,
- NULL, dma_addr, hmm_range, &vm->last_update);
+ NULL, dma_addr, hmm_range,
+ NULL, NULL, &vm->last_update);
for (j = last_start - prange->start; j <= i; j++)
dma_addr[j] |= last_domain;
@@ -1534,7 +1535,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
last_start = prange->start + i + 1;
}
- r = amdgpu_vm_update_pdes(adev, vm, false);
+ r = amdgpu_vm_update_pdes(adev, vm, false, NULL);
if (r) {
pr_debug("failed %d to update directories 0x%lx\n", r,
prange->start);
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [RFC PATCH 3/3] drm/amdgpu: use drm_gpusvm notifier lock for VM eviction
2026-08-30 15:40 [RFC PATCH 0/3] drm/amdgpu: unify SVM notifier and VM eviction locking Honglei Huang
2026-08-30 15:40 ` [RFC PATCH 1/3] drm/amdgpu: make VM eviction lock read/write to match notifier lock Honglei Huang
2026-08-30 15:40 ` [RFC PATCH 2/3] drm/amdgpu: recheck range validity after page table allocation Honglei Huang
@ 2026-08-30 15:40 ` Honglei Huang
2026-09-01 4:34 ` [RFC PATCH 0/3] drm/amdgpu: unify SVM notifier and VM eviction locking Huang, Honglei
3 siblings, 0 replies; 5+ messages in thread
From: Honglei Huang @ 2026-08-30 15:40 UTC (permalink / raw)
To: Christian.Koenig, Alexander.Deucher, Felix.Kuehling, Oak.Zeng,
Jenny-Jing.Liu, Philip.Yang, Xiaogang.Chen, Ray.Huang,
Junhua.Shen, timur.kristof, natalie.vock
Cc: amd-gfx, honghuan, Christian König
From: Honglei Huang <honghuan@amd.com>
SVM mapping updates, MMU notifier unmaps and VM eviction need to use the
same lock. Separate locks cannot guarantee the order of GPU page table
updates when CPU mappings are invalidated concurrently.
Point vm->eviction_lock at the drm_gpusvm notifier rwsem for SVM VMs.
Hold the read side across range validation, PTE and PDE updates, and
mapping publication. Use the write side for MMU notifier invalidation,
attribute PTE zaps and VM eviction.
Publish the selected lock before making vm->svm visible. During
teardown, reserve the root BO and remove the MMU notifiers while the
alias is still valid, then restore the default lock before releasing the
SVM context.
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Honglei Huang <honghuan@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c | 37 +++++++++----
drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c | 53 ++++++++-----------
2 files changed, 50 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c
index c0db597b1e..fcbe23cf7f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm.c
@@ -157,10 +157,13 @@ amdgpu_svm_lookup_by_pasid(struct amdgpu_device *adev, uint32_t pasid)
amdgpu_pasid_lock(&irqflags);
fpriv = amdgpu_pasid_get_fpriv_locked(pasid);
vm = fpriv ? &fpriv->vm : NULL;
- if (vm && vm->svm) {
- svm = vm->svm;
+ /*
+ * eviction_lock changes when SVM is enabled, so acquire vm->svm
+ * after the new lock alias is published.
+ */
+ svm = vm ? smp_load_acquire(&vm->svm) : NULL;
+ if (svm)
kref_get(&svm->refcount);
- }
amdgpu_pasid_unlock(irqflags);
return svm;
@@ -488,6 +491,8 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev,
struct amdgpu_svm *svm;
int ret;
+ dma_resv_assert_held(vm->root.bo->tbo.base.resv);
+
if (vm->svm)
return 0;
@@ -522,7 +527,9 @@ static int amdgpu_svm_init_compute(struct amdgpu_device *adev,
1UL << (svm->default_granularity + PAGE_SHIFT),
svm->xnack_enabled ? "enabled" : "disabled");
- vm->svm = svm;
+ vm->eviction_lock = &svm->gpusvm.notifier_lock;
+ /* Publish the context after its notifier lock alias. */
+ smp_store_release(&vm->svm, svm);
return 0;
err_free:
@@ -598,31 +605,43 @@ void amdgpu_svm_close(struct amdgpu_vm *vm)
* amdgpu_svm_fini() - Finalize and release a VM's SVM context
* @vm: The VM whose SVM context is being torn down.
*
- * Close the context, tear down the embedded drm_gpusvm under the SVM lock,
- * destroy the attribute tree and work queues, and drop the context
- * reference. Safe to call on a VM without an SVM context.
+ * Tear down the embedded drm_gpusvm, attribute tree and work queues, and drop
+ * the context reference.
*/
void amdgpu_svm_fini(struct amdgpu_vm *vm)
{
struct amdgpu_svm *svm = vm->svm;
+ struct amdgpu_bo *root;
if (!svm)
return;
amdgpu_svm_close(vm);
amdgpu_svm_lock(svm);
+
+ root = amdgpu_bo_ref(vm->root.bo);
+ amdgpu_bo_reserve(root, true);
+
drm_gpusvm_fini(&svm->gpusvm);
+
+ vm->eviction_lock = &vm->default_eviction_lock;
+ /* Publish the lock reset before making SVM unavailable. */
+ smp_store_release(&vm->svm, NULL);
+
amdgpu_svm_unlock(svm);
amdgpu_svm_attr_tree_destroy(svm->attr_tree);
amdgpu_svm_work_fini(svm);
- vm->svm = NULL;
amdgpu_svm_put(svm);
+
+ amdgpu_bo_unreserve(root);
+ amdgpu_bo_unref(&root);
}
bool amdgpu_svm_is_enabled(struct amdgpu_vm *vm)
{
- return vm->svm != NULL;
+ /* Pairs with publishing vm->svm after its lock alias is initialized. */
+ return smp_load_acquire(&vm->svm) != NULL;
}
static int amdgpu_svm_copy_attrs(const struct drm_amdgpu_gem_svm *args,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
index 3e622ac059..046910c2a0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_svm_range.c
@@ -65,16 +65,14 @@ amdgpu_svm_range_zap_ptes(struct amdgpu_svm *svm,
unsigned long last_page)
{
struct dma_fence *fence = NULL;
- unsigned int flags;
int ret;
if (last_page < start_page)
return 0;
- flags = memalloc_noreclaim_save();
- ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, NULL,
- start_page, last_page, 0, NULL, &fence);
- memalloc_noreclaim_restore(flags);
+ ret = amdgpu_vm_unmap_range(svm->adev, svm->vm, NULL, start_page,
+ last_page, 0, svm->vm->eviction_lock,
+ &fence);
if (fence) {
dma_fence_wait(fence, false);
@@ -210,20 +208,11 @@ amdgpu_svm_range_update_gpu_range(struct amdgpu_svm *svm,
mapped_pages += seg_pages;
is_last_seg = mapped_pages == npages;
- /*
- * The struct hmm_range hmm_range field inside amdgpu_hmm_range
- * is drm_gpusvm internal state. It is not exposed to the
- * implementing driver and is managed entirely by the framework
- * as part of the framework logic. This amdgpu_vm_map_range call
- * always runs inside the drm_gpusvm notifier, so omitting
- * hmm_range here is safe.
- */
- ret = amdgpu_vm_map_range(svm->adev, svm->vm,
- flush_tlb && is_last_seg, true, NULL,
- start_page, last_page, pte_flags,
- 0, entry->addr, NULL, NULL, NULL,
- NULL, NULL,
- wait_fence && is_last_seg ? fence : NULL);
+ ret = amdgpu_vm_map_range(
+ svm->adev, svm->vm, flush_tlb && is_last_seg, true,
+ NULL, start_page, last_page, pte_flags, 0, entry->addr,
+ NULL, NULL, NULL, svm->vm->eviction_lock, base,
+ wait_fence && is_last_seg ? fence : NULL);
if (ret)
return ret;
}
@@ -304,14 +293,12 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm,
{
struct drm_exec exec;
struct dma_fence *fence = NULL;
- unsigned int flags;
int ret;
ret = amdgpu_svm_range_lock_vm_pd(svm, &exec, intr);
if (ret)
return ret;
- flags = memalloc_noreclaim_save();
drm_gpusvm_notifier_lock(&svm->gpusvm);
if (!amdgpu_svm_range_pages_valid(svm, range)) {
@@ -323,24 +310,25 @@ int amdgpu_svm_range_update_mapping(struct amdgpu_svm *svm,
wait, wait ? &fence : NULL);
}
- drm_gpusvm_notifier_unlock(&svm->gpusvm);
- memalloc_noreclaim_restore(flags);
-
- if (!ret && fence)
- dma_fence_wait(fence, intr);
- dma_fence_put(fence);
-
if (!ret)
- ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false, NULL);
+ ret = amdgpu_vm_update_pdes(svm->adev, svm->vm, false,
+ svm->vm->eviction_lock);
if (!ret) {
- if (flush_tlb)
- amdgpu_svm_flush_tlb(svm);
WRITE_ONCE(range->attr_flags, attrs->flags);
WRITE_ONCE(range->gpu_mapped, true);
range->validate_timestamp = ktime_get_boottime();
}
+ drm_gpusvm_notifier_unlock(&svm->gpusvm);
+
+ if (!ret && fence)
+ dma_fence_wait(fence, intr);
+ dma_fence_put(fence);
+
+ if (!ret && flush_tlb)
+ amdgpu_svm_flush_tlb(svm);
+
drm_exec_fini(&exec);
return ret;
}
@@ -583,9 +571,12 @@ amdgpu_svm_range_invalidate_interval(struct amdgpu_svm *svm,
crosses_boundary ? "ATTR DESTROY" :
"ATTR ZAP PTE");
+ /* Use the unified eviction/MMU notifier lock for PTE zaps. */
+ down_write(svm->vm->eviction_lock);
ret = amdgpu_svm_range_zap_ptes(svm, svm_range,
drm_gpusvm_range_start(range) >> PAGE_SHIFT,
(drm_gpusvm_range_end(range) >> PAGE_SHIFT) - 1);
+ up_write(svm->vm->eviction_lock);
if (ret < 0) {
drm_exec_fini(&exec);
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread