linux-kernel - Re: Revert "dmaengine: pl330: add DMA

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]

Message-ID: <CAJz5OpcGX_3u5POExdhnBbRu-T-m7KodGAvk50BOfZWKeH--kg@mail.gmail.com>
Date:   Tue, 8 May 2018 10:36:18 -0400
From:   Frank Mori Hess <fmh6jj@...il.com>
To:     Marek Szyprowski <m.szyprowski@...sung.com>
Cc:     Vinod Koul <vkoul@...nel.org>, dmaengine@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Dan Williams <dan.j.williams@...el.com>,
        r.baldyga@...kerion.com, Krzysztof Kozlowski <krzk@...nel.org>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: Revert "dmaengine: pl330: add DMA_PAUSE feature"

On Tue, May 8, 2018 at 5:04 AM, Marek Szyprowski
<m.szyprowski@...sung.com> wrote:
> Hi Frank and Vinod,
>
> On 2018-04-28 23:50, Frank Mori Hess wrote:
>> This reverts commit 88987d2c7534a0269f567fb101e6d71a08f0f01d.
>>
>> The pl330.c pause implementation violates the dmaengine requirement
>> for no data loss, since it relies on the DMAKILL
>> instruction.  However, DMAKILL discards in-flight data from the
>> dma controller's fifo.  This is documented in the dma-330 manual
>> and I have observed it with hardware doing device-to-memory burst
>> transfers.  The discarded data may or may not show up in the
>> residue count, depending on timing (resulting in data corruption
>> effectively).
>>
>> Signed-off-by: Frank Mori Hess <fmh6jj@...il.com>
>
> This revert completely breaks serial driver operation on almost all Exynos
> SoCs, because serial driver relies on having PAUSE feature and proper
> residue reporting from dma engine. Please drop it if possible.
>

It will cause the serial driver to not use the pl330.c driver for dma,
the serial driver will fall back on using the cpu.  This is
unfortunate, but the dma hardware simply does not support pause.  The
"nice" stop instruction DMAEND is not allowed to be inserted using the
debug instruction register.  The only possibility for implementing
pause would be to make the dma transfer do a DMAWFE (wait for event)
before every transfer.  Then you would need to devote another dma
thread to doing nothing but DMASEV (send event) to keep the transfer
going.  The pause could then DMAKILL the event-generating thread
rather than the transfer thread.  I don't know exactly what the
performance impact would be, but it couldn't be good.

The serial driver could be modified to still use dma for TX, since it
only needs pause for RX.  Also, if your serial hardware can report
exactly how many bytes it has sitting in its rx fifo, the serial
driver could be modified to use pause-less dma for RX.  This is
actually what I did for the custom serial hardware I'm using with a
dma-330, although our serial hardware has a very large rx fifo which
makes this scheme worthwhile.