[PATCH v2] drm/amdgpu: Verify root PD is mapped into kernel address space.

[Christian König]

Am 06.07.2018 um 16:22 schrieb Andrey Grodzovsky:
>
>
> On 07/06/2018 09:55 AM, Christian König wrote:
>> Am 06.07.2018 um 15:50 schrieb Andrey Grodzovsky:
>>>
>>>
>>> On 07/06/2018 04:02 AM, Christian König wrote:
>>>> Am 05.07.2018 um 20:56 schrieb Andrey Grodzovsky:
>>>>> Problem: When PD/PT update made by CPU root PD was not yet mapped 
>>>>> causing
>>>>> page fault.
>>>>>
>>>>> Fix: Verify root PD is mapped into CPU address space.
>>>>>
>>>>> v2:
>>>>> Make sure that we add the root PD to the relocated list
>>>>> since then it's get mapped into CPU address space bt default
>>>>> in amdgpu_vm_update_directories.
>>>>>
>>>>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=107065
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>>>>>
>>>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky at amd.com>
>>>>> ---
>>>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++++-
>>>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> index 845f73a..1a8caf1 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>>>> @@ -156,6 +156,9 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>> amdgpu_vm_bo_base *base,
>>>>>           return;
>>>>>       list_add_tail(&base->bo_list, &bo->va);
>>>>>   +    if (bo->tbo.type == ttm_bo_type_kernel)
>>>>> +        list_move(&base->vm_status, &vm->relocated);
>>>>> +
>>>>>       if (bo->tbo.resv != vm->root.base.bo->tbo.resv)
>>>>>           return;
>>>>>   @@ -168,7 +171,8 @@ static void amdgpu_vm_bo_base_init(struct 
>>>>> amdgpu_vm_bo_base *base,
>>>>>        * is currently evicted. add the bo to the evicted list to 
>>>>> make sure it
>>>>>        * is validated on next vm use to avoid fault.
>>>>>        * */
>>>>> -    list_move_tail(&base->vm_status, &vm->evicted);
>>>>> +    if (bo->tbo.type != ttm_bo_type_kernel)
>>>>> +        list_move_tail(&base->vm_status, &vm->evicted);
>>>>
>>>> You need to drop that chunk, the evicted state supersedes the 
>>>> relocated state (e.g. when they are validated they move from 
>>>> evicted to relocated).
>>>
>>> I don't get it, can you explain more in detail why it's OK to remove 
>>> it ? They will not be in evicted list any more.
>>
>> See what happens with the BOs on the evicted list -> they get moved 
>> to the relocated list after validation.
>>
>> Christian.
>
> Oh, so you meant keep the original code right ? The 
> "list_move_tail(&base->vm_status, &vm->evicted); ?
> Meaning just remove the " if (bo->tbo.type != ttm_bo_type_kernel)" ?

Yes, yes, of course. I mean you should drop the chunk of changes, e.g. 
keep it as it is.

Sorry for the confusion,
Christian.

>
> ANdrey
>
>>
>>>
>>> Andrey
>>>
>>>>
>>>> Additional to that the now superfluous move in 
>>>> amdgpu_vm_alloc_levels() can be removed.
>>>>
>>>> Christian.
>>>>
>>>>>   }
>>>>>     /**
>>>>
>>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>