linux-kernel - [PATCH 2/2 v3] dmaengine: rcar-dmac: use TCRB instead of TCR for residue

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:   Fri, 17 Nov 2017 02:09:32 +0000
From:   Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Vinod Koul <vinod.koul@...el.com>
Cc:     Niklas Söderlund 
        <niklas.soderlund+renesas@...natech.se>, dmaengine@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@...esas.com>
Subject: [PATCH 2/2 v3] dmaengine: rcar-dmac: use TCRB instead of TCR for residue

From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>

SYS/RT/Audio DMAC includes independent data buffers for reading
and writing. Therefore, the read transfer counter and write transfer
counter have different values.
TCR indicates read counter, and TCRB indicates write counter.
The relationship is like below.

	        TCR       TCRB
	[SOURCE] -> [DMAC] -> [SINK]

In the MEM_TO_DEV direction, what really matters is how much data has
been written to the device. If the DMA is interrupted between read and
write, then, the data doesn't end up in the destination, so shouldn't
be counted. TCRB is thus the register we should use in this cases.

In the DEV_TO_MEM direction, the situation is more complex. Both the
read and write side are important. What matters from a data consumer
point of view is how much data has been written to memory.
On the other hand, if the transfer is interrupted between read and
write, we'll end up losing data. It can also be important to report.

In the MEM_TO_MEM direction, what matters is of course how much data
has been written to memory from data consumer point of view.
Here, because read and write have independent data buffers, it will
take a while for TCR and TCRB to become equal. Thus we should check
TCRB in this case, too.

Thus, all cases we should check TCRB instead of TCR.

Without this patch, Sound Capture has noise after PulseAudio support
(= 07b7acb51d2 ("ASoC: rsnd: update pointer more accurate")), because
the recorder will use wrong residue counter which indicates transferred
from sound device, but in reality the data was not yet put to memory
and recorder will record it.

However, because DMAC is buffering data until it can be transferable
size, TCRB might not be updated.
For example, if consumer doesn't know how much data can be received,
it requests enough size to DMAC. But in reality, it might receive very
few data. In such case, DMAC just buffered it until transferable size,
and no TCRB updated.

In such case, this buffered data will be transferred if CHCR::DE bit was
cleared, and this is happen if rcar_dmac_chan_halt(). In other word, it
happen when consumer called dmaengine_terminate_all().

Because of this behavior, it need to flush buffered data when it returns
"residue" (= dmaengine_tx_status()).
Otherwise, consumer might calculate wrong things if it called
dmaengine_tx_status() and dmaengine_terminate_all() consecutively.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
Tested-by: Hiroyuki Yokoyama <hiroyuki.yokoyama.vx@...esas.com>
Tested-by: Ryo Kodama <ryo.kodama.vz@...esas.com>
Tested-by: Geert Uytterhoeven <geert+renesas@...der.be>
---
v2 -> v3

 - fixups many typo on gitlog / comment
 - added Geert's Tested-by

 drivers/dma/sh/rcar-dmac.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index c99fd0f..3bbd11d 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -761,6 +761,23 @@ static void rcar_dmac_chcr_de_barrier(struct rcar_dmac_chan *chan)
 	dev_err(chan->chan.device->dev, "CHCR DE check error\n");
 }

+static void rcar_dmac_sync_tcr(struct rcar_dmac_chan *chan)
+{
+	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
+
+	if (!(chcr & RCAR_DMACHCR_DE))
+		return;
+
+	/* set DE=0 and flush remaining data */
+	rcar_dmac_chan_write(chan, RCAR_DMACHCR, (chcr & ~RCAR_DMACHCR_DE));
+
+	/* make sure all remaining data was flushed */
+	rcar_dmac_chcr_de_barrier(chan);
+
+	/* back DE */
+	rcar_dmac_chan_write(chan, RCAR_DMACHCR, chcr);
+}
+
 static void rcar_dmac_chan_halt(struct rcar_dmac_chan *chan)
 {
 	u32 chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
@@ -1329,8 +1346,11 @@ static unsigned int rcar_dmac_chan_get_residue(struct rcar_dmac_chan *chan,
 		residue += chunk->size;
 	}

+	if (desc->direction == DMA_DEV_TO_MEM)
+		rcar_dmac_sync_tcr(chan);
+
 	/* Add the residue for the current chunk. */
-	residue += rcar_dmac_chan_read(chan, RCAR_DMATCR) << desc->xfer_shift;
+	residue += rcar_dmac_chan_read(chan, RCAR_DMATCRB) << desc->xfer_shift;

 	return residue;
 }
-- 
1.9.1