From b7e8b086ffbc03b890ed22ae63ed5e5bd319d184 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 28 Jul 2021 15:05:49 +0200 Subject: drm/amdgpu: unbind in amdgpu_ttm_tt_unpopulate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doing this in amdgpu_ttm_backend_destroy() is to late. It turned out that this is not a good idea at all because it leaves pointers to freed up system memory pages in the GART tables of the drivers. Signed-off-by: Christian König Reviewed-by: Thomas Hellström Link: https://patchwork.freedesktop.org/patch/msgid/20210728130552.2074-2-christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 9fd5db58067d..75d45623388a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1066,7 +1066,6 @@ static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev, { struct amdgpu_ttm_tt *gtt = (void *)ttm; - amdgpu_ttm_backend_unbind(bdev, ttm); ttm_tt_destroy_common(bdev, ttm); if (gtt->usertask) put_task_struct(gtt->usertask); @@ -1148,6 +1147,8 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev, struct amdgpu_ttm_tt *gtt = (void *)ttm; struct amdgpu_device *adev; + amdgpu_ttm_backend_unbind(bdev, ttm); + if (gtt && gtt->userptr) { amdgpu_ttm_tt_set_user_pages(ttm, NULL); kfree(ttm->sg); -- cgit From d5f45d1e2f08685c34483719b39f91010d6222e8 Mon Sep 17 00:00:00 2001 From: Christian König Date: Wed, 28 Jul 2021 15:05:52 +0200 Subject: drm/ttm: remove ttm_tt_destroy_common v2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the functionality into ttm_tt_fini and ttm_bo_tt_destroy instead. We don't need this any more since we removed the unbind from the destroy code paths in the drivers. Also add a warning to ttm_tt_fini() if we try to fini a still populated TT object. v2: instead of reverting the patch move the functionality to different places. Signed-off-by: Christian König Reviewed-by: Thomas Hellström Link: https://patchwork.freedesktop.org/patch/msgid/20210728130552.2074-5-christian.koenig@amd.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 75d45623388a..489e22190e29 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1066,7 +1066,6 @@ static void amdgpu_ttm_backend_destroy(struct ttm_device *bdev, { struct amdgpu_ttm_tt *gtt = (void *)ttm; - ttm_tt_destroy_common(bdev, ttm); if (gtt->usertask) put_task_struct(gtt->usertask); -- cgit From c92db8d64f9e0313e7ecdc9500db93a5040c9370 Mon Sep 17 00:00:00 2001 From: Christian König Date: Tue, 7 Sep 2021 09:37:52 +0200 Subject: drm/amdgpu: fix use after free during BO move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The memory backing old_mem is already freed at that point, move the check a bit more up. Signed-off-by: Christian König Fixes: bfa3357ef9ab ("drm/ttm: allocate resource object instead of embedding it v2") Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1699 Acked-by: Nirmoy Das Reviewed-by: Michel Dänzer Signed-off-by: Alex Deucher Cc: stable@vger.kernel.org --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 38dade421d46..94126dc39688 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -515,6 +515,15 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, goto out; } + if (bo->type == ttm_bo_type_device && + new_mem->mem_type == TTM_PL_VRAM && + old_mem->mem_type != TTM_PL_VRAM) { + /* amdgpu_bo_fault_reserve_notify will re-set this if the CPU + * accesses the BO after it's moved. + */ + abo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; + } + if (adev->mman.buffer_funcs_enabled) { if (((old_mem->mem_type == TTM_PL_SYSTEM && new_mem->mem_type == TTM_PL_VRAM) || @@ -545,15 +554,6 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict, return r; } - if (bo->type == ttm_bo_type_device && - new_mem->mem_type == TTM_PL_VRAM && - old_mem->mem_type != TTM_PL_VRAM) { - /* amdgpu_bo_fault_reserve_notify will re-set this if the CPU - * accesses the BO after it's moved. - */ - abo->flags &= ~AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED; - } - out: /* update statistics */ atomic64_add(bo->base.size, &adev->num_bytes_moved); -- cgit From 21856e1e342505d79803d7342da3a348981b431c Mon Sep 17 00:00:00 2001 From: Matthew Auld Date: Mon, 27 Sep 2021 12:41:04 +0100 Subject: drm/ttm: move ttm_tt_{add, clear}_mapping into amdgpu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that setting page->index shouldn't be needed anymore, we are just left with setting page->mapping, and here it looks like amdgpu is the only user, where pointing the page->mapping at the dev_mapping is used to verify that the pages do indeed belong to the device, if userspace later tries to touch them. v2(Christian): - Drop the functions altogether and just inline modifying the page->mapping Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Christian König Reviewed-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20210927114114.152310-3-matthew.auld@intel.com Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 1129e17e9f09..60b12bb55244 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -1119,6 +1119,8 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev, { struct amdgpu_device *adev = amdgpu_ttm_adev(bdev); struct amdgpu_ttm_tt *gtt = (void *)ttm; + pgoff_t i; + int ret; /* user pages are bound by amdgpu_ttm_tt_pin_userptr() */ if (gtt->userptr) { @@ -1131,7 +1133,14 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev, if (ttm->page_flags & TTM_PAGE_FLAG_SG) return 0; - return ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx); + ret = ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx); + if (ret) + return ret; + + for (i = 0; i < ttm->num_pages; ++i) + ttm->pages[i]->mapping = bdev->dev_mapping; + + return 0; } /* @@ -1145,6 +1154,7 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev, { struct amdgpu_ttm_tt *gtt = (void *)ttm; struct amdgpu_device *adev; + pgoff_t i; amdgpu_ttm_backend_unbind(bdev, ttm); @@ -1158,6 +1168,9 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev, if (ttm->page_flags & TTM_PAGE_FLAG_SG) return; + for (i = 0; i < ttm->num_pages; ++i) + ttm->pages[i]->mapping = NULL; + adev = amdgpu_ttm_adev(bdev); return ttm_pool_free(&adev->mman.bdev.pool, ttm); } -- cgit From 43d46f0b78bba5dc5ffb6f1b9a1d4c8d0c5dd1fc Mon Sep 17 00:00:00 2001 From: Matthew Auld Date: Wed, 29 Sep 2021 14:26:27 +0100 Subject: drm/ttm: s/FLAG_SG/FLAG_EXTERNAL/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It covers more than just ttm_bo_type_sg usage, like with say dma-buf, since one other user is userptr in amdgpu, and in the future we might have some more. Hence EXTERNAL is likely a more suitable name. v2(Christian): - Rename these to TTM_TT_FLAGS_* - Fix up all the holes in the flag values Suggested-by: Christian König Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Christian König Acked-by: Christian König Link: https://patchwork.freedesktop.org/patch/msgid/20210929132629.353541-1-matthew.auld@intel.com Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 60b12bb55244..e8d70b6e6737 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -894,7 +894,7 @@ static int amdgpu_ttm_backend_bind(struct ttm_device *bdev, DRM_ERROR("failed to pin userptr\n"); return r; } - } else if (ttm->page_flags & TTM_PAGE_FLAG_SG) { + } else if (ttm->page_flags & TTM_TT_FLAG_EXTERNAL) { if (!ttm->sg) { struct dma_buf_attachment *attach; struct sg_table *sgt; @@ -1130,7 +1130,7 @@ static int amdgpu_ttm_tt_populate(struct ttm_device *bdev, return 0; } - if (ttm->page_flags & TTM_PAGE_FLAG_SG) + if (ttm->page_flags & TTM_TT_FLAG_EXTERNAL) return 0; ret = ttm_pool_alloc(&adev->mman.bdev.pool, ttm, ctx); @@ -1165,7 +1165,7 @@ static void amdgpu_ttm_tt_unpopulate(struct ttm_device *bdev, return; } - if (ttm->page_flags & TTM_PAGE_FLAG_SG) + if (ttm->page_flags & TTM_TT_FLAG_EXTERNAL) return; for (i = 0; i < ttm->num_pages; ++i) @@ -1198,8 +1198,8 @@ int amdgpu_ttm_tt_set_userptr(struct ttm_buffer_object *bo, return -ENOMEM; } - /* Set TTM_PAGE_FLAG_SG before populate but after create. */ - bo->ttm->page_flags |= TTM_PAGE_FLAG_SG; + /* Set TTM_TT_FLAG_EXTERNAL before populate but after create. */ + bo->ttm->page_flags |= TTM_TT_FLAG_EXTERNAL; gtt = (void *)bo->ttm; gtt->userptr = addr; -- cgit From 58144d283712c9e80e528e001af6ac5aeee71af2 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Wed, 6 Oct 2021 17:55:00 +0200 Subject: drm/amdgpu: unify BO evicting method in amdgpu_ttm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unify BO evicting functionality for possible memory types in amdgpu_ttm.c. Signed-off-by: Nirmoy Das Reviewed-by: Christian König Signed-off-by: Alex Deucher --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c') diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 820fcb24231f..5d5c42ae388b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -2036,6 +2036,36 @@ error_free: return r; } +/** + * amdgpu_ttm_evict_resources - evict memory buffers + * @adev: amdgpu device object + * @mem_type: evicted BO's memory type + * + * Evicts all @mem_type buffers on the lru list of the memory type. + * + * Returns: + * 0 for success or a negative error code on failure. + */ +int amdgpu_ttm_evict_resources(struct amdgpu_device *adev, int mem_type) +{ + struct ttm_resource_manager *man; + + switch (mem_type) { + case TTM_PL_VRAM: + case TTM_PL_TT: + case AMDGPU_PL_GWS: + case AMDGPU_PL_GDS: + case AMDGPU_PL_OA: + man = ttm_manager_type(&adev->mman.bdev, mem_type); + break; + default: + DRM_ERROR("Trying to evict invalid memory type\n"); + return -EINVAL; + } + + return ttm_resource_manager_evict_all(&adev->mman.bdev, man); +} + #if defined(CONFIG_DEBUG_FS) static int amdgpu_mm_vram_table_show(struct seq_file *m, void *unused) -- cgit