linux-kernel - RE: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports cached reads

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: <BYAPR08MB4437B83D7D50F4135D0D40A9BBC4A@BYAPR08MB4437.namprd08.prod.outlook.com>
Date:   Tue, 3 Oct 2023 11:29:33 +0000
From:   Domenico Punzo <dpunzo@...ron.com>
To:     Martin Hundebøll <martin@...nix.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
CC:     Rouven Czerwinski <r.czerwinski@...gutronix.de>,
        Måns Rullgård <mans@...sr.com>,
        Alexander Shiyan <eagle.alexander923@...il.com>,
        Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        JaimeLiao <jaimeliao.tw@...il.com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Sean Nyekjær <sean@...nix.com>,
        Bean Huo <beanhuo@...ron.com>
Subject: RE: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports
 cached reads

Micron Confidential

Hello Miquel,

Here is a short list of devices having cache read with ECC enabled.

MT29F2G08ABAGAH4, MT29F2G08ABBGAH4, MT29F2G16ABBGAH4
MT29F1G08ABAFAH4, MT29F1G08ABBFAH4, MT29F1G16ABBFAH4

Thanks.
Regards,
Domenico P.




Micron Confidential
-----Original Message-----
From: Martin Hundebøll <martin@...nix.com>
Sent: Thursday, September 28, 2023 9:20 AM
To: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Rouven Czerwinski <r.czerwinski@...gutronix.de>; Måns Rullgård <mans@...sr.com>; Alexander Shiyan <eagle.alexander923@...il.com>; Richard Weinberger <richard@....at>; Vignesh Raghavendra <vigneshr@...com>; JaimeLiao <jaimeliao.tw@...il.com>; kernel@...gutronix.de; stable@...r.kernel.org; linux-mtd@...ts.infradead.org; linux-kernel@...r.kernel.org; Sean Nyekjær <sean@...nix.com>; Domenico Punzo <dpunzo@...ron.com>; Bean Huo <beanhuo@...ron.com>
Subject: [EXT] Re: [PATCH v2] mtd: rawnand: Ensure the nand chip supports cached reads

CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you recognize the sender and were expecting this message.


Hi Miquel,

On Wed, 2023-09-27 at 17:05 +0200, Miquel Raynal wrote:
> Hi Martin,
>
> miquel.raynal@...tlin.com wrote on Tue, 26 Sep 2023 13:27:25 +0200:
>
> > Hi Martin,
> >
> > + Bean and Domenico, there is a question for you below.
> >
> > martin@...nix.com wrote on Mon, 25 Sep 2023 13:01:06 +0200:
> >
> > > Hi Rouven,
> > >
> > > On Fri, 2023-09-22 at 16:17 +0200, Rouven Czerwinski wrote:
> > > > Both the JEDEC and ONFI specification say that read cache
> > > > sequential support is an optional command. This means that we
> > > > not only need to check whether the individual controller
> > > > supports the command, we also need to check the parameter pages
> > > > for both ONFI and JEDEC NAND flashes before enabling sequential
> > > > cache reads.
> > > >
> > > > This fixes support for NAND flashes which don't support enabling
> > > > cache reads, i.e. Samsung K9F4G08U0F or Toshiba TC58NVG0S3HTA00.
> > > >
> > > > Sequential cache reads are now only available for ONFI and JEDEC
> > > > devices, if individual vendors implement this, it needs to be
> > > > enabled per vendor.
> > > >
> > > > Tested on i.MX6Q with a Samsung NAND flash chip that doesn't
> > > > support sequential reads.
> > > >
> > > > Fixes: 003fe4b9545b ("mtd: rawnand: Support for sequential cache
> > > > reads")
> > > > Cc: stable@...r.kernel.org
> > > > Signed-off-by: Rouven Czerwinski <r.czerwinski@...gutronix.de>
> > >
> > > Thanks for this. It works as expected for my Toshiba chip,
> > > obviously because it doesn't use ONFI or JEDEC.
> > >
> > > Unfortunately, my Micron chip does use ONFI, and it sets the
> > > cached-
> > > read-supported bit. It then fails when reading afterwords:
>
> I might have over reacted regarding my findings in Micron's datasheet,
> I need to know if you use the on-die ECC engine or if you use the one
> on the controller. In the former case the failure is expected. In the
> latter case, it's not.

I use the default, which seems to be the controller engine?

// Martin

> Thanks,
> Miquèl
>
> > > kernel: ONFI_OPT_CMD_READ_CACHE # debug added by me
> > > kernel: nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > > kernel: nand: Micron MT29F4G08ABAFAWP
> > > kernel: nand: 512 MiB, SLC, erase size: 256 KiB, page size: 4096,
> > > OOB
> > > size: 256
> > > kernel: nand: continued read supported # debug added by me
> > > kernel: Bad block table found at page 131008, version 0x01
> > > kernel: Bad block table found at page 130944, version 0x01
> > > kernel: 2 fixed-partitions partitions found on MTD device gpmi-
> > > nand
> > > kernel: Creating 2 MTD partitions on "gpmi-nand":
> > > kernel: 0x000000000000-0x000000800000 : "boot"
> > > kernel: 0x000000800000-0x000020000000 : "ubi"
> > > kernel: gpmi-nand 1806000.nand-controller: driver registered.
> > >
> > > ...
> > >
> > > kernel: ubi0: default fastmap pool size: 100
> > > kernel: ubi0: default fastmap WL pool size: 50
> > > kernel: ubi0: attaching mtd1
> > > kernel: ubi0: scanning is finished
> > > kernel: ubi0: attached mtd1 (name "ubi", size 504 MiB)
> > > kernel: ubi0: PEB size: 262144 bytes (256 KiB), LEB size: 253952
> > > bytes
> > > kernel: ubi0: min./max. I/O unit sizes: 4096/4096, sub-page size
> > > 4096
> > > kernel: ubi0: VID header offset: 4096 (aligned 4096), data
> > > offset: 8192
> > > kernel: ubi0: good PEBs: 2012, bad PEBs: 4, corrupted PEBs: 0
> > > kernel: ubi0: user volume: 9, internal volumes: 1, max. volumes
> > > count:
> > > 128
> > > kernel: ubi0: max/mean erase counter: 4/2, WL threshold: 4096,
> > > image sequence number: 1431497221
> > > kernel: ubi0: available PEBs: 12, total reserved PEBs: 2000, PEBs
> > > reserved for bad PEB handling: 36
> > > kernel: block ubiblock0_4: created from ubi0:4(rootfs.a)
> > > kernel: ubi0: background thread "ubi_bgt0d" started, PID 36
> > > kernel: block ubiblock0_6: created from ubi0:6(appfs.a)
> > > kernel: block ubiblock0_7: created from ubi0:7(appfs.b)
> > >
> > > ...
> > >
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > (d-sysctl)[55]: systemd-sysctl.service: Failed to set up
> > > credentials:
> > > Protocol error
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b73162:14f0]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > systemd[1]: Starting Create Static Device Nodes in /dev...
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:ed1]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:838]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6d15c:1dae]
> > > kernel: SQUASHFS error: Unable to read directory block
> > > [4b6f15e:125]
> > >
> > > I've briefly tried adding some error info the the squashfs error
> > > messages, but it looks like it's getting bad data. I.e. one
> > > failure a sanity check of `dir_count`:
> > >
> > > if (dir_count > SQUASHFS_DIR_COUNT)
> > >         goto data_error;
> > >
> > > It fails with `dir_count` being 1952803684 ...
> > >
> > > So is this a case of wrong/bad timings?
> > >
> > > Miquel:
> > > I can tell from the code, that the READCACHESEQ operations are
> > > followed by NAND_OP_WAIT_RDY(tR_max, tRR_min). From the Micron
> > > datasheet[0], it should be NAND_OP_WAIT_RDY(tRCBSY_max, tRR_min),
> > > where tRCBSY is defined to be between 3 and 25 µs.
> >
> > I found a place in the ONFI spec states taht tRCBSY_max should be
> > between 3 and tR_max, so indeed we should be fine on that regard.
> >
> > However, I asked myself whether we could have issues when crossing
> > boundaries. Block boundaries should be fine, however your device
> > does not support crossing plane boundaries, as bit 4 ("read cache
> > supported") of byte 114 ("Multi-plane operation attributes") in the
> > memory organization block of the parameter page is not set (the
> > value of the byte should be 0x0E if I get it right.
> >
> > Anyway, our main issue here does not seem related to the boundaries.
> > It does not seem to be explicitly marked anywhere else but on the
> > front
> > page:
> >         Advanced command set
> >         – Program page cache mode (4)
> >         – Read page cache mode (4)
> >         – Two-plane commands (4)
> >
> >         (4) These commands supported only with ECC disabled.
> >
> > Read page cache mode without ECC makes the feature pretty useless
> > IMHO.
> >
> > Bean, Domenico, how do we know which devices allow ECC correction
> > during sequential page reads and which don't? Is there a (vendor?)
> > bit somewhere in the parameter page for that? Do we have any way to
> > know besides a list of devices allowing that? If so, can you provide
> > one with a few IDs?
> >
> > Thanks,
> > Miquèl