linux-kernel - Re: [PATCH v2] mtd: spi-nor: Add support for SPI boot flash access for AMD Family 16h

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] [day] [month] [year] [list]

Date:   Wed, 31 Oct 2018 14:17:42 +0100
From:   Boris Brezillon <boris.brezillon@...tlin.com>
To:     "Grandbois, Brett" <brett.grandbois@...ngear.com>
Cc:     Richard Weinberger <richard@....at>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Marek Vasut <marek.vasut@...il.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        Brian Norris <computersforpeace@...il.com>,
        David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH v2] mtd: spi-nor: Add support for SPI boot flash access
 for AMD Family 16h

On Wed, 31 Oct 2018 03:18:28 +0000
"Grandbois, Brett" <brett.grandbois@...ngear.com> wrote:

> On 30/10/18 6:26 pm, Boris Brezillon wrote:
> > On Mon, 29 Oct 2018 23:15:42 +0000
> > "Grandbois, Brett"<brett.grandbois@...ngear.com>  wrote:
> >  
> >> On 28/10/18 1:39 am, Boris Brezillon wrote:  
> >>> Hi Brett,
> >>>
> >>> On Tue, 16 Oct 2018 00:57:41 +0000
> >>> "Grandbois, Brett"<brett.grandbois@...ngear.com>   wrote:
> >>>     
> >>>> Add support to expose the SPI boot flash on AMD Family 16h CPUs
> >>>> as a standard mtd device to give userspace BIOS updaters greater
> >>>> feature support.  The BIOS and Kernel Developer's Guide refers to
> >>>> this as the 'SPI ROM' controller and so the driver follows that
> >>>> naming convention for consistency.
> >>>>     
> >>> We're currently trying to convert spi-nor controller drivers to
> >>> the spi-mem interface [1]. Can you look at this new interface and
> >>> tell me if you'd be able to implement it? If that's not possible,
> >>> then I'd prefer to have this driver implement the mtd_info
> >>> interface directly.  
> >> So from going over the spi-mem interface it looks like the intent
> >> is for these sorts of devices to be a standard spi_controller with
> >> only mem_ops defined and the transfer/_one/_one_message left as
> >> NULL?  Is that correct?  
> > Yes
> >  
> >> That's a bit of a pivot from how it's currently done
> >> (it's conceptually similar to the intel-spi-pci driver so I was
> >> following that)  
> > Yes, and that's exactly what I'd like to avoid. intel-spi-pci will
> > probably be the trickiest conversion, so I'd like to avoid having
> > another one ;-).
> >  
> >> but I should be able to rework it to the new
> >> interface.  This then lives under drivers/spi and thus should be
> >> submitted to linux-spi?  
> > Actually, if the controller is only ever connected to the same SPI
> > NOR chip (no need for advanced detection scheme) and does not
> > support support Octo/Quad/Dual modes (or any other advanced
> > features), you'll be better off implementing
> > mtd->_read/_write/_erase() directly (the driver would then live in
> > drivers/mtd/devices/).  
> 
> Hmm, conceptually the device is better suited to the mtd layer than
> the spi layer.  The HW is designed to only ever access spi flash
> chips, not as a general spi master controller, so really the end
> result of it should always only ever be 1 mtd device.  Unfortunately
> as the device probing and command sequences for this are the same as
> implemented in spi-nor I'd either be duplicating a lot of existing
> code, or just wrapping around spi_nor_scan which sounds like we're
> back to the dedicated spi-nor controller you're trying to move away
> from.
> 
> The spi-mem interface ops do nicely match up with what the controller
> supports, including the new direct mapping mode which I'd be able to
> make use of, so as long as there aren't any issues with supporting
> only mem_ops and not the message transfers

That's allowed.

> then it's probably the way to go.

Sounds good then.