linux-kernel - Re: [PATCH] dmaengine: rcar-dmac: use DMATCRB when xxx_TO

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: <87tvyy7i7o.wl%kuninori.morimoto.gx@renesas.com>
Date:   Tue, 17 Oct 2017 00:12:47 +0000
From:   Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Vinod Koul <vinod.koul@...el.com>,
        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: Re: [PATCH] dmaengine: rcar-dmac: use DMATCRB when xxx_TO_MEM direction


Hi Laurent

Thank you for your review

> (By the way the subject line should have mentioned v2)

Yeah. I didn't because I exchanged title...

> >          TCR       TCRB
> >  [SOURCE] -> [DMAC] -> [DESTINATION]
(snip)
> In the MEM_TO_DEV direction, what really matters is how much data has been 
> written to memory or to the device. If the DMA is interrupted between read and 
> write, the data read but not written doesn't matter. It 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 (read from the 
> device but never written to memory), which can also be important to report.
> 
> In practice, however, I see why DMA could be interrupted between read and 
> write in the MEM_TO_DEV case if the device doesn't acknowledge a write for 
> whatever reason. Interruptions in the DEV_TO_MEM case would surprise me, as 
> the write side shouldn't prevent data from reaching memory (or we'd have much 
> worse problems than DMA). Both TCR and TCRB should thus be equivalent in this 
> case.
> 
> Similarly the MEM_TO_MEM case should be fine with both TCR and TCRB.
> 
> There could be problems I'm not aware of, so the explanation above might not 
> be correct, but if it is you could use TCRB unconditionally as you did in v1. 
> In any case, this patch uses TCR for MEM_TO_DEV, while I think the correct 
> register in that case is TCRB.
> 
> I believe it would be worth capturing the above explanation in the commit 
> message and/or comment.

Thank you for your explanation.
My 1st patch focused to "transfer completed" count (= TCRB) for all case.
In any case, "completed" information should be used.
But in MEM_TO_DEV case, I thought if is OK if data was read from MEM
(= the data will be send to DEV automatically, I didn't care about interruption)
But yes, your opinion is correct I think.

I think MEM_TO_MEM should use TCRB.
I think logic is same as your MEM_TO_DEV explanation ?

Anyway, in all case I can use TCRB in v3 patch,
and it needs abouve explanation. 

Best regards
---
Kuninori Morimoto