| 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
| ||
|
Message-ID: <20211015081313.60018976@xps13>
Date: Fri, 15 Oct 2021 08:13:13 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Paul Cercueil <paul@...pouillou.net>
Cc: Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
Harvey Hunt <harveyhuntnexus@...il.com>, list@...ndingux.net,
linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mips@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 2/3] mtd: rawnand: Export
nand_read_page_hwecc_oob_first()
Hi Paul,
paul@...pouillou.net wrote on Sat, 9 Oct 2021 20:49:51 +0200:
> Move the function nand_read_page_hwecc_oob_first() (previously
> nand_davinci_read_page_hwecc_oob_first()) to nand_base.c, and export it
> as a GPL symbol, so that it can be used by more modules.
>
> Cc: <stable@...r.kernel.org> # v5.2
> Fixes: a0ac778eb82c ("mtd: rawnand: ingenic: Add support for the JZ4740")
> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> ---
> drivers/mtd/nand/raw/davinci_nand.c | 70 +----------------------------
> drivers/mtd/nand/raw/nand_base.c | 69 ++++++++++++++++++++++++++++
> include/linux/mtd/rawnand.h | 2 +
> 3 files changed, 72 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/davinci_nand.c b/drivers/mtd/nand/raw/davinci_nand.c
> index 89de24d3bb7a..45fec8c192ab 100644
> --- a/drivers/mtd/nand/raw/davinci_nand.c
> +++ b/drivers/mtd/nand/raw/davinci_nand.c
> @@ -371,74 +371,6 @@ static int nand_davinci_correct_4bit(struct nand_chip *chip, u_char *data,
> return corrected;
> }
>
> -/**
> - * nand_read_page_hwecc_oob_first - hw ecc, read oob first
> - * @chip: nand chip info structure
> - * @buf: buffer to store read data
> - * @oob_required: caller requires OOB data read to chip->oob_poi
> - * @page: page number to read
> - *
> - * Hardware ECC for large page chips, require OOB to be read first. For this
> - * ECC mode, the write_page method is re-used from ECC_HW. These methods
> - * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with
> - * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from
> - * the data area, by overwriting the NAND manufacturer bad block markings.
> - */
> -static int nand_davinci_read_page_hwecc_oob_first(struct nand_chip *chip,
> - uint8_t *buf,
> - int oob_required, int page)
> -{
> - struct mtd_info *mtd = nand_to_mtd(chip);
> - int i, eccsize = chip->ecc.size, ret;
> - int eccbytes = chip->ecc.bytes;
> - int eccsteps = chip->ecc.steps;
> - uint8_t *p = buf;
> - uint8_t *ecc_code = chip->ecc.code_buf;
> - unsigned int max_bitflips = 0;
> -
> - /* Read the OOB area first */
> - ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
> - if (ret)
> - return ret;
> -
> - ret = nand_read_page_op(chip, page, 0, NULL, 0);
> - if (ret)
> - return ret;
> -
> - ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> - chip->ecc.total);
> - if (ret)
> - return ret;
> -
> - for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> - int stat;
> -
> - chip->ecc.hwctl(chip, NAND_ECC_READ);
> -
> - ret = nand_read_data_op(chip, p, eccsize, false, false);
> - if (ret)
> - return ret;
> -
> - stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
> - if (stat == -EBADMSG &&
> - (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
> - /* check for empty pages with bitflips */
> - stat = nand_check_erased_ecc_chunk(p, eccsize,
> - &ecc_code[i],
> - eccbytes, NULL, 0,
> - chip->ecc.strength);
> - }
> -
> - if (stat < 0) {
> - mtd->ecc_stats.failed++;
> - } else {
> - mtd->ecc_stats.corrected += stat;
> - max_bitflips = max_t(unsigned int, max_bitflips, stat);
> - }
> - }
> - return max_bitflips;
> -}
> -
> /*----------------------------------------------------------------------*/
>
> /* An ECC layout for using 4-bit ECC with small-page flash, storing
> @@ -648,7 +580,7 @@ static int davinci_nand_attach_chip(struct nand_chip *chip)
> } else if (chunks == 4 || chunks == 8) {
> mtd_set_ooblayout(mtd,
> nand_get_large_page_ooblayout());
> - chip->ecc.read_page = nand_davinci_read_page_hwecc_oob_first;
> + chip->ecc.read_page = nand_read_page_hwecc_oob_first;
> } else {
> return -EIO;
> }
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 3d6c6e880520..cb5f343b9fa2 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3160,6 +3160,75 @@ static int nand_read_page_hwecc(struct nand_chip *chip, uint8_t *buf,
> return max_bitflips;
> }
>
> +/**
> + * nand_read_page_hwecc_oob_first - Hardware ECC page read with ECC
> + * data read from OOB area
> + * @chip: nand chip info structure
> + * @buf: buffer to store read data
> + * @oob_required: caller requires OOB data read to chip->oob_poi
> + * @page: page number to read
> + *
> + * Hardware ECC for large page chips, require OOB to be read first. For this
requires
With this ECC configuration?
> + * ECC mode, the write_page method is re-used from ECC_HW. These methods
I do not understand this sentence nor the next one about syndrome. I
believe it is related to your engine and should not leak into the core.
> + * read/write ECC from the OOB area, unlike the ECC_HW_SYNDROME support with
> + * multiple ECC steps, follows the "infix ECC" scheme and reads/writes ECC from
> + * the data area, by overwriting the NAND manufacturer bad block markings.
That's a sentence I don't like. What do you mean exactly?
What "Infix ECC" scheme is?
Do you mean that unlike the syndrome mode it *does not* overwrite the
BBM ?
> + */
> +int nand_read_page_hwecc_oob_first(struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + int i, eccsize = chip->ecc.size, ret;
> + int eccbytes = chip->ecc.bytes;
> + int eccsteps = chip->ecc.steps;
> + uint8_t *p = buf;
> + uint8_t *ecc_code = chip->ecc.code_buf;
> + unsigned int max_bitflips = 0;
> +
> + /* Read the OOB area first */
> + ret = nand_read_oob_op(chip, page, 0, chip->oob_poi, mtd->oobsize);
> + if (ret)
> + return ret;
> +
> + ret = nand_read_page_op(chip, page, 0, NULL, 0);
Definitely not, your are requesting the chip to do the read_page
operation twice. You only need a nand_change_read_column I believe.
> + if (ret)
> + return ret;
> +
> + ret = mtd_ooblayout_get_eccbytes(mtd, ecc_code, chip->oob_poi, 0,
> + chip->ecc.total);
> + if (ret)
> + return ret;
> +
> + for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
> + int stat;
> +
> + chip->ecc.hwctl(chip, NAND_ECC_READ);
> +
> + ret = nand_read_data_op(chip, p, eccsize, false, false);
> + if (ret)
> + return ret;
> +
> + stat = chip->ecc.correct(chip, p, &ecc_code[i], NULL);
> + if (stat == -EBADMSG &&
> + (chip->ecc.options & NAND_ECC_GENERIC_ERASED_CHECK)) {
> + /* check for empty pages with bitflips */
> + stat = nand_check_erased_ecc_chunk(p, eccsize,
> + &ecc_code[i],
> + eccbytes, NULL, 0,
> + chip->ecc.strength);
> + }
> +
> + if (stat < 0) {
> + mtd->ecc_stats.failed++;
> + } else {
> + mtd->ecc_stats.corrected += stat;
> + max_bitflips = max_t(unsigned int, max_bitflips, stat);
> + }
> + }
> + return max_bitflips;
> +}
> +EXPORT_SYMBOL_GPL(nand_read_page_hwecc_oob_first);
> +
> /**
> * nand_read_page_syndrome - [REPLACEABLE] hardware ECC syndrome based page read
> * @chip: nand chip info structure
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index b2f9dd3cbd69..5b88cd51fadb 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -1539,6 +1539,8 @@ int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
> bool force_8bit, bool check_only);
> int nand_write_data_op(struct nand_chip *chip, const void *buf,
> unsigned int len, bool force_8bit);
> +int nand_read_page_hwecc_oob_first(struct nand_chip *chip, uint8_t *buf,
> + int oob_required, int page);
You certainly want to add this symbol closer to the other read/write
page helpers?
>
> /* Scan and identify a NAND device */
> int nand_scan_with_ids(struct nand_chip *chip, unsigned int max_chips,
Thanks,
Miquèl
Powered by blists - more mailing lists