| 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: <b7c0624d238429b04692e51c3df3dabc@mail.gmail.com>
Date: Thu, 10 Jul 2025 16:08:08 -0700
From: William Zhang <william.zhang@...adcom.com>
To: david regan <dregan@...adcom.com>, Miquèl Raynal <miquel.raynal@...tlin.com>
Cc: linux-kernel@...r.kernel.org, linux-mtd@...ts.infradead.org,
bcm-kernel-feedback-list@...adcom.com, Anand Gore <anand.gore@...adcom.com>,
Florian Fainelli <florian.fainelli@...adcom.com>, Kamal Dasu <kamal.dasu@...adcom.com>,
Dan Beygelman <dan.beygelman@...adcom.com>, Álvaro Fernández Rojas <noltari@...il.com>,
rafal@...ecki.pl, computersforpeace@...il.com, frieder.schrempf@...tron.de,
vigneshr@...com, richard@....at, bbrezillon@...nel.org, kdasu.kdev@...il.com,
jaimeliao.tw@...il.com, kilobyte@...band.pl, jonas.gorski@...il.com,
dgcbueu@...il.com, dregan@...l.com
Subject: RE: [PATCH v3] mtd: nand: brcmnand: fix mtd corrected bits stat
> -----Original Message-----
> From: William Zhang <william.zhang@...adcom.com>
> Sent: Wednesday, July 2, 2025 9:26 PM
> To: david regan <dregan@...adcom.com>
> Cc: linux-kernel@...r.kernel.org; linux-mtd@...ts.infradead.org;
> bcm-kernel-
> feedback-list@...adcom.com; anand.gore@...adcom.com;
> florian.fainelli@...adcom.com; kamal.dasu@...adcom.com;
> dan.beygelman@...adcom.com; Miquèl Raynal <miquel.raynal@...tlin.com>;
> Álvaro Fernández Rojas <noltari@...il.com>; rafal@...ecki.pl;
> computersforpeace@...il.com; frieder.schrempf@...tron.de;
> vigneshr@...com; richard@....at; bbrezillon@...nel.org;
> kdasu.kdev@...il.com; jaimeliao.tw@...il.com; kilobyte@...band.pl;
> jonas.gorski@...il.com; dgcbueu@...il.com; dregan@...l.com
> Subject: Re: [PATCH v3] mtd: nand: brcmnand: fix mtd corrected bits stat
>
> On Wed, Jul 2, 2025 at 7:47 PM david regan <dregan@...adcom.com> wrote:
> >
> > From: David Regan <dregan@...adcom.com>
> >
> > Currently we attempt to get the amount of flipped bits from a hardware
> > location which is reset on every subpage. Instead obtain total flipped
> > bits stat from hardware accumulator. In addition identify the correct
> > maximum subpage corrected bits.
> >
> > Signed-off-by: David Regan <dregan@...adcom.com>
> > ---
> > v3: Use brcmnand_corr_total to obtain maximum subpage flipped
> > bits for further backwards compatibility.
> >
> > v2: Add >= v4 NAND controller support as requested by Jonas.
> > mtd->ecc_stats.corrected accumulates instead of set to total.
> > Remove DMA specific flipped bits count.
> >
> > ---
> > drivers/mtd/nand/raw/brcmnand/brcmnand.c | 53 +++++++++++++++++-------
> > 1 file changed, 38 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > index 62bdda3be92f..b13f5f8f0eec 100644
> > --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> > @@ -359,6 +359,7 @@ enum brcmnand_reg {
> > BRCMNAND_CORR_THRESHOLD_EXT,
> > BRCMNAND_UNCORR_COUNT,
> > BRCMNAND_CORR_COUNT,
> > + BRCMNAND_READ_ERROR_COUNT,
> > BRCMNAND_CORR_EXT_ADDR,
> > BRCMNAND_CORR_ADDR,
> > BRCMNAND_UNCORR_EXT_ADDR,
> > @@ -389,6 +390,7 @@ static const u16 brcmnand_regs_v21[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0,
> > [BRCMNAND_UNCORR_COUNT] = 0,
> > [BRCMNAND_CORR_COUNT] = 0,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x60,
> > [BRCMNAND_CORR_ADDR] = 0x64,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x68,
> > @@ -419,6 +421,7 @@ static const u16 brcmnand_regs_v33[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0,
> > [BRCMNAND_UNCORR_COUNT] = 0,
> > [BRCMNAND_CORR_COUNT] = 0,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0x80,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x70,
> > [BRCMNAND_CORR_ADDR] = 0x74,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x78,
> > @@ -449,6 +452,7 @@ static const u16 brcmnand_regs_v50[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0,
> > [BRCMNAND_UNCORR_COUNT] = 0,
> > [BRCMNAND_CORR_COUNT] = 0,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0x80,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x70,
> > [BRCMNAND_CORR_ADDR] = 0x74,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x78,
> > @@ -479,6 +483,7 @@ static const u16 brcmnand_regs_v60[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0xc4,
> > [BRCMNAND_UNCORR_COUNT] = 0xfc,
> > [BRCMNAND_CORR_COUNT] = 0x100,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0x104,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x10c,
> > [BRCMNAND_CORR_ADDR] = 0x110,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x114,
> > @@ -509,6 +514,7 @@ static const u16 brcmnand_regs_v71[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0xe0,
> > [BRCMNAND_UNCORR_COUNT] = 0xfc,
> > [BRCMNAND_CORR_COUNT] = 0x100,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0x104,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x10c,
> > [BRCMNAND_CORR_ADDR] = 0x110,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x114,
> > @@ -539,6 +545,7 @@ static const u16 brcmnand_regs_v72[] = {
> > [BRCMNAND_CORR_THRESHOLD_EXT] = 0xe0,
> > [BRCMNAND_UNCORR_COUNT] = 0xfc,
> > [BRCMNAND_CORR_COUNT] = 0x100,
> > + [BRCMNAND_READ_ERROR_COUNT] = 0x104,
> > [BRCMNAND_CORR_EXT_ADDR] = 0x10c,
> > [BRCMNAND_CORR_ADDR] = 0x110,
> > [BRCMNAND_UNCORR_EXT_ADDR] = 0x114,
> > @@ -959,11 +966,11 @@ static inline u16 brcmnand_cs_offset(struct
> brcmnand_controller *ctrl, int cs,
> > return offs_cs0 + cs * ctrl->reg_spacing + cs_offs;
> > }
> >
> > -static inline u32 brcmnand_count_corrected(struct brcmnand_controller
> > *ctrl)
> > +static inline u32 brcmnand_corr_total(struct brcmnand_controller *ctrl)
> > {
> > - if (ctrl->nand_version < 0x0600)
> > - return 1;
> > - return brcmnand_read_reg(ctrl, BRCMNAND_CORR_COUNT);
> > + if (ctrl->nand_version < 0x400)
> > + return 0;
> > + return brcmnand_read_reg(ctrl, BRCMNAND_READ_ERROR_COUNT);
> > }
> >
> > static void brcmnand_wr_corr_thresh(struct brcmnand_host *host, u8 val)
> > @@ -2066,15 +2073,20 @@ static int brcmnand_dma_trans(struct
> brcmnand_host *host, u64 addr, u32 *buf,
> > */
> > static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip
> *chip,
> > u64 addr, unsigned int trans, u32 *buf,
> > - u8 *oob, u64 *err_addr)
> > + u8 *oob, u64 *err_addr, unsigned int
> > *corr)
> > {
> > struct brcmnand_host *host = nand_get_controller_data(chip);
> > struct brcmnand_controller *ctrl = host->ctrl;
> > int i, ret = 0;
> > + unsigned int prev_corr;
> > +
> > + if (corr)
> > + *corr = 0;
> >
> > brcmnand_clear_ecc_addr(ctrl);
> >
> > for (i = 0; i < trans; i++, addr += FC_BYTES) {
> > + prev_corr = brcmnand_corr_total(ctrl);
> > brcmnand_set_cmd_addr(mtd, addr);
> > /* SPARE_AREA_READ does not use ECC, so just use
> > PAGE_READ */
> > brcmnand_send_cmd(host, CMD_PAGE_READ);
> > @@ -2099,13 +2111,16 @@ static int brcmnand_read_by_pio(struct mtd_info
> *mtd, struct nand_chip *chip,
> >
> > if (*err_addr)
> > ret = -EBADMSG;
> > - }
> > + else {
> > + *err_addr =
> > brcmnand_get_correcc_addr(ctrl);
> >
> > - if (!ret) {
> > - *err_addr = brcmnand_get_correcc_addr(ctrl);
> > + if (*err_addr) {
> > + ret = -EUCLEAN;
> >
> > - if (*err_addr)
> > - ret = -EUCLEAN;
> > + if (corr &&
> > (brcmnand_corr_total(ctrl) - prev_corr) >
> *corr)
> > + *corr =
> > brcmnand_corr_total(ctrl) - prev_corr;
> > + }
> > + }
> > }
> > }
> >
> > @@ -2173,6 +2188,8 @@ static int brcmnand_read(struct mtd_info *mtd,
> struct nand_chip *chip,
> > int err;
> > bool retry = true;
> > bool edu_err = false;
> > + unsigned int corrected = 0; /* max corrected bits per subpage */
> > + unsigned int prev_tot = brcmnand_corr_total(ctrl);
> >
> > dev_dbg(ctrl->dev, "read %llx -> %p\n", (unsigned long
> > long)addr, buf);
> >
> > @@ -2200,9 +2217,11 @@ static int brcmnand_read(struct mtd_info *mtd,
> struct nand_chip *chip,
> > memset(oob, 0x99, mtd->oobsize);
> >
> > err = brcmnand_read_by_pio(mtd, chip, addr, trans, buf,
> > - oob, &err_addr);
> > + oob, &err_addr, &corrected);
> > }
> >
> > + mtd->ecc_stats.corrected += brcmnand_corr_total(ctrl) -
> > prev_tot;
> > +
> > if (mtd_is_eccerr(err)) {
> > /*
> > * On controller version and 7.0, 7.1 , DMA read after a
> > @@ -2240,16 +2259,20 @@ static int brcmnand_read(struct mtd_info *mtd,
> struct nand_chip *chip,
> > }
> >
> > if (mtd_is_bitflip(err)) {
> > - unsigned int corrected = brcmnand_count_corrected(ctrl);
> > -
> > /* in case of EDU correctable error we read again using
> > PIO */
> > if (edu_err)
> > err = brcmnand_read_by_pio(mtd, chip, addr,
> > trans, buf,
> > - oob, &err_addr);
> > + oob, &err_addr,
> > &corrected);
> >
> > dev_dbg(ctrl->dev, "corrected error at 0x%llx\n",
> > (unsigned long long)err_addr);
> > - mtd->ecc_stats.corrected += corrected;
> > + /*
> > + * if flipped bits accumulator is not supported but we
> > detected
> > + * a correction, increase stat by 1 to match previous
> > behavior.
> > + */
> > + if (brcmnand_corr_total(ctrl) == prev_tot)
> > + mtd->ecc_stats.corrected++;
> > +
> > /* Always exceed the software-imposed threshold */
> > return max(mtd->bitflip_threshold, corrected);
> > }
> > --
> > 2.43.5
> >
> Reviewed-by: William Zhang <william.zhang@...adcom.com>
Hi Miquel,
I wonder if you get chance to take a look of this latest patch. We believe
we addressed
all your concerns with the correct test results you requested(test results
were in the v2 patch
thread). Let us know if there is anything else we need to update.
Thanks,
William
Download attachment "smime.p7s" of type "application/pkcs7-signature" (4199 bytes)
Powered by blists - more mailing lists