| 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: <20170409183301.037d3f95@bbrezillon>
Date: Sun, 9 Apr 2017 18:33:01 +0200
From: Boris Brezillon <boris.brezillon@...e-electrons.com>
To: Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc: linux-mtd@...ts.infradead.org, Enrico Jorns <ejo@...gutronix.de>,
Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>,
Dinh Nguyen <dinguyen@...nel.org>,
Marek Vasut <marek.vasut@...il.com>,
Graham Moore <grmoore@...nsource.altera.com>,
David Woodhouse <dwmw2@...radead.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Chuanxiao Dong <chuanxiao.dong@...el.com>,
Jassi Brar <jaswinder.singh@...aro.org>,
devicetree@...r.kernel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Brian Norris <computersforpeace@...il.com>,
Richard Weinberger <richard@....at>,
Cyrille Pitchen <cyrille.pitchen@...el.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v3 14/37] mtd: nand: denali: support "nand-ecc-strength"
DT property
On Mon, 3 Apr 2017 12:16:34 +0900
Masahiro Yamada <yamada.masahiro@...ionext.com> wrote:
> Hi Boris,
>
>
>
> 2017-03-31 18:46 GMT+09:00 Boris Brezillon <boris.brezillon@...e-electrons.com>:
>
> > You can try something like that when no explicit ecc.strength and
> > ecc.size has been set in the DT and when ECC_MAXIMIZE was not passed.
> >
> > static int
> > denali_get_closest_ecc_strength(struct denali_nand_info *denali,
> > int strength)
> > {
> > /*
> > * Whatever you need to select a strength that is greater than
> > * or equal to strength.
> > */
> >
> > return X;
> > }
>
>
> Is here anything specific to Denali?
Well, only the denali driver knows what the hardware supports, though
having a generic function that takes a table of supported strengths
would work.
>
>
> > static int denali_try_to_match_ecc_req(struct denali_nand_info *denali)
> > {
> > struct nand_chip *chip = &denali->nand;
> > struct mtd_info *mtd = nand_to_mtd(chip);
> > int max_ecc_bytes = mtd->oobsize - denali->bbtskipbytes;
> > int ecc_steps, ecc_strength, ecc_bytes;
> > int ecc_size = chip->ecc_step_ds;
> > int ecc_strength = chip->ecc_strength_ds;
> >
> > /*
> > * No information provided by the NAND chip, let the core
> > * maximize the strength.
> > */
> > if (!ecc_size || !ecc_strength)
> > return -ENOTSUPP;
> >
> > if (ecc_size > 512)
> > ecc_size = 1024;
> > else
> > ecc_size = 512;
> >
> > /* Adjust ECC step size based on hardware support. */
> > if (ecc_size == 1024 &&
> > !(denali->caps & DENALI_CAP_ECC_SIZE_1024))
> > ecc_size = 512;
> > else if(ecc_size == 512 &&
> > !(denali->caps & DENALI_CAP_ECC_SIZE_512))
> > ecc_size = 1024;
> >
> > if (ecc_size < chip->ecc_size_ds) {
> > /*
> > * When the selected size if smaller than the expected
> > * one we try to use the same strength but on 512 blocks
> > * so that we can still fix the same number of errors
> > * even if they are concentrated in the first 512bytes
> > * of a 1024bytes portion.
> > */
> > ecc_strength = chip->ecc_strength_ds;
> > ecc_strength = denali_get_closest_ecc_strength(denali,
> > ecc_strength);
> > } else {
> > /* Always prefer 1024bytes ECC blocks when possible. */
> > if (ecc_size != 1024 &&
> > (denali->caps & DENALI_CAP_ECC_SIZE_1024) &&
> > mtd->writesize > 1024)
> > ecc_size = 1024;
> >
> > /*
> > * Adjust the strength based on the selected ECC step
> > * size.
> > */
> > ecc_strength = DIV_ROUND_UP(ecc_size,
> > chip->ecc_step_ds) *
> > chip->ecc_strength_ds;
> > }
> >
> > ecc_bytes = denali_calc_ecc_bytes(ecc_size,
> > ecc_strength);
> > ecc_bytes *= mtd->writesize / ecc_size;
> >
> > /*
> > * If we don't have enough space, let the core maximize
> > * the strength.
> > */
> > if (ecc_bytes > max_ecc_bytes)
> > return -ENOTSUPP;
> >
> > chip->ecc.strength = ecc_strength;
> > chip->ecc.size = ecc_size;
> >
> > return 0;
> > }
>
>
> As a whole, this does not seem to driver-specific.
It's almost controller-agnostic, except for the denali_calc_ecc_bytes()
function, but I guess we could ask drivers to implement a hook that is
passed the ECC step size and strength and returns the associated
number of ECC bytes.
>
>
> [1] A driver provides some pairs of (ecc_strength, ecc_size)
> it can support.
>
> [2] The core framework knows the chip's requirement
> (ecc_strength_ds, ecc_size_ds).
>
>
> Then, the core framework provides a function
> to return a most recommended (ecc_strength, ecc_size).
>
>
>
> struct nand_ecc_spec {
> int ecc_strength;
> int ecc_size;
> };
>
> /*
> * This function choose the most recommented (ecc_str, ecc_size)
> * "recommended" means: minimum ecc stregth that meets
> * the chip's requirment.
> *
> *
> * @chip - nand_chip
> * @controller_ecc_spec - Array of (ecc_str, ecc_size) supported by the
> controller. (terminated by NULL as sentinel)
> */
> struct nand_ecc_spec * nand_try_to_match_ecc_req(struct nand_chip *chip,
> struct nand_ecc_spec
> *controller_ecc_spec)
> {
> /*
> * Return the pointer to the most recommended
> * struct nand_ecc_spec.
> * If nothing suitable found, return NULL.
> */
> }
>
I like the idea, except I would do this slightly differently to avoid
declaring all combinations of stepsize and strengths
struct nand_ecc_stepsize_info {
int stepsize;
int nstrengths;
int *strengths;
};
struct nand_ecc_engine_caps {
int nstepsizes;
struct nand_ecc_stepsize_info *stepsizes;
int (*calc_ecc_bytes)(int stepsize, int strength);
};
int nand_try_to_match_ecc_req(struct nand_chip *chip,
const struct nand_ecc_engine_caps *caps,
struct nand_ecc_spec *spec)
{
/*
* Find the most appropriate setting based on the ECC engine
* caps and fill the spec object accordingly.
* Returns 0 in case of success and a negative error code
* otherwise.
*/
}
Note that nand_try_to_match_ecc_req() has to be more generic than
denali_try_to_match_ecc_req() WRT step sizes, which will probably
complexify the logic.
Powered by blists - more mailing lists