linux-kernel - Re: [EXT] Re: [PATCH v3 04/12] mtd: rawnand: introduce struct onfi

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:   Tue, 4 Jun 2019 15:00:47 +0200
From:   Boris Brezillon <boris.brezillon@...labora.com>
To:     "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@...ron.com>
Cc:     Lucas Stach <dev@...xeye.de>,
        Vignesh Raghavendra <vigneshr@...com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Marcel Ziswiler <marcel.ziswiler@...adex.com>,
        Richard Weinberger <richard@....at>,
        Yixun Lan <yixun.lan@...ogic.com>,
        Chuanhong Guo <gch981213@...il.com>,
        Stefan Agner <stefan@...er.ch>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Paul Cercueil <paul@...pouillou.net>,
        Marek Vasut <marek.vasut@...il.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        Frieder Schrempf <frieder.schrempf@...tron.de>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Anders Roxell <anders.roxell@...aro.org>,
        Brian Norris <computersforpeace@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        "Bean Huo \(beanhuo\)" <beanhuo@...ron.com>
Subject: Re: [EXT] Re: [PATCH v3 04/12] mtd: rawnand: introduce struct
 onfi_helper

On Tue, 4 Jun 2019 12:02:28 +0000
"Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@...ron.com> wrote:

> Hi Boris,
> 
> > > Create onfi_helper object. This is base to turn ONFI code to generic.
> > >
> > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@...ron.com>
> > > ---
> > >  include/linux/mtd/nand.h | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> > > index 3cdf06cae8b6..645dde4c5797 100644
> > > --- a/include/linux/mtd/nand.h
> > > +++ b/include/linux/mtd/nand.h
> > > @@ -11,6 +11,7 @@
> > >  #define __LINUX_MTD_NAND_H
> > >
> > >  #include <linux/mtd/mtd.h>
> > > +#include <linux/mtd/onfi.h>
> > >
> > >  /**
> > >   * struct nand_memory_organization - Memory organization structure
> > > @@ -157,6 +158,24 @@ struct nand_ops {
> > >  	bool (*isbad)(struct nand_device *nand, const struct nand_pos  
> > *pos);  
> > >  };
> > >
> > > +/**
> > > + * struct onfi_helper - ONFI helper functions that should be implemented  
> > by  
> > > + * specialized layers (raw NAND, SPI NAND, etc.)
> > > + * @page: Page number for ONFI parameter table
> > > + * @check_revision: Check ONFI revision number
> > > + * @parameter_page_read: Function to read parameter pages
> > > + * @init_intf_data: Initialize interface specific data or fixups
> > > + */
> > > +struct onfi_helper {
> > > +	u8 page;
> > > +	int (*check_revision)(struct nand_device *base,
> > > +			      struct nand_onfi_params *p, int *onfi_version);
> > > +	int (*parameter_page_read)(struct nand_device *base, u8 page,
> > > +				   void *buf, unsigned int len);
> > > +	int (*init_intf_data)(struct nand_device *base,
> > > +			      struct nand_onfi_params *p);
> > > +};
> > > +
> > >  /**
> > >   * struct nand_device - NAND device
> > >   * @mtd: MTD instance attached to the NAND device
> > > @@ -165,6 +184,7 @@ struct nand_ops {
> > >   * @rowconv: position to row address converter
> > >   * @bbt: bad block table info
> > >   * @ops: NAND operations attached to the NAND device
> > > + * @helper: Helper functions to detect and initialize ONFI NAND
> > >   *
> > >   * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND,  
> > OneNAND)  
> > >   * should declare their own NAND object embedding a nand_device struct  
> > (that's  
> > > @@ -183,6 +203,7 @@ struct nand_device {
> > >  	struct nand_row_converter rowconv;
> > >  	struct nand_bbt bbt;
> > >  	const struct nand_ops *ops;
> > > +	struct onfi_helper helper;  
> > 
> > Sorry, but I don't think that's the right solution. When I said we
> > should have ONFI code shared I was thinking about the code that parses
> > the ONFI struct/data to extract nand_memory_organization bits or other
> > generic info, not something that would abstract how to retrieve the
> > ONFI param page. Clearly, the generic NAND layer is not supposed to
> > handle such protocol/low-level details.
> >   
> 
> In that case, I am thinking to design as follows, which splits into generic independent code.
> Let me know, if you have any concerns or inputs.
> 
> I will parsing code from nand_onfi_detect function and move it to mtd/nand/onfi.c.
> Also, I will move functions like sanitize_string, nand_bit_wise_majority, onfi_crc16, and 
> any other generic info to mtd/nand/onfi.c.

Sounds good.