linux-kernel - Re: [PATCH] spi: mediatek: Manually set dma_ops for 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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Wed, 25 Jan 2017 18:24:12 +0800
From:   Daniel Kurtz <djkurtz@...omium.org>
To:     Robin Murphy <robin.murphy@....com>
Cc:     Leilk Liu <leilk.liu@...iatek.com>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:SPI SUBSYSTEM" <linux-spi@...r.kernel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Mark Brown <broonie@...nel.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>, groeck@...omium.org,
        Dmitry Torokhov <dtor@...omium.org>,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] spi: mediatek: Manually set dma_ops for spi_master device

Hi Robin,

On Tue, Jan 24, 2017 at 11:14 PM, Robin Murphy <robin.murphy@....com> wrote:
> Hi Dan,

[snip...]

>> And I really don't know why we needed to set the coherent_dma_mask to 0 to
>> avoid SPI transaction errors.
>
> Following what I mentioned above, "git grep dma_alloc drivers/spi" makes
> it seem like coherent DMA isn't used anywhere relevant, which is rather
> puzzling. Unless of course somewhere along the line somebody's done the
> dev->dma_mask = &dev->dma_coherent_mask hack, with the result that
> writing one mask hits both, making *all* DMA fail and big transfers fall
> back to PIO.

You mean this last line?

>> @@ -575,6 +576,10 @@ static int mtk_spi_probe(struct platform_device *pdev)
>>               goto err_put_master;
>>       }
>>
>> +     /* Call of_dma_configure to set up spi_master's dma_ops */
>> +     of_dma_configure(&master->dev, master->dev.of_node);
>> +     /* But explicitly set the coherent_dma_mask to 0 */
>> +     master->dev.coherent_dma_mask = 0;
>>       if (!pdev->dev.dma_mask)
>>               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
                                  ^^^^^

As predicted, setting the dma_mask = 0 causes "all DMA to fail".  In
particular, dma_capable() always returns false, so
swiotlb_map_sg_attrs() takes the map_single() path, instead of just
assigning:

   sg->dma_address = dev_addr;

swiotlb_map_sg_attrs(struct device *hwdev, struct scatterlist *sgl, int nelems,
                     enum dma_data_direction dir, unsigned long attrs)
{
        struct scatterlist *sg;
        int i;

        BUG_ON(dir == DMA_NONE);

        for_each_sg(sgl, sg, nelems, i) {
                phys_addr_t paddr = sg_phys(sg);
                dma_addr_t dev_addr = phys_to_dma(hwdev, paddr);

                if (swiotlb_force == SWIOTLB_FORCE ||
                    !dma_capable(hwdev, dev_addr, sg->length)) {
                        phys_addr_t map = map_single(hwdev, sg_phys(sg),
                                                     sg->length, dir, attrs);
                        ...
                        }
                        sg->dma_address = phys_to_dma(hwdev, map);
                } else
                        sg->dma_address = dev_addr;
                sg_dma_len(sg) = sg->length;
        }
        return nelems;
}

So, I think this means that the only reason the MTK SPI driver ever
worked before was that it was tested on an older kernel, so the
spi_master was defaulting to swiotlb_dma_ops with a 0 dma_mask, and
therefore it was using SWIOTLB bounce buffers (via 'map_single'), and
not actually ever doing real DMA.

I'm in a bit over my head here, any suggestions on how to fix this?

-Dan