linux-kernel - Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the MID 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]

Date:   Fri, 22 May 2020 02:33:27 +0300
From:   Serge Semin <Sergey.Semin@...kalelectronics.ru>
To:     Mark Brown <broonie@...nel.org>,
        Grant Likely <grant.likely@...retlab.ca>,
        Vinod Koul <vkoul@...nel.org>, Feng Tang <feng.tang@...el.com>,
        Alan Cox <alan@...ux.intel.com>,
        Linus Walleij <linus.walleij@...ricsson.com>
CC:     Serge Semin <fancer.lancer@...il.com>,
        Georgy Vlasov <Georgy.Vlasov@...kalelectronics.ru>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        Ralf Baechle <ralf@...ux-mips.org>,
        Arnd Bergmann <arnd@...db.de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Rob Herring <robh+dt@...nel.org>, <linux-mips@...r.kernel.org>,
        <devicetree@...r.kernel.org>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Wan Ahmad Zainie <wan.ahmad.zainie.wan.mohamad@...el.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Clement Leger <cleger@...ray.eu>, <linux-spi@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 01/16] spi: dw: Add Tx/Rx finish wait methods to the
 MID DMA

Mark, Andy,

On Thu, May 21, 2020 at 04:21:51AM +0300, Serge Semin wrote:
>  

[nip]

> +static void dw_spi_dma_calc_delay(struct dw_spi *dws, u32 nents,
> +				  struct spi_delay *delay)
> +{
> +	unsigned long ns, us;
> +
> +	ns = (NSEC_PER_SEC / spi_get_clk(dws)) * nents * dws->n_bytes *
> +	     BITS_PER_BYTE;
> +
> +	if (ns <= NSEC_PER_USEC) {
> +		delay->unit = SPI_DELAY_UNIT_NSECS;
> +		delay->value = ns;
> +	} else {
> +		us = DIV_ROUND_UP(ns, NSEC_PER_USEC);
> +		delay->unit = SPI_DELAY_UNIT_USECS;
> +		delay->value = clamp_val(us, 0, USHRT_MAX);
> +	}
> +}
> +
> +static inline bool dw_spi_dma_tx_busy(struct dw_spi *dws)
> +{
> +	return !(dw_readl(dws, DW_SPI_SR) & SR_TF_EMPT);
> +}
> +
> +static void dw_spi_dma_wait_tx_done(struct dw_spi *dws)
> +{
> +	int retry = WAIT_RETRIES;
> +	struct spi_delay delay;
> +	u32 nents;
> +
> +	nents = dw_readl(dws, DW_SPI_TXFLR);
> +	dw_spi_dma_calc_delay(dws, nents, &delay);
> +
> +	while (dw_spi_dma_tx_busy(dws) && retry--)

> +		spi_delay_exec(&delay, NULL);

I've just discovered using spi_delay_exec() wasn't a good idea here. Look at the
call stack:
dw_dma_tasklet() -> dwc_scan_descriptors() -> dwc_descriptor_complete() ->
dw_spi_dma_tx_done() -> spi_delay_exec() -> usleep_range() -> ...

So tasklet is calling a sleeping function.((( I've absolutely forgotten to check
the context the DMA completion function is called with. We'll have to manually
select either ndelay or udelay here and nothing else. Since basically both
functions represent an atomic context and most of the platforms ndelay fallsback to
udelay, I'll get the ndelay back to the wait functions. I'll resend a patchset
shortly.

-Sergey