Re: [PATCH 10/14] drm/amdgpu/vm: Use init PTE flags and NOALLOC in amdgpu_vm_handle_fault()

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

On 2026. július 2., csütörtök 12:22:23 közép-európai nyári idő Christian König 
wrote:
> On 7/1/26 18:17, Timur Kristóf wrote:
> > PTE_IS_PTE seems necessary for handling retry faults on GFX12.
> > 
> > For reference see:
> > amdgpu_vm_pte_update_flags() that explains the problem
> > svm_range_get_pte_flags() that uses the flag on GFX12
> > 
> > Also add NOALLOC on GFX10.3+ as we don't need to allocate
> > the fault handling PTE in the infinity cache (MALL).
> > 
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> > 
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index 32719f31b6c9..a915d061085f
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -3044,7 +3044,8 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> > *adev, u32 pasid,> 
> >  	}
> >  	
> >  	addr /= AMDGPU_GPU_PAGE_SIZE;
> > 
> > -	flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
> > +	flags = adev->gmc.init_pte_flags |
> > +		AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
> > 
> >  		AMDGPU_PTE_SYSTEM;
> 
> That doesn't make sense the PTE flags should be adjusted to the device
> specific flags by the callbacks.

Can you please elaborate on what is it that doesn't make sense here and why, 
and what you recommend to do instead?

> >  	if (is_compute_context) {
> > 
> > @@ -3054,11 +3055,14 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device
> > *adev, u32 pasid,> 
> >  		flags = AMDGPU_VM_NORETRY_FLAGS;
> >  		value = 0;
> >  	
> >  	} else if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
> > 
> > +		/* Don't allocate this PTE in the MALL */
> > +		if (amdgpu_ip_version(adev, GC_HWIP, 0) >= 
IP_VERSION(10, 3, 0))
> > +			flags |= AMDGPU_PTE_NOALLOC;
> > +
> 
> This doesn't make sense either, mall allocation for the dummy page should be
> perfectly fine.

I think we shouldn't waste space for this PTE in the MALL.

> 
> >  		/* Redirect the access to the dummy page */
> >  		value = adev->dummy_page_addr;
> >  		flags |= AMDGPU_PTE_EXECUTABLE | AMDGPU_PTE_READABLE |
> >  		
> >  			AMDGPU_PTE_WRITEABLE;
> > 
> > -
> > 
> >  	} else {
> >  	
> >  		/* Let the hw retry silently on the PTE */
> >  		value = 0;