On Thursday, September 17, 2026 1:01:49 PM Central European Summer Time Huang
Yu wrote:
> From: Christian König <[email protected]>
>
> 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.
>
> Signed-off-by: Christian König <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 55 +++++++++++-----
> .../gpu/drm/amd/amdgpu/amdgpu_vm_internal.h | 3 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 62 +++++++++++++++++++
> 3 files changed, 105 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 5675925ca656..e60143d4990c
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -3013,10 +3013,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 *values, flags;
> struct amdgpu_vm *vm;
> + struct drm_exec exec;
> + unsigned int idx;
> int r;
>
> drm_exec_init(&exec, 0, 1);
> @@ -3060,24 +3062,21 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> *adev, u32 pasid, }
>
> addr /= AMDGPU_GPU_PAGE_SIZE;
> - 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;
> + values = 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;
> -
> + flags = 0;
Why do you set the flags to zero here?
> + values = adev->vm_manager.dummy_dst;
> } else {
> /* Let the hw retry silently on the PTE */
> - value = 0;
> + flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
> + AMDGPU_PTE_SYSTEM;
> + values = NULL;
> }
>
> r = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
> @@ -3086,12 +3085,38 @@ 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;
> + }
>
> - r = amdgpu_vm_update_pdes(adev, vm, true);
> + amdgpu_vm_eviction_lock(vm);
> + if (vm->evicting) {
> + r = -EBUSY;
> + goto error_dev_exit;
> + }
> +
> + memset(¶ms, 0, sizeof(params));
> + params.adev = adev;
> + params.vm = vm;
> + params.immediate = true;
> + params.pages_addr = NULL;
> +
> + r = vm->update_funcs->prepare(¶ms, NULL,
> +
AMDGPU_KERNEL_JOB_ID_VM_UPDATE_PDES);
> + if (r)
> + goto error_dev_exit;
> +
> + amdgpu_vm_pt_leaves(¶ms, addr, addr, values, flags);
> +
> + r = vm->update_funcs->commit(¶ms, &vm->last_update);
I think this shouldn't pass the last VM update.
Note that amdgpu_vm_sdma_commit() doesn't return a fence anyway when
params.immediate is true, so I think it's OK to just pass NULL here.
> + if (r)
> + goto error_dev_exit;
> +
> + amdgpu_vm_eviction_unlock(vm);
> +
> +error_dev_exit:
> + drm_dev_exit(idx);
>
> error_unlock:
> drm_exec_fini(&exec);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h index
> 16cc0d5f5894..7d3c0904e1a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_internal.h
> @@ -121,6 +121,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
> 3a6449d60988..2f542c731447 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c
> @@ -936,6 +936,68 @@ int amdgpu_vm_ptes_update(struct
> amdgpu_vm_update_params *params, return 0;
> }
>
> +/**
> + * amdgpu_vm_pt_leaves - update leave PDEs/PTEs
Typo: should be "leaf" instead of "leave".
> + *
> + * @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: mapping flags
> + *
> + * Update the leave 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) {
> + uint64_t entry_end, pe_start, val;
> + unsigned int shift, mask;
> + struct amdgpu_bo *pt;
> + unsigned int nptes;
> +
> + /* Walk to the leave entries */
> + if (amdgpu_vm_pt_descendant(adev, &cursor))
> + continue;
> +
> + pt = cursor.parent->bo;
> + shift = amdgpu_vm_pt_level_shift(adev, cursor.level -
1);
> + mask = amdgpu_vm_pt_entries_mask(adev, cursor.level -
1);
> +
> + /* Looks good so far, calculate parameters for the
update */
> + pe_start = ((cursor.pfn >> shift) & mask) * 8;
> +
> + 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;
> + /*
> + * This can happen when we set higher level PDEs to
unmap and/or
> + * silent to stop fault floods.
> + */
> + nptes = max(nptes, 1u);
> +
> + val = dst ? dst[cursor.level] : 0;
> + trace_amdgpu_vm_update_ptes(params, cursor.pfn,
entry_end,
> + min(nptes, 32u),
val, 0, flags);
> +
> + params->vm->update_funcs->update(params,
to_amdgpu_bo_vm(pt),
> + pe_start,
val, nptes, 0,
> + flags);
> + amdgpu_vm_pt_next(adev, &cursor);
> + }
> +}
> +
> /**
> * amdgpu_vm_pt_map_tables - have bo of root PD cpu accessible
> * @adev: amdgpu device structure
