[PATCH 2/2] amd/amdgpu: force to trigger a no-retry-fault after a retry-fault

[Christian König]

Am 19.11.19 um 17:45 schrieb Felix Kuehling:
> On 2019-11-19 11:37, Alex Sierra wrote:
>> Only for the debugger use case.
>>
>> [why]
>> Avoid endless translation retries, after an invalid address access has
>> been issued to the GPU. Instead, the trap handler is forced to enter by
>> generating a no-retry-fault.
>> A s_trap instruction is inserted in the debugger case to let the wave to
>> enter trap handler to save context.
>>
>> [how]
>> Intentionally using an invalid flag combination (F and P set at the same
>> time) to trigger a no-retry-fault, after a retry-fault happens. This is
>> only valid under compute context.
>>
>> Change-Id: I4180c30e2631dc0401cbd6171f8a6776e4733c9a
>> Signed-off-by: Alex Sierra <alex.sierra at amd.com>
>
> This commit adds some unnecessary empty lines. See inline. With that 
> fixed, the series is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>

I actually like the empty lines, nicely emphases that a new block starts.

Maybe note in the code comment that the flags combination is 
intentionally invalid as well.

But either way the series is Reviewed-by: Christian König 
<christian.koenig at amd.com>.

Regards,
Christian.

>
> Please also give Christian a chance to review.
>
> Thanks,
>   Felix
>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index d51ac8771ae0..cd36195ff8be 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -3202,11 +3202,18 @@ bool amdgpu_vm_handle_fault(struct 
>> amdgpu_device *adev, unsigned int pasid,
>>       flags = AMDGPU_PTE_VALID | AMDGPU_PTE_SNOOPED |
>>           AMDGPU_PTE_SYSTEM;
>>   -    if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
>> +    if (vm->is_compute_context) {
>> +        /* Setting PTE flags to trigger a no-retry-fault  */
>> +        flags = AMDGPU_PTE_EXECUTABLE | AMDGPU_PDE_PTE |
>> +            AMDGPU_PTE_TF;
>> +        value = 0;
>> +
> Unnecessary blank line.
>> +    } else if (amdgpu_vm_fault_stop == AMDGPU_VM_FAULT_STOP_NEVER) {
>>           /* Redirect the access to the dummy page */
>>           value = adev->dummy_page_addr;
>>           flags |= AMDGPU_PTE_EXECUTABLE | AMDGPU_PTE_READABLE |
>>               AMDGPU_PTE_WRITEABLE;
>> +
> Unnecessary blank line.
>>       } else {
>>           /* Let the hw retry silently on the PTE */
>>           value = 0;
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx