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

["Nicolai Hähnle" <nhaehnle-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 30.01.2017 13:57, Christian König wrote:
> From: Christian König <christian.koenig@amd.com>
>
> For PRT support we need mappings which aren't backed by any memory.
>
> Signed-off-by: Christian König <christian.koenig@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.

Come to think of it, what if offset + size wraps around? There should 
probably be an explicit check for that.

Cheers,
Nicolai

>
>  	last_pfn = eaddr / AMDGPU_GPU_PAGE_SIZE;
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx