| 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: <259fd7d260dc303e50804986c62d3176ace40da0.camel@pengutronix.de>
Date: Fri, 22 Sep 2023 12:07:42 +0200
From: Rouven Czerwinski <r.czerwinski@...gutronix.de>
To: Martin Hundebøll <martin@...nix.com>,
Måns Rullgård <mans@...sr.com>,
Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
Vignesh Raghavendra <vigneshr@...com>,
JaimeLiao <jaimeliao.tw@...il.com>
Cc: kernel@...gutronix.de, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mtd: rawnand: check nand support for cache reads
On Fri, 2023-09-22 at 12:04 +0200, Martin Hundebøll wrote:
> On Fri, 2023-09-22 at 12:01 +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 implements 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 no 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")
> >
> > Signed-off-by: Rouven Czerwinski <r.czerwinski@...gutronix.de>
> > ---
> > @Martin, Måns:
> > I would appreciate if you could test this on your hardware.
> >
> > @Miguel:
> > I didn't have the time to test this on ONFI/JEDEC devices with
> > support
> > yet, I'd be fine if you hold off merging this.
> >
> > drivers/mtd/nand/raw/nand_base.c | 3 +++
> > drivers/mtd/nand/raw/nand_jedec.c | 3 +++
> > drivers/mtd/nand/raw/nand_onfi.c | 3 +++
> > include/linux/mtd/jedec.h | 3 +++
> > include/linux/mtd/onfi.h | 1 +
> > include/linux/mtd/rawnand.h | 1 +
> > 6 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_base.c
> > b/drivers/mtd/nand/raw/nand_base.c
> > index d4b55155aeae..1fcac403cee6 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -5110,6 +5110,9 @@ static void
> > rawnand_check_cont_read_support(struct nand_chip *chip)
> > {
> > struct mtd_info *mtd = nand_to_mtd(chip);
> >
> > + if (!chip->parameters.supports_read_cache)
> > + return;
> > +
> > if (chip->read_retries)
> > return;
> >
> > diff --git a/drivers/mtd/nand/raw/nand_jedec.c
> > b/drivers/mtd/nand/raw/nand_jedec.c
> > index 836757717660..e6ecbc4b2493 100644
> > --- a/drivers/mtd/nand/raw/nand_jedec.c
> > +++ b/drivers/mtd/nand/raw/nand_jedec.c
> > @@ -94,6 +94,9 @@ int nand_jedec_detect(struct nand_chip *chip)
> > goto free_jedec_param_page;
> > }
> >
> > + if (p->opt_cmd[0] & JEDEC_OPT_CMD_READ_CACHE)
> > + chip->parameters.supports_read_cache;
>
> Missing ` = true` here ?
>
> > +
> > memorg->pagesize = le32_to_cpu(p->byte_per_page);
> > mtd->writesize = memorg->pagesize;
> >
> > diff --git a/drivers/mtd/nand/raw/nand_onfi.c
> > b/drivers/mtd/nand/raw/nand_onfi.c
> > index f15ef90aec8c..bf79bf2b5866 100644
> > --- a/drivers/mtd/nand/raw/nand_onfi.c
> > +++ b/drivers/mtd/nand/raw/nand_onfi.c
> > @@ -303,6 +303,9 @@ int nand_onfi_detect(struct nand_chip *chip)
> > ONFI_FEATURE_ADDR_TIMING_MODE, 1);
> > }
> >
> > + if (le16_to_cpu(p->opt_cmd) & ONFI_OPT_CMD_READ_CACHE)
> > + chip->parameters.supports_read_cache;
>
> And here?
>
> // Martin
Argh, totally. This should still fix your device, but cause performance
regressions on devices supporting cached sequential reads.
Unfortunately I don't have hardware to test this.
Fixed locally, I'll send a v2 later.
> > +
> > onfi = kzalloc(sizeof(*onfi), GFP_KERNEL);
> > if (!onfi) {
> > ret = -ENOMEM;
> > diff --git a/include/linux/mtd/jedec.h b/include/linux/mtd/jedec.h
> > index 0b6b59f7cfbd..56047a4e54c9 100644
> > --- a/include/linux/mtd/jedec.h
> > +++ b/include/linux/mtd/jedec.h
> > @@ -21,6 +21,9 @@ struct jedec_ecc_info {
> > /* JEDEC features */
> > #define JEDEC_FEATURE_16_BIT_BUS (1 << 0)
> >
> > +/* JEDEC Optional Commands */
> > +#define JEDEC_OPT_CMD_READ_CACHE BIT(1)
> > +
> > struct nand_jedec_params {
> > /* rev info and features block */
> > /* 'J' 'E' 'S' 'D' */
> > diff --git a/include/linux/mtd/onfi.h b/include/linux/mtd/onfi.h
> > index a7376f9beddf..55ab2e4d62f9 100644
> > --- a/include/linux/mtd/onfi.h
> > +++ b/include/linux/mtd/onfi.h
> > @@ -55,6 +55,7 @@
> > #define ONFI_SUBFEATURE_PARAM_LEN 4
> >
> > /* ONFI optional commands SET/GET FEATURES supported? */
> > +#define ONFI_OPT_CMD_READ_CACHE BIT(1)
> > #define ONFI_OPT_CMD_SET_GET_FEATURES BIT(2)
> >
> > struct nand_onfi_params {
> > diff --git a/include/linux/mtd/rawnand.h
> > b/include/linux/mtd/rawnand.h
> > index 90a141ba2a5a..766856fcaba8 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -233,6 +233,7 @@ struct nand_parameters {
> > /* Generic parameters */
> > const char *model;
> > bool supports_set_get_features;
> > + bool supports_read_cache;
> > DECLARE_BITMAP(set_feature_list, ONFI_FEATURE_NUMBER);
> > DECLARE_BITMAP(get_feature_list, ONFI_FEATURE_NUMBER);
> >
>
>
Powered by blists - more mailing lists