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 <[email protected]>
> ---
> 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.
>
> 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.
Regards,
Christian.
> /* 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;
