[PATCH 1/9] drm/amdgpu: rework eviction lock handling into critical section v3

[PATCH 1/9] drm/amdgpu: rework eviction lock handling into critical section v3

[Christian König]

Taking the eviction lock is actually just one step which we need to do
in the critical section handling.

Rename the functions to reflect that, use the update parameters instead of the
vm to save the GFP flags.

v2: rebased and reordered
v3: fix rebase artefact, fix error handling in amdgpu_vm_pt_alloc

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Natalie Vock <nat@pixelcluster.dev>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  8 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  1 -
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   | 43 ++++++++++++++-----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 40 ++++++++---------
 4 files changed, 54 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 29a66e39f3d60..7b9494375649f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1170,11 +1170,9 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	params.override_pte = allow_override && adev->gmc.override_pte;
 	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
 
-	amdgpu_vm_eviction_lock(vm);
-	if (vm->evicting) {
-		r = -EBUSY;
+	r = amdgpu_vm_begin_critical(&params);
+	if (r)
 		goto error_free;
-	}
 
 	if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) {
 		struct dma_fence *tmp = dma_fence_get_stub();
@@ -1258,7 +1256,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 
 error_free:
 	kfree(tlb_cb);
-	amdgpu_vm_eviction_unlock(vm);
+	amdgpu_vm_end_critical(&params);
 	drm_dev_exit(idx);
 	return r;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 0f6634b749746..98cdd7e3475fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -289,7 +289,6 @@ struct amdgpu_vm {
 	 */
 	struct mutex		eviction_lock;
 	bool			evicting;
-	unsigned int		saved_flags;
 
 	/* Memory statistics for this vm, protected by stats_lock */
 	spinlock_t		stats_lock;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index ca86eaac75235..8ebb0b033291e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -51,7 +51,7 @@ struct amdgpu_vm_update_params {
 	struct amdgpu_device *adev;
 
 	/**
-	 * @vm: optional amdgpu_vm we do this update for
+	 * @vm: amdgpu_vm we do this update for
 	 */
 	struct amdgpu_vm *vm;
 
@@ -93,6 +93,11 @@ struct amdgpu_vm_update_params {
 	 */
 	bool override_pte;
 
+	/**
+	 * @saved_flags: Saved flags for GFP reduction.
+	 */
+	unsigned int saved_flags;
+
 	/**
 	 * @tlb_flush_waitlist: temporary storage for BOs until tlb_flush
 	 */
@@ -126,21 +131,37 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
 			    struct amdgpu_vm_update_params *params);
 int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 
-/*
- * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
- * happens while holding this lock anywhere to prevent deadlocks when
- * an MMU notifier runs in reclaim-FS context.
+/**
+ * amdgpu_vm_begin_critical - start the critical section of the update
+ * @p: The update parameters
+ *
+ * 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.
+ *
+ * Returns:
+ *
+ * 0 on success or a negative error code on failure.
+ * Even on error amdgpu_vm_end_critical() must still be called to clean up!
  */
-static inline void amdgpu_vm_eviction_lock(struct amdgpu_vm *vm)
+static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
 {
-	mutex_lock(&vm->eviction_lock);
-	vm->saved_flags = memalloc_noreclaim_save();
+	mutex_lock(&p->vm->eviction_lock);
+	p->saved_flags = memalloc_noreclaim_save();
+	if (p->vm->evicting)
+		return -EBUSY;
+	return 0;
 }
 
-static inline void amdgpu_vm_eviction_unlock(struct amdgpu_vm *vm)
+/**
+ * amdgpu_vm_end_critical - end the critical section of the update
+ * @p: The update parameters
+ *
+ * Restore the GFP flags and drop the lock.
+ */
+static inline void amdgpu_vm_end_critical(struct amdgpu_vm_update_params *p)
 {
-	memalloc_noreclaim_restore(vm->saved_flags);
-	mutex_unlock(&vm->eviction_lock);
+	memalloc_noreclaim_restore(p->saved_flags);
+	mutex_unlock(&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 1cdf2b854f261..c03327f1242d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -505,52 +505,51 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 /**
  * amdgpu_vm_pt_alloc - Allocate a specific page table
  *
- * @adev: amdgpu_device pointer
- * @vm: VM to allocate page tables for
+ * @p: see amdgpu_vm_update_params definition
  * @cursor: Which page table to allocate
- * @immediate: use an immediate update
  *
  * Make sure a specific page table or directory is allocated.
  *
  * Returns:
- * 1 if page table needed to be allocated, 0 if page table was already
- * allocated, negative errno if an error occurred.
+ *
+ * 0 on success or a negative error code on failure.
  */
-static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
-			      struct amdgpu_vm *vm,
-			      struct amdgpu_vm_pt_cursor *cursor,
-			      bool immediate)
+static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
+			      struct amdgpu_vm_pt_cursor *cursor)
 {
 	struct amdgpu_vm_bo_base *entry = cursor->entry;
 	struct amdgpu_bo *pt_bo;
 	struct amdgpu_bo_vm *pt;
-	int r;
+	int r, r2;
 
 	if (entry->bo)
 		return 0;
 
-	amdgpu_vm_eviction_unlock(vm);
-	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt,
-				vm->root.bo->xcp_id);
-	amdgpu_vm_eviction_lock(vm);
+	amdgpu_vm_end_critical(p);
+	r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, p->immediate,
+				&pt, p->vm->root.bo->xcp_id);
+	r2 = amdgpu_vm_begin_critical(p);
 	if (r)
 		return r;
+	if (r2)
+		goto error_free_pt;
 
 	/* 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, vm, pt_bo);
-	r = amdgpu_vm_pt_clear(adev, vm, pt, immediate);
+	amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
+	r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate);
 	if (r)
-		goto error_free_pt;
+		goto error_unpin;
 
 	return 0;
 
-error_free_pt:
-	if (vm->is_npa)
+error_unpin:
+	if (p->vm->is_npa)
 		amdgpu_bo_unpin(pt_bo);
+error_free_pt:
 	amdgpu_bo_unref(&pt_bo);
 	return r;
 }
@@ -838,8 +837,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
 			/* make sure that the page tables covering the
 			 * address range are actually allocated
 			 */
-			r = amdgpu_vm_pt_alloc(params->adev, params->vm,
-					       &cursor, params->immediate);
+			r = amdgpu_vm_pt_alloc(params, &cursor);
 			if (r)
 				return r;
 		}
-- 
2.43.0

[PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Christian König]

That was broken since adding the NPA support.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index c03327f1242d3..e8f441e018839 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device *adev,
 	     amdgpu_vm_pt_continue_dfs((start), (entry));			\
 	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), &(cursor)))
 
+/* Return the flags used for cleared PDEs/PTES */
+static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
+					struct amdgpu_vm *vm,
+					unsigned int level)
+{
+	uint64_t flags;
+
+	if (adev->asic_type < CHIP_VEGA10)
+		return 0;
+
+	if (level != AMDGPU_VM_PTB) {
+		uint64_t value = 0;
+
+		flags = AMDGPU_PDE_PTE_FLAG(adev);
+		if (vm->is_npa)
+			flags |= adev->gmc.noretry_flags;
+		amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
+	} else if (vm->is_npa) {
+		flags = adev->gmc.noretry_flags;
+	} else {
+		/* Workaround for fault priority problem on GMC9 */
+		flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
+	}
+
+	return flags;
+}
+
 /**
  * amdgpu_vm_pt_clear - initially clear the PDs/PTs
  *
@@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	struct ttm_operation_ctx ctx = { true, false };
 	struct amdgpu_vm_update_params params;
 	struct amdgpu_bo *ancestor = &vmbo->bo;
-	unsigned int entries;
 	struct amdgpu_bo *bo = &vmbo->bo;
-	uint64_t value = 0, flags = 0;
-	uint64_t addr;
+	unsigned int entries;
+	uint64_t flags;
 	int r, idx;
 
 	/* Figure out our place in the hierarchy */
@@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	if (r)
 		goto exit;
 
-	addr = 0;
-
-	if (adev->asic_type >= CHIP_VEGA10) {
-		if (level != AMDGPU_VM_PTB) {
-			if (vm->is_npa)
-				flags = adev->gmc.noretry_flags;
-			/* Handle leaf PDEs as PTEs */
-			flags |= AMDGPU_PDE_PTE_FLAG(adev);
-			amdgpu_gmc_get_vm_pde(adev, level,
-					      &value, &flags);
-		} else if (vm->is_npa) {
-			flags = adev->gmc.noretry_flags;
-		} else {
-			/* Workaround for fault priority problem on GMC9 */
-			flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
-		}
-	}
-
-	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
-				     value, flags);
+	flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
+	r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0, flags);
 	if (r)
 		goto exit;
 
@@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct amdgpu_vm_update_params *params,
 		flags |= AMDGPU_PDE_PTE_FLAG(params->adev);
 		amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
 
-	} else if (adev->asic_type >= CHIP_VEGA10 &&
-		   !(flags & AMDGPU_PTE_VALID) &&
+	} else if (!(flags & AMDGPU_PTE_VALID) &&
 		   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
-
-		/* Workaround for fault priority problem on GMC9 and GFX12,
-		 * EXECUTABLE for GMC9 fault priority and init_pte_flags
-		 * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
-		 */
-		flags |= AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
+		flags |= amdgpu_vm_pt_clear_flags(adev, params->vm, level);
 	}
 
 	/*
-- 
2.43.0

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Timur Kristóf]

On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian 
König wrote:
> That was broken since adding the NPA support.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>  1 file changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
> c03327f1242d3..e8f441e018839 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));			
\
>  	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), 
&(cursor)))
> 
> +/* Return the flags used for cleared PDEs/PTES */
> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
> +					struct amdgpu_vm *vm,
> +					unsigned int level)
> +{
> +	uint64_t flags;
> +
> +	if (adev->asic_type < CHIP_VEGA10)
> +		return 0;
> +
> +	if (level != AMDGPU_VM_PTB) {
> +		uint64_t value = 0;
> +
> +		flags = AMDGPU_PDE_PTE_FLAG(adev);
> +		if (vm->is_npa)
> +			flags |= adev->gmc.noretry_flags;
> +		amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
> +	} else if (vm->is_npa) {
> +		flags = adev->gmc.noretry_flags;
> +	} else {
> +		/* Workaround for fault priority problem on GMC9 */
> +		flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
> +	}
> +
> +	return flags;
> +}
> +
>  /**
>   * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>   *
> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>  	struct amdgpu_vm_update_params params;
>  	struct amdgpu_bo *ancestor = &vmbo->bo;
> -	unsigned int entries;
>  	struct amdgpu_bo *bo = &vmbo->bo;
> -	uint64_t value = 0, flags = 0;
> -	uint64_t addr;
> +	unsigned int entries;
> +	uint64_t flags;
>  	int r, idx;
> 
>  	/* Figure out our place in the hierarchy */
> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, if (r)
>  		goto exit;
> 
> -	addr = 0;
> -
> -	if (adev->asic_type >= CHIP_VEGA10) {
> -		if (level != AMDGPU_VM_PTB) {
> -			if (vm->is_npa)
> -				flags = adev->gmc.noretry_flags;
> -			/* Handle leaf PDEs as PTEs */
> -			flags |= AMDGPU_PDE_PTE_FLAG(adev);
> -			amdgpu_gmc_get_vm_pde(adev, level,
> -					      &value, &flags);
> -		} else if (vm->is_npa) {
> -			flags = adev->gmc.noretry_flags;
> -		} else {
> -			/* Workaround for fault priority problem on 
GMC9 */
> -			flags = AMDGPU_PTE_EXECUTABLE | adev-
>gmc.init_pte_flags;
> -		}
> -	}
> -
> -	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
> -				     value, flags);
> +	flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
> +	r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0, 
flags);
>  	if (r)
>  		goto exit;
> 
> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
> amdgpu_vm_update_params *params, flags |=
> AMDGPU_PDE_PTE_FLAG(params->adev);
>  		amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
> 
> -	} else if (adev->asic_type >= CHIP_VEGA10 &&
> -		   !(flags & AMDGPU_PTE_VALID) &&
> +	} else if (!(flags & AMDGPU_PTE_VALID) &&
>  		   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
> -
> -		/* Workaround for fault priority problem on GMC9 and 
GFX12,
> -		 * EXECUTABLE for GMC9 fault priority and init_pte_flags
> -		 * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
> -		 */
> -		flags |= AMDGPU_PTE_EXECUTABLE | adev-
>gmc.init_pte_flags;
> +		flags |= amdgpu_vm_pt_clear_flags(adev, params->vm, 
level);
>  	}
> 
>  	/*

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Christian König]

@Felix and @Philip any objections to this patch?

It is actually a bug fix for the NPA support.

Regards,
Christian.

On 9/28/26 21:08, Timur Kristóf wrote:
> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian 
> König wrote:
>> That was broken since adding the NPA support.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
> 
> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> 
>> ---
>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>  1 file changed, 33 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>> c03327f1242d3..e8f441e018839 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));			
> \
>>  	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), 
> &(cursor)))
>>
>> +/* Return the flags used for cleared PDEs/PTES */
>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>> +					struct amdgpu_vm *vm,
>> +					unsigned int level)
>> +{
>> +	uint64_t flags;
>> +
>> +	if (adev->asic_type < CHIP_VEGA10)
>> +		return 0;
>> +
>> +	if (level != AMDGPU_VM_PTB) {
>> +		uint64_t value = 0;
>> +
>> +		flags = AMDGPU_PDE_PTE_FLAG(adev);
>> +		if (vm->is_npa)
>> +			flags |= adev->gmc.noretry_flags;
>> +		amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>> +	} else if (vm->is_npa) {
>> +		flags = adev->gmc.noretry_flags;
>> +	} else {
>> +		/* Workaround for fault priority problem on GMC9 */
>> +		flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>> +	}
>> +
>> +	return flags;
>> +}
>> +
>>  /**
>>   * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>   *
>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>  	struct amdgpu_vm_update_params params;
>>  	struct amdgpu_bo *ancestor = &vmbo->bo;
>> -	unsigned int entries;
>>  	struct amdgpu_bo *bo = &vmbo->bo;
>> -	uint64_t value = 0, flags = 0;
>> -	uint64_t addr;
>> +	unsigned int entries;
>> +	uint64_t flags;
>>  	int r, idx;
>>
>>  	/* Figure out our place in the hierarchy */
>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>> struct amdgpu_vm *vm, if (r)
>>  		goto exit;
>>
>> -	addr = 0;
>> -
>> -	if (adev->asic_type >= CHIP_VEGA10) {
>> -		if (level != AMDGPU_VM_PTB) {
>> -			if (vm->is_npa)
>> -				flags = adev->gmc.noretry_flags;
>> -			/* Handle leaf PDEs as PTEs */
>> -			flags |= AMDGPU_PDE_PTE_FLAG(adev);
>> -			amdgpu_gmc_get_vm_pde(adev, level,
>> -					      &value, &flags);
>> -		} else if (vm->is_npa) {
>> -			flags = adev->gmc.noretry_flags;
>> -		} else {
>> -			/* Workaround for fault priority problem on 
> GMC9 */
>> -			flags = AMDGPU_PTE_EXECUTABLE | adev-
>> gmc.init_pte_flags;
>> -		}
>> -	}
>> -
>> -	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>> -				     value, flags);
>> +	flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>> +	r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0, 
> flags);
>>  	if (r)
>>  		goto exit;
>>
>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>> amdgpu_vm_update_params *params, flags |=
>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>  		amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>
>> -	} else if (adev->asic_type >= CHIP_VEGA10 &&
>> -		   !(flags & AMDGPU_PTE_VALID) &&
>> +	} else if (!(flags & AMDGPU_PTE_VALID) &&
>>  		   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>> -
>> -		/* Workaround for fault priority problem on GMC9 and 
> GFX12,
>> -		 * EXECUTABLE for GMC9 fault priority and init_pte_flags
>> -		 * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>> -		 */
>> -		flags |= AMDGPU_PTE_EXECUTABLE | adev-
>> gmc.init_pte_flags;
>> +		flags |= amdgpu_vm_pt_clear_flags(adev, params->vm, 
> level);
>>  	}
>>
>>  	/*
> 
> 
> 
> 

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Kuehling, Felix]

[+Mukul]

On 2026-09-30 05:02, Christian König wrote:
> @Felix and @Philip any objections to this patch?
>
> It is actually a bug fix for the NPA support.

As I understand it, this fixes the flags used when unmapping memory from 
the NPA VM. We passed adev->gmc.noretry_flags from 
amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in the 
flags parameter. I guess that's no longer needed with your fix. But I 
think the end result would be the same, right?

Regards,
   Felix


>
> Regards,
> Christian.
>
> On 9/28/26 21:08, Timur Kristóf wrote:
>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian
>> König wrote:
>>> That was broken since adding the NPA support.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>>   1 file changed, 33 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>>> c03327f1242d3..e8f441e018839 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));			
>> \
>>>   	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
>> &(cursor)))
>>> +/* Return the flags used for cleared PDEs/PTES */
>>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>>> +					struct amdgpu_vm *vm,
>>> +					unsigned int level)
>>> +{
>>> +	uint64_t flags;
>>> +
>>> +	if (adev->asic_type < CHIP_VEGA10)
>>> +		return 0;
>>> +
>>> +	if (level != AMDGPU_VM_PTB) {
>>> +		uint64_t value = 0;
>>> +
>>> +		flags = AMDGPU_PDE_PTE_FLAG(adev);
>>> +		if (vm->is_npa)
>>> +			flags |= adev->gmc.noretry_flags;
>>> +		amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>> +	} else if (vm->is_npa) {
>>> +		flags = adev->gmc.noretry_flags;
>>> +	} else {
>>> +		/* Workaround for fault priority problem on GMC9 */
>>> +		flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>>> +	}
>>> +
>>> +	return flags;
>>> +}
>>> +
>>>   /**
>>>    * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>>    *
>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>>   	struct amdgpu_vm_update_params params;
>>>   	struct amdgpu_bo *ancestor = &vmbo->bo;
>>> -	unsigned int entries;
>>>   	struct amdgpu_bo *bo = &vmbo->bo;
>>> -	uint64_t value = 0, flags = 0;
>>> -	uint64_t addr;
>>> +	unsigned int entries;
>>> +	uint64_t flags;
>>>   	int r, idx;
>>>
>>>   	/* Figure out our place in the hierarchy */
>>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>> struct amdgpu_vm *vm, if (r)
>>>   		goto exit;
>>>
>>> -	addr = 0;
>>> -
>>> -	if (adev->asic_type >= CHIP_VEGA10) {
>>> -		if (level != AMDGPU_VM_PTB) {
>>> -			if (vm->is_npa)
>>> -				flags = adev->gmc.noretry_flags;
>>> -			/* Handle leaf PDEs as PTEs */
>>> -			flags |= AMDGPU_PDE_PTE_FLAG(adev);
>>> -			amdgpu_gmc_get_vm_pde(adev, level,
>>> -					      &value, &flags);
>>> -		} else if (vm->is_npa) {
>>> -			flags = adev->gmc.noretry_flags;
>>> -		} else {
>>> -			/* Workaround for fault priority problem on
>> GMC9 */
>>> -			flags = AMDGPU_PTE_EXECUTABLE | adev-
>>> gmc.init_pte_flags;
>>> -		}
>>> -	}
>>> -
>>> -	r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>>> -				     value, flags);
>>> +	flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>>> +	r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
>> flags);
>>>   	if (r)
>>>   		goto exit;
>>>
>>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>>> amdgpu_vm_update_params *params, flags |=
>>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>>   		amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>>
>>> -	} else if (adev->asic_type >= CHIP_VEGA10 &&
>>> -		   !(flags & AMDGPU_PTE_VALID) &&
>>> +	} else if (!(flags & AMDGPU_PTE_VALID) &&
>>>   		   !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>>> -
>>> -		/* Workaround for fault priority problem on GMC9 and
>> GFX12,
>>> -		 * EXECUTABLE for GMC9 fault priority and init_pte_flags
>>> -		 * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>>> -		 */
>>> -		flags |= AMDGPU_PTE_EXECUTABLE | adev-
>>> gmc.init_pte_flags;
>>> +		flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
>> level);
>>>   	}
>>>
>>>   	/*
>>
>>
>>

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Christian König]

On 9/30/26 16:44, Kuehling, Felix wrote:
> [+Mukul]
> 
> On 2026-09-30 05:02, Christian König wrote:
>> @Felix and @Philip any objections to this patch?
>>
>> It is actually a bug fix for the NPA support.
> 
> As I understand it, this fixes the flags used when unmapping memory from the NPA VM. We passed adev->gmc.noretry_flags from amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in the flags parameter. I guess that's no longer needed with your fix. But I think the end result would be the same, right?

Not quite, passing adev->gmc.noretry_flags to amdgpu_vm_update_range() is completely broken as well and also need fixing.

The flags parameter to amdgpu_vm_update_range() can't contain the AMDGPU_PTE_TF nor the AMDGPU_PDE_PTE flag because those are overwritten by the PTE callbacks.

That's why setting the AMDGPU_PTE_IS_PTE for gfx12 through the init_pte_flags is completely broken as well.

We seriously need to stop doing such hacks.

Regards,
Christian.

> 
> Regards,
>   Felix
> 
> 
>>
>> Regards,
>> Christian.
>>
>> On 9/28/26 21:08, Timur Kristóf wrote:
>>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian
>>> König wrote:
>>>> That was broken since adding the NPA support.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>
>>>> ---
>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>>>   1 file changed, 33 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>>>> c03327f1242d3..e8f441e018839 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>>>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));           
>>> \
>>>>            (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
>>> &(cursor)))
>>>> +/* Return the flags used for cleared PDEs/PTES */
>>>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>>>> +                    struct amdgpu_vm *vm,
>>>> +                    unsigned int level)
>>>> +{
>>>> +    uint64_t flags;
>>>> +
>>>> +    if (adev->asic_type < CHIP_VEGA10)
>>>> +        return 0;
>>>> +
>>>> +    if (level != AMDGPU_VM_PTB) {
>>>> +        uint64_t value = 0;
>>>> +
>>>> +        flags = AMDGPU_PDE_PTE_FLAG(adev);
>>>> +        if (vm->is_npa)
>>>> +            flags |= adev->gmc.noretry_flags;
>>>> +        amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>>> +    } else if (vm->is_npa) {
>>>> +        flags = adev->gmc.noretry_flags;
>>>> +    } else {
>>>> +        /* Workaround for fault priority problem on GMC9 */
>>>> +        flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>>>> +    }
>>>> +
>>>> +    return flags;
>>>> +}
>>>> +
>>>>   /**
>>>>    * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>>>    *
>>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>>>       struct amdgpu_vm_update_params params;
>>>>       struct amdgpu_bo *ancestor = &vmbo->bo;
>>>> -    unsigned int entries;
>>>>       struct amdgpu_bo *bo = &vmbo->bo;
>>>> -    uint64_t value = 0, flags = 0;
>>>> -    uint64_t addr;
>>>> +    unsigned int entries;
>>>> +    uint64_t flags;
>>>>       int r, idx;
>>>>
>>>>       /* Figure out our place in the hierarchy */
>>>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>> struct amdgpu_vm *vm, if (r)
>>>>           goto exit;
>>>>
>>>> -    addr = 0;
>>>> -
>>>> -    if (adev->asic_type >= CHIP_VEGA10) {
>>>> -        if (level != AMDGPU_VM_PTB) {
>>>> -            if (vm->is_npa)
>>>> -                flags = adev->gmc.noretry_flags;
>>>> -            /* Handle leaf PDEs as PTEs */
>>>> -            flags |= AMDGPU_PDE_PTE_FLAG(adev);
>>>> -            amdgpu_gmc_get_vm_pde(adev, level,
>>>> -                          &value, &flags);
>>>> -        } else if (vm->is_npa) {
>>>> -            flags = adev->gmc.noretry_flags;
>>>> -        } else {
>>>> -            /* Workaround for fault priority problem on
>>> GMC9 */
>>>> -            flags = AMDGPU_PTE_EXECUTABLE | adev-
>>>> gmc.init_pte_flags;
>>>> -        }
>>>> -    }
>>>> -
>>>> -    r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>>>> -                     value, flags);
>>>> +    flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>>>> +    r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
>>> flags);
>>>>       if (r)
>>>>           goto exit;
>>>>
>>>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>>>> amdgpu_vm_update_params *params, flags |=
>>>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>>>           amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>>>
>>>> -    } else if (adev->asic_type >= CHIP_VEGA10 &&
>>>> -           !(flags & AMDGPU_PTE_VALID) &&
>>>> +    } else if (!(flags & AMDGPU_PTE_VALID) &&
>>>>              !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>>>> -
>>>> -        /* Workaround for fault priority problem on GMC9 and
>>> GFX12,
>>>> -         * EXECUTABLE for GMC9 fault priority and init_pte_flags
>>>> -         * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>>>> -         */
>>>> -        flags |= AMDGPU_PTE_EXECUTABLE | adev-
>>>> gmc.init_pte_flags;
>>>> +        flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
>>> level);
>>>>       }
>>>>
>>>>       /*
>>>
>>>
>>>

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Kuehling, Felix]

On 2026-09-30 10:59, Christian König wrote:
> On 9/30/26 16:44, Kuehling, Felix wrote:
>> [+Mukul]
>>
>> On 2026-09-30 05:02, Christian König wrote:
>>> @Felix and @Philip any objections to this patch?
>>>
>>> It is actually a bug fix for the NPA support.
>> As I understand it, this fixes the flags used when unmapping memory from the NPA VM. We passed adev->gmc.noretry_flags from amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in the flags parameter. I guess that's no longer needed with your fix. But I think the end result would be the same, right?
> Not quite, passing adev->gmc.noretry_flags to amdgpu_vm_update_range() is completely broken as well and also need fixing.
>
> The flags parameter to amdgpu_vm_update_range() can't contain the AMDGPU_PTE_TF nor the AMDGPU_PDE_PTE flag because those are overwritten by the PTE callbacks.

I'm pretty sure Mukul tested this and got something that worked as 
expected, at least for a time. I'm not sure if it regressed since, maybe 
during upstreaming. @Mukul, can you comment?


>
> That's why setting the AMDGPU_PTE_IS_PTE for gfx12 through the init_pte_flags is completely broken as well.
>
> We seriously need to stop doing such hacks.

Is there any documentation that tells us what flags can be used where. 
This isn't obvious at all.

Thanks,
   Felix


>
> Regards,
> Christian.
>
>> Regards,
>>    Felix
>>
>>
>>> Regards,
>>> Christian.
>>>
>>> On 9/28/26 21:08, Timur Kristóf wrote:
>>>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian
>>>> König wrote:
>>>>> That was broken since adding the NPA support.
>>>>>
>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>
>>>>> ---
>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>>>>    1 file changed, 33 insertions(+), 31 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>>>>> c03327f1242d3..e8f441e018839 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>>>>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));
>>>> \
>>>>>             (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
>>>> &(cursor)))
>>>>> +/* Return the flags used for cleared PDEs/PTES */
>>>>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>>>>> +                    struct amdgpu_vm *vm,
>>>>> +                    unsigned int level)
>>>>> +{
>>>>> +    uint64_t flags;
>>>>> +
>>>>> +    if (adev->asic_type < CHIP_VEGA10)
>>>>> +        return 0;
>>>>> +
>>>>> +    if (level != AMDGPU_VM_PTB) {
>>>>> +        uint64_t value = 0;
>>>>> +
>>>>> +        flags = AMDGPU_PDE_PTE_FLAG(adev);
>>>>> +        if (vm->is_npa)
>>>>> +            flags |= adev->gmc.noretry_flags;
>>>>> +        amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>>>> +    } else if (vm->is_npa) {
>>>>> +        flags = adev->gmc.noretry_flags;
>>>>> +    } else {
>>>>> +        /* Workaround for fault priority problem on GMC9 */
>>>>> +        flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>>>>> +    }
>>>>> +
>>>>> +    return flags;
>>>>> +}
>>>>> +
>>>>>    /**
>>>>>     * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>>>>     *
>>>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>>>>        struct amdgpu_vm_update_params params;
>>>>>        struct amdgpu_bo *ancestor = &vmbo->bo;
>>>>> -    unsigned int entries;
>>>>>        struct amdgpu_bo *bo = &vmbo->bo;
>>>>> -    uint64_t value = 0, flags = 0;
>>>>> -    uint64_t addr;
>>>>> +    unsigned int entries;
>>>>> +    uint64_t flags;
>>>>>        int r, idx;
>>>>>
>>>>>        /* Figure out our place in the hierarchy */
>>>>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>>> struct amdgpu_vm *vm, if (r)
>>>>>            goto exit;
>>>>>
>>>>> -    addr = 0;
>>>>> -
>>>>> -    if (adev->asic_type >= CHIP_VEGA10) {
>>>>> -        if (level != AMDGPU_VM_PTB) {
>>>>> -            if (vm->is_npa)
>>>>> -                flags = adev->gmc.noretry_flags;
>>>>> -            /* Handle leaf PDEs as PTEs */
>>>>> -            flags |= AMDGPU_PDE_PTE_FLAG(adev);
>>>>> -            amdgpu_gmc_get_vm_pde(adev, level,
>>>>> -                          &value, &flags);
>>>>> -        } else if (vm->is_npa) {
>>>>> -            flags = adev->gmc.noretry_flags;
>>>>> -        } else {
>>>>> -            /* Workaround for fault priority problem on
>>>> GMC9 */
>>>>> -            flags = AMDGPU_PTE_EXECUTABLE | adev-
>>>>> gmc.init_pte_flags;
>>>>> -        }
>>>>> -    }
>>>>> -
>>>>> -    r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>>>>> -                     value, flags);
>>>>> +    flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>>>>> +    r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
>>>> flags);
>>>>>        if (r)
>>>>>            goto exit;
>>>>>
>>>>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>>>>> amdgpu_vm_update_params *params, flags |=
>>>>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>>>>            amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>>>>
>>>>> -    } else if (adev->asic_type >= CHIP_VEGA10 &&
>>>>> -           !(flags & AMDGPU_PTE_VALID) &&
>>>>> +    } else if (!(flags & AMDGPU_PTE_VALID) &&
>>>>>               !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>>>>> -
>>>>> -        /* Workaround for fault priority problem on GMC9 and
>>>> GFX12,
>>>>> -         * EXECUTABLE for GMC9 fault priority and init_pte_flags
>>>>> -         * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>>>>> -         */
>>>>> -        flags |= AMDGPU_PTE_EXECUTABLE | adev-
>>>>> gmc.init_pte_flags;
>>>>> +        flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
>>>> level);
>>>>>        }
>>>>>
>>>>>        /*
>>>>
>>>>

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Christian König]

On 9/30/26 18:12, Kuehling, Felix wrote:
> On 2026-09-30 10:59, Christian König wrote:
>> On 9/30/26 16:44, Kuehling, Felix wrote:
>>> [+Mukul]
>>>
>>> On 2026-09-30 05:02, Christian König wrote:
>>>> @Felix and @Philip any objections to this patch?
>>>>
>>>> It is actually a bug fix for the NPA support.
>>> As I understand it, this fixes the flags used when unmapping memory from the NPA VM. We passed adev->gmc.noretry_flags from amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in the flags parameter. I guess that's no longer needed with your fix. But I think the end result would be the same, right?
>> Not quite, passing adev->gmc.noretry_flags to amdgpu_vm_update_range() is completely broken as well and also need fixing.
>>
>> The flags parameter to amdgpu_vm_update_range() can't contain the AMDGPU_PTE_TF nor the AMDGPU_PDE_PTE flag because those are overwritten by the PTE callbacks.
> 
> I'm pretty sure Mukul tested this and got something that worked as expected, at least for a time. I'm not sure if it regressed since, maybe during upstreaming. @Mukul, can you comment?

It could be that this works by coincident, e.g. that we set the flag but never clear it. But it is definately not a good idea to rely on that.

>>
>> That's why setting the AMDGPU_PTE_IS_PTE for gfx12 through the init_pte_flags is completely broken as well.
>>
>> We seriously need to stop doing such hacks.
> 
> Is there any documentation that tells us what flags can be used where. This isn't obvious at all.

No, I mean I thought that this would be obvious.

The AMDGPU_PTE_TF, AMDGPU_PDE_PTE and AMDGPU_PTE_IS_PTE control the walker behavior.

So we need to set them depending on what the VM code decides on which layer PDEs and on which layer PTEs are.

I will add some comment to the code.

Thanks,
Christian.

> 
> Thanks,
>   Felix
> 
> 
>>
>> Regards,
>> Christian.
>>
>>> Regards,
>>>    Felix
>>>
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> On 9/28/26 21:08, Timur Kristóf wrote:
>>>>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári idő Christian
>>>>> König wrote:
>>>>>> That was broken since adding the NPA support.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>>
>>>>>> ---
>>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64 ++++++++++++-----------
>>>>>>    1 file changed, 33 insertions(+), 31 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>>>>>> c03327f1242d3..e8f441e018839 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device
>>>>>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));
>>>>> \
>>>>>>             (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
>>>>> &(cursor)))
>>>>>> +/* Return the flags used for cleared PDEs/PTES */
>>>>>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
>>>>>> +                    struct amdgpu_vm *vm,
>>>>>> +                    unsigned int level)
>>>>>> +{
>>>>>> +    uint64_t flags;
>>>>>> +
>>>>>> +    if (adev->asic_type < CHIP_VEGA10)
>>>>>> +        return 0;
>>>>>> +
>>>>>> +    if (level != AMDGPU_VM_PTB) {
>>>>>> +        uint64_t value = 0;
>>>>>> +
>>>>>> +        flags = AMDGPU_PDE_PTE_FLAG(adev);
>>>>>> +        if (vm->is_npa)
>>>>>> +            flags |= adev->gmc.noretry_flags;
>>>>>> +        amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>>>>> +    } else if (vm->is_npa) {
>>>>>> +        flags = adev->gmc.noretry_flags;
>>>>>> +    } else {
>>>>>> +        /* Workaround for fault priority problem on GMC9 */
>>>>>> +        flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>>>>>> +    }
>>>>>> +
>>>>>> +    return flags;
>>>>>> +}
>>>>>> +
>>>>>>    /**
>>>>>>     * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>>>>>     *
>>>>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>>>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true, false };
>>>>>>        struct amdgpu_vm_update_params params;
>>>>>>        struct amdgpu_bo *ancestor = &vmbo->bo;
>>>>>> -    unsigned int entries;
>>>>>>        struct amdgpu_bo *bo = &vmbo->bo;
>>>>>> -    uint64_t value = 0, flags = 0;
>>>>>> -    uint64_t addr;
>>>>>> +    unsigned int entries;
>>>>>> +    uint64_t flags;
>>>>>>        int r, idx;
>>>>>>
>>>>>>        /* Figure out our place in the hierarchy */
>>>>>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
>>>>>> struct amdgpu_vm *vm, if (r)
>>>>>>            goto exit;
>>>>>>
>>>>>> -    addr = 0;
>>>>>> -
>>>>>> -    if (adev->asic_type >= CHIP_VEGA10) {
>>>>>> -        if (level != AMDGPU_VM_PTB) {
>>>>>> -            if (vm->is_npa)
>>>>>> -                flags = adev->gmc.noretry_flags;
>>>>>> -            /* Handle leaf PDEs as PTEs */
>>>>>> -            flags |= AMDGPU_PDE_PTE_FLAG(adev);
>>>>>> -            amdgpu_gmc_get_vm_pde(adev, level,
>>>>>> -                          &value, &flags);
>>>>>> -        } else if (vm->is_npa) {
>>>>>> -            flags = adev->gmc.noretry_flags;
>>>>>> -        } else {
>>>>>> -            /* Workaround for fault priority problem on
>>>>> GMC9 */
>>>>>> -            flags = AMDGPU_PTE_EXECUTABLE | adev-
>>>>>> gmc.init_pte_flags;
>>>>>> -        }
>>>>>> -    }
>>>>>> -
>>>>>> -    r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>>>>>> -                     value, flags);
>>>>>> +    flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>>>>>> +    r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
>>>>> flags);
>>>>>>        if (r)
>>>>>>            goto exit;
>>>>>>
>>>>>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>>>>>> amdgpu_vm_update_params *params, flags |=
>>>>>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>>>>>            amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>>>>>
>>>>>> -    } else if (adev->asic_type >= CHIP_VEGA10 &&
>>>>>> -           !(flags & AMDGPU_PTE_VALID) &&
>>>>>> +    } else if (!(flags & AMDGPU_PTE_VALID) &&
>>>>>>               !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>>>>>> -
>>>>>> -        /* Workaround for fault priority problem on GMC9 and
>>>>> GFX12,
>>>>>> -         * EXECUTABLE for GMC9 fault priority and init_pte_flags
>>>>>> -         * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>>>>>> -         */
>>>>>> -        flags |= AMDGPU_PTE_EXECUTABLE | adev-
>>>>>> gmc.init_pte_flags;
>>>>>> +        flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
>>>>> level);
>>>>>>        }
>>>>>>
>>>>>>        /*
>>>>>
>>>>>

Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Mukul Joshi]

On 9/30/2026 12:12 PM, Kuehling, Felix wrote:
> On 2026-09-30 10:59, Christian König wrote:
>> On 9/30/26 16:44, Kuehling, Felix wrote:
>>> [+Mukul]
>>>
>>> On 2026-09-30 05:02, Christian König wrote:
>>>> @Felix and @Philip any objections to this patch?
>>>>
>>>> It is actually a bug fix for the NPA support.
>>> As I understand it, this fixes the flags used when unmapping memory
>>> from the NPA VM. We passed adev->gmc.noretry_flags from
>>> amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in
>>> the flags parameter. I guess that's no longer needed with your fix.
>>> But I think the end result would be the same, right?
>> Not quite, passing adev->gmc.noretry_flags to
>> amdgpu_vm_update_range() is completely broken as well and also need
>> fixing.
>>
>> The flags parameter to amdgpu_vm_update_range() can't contain the
>> AMDGPU_PTE_TF nor the AMDGPU_PDE_PTE flag because those are
>> overwritten by the PTE callbacks.
>
> I'm pretty sure Mukul tested this and got something that worked as
> expected, at least for a time. I'm not sure if it regressed since,
> maybe during upstreaming. @Mukul, can you comment?
>
yes we have been testing this and it works fine. Having said that, the
patch looks good to me.

However, I have a question on one of the change in the patch below.


>
>>
>> That's why setting the AMDGPU_PTE_IS_PTE for gfx12 through the
>> init_pte_flags is completely broken as well.
>>
>> We seriously need to stop doing such hacks.
>
> Is there any documentation that tells us what flags can be used where.
> This isn't obvious at all.
>
> Thanks,
>   Felix
>
>
>>
>> Regards,
>> Christian.
>>
>>> Regards,
>>>    Felix
>>>
>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> On 9/28/26 21:08, Timur Kristóf wrote:
>>>>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári
>>>>> idő Christian
>>>>> König wrote:
>>>>>> That was broken since adding the NPA support.
>>>>>>
>>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>>
>>>>>> ---
>>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64
>>>>>> ++++++++++++-----------
>>>>>>    1 file changed, 33 insertions(+), 31 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
>>>>>> c03327f1242d3..e8f441e018839 100644
>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
>>>>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct
>>>>>> amdgpu_device
>>>>>> *adev, amdgpu_vm_pt_continue_dfs((start), (entry));
>>>>> \
>>>>>>             (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev),
>>>>> &(cursor)))
>>>>>> +/* Return the flags used for cleared PDEs/PTES */
>>>>>> +static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device
>>>>>> *adev,
>>>>>> +                    struct amdgpu_vm *vm,
>>>>>> +                    unsigned int level)
>>>>>> +{
>>>>>> +    uint64_t flags;
>>>>>> +
>>>>>> +    if (adev->asic_type < CHIP_VEGA10)
>>>>>> +        return 0;
>>>>>> +
>>>>>> +    if (level != AMDGPU_VM_PTB) {
>>>>>> +        uint64_t value = 0;
>>>>>> +
>>>>>> +        flags = AMDGPU_PDE_PTE_FLAG(adev);
>>>>>> +        if (vm->is_npa)
>>>>>> +            flags |= adev->gmc.noretry_flags;
>>>>>> +        amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
>>>>>> +    } else if (vm->is_npa) {
>>>>>> +        flags = adev->gmc.noretry_flags;
>>>>>> +    } else {
>>>>>> +        /* Workaround for fault priority problem on GMC9 */
>>>>>> +        flags = AMDGPU_PTE_EXECUTABLE | adev->gmc.init_pte_flags;
>>>>>> +    }
>>>>>> +
>>>>>> +    return flags;
>>>>>> +}
>>>>>> +
>>>>>>    /**
>>>>>>     * amdgpu_vm_pt_clear - initially clear the PDs/PTs
>>>>>>     *
>>>>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device
>>>>>> *adev,
>>>>>> struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = { true,
>>>>>> false };
>>>>>>        struct amdgpu_vm_update_params params;
>>>>>>        struct amdgpu_bo *ancestor = &vmbo->bo;
>>>>>> -    unsigned int entries;
>>>>>>        struct amdgpu_bo *bo = &vmbo->bo;
>>>>>> -    uint64_t value = 0, flags = 0;
>>>>>> -    uint64_t addr;
>>>>>> +    unsigned int entries;
>>>>>> +    uint64_t flags;
>>>>>>        int r, idx;
>>>>>>
>>>>>>        /* Figure out our place in the hierarchy */
>>>>>> @@ -404,26 +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device
>>>>>> *adev,
>>>>>> struct amdgpu_vm *vm, if (r)
>>>>>>            goto exit;
>>>>>>
>>>>>> -    addr = 0;
>>>>>> -
>>>>>> -    if (adev->asic_type >= CHIP_VEGA10) {
>>>>>> -        if (level != AMDGPU_VM_PTB) {
>>>>>> -            if (vm->is_npa)
>>>>>> -                flags = adev->gmc.noretry_flags;
>>>>>> -            /* Handle leaf PDEs as PTEs */
>>>>>> -            flags |= AMDGPU_PDE_PTE_FLAG(adev);
>>>>>> -            amdgpu_gmc_get_vm_pde(adev, level,
>>>>>> -                          &value, &flags);
>>>>>> -        } else if (vm->is_npa) {
>>>>>> -            flags = adev->gmc.noretry_flags;
>>>>>> -        } else {
>>>>>> -            /* Workaround for fault priority problem on
>>>>> GMC9 */
>>>>>> -            flags = AMDGPU_PTE_EXECUTABLE | adev-
>>>>>> gmc.init_pte_flags;
>>>>>> -        }
>>>>>> -    }
>>>>>> -
>>>>>> -    r = vm->update_funcs->update(&params, vmbo, addr, 0, entries,
>>>>>> -                     value, flags);
>>>>>> +    flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
>>>>>> +    r = vm->update_funcs->update(&params, vmbo, 0, 0, entries, 0,
>>>>> flags);

We were passing addr before to the update() call but with this change,
we are passing 0 now for the addr argument now.

Why is that?

Regards,

Mukul

>>>>>>        if (r)
>>>>>>            goto exit;
>>>>>>
>>>>>> @@ -712,15 +720,9 @@ static void amdgpu_vm_pte_update_flags(struct
>>>>>> amdgpu_vm_update_params *params, flags |=
>>>>>> AMDGPU_PDE_PTE_FLAG(params->adev);
>>>>>>            amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
>>>>>>
>>>>>> -    } else if (adev->asic_type >= CHIP_VEGA10 &&
>>>>>> -           !(flags & AMDGPU_PTE_VALID) &&
>>>>>> +    } else if (!(flags & AMDGPU_PTE_VALID) &&
>>>>>>               !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
>>>>>> -
>>>>>> -        /* Workaround for fault priority problem on GMC9 and
>>>>> GFX12,
>>>>>> -         * EXECUTABLE for GMC9 fault priority and init_pte_flags
>>>>>> -         * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
>>>>>> -         */
>>>>>> -        flags |= AMDGPU_PTE_EXECUTABLE | adev-
>>>>>> gmc.init_pte_flags;
>>>>>> +        flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
>>>>> level);
>>>>>>        }
>>>>>>
>>>>>>        /*
>>>>>
>>>>>

RE: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation

[Joshi, Mukul]

AMD General

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Mukul
> Joshi
> Sent: Thursday, October 1, 2026 9:36 AM
> To: Kuehling, Felix <Felix.Kuehling@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Timur Kristóf <timur.kristof@gmail.com>;
> natalie.vock@gmx.de; Huang, Honglei1 <Honglei1.Huang@amd.com>; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Yang, Philip
> <Philip.Yang@amd.com>; cascardo@igalia.com; tvrtko.ursulin@igalia.com
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 2/9] drm/amdgpu: fix cleared PDE/PTE flag generation
>
>
> On 9/30/2026 12:12 PM, Kuehling, Felix wrote:
> > On 2026-09-30 10:59, Christian König wrote:
> >> On 9/30/26 16:44, Kuehling, Felix wrote:
> >>> [+Mukul]
> >>>
> >>> On 2026-09-30 05:02, Christian König wrote:
> >>>> @Felix and @Philip any objections to this patch?
> >>>>
> >>>> It is actually a bug fix for the NPA support.
> >>> As I understand it, this fixes the flags used when unmapping memory
> >>> from the NPA VM. We passed adev->gmc.noretry_flags from
> >>> amdgpu_ualink_unmap_npa_addr to amdgpu_vm_update_range explicitly in
> >>> the flags parameter. I guess that's no longer needed with your fix.
> >>> But I think the end result would be the same, right?
> >> Not quite, passing adev->gmc.noretry_flags to
> >> amdgpu_vm_update_range() is completely broken as well and also need
> >> fixing.
> >>
> >> The flags parameter to amdgpu_vm_update_range() can't contain the
> >> AMDGPU_PTE_TF nor the AMDGPU_PDE_PTE flag because those are
> >> overwritten by the PTE callbacks.
> >
> > I'm pretty sure Mukul tested this and got something that worked as
> > expected, at least for a time. I'm not sure if it regressed since,
> > maybe during upstreaming. @Mukul, can you comment?
> >
> yes we have been testing this and it works fine. Having said that, the patch looks
> good to me.
>
> However, I have a question on one of the change in the patch below.
>
>
> >
> >>
> >> That's why setting the AMDGPU_PTE_IS_PTE for gfx12 through the
> >> init_pte_flags is completely broken as well.
> >>
> >> We seriously need to stop doing such hacks.
> >
> > Is there any documentation that tells us what flags can be used where.
> > This isn't obvious at all.
> >
> > Thanks,
> >   Felix
> >
> >
> >>
> >> Regards,
> >> Christian.
> >>
> >>> Regards,
> >>>    Felix
> >>>
> >>>
> >>>> Regards,
> >>>> Christian.
> >>>>
> >>>> On 9/28/26 21:08, Timur Kristóf wrote:
> >>>>> On 2026. szeptember 28., hétfő 11:10:34 keleti államokbeli nyári
> >>>>> idő Christian König wrote:
> >>>>>> That was broken since adding the NPA support.
> >>>>>>
> >>>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
> >>>>> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> >>>>>
> >>>>>> ---
> >>>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 64
> >>>>>> ++++++++++++-----------
> >>>>>>    1 file changed, 33 insertions(+), 31 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> >>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
> >>>>>> c03327f1242d3..e8f441e018839 100644
> >>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> >>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> >>>>>> @@ -346,6 +346,33 @@ static void amdgpu_vm_pt_next_dfs(struct
> >>>>>> amdgpu_device *adev, amdgpu_vm_pt_continue_dfs((start), (entry));
> >>>>> \
> >>>>>>             (entry) = (cursor).entry,
> >>>>>> amdgpu_vm_pt_next_dfs((adev),
> >>>>> &(cursor)))
> >>>>>> +/* Return the flags used for cleared PDEs/PTES */ static
> >>>>>> +uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device
> >>>>>> *adev,
> >>>>>> +                    struct amdgpu_vm *vm,
> >>>>>> +                    unsigned int level) {
> >>>>>> +    uint64_t flags;
> >>>>>> +
> >>>>>> +    if (adev->asic_type < CHIP_VEGA10)
> >>>>>> +        return 0;
> >>>>>> +
> >>>>>> +    if (level != AMDGPU_VM_PTB) {
> >>>>>> +        uint64_t value = 0;
> >>>>>> +
> >>>>>> +        flags = AMDGPU_PDE_PTE_FLAG(adev);
> >>>>>> +        if (vm->is_npa)
> >>>>>> +            flags |= adev->gmc.noretry_flags;
> >>>>>> +        amdgpu_gmc_get_vm_pde(adev, level, &value, &flags);
> >>>>>> +    } else if (vm->is_npa) {
> >>>>>> +        flags = adev->gmc.noretry_flags;
> >>>>>> +    } else {
> >>>>>> +        /* Workaround for fault priority problem on GMC9 */
> >>>>>> +        flags = AMDGPU_PTE_EXECUTABLE |
> >>>>>> +adev->gmc.init_pte_flags;
> >>>>>> +    }
> >>>>>> +
> >>>>>> +    return flags;
> >>>>>> +}
> >>>>>> +
> >>>>>>    /**
> >>>>>>     * amdgpu_vm_pt_clear - initially clear the PDs/PTs
> >>>>>>     *
> >>>>>> @@ -366,10 +393,9 @@ int amdgpu_vm_pt_clear(struct amdgpu_device
> >>>>>> *adev, struct amdgpu_vm *vm, struct ttm_operation_ctx ctx = {
> >>>>>> true, false };
> >>>>>>        struct amdgpu_vm_update_params params;
> >>>>>>        struct amdgpu_bo *ancestor = &vmbo->bo;
> >>>>>> -    unsigned int entries;
> >>>>>>        struct amdgpu_bo *bo = &vmbo->bo;
> >>>>>> -    uint64_t value = 0, flags = 0;
> >>>>>> -    uint64_t addr;
> >>>>>> +    unsigned int entries;
> >>>>>> +    uint64_t flags;
> >>>>>>        int r, idx;
> >>>>>>
> >>>>>>        /* Figure out our place in the hierarchy */ @@ -404,26
> >>>>>> +430,8 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev,
> >>>>>> struct amdgpu_vm *vm, if (r)
> >>>>>>            goto exit;
> >>>>>>
> >>>>>> -    addr = 0;
> >>>>>> -
> >>>>>> -    if (adev->asic_type >= CHIP_VEGA10) {
> >>>>>> -        if (level != AMDGPU_VM_PTB) {
> >>>>>> -            if (vm->is_npa)
> >>>>>> -                flags = adev->gmc.noretry_flags;
> >>>>>> -            /* Handle leaf PDEs as PTEs */
> >>>>>> -            flags |= AMDGPU_PDE_PTE_FLAG(adev);
> >>>>>> -            amdgpu_gmc_get_vm_pde(adev, level,
> >>>>>> -                          &value, &flags);
> >>>>>> -        } else if (vm->is_npa) {
> >>>>>> -            flags = adev->gmc.noretry_flags;
> >>>>>> -        } else {
> >>>>>> -            /* Workaround for fault priority problem on
> >>>>> GMC9 */
> >>>>>> -            flags = AMDGPU_PTE_EXECUTABLE | adev-
> >>>>>> gmc.init_pte_flags;
> >>>>>> -        }
> >>>>>> -    }
> >>>>>> -
> >>>>>> -    r = vm->update_funcs->update(&params, vmbo, addr, 0,
> >>>>>> entries,
> >>>>>> -                     value, flags);
> >>>>>> +    flags = amdgpu_vm_pt_clear_flags(adev, vm, level);
> >>>>>> +    r = vm->update_funcs->update(&params, vmbo, 0, 0, entries,
> >>>>>> +0,
> >>>>> flags);
>
> We were passing addr before to the update() call but with this change, we are
> passing 0 now for the addr argument now.
>

Sorry I meant to ask about the v"alue" args. Earlier, we would get the value from amdgpu_gmc_get_vm_pde()
But now we are ignoring it and sending the value args as 0.

Why is that? Should we keep it the same as before?

Regards,
Mukul

> Why is that?
>
> Regards,
>
> Mukul
>
> >>>>>>        if (r)
> >>>>>>            goto exit;
> >>>>>>
> >>>>>> @@ -712,15 +720,9 @@ static void
> >>>>>> amdgpu_vm_pte_update_flags(struct amdgpu_vm_update_params
> >>>>>> *params, flags |= AMDGPU_PDE_PTE_FLAG(params->adev);
> >>>>>>            amdgpu_gmc_get_vm_pde(adev, level, &addr, &flags);
> >>>>>>
> >>>>>> -    } else if (adev->asic_type >= CHIP_VEGA10 &&
> >>>>>> -           !(flags & AMDGPU_PTE_VALID) &&
> >>>>>> +    } else if (!(flags & AMDGPU_PTE_VALID) &&
> >>>>>>               !(flags & AMDGPU_PTE_PRT_FLAG(params->adev))) {
> >>>>>> -
> >>>>>> -        /* Workaround for fault priority problem on GMC9 and
> >>>>> GFX12,
> >>>>>> -         * EXECUTABLE for GMC9 fault priority and init_pte_flags
> >>>>>> -         * (e.g. AMDGPU_PTE_IS_PTE on GFX12)
> >>>>>> -         */
> >>>>>> -        flags |= AMDGPU_PTE_EXECUTABLE | adev-
> >>>>>> gmc.init_pte_flags;
> >>>>>> +        flags |= amdgpu_vm_pt_clear_flags(adev, params->vm,
> >>>>> level);
> >>>>>>        }
> >>>>>>
> >>>>>>        /*
> >>>>>
> >>>>>

[PATCH 3/9] drm/amdgpu: allocate and fill dummy PDs/PTs

[Christian König]

Allocate some PDs/PTs which just point to the dummy page.

Those can be used in page faults to redirect recoverable page faults
to the dummy page.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 11 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  7 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |  2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 57 +++++++++++++++++++
 4 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 7b9494375649f..f02a99b753c22 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2908,8 +2908,10 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm)
  *
  * Initialize the VM manager structures
  */
-void amdgpu_vm_manager_init(struct amdgpu_device *adev)
+int amdgpu_vm_manager_init(struct amdgpu_device *adev)
 {
+	int r;
+
 	/* Concurrent flushes are only possible starting with Vega10 and
 	 * are broken on Navi10 and Navi14.
 	 */
@@ -2921,6 +2923,10 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 	spin_lock_init(&adev->vm_manager.prt_lock);
 	atomic_set(&adev->vm_manager.num_prt_users, 0);
 
+	r = amdgpu_vm_pt_alloc_dummies(adev);
+	if (r)
+		return r;
+
 	/* If not overridden by the user, by default, only in large BAR systems
 	 * Compute VM tables will be updated by CPU
 	 */
@@ -2940,6 +2946,8 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 #else
 	adev->vm_manager.vm_update_mode = 0;
 #endif
+
+	return 0;
 }
 
 /**
@@ -2951,6 +2959,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
  */
 void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
 {
+	amdgpu_vm_pt_free_dummies(adev);
 	amdgpu_vmid_mgr_fini(adev);
 	amdgpu_pasid_mgr_cleanup();
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 98cdd7e3475fb..c59647554b416 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -411,8 +411,11 @@ struct amdgpu_vm_manager {
 	int					vm_update_mode;
 
 	/* Global registration of recent page fault information */
-	struct amdgpu_vm_fault_info	fault_info;
+	struct amdgpu_vm_fault_info		fault_info;
 	unsigned int				npa_vmid;
+
+	struct amdgpu_bo			*dummy_pd[AMDGPU_VM_PTB + 1];
+	uint64_t				dummy_dst[AMDGPU_VM_PTB + 1];
 };
 
 struct amdgpu_bo_va_mapping;
@@ -424,7 +427,7 @@ struct amdgpu_bo_va_mapping;
 extern const struct amdgpu_vm_update_funcs amdgpu_vm_cpu_funcs;
 extern const struct amdgpu_vm_update_funcs amdgpu_vm_sdma_funcs;
 
-void amdgpu_vm_manager_init(struct amdgpu_device *adev);
+int amdgpu_vm_manager_init(struct amdgpu_device *adev);
 void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
 
 long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index 8ebb0b033291e..3c48a3401e2a4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -130,6 +130,8 @@ void amdgpu_vm_pt_free_work(struct work_struct *work);
 void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
 			    struct amdgpu_vm_update_params *params);
 int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm *vm);
+int amdgpu_vm_pt_alloc_dummies(struct amdgpu_device *adev);
+void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev);
 
 /**
  * amdgpu_vm_begin_critical - start the critical section of the update
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index e8f441e018839..285f17c7705b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -990,3 +990,60 @@ int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 
 	return 0;
 }
+
+/* amdgpu_vm_pt_alloc_dummies - allocate dummy PDs/PTs
+ *
+ * @adev: the amdgpu device pointer
+ *
+ * Allocate some dummy PDs/PTs which can be used to redirect page faults to the
+ * dummy page.
+ */
+int amdgpu_vm_pt_alloc_dummies(struct amdgpu_device *adev)
+{
+	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
+	int r;
+
+	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; level--) {
+		size_t size =  amdgpu_vm_pt_size(adev, level);
+		uint64_t addr, flags;
+		void *ptr;
+
+		r = amdgpu_bo_create_kernel(adev, size, 0,
+					    AMDGPU_GEM_DOMAIN_VRAM,
+					    &vm_mgr->dummy_pd[level],
+					    &vm_mgr->dummy_dst[level],
+					    &ptr);
+		if (r)
+			return r;
+
+		if (level == AMDGPU_VM_PTB) {
+			addr = adev->dummy_page_addr;
+			/*
+			 * TODO: We want to have separate dummies for reads and
+			 * writes.
+			 */
+			flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
+				AMDGPU_PTE_SYSTEM | AMDGPU_PTE_EXECUTABLE |
+				AMDGPU_PTE_READABLE | AMDGPU_PTE_WRITEABLE;
+		} else {
+			amdgpu_gmc_get_pde_for_bo(vm_mgr->dummy_pd[level + 1],
+						  level, &addr, &flags);
+		}
+
+		for (int i = 0; i < amdgpu_vm_pt_num_entries(adev, level); i++)
+			amdgpu_gmc_set_pte_pde(adev, ptr, i, addr, flags);
+	}
+
+	return 0;
+}
+
+void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev)
+{
+	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
+	void *ptr;
+
+	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; level--)
+		amdgpu_bo_free_kernel(&vm_mgr->dummy_pd[level],
+				      &vm_mgr->dummy_dst[level],
+				      &ptr);
+}
-- 
2.43.0

Re: [PATCH 3/9] drm/amdgpu: allocate and fill dummy PDs/PTs

[Timur Kristóf]

On 2026. szeptember 28., hétfő 11:10:35 keleti államokbeli nyári idő Christian 
König wrote:
> Allocate some PDs/PTs which just point to the dummy page.
> 
> Those can be used in page faults to redirect recoverable page faults
> to the dummy page.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 11 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  7 ++-
>  .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |  2 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 57 +++++++++++++++++++
>  4 files changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 7b9494375649f..f02a99b753c22
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2908,8 +2908,10 @@ void amdgpu_vm_fini(struct amdgpu_device *adev,
> struct amdgpu_vm *vm) *
>   * Initialize the VM manager structures
>   */
> -void amdgpu_vm_manager_init(struct amdgpu_device *adev)
> +int amdgpu_vm_manager_init(struct amdgpu_device *adev)
>  {
> +	int r;
> +
>  	/* Concurrent flushes are only possible starting with Vega10 and
>  	 * are broken on Navi10 and Navi14.
>  	 */
> @@ -2921,6 +2923,10 @@ void amdgpu_vm_manager_init(struct amdgpu_device
> *adev) spin_lock_init(&adev->vm_manager.prt_lock);
>  	atomic_set(&adev->vm_manager.num_prt_users, 0);
> 
> +	r = amdgpu_vm_pt_alloc_dummies(adev);
> +	if (r)
> +		return r;
> +
>  	/* If not overridden by the user, by default, only in large BAR 
systems
>  	 * Compute VM tables will be updated by CPU
>  	 */
> @@ -2940,6 +2946,8 @@ void amdgpu_vm_manager_init(struct amdgpu_device
> *adev) #else
>  	adev->vm_manager.vm_update_mode = 0;
>  #endif
> +
> +	return 0;
>  }
> 
>  /**
> @@ -2951,6 +2959,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device
> *adev) */
>  void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
>  {
> +	amdgpu_vm_pt_free_dummies(adev);
>  	amdgpu_vmid_mgr_fini(adev);
>  	amdgpu_pasid_mgr_cleanup();
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 98cdd7e3475fb..c59647554b416
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -411,8 +411,11 @@ struct amdgpu_vm_manager {
>  	int					
vm_update_mode;
> 
>  	/* Global registration of recent page fault information */
> -	struct amdgpu_vm_fault_info	fault_info;
> +	struct amdgpu_vm_fault_info		fault_info;
>  	unsigned int				npa_vmid;
> +
> +	struct amdgpu_bo			*dummy_pd[AMDGPU_VM_PTB 
+ 1];
> +	uint64_t				
dummy_dst[AMDGPU_VM_PTB + 1];
>  };
> 
>  struct amdgpu_bo_va_mapping;
> @@ -424,7 +427,7 @@ struct amdgpu_bo_va_mapping;
>  extern const struct amdgpu_vm_update_funcs amdgpu_vm_cpu_funcs;
>  extern const struct amdgpu_vm_update_funcs amdgpu_vm_sdma_funcs;
> 
> -void amdgpu_vm_manager_init(struct amdgpu_device *adev);
> +int amdgpu_vm_manager_init(struct amdgpu_device *adev);
>  void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
> 
>  long amdgpu_vm_wait_idle(struct amdgpu_vm *vm, long timeout);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index
> 8ebb0b033291e..3c48a3401e2a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -130,6 +130,8 @@ void amdgpu_vm_pt_free_work(struct work_struct *work);
>  void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
>  			    struct amdgpu_vm_update_params *params);
>  int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm
> *vm); +int amdgpu_vm_pt_alloc_dummies(struct amdgpu_device *adev);
> +void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev);
> 
>  /**
>   * amdgpu_vm_begin_critical - start the critical section of the update
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
> e8f441e018839..285f17c7705b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -990,3 +990,60 @@ int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev,
> struct amdgpu_vm *vm)
> 
>  	return 0;
>  }
> +
> +/* amdgpu_vm_pt_alloc_dummies - allocate dummy PDs/PTs
> + *
> + * @adev: the amdgpu device pointer
> + *
> + * Allocate some dummy PDs/PTs which can be used to redirect page faults to
> the + * dummy page.
> + */
> +int amdgpu_vm_pt_alloc_dummies(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
> +	int r;
> +
> +	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; 
level--) {
> +		size_t size =  amdgpu_vm_pt_size(adev, level);
> +		uint64_t addr, flags;
> +		void *ptr;
> +
> +		r = amdgpu_bo_create_kernel(adev, size, 0,
> +					    
AMDGPU_GEM_DOMAIN_VRAM,
> +					    &vm_mgr-
>dummy_pd[level],
> +					    &vm_mgr-
>dummy_dst[level],
> +					    &ptr);
> +		if (r)
> +			return r;
> +
> +		if (level == AMDGPU_VM_PTB) {
> +			addr = adev->dummy_page_addr;
> +			/*
> +			 * TODO: We want to have separate dummies for 
reads and
> +			 * writes.
> +			 */
> +			flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED 
|
> +				AMDGPU_PTE_SYSTEM | 
AMDGPU_PTE_EXECUTABLE |
> +				AMDGPU_PTE_READABLE | 
AMDGPU_PTE_WRITEABLE;
> +		} else {
> +			amdgpu_gmc_get_pde_for_bo(vm_mgr-
>dummy_pd[level + 1],
> +						  level, 
&addr, &flags);
> +		}

The AMDGPU_PTE_IS_PTE flag is missing for GFX12.

I think we should either set that flag for GFX12 here, or use init_pte_flags.
Otherwise retry faults will regress on GFX12 after this refactor.


> +
> +		for (int i = 0; i < amdgpu_vm_pt_num_entries(adev, 
level); i++)
> +			amdgpu_gmc_set_pte_pde(adev, ptr, i, addr, 
flags);
> +	}
> +
> +	return 0;
> +}
> +
> +void amdgpu_vm_pt_free_dummies(struct amdgpu_device *adev)
> +{
> +	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
> +	void *ptr;
> +
> +	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; 
level--)
> +		amdgpu_bo_free_kernel(&vm_mgr->dummy_pd[level],
> +				      &vm_mgr->dummy_dst[level],
> +				      &ptr);
> +}

[PATCH 4/9] drm/amdgpu: add amdgpu_vm_pt_leaves() v2

[Christian König]

Add a new function amdgpu_vm_update_leaves() to avoid memory allocation
on page faults.

The idea is to only update the leave PDEs/PTEs to let them point to the
dummy page.

v2: fix of by one, rework the function to work correctly on PTB as well.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 54 ++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  5 +-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |  3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 74 +++++++++++++++++++
 4 files changed, 119 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index f02a99b753c22..d6358cbfcc9b1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -3078,10 +3078,12 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
 			    u32 vmid, u32 node_id, uint64_t addr,
 			    uint64_t ts, bool write_fault)
 {
+	struct amdgpu_vm_update_params params;
 	bool is_compute_context = false;
-	struct drm_exec exec;
-	uint64_t value, flags;
+	uint64_t *dst, flags[AMDGPU_VM_MAX_LEVEL];
 	struct amdgpu_vm *vm;
+	struct drm_exec exec;
+	unsigned int idx;
 	int r;
 
 	drm_exec_init(&exec, 0, 1);
@@ -3125,24 +3127,24 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
 	}
 
 	addr /= AMDGPU_GPU_PAGE_SIZE;
-	flags = adev->gmc.init_pte_flags |
-		AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
-		AMDGPU_PTE_SYSTEM;
-
 	if (is_compute_context) {
 		/* Intentionally setting invalid PTE flag
 		 * combination to force a no-retry-fault
 		 */
-		flags = AMDGPU_VM_NORETRY_FLAGS;
-		value = 0;
+		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
+			flags[i] = AMDGPU_VM_NORETRY_FLAGS;
+		dst = NULL;
 	} else if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
 		/* Redirect the access to the dummy page */
-		value = adev->dummy_page_addr;
-		flags |= AMDGPU_PTE_EXECUTABLE | AMDGPU_PTE_READABLE |
-			 AMDGPU_PTE_WRITEABLE;
+		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
+			flags[i] = 0;
+		dst = adev->vm_manager.dummy_dst;
 	} else {
 		/* Let the hw retry silently on the PTE */
-		value = 0;
+		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
+			flags[i] = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
+				AMDGPU_PTE_SYSTEM;
+		dst = NULL;
 	}
 
 	r = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
@@ -3151,12 +3153,32 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
 		goto error_unlock;
 	}
 
-	r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
-				   NULL, addr, addr, flags, value, 0, NULL, NULL, NULL);
-	if (r)
+	if (!drm_dev_enter(adev_to_drm(adev), &idx)) {
+		r = -ENODEV;
 		goto error_unlock;
+	}
+
+	memset(&params, 0, sizeof(params));
+	params.adev = adev;
+	params.vm = vm;
+	params.immediate = true;
+
+	r = amdgpu_vm_begin_critical(&params);
+	if (r)
+		goto error_end_critical;
+
+	r = vm->update_funcs->prepare(&params, NULL,
+				      AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
+	if (r)
+		goto error_end_critical;
 
-	r = amdgpu_vm_update_pdes(adev, vm, true);
+	amdgpu_vm_pt_leaves(&params, addr, addr + 1, dst, flags);
+
+	r = vm->update_funcs->commit(&params, &vm->last_update);
+
+error_end_critical:
+	amdgpu_vm_end_critical(&params);
+	drm_dev_exit(idx);
 
 error_unlock:
 	drm_exec_fini(&exec);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index c59647554b416..ec5cd38fe4e34 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -195,7 +195,10 @@ enum amdgpu_vm_level {
 	AMDGPU_VM_PDB2,
 	AMDGPU_VM_PDB1,
 	AMDGPU_VM_PDB0,
-	AMDGPU_VM_PTB
+	AMDGPU_VM_PTB,
+
+	/* Not HW level, but for array sizing */
+	AMDGPU_VM_MAX_LEVEL
 };
 
 /* base structure for tracking BO usage in a VM */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index 3c48a3401e2a4..dafdb3a001b8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -126,6 +126,9 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params,
 int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
 			  uint64_t start, uint64_t end,
 			  uint64_t dst, uint64_t flags);
+void amdgpu_vm_pt_leaves(struct amdgpu_vm_update_params *params,
+			 uint64_t start, uint64_t end,
+			 int64_t *dst, uint64_t *flags);
 void amdgpu_vm_pt_free_work(struct work_struct *work);
 void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
 			    struct amdgpu_vm_update_params *params);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 285f17c7705b4..27003b03b5fdb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -963,6 +963,80 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
 	return 0;
 }
 
+/**
+ * amdgpu_vm_pt_leaves - update leaf PDEs/PTEs
+ *
+ * @params: see amdgpu_vm_update_params definition
+ * @start: start of GPU address range
+ * @end: end of GPU address range
+ * @dst: optional array with one dst addr per layer
+ * @flags: array of mapping flags per layer
+ *
+ * Update the leaf PDEs/PTEs in the range @start - @end without allocating or
+ * freeing page tables.
+ *
+ * Returns:
+ * 0 for success, negative error code for failure.
+ */
+void amdgpu_vm_pt_leaves(struct amdgpu_vm_update_params *params,
+			 uint64_t start, uint64_t end,
+			 int64_t *dst, uint64_t *flags)
+{
+	struct amdgpu_device *adev = params->adev;
+	struct amdgpu_vm_pt_cursor cursor;
+
+	amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
+	while (cursor.pfn < end) {
+		unsigned int level, shift, mask, nptes;
+		uint64_t pe_start, entry_start, entry_end, d, f;
+		struct amdgpu_bo *pt;
+
+		/* Walk to the leave entries */
+		if (amdgpu_vm_pt_descendant(adev, &cursor))
+			continue;
+
+		if (cursor.entry->bo) {
+			level = cursor.level;
+			pt = cursor.entry->bo;
+		} else {
+			level = cursor.level - 1;
+			pt = cursor.parent->bo;
+		}
+
+		shift = amdgpu_vm_pt_level_shift(adev, level);
+		mask = amdgpu_vm_pt_entries_mask(adev, level);
+
+		/* Looks good so far, calculate parameters for the update */
+		pe_start = ((cursor.pfn >> shift) & mask) * 8;
+
+		entry_start = cursor.pfn;
+		if (cursor.entry->bo) {
+			entry_end = ((uint64_t)mask + 1) << shift;
+			entry_end += cursor.pfn & ~(entry_end - 1);
+			entry_end = min(entry_end, end);
+
+			nptes = (entry_end - cursor.pfn) >> shift;
+			amdgpu_vm_pt_next(adev, &cursor);
+		} else {
+			nptes = 0;
+			entry_end = cursor.pfn;
+			do {
+				nptes += 1;
+				entry_end += 1 << shift;
+				amdgpu_vm_pt_next(adev, &cursor);
+			} while (cursor.parent && cursor.parent->bo == pt &&
+				 cursor.pfn < end && !cursor.entry->bo);
+		}
+
+		d = dst ? dst[level] : 0;
+		f = flags[level];
+		trace_amdgpu_vm_update_ptes(params, entry_start, entry_end,
+					    min(nptes, 32u), d, 0, f);
+		params->vm->update_funcs->update(params, to_amdgpu_bo_vm(pt),
+						 pe_start, d, nptes, 0, f);
+	}
+}
+
 /**
  * amdgpu_vm_pt_map_tables - have bo of root PD cpu accessible
  * @adev: amdgpu device structure
-- 
2.43.0

Re: [PATCH 4/9] drm/amdgpu: add amdgpu_vm_pt_leaves() v2

[Timur Kristóf]

On 2026. szeptember 28., hétfő 11:10:36 keleti államokbeli nyári idő Christian 
König wrote:
> Add a new function amdgpu_vm_update_leaves() to avoid memory allocation
> on page faults.
> 
> The idea is to only update the leave PDEs/PTEs to let them point to the
> dummy page.
> 
> v2: fix of by one, rework the function to work correctly on PTB as well.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 54 ++++++++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  5 +-
>  .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |  3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 74 +++++++++++++++++++
>  4 files changed, 119 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index f02a99b753c22..d6358cbfcc9b1
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -3078,10 +3078,12 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> *adev, u32 pasid, u32 vmid, u32 node_id, uint64_t addr,
>  			    uint64_t ts, bool write_fault)
>  {
> +	struct amdgpu_vm_update_params params;
>  	bool is_compute_context = false;
> -	struct drm_exec exec;
> -	uint64_t value, flags;
> +	uint64_t *dst, flags[AMDGPU_VM_MAX_LEVEL];
>  	struct amdgpu_vm *vm;
> +	struct drm_exec exec;
> +	unsigned int idx;
>  	int r;
> 
>  	drm_exec_init(&exec, 0, 1);
> @@ -3125,24 +3127,24 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> *adev, u32 pasid, }
> 
>  	addr /= AMDGPU_GPU_PAGE_SIZE;
> -	flags = adev->gmc.init_pte_flags |
> -		AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
> -		AMDGPU_PTE_SYSTEM;
> -
>  	if (is_compute_context) {
>  		/* Intentionally setting invalid PTE flag
>  		 * combination to force a no-retry-fault
>  		 */
> -		flags = AMDGPU_VM_NORETRY_FLAGS;
> -		value = 0;
> +		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
> +			flags[i] = AMDGPU_VM_NORETRY_FLAGS;
> +		dst = NULL;
>  	} else if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
>  		/* Redirect the access to the dummy page */
> -		value = adev->dummy_page_addr;
> -		flags |= AMDGPU_PTE_EXECUTABLE | AMDGPU_PTE_READABLE |
> -			 AMDGPU_PTE_WRITEABLE;
> +		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
> +			flags[i] = 0;
> +		dst = adev->vm_manager.dummy_dst;
>  	} else {
>  		/* Let the hw retry silently on the PTE */
> -		value = 0;
> +		for (int i = 0; i < AMDGPU_VM_MAX_LEVEL; ++i)
> +			flags[i] = AMDGPU_PTE_VALID | 
AMDGPU_PTE_SNOOPED |
> +				AMDGPU_PTE_SYSTEM;
> +		dst = NULL;
>  	}
> 
>  	r = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
> @@ -3151,12 +3153,32 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> *adev, u32 pasid, goto error_unlock;
>  	}
> 
> -	r = amdgpu_vm_update_range(adev, vm, true, false, false, false,
> -				   NULL, addr, addr, flags, value, 
0, NULL, NULL, NULL);
> -	if (r)
> +	if (!drm_dev_enter(adev_to_drm(adev), &idx)) {
> +		r = -ENODEV;
>  		goto error_unlock;
> +	}
> +
> +	memset(&params, 0, sizeof(params));
> +	params.adev = adev;
> +	params.vm = vm;
> +	params.immediate = true;
> +
> +	r = amdgpu_vm_begin_critical(&params);
> +	if (r)
> +		goto error_end_critical;
> +
> +	r = vm->update_funcs->prepare(&params, NULL,
> +				      
AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
> +	if (r)
> +		goto error_end_critical;
> 
> -	r = amdgpu_vm_update_pdes(adev, vm, true);
> +	amdgpu_vm_pt_leaves(&params, addr, addr + 1, dst, flags);
> +
> +	r = vm->update_funcs->commit(&params, &vm->last_update);

We shouldn't overwrite vm->last_update here.
You can just pass NULL here for now.

> +
> +error_end_critical:
> +	amdgpu_vm_end_critical(&params);
> +	drm_dev_exit(idx);
> 
>  error_unlock:
>  	drm_exec_fini(&exec);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index c59647554b416..ec5cd38fe4e34
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -195,7 +195,10 @@ enum amdgpu_vm_level {
>  	AMDGPU_VM_PDB2,
>  	AMDGPU_VM_PDB1,
>  	AMDGPU_VM_PDB0,
> -	AMDGPU_VM_PTB
> +	AMDGPU_VM_PTB,
> +
> +	/* Not HW level, but for array sizing */
> +	AMDGPU_VM_MAX_LEVEL

Instead of AMDGPU_VM_MAX_LEVEL this should be called AMDGPU_VM_NUM_LEVELS

>  };
> 
>  /* base structure for tracking BO usage in a VM */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index
> 3c48a3401e2a4..dafdb3a001b8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -126,6 +126,9 @@ int amdgpu_vm_pde_update(struct amdgpu_vm_update_params
> *params, int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
> uint64_t start, uint64_t end,
>  			  uint64_t dst, uint64_t flags);
> +void amdgpu_vm_pt_leaves(struct amdgpu_vm_update_params *params,
> +			 uint64_t start, uint64_t end,
> +			 int64_t *dst, uint64_t *flags);
>  void amdgpu_vm_pt_free_work(struct work_struct *work);
>  void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
>  			    struct amdgpu_vm_update_params *params);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c index
> 285f17c7705b4..27003b03b5fdb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -963,6 +963,80 @@ int amdgpu_vm_ptes_update(struct
> amdgpu_vm_update_params *params, return 0;
>  }
> 
> +/**
> + * amdgpu_vm_pt_leaves - update leaf PDEs/PTEs
> + *
> + * @params: see amdgpu_vm_update_params definition
> + * @start: start of GPU address range
> + * @end: end of GPU address range
> + * @dst: optional array with one dst addr per layer
> + * @flags: array of mapping flags per layer
> + *
> + * Update the leaf PDEs/PTEs in the range @start - @end without allocating
> or + * freeing page tables.
> + *
> + * Returns:
> + * 0 for success, negative error code for failure.
> + */
> +void amdgpu_vm_pt_leaves(struct amdgpu_vm_update_params *params,
> +			 uint64_t start, uint64_t end,
> +			 int64_t *dst, uint64_t *flags)
> +{
> +	struct amdgpu_device *adev = params->adev;
> +	struct amdgpu_vm_pt_cursor cursor;
> +
> +	amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
> +	while (cursor.pfn < end) {
> +		unsigned int level, shift, mask, nptes;
> +		uint64_t pe_start, entry_start, entry_end, d, f;
> +		struct amdgpu_bo *pt;
> +
> +		/* Walk to the leave entries */
> +		if (amdgpu_vm_pt_descendant(adev, &cursor))
> +			continue;
> +
> +		if (cursor.entry->bo) {
> +			level = cursor.level;
> +			pt = cursor.entry->bo;
> +		} else {
> +			level = cursor.level - 1;
> +			pt = cursor.parent->bo;
> +		}
> +
> +		shift = amdgpu_vm_pt_level_shift(adev, level);
> +		mask = amdgpu_vm_pt_entries_mask(adev, level);
> +
> +		/* Looks good so far, calculate parameters for the 
update */
> +		pe_start = ((cursor.pfn >> shift) & mask) * 8;
> +
> +		entry_start = cursor.pfn;
> +		if (cursor.entry->bo) {
> +			entry_end = ((uint64_t)mask + 1) << shift;
> +			entry_end += cursor.pfn & ~(entry_end - 1);
> +			entry_end = min(entry_end, end);
> +
> +			nptes = (entry_end - cursor.pfn) >> shift;
> +			amdgpu_vm_pt_next(adev, &cursor);
> +		} else {
> +			nptes = 0;
> +			entry_end = cursor.pfn;
> +			do {
> +				nptes += 1;
> +				entry_end += 1 << shift;
> +				amdgpu_vm_pt_next(adev, &cursor);
> +			} while (cursor.parent && cursor.parent->bo 
== pt &&
> +				 cursor.pfn < end && !
cursor.entry->bo);
> +		}
> +
> +		d = dst ? dst[level] : 0;
> +		f = flags[level];
> +		trace_amdgpu_vm_update_ptes(params, entry_start, 
entry_end,
> +					    min(nptes, 32u), d, 
0, f);
> +		params->vm->update_funcs->update(params, 
to_amdgpu_bo_vm(pt),
> +						 pe_start, 
d, nptes, 0, f);
> +	}
> +}
> +
>  /**
>   * amdgpu_vm_pt_map_tables - have bo of root PD cpu accessible
>   * @adev: amdgpu device structure

[PATCH 5/9] drm/amdgpu: drop immediate updates from amdgpu_vm_update_range

[Christian König]

That case is handled by amdgpu_vm_update_leaves now.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c    | 42 +++++++++----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 21 ++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        | 11 +++--
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |  5 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 13 ++----
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c          |  4 +-
 6 files changed, 43 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 6e937f31a5b1a..aac97dca36e33 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -2056,9 +2056,9 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
 		return r;
 	}
 
-	r = amdgpu_vm_update_range(adev, vm, false, false, true,
-				false, NULL, npa_addr, npa_addr + size - 1,
-				pte_value, 0, 0, NULL, NULL, &vm->last_update);
+	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
+				   npa_addr + size - 1, pte_value, 0, 0, NULL,
+				   NULL, &vm->last_update);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed to unmap NPA addr (%llx) from NPA VM\n", npa_addr);
@@ -2108,10 +2108,10 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
 		return r;
 	}
 
-	r = amdgpu_vm_update_range(adev, vm, false, false, true,
-				false, NULL, npa_addr, npa_addr + size - 1,
-				pte_flags, offset, adev->vm_manager.vram_base_offset,
-				bo->tbo.resource, NULL, &vm->last_update);
+	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
+				   npa_addr + size - 1, pte_flags, offset,
+				   adev->vm_manager.vram_base_offset,
+				   bo->tbo.resource, NULL, &vm->last_update);
 	if (r) {
 		dev_warn(adev->dev,
 			"Failed to map NPA addr (%llx) into NPA VM\n", npa_addr);
@@ -2645,10 +2645,12 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
 			"RETRY-RPC: setting PTE.X=1 for NPA:%llx remote:%u pte:0x%llx\n",
 			npa_addr, remote_acc_id, pte_flags);
 
-		r = amdgpu_vm_update_range(adev, vm, false, false, true,
-					false, NULL, npa_addr, npa_addr + size - 1,
-					pte_flags, 0, adev->vm_manager.vram_base_offset,
-					bo->tbo.resource, NULL, &vm->last_update);
+		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
+					   npa_addr, npa_addr + size - 1,
+					   pte_flags, 0,
+					   adev->vm_manager.vram_base_offset,
+					   bo->tbo.resource, NULL,
+					   &vm->last_update);
 
 		if (r)
 			dev_warn(adev->dev,
@@ -2781,10 +2783,10 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
 			npa_addr, exp_xa_node->handle.handle_hi, exp_xa_node->handle.handle_lo,
 			remote_acc_id, pte_value);
 
-		r = amdgpu_vm_update_range(adev, vm, false,
-					   false, true, false, NULL, npa_addr,
-					   npa_addr + size - 1, pte_value, 0,
-					   0, NULL, NULL, &vm->last_update);
+		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
+					   npa_addr, npa_addr + size - 1,
+					   pte_value, 0, 0, NULL, NULL,
+					   &vm->last_update);
 
 		if (r)
 			dev_err(adev->dev,
@@ -4567,9 +4569,8 @@ static int amdgpu_ualink_npa_vm_map_range(struct amdgpu_device *adev, struct amd
 		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
 		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
 
-	r = amdgpu_vm_update_range(adev, npa_vm, false, false, true,
-				   false, NULL, npa_in_pages,
-				   npa_in_pages + size_in_pages - 1,
+	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
+				   npa_in_pages, npa_in_pages + size_in_pages - 1,
 				   pte_flags, offset, adev->vm_manager.vram_base_offset,
 				   bo->tbo.resource, NULL, &npa_vm->last_update);
 	if (r)
@@ -4602,9 +4603,8 @@ static int amdgpu_ualink_npa_vm_unmap_range(struct amdgpu_device *adev, struct a
 		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
 		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
 
-	r = amdgpu_vm_update_range(adev, npa_vm, false, false, true,
-				   false, NULL, npa_in_pages,
-				   npa_in_pages + size_in_pages - 1,
+	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
+				   npa_in_pages, npa_in_pages + size_in_pages - 1,
 				   pte_flags, offset, 0, bo->tbo.resource, NULL,
 				   &npa_vm->last_update);
 	if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index d6358cbfcc9b1..017ec0ac0870d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1108,7 +1108,6 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
  *
  * @adev: amdgpu_device pointer to use for commands
  * @vm: the VM to update the range
- * @immediate: immediate submission in a page fault
  * @unlocked: unlocked invalidation during MM callback
  * @flush_tlb: trigger tlb invalidation after update completed
  * @allow_override: change MTYPE for local NUMA nodes
@@ -1128,12 +1127,11 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
  * 0 for success, negative erro code for failure.
  */
 int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			   bool immediate, bool unlocked, bool flush_tlb,
-			   bool allow_override, struct amdgpu_sync *sync,
-			   uint64_t start, uint64_t last, uint64_t flags,
-			   uint64_t offset, uint64_t vram_base,
-			   struct ttm_resource *res, dma_addr_t *pages_addr,
-			   struct dma_fence **fence)
+			   bool unlocked, bool flush_tlb, bool allow_override,
+			   struct amdgpu_sync *sync, uint64_t start,
+			   uint64_t last, uint64_t flags, uint64_t offset,
+			   uint64_t vram_base, struct ttm_resource *res,
+			   dma_addr_t *pages_addr, struct dma_fence **fence)
 {
 	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
 	struct amdgpu_vm_update_params params;
@@ -1163,7 +1161,6 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	memset(&params, 0, sizeof(params));
 	params.adev = adev;
 	params.vm = vm;
-	params.immediate = immediate;
 	params.pages_addr = pages_addr;
 	params.unlocked = unlocked;
 	params.needs_flush = flush_tlb;
@@ -1389,7 +1386,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 
 		trace_amdgpu_vm_bo_update(mapping);
 
-		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb,
+		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb,
 					   !uncached, &sync, mapping->start,
 					   mapping->last, update_flags,
 					   mapping->offset, vram_base, mem,
@@ -1600,7 +1597,7 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			struct amdgpu_bo_va_mapping, list);
 		list_del(&mapping->list);
 
-		r = amdgpu_vm_update_range(adev, vm, false, false, true, false,
+		r = amdgpu_vm_update_range(adev, vm, false, true, false,
 					   &sync, mapping->start, mapping->last,
 					   0, 0, 0, NULL, NULL, &f);
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
@@ -2661,7 +2658,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	vm->tlb_fence_context = dma_fence_context_alloc(1);
 
 	r = amdgpu_vm_pt_create(adev, vm, adev->vm_manager.root_level,
-				false, &root, xcp_id);
+				&root, xcp_id);
 	if (r)
 		goto error_free_delayed;
 
@@ -2677,7 +2674,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	if (r)
 		goto error_free_root;
 
-	r = amdgpu_vm_pt_clear(adev, vm, root, false);
+	r = amdgpu_vm_pt_clear(adev, vm, root);
 	if (r)
 		goto error_free_root;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ec5cd38fe4e34..3ff17625a8c0c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -466,12 +466,11 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
 void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
 			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
 int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			   bool immediate, bool unlocked, bool flush_tlb,
-			   bool allow_override, struct amdgpu_sync *sync,
-			   uint64_t start, uint64_t last, uint64_t flags,
-			   uint64_t offset, uint64_t vram_base,
-			   struct ttm_resource *res, dma_addr_t *pages_addr,
-			   struct dma_fence **fence);
+			   bool unlocked, bool flush_tlb, bool allow_override,
+			   struct amdgpu_sync *sync, uint64_t start,
+			   uint64_t last, uint64_t flags, uint64_t offset,
+			   uint64_t vram_base, struct ttm_resource *res,
+			   dma_addr_t *pages_addr, struct dma_fence **fence);
 int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			struct amdgpu_bo_va *bo_va,
 			bool clear);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index dafdb3a001b8e..89fdeff0575ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -116,10 +116,9 @@ struct amdgpu_vm_update_funcs {
 };
 
 int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-		       struct amdgpu_bo_vm *vmbo, bool immediate);
+		       struct amdgpu_bo_vm *vmbo);
 int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			int level, bool immediate, struct amdgpu_bo_vm **vmbo,
-			int32_t xcp_id);
+			int level, struct amdgpu_bo_vm **vmbo, int32_t xcp_id);
 void amdgpu_vm_pt_free_root(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 int amdgpu_vm_pde_update(struct amdgpu_vm_update_params *params,
 			 struct amdgpu_vm_bo_base *entry);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 27003b03b5fdb..50c41e39e2a2b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -379,7 +379,6 @@ static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
  * @adev: amdgpu_device pointer
  * @vm: VM to clear BO from
  * @vmbo: BO to clear
- * @immediate: use an immediate update
  *
  * Root PD needs to be reserved when calling this.
  *
@@ -387,7 +386,7 @@ static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
  * 0 on success, errno otherwise.
  */
 int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-		       struct amdgpu_bo_vm *vmbo, bool immediate)
+		       struct amdgpu_bo_vm *vmbo)
 {
 	unsigned int level = adev->vm_manager.root_level;
 	struct ttm_operation_ctx ctx = { true, false };
@@ -423,7 +422,6 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	memset(&params, 0, sizeof(params));
 	params.adev = adev;
 	params.vm = vm;
-	params.immediate = immediate;
 
 	r = vm->update_funcs->prepare(&params, NULL,
 				      AMDGPU_KERNEL_JOB_ID_VM_PT_CLEAR);
@@ -447,13 +445,11 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
  * @adev: amdgpu_device pointer
  * @vm: requesting vm
  * @level: the page table level
- * @immediate: use a immediate update
  * @vmbo: pointer to the buffer object pointer
  * @xcp_id: GPU partition id
  */
 int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			int level, bool immediate, struct amdgpu_bo_vm **vmbo,
-			int32_t xcp_id)
+			int level, struct amdgpu_bo_vm **vmbo, int32_t xcp_id)
 {
 	struct amdgpu_bo_param bp;
 	unsigned int num_entries;
@@ -484,7 +480,6 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
 
 	bp.type = ttm_bo_type_kernel;
-	bp.no_wait_gpu = immediate;
 	bp.xcp_id_plus1 = xcp_id + 1;
 
 	if (vm->root.bo)
@@ -534,7 +529,7 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
 		return 0;
 
 	amdgpu_vm_end_critical(p);
-	r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, p->immediate,
+	r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level,
 				&pt, p->vm->root.bo->xcp_id);
 	r2 = amdgpu_vm_begin_critical(p);
 	if (r)
@@ -548,7 +543,7 @@ static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
 	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, p->immediate);
+	r = amdgpu_vm_pt_clear(p->adev, p->vm, pt);
 	if (r)
 		goto error_unpin;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index a99dbedf3cd4f..411bd3394442a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1411,7 +1411,7 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		return -EINVAL;
 	}
 
-	return amdgpu_vm_update_range(adev, vm, false, true, true, false, NULL, gpu_start,
+	return amdgpu_vm_update_range(adev, vm, true, true, false, NULL, gpu_start,
 				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
 				      fence);
 }
@@ -1523,7 +1523,7 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
 			 (last_domain == SVM_RANGE_VRAM_DOMAIN) ? 1 : 0,
 			 pte_flags);
 
-		r = amdgpu_vm_update_range(adev, vm, false, false, flush_tlb, true,
+		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb, true,
 					   NULL, gpu_start, gpu_end,
 					   pte_flags,
 					   (last_start - prange->start) << PAGE_SHIFT,
-- 
2.43.0

[PATCH 6/9] drm/amdgpu: drop immediate updates from amdgpu_vm_update_pdes

[Christian König]

That case is handled by amdgpu_vm_update_leaves now.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/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_ualink.c       | 12 ++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c        |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c           |  5 +----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h           |  3 +--
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c             |  2 +-
 8 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index a5d1d83ee5d32..cb46111b2dc8c 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);
 	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 e7a9c2de46131..eef3acfdb7c73 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1182,7 +1182,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);
 	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 4e4814caf61a9..86a1239e49149 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -811,7 +811,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);
 	if (r)
 		goto error;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index aac97dca36e33..899153716b7ef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -2065,7 +2065,7 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
 		goto out;
 	}
 
-	r = amdgpu_vm_update_pdes(adev, vm, false);
+	r = amdgpu_vm_update_pdes(adev, vm);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed %d to update page directories during unmapping NPA: 0x%llx\n",
@@ -2119,7 +2119,7 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
 		goto out;
 	}
 
-	r = amdgpu_vm_update_pdes(adev, vm, false);
+	r = amdgpu_vm_update_pdes(adev, vm);
 	if (r) {
 		dev_err(adev->dev,
 			"failed %d to update page directories for NPA: 0x%llx\n",
@@ -2661,7 +2661,7 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
 			break;
 	}
 
-	r = amdgpu_vm_update_pdes(adev, vm, false);
+	r = amdgpu_vm_update_pdes(adev, vm);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed %d to update page directories during force retry rpcs\n",
@@ -2798,7 +2798,7 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
 			break;
 	}
 
-	r = amdgpu_vm_update_pdes(adev, vm, false);
+	r = amdgpu_vm_update_pdes(adev, vm);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed %d to update page directories during all NPA addresses unmapping\n",
@@ -4873,7 +4873,7 @@ static void amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev)
 		}
 	}
 
-	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm, false);
+	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm);
 	if (r) {
 		dev_dbg(adev->dev, "failed %d to update directories\n", r);
 		goto out_unreserve;
@@ -5016,7 +5016,7 @@ static int amdgpu_ualink_metadata_npa_mapping(struct amdgpu_device *adev)
 		}
 	}
 
-	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm, false);
+	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm);
 	if (r) {
 		dev_dbg(adev->dev, "failed %d to update directories\n", r);
 		goto error_npa_mapping;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 75d5ceaa53c65..cf427037a8303 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1173,7 +1173,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);
 	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 017ec0ac0870d..cc6c419ff6fa2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -985,15 +985,13 @@ 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
  *
  * Makes sure all directories are up to date.
  *
  * Returns:
  * 0 for success, error for failure.
  */
-int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
-			  struct amdgpu_vm *vm, bool immediate)
+int amdgpu_vm_update_pdes(struct amdgpu_device *adev, struct amdgpu_vm *vm)
 {
 	struct amdgpu_vm_update_params params;
 	struct amdgpu_vm_bo_base *entry, *tmp;
@@ -1011,7 +1009,6 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
 	memset(&params, 0, sizeof(params));
 	params.adev = adev;
 	params.vm = vm;
-	params.immediate = immediate;
 
 	r = vm->update_funcs->prepare(&params, NULL,
 				      AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 3ff17625a8c0c..a3556cf6dd525 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -451,8 +451,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 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);
+int amdgpu_vm_update_pdes(struct amdgpu_device *adev, struct amdgpu_vm *vm);
 int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			  struct amdgpu_vm *vm,
 			  struct dma_fence **fence);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 411bd3394442a..af8d2ede183a1 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1540,7 +1540,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);
 	if (r) {
 		pr_debug("failed %d to update directories 0x%lx\n", r,
 			 prange->start);
-- 
2.43.0

Re: [PATCH 6/9] drm/amdgpu: drop immediate updates from amdgpu_vm_update_pdes

[Timur Kristóf]

On 2026. szeptember 28., hétfő 11:10:38 keleti államokbeli nyári idő Christian 
König wrote:
> That case is handled by amdgpu_vm_update_leaves now.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>

Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>

> ---
>  drivers/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_ualink.c       | 12 ++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c        |  2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c           |  5 +----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h           |  3 +--
>  drivers/gpu/drm/amd/amdkfd/kfd_svm.c             |  2 +-
>  8 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index
> a5d1d83ee5d32..cb46111b2dc8c 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);
>  	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 e7a9c2de46131..eef3acfdb7c73
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -1182,7 +1182,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);
>  	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
> 4e4814caf61a9..86a1239e49149 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -811,7 +811,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);
>  	if (r)
>  		goto error;
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c index
> aac97dca36e33..899153716b7ef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -2065,7 +2065,7 @@ static int amdgpu_ualink_unmap_npa_addr(struct
> amdgpu_device *adev, goto out;
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, vm, false);
> +	r = amdgpu_vm_update_pdes(adev, vm);
>  	if (r) {
>  		dev_err(adev->dev,
>  			"Failed %d to update page directories during 
unmapping NPA: 0x%llx\n",
> @@ -2119,7 +2119,7 @@ static int amdgpu_ualink_map_npa_addr(struct
> amdgpu_device *adev, u64 npa_addr, goto out;
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, vm, false);
> +	r = amdgpu_vm_update_pdes(adev, vm);
>  	if (r) {
>  		dev_err(adev->dev,
>  			"failed %d to update page directories for 
NPA: 0x%llx\n",
> @@ -2661,7 +2661,7 @@ static void amdgpu_ualink_force_retry_rpcs(struct
> amdgpu_device *adev, break;
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, vm, false);
> +	r = amdgpu_vm_update_pdes(adev, vm);
>  	if (r) {
>  		dev_err(adev->dev,
>  			"Failed %d to update page directories during 
force retry rpcs\n",
> @@ -2798,7 +2798,7 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct
> amdgpu_device *adev, break;
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, vm, false);
> +	r = amdgpu_vm_update_pdes(adev, vm);
>  	if (r) {
>  		dev_err(adev->dev,
>  			"Failed %d to update page directories during 
all NPA addresses
> unmapping\n", @@ -4873,7 +4873,7 @@ static void
> amdgpu_ualink_metadata_npa_unmapping(struct amdgpu_device *adev) }
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm, false);
> +	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm);
>  	if (r) {
>  		dev_dbg(adev->dev, "failed %d to update directories\n", 
r);
>  		goto out_unreserve;
> @@ -5016,7 +5016,7 @@ static int amdgpu_ualink_metadata_npa_mapping(struct
> amdgpu_device *adev) }
>  	}
> 
> -	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm, false);
> +	r = amdgpu_vm_update_pdes(adev, &adev->ualink.npa_vm);
>  	if (r) {
>  		dev_dbg(adev->dev, "failed %d to update directories\n", 
r);
>  		goto error_npa_mapping;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index
> 75d5ceaa53c65..cf427037a8303 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1173,7 +1173,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);
>  	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 017ec0ac0870d..cc6c419ff6fa2
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -985,15 +985,13 @@ 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
>   *
>   * Makes sure all directories are up to date.
>   *
>   * Returns:
>   * 0 for success, error for failure.
>   */
> -int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
> -			  struct amdgpu_vm *vm, bool immediate)
> +int amdgpu_vm_update_pdes(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> {
>  	struct amdgpu_vm_update_params params;
>  	struct amdgpu_vm_bo_base *entry, *tmp;
> @@ -1011,7 +1009,6 @@ int amdgpu_vm_update_pdes(struct amdgpu_device *adev,
>  	memset(&params, 0, sizeof(params));
>  	params.adev = adev;
>  	params.vm = vm;
> -	params.immediate = immediate;
> 
>  	r = vm->update_funcs->prepare(&params, NULL,
>  				      
AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 3ff17625a8c0c..a3556cf6dd525
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -451,8 +451,7 @@ int amdgpu_vm_validate(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, 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);
> +int amdgpu_vm_update_pdes(struct amdgpu_device *adev, struct amdgpu_vm
> *vm); int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>  			  struct amdgpu_vm *vm,
>  			  struct dma_fence **fence);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c index 411bd3394442a..af8d2ede183a1
> 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1540,7 +1540,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);
>  	if (r) {
>  		pr_debug("failed %d to update directories 0x%lx\n", r,
>  			 prange->start);

[PATCH 7/9] drm/amdgpu: split amdgpu_vm_update_range v3

[Christian König]

Split amdgpu_vm_update_range into two functions.

amdgpu_vm_map_range() is for mapping PTEs into a range and updates
which can be done while holding the VM lock.

amdgpu_vm_unmap_range() is for unmapping PTEs without holding the VM
lock in MMU notifiers.

v2: add unlocked parameter and drop flags
v3: use correct clear flags

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h       |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c    |  50 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 117 +++++++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  16 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     |  40 ++----
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c          |  17 ++-
 7 files changed, 157 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index ae569b4497f6a..d4a4e51f3b1da 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -47,7 +47,7 @@ enum amdgpu_ib_pool_type;
 /* Internal kernel job ids. (decreasing values, starting from U64_MAX). */
 #define AMDGPU_KERNEL_JOB_ID_VM_UPDATE              (18446744073709551615ULL)
 #define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES         (18446744073709551614ULL)
-#define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_RANGE        (18446744073709551613ULL)
+#define AMDGPU_KERNEL_JOB_ID_VM_MAP_RANGE           (18446744073709551613ULL)
 #define AMDGPU_KERNEL_JOB_ID_VM_PT_CLEAR            (18446744073709551612ULL)
 #define AMDGPU_KERNEL_JOB_ID_TTM_MAP_BUFFER         (18446744073709551611ULL)
 #define AMDGPU_KERNEL_JOB_ID_TTM_ACCESS_MEMORY_SDMA (18446744073709551610ULL)
@@ -63,6 +63,7 @@ enum amdgpu_ib_pool_type;
 #define AMDGPU_KERNEL_JOB_ID_SDMA_RING_TEST         (18446744073709551600ULL)
 #define AMDGPU_KERNEL_JOB_ID_VPE_RING_TEST          (18446744073709551599ULL)
 #define AMDGPU_KERNEL_JOB_ID_RUN_SHADER             (18446744073709551598ULL)
+#define AMDGPU_KERNEL_JOB_ID_VM_UNMAP_RANGE         (18446744073709551597ULL)
 
 struct amdgpu_job {
 	struct drm_sched_job    base;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index 899153716b7ef..b7975c77b1fe2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -2056,9 +2056,9 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
 		return r;
 	}
 
-	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
-				   npa_addr + size - 1, pte_value, 0, 0, NULL,
-				   NULL, &vm->last_update);
+	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
+				npa_addr + size - 1, pte_value, 0, 0, NULL,
+				NULL, &vm->last_update);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed to unmap NPA addr (%llx) from NPA VM\n", npa_addr);
@@ -2108,10 +2108,10 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
 		return r;
 	}
 
-	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
-				   npa_addr + size - 1, pte_flags, offset,
-				   adev->vm_manager.vram_base_offset,
-				   bo->tbo.resource, NULL, &vm->last_update);
+	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
+				npa_addr + size - 1, pte_flags, offset,
+				adev->vm_manager.vram_base_offset,
+				bo->tbo.resource, NULL, &vm->last_update);
 	if (r) {
 		dev_warn(adev->dev,
 			"Failed to map NPA addr (%llx) into NPA VM\n", npa_addr);
@@ -2645,12 +2645,12 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
 			"RETRY-RPC: setting PTE.X=1 for NPA:%llx remote:%u pte:0x%llx\n",
 			npa_addr, remote_acc_id, pte_flags);
 
-		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
-					   npa_addr, npa_addr + size - 1,
-					   pte_flags, 0,
-					   adev->vm_manager.vram_base_offset,
-					   bo->tbo.resource, NULL,
-					   &vm->last_update);
+		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
+					npa_addr, npa_addr + size - 1,
+					pte_flags, 0,
+					adev->vm_manager.vram_base_offset,
+					bo->tbo.resource, NULL,
+					&vm->last_update);
 
 		if (r)
 			dev_warn(adev->dev,
@@ -2783,10 +2783,10 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
 			npa_addr, exp_xa_node->handle.handle_hi, exp_xa_node->handle.handle_lo,
 			remote_acc_id, pte_value);
 
-		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
-					   npa_addr, npa_addr + size - 1,
-					   pte_value, 0, 0, NULL, NULL,
-					   &vm->last_update);
+		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
+					npa_addr, npa_addr + size - 1,
+					pte_value, 0, 0, NULL, NULL,
+					&vm->last_update);
 
 		if (r)
 			dev_err(adev->dev,
@@ -4569,10 +4569,10 @@ static int amdgpu_ualink_npa_vm_map_range(struct amdgpu_device *adev, struct amd
 		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
 		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
 
-	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
-				   npa_in_pages, npa_in_pages + size_in_pages - 1,
-				   pte_flags, offset, adev->vm_manager.vram_base_offset,
-				   bo->tbo.resource, NULL, &npa_vm->last_update);
+	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
+				npa_in_pages, npa_in_pages + size_in_pages - 1,
+				pte_flags, offset, adev->vm_manager.vram_base_offset,
+				bo->tbo.resource, NULL, &npa_vm->last_update);
 	if (r)
 		dev_dbg(adev->dev, "failed %d to map npa 0x%llx to NPA VM\n", r,
 			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
@@ -4603,10 +4603,10 @@ static int amdgpu_ualink_npa_vm_unmap_range(struct amdgpu_device *adev, struct a
 		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
 		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
 
-	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
-				   npa_in_pages, npa_in_pages + size_in_pages - 1,
-				   pte_flags, offset, 0, bo->tbo.resource, NULL,
-				   &npa_vm->last_update);
+	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
+				npa_in_pages, npa_in_pages + size_in_pages - 1,
+				pte_flags, offset, 0, bo->tbo.resource, NULL,
+				&npa_vm->last_update);
 	if (r)
 		dev_dbg(adev->dev, "failed %d to unmap npa 0x%llx from NPA VM\n", r,
 			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index cc6c419ff6fa2..77ddb7fa96fb0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1101,11 +1101,10 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
 }
 
 /**
- * amdgpu_vm_update_range - update a range in the vm page table
+ * amdgpu_vm_map_range - map something to a range in the vm page tables
  *
  * @adev: amdgpu_device pointer to use for commands
  * @vm: the VM to update the range
- * @unlocked: unlocked invalidation during MM callback
  * @flush_tlb: trigger tlb invalidation after update completed
  * @allow_override: change MTYPE for local NUMA nodes
  * @sync: fences we need to sync to
@@ -1118,23 +1117,26 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
  * @pages_addr: DMA addresses to use for mapping
  * @fence: optional resulting fence
  *
- * Fill in the page table entries between @start and @last.
+ * Fill in the page table entries between @start and @last. Allocate and free
+ * new page tables as needed. Can only be called while holding the VM lock.
  *
  * Returns:
  * 0 for success, negative erro code for failure.
  */
-int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			   bool unlocked, bool flush_tlb, bool allow_override,
-			   struct amdgpu_sync *sync, uint64_t start,
-			   uint64_t last, uint64_t flags, uint64_t offset,
-			   uint64_t vram_base, struct ttm_resource *res,
-			   dma_addr_t *pages_addr, struct dma_fence **fence)
+int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			bool flush_tlb, bool allow_override,
+			struct amdgpu_sync *sync, uint64_t start,
+			uint64_t last, uint64_t flags, uint64_t offset,
+			uint64_t vram_base, struct ttm_resource *res,
+			dma_addr_t *pages_addr, struct dma_fence **fence)
 {
 	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
 	struct amdgpu_vm_update_params params;
 	struct amdgpu_res_cursor cursor;
 	int r, idx;
 
+	amdgpu_vm_assert_locked(vm);
+
 	if (!drm_dev_enter(adev_to_drm(adev), &idx))
 		return -ENODEV;
 
@@ -1159,7 +1161,6 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	params.adev = adev;
 	params.vm = vm;
 	params.pages_addr = pages_addr;
-	params.unlocked = unlocked;
 	params.needs_flush = flush_tlb;
 	params.override_pte = allow_override && adev->gmc.override_pte;
 	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
@@ -1168,7 +1169,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	if (r)
 		goto error_free;
 
-	if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) {
+	if (!dma_fence_is_signaled(vm->last_unlocked)) {
 		struct dma_fence *tmp = dma_fence_get_stub();
 
 		amdgpu_bo_fence(vm->root.bo, vm->last_unlocked, true);
@@ -1177,7 +1178,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	}
 
 	r = vm->update_funcs->prepare(&params, sync,
-				      AMDGPU_KERNEL_JOB_ID_VM_UPDATE_RANGE);
+				      AMDGPU_KERNEL_JOB_ID_VM_MAP_RANGE);
 	if (r)
 		goto error_free;
 
@@ -1255,6 +1256,82 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	return r;
 }
 
+/**
+ * amdgpu_vm_unmap_range - clear leave PTEs to unmap something
+ *
+ * @adev: amdgpu_device pointer to use for commands
+ * @vm: the VM to update the range
+ * @unlocked: unlocked invalidation during MM callback
+ * @sync: fences we need to sync to
+ * @start: start of unmapped range
+ * @last: last unmapped entry
+ * @fence: optional resulting fence
+ *
+ * Fill in the page table entries between @start and @last with a fixed flags
+ * value without allocating or freeing page tables. Can be used without locking
+ * the VM.
+ *
+ * Returns:
+ * 0 for success, negative erro code for failure.
+ */
+int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			  bool unlocked, struct amdgpu_sync *sync,
+			  uint64_t start, uint64_t last,
+			  struct dma_fence **fence)
+{
+	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
+	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
+	struct amdgpu_vm_update_params params;
+	uint64_t flags[AMDGPU_VM_MAX_LEVEL];
+	int r, idx;
+
+	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(&params, 0, sizeof(params));
+	params.adev = adev;
+	params.vm = vm;
+	params.needs_flush = true;
+	params.unlocked = unlocked;
+	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
+
+	r = amdgpu_vm_begin_critical(&params);
+	if (r)
+		goto error_end_critical;
+
+	r = vm->update_funcs->prepare(&params, sync,
+				      AMDGPU_KERNEL_JOB_ID_VM_UNMAP_RANGE);
+	if (r)
+		goto error_free;
+
+	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; level--)
+		flags[level] = amdgpu_vm_pt_clear_flags(adev, vm, level);
+
+	amdgpu_vm_pt_leaves(&params, start, last + 1, NULL, flags);
+
+	r = vm->update_funcs->commit(&params, fence);
+	if (r)
+		goto error_free;
+
+	amdgpu_vm_tlb_flush(&params, fence, tlb_cb);
+	amdgpu_vm_pt_free_list(adev, &params);
+	tlb_cb = NULL;
+
+error_free:
+	kfree(tlb_cb);
+
+error_end_critical:
+	amdgpu_vm_end_critical(&params);
+	drm_dev_exit(idx);
+	return r;
+}
+
 void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
 			  struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM])
 {
@@ -1383,11 +1460,11 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 
 		trace_amdgpu_vm_bo_update(mapping);
 
-		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb,
-					   !uncached, &sync, mapping->start,
-					   mapping->last, update_flags,
-					   mapping->offset, vram_base, mem,
-					   pages_addr, last_update);
+		r = amdgpu_vm_map_range(adev, vm, flush_tlb, !uncached, &sync,
+					mapping->start, mapping->last,
+					update_flags, mapping->offset,
+					vram_base, mem, pages_addr,
+					last_update);
 		if (r)
 			goto error_free;
 	}
@@ -1594,9 +1671,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			struct amdgpu_bo_va_mapping, list);
 		list_del(&mapping->list);
 
-		r = amdgpu_vm_update_range(adev, vm, false, true, false,
-					   &sync, mapping->start, mapping->last,
-					   0, 0, 0, NULL, NULL, &f);
+		r = amdgpu_vm_map_range(adev, vm, true, false,
+					&sync, mapping->start, mapping->last,
+					0, 0, 0, NULL, NULL, &f);
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
 			dma_fence_put(f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index a3556cf6dd525..a6f6541758609 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -464,12 +464,16 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
 				uint32_t xcc_mask);
 void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
 			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
-int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
-			   bool unlocked, bool flush_tlb, bool allow_override,
-			   struct amdgpu_sync *sync, uint64_t start,
-			   uint64_t last, uint64_t flags, uint64_t offset,
-			   uint64_t vram_base, struct ttm_resource *res,
-			   dma_addr_t *pages_addr, struct dma_fence **fence);
+int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			bool flush_tlb, bool allow_override,
+			struct amdgpu_sync *sync, uint64_t start,
+			uint64_t last, uint64_t flags, uint64_t offset,
+			uint64_t vram_base, struct ttm_resource *res,
+			dma_addr_t *pages_addr, struct dma_fence **fence);
+int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+			  bool unlocked, struct amdgpu_sync *sync,
+			  uint64_t start, uint64_t last,
+			  struct dma_fence **fence);
 int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			struct amdgpu_bo_va *bo_va,
 			bool clear);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index 89fdeff0575ef..db748e8b3bf38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -115,6 +115,9 @@ struct amdgpu_vm_update_funcs {
 		      struct dma_fence **fence);
 };
 
+uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
+				  struct amdgpu_vm *vm,
+				  unsigned int level);
 int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		       struct amdgpu_bo_vm *vmbo);
 int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
index 50c41e39e2a2b..dea80796f1393 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
@@ -347,9 +347,9 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device *adev,
 	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), &(cursor)))
 
 /* Return the flags used for cleared PDEs/PTES */
-static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
-					struct amdgpu_vm *vm,
-					unsigned int level)
+uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
+				  struct amdgpu_vm *vm,
+				  unsigned int level)
 {
 	uint64_t flags;
 
@@ -590,7 +590,6 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
 			    struct amdgpu_vm_update_params *params)
 {
 	struct amdgpu_vm_bo_base *entry, *next;
-	bool unlocked = params->unlocked;
 
 	if (list_empty(&params->tlb_flush_waitlist))
 		return;
@@ -598,7 +597,7 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
 	/*
 	 * unlocked unmap clear page table leaves, warning to free the page entry.
 	 */
-	WARN_ON(unlocked);
+	WARN_ON(params->unlocked);
 
 	list_for_each_entry_safe(entry, next, &params->tlb_flush_waitlist, vm_status)
 		amdgpu_vm_pt_free(entry);
@@ -830,23 +829,17 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
 		uint64_t incr, entry_end, pe_start;
 		struct amdgpu_bo *pt;
 
-		if (!params->unlocked) {
-			/* make sure that the page tables covering the
-			 * address range are actually allocated
-			 */
-			r = amdgpu_vm_pt_alloc(params, &cursor);
-			if (r)
-				return r;
-		}
+		/* make sure that the page tables covering the
+		 * address range are actually allocated
+		 */
+		r = amdgpu_vm_pt_alloc(params, &cursor);
+		if (r)
+			return r;
 
 		shift = amdgpu_vm_pt_level_shift(adev, cursor.level);
 		parent_shift = amdgpu_vm_pt_level_shift(adev, cursor.level - 1);
-		if (params->unlocked) {
-			/* Unlocked updates are only allowed on the leaves */
-			if (amdgpu_vm_pt_descendant(adev, &cursor))
-				continue;
-		} else if (adev->asic_type < CHIP_VEGA10 &&
-			   (flags & AMDGPU_PTE_VALID)) {
+		if (adev->asic_type < CHIP_VEGA10 &&
+		    (flags & AMDGPU_PTE_VALID)) {
 			/* No huge page support before GMC v9 */
 			if (cursor.level != AMDGPU_VM_PTB) {
 				if (!amdgpu_vm_pt_descendant(adev, &cursor))
@@ -892,14 +885,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
 		mask = amdgpu_vm_pt_entries_mask(adev, cursor.level);
 		pe_start = ((cursor.pfn >> shift) & mask) * 8;
 
-		if (cursor.level < AMDGPU_VM_PTB && params->unlocked)
-			/*
-			 * MMU notifier callback unlocked unmap huge page, leave is PDE entry,
-			 * only clear one entry. Next entry search again for PDE or PTE leave.
-			 */
-			entry_end = 1ULL << shift;
-		else
-			entry_end = ((uint64_t)mask + 1) << shift;
+		entry_end = ((uint64_t)mask + 1) << shift;
 		entry_end += cursor.pfn & ~(entry_end - 1);
 		entry_end = min(entry_end, end);
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index af8d2ede183a1..263e68d29360d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1396,7 +1396,6 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			 uint64_t start, uint64_t last,
 			 struct dma_fence **fence)
 {
-	uint64_t init_pte_value = adev->gmc.init_pte_flags;
 	uint64_t gpu_start, gpu_end;
 
 	/* Convert CPU page range to GPU page range */
@@ -1411,9 +1410,8 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		return -EINVAL;
 	}
 
-	return amdgpu_vm_update_range(adev, vm, true, true, false, NULL, gpu_start,
-				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
-				      fence);
+	return amdgpu_vm_unmap_range(adev, vm, true, NULL, gpu_start, gpu_end,
+				     fence);
 }
 
 static int
@@ -1523,12 +1521,11 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
 			 (last_domain == SVM_RANGE_VRAM_DOMAIN) ? 1 : 0,
 			 pte_flags);
 
-		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb, true,
-					   NULL, 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, &vm->last_update);
+		r = amdgpu_vm_map_range(adev, vm, flush_tlb, true, NULL,
+					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, &vm->last_update);
 
 		for (j = last_start - prange->start; j <= i; j++)
 			dma_addr[j] |= last_domain;
-- 
2.43.0

Re: [PATCH 7/9] drm/amdgpu: split amdgpu_vm_update_range v3

[Kuehling, Felix]

On 2026-09-28 11:10, Christian König wrote:
> Split amdgpu_vm_update_range into two functions.
>
> amdgpu_vm_map_range() is for mapping PTEs into a range and updates
> which can be done while holding the VM lock.
>
> amdgpu_vm_unmap_range() is for unmapping PTEs without holding the VM
> lock in MMU notifiers.

The naming is a little misleading. As I understand it, 
amdgpu_vm_map_range still supports unmapping for the case that the page 
tables can be locked. In fact, this should be the preferred method for 
anything except MMU notifiers because amdgpu_vm_unmap_range cannot 
allocate or free page tables.

Regards,
   Felix


>
> v2: add unlocked parameter and drop flags
> v3: use correct clear flags
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h       |   3 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c    |  50 ++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 117 +++++++++++++++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  16 ++-
>   .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   |   3 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     |  40 ++----
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c          |  17 ++-
>   7 files changed, 157 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index ae569b4497f6a..d4a4e51f3b1da 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -47,7 +47,7 @@ enum amdgpu_ib_pool_type;
>   /* Internal kernel job ids. (decreasing values, starting from U64_MAX). */
>   #define AMDGPU_KERNEL_JOB_ID_VM_UPDATE              (18446744073709551615ULL)
>   #define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES         (18446744073709551614ULL)
> -#define AMDGPU_KERNEL_JOB_ID_VM_UPDATE_RANGE        (18446744073709551613ULL)
> +#define AMDGPU_KERNEL_JOB_ID_VM_MAP_RANGE           (18446744073709551613ULL)
>   #define AMDGPU_KERNEL_JOB_ID_VM_PT_CLEAR            (18446744073709551612ULL)
>   #define AMDGPU_KERNEL_JOB_ID_TTM_MAP_BUFFER         (18446744073709551611ULL)
>   #define AMDGPU_KERNEL_JOB_ID_TTM_ACCESS_MEMORY_SDMA (18446744073709551610ULL)
> @@ -63,6 +63,7 @@ enum amdgpu_ib_pool_type;
>   #define AMDGPU_KERNEL_JOB_ID_SDMA_RING_TEST         (18446744073709551600ULL)
>   #define AMDGPU_KERNEL_JOB_ID_VPE_RING_TEST          (18446744073709551599ULL)
>   #define AMDGPU_KERNEL_JOB_ID_RUN_SHADER             (18446744073709551598ULL)
> +#define AMDGPU_KERNEL_JOB_ID_VM_UNMAP_RANGE         (18446744073709551597ULL)
>   
>   struct amdgpu_job {
>   	struct drm_sched_job    base;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index 899153716b7ef..b7975c77b1fe2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -2056,9 +2056,9 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
>   		return r;
>   	}
>   
> -	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
> -				   npa_addr + size - 1, pte_value, 0, 0, NULL,
> -				   NULL, &vm->last_update);
> +	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
> +				npa_addr + size - 1, pte_value, 0, 0, NULL,
> +				NULL, &vm->last_update);
>   	if (r) {
>   		dev_err(adev->dev,
>   			"Failed to unmap NPA addr (%llx) from NPA VM\n", npa_addr);
> @@ -2108,10 +2108,10 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
>   		return r;
>   	}
>   
> -	r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL, npa_addr,
> -				   npa_addr + size - 1, pte_flags, offset,
> -				   adev->vm_manager.vram_base_offset,
> -				   bo->tbo.resource, NULL, &vm->last_update);
> +	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
> +				npa_addr + size - 1, pte_flags, offset,
> +				adev->vm_manager.vram_base_offset,
> +				bo->tbo.resource, NULL, &vm->last_update);
>   	if (r) {
>   		dev_warn(adev->dev,
>   			"Failed to map NPA addr (%llx) into NPA VM\n", npa_addr);
> @@ -2645,12 +2645,12 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
>   			"RETRY-RPC: setting PTE.X=1 for NPA:%llx remote:%u pte:0x%llx\n",
>   			npa_addr, remote_acc_id, pte_flags);
>   
> -		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
> -					   npa_addr, npa_addr + size - 1,
> -					   pte_flags, 0,
> -					   adev->vm_manager.vram_base_offset,
> -					   bo->tbo.resource, NULL,
> -					   &vm->last_update);
> +		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
> +					npa_addr, npa_addr + size - 1,
> +					pte_flags, 0,
> +					adev->vm_manager.vram_base_offset,
> +					bo->tbo.resource, NULL,
> +					&vm->last_update);
>   
>   		if (r)
>   			dev_warn(adev->dev,
> @@ -2783,10 +2783,10 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
>   			npa_addr, exp_xa_node->handle.handle_hi, exp_xa_node->handle.handle_lo,
>   			remote_acc_id, pte_value);
>   
> -		r = amdgpu_vm_update_range(adev, vm, false, true, false, NULL,
> -					   npa_addr, npa_addr + size - 1,
> -					   pte_value, 0, 0, NULL, NULL,
> -					   &vm->last_update);
> +		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
> +					npa_addr, npa_addr + size - 1,
> +					pte_value, 0, 0, NULL, NULL,
> +					&vm->last_update);
>   
>   		if (r)
>   			dev_err(adev->dev,
> @@ -4569,10 +4569,10 @@ static int amdgpu_ualink_npa_vm_map_range(struct amdgpu_device *adev, struct amd
>   		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
>   		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
>   
> -	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
> -				   npa_in_pages, npa_in_pages + size_in_pages - 1,
> -				   pte_flags, offset, adev->vm_manager.vram_base_offset,
> -				   bo->tbo.resource, NULL, &npa_vm->last_update);
> +	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
> +				npa_in_pages, npa_in_pages + size_in_pages - 1,
> +				pte_flags, offset, adev->vm_manager.vram_base_offset,
> +				bo->tbo.resource, NULL, &npa_vm->last_update);
>   	if (r)
>   		dev_dbg(adev->dev, "failed %d to map npa 0x%llx to NPA VM\n", r,
>   			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
> @@ -4603,10 +4603,10 @@ static int amdgpu_ualink_npa_vm_unmap_range(struct amdgpu_device *adev, struct a
>   		offset, size_in_pages << AMDGPU_GPU_PAGE_SHIFT, pte_flags,
>   		npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
>   
> -	r = amdgpu_vm_update_range(adev, npa_vm, false, true, false, NULL,
> -				   npa_in_pages, npa_in_pages + size_in_pages - 1,
> -				   pte_flags, offset, 0, bo->tbo.resource, NULL,
> -				   &npa_vm->last_update);
> +	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
> +				npa_in_pages, npa_in_pages + size_in_pages - 1,
> +				pte_flags, offset, 0, bo->tbo.resource, NULL,
> +				&npa_vm->last_update);
>   	if (r)
>   		dev_dbg(adev->dev, "failed %d to unmap npa 0x%llx from NPA VM\n", r,
>   			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index cc6c419ff6fa2..77ddb7fa96fb0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1101,11 +1101,10 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>   }
>   
>   /**
> - * amdgpu_vm_update_range - update a range in the vm page table
> + * amdgpu_vm_map_range - map something to a range in the vm page tables
>    *
>    * @adev: amdgpu_device pointer to use for commands
>    * @vm: the VM to update the range
> - * @unlocked: unlocked invalidation during MM callback
>    * @flush_tlb: trigger tlb invalidation after update completed
>    * @allow_override: change MTYPE for local NUMA nodes
>    * @sync: fences we need to sync to
> @@ -1118,23 +1117,26 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>    * @pages_addr: DMA addresses to use for mapping
>    * @fence: optional resulting fence
>    *
> - * Fill in the page table entries between @start and @last.
> + * Fill in the page table entries between @start and @last. Allocate and free
> + * new page tables as needed. Can only be called while holding the VM lock.
>    *
>    * Returns:
>    * 0 for success, negative erro code for failure.
>    */
> -int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			   bool unlocked, bool flush_tlb, bool allow_override,
> -			   struct amdgpu_sync *sync, uint64_t start,
> -			   uint64_t last, uint64_t flags, uint64_t offset,
> -			   uint64_t vram_base, struct ttm_resource *res,
> -			   dma_addr_t *pages_addr, struct dma_fence **fence)
> +int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +			bool flush_tlb, bool allow_override,
> +			struct amdgpu_sync *sync, uint64_t start,
> +			uint64_t last, uint64_t flags, uint64_t offset,
> +			uint64_t vram_base, struct ttm_resource *res,
> +			dma_addr_t *pages_addr, struct dma_fence **fence)
>   {
>   	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
>   	struct amdgpu_vm_update_params params;
>   	struct amdgpu_res_cursor cursor;
>   	int r, idx;
>   
> +	amdgpu_vm_assert_locked(vm);
> +
>   	if (!drm_dev_enter(adev_to_drm(adev), &idx))
>   		return -ENODEV;
>   
> @@ -1159,7 +1161,6 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	params.adev = adev;
>   	params.vm = vm;
>   	params.pages_addr = pages_addr;
> -	params.unlocked = unlocked;
>   	params.needs_flush = flush_tlb;
>   	params.override_pte = allow_override && adev->gmc.override_pte;
>   	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
> @@ -1168,7 +1169,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	if (r)
>   		goto error_free;
>   
> -	if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) {
> +	if (!dma_fence_is_signaled(vm->last_unlocked)) {
>   		struct dma_fence *tmp = dma_fence_get_stub();
>   
>   		amdgpu_bo_fence(vm->root.bo, vm->last_unlocked, true);
> @@ -1177,7 +1178,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	}
>   
>   	r = vm->update_funcs->prepare(&params, sync,
> -				      AMDGPU_KERNEL_JOB_ID_VM_UPDATE_RANGE);
> +				      AMDGPU_KERNEL_JOB_ID_VM_MAP_RANGE);
>   	if (r)
>   		goto error_free;
>   
> @@ -1255,6 +1256,82 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	return r;
>   }
>   
> +/**
> + * amdgpu_vm_unmap_range - clear leave PTEs to unmap something
> + *
> + * @adev: amdgpu_device pointer to use for commands
> + * @vm: the VM to update the range
> + * @unlocked: unlocked invalidation during MM callback
> + * @sync: fences we need to sync to
> + * @start: start of unmapped range
> + * @last: last unmapped entry
> + * @fence: optional resulting fence
> + *
> + * Fill in the page table entries between @start and @last with a fixed flags
> + * value without allocating or freeing page tables. Can be used without locking
> + * the VM.
> + *
> + * Returns:
> + * 0 for success, negative erro code for failure.
> + */
> +int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +			  bool unlocked, struct amdgpu_sync *sync,
> +			  uint64_t start, uint64_t last,
> +			  struct dma_fence **fence)
> +{
> +	struct amdgpu_vm_manager *vm_mgr = &adev->vm_manager;
> +	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
> +	struct amdgpu_vm_update_params params;
> +	uint64_t flags[AMDGPU_VM_MAX_LEVEL];
> +	int r, idx;
> +
> +	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(&params, 0, sizeof(params));
> +	params.adev = adev;
> +	params.vm = vm;
> +	params.needs_flush = true;
> +	params.unlocked = unlocked;
> +	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
> +
> +	r = amdgpu_vm_begin_critical(&params);
> +	if (r)
> +		goto error_end_critical;
> +
> +	r = vm->update_funcs->prepare(&params, sync,
> +				      AMDGPU_KERNEL_JOB_ID_VM_UNMAP_RANGE);
> +	if (r)
> +		goto error_free;
> +
> +	for (int level = AMDGPU_VM_PTB; level != vm_mgr->root_level; level--)
> +		flags[level] = amdgpu_vm_pt_clear_flags(adev, vm, level);
> +
> +	amdgpu_vm_pt_leaves(&params, start, last + 1, NULL, flags);
> +
> +	r = vm->update_funcs->commit(&params, fence);
> +	if (r)
> +		goto error_free;
> +
> +	amdgpu_vm_tlb_flush(&params, fence, tlb_cb);
> +	amdgpu_vm_pt_free_list(adev, &params);
> +	tlb_cb = NULL;
> +
> +error_free:
> +	kfree(tlb_cb);
> +
> +error_end_critical:
> +	amdgpu_vm_end_critical(&params);
> +	drm_dev_exit(idx);
> +	return r;
> +}
> +
>   void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
>   			  struct amdgpu_mem_stats stats[__AMDGPU_PL_NUM])
>   {
> @@ -1383,11 +1460,11 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>   
>   		trace_amdgpu_vm_bo_update(mapping);
>   
> -		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb,
> -					   !uncached, &sync, mapping->start,
> -					   mapping->last, update_flags,
> -					   mapping->offset, vram_base, mem,
> -					   pages_addr, last_update);
> +		r = amdgpu_vm_map_range(adev, vm, flush_tlb, !uncached, &sync,
> +					mapping->start, mapping->last,
> +					update_flags, mapping->offset,
> +					vram_base, mem, pages_addr,
> +					last_update);
>   		if (r)
>   			goto error_free;
>   	}
> @@ -1594,9 +1671,9 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va_mapping, list);
>   		list_del(&mapping->list);
>   
> -		r = amdgpu_vm_update_range(adev, vm, false, true, false,
> -					   &sync, mapping->start, mapping->last,
> -					   0, 0, 0, NULL, NULL, &f);
> +		r = amdgpu_vm_map_range(adev, vm, true, false,
> +					&sync, mapping->start, mapping->last,
> +					0, 0, 0, NULL, NULL, &f);
>   		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>   		if (r) {
>   			dma_fence_put(f);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index a3556cf6dd525..a6f6541758609 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -464,12 +464,16 @@ int amdgpu_vm_flush_compute_tlb(struct amdgpu_device *adev,
>   				uint32_t xcc_mask);
>   void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
>   			    struct amdgpu_vm *vm, struct amdgpu_bo *bo);
> -int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> -			   bool unlocked, bool flush_tlb, bool allow_override,
> -			   struct amdgpu_sync *sync, uint64_t start,
> -			   uint64_t last, uint64_t flags, uint64_t offset,
> -			   uint64_t vram_base, struct ttm_resource *res,
> -			   dma_addr_t *pages_addr, struct dma_fence **fence);
> +int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +			bool flush_tlb, bool allow_override,
> +			struct amdgpu_sync *sync, uint64_t start,
> +			uint64_t last, uint64_t flags, uint64_t offset,
> +			uint64_t vram_base, struct ttm_resource *res,
> +			dma_addr_t *pages_addr, struct dma_fence **fence);
> +int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +			  bool unlocked, struct amdgpu_sync *sync,
> +			  uint64_t start, uint64_t last,
> +			  struct dma_fence **fence);
>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			struct amdgpu_bo_va *bo_va,
>   			bool clear);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> index 89fdeff0575ef..db748e8b3bf38 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -115,6 +115,9 @@ struct amdgpu_vm_update_funcs {
>   		      struct dma_fence **fence);
>   };
>   
> +uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
> +				  struct amdgpu_vm *vm,
> +				  unsigned int level);
>   int amdgpu_vm_pt_clear(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		       struct amdgpu_bo_vm *vmbo);
>   int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> index 50c41e39e2a2b..dea80796f1393 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -347,9 +347,9 @@ static void amdgpu_vm_pt_next_dfs(struct amdgpu_device *adev,
>   	     (entry) = (cursor).entry, amdgpu_vm_pt_next_dfs((adev), &(cursor)))
>   
>   /* Return the flags used for cleared PDEs/PTES */
> -static uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
> -					struct amdgpu_vm *vm,
> -					unsigned int level)
> +uint64_t amdgpu_vm_pt_clear_flags(struct amdgpu_device *adev,
> +				  struct amdgpu_vm *vm,
> +				  unsigned int level)
>   {
>   	uint64_t flags;
>   
> @@ -590,7 +590,6 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
>   			    struct amdgpu_vm_update_params *params)
>   {
>   	struct amdgpu_vm_bo_base *entry, *next;
> -	bool unlocked = params->unlocked;
>   
>   	if (list_empty(&params->tlb_flush_waitlist))
>   		return;
> @@ -598,7 +597,7 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
>   	/*
>   	 * unlocked unmap clear page table leaves, warning to free the page entry.
>   	 */
> -	WARN_ON(unlocked);
> +	WARN_ON(params->unlocked);
>   
>   	list_for_each_entry_safe(entry, next, &params->tlb_flush_waitlist, vm_status)
>   		amdgpu_vm_pt_free(entry);
> @@ -830,23 +829,17 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
>   		uint64_t incr, entry_end, pe_start;
>   		struct amdgpu_bo *pt;
>   
> -		if (!params->unlocked) {
> -			/* make sure that the page tables covering the
> -			 * address range are actually allocated
> -			 */
> -			r = amdgpu_vm_pt_alloc(params, &cursor);
> -			if (r)
> -				return r;
> -		}
> +		/* make sure that the page tables covering the
> +		 * address range are actually allocated
> +		 */
> +		r = amdgpu_vm_pt_alloc(params, &cursor);
> +		if (r)
> +			return r;
>   
>   		shift = amdgpu_vm_pt_level_shift(adev, cursor.level);
>   		parent_shift = amdgpu_vm_pt_level_shift(adev, cursor.level - 1);
> -		if (params->unlocked) {
> -			/* Unlocked updates are only allowed on the leaves */
> -			if (amdgpu_vm_pt_descendant(adev, &cursor))
> -				continue;
> -		} else if (adev->asic_type < CHIP_VEGA10 &&
> -			   (flags & AMDGPU_PTE_VALID)) {
> +		if (adev->asic_type < CHIP_VEGA10 &&
> +		    (flags & AMDGPU_PTE_VALID)) {
>   			/* No huge page support before GMC v9 */
>   			if (cursor.level != AMDGPU_VM_PTB) {
>   				if (!amdgpu_vm_pt_descendant(adev, &cursor))
> @@ -892,14 +885,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
>   		mask = amdgpu_vm_pt_entries_mask(adev, cursor.level);
>   		pe_start = ((cursor.pfn >> shift) & mask) * 8;
>   
> -		if (cursor.level < AMDGPU_VM_PTB && params->unlocked)
> -			/*
> -			 * MMU notifier callback unlocked unmap huge page, leave is PDE entry,
> -			 * only clear one entry. Next entry search again for PDE or PTE leave.
> -			 */
> -			entry_end = 1ULL << shift;
> -		else
> -			entry_end = ((uint64_t)mask + 1) << shift;
> +		entry_end = ((uint64_t)mask + 1) << shift;
>   		entry_end += cursor.pfn & ~(entry_end - 1);
>   		entry_end = min(entry_end, end);
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index af8d2ede183a1..263e68d29360d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1396,7 +1396,6 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			 uint64_t start, uint64_t last,
>   			 struct dma_fence **fence)
>   {
> -	uint64_t init_pte_value = adev->gmc.init_pte_flags;
>   	uint64_t gpu_start, gpu_end;
>   
>   	/* Convert CPU page range to GPU page range */
> @@ -1411,9 +1410,8 @@ svm_range_unmap_from_gpu(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		return -EINVAL;
>   	}
>   
> -	return amdgpu_vm_update_range(adev, vm, true, true, false, NULL, gpu_start,
> -				      gpu_end, init_pte_value, 0, 0, NULL, NULL,
> -				      fence);
> +	return amdgpu_vm_unmap_range(adev, vm, true, NULL, gpu_start, gpu_end,
> +				     fence);
>   }
>   
>   static int
> @@ -1523,12 +1521,11 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>   			 (last_domain == SVM_RANGE_VRAM_DOMAIN) ? 1 : 0,
>   			 pte_flags);
>   
> -		r = amdgpu_vm_update_range(adev, vm, false, flush_tlb, true,
> -					   NULL, 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, &vm->last_update);
> +		r = amdgpu_vm_map_range(adev, vm, flush_tlb, true, NULL,
> +					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, &vm->last_update);
>   
>   		for (j = last_start - prange->start; j <= i; j++)
>   			dma_addr[j] |= last_domain;

[PATCH 8/9] drm/amdgpu: fix the HMM range handling for KFD SVM v2

[Christian König]

It's mandatory that we have this check inside the VM handling or
otherwise page table allocation and filling PTEs doesn't work correctly.

This allows to remove the buggy SVM range lock, but that's not part of
this patch set.

Only compile tested!

v2: fix broken compile after ualink merge

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c    | 13 ++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 10 ++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  4 ++-
 .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   | 10 ++++++
 drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 32 ++++++++-----------
 5 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
index b7975c77b1fe2..c1bb74cb45eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
@@ -2058,7 +2058,7 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
 
 	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
 				npa_addr + size - 1, pte_value, 0, 0, NULL,
-				NULL, &vm->last_update);
+				NULL, NULL, &vm->last_update);
 	if (r) {
 		dev_err(adev->dev,
 			"Failed to unmap NPA addr (%llx) from NPA VM\n", npa_addr);
@@ -2111,7 +2111,7 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
 	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
 				npa_addr + size - 1, pte_flags, offset,
 				adev->vm_manager.vram_base_offset,
-				bo->tbo.resource, NULL, &vm->last_update);
+				bo->tbo.resource, NULL, NULL, &vm->last_update);
 	if (r) {
 		dev_warn(adev->dev,
 			"Failed to map NPA addr (%llx) into NPA VM\n", npa_addr);
@@ -2649,7 +2649,7 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
 					npa_addr, npa_addr + size - 1,
 					pte_flags, 0,
 					adev->vm_manager.vram_base_offset,
-					bo->tbo.resource, NULL,
+					bo->tbo.resource, NULL, NULL,
 					&vm->last_update);
 
 		if (r)
@@ -2785,7 +2785,7 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
 
 		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
 					npa_addr, npa_addr + size - 1,
-					pte_value, 0, 0, NULL, NULL,
+					pte_value, 0, 0, NULL, NULL, NULL,
 					&vm->last_update);
 
 		if (r)
@@ -4572,7 +4572,8 @@ static int amdgpu_ualink_npa_vm_map_range(struct amdgpu_device *adev, struct amd
 	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
 				npa_in_pages, npa_in_pages + size_in_pages - 1,
 				pte_flags, offset, adev->vm_manager.vram_base_offset,
-				bo->tbo.resource, NULL, &npa_vm->last_update);
+				bo->tbo.resource, NULL, NULL,
+				&npa_vm->last_update);
 	if (r)
 		dev_dbg(adev->dev, "failed %d to map npa 0x%llx to NPA VM\n", r,
 			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
@@ -4606,7 +4607,7 @@ static int amdgpu_ualink_npa_vm_unmap_range(struct amdgpu_device *adev, struct a
 	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
 				npa_in_pages, npa_in_pages + size_in_pages - 1,
 				pte_flags, offset, 0, bo->tbo.resource, NULL,
-				&npa_vm->last_update);
+				NULL, &npa_vm->last_update);
 	if (r)
 		dev_dbg(adev->dev, "failed %d to unmap npa 0x%llx from NPA VM\n", r,
 			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 77ddb7fa96fb0..55bfe4eebc190 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1115,6 +1115,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
  * @vram_base: base for vram mappings
  * @res: ttm_resource to map
  * @pages_addr: DMA addresses to use for mapping
+ * @hmm_range: to check validity of DMA addresses
  * @fence: optional resulting fence
  *
  * Fill in the page table entries between @start and @last. Allocate and free
@@ -1128,7 +1129,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			struct amdgpu_sync *sync, uint64_t start,
 			uint64_t last, uint64_t flags, uint64_t offset,
 			uint64_t vram_base, struct ttm_resource *res,
-			dma_addr_t *pages_addr, struct dma_fence **fence)
+			dma_addr_t *pages_addr,
+			struct amdgpu_hmm_range *hmm_range,
+			struct dma_fence **fence)
 {
 	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
 	struct amdgpu_vm_update_params params;
@@ -1161,6 +1164,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 	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(&params.tlb_flush_waitlist);
@@ -1463,7 +1467,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
 		r = amdgpu_vm_map_range(adev, vm, flush_tlb, !uncached, &sync,
 					mapping->start, mapping->last,
 					update_flags, mapping->offset,
-					vram_base, mem, pages_addr,
+					vram_base, mem, pages_addr, NULL,
 					last_update);
 		if (r)
 			goto error_free;
@@ -1673,7 +1677,7 @@ 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, &f);
+					0, 0, 0, NULL, NULL, NULL, &f);
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
 			dma_fence_put(f);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index a6f6541758609..9b07df5a2b816 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -469,7 +469,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			struct amdgpu_sync *sync, uint64_t start,
 			uint64_t last, uint64_t flags, uint64_t offset,
 			uint64_t vram_base, struct ttm_resource *res,
-			dma_addr_t *pages_addr, struct dma_fence **fence);
+			dma_addr_t *pages_addr,
+			struct amdgpu_hmm_range *hmm_range,
+			struct dma_fence **fence);
 int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 			  bool unlocked, struct amdgpu_sync *sync,
 			  uint64_t start, uint64_t last,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
index db748e8b3bf38..8a0e5fcbd55ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
@@ -26,6 +26,7 @@
 
 #include <linux/types.h>
 #include <linux/list.h>
+#include "amdgpu_hmm.h"
 #include "amdgpu_vm.h"
 
 struct amdgpu_device;
@@ -72,6 +73,13 @@ struct amdgpu_vm_update_params {
 	 */
 	dma_addr_t *pages_addr;
 
+	/**
+	 * @hmm_range:
+	 *
+	 * Used to check the validity of pages_addr.
+	 */
+	struct amdgpu_hmm_range *hmm_range;
+
 	/**
 	 * @job: job to used for hw submission
 	 */
@@ -156,6 +164,8 @@ static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
 	p->saved_flags = memalloc_noreclaim_save();
 	if (p->vm->evicting)
 		return -EBUSY;
+	if (p->hmm_range && !amdgpu_hmm_range_valid(p->hmm_range))
+		return -EAGAIN;
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 263e68d29360d..8aebcfd58579b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1465,7 +1465,8 @@ svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start,
 static int
 svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
 		     unsigned long offset, unsigned long npages, bool readonly,
-		     dma_addr_t *dma_addr, struct amdgpu_device *bo_adev,
+		     dma_addr_t *dma_addr, struct amdgpu_hmm_range *hmm_range,
+		     struct amdgpu_device *bo_adev,
 		     struct dma_fence **fence, bool flush_tlb)
 {
 	struct amdgpu_device *adev = pdd->dev->adev;
@@ -1525,7 +1526,7 @@ 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, &vm->last_update);
+					NULL, dma_addr, hmm_range, &vm->last_update);
 
 		for (j = last_start - prange->start; j <= i; j++)
 			dma_addr[j] |= last_domain;
@@ -1552,7 +1553,9 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
 }
 
 static int
-svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
+svm_range_map_to_gpus(struct svm_range *prange,
+		      struct amdgpu_hmm_range *hmm_range,
+		      unsigned long offset,
 		      unsigned long npages, bool readonly,
 		      unsigned long *bitmap, bool wait, bool flush_tlb)
 {
@@ -1588,7 +1591,7 @@ svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
 		set_bit(gpuidx, prange->bitmap_mapped);
 
 		r = svm_range_map_to_gpu(pdd, prange, offset, npages, readonly,
-					 prange->dma_addr[gpuidx],
+					 prange->dma_addr[gpuidx], hmm_range,
 					 bo_adev, wait ? &fence : NULL,
 					 flush_tlb);
 		if (r)
@@ -1862,18 +1865,6 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 
 		svm_range_lock(prange);
 
-		/* Free backing memory of hmm_range if it was initialized
-		 * Override return value to TRY AGAIN only if prior returns
-		 * were successful
-		 */
-		if (range && !amdgpu_hmm_range_valid(range) && !r) {
-			pr_debug("hmm update the range, need validate again\n");
-			r = -EAGAIN;
-		}
-
-		/* Free the hmm range */
-		amdgpu_hmm_range_free(range);
-
 		if (!r && !list_empty(&prange->child_list)) {
 			pr_debug("range split by unmap in parallel, validate again\n");
 			r = -EAGAIN;
@@ -1885,11 +1876,16 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
 			if (map_start_vma <= map_last_vma) {
 				offset = map_start_vma - prange->start;
 				npages = map_last_vma - map_start_vma + 1;
-				r = svm_range_map_to_gpus(prange, offset, npages, readonly,
-							  ctx->bitmap, wait, flush_tlb);
+				r = svm_range_map_to_gpus(prange, range, offset,
+							  npages, readonly,
+							  ctx->bitmap, wait,
+							  flush_tlb);
 			}
 		}
 
+		/* Free the hmm range */
+		amdgpu_hmm_range_free(range);
+
 		if (!r && next == end)
 			prange->mapping_done = true;
 		else
-- 
2.43.0

Re: [PATCH 8/9] drm/amdgpu: fix the HMM range handling for KFD SVM v2

[Kuehling, Felix]

On 2026-09-28 11:10, Christian König wrote:
> It's mandatory that we have this check inside the VM handling or
> otherwise page table allocation and filling PTEs doesn't work correctly.
>
> This allows to remove the buggy SVM range lock, but that's not part of
> this patch set.
>
> Only compile tested!

@Philip, do you have time to test this? Do we need to ask someone else?

Thanks,
   Felix


>
> v2: fix broken compile after ualink merge
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c    | 13 ++++----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        | 10 ++++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  4 ++-
>   .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   | 10 ++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c          | 32 ++++++++-----------
>   5 files changed, 41 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> index b7975c77b1fe2..c1bb74cb45eda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ualink.c
> @@ -2058,7 +2058,7 @@ static int amdgpu_ualink_unmap_npa_addr(struct amdgpu_device *adev,
>   
>   	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
>   				npa_addr + size - 1, pte_value, 0, 0, NULL,
> -				NULL, &vm->last_update);
> +				NULL, NULL, &vm->last_update);
>   	if (r) {
>   		dev_err(adev->dev,
>   			"Failed to unmap NPA addr (%llx) from NPA VM\n", npa_addr);
> @@ -2111,7 +2111,7 @@ static int amdgpu_ualink_map_npa_addr(struct amdgpu_device *adev, u64 npa_addr,
>   	r = amdgpu_vm_map_range(adev, vm, true, false, NULL, npa_addr,
>   				npa_addr + size - 1, pte_flags, offset,
>   				adev->vm_manager.vram_base_offset,
> -				bo->tbo.resource, NULL, &vm->last_update);
> +				bo->tbo.resource, NULL, NULL, &vm->last_update);
>   	if (r) {
>   		dev_warn(adev->dev,
>   			"Failed to map NPA addr (%llx) into NPA VM\n", npa_addr);
> @@ -2649,7 +2649,7 @@ static void amdgpu_ualink_force_retry_rpcs(struct amdgpu_device *adev,
>   					npa_addr, npa_addr + size - 1,
>   					pte_flags, 0,
>   					adev->vm_manager.vram_base_offset,
> -					bo->tbo.resource, NULL,
> +					bo->tbo.resource, NULL, NULL,
>   					&vm->last_update);
>   
>   		if (r)
> @@ -2785,7 +2785,7 @@ static void amdgpu_ualink_unmap_all_npa_addr(struct amdgpu_device *adev,
>   
>   		r = amdgpu_vm_map_range(adev, vm, true, false, NULL,
>   					npa_addr, npa_addr + size - 1,
> -					pte_value, 0, 0, NULL, NULL,
> +					pte_value, 0, 0, NULL, NULL, NULL,
>   					&vm->last_update);
>   
>   		if (r)
> @@ -4572,7 +4572,8 @@ static int amdgpu_ualink_npa_vm_map_range(struct amdgpu_device *adev, struct amd
>   	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
>   				npa_in_pages, npa_in_pages + size_in_pages - 1,
>   				pte_flags, offset, adev->vm_manager.vram_base_offset,
> -				bo->tbo.resource, NULL, &npa_vm->last_update);
> +				bo->tbo.resource, NULL, NULL,
> +				&npa_vm->last_update);
>   	if (r)
>   		dev_dbg(adev->dev, "failed %d to map npa 0x%llx to NPA VM\n", r,
>   			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
> @@ -4606,7 +4607,7 @@ static int amdgpu_ualink_npa_vm_unmap_range(struct amdgpu_device *adev, struct a
>   	r = amdgpu_vm_map_range(adev, npa_vm, true, false, NULL,
>   				npa_in_pages, npa_in_pages + size_in_pages - 1,
>   				pte_flags, offset, 0, bo->tbo.resource, NULL,
> -				&npa_vm->last_update);
> +				NULL, &npa_vm->last_update);
>   	if (r)
>   		dev_dbg(adev->dev, "failed %d to unmap npa 0x%llx from NPA VM\n", r,
>   			npa_in_pages << AMDGPU_GPU_PAGE_SHIFT);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 77ddb7fa96fb0..55bfe4eebc190 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1115,6 +1115,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>    * @vram_base: base for vram mappings
>    * @res: ttm_resource to map
>    * @pages_addr: DMA addresses to use for mapping
> + * @hmm_range: to check validity of DMA addresses
>    * @fence: optional resulting fence
>    *
>    * Fill in the page table entries between @start and @last. Allocate and free
> @@ -1128,7 +1129,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			struct amdgpu_sync *sync, uint64_t start,
>   			uint64_t last, uint64_t flags, uint64_t offset,
>   			uint64_t vram_base, struct ttm_resource *res,
> -			dma_addr_t *pages_addr, struct dma_fence **fence)
> +			dma_addr_t *pages_addr,
> +			struct amdgpu_hmm_range *hmm_range,
> +			struct dma_fence **fence)
>   {
>   	struct amdgpu_vm_tlb_seq_struct *tlb_cb;
>   	struct amdgpu_vm_update_params params;
> @@ -1161,6 +1164,7 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	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(&params.tlb_flush_waitlist);
> @@ -1463,7 +1467,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>   		r = amdgpu_vm_map_range(adev, vm, flush_tlb, !uncached, &sync,
>   					mapping->start, mapping->last,
>   					update_flags, mapping->offset,
> -					vram_base, mem, pages_addr,
> +					vram_base, mem, pages_addr, NULL,
>   					last_update);
>   		if (r)
>   			goto error_free;
> @@ -1673,7 +1677,7 @@ 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, &f);
> +					0, 0, 0, NULL, NULL, NULL, &f);
>   		amdgpu_vm_free_mapping(adev, vm, mapping, f);
>   		if (r) {
>   			dma_fence_put(f);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index a6f6541758609..9b07df5a2b816 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -469,7 +469,9 @@ int amdgpu_vm_map_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			struct amdgpu_sync *sync, uint64_t start,
>   			uint64_t last, uint64_t flags, uint64_t offset,
>   			uint64_t vram_base, struct ttm_resource *res,
> -			dma_addr_t *pages_addr, struct dma_fence **fence);
> +			dma_addr_t *pages_addr,
> +			struct amdgpu_hmm_range *hmm_range,
> +			struct dma_fence **fence);
>   int amdgpu_vm_unmap_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   			  bool unlocked, struct amdgpu_sync *sync,
>   			  uint64_t start, uint64_t last,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> index db748e8b3bf38..8a0e5fcbd55ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -26,6 +26,7 @@
>   
>   #include <linux/types.h>
>   #include <linux/list.h>
> +#include "amdgpu_hmm.h"
>   #include "amdgpu_vm.h"
>   
>   struct amdgpu_device;
> @@ -72,6 +73,13 @@ struct amdgpu_vm_update_params {
>   	 */
>   	dma_addr_t *pages_addr;
>   
> +	/**
> +	 * @hmm_range:
> +	 *
> +	 * Used to check the validity of pages_addr.
> +	 */
> +	struct amdgpu_hmm_range *hmm_range;
> +
>   	/**
>   	 * @job: job to used for hw submission
>   	 */
> @@ -156,6 +164,8 @@ static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
>   	p->saved_flags = memalloc_noreclaim_save();
>   	if (p->vm->evicting)
>   		return -EBUSY;
> +	if (p->hmm_range && !amdgpu_hmm_range_valid(p->hmm_range))
> +		return -EAGAIN;
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 263e68d29360d..8aebcfd58579b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1465,7 +1465,8 @@ svm_range_unmap_from_gpus(struct svm_range *prange, unsigned long start,
>   static int
>   svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>   		     unsigned long offset, unsigned long npages, bool readonly,
> -		     dma_addr_t *dma_addr, struct amdgpu_device *bo_adev,
> +		     dma_addr_t *dma_addr, struct amdgpu_hmm_range *hmm_range,
> +		     struct amdgpu_device *bo_adev,
>   		     struct dma_fence **fence, bool flush_tlb)
>   {
>   	struct amdgpu_device *adev = pdd->dev->adev;
> @@ -1525,7 +1526,7 @@ 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, &vm->last_update);
> +					NULL, dma_addr, hmm_range, &vm->last_update);
>   
>   		for (j = last_start - prange->start; j <= i; j++)
>   			dma_addr[j] |= last_domain;
> @@ -1552,7 +1553,9 @@ svm_range_map_to_gpu(struct kfd_process_device *pdd, struct svm_range *prange,
>   }
>   
>   static int
> -svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
> +svm_range_map_to_gpus(struct svm_range *prange,
> +		      struct amdgpu_hmm_range *hmm_range,
> +		      unsigned long offset,
>   		      unsigned long npages, bool readonly,
>   		      unsigned long *bitmap, bool wait, bool flush_tlb)
>   {
> @@ -1588,7 +1591,7 @@ svm_range_map_to_gpus(struct svm_range *prange, unsigned long offset,
>   		set_bit(gpuidx, prange->bitmap_mapped);
>   
>   		r = svm_range_map_to_gpu(pdd, prange, offset, npages, readonly,
> -					 prange->dma_addr[gpuidx],
> +					 prange->dma_addr[gpuidx], hmm_range,
>   					 bo_adev, wait ? &fence : NULL,
>   					 flush_tlb);
>   		if (r)
> @@ -1862,18 +1865,6 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   
>   		svm_range_lock(prange);
>   
> -		/* Free backing memory of hmm_range if it was initialized
> -		 * Override return value to TRY AGAIN only if prior returns
> -		 * were successful
> -		 */
> -		if (range && !amdgpu_hmm_range_valid(range) && !r) {
> -			pr_debug("hmm update the range, need validate again\n");
> -			r = -EAGAIN;
> -		}
> -
> -		/* Free the hmm range */
> -		amdgpu_hmm_range_free(range);
> -
>   		if (!r && !list_empty(&prange->child_list)) {
>   			pr_debug("range split by unmap in parallel, validate again\n");
>   			r = -EAGAIN;
> @@ -1885,11 +1876,16 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   			if (map_start_vma <= map_last_vma) {
>   				offset = map_start_vma - prange->start;
>   				npages = map_last_vma - map_start_vma + 1;
> -				r = svm_range_map_to_gpus(prange, offset, npages, readonly,
> -							  ctx->bitmap, wait, flush_tlb);
> +				r = svm_range_map_to_gpus(prange, range, offset,
> +							  npages, readonly,
> +							  ctx->bitmap, wait,
> +							  flush_tlb);
>   			}
>   		}
>   
> +		/* Free the hmm range */
> +		amdgpu_hmm_range_free(range);
> +
>   		if (!r && next == end)
>   			prange->mapping_done = true;
>   		else

Re: [PATCH 8/9] drm/amdgpu: fix the HMM range handling for KFD SVM v2

[Olivier Kaloudoff]

Hi Felix,

> @Philip, do you have time to test this? Do we need to ask someone
> else?

Independent datapoint: I've been running this patch (as part of the
Sep-30 8-patch set, hand-backported to 7.2.6) on gfx906 / MI50 under
exactly the workload it targets - KFD SVM map/unmap/evict/restore churn
from ROCm inference (vLLM model staging + llama.cpp) plus memory
pressure.

Previously this setup produced deterministic MMHUB "PTE not valid"
faults and SVM restore-worker wedges on every run. With the series:
zero faults across sustained sessions.

So at least on GFX9-class hardware with the classic KFD SVM path, this
version of the HMM range handling is behaving correctly. Obviously
Philip's coverage on the NPA/UALink side is still needed - we have no
such hardware.

Tested-by: Olivier Kaloudoff <olivier.kaloudoff@gmail.com>

Regards,
Olivier

[PATCH 9/9] drm/amdgpu: use range unmap in amdgpu_vm_clear_freed

[Christian König]

Instead of abusing the mapping function. Has the clear advantage that
unmap_range can't fail with page tables allocation should anything go
wrong.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 55bfe4eebc190..bdbd40e29528b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1675,9 +1675,8 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
 			struct amdgpu_bo_va_mapping, list);
 		list_del(&mapping->list);
 
-		r = amdgpu_vm_map_range(adev, vm, true, false,
-					&sync, mapping->start, mapping->last,
-					0, 0, 0, NULL, NULL, NULL, &f);
+		r = amdgpu_vm_unmap_range(adev, vm, false, &sync,
+					  mapping->start, mapping->last, &f);
 		amdgpu_vm_free_mapping(adev, vm, mapping, f);
 		if (r) {
 			dma_fence_put(f);
-- 
2.43.0

Re: [PATCH 1/9] drm/amdgpu: rework eviction lock handling into critical section v3

[Timur Kristóf]

On 2026. szeptember 28., hétfő 11:10:33 keleti államokbeli nyári idő Christian 
König wrote:
> Taking the eviction lock is actually just one step which we need to do
> in the critical section handling.
> 
> Rename the functions to reflect that, use the update parameters instead of
> the vm to save the GFP flags.
> 
> v2: rebased and reordered
> v3: fix rebase artefact, fix error handling in amdgpu_vm_pt_alloc
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Natalie Vock <nat@pixelcluster.dev>

Reviewed-by: Timur Kristóf <timur.kristof@gmail.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  8 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  1 -
>  .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   | 43 ++++++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 40 ++++++++---------
>  4 files changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 29a66e39f3d60..7b9494375649f
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1170,11 +1170,9 @@ int amdgpu_vm_update_range(struct amdgpu_device
> *adev, struct amdgpu_vm *vm, params.override_pte = allow_override &&
> adev->gmc.override_pte;
>  	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
> 
> -	amdgpu_vm_eviction_lock(vm);
> -	if (vm->evicting) {
> -		r = -EBUSY;
> +	r = amdgpu_vm_begin_critical(&params);
> +	if (r)
>  		goto error_free;
> -	}
> 
>  	if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) {
>  		struct dma_fence *tmp = dma_fence_get_stub();
> @@ -1258,7 +1256,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev,
> struct amdgpu_vm *vm,
> 
>  error_free:
>  	kfree(tlb_cb);
> -	amdgpu_vm_eviction_unlock(vm);
> +	amdgpu_vm_end_critical(&params);
>  	drm_dev_exit(idx);
>  	return r;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index 0f6634b749746..98cdd7e3475fb
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -289,7 +289,6 @@ struct amdgpu_vm {
>  	 */
>  	struct mutex		eviction_lock;
>  	bool			evicting;
> -	unsigned int		saved_flags;
> 
>  	/* Memory statistics for this vm, protected by stats_lock */
>  	spinlock_t		stats_lock;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index
> ca86eaac75235..8ebb0b033291e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -51,7 +51,7 @@ struct amdgpu_vm_update_params {
>  	struct amdgpu_device *adev;
> 
>  	/**
> -	 * @vm: optional amdgpu_vm we do this update for
> +	 * @vm: amdgpu_vm we do this update for
>  	 */
>  	struct amdgpu_vm *vm;
> 
> @@ -93,6 +93,11 @@ struct amdgpu_vm_update_params {
>  	 */
>  	bool override_pte;
> 
> +	/**
> +	 * @saved_flags: Saved flags for GFP reduction.
> +	 */
> +	unsigned int saved_flags;
> +
>  	/**
>  	 * @tlb_flush_waitlist: temporary storage for BOs until tlb_flush
>  	 */
> @@ -126,21 +131,37 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device
> *adev, struct amdgpu_vm_update_params *params);
>  int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm
> *vm);
> 
> -/*
> - * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
> - * happens while holding this lock anywhere to prevent deadlocks when
> - * an MMU notifier runs in reclaim-FS context.
> +/**
> + * amdgpu_vm_begin_critical - start the critical section of the update
> + * @p: The update parameters
> + *
> + * 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. + *
> + * Returns:
> + *
> + * 0 on success or a negative error code on failure.
> + * Even on error amdgpu_vm_end_critical() must still be called to clean up!
> */
> -static inline void amdgpu_vm_eviction_lock(struct amdgpu_vm *vm)
> +static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params
> *p) {
> -	mutex_lock(&vm->eviction_lock);
> -	vm->saved_flags = memalloc_noreclaim_save();
> +	mutex_lock(&p->vm->eviction_lock);
> +	p->saved_flags = memalloc_noreclaim_save();
> +	if (p->vm->evicting)
> +		return -EBUSY;
> +	return 0;
>  }
> 
> -static inline void amdgpu_vm_eviction_unlock(struct amdgpu_vm *vm)
> +/**
> + * amdgpu_vm_end_critical - end the critical section of the update
> + * @p: The update parameters
> + *
> + * Restore the GFP flags and drop the lock.
> + */
> +static inline void amdgpu_vm_end_critical(struct amdgpu_vm_update_params
> *p) {
> -	memalloc_noreclaim_restore(vm->saved_flags);
> -	mutex_unlock(&vm->eviction_lock);
> +	memalloc_noreclaim_restore(p->saved_flags);
> +	mutex_unlock(&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
> 1cdf2b854f261..c03327f1242d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -505,52 +505,51 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev,
> struct amdgpu_vm *vm, /**
>   * amdgpu_vm_pt_alloc - Allocate a specific page table
>   *
> - * @adev: amdgpu_device pointer
> - * @vm: VM to allocate page tables for
> + * @p: see amdgpu_vm_update_params definition
>   * @cursor: Which page table to allocate
> - * @immediate: use an immediate update
>   *
>   * Make sure a specific page table or directory is allocated.
>   *
>   * Returns:
> - * 1 if page table needed to be allocated, 0 if page table was already
> - * allocated, negative errno if an error occurred.
> + *
> + * 0 on success or a negative error code on failure.
>   */
> -static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
> -			      struct amdgpu_vm *vm,
> -			      struct amdgpu_vm_pt_cursor *cursor,
> -			      bool immediate)
> +static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
> +			      struct amdgpu_vm_pt_cursor *cursor)
>  {
>  	struct amdgpu_vm_bo_base *entry = cursor->entry;
>  	struct amdgpu_bo *pt_bo;
>  	struct amdgpu_bo_vm *pt;
> -	int r;
> +	int r, r2;
> 
>  	if (entry->bo)
>  		return 0;
> 
> -	amdgpu_vm_eviction_unlock(vm);
> -	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt,
> -				vm->root.bo->xcp_id);
> -	amdgpu_vm_eviction_lock(vm);
> +	amdgpu_vm_end_critical(p);
> +	r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, p-
>immediate,
> +				&pt, p->vm->root.bo->xcp_id);
> +	r2 = amdgpu_vm_begin_critical(p);
>  	if (r)
>  		return r;
> +	if (r2)
> +		goto error_free_pt;
> 
>  	/* 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, vm, pt_bo);
> -	r = amdgpu_vm_pt_clear(adev, vm, pt, immediate);
> +	amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
> +	r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate);
>  	if (r)
> -		goto error_free_pt;
> +		goto error_unpin;
> 
>  	return 0;
> 
> -error_free_pt:
> -	if (vm->is_npa)
> +error_unpin:
> +	if (p->vm->is_npa)
>  		amdgpu_bo_unpin(pt_bo);
> +error_free_pt:
>  	amdgpu_bo_unref(&pt_bo);
>  	return r;
>  }
> @@ -838,8 +837,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params
> *params, /* make sure that the page tables covering the
>  			 * address range are actually allocated
>  			 */
> -			r = amdgpu_vm_pt_alloc(params->adev, params-
>vm,
> -					       &cursor, 
params->immediate);
> +			r = amdgpu_vm_pt_alloc(params, &cursor);
>  			if (r)
>  				return r;
>  		}

Re: [PATCH 1/9] drm/amdgpu: rework eviction lock handling into critical section v3

[Huang, Honglei]

Will rebase the amdgpu drm svm on this series.

Regards,
Honglei

On 9/28/2026 11:10 PM, Christian König wrote:
> Taking the eviction lock is actually just one step which we need to do
> in the critical section handling.
> 
> Rename the functions to reflect that, use the update parameters instead of the
> vm to save the GFP flags.
> 
> v2: rebased and reordered
> v3: fix rebase artefact, fix error handling in amdgpu_vm_pt_alloc
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Reviewed-by: Natalie Vock <nat@pixelcluster.dev>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c        |  8 ++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h        |  1 -
>   .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h   | 43 ++++++++++++++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c     | 40 ++++++++---------
>   4 files changed, 54 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 29a66e39f3d60..7b9494375649f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1170,11 +1170,9 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   	params.override_pte = allow_override && adev->gmc.override_pte;
>   	INIT_LIST_HEAD(&params.tlb_flush_waitlist);
>   
> -	amdgpu_vm_eviction_lock(vm);
> -	if (vm->evicting) {
> -		r = -EBUSY;
> +	r = amdgpu_vm_begin_critical(&params);
> +	if (r)
>   		goto error_free;
> -	}
>   
>   	if (!unlocked && !dma_fence_is_signaled(vm->last_unlocked)) {
>   		struct dma_fence *tmp = dma_fence_get_stub();
> @@ -1258,7 +1256,7 @@ int amdgpu_vm_update_range(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   
>   error_free:
>   	kfree(tlb_cb);
> -	amdgpu_vm_eviction_unlock(vm);
> +	amdgpu_vm_end_critical(&params);
>   	drm_dev_exit(idx);
>   	return r;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index 0f6634b749746..98cdd7e3475fb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -289,7 +289,6 @@ struct amdgpu_vm {
>   	 */
>   	struct mutex		eviction_lock;
>   	bool			evicting;
> -	unsigned int		saved_flags;
>   
>   	/* Memory statistics for this vm, protected by stats_lock */
>   	spinlock_t		stats_lock;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> index ca86eaac75235..8ebb0b033291e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -51,7 +51,7 @@ struct amdgpu_vm_update_params {
>   	struct amdgpu_device *adev;
>   
>   	/**
> -	 * @vm: optional amdgpu_vm we do this update for
> +	 * @vm: amdgpu_vm we do this update for
>   	 */
>   	struct amdgpu_vm *vm;
>   
> @@ -93,6 +93,11 @@ struct amdgpu_vm_update_params {
>   	 */
>   	bool override_pte;
>   
> +	/**
> +	 * @saved_flags: Saved flags for GFP reduction.
> +	 */
> +	unsigned int saved_flags;
> +
>   	/**
>   	 * @tlb_flush_waitlist: temporary storage for BOs until tlb_flush
>   	 */
> @@ -126,21 +131,37 @@ void amdgpu_vm_pt_free_list(struct amdgpu_device *adev,
>   			    struct amdgpu_vm_update_params *params);
>   int amdgpu_vm_pt_map_tables(struct amdgpu_device *adev, struct amdgpu_vm *vm);
>   
> -/*
> - * vm eviction_lock can be taken in MMU notifiers. Make sure no reclaim-FS
> - * happens while holding this lock anywhere to prevent deadlocks when
> - * an MMU notifier runs in reclaim-FS context.
> +/**
> + * amdgpu_vm_begin_critical - start the critical section of the update
> + * @p: The update parameters
> + *
> + * 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.
> + *
> + * Returns:
> + *
> + * 0 on success or a negative error code on failure.
> + * Even on error amdgpu_vm_end_critical() must still be called to clean up!
>    */
> -static inline void amdgpu_vm_eviction_lock(struct amdgpu_vm *vm)
> +static inline int amdgpu_vm_begin_critical(struct amdgpu_vm_update_params *p)
>   {
> -	mutex_lock(&vm->eviction_lock);
> -	vm->saved_flags = memalloc_noreclaim_save();
> +	mutex_lock(&p->vm->eviction_lock);
> +	p->saved_flags = memalloc_noreclaim_save();
> +	if (p->vm->evicting)
> +		return -EBUSY;
> +	return 0;
>   }
>   
> -static inline void amdgpu_vm_eviction_unlock(struct amdgpu_vm *vm)
> +/**
> + * amdgpu_vm_end_critical - end the critical section of the update
> + * @p: The update parameters
> + *
> + * Restore the GFP flags and drop the lock.
> + */
> +static inline void amdgpu_vm_end_critical(struct amdgpu_vm_update_params *p)
>   {
> -	memalloc_noreclaim_restore(vm->saved_flags);
> -	mutex_unlock(&vm->eviction_lock);
> +	memalloc_noreclaim_restore(p->saved_flags);
> +	mutex_unlock(&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 1cdf2b854f261..c03327f1242d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -505,52 +505,51 @@ int amdgpu_vm_pt_create(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   /**
>    * amdgpu_vm_pt_alloc - Allocate a specific page table
>    *
> - * @adev: amdgpu_device pointer
> - * @vm: VM to allocate page tables for
> + * @p: see amdgpu_vm_update_params definition
>    * @cursor: Which page table to allocate
> - * @immediate: use an immediate update
>    *
>    * Make sure a specific page table or directory is allocated.
>    *
>    * Returns:
> - * 1 if page table needed to be allocated, 0 if page table was already
> - * allocated, negative errno if an error occurred.
> + *
> + * 0 on success or a negative error code on failure.
>    */
> -static int amdgpu_vm_pt_alloc(struct amdgpu_device *adev,
> -			      struct amdgpu_vm *vm,
> -			      struct amdgpu_vm_pt_cursor *cursor,
> -			      bool immediate)
> +static int amdgpu_vm_pt_alloc(struct amdgpu_vm_update_params *p,
> +			      struct amdgpu_vm_pt_cursor *cursor)
>   {
>   	struct amdgpu_vm_bo_base *entry = cursor->entry;
>   	struct amdgpu_bo *pt_bo;
>   	struct amdgpu_bo_vm *pt;
> -	int r;
> +	int r, r2;
>   
>   	if (entry->bo)
>   		return 0;
>   
> -	amdgpu_vm_eviction_unlock(vm);
> -	r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate, &pt,
> -				vm->root.bo->xcp_id);
> -	amdgpu_vm_eviction_lock(vm);
> +	amdgpu_vm_end_critical(p);
> +	r = amdgpu_vm_pt_create(p->adev, p->vm, cursor->level, p->immediate,
> +				&pt, p->vm->root.bo->xcp_id);
> +	r2 = amdgpu_vm_begin_critical(p);
>   	if (r)
>   		return r;
> +	if (r2)
> +		goto error_free_pt;
>   
>   	/* 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, vm, pt_bo);
> -	r = amdgpu_vm_pt_clear(adev, vm, pt, immediate);
> +	amdgpu_vm_bo_base_init(entry, p->vm, pt_bo);
> +	r = amdgpu_vm_pt_clear(p->adev, p->vm, pt, p->immediate);
>   	if (r)
> -		goto error_free_pt;
> +		goto error_unpin;
>   
>   	return 0;
>   
> -error_free_pt:
> -	if (vm->is_npa)
> +error_unpin:
> +	if (p->vm->is_npa)
>   		amdgpu_bo_unpin(pt_bo);
> +error_free_pt:
>   	amdgpu_bo_unref(&pt_bo);
>   	return r;
>   }
> @@ -838,8 +837,7 @@ int amdgpu_vm_ptes_update(struct amdgpu_vm_update_params *params,
>   			/* make sure that the page tables covering the
>   			 * address range are actually allocated
>   			 */
> -			r = amdgpu_vm_pt_alloc(params->adev, params->vm,
> -					       &cursor, params->immediate);
> +			r = amdgpu_vm_pt_alloc(params, &cursor);
>   			if (r)
>   				return r;
>   		}