linux-kernel - Re: Tearing down DMA transfer setup after DMA client has finished

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: <yw1x8ts78w33.fsf@unicorn.mansr.com>
Date:   Fri, 25 Nov 2016 14:37:04 +0000
From:   Måns Rullgård <mans@...sr.com>
To:     Mason <slash.tmp@...e.fr>
Cc:     Vinod Koul <vinod.koul@...el.com>, dmaengine@...r.kernel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        Dan Williams <dan.j.williams@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Jon Mason <jdmason@...zu.us>, Mark Brown <broonie@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Lee Jones <lee.jones@...aro.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Arnd Bergmann <arnd@...db.de>,
        Maxime Ripard <maxime.ripard@...e-electrons.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
Subject: Re: Tearing down DMA transfer setup after DMA client has finished

Mason <slash.tmp@...e.fr> writes:

> On 25/11/2016 14:11, Måns Rullgård wrote:
>
>> Mason writes:
>> 
>>> It seems there is a disconnect between what Linux expects - an IRQ
>>> when the transfer is complete - and the quirks of this HW :-(
>>>
>>> On this system, there are MBUS "agents" connected via a "switch box".
>>> An agent fires an IRQ when it has dealt with its *half* of the transfer.
>>>
>>> SOURCE_AGENT <---> SBOX <---> DESTINATION_AGENT
>>>
>>> Here are the steps for a transfer, in the general case:
>>>
>>> 1) setup the sbox to connect SOURCE TO DEST
>>> 2) configure source to send N bytes
>>> 3) configure dest to receive N bytes
>>>
>>> When SOURCE_AGENT has sent N bytes, it fires an IRQ
>>> When DEST_AGENT has received N bytes, it fires an IRQ
>>> The sbox connection can be torn down only when the destination
>>> agent has received all bytes.
>>> (And the twist is that some agents do not have an IRQ line.)
>>>
>>> The system provides 3 RAM-to-sbox agents (read channels)
>>> and 3 sbox-to-RAM agents (write channels).
>>>
>>> The NAND Flash controller read and write agents do not have
>>> IRQ lines.
>>>
>>> So for a NAND-to-memory transfer (read from device)
>>> - nothing happens when the NFC has finished sending N bytes to the sbox
>>> - the write channel fires an IRQ when it has received N bytes
>>>
>>> In that case, one IRQ fires when the transfer is complete,
>>> like Linux expects.
>>>
>>> For a memory-to-NAND transfer (write to device)
>>> - the read channel fires an IRQ when it has sent N bytes
>>> - the NFC driver is supposed to poll the NFC to determine
>>> when the controller has finished writing N bytes
>>>
>>> In that case, the IRQ does not indicate that the transfer
>>> is complete, merely that the sending half has finished
>>> its part.
>> 
>> When does your NAND controller signal completion?  When it has received
>> the DMA data, or only when it has finished the actual write operation?
>
> The NAND controller provides a STATUS register.
> Bit 31 is the CMD_READY bit.
> This bit goes to 0 when the controller is busy, and to 1
> when the controller is ready to accept the next command.
>
> The NFC driver is doing:
>
> 	res = wait_for_completion_timeout(&tx_done, HZ);
> 	if (res > 0)
> 		err = readl_poll_timeout(addr, val, val & CMD_READY, 0, 1000);
>
> So basically, sleep until the memory agent IRQ falls,
> then spin until the controller is idle.

This doesn't answer my question.  Waiting for the entire operation to
finish isn't necessary.  The dma driver only needs to wait until all the
data has been received by the nand controller, not until the controller
is completely finished with the command.  Does the nand controller
provide an indication for completion of the dma independently of the
progress of the write command?  The dma glue Sigma added to the
Designware sata controller does this.

> Did you see that adding a 10 µs delay at the start of
> tangox_dma_pchan_detach() makes the system no longer
> fail (passes an mtd_speedtest).

Yes, but maybe that's much longer than is actually necessary.

>>> I think it is possible to have a generic solution:
>>> Right now, the callback is called from tasklet context.
>>> If we can have a new flag to have the callback invoked
>>> directly from the ISR, then the driver for the client
>>> device can do what is required.
>> 
>> No, that won't work.  The callback shouldn't run in interrupt context.
>
> What if the callback only spun for, at most, 10 µs ?
>
> 	readl_poll_timeout(addr, val, val & CMD_READY, 0, 10);

That's far too long to wait in interrupt of tasklet context.

-- 
Måns Rullgård