linux-kernel - Re: [PATCH 1/2] spi: When no dma_chan map buffers with spi

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 for Android: free password hash cracker in your pocket

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

Message-ID: <81ab617f-95fb-17d1-88d2-5cf5f74bab03@arm.com>
Date:   Mon, 30 Jan 2017 11:44:30 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Daniel Kurtz <djkurtz@...omium.org>
Cc:     dtor@...omium.org, groeck@...omium.org,
        Nicolas Boichat <drinkcat@...omium.org>,
        Leilk Liu <leilk.liu@...iatek.com>,
        Mark Brown <broonie@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        "open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>
Subject: Re: [PATCH 1/2] spi: When no dma_chan map buffers with spi_master's
 parent

On 28/01/17 22:17, Andy Shevchenko wrote:
> On Thu, Jan 26, 2017 at 6:21 PM, Daniel Kurtz <djkurtz@...omium.org> wrote:
>> Back before commit 1dccb598df54 ("arm64: simplify dma_get_ops"), for
>> arm64, devices for which dma_ops were not explicitly set were automatically
>> configured to use swiotlb_dma_ops, since this was hard-coded as the
>> global "dma_ops" in arm64_dma_init().
>>
>> Now that global "dma_ops" has been removed, all devices much have their
>> dma_ops explicitly set by a call to arch_setup_dma_ops(), otherwise the
>> device is assigned dummy_dma_ops, and thus calls to map_sg for such a
>> device will fail (return 0).
>>
>> Mediatek SPI uses DMA but does not use a dma channel.  Support for this
>> was added by commit c37f45b5f1cd ("spi: support spi without dma channel
>> to use can_dma()"), which uses the master_spi dev to DMA map buffers.
>>
>> The master_spi device is not a platform device, rather it is created
>> in spi_alloc_device(), and therefore its dma_ops are never set.
>>
>> Therefore, when the mediatek SPI driver when it does DMA (for large SPI
>> transactions > 32 bytes), SPI will use spi_map_buf()->dma_map_sg() to
>> map the buffer for use in DMA.  But dma_map_sg()->dma_map_sg_attrs() returns
>> 0, because ops->map_sg is dummy_dma_ops->__dummy_map_sg, and hence
>> spi_map_buf() returns -ENOMEM (-12).
>>
>> Fix this by using the real spi_master's parent device which should be a
>> real physical device with DMA properties.
> 
> Wouldn't be better to copy necessary stuff from parent device to its
> child? Like DMA mask propagation (I dunno if there are such in SPI,
> but at least many other drivers are doing that).

Absolutely not. The "necessary stuff" includes arch-specific things
which would require filling the SPI code with arch-specific #ifdefs, and
may go as far as private data structures held by other drivers (e.g.
IOMMUs) which the SPI code cannot even touch, let alone know what to do
with. The "parent device" here *is* the real DMA master device (as
described by firmware and configured by the arch code accordingly), so
using any other device for DMA is just plain wrong.

Robin.