linux-kernel - Re: [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support

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: <1cd1b952-9126-516f-d8a7-91149a815ff0@microchip.com>
Date:   Wed, 30 Sep 2020 09:57:06 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <p.yadav@...com>, <miquel.raynal@...tlin.com>, <richard@....at>,
        <vigneshr@...com>, <linux-mtd@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
CC:     <nsekhar@...com>, <boris.brezillon@...labora.com>
Subject: Re: [PATCH v13 00/15] mtd: spi-nor: add xSPI Octal DTR support

On 9/16/20 3:44 PM, Pratyush Yadav wrote:
> Hi,

Hello,

> 
> This series adds support for Octal DTR flashes in the SPI NOR framework,
> and then adds hooks for the Cypress Semper and Micron Xcella flashes to
> allow running them in Octal DTR mode. This series assumes that the flash
> is handed to the kernel in Legacy SPI mode.
> 

I like this series. There are some comments that can be addressed, no big
deal though.

I think that we shouldn't let the door open for users with flashes that
enter in a X-X-X mode in a non-volatile way. Think of two flashes that have
the same X-X-X mode enable sequence, but in which only the EN bit differs:
for one the EN bit is volatile and for the other it is non-volatile. Users
of the later flash that try to enable the X-X-X mode (using our code) will
end up with the flash in a mode from which they can't recover. Thus my advice
is to consider by default all the flashes, as X-X-X mode non-volatile flashes,
and to not let them use the X-X-X mode enable methods. Flashes that can enter
X-X-X modes in a volatile way, should discover this capability by parsing the
optional SFDP SCCR Map. Those that don't define this table, should pass this
capability as a flash_info flag when declaring the flash. With these, users
should be conscious about the V or NV modes, and the risk to end up with
flashes for which there is no software to recover is diminished. This is what
I tried in RFC 1/3 and RFC 3/3. I think those 2 patches should be part of
this series. 14/15 and 15/15 should be updated accordingly. RFC 2/3 has room
for discussions because it provides access system-wise, while ideally would be
to do it at flash granularity. I'll wait for your feedback on those.

> Tested on TI J721E EVM with 1-bit ECC on the Cypress flash.

As a tip, when introducing some big changes to the core, would be nice to be
assured that things that worked previously are still working now.
An erase-write-read-back test in Quad SPI would suffice. Probably you have
already did that, but I haven't seen it mentioned.

Cheers,
ta