[PATCH 1/6] drm/amdgpu: add support for BO_VAs without BO

[Nicolai Hähnle]

On 02.02.2017 11:26, Christian König wrote:
> Am 30.01.2017 um 15:43 schrieb Nicolai Hähnle:
>> On 30.01.2017 13:57, Christian König wrote:
>>> From: Christian König <christian.koenig at amd.com>
>>>
>>> For PRT support we need mappings which aren't backed by any memory.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++------
>>>  1 file changed, 14 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 8e6030d..87eae9b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1117,7 +1117,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device
>>> *adev,
>>>      struct fence *exclusive;
>>>      int r;
>>>
>>> -    if (clear) {
>>> +    if (clear || !bo_va->bo) {
>>>          mem = NULL;
>>>          nodes = NULL;
>>>          exclusive = NULL;
>>> @@ -1134,9 +1134,15 @@ int amdgpu_vm_bo_update(struct amdgpu_device
>>> *adev,
>>>          exclusive = reservation_object_get_excl(bo_va->bo->tbo.resv);
>>>      }
>>>
>>> -    flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>>> -    gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>>> -        adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ? flags : 0;
>>> +    if (bo_va->bo) {
>>> +        flags = amdgpu_ttm_tt_pte_flags(adev, bo_va->bo->tbo.ttm, mem);
>>> +        gtt_flags = (amdgpu_ttm_is_bound(bo_va->bo->tbo.ttm) &&
>>> +            adev == amdgpu_ttm_adev(bo_va->bo->tbo.bdev)) ?
>>> +            flags : 0;
>>> +    } else {
>>> +        flags = 0x0;
>>> +        gtt_flags = ~0x0;
>>> +    }
>>>
>>>      spin_lock(&vm->status_lock);
>>>      if (!list_empty(&bo_va->vm_status))
>>> @@ -1271,7 +1277,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct
>>> amdgpu_device *adev,
>>>      INIT_LIST_HEAD(&bo_va->invalids);
>>>      INIT_LIST_HEAD(&bo_va->vm_status);
>>>
>>> -    list_add_tail(&bo_va->bo_list, &bo->va);
>>> +    if (bo)
>>> +        list_add_tail(&bo_va->bo_list, &bo->va);
>>>
>>>      return bo_va;
>>>  }
>>> @@ -1309,7 +1316,8 @@ int amdgpu_vm_bo_map(struct amdgpu_device *adev,
>>>
>>>      /* make sure object fit at this offset */
>>>      eaddr = saddr + size - 1;
>>> -    if ((saddr >= eaddr) || (offset + size >
>>> amdgpu_bo_size(bo_va->bo)))
>>> +    if (bo_va->bo && (saddr >= eaddr ||
>>> +              (offset + size > amdgpu_bo_size(bo_va->bo))))
>>>          return -EINVAL;
>>
>> At least the saddr >= eaddr check should probably apply apply.
>
> Oh, yes of course.
>
>>
>> Come to think of it, what if offset + size wraps around?
>
> Well that is exactly what the saddr >= eaddr check is good for :)

But eaddr doesn't take offset into account. It seems to me that offset 
can be arbitrarily large and this isn't checked anywhere. At least I 
don't see it.

Nicolai