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

[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)" ?

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