With CONFIG_DMABUF_DEBUG=y, dma_buf_map_attachment() hands importers a copy of the attachment sg_table with the struct page pointers stripped and sg->length zeroed; only sg_dma_address()/sg_dma_len() are carried over. msm consumes sg->length and sg_phys() in both of its map paths:
- msm_iommu_pagetable_map() (userspace managed, per-process GPU pagetables) walks the sg_table with sg->length and sg_phys() - msm_iommu_map() (kernel managed mappings: display, and TTBR1 for the GPU), via iommu_map_sgtable(), which consumes sg->length and sg_phys() as well With a page-stripped sg_table both paths silently map nothing and return success. Userspace then observes arm-smmu translation faults once the GPU first touches the mapping, e.g. during hardware video decode: gpu fault: ttbr0=000000088a889000 iova=000000010741c000 dir=READ type=TRANSLATION source=UCHE and __arm_lpae_unmap() WARNs for the never-mapped ranges when the GEM handles are closed (a WARN storm of ~470 traces within a minute of video playback on my x1e78100 laptop). For sg entries that still carry a struct page (native objects, and imports without the DMABUF_DEBUG wrapper) keep using sg_phys(), so native objects which msm never dma-maps itself (non-MSM_BO_WC) are unaffected. For page-less entries, recover the physical address from the DMA address instead: dmabuf attachments are dma-mapped against the msm drm device, so the dma_addr -> phys lookup can be done with iommu_iova_to_phys() in that device's DMA-API domain, cached per VM in struct msm_mmu::dma_domain at msm_gem_vm_create() time. If the drm device is direct mapped the DMA address already is a physical address and the lookup degenerates to the identity. Applied on top of "drm/msm/gem: Drop use of pages for imported dma-bufs" [1], which removes the remaining struct page consumers for imported buffers. With both, hardware video decode works with DMABUF_DEBUG=y, tested with clapper and chromium on x1e78100 (Snapdragon X1E78100): zero arm-smmu faults, zero io-pgtable WARNs, correct frames. Without this patch, the same system logs a WARN trace per unmap and falls back to a copy path for video playback. [1] <[email protected]> Suggested-by: Rob Clark <[email protected]> Cc: Rob Clark <[email protected]> Cc: Dmitry Baryshkov <[email protected]> Cc: Christian König <[email protected]> Signed-off-by: Jianfeng Liu <[email protected]> --- drivers/gpu/drm/msm/msm_gem_vma.c | 9 ++++ drivers/gpu/drm/msm/msm_iommu.c | 90 ++++++++++++++++++++++++++++++- drivers/gpu/drm/msm/msm_mmu.h | 13 +++++ 3 files changed, 110 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c index f687a629629d3..322b96e0e07ec 100644 --- a/drivers/gpu/drm/msm/msm_gem_vma.c +++ b/drivers/gpu/drm/msm/msm_gem_vma.c @@ -840,6 +840,15 @@ msm_gem_vm_create(struct drm_device *drm, struct msm_mmu *mmu, const char *name, goto err_free_vm; } + /* + * dma-buf imports attach against the msm drm device, and their + * sg_dma_address() lives in that device's DMA-API domain. Keep it + * so the map paths can translate page-less sg_table entries (the + * DMABUF_DEBUG wrapper) back to physical addresses. + */ + if (device_iommu_mapped(drm->dev)) + mmu->dma_domain = iommu_get_dma_domain(drm->dev); + if (!managed) { struct drm_sched_init_args args = { .ops = &msm_vm_bind_ops, diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c index da6782fca6bd2..8407a37f9efee 100644 --- a/drivers/gpu/drm/msm/msm_iommu.c +++ b/drivers/gpu/drm/msm/msm_iommu.c @@ -140,6 +140,24 @@ static int msm_iommu_pagetable_unmap(struct msm_mmu *mmu, u64 iova, return ret; } +/** + * msm_mmu_dma_to_phys() - recover the physical address of a dma address + * + * dma-buf attachments are dma-mapped against the msm drm device, so the + * DMA domain of that device (msm_mmu::dma_domain) holds the mapping. + * For a direct-mapped drm device the DMA address already is a physical + * address. + */ +static phys_addr_t msm_mmu_dma_to_phys(struct msm_mmu *mmu, dma_addr_t dma_addr) +{ + struct iommu_domain *dma_domain = mmu->dma_domain; + + if (!dma_domain) + return (phys_addr_t)dma_addr; + + return iommu_iova_to_phys(dma_domain, dma_addr); +} + static int msm_iommu_pagetable_map_prr(struct msm_mmu *mmu, u64 iova, size_t len, int prot) { struct msm_iommu_pagetable *pagetable = to_pagetable(mmu); @@ -184,8 +202,35 @@ static int msm_iommu_pagetable_map(struct msm_mmu *mmu, u64 iova, return msm_iommu_pagetable_map_prr(mmu, iova, len, prot); for_each_sgtable_sg(sgt, sg, i) { - size_t size = sg->length; - phys_addr_t phys = sg_phys(sg); + size_t size; + phys_addr_t phys; + + if (sg_page(sg)) { + /* CPU-view entry: native objects, and imported + * sg_tables that still carry struct page + */ + size = sg->length; + phys = sg_phys(sg); + } else { + /* + * Page-less entry, e.g. the sg_table wrapper + * that dma_buf_map_attachment() hands out when + * CONFIG_DMABUF_DEBUG=y (page pointers stripped, + * sg->length zeroed, only the DMA fields carried + * over). Recover the physical address by + * translating the DMA address through the drm + * device's DMA-API domain. + */ + size = sg_dma_len(sg); + phys = msm_mmu_dma_to_phys(mmu, sg_dma_address(sg)); + + if (!size || !phys) { + dev_err(mmu->dev, + "cannot map page-less sg entry: dma=%pad len=%zu\n", + &sg_dma_address(sg), size); + return -EINVAL; + } + } if (!len) break; @@ -697,6 +742,47 @@ static int msm_iommu_map(struct msm_mmu *mmu, uint64_t iova, if (iova & BIT_ULL(48)) iova |= GENMASK_ULL(63, 49); + /* + * With CONFIG_DMABUF_DEBUG=y, imported sg_tables carry no struct + * page and sg->length is zeroed; iommu_map_sgtable() would consume + * zero length and silently map nothing. Map from the (translated) + * DMA addresses instead. + */ + if (!sg_page(sgt->sgl)) { + struct scatterlist *sg; + size_t mapped = 0; + unsigned int i; + + for_each_sgtable_dma_sg(sgt, sg, i) { + phys_addr_t phys = + msm_mmu_dma_to_phys(mmu, sg_dma_address(sg)); + size_t size = sg_dma_len(sg); + + if (!phys || !size) { + ret = -EINVAL; + goto err_unmap; + } + + ret = iommu_map(iommu->domain, iova + mapped, phys, + size, prot, GFP_KERNEL); + if (ret) + goto err_unmap; + + mapped += size; + } + + if (mapped != len) { + ret = -EINVAL; + goto err_unmap; + } + + return 0; + +err_unmap: + iommu_unmap(iommu->domain, iova, mapped); + return ret; + } + ret = iommu_map_sgtable(iommu->domain, iova, sgt, prot); if (ret < 0) return ret; diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h index 8915662fbd4d0..116daf6ce47cb 100644 --- a/drivers/gpu/drm/msm/msm_mmu.h +++ b/drivers/gpu/drm/msm/msm_mmu.h @@ -64,6 +64,19 @@ struct msm_mmu { * msm_gem_vm::mmu_lock. */ struct msm_mmu_prealloc *prealloc; + + /** + * @dma_domain: DMA-API domain of the msm drm device + * + * dma-buf attachments are dma-mapped against the msm drm device, + * so this domain holds the mapping dma_addr -> phys for imported + * buffers. Used to recover the physical address of sg_table + * entries which carry no struct page (e.g. the page-stripped + * sg_table wrapper that dma_buf_map_attachment() hands out when + * CONFIG_DMABUF_DEBUG=y). NULL if the drm device is direct + * mapped, in which case DMA addresses are physical addresses. + */ + struct iommu_domain *dma_domain; }; static inline void msm_mmu_init(struct msm_mmu *mmu, struct device *dev, -- 2.47.3
