Skip to content

Commit 9aad03e

Browse files
suijingfenglynxeye-dev
authored andcommitted
drm/etnaviv: Drop the offset in page manipulation
The etnaviv driver, both kernel space and user space, assumes that GPU page size is 4KiB. Its IOMMU map/unmap 4KiB physical address range once a time. If 'sg->offset != 0' is true, then the current implementation will map the IOVA to a wrong area, which may lead to coherency problem. Picture 0 and 1 give the illustration, see below. PA start drifted | |<--- 'sg_dma_address(sg) - sg->offset' | .------ sg_dma_address(sg) | | .---- sg_dma_len(sg) |<-sg->offset->| | V |<-->| Another one cpu page +----+----+----+----+ +----+----+----+----+ |xxxx| |||||| ||||||||||||||||||||| +----+----+----+----+ +----+----+----+----+ ^ ^ ^ ^ |<--- da_len --->| | | | | | | | .--------------' | | | | .----------------' | | | | .----------------' | | | | | | +----+----+----+----+ | | ||||||||||||||||||||| | | +----+----+----+----+ | | | '--------------. da_len = sg_dma_len(sg) + sg->offset, using | | 'sg_dma_len(sg) + sg->offset' will lead to GPUVA +----+ ~~~~~~~~~~~~~+ collision, but min_t(unsigned int, da_len, va_len) |xxxx| | will clamp it to correct size. But the IOVA will +----+ ~~~~~~~~~~~~~+ be redirect to wrong area. ^ | Picture 0: Possibly wrong implementation. GPUVA (IOVA) -------------------------------------------------------------------------- .------- sg_dma_address(sg) | .---- sg_dma_len(sg) |<-sg->offset->| | | |<-->| another one cpu page +----+----+----+----+ +----+----+----+----+ | |||||| ||||||||||||||||||||| +----+----+----+----+ +----+----+----+----+ ^ ^ ^ ^ | | | | .--------------' | | | | | | | | .--------------' | | | | .----------------' | | | | .----------------' | | | | +----+ +----+----+----+----+ |||||| ||||||||||||||||||||| The first one is SZ_4K, the second is SZ_16K +----+ +----+----+----+----+ ^ | Picture 1: Perfectly correct implementation. GPUVA (IOVA) If sg->offset != 0 is true, IOVA will be mapped to wrong physical address. Either because there doesn't contain the data or there contains wrong data. Strictly speaking, the memory area that before sg_dma_address(sg) doesn't belong to us, and it's likely that the area is being used by other process. Because we don't want to introduce confusions about which part is visible to the GPU, we assumes that the size of GPUVA is always 4KiB aligned. This is very relaxed requirement, since we already made the decision that GPU page size is 4KiB (as a canonical decision). And softpin feature is landed, Mesa's util_vma_heap_alloc() will certainly report correct length of GPUVA to kernel with desired alignment ensured. With above statements agreed, drop the "offset in page" manipulation will return us a correct implementation at any case. Fixes: a8c21a5 ("drm/etnaviv: add initial etnaviv DRM driver") Signed-off-by: Sui Jingfeng <[email protected]> Signed-off-by: Lucas Stach <[email protected]>
1 parent 16ab70e commit 9aad03e

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

drivers/gpu/drm/etnaviv/etnaviv_mmu.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ static int etnaviv_iommu_map(struct etnaviv_iommu_context *context,
8282
return -EINVAL;
8383

8484
for_each_sgtable_dma_sg(sgt, sg, i) {
85-
phys_addr_t pa = sg_dma_address(sg) - sg->offset;
86-
unsigned int da_len = sg_dma_len(sg) + sg->offset;
85+
phys_addr_t pa = sg_dma_address(sg);
86+
unsigned int da_len = sg_dma_len(sg);
8787
unsigned int bytes = min_t(unsigned int, da_len, va_len);
8888

8989
VERB("map[%d]: %08x %pap(%x)", i, iova, &pa, bytes);

0 commit comments

Comments
 (0)