linux-kernel - Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio

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:   Thu, 11 Jun 2020 14:40:42 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Robin Gong <yibin.gong@....com>
Cc:     shawnguo@...nel.org, s.hauer@...gutronix.de, festevam@...il.com,
        robin.murphy@....com, matthias.schiffer@...tq-group.com,
        kernel@...gutronix.de, linux-imx@....com,
        linux-spi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 RFC 1/2] spi: introduce fallback to pio

On Thu, Jun 11, 2020 at 08:58:29PM +0800, Robin Gong wrote:
> Add SPI_CONTROLLER_FALLBACK to fallback to pio mode in case dma transfer
> failed.
> If spi client driver want to enable this feature please set master->flags
> with SPI_MASTER_FALLBACK and add master->fallback checking in its can_dma()
> as spi-imx.c

If we were going to do this I don't see why we'd have a flag for this
rather than just doing it unconditionally but...

>  			ret = ctlr->transfer_one(ctlr, msg->spi, xfer);
>  			if (ret < 0) {
> +				if (ctlr->cur_msg_mapped &&
> +				   (ctlr->flags & SPI_CONTROLLER_FALLBACK)) {
> +					__spi_unmap_msg(ctlr, msg);
> +					ctlr->fallback = true;
> +					goto fallback_pio;
> +				}

...I don't think this can work sensibly - this is going to try PIO if
there's *any* error.  We might have had some sort of issue during the
transfer for example so have some noise on the bus.  Like I said on a
prior version of this I really think that we need to be figuring out if
the DMA controller can support the transaction before we even map the
buffer for it, having the controller just randomly fail underneath the
consumer just does not sound robust.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)