From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943665AbdDTIZI (ORCPT ); Thu, 20 Apr 2017 04:25:08 -0400 Received: from mail-oi0-f65.google.com ([209.85.218.65]:33974 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S942172AbdDTIZC (ORCPT ); Thu, 20 Apr 2017 04:25:02 -0400 MIME-Version: 1.0 In-Reply-To: <8df7c5a9-c71c-4ee9-9bc2-9c861cf9796c@kapsi.fi> References: <20170419182413.866327-1-arnd@arndb.de> <8df7c5a9-c71c-4ee9-9bc2-9c861cf9796c@kapsi.fi> From: Arnd Bergmann Date: Thu, 20 Apr 2017 10:25:01 +0200 X-Google-Sender-Auth: LNWPk2zQdQMJhWdSSTBYtWBAyXs Message-ID: Subject: Re: [PATCH] [RFC] gpu: host1x: shut up warning about DMA API misuse To: Mikko Perttunen Cc: Thierry Reding , Linux ARM , Mikko Perttunen , dri-devel , linux-tegra@vger.kernel.org, Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 20, 2017 at 9:02 AM, Mikko Perttunen wrote: > On 19.04.2017 21:24, Arnd Bergmann wrote: >> >> When dma_addr_t and phys_addr_t are not the same size, we get a warning >> from the dma_alloc_wc function: >> >> drivers/gpu/host1x/cdma.c: In function 'host1x_pushbuffer_init': >> drivers/gpu/host1x/cdma.c:94:48: error: passing argument 3 of >> 'dma_alloc_wc' from incompatible pointer type >> [-Werror=incompatible-pointer-types] >> pb->mapped = dma_alloc_wc(host1x->dev, size, &pb->phys, >> ^ >> In file included from drivers/gpu/host1x/cdma.c:22:0: >> include/linux/dma-mapping.h:761:37: note: expected 'dma_addr_t * {aka long >> long unsigned int *}' but argument is of type 'phys_addr_t * {aka unsigned >> int *}' >> static noinline_if_stackbloat void *dma_alloc_wc(struct device *dev, >> size_t size, >> ^~~~~~~~~~~~ >> drivers/gpu/host1x/cdma.c:113:48: error: passing argument 3 of >> 'dma_alloc_wc' from incompatible pointer type >> [-Werror=incompatible-pointer-types] >> pb->mapped = dma_alloc_wc(host1x->dev, size, &pb->phys, >> ^ >> In file included from drivers/gpu/host1x/cdma.c:22:0: >> include/linux/dma-mapping.h:761:37: note: expected 'dma_addr_t * {aka long >> long unsigned int *}' but argument is of type 'phys_addr_t * {aka unsigned >> int *}' >> static noinline_if_stackbloat void *dma_alloc_wc(struct device *dev, >> size_t size, >> ^~~~~~~~~~~~ >> >> The problem here is that dma_alloc_wc() returns a pointer to a dma_addr_t >> that may already be translated by an IOMMU, but the driver passes this >> into iommu_map() as a physical address. This works by accident only when >> the IOMMU does not get registered with the DMA API and there is a 1:1 >> mapping between physical addresses as seen by the CPU and the device. >> >> The fundamental problem here is the lack of a generic API to do what the >> driver wants: allocating CPU-visible memory for use by a device through >> user-defined IOMMU settings. Neither the dma-mapping API nor the IOMMU >> API can do this by itself, and combining the two is not well-defined. >> >> This patch addresses the type mismatch by adding a third pointer into the >> push_buffer structure: in addition to the address as seen from the CPU >> and the address inside of the local IOMMU domain, the pb->alloc pointer >> is the token returned by dma_alloc_wc(), and this is what we later need >> to pass into dma_free_wc(). >> >> The address we pass into iommu_map() however is the physical address >> computed from virt_to_phys(), assuming that the pointer we have here >> is part of the linear mapping (which is also questionable, e.g. when we >> have a non-coherent device on ARM32 this may be false). Also, when >> the DMA API uses the IOMMU to allocate the pointer for the default >> domain, we end up with two IOMMU mappings for the same physical address. >> > > I think we have a "policy" on Tegra that the DMA API will never allocate > using the IOMMU (Thierry can elaborate on this), which is why I wrote the > code with that assumption. Essentially, we have made the DMA API into the > API that allocates CPU-visible memory. I don't think this can be a per-platform policy. > Considering that, I'm wondering if we can just have a temporary local > dma_addr_t and then cast that to phys_addr_t, combined with a good comment? That was my first approach, and it does address the warning, but I did not send it because it still felt too wrong. Arnd