[PATCH] drm/amdgpu: Update invalid PTE flag setting

[Christian König]

Am 06.04.23 um 17:01 schrieb Felix Kuehling:
> Am 2023-04-06 um 10:36 schrieb Christian König:
>> Am 04.04.23 um 23:59 schrieb Mukul Joshi:
>>> Update the invalid PTE flag setting to ensure, in addition
>>> to transitioning the retry fault to a no-retry fault, it
>>> also causes the wavefront to enter the trap handler. With the
>>> current setting, it only transitions to a no-retry fault.
>>
>> Well that was actually the intended result. Why should it enter the 
>> trap handler here?
>
> We need the trap handler for reporting the fault to the debugger. The 
> VM fault interrupt itself doesn't provide sufficient information to 
> find the wave and the program counter that triggered the fault. It 
> also doesn't work well when several waves are faulting on the same 
> address.

Interesting. But then we still have the problem that VSP set is actually 
a valid combination when this is applied to a non PTB.

So that really doesn't look like a good idea to me.

Regards,
Christian.

>
> Regards,
>   Felix
>
>
>>
>>>
>>> Signed-off-by: Mukul Joshi <mukul.joshi at amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index af6f26a97fc5..5df4f7bb241f 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2488,7 +2488,7 @@ bool amdgpu_vm_handle_fault(struct 
>>> amdgpu_device *adev, u32 pasid,
>>>           /* Intentionally setting invalid PTE flag
>>>            * combination to force a no-retry-fault
>>>            */
>>> -        flags = AMDGPU_PTE_SNOOPED | AMDGPU_PTE_PRT;
>>> +        flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SYSTEM | AMDGPU_PTE_PRT;
>>
>> As far as I can see this is actually a valid combination and would 
>> not have the desired result.
>>
>> Regards,
>> Christian.
>>
>>>           value = 0;
>>>       } else if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
>>>           /* Redirect the access to the dummy page */
>>