[PATCH 2/2] drm/amdgpu: fix debug sdma vram access checks

[Felix Kuehling]

Am 2022-01-17 um 6:43 p.m. schrieb Jonathan Kim:
> drm_dev_enter returns true when accessiable so correct the error check.
>
> Source VRAM buffer is reserved by TTM but not pinned so the gpu offset
> fetch throws a warning.  Get the gpu offset without a check and then
> double check to make sure the source buffer hasn't fallen into a hole.
> Otherwise use MMIO access to deal with non-contiguous VRAM cases as
> usual.

There is a way to get the correct offset for non-contiguous memory the
same way amdgpu_ttm_access_memory does, using amdgpu_res_first. Since
you only need a single page, you should never need amdgpu_res_next. Just
make sure that the cursor.size >= len.

Regards,
  Felix


>
> Signed-off-by: Jonathan Kim <jonathan.kim at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 6178ae7ba624..0acfd872bfef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1440,6 +1440,7 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo,
>  	struct dma_fence *fence;
>  	uint64_t src_addr, dst_addr;
>  	unsigned int num_dw;
> +	bool vram_is_contiguous;
>  	int r, idx;
>  
>  	if (len != PAGE_SIZE)
> @@ -1448,9 +1449,8 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo,
>  	if (!adev->mman.sdma_access_ptr)
>  		return -EACCES;
>  
> -	r = drm_dev_enter(adev_to_drm(adev), &idx);
> -	if (r)
> -		return r;
> +	if (!drm_dev_enter(adev_to_drm(adev), &idx))
> +		return -ENODEV;
>  
>  	if (write)
>  		memcpy(adev->mman.sdma_access_ptr, buf, len);
> @@ -1460,7 +1460,18 @@ static int amdgpu_ttm_access_memory_sdma(struct ttm_buffer_object *bo,
>  	if (r)
>  		goto out;
>  
> -	src_addr = amdgpu_bo_gpu_offset(abo);
> +	src_addr = amdgpu_bo_gpu_offset_no_check(abo);
> +	vram_is_contiguous = (adev->gmc.real_vram_size - 1 ==
> +				adev->gmc.vram_end - adev->gmc.vram_start) &&
> +				src_addr >= adev->gmc.vram_start &&
> +				src_addr + len <= adev->gmc.vram_end;
> +
> +	if (!vram_is_contiguous) {
> +		amdgpu_job_free(job);
> +		r = -EACCES;
> +		goto out;
> +	}
> +
>  	dst_addr = amdgpu_bo_gpu_offset(adev->mman.sdma_access_bo);
>  	if (write)
>  		swap(src_addr, dst_addr);