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

[Andrey Grodzovsky]

On 07/06/2018 10:30 AM, Christian König wrote:
> 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.

OK. Thanks.

Andrey

>
>>
>> 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
>>
>