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

[christian.koenig@xxxxxxx (Christian König)]

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.

>
> Andrey
>
>>
>> Additional to that the now superfluous move in 
>> amdgpu_vm_alloc_levels() can be removed.
>>
>> Christian.
>>
>>> Â  }
>>> Â  Â  /**
>>
>