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: <20180509065922.GC7985@vkoul-mobl>
Date:   Wed, 9 May 2018 12:29:22 +0530
From:   Vinod Koul <vinod.koul@...aro.org>
To:     Frank Mori Hess <fmh6jj@...il.com>
Cc:     Marek Szyprowski <m.szyprowski@...sung.com>,
        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 08-05-18, 10:36, Frank Mori Hess wrote:
> 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.

Hi Marek,

I would appreciate if you can review the pl330 changes as that clearly seems to
impact you. This was in review for quite a bit

> 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.

That makes sense to me. If dma doesnt support, then why should SW claim broken
support..

-- 
~Vinod