linux-kernel - Re: [PATCH v5 3/9] dma-mapping: add dma_{map,unmap}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 11 Mar 2016 13:46:42 +0000
From:	Robin Murphy <robin.murphy@....com>
To:	Dan Williams <dan.j.williams@...el.com>,
	Christoph Hellwig <hch@...radead.org>,
	Vinod Koul <vinod.koul@...el.com>,
	linux-renesas-soc@...r.kernel.org,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
	iommu@...ts.linux-foundation.org,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	geert+renesas@...der.be, Linus Walleij <linus.walleij@...aro.org>,
	Arnd Bergmann <arnd@...db.de>, linux-arch@...r.kernel.org
Subject: Re: [PATCH v5 3/9] dma-mapping: add dma_{map,unmap}_resource

Hi Dan,

On 11/03/16 06:47, Dan Williams wrote:
> On Thu, Mar 10, 2016 at 8:05 AM, Niklas S??derlund
> <niklas.soderlund@...natech.se> wrote:
>> Hi Christoph,
>>
>> On 2016-03-07 23:38:47 -0800, Christoph Hellwig wrote:
>>> Please add some documentation on where/how this should be used.  It's
>>> not a very obvious interface.
>>
>> Good idea, I have added the following to Documentation/DMA-API.txt and
>> folded it in to this patch. Do you feel it's adequate and do you know
>> anywhere else I should add documentation?
>>
>> diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
>> index 45ef3f2..248556a 100644
>> --- a/Documentation/DMA-API.txt
>> +++ b/Documentation/DMA-API.txt
>> @@ -277,14 +277,29 @@ and <size> parameters are provided to do partial page mapping, it is
>>   recommended that you never use these unless you really know what the
>>   cache width is.
>>
>> +dma_addr_t
>> +dma_map_resource(struct device *dev, phys_addr_t phys_addr, size_t size,
>> +                enum dma_data_direction dir, struct dma_attrs *attrs)
>> +
>> +Maps a MMIO region so it can be accessed by the device and returns the
>> +DMA address of the memory. API should only be used to map device MMIO,
>> +mapping of RAM is not permitted.
>> +
>
> I think it is confusing to use the dma_ prefix for this peer-to-peer
> mmio functionality.  dma_addr_t is a device's view of host memory.
> Something like bus_addr_t bus_map_resource().  Doesn't this routine
> also need the source device in addition to the target device?  The
> resource address is from the perspective of the host cpu, it may be a
> different address space in the view of two devices relative to each
> other.

Hmm, the trouble with that is that when the DMA master is behind an 
IOMMU, the address space as seen by the device is dynamic and whatever 
we decide it to be, so there is no distinction between a "DMA" address 
and a "bus" address.

In practice the dmaengine API has clearly worked for however long with 
slave MMIO addresses being a dma_addr_t, and it doesn't look like anyone 
objected to the change to phys_addr_t in -next either. If nothing is 
using bus_addr_t anyway, what's the right thing to do? Looking up 
through higher abstraction layers, we have the likes of struct 
snd_dmaengine_dai_dma_data also expecting the slave address to be a 
dma_addr_t, leading to things like the direct casting in 
bcm2835_i2s_probe() for the non-IOMMU dma != phys != bus case that could 
also be cleaned up with this proposed interface.

Robin.