linux-kernel - Re: [PATCH] mmc: sdhci-tegra: Set DMA mask

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] [day] [month] [year] [list]

Date:	Tue, 1 Mar 2016 13:21:54 +0900
From:	Alexandre Courbot <acourbot@...dia.com>
To:	Arnd Bergmann <arnd@...db.de>, Alexandre Courbot <gnurou@...il.com>
CC:	Ulf Hansson <ulf.hansson@...aro.org>,
	Stephen Warren <swarren@...dotorg.org>,
	Thierry Reding <thierry.reding@...il.com>,
	linux-mmc <linux-mmc@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: sdhci-tegra: Set DMA mask

On 02/26/2016 08:31 PM, Arnd Bergmann wrote:
> On Friday 26 February 2016 16:24:34 Alexandre Courbot wrote:
>> On Thu, Feb 25, 2016 at 11:52 PM, Arnd Bergmann <arnd@...db.de> wrote:
>>>> Actually even if we specify a dma-ranges on the parent DT node, the
>>>> DMA range will still be limited to 32 bits because of the following
>>>> code in of_dma_configure():
>>>>
>>>>      /*
>>>>       * Set default coherent_dma_mask to 32 bit.  Drivers are expected to
>>>>       * setup the correct supported mask.
>>>>       */
>>>>      if (!dev->coherent_dma_mask)
>>>>          dev->coherent_dma_mask = DMA_BIT_MASK(32);
>>>>
>>>>      /*
>>>>       * Set it to coherent_dma_mask by default if the architecture
>>>>       * code has not set it.
>>>>       */
>>>>      if (!dev->dma_mask)
>>>>          dev->dma_mask = &dev->coherent_dma_mask;
>>>>
>>>>      ....
>>>>      /* gets dma-ranges into dma_addr and size */
>>>>      ....
>>>>
>>>>
>>>>      *dev->dma_mask = min((*dev->dma_mask),
>>>>                   DMA_BIT_MASK(ilog2(dma_addr + size)));
>>>>
>>>> So unless the DMA mask is set on the device before of_dma_configure()
>>>> is called, the min() statement will choose the 32 bits mask that has
>>>> been previously set. So IIUC in any case, the driver will need to call
>>>> dma_set_mask()
>>>
>>> Yes, the driver definitely has to call dma_set_mask(), the property of
>>> the parent bus is used to make that fail when the bus doesn't support
>>> it.
>>
>> And that's where things seem to stop working: the driver's probe
>> function is invoked by the platform bus, *after* of_dma_configure() is
>> called. So unless I am missing something there is no way for the
>> driver to set the DMA mask in such a way that of_dma_configure() can
>> see it and do the right thing.
>>
>> In other words, most of the DMA mask logic in of_dma_configure()
>> doesn't seem to have any effect (and a 32 bits mask will be set), at
>> least on the platform bus.
>
> That is correct: of_dma_configure has to set a 32-bit DMA mask
> because that is the default that we expect to see in Linux drivers.
>
> A lot of drivers don't call dma_set_mask() at all, so this is
> the most reasonable value that typically works, unless the
> device is more limited, or you want the extra performance you
> get on devices that actually support a bigger mask.
>
>>>> Can I have your thoughts on this? Am I missing something?
>>>
>>> One point: I think the dma_set_mask() probably should be in the
>>> generic part of the sdhci driver, not the tegra specific portion.
>>
>> Ok, but then how does the generic part of the driver knows which DMA
>> mask applies to the device?
>
> If dma_set_mask() succeeds when passed a 64-bit mask, the driver
> can pass high addresses into dma_map_*() and put the result into
> the 64-bit DMA registers of the device. That is all the driver
> needs to know here.
>
> When the bus is more limited than the device, we either have
> an swiotlb/iommu that will use bounce buffers to map dma_map_* work
> anyway (using low DMA addresses for high memory), or we don't have
> an swiotlb and the dma_set_mask() operation has to fail.

Ok, I think I understand. I was expecting of_dma_configure() to do more 
than it actually needs to and be the final arbiter of a device's DMA 
mask. That's obviously not the case - thanks for the thorough explanation.

Let me send a v2 of this to see if I got it properly.