Am 04.11.20 um 18:38 schrieb Daniel Vetter:
On Wed, Nov 04, 2020 at 02:00:21PM +0100, Christian König wrote:
This is deprecated.
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
So I tried to prove to myself that ttm doesn't access ->pages for these
cases, and kinda couldn't. We still seem to allocate the pages array and
all that, and there's lots of code using ->pages all over. And between
ttm_bo_type_sg and TTM_PAGE_FLAG_SG I didn't manage to chase a whole lot
of paths to their full conclusion.
Nope, see the amdgpu code:
if (ttm_sg_tt_init(>t->ttm, bo, page_flags, caching)) {
And then what ttm_sg_tt_init() does:
if (page_flags & TTM_PAGE_FLAG_SG)
ret = ttm_sg_tt_alloc_page_directory(ttm);
else
ret = ttm_dma_tt_alloc_page_directory(ttm);
And then finally what ttm_sg_tt_alloc_page_directory() does:
ttm->dma_address = kvmalloc_array(ttm->num_pages,....
ttm->pages should be NULL in this case if I'm not completely mistaken.
For or imported DMA-buf s we shouldn't have a ttm->pages in amdgpu for
quite a while and that works perfectly fine.
So I reduced my ambitions and wanted to prove that at least for dma-buf
imports aka ttm_bo_type_sg, we're guaranteed that we don't try to mmap
these to userspace. And also failed to find that check.
See ttm_bo_vm_reserve():
/*
* Refuse to fault imported pages. This should be handled
* (if at all) by redirecting mmap to the exporter.
*/
if (bo->ttm && (bo->ttm->page_flags & TTM_PAGE_FLAG_SG)) {
dma_resv_unlock(bo->base.resv);
return VM_FAULT_SIGBUS;
}
btw this is across all drivers, mostly ttm code, not radeon specific.
Well instead of those patches we could as well switch over radeon and
nouveau to using ttm_sg_tt_init() as well (or merge ttm_sg_tt_init()
into ttm_dma_tt_init).
That's probably the much cleaner approach, but when I wrote
ttm_sg_tt_init() I wasn't sure if radeon/nouveau where using ttm->pages
for something, e.g. basically the same concern you have.
Regards,
Christian.
So conclusion, still a mess here that at least I can't see throug clearly
:-/ here = ttm_tt and the entire backing storage handling and everything
that ties into it. Probably the area that still has the most midlayer feel
to ttm with all the refactoring in-flight in still.
tldr; tried to review patches 1-3, gave up.
Cheers, Daniel
---
drivers/gpu/drm/radeon/radeon_ttm.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 95038ac3382e..f41fcee35f70 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -494,8 +494,8 @@ static int radeon_ttm_tt_pin_userptr(struct ttm_bo_device *bdev, struct ttm_tt *
if (r)
goto release_sg;
- drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
- gtt->ttm.dma_address, ttm->num_pages);
+ drm_prime_sg_to_page_addr_arrays(ttm->sg, NULL, gtt->ttm.dma_address,
+ ttm->num_pages);
return 0;
@@ -673,8 +673,9 @@ static int radeon_ttm_tt_populate(struct ttm_bo_device *bdev,
}
if (slave && ttm->sg) {
- drm_prime_sg_to_page_addr_arrays(ttm->sg, ttm->pages,
- gtt->ttm.dma_address, ttm->num_pages);
+ drm_prime_sg_to_page_addr_arrays(ttm->sg, NULL,
+ gtt->ttm.dma_address,
+ ttm->num_pages);
return 0;
}
--
2.25.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
