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

["Timur Kristóf" <timur.kristof@gmail.com>]

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);
> +}