On 7/2/26 13:28, Timur Kristóf wrote:
> 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 <[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.
>
> Can you please elaborate on what is it that doesn't make sense here and why,
> and what you recommend to do instead?
I had to dig up what the problem here is as well. We use the ASIC specific
AMDGPU_PTE_* flags directly.
Instead we should use the AMDGPU_VM_PAGE_* flags and then call
amdgpu_gmc_get_vm_pte(adev, vm, NULL, in_flags, &out_flags) to translate the
AMDGPU_VM_PAGE_* flags into the ASIC specific ones.
This will automatically add flags like AMDGPU_PTE_TF and AMDGPU_PTE_IS_PTE
etc...
>>> 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.
Hui? The PTE doesn't enter the MALL, it is the dummy page which enters the MALL
and since it is only 4k it basically doesn't waste much space.
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;
>
>
>
>
