| 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: <20240513094217.4c1a0fc2@xps-13>
Date: Mon, 13 May 2024 09:42:17 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Sascha Hauer <s.hauer@...gutronix.de>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
<vigneshr@...com>, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] mtd: nand: mxc_nand: support software ECC
Hi Sascha,
s.hauer@...gutronix.de wrote on Wed, 08 May 2024 13:08:29 +0200:
> With these changes the driver can be used with software BCH ECC which
> is useful for NAND chips that require a stronger ECC than the i.MX
> hardware supports.
>
> The controller normally interleaves user data with OOB data when
> accessing the NAND chip. With Software BCH ECC we write the data
> to the NAND in a way that the raw data on the NAND chip matches the
> way the NAND layer sees it. This way commands like NAND_CMD_RNDOUT
> work as expected.
>
> This was tested on i.MX27 but should work on the other SoCs supported
> by this driver as well.
>
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> ---
> drivers/mtd/nand/raw/mxc_nand.c | 84 ++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 75 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index 1e8b4553e03ba..7004fd57c80f7 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -178,6 +178,7 @@ struct mxc_nand_host {
> int used_oobsize;
> int active_cs;
> unsigned int ecc_stats_v1;
> + unsigned int column;
>
> struct completion op_completion;
>
> @@ -1397,10 +1398,10 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
> chip->ecc.bytes = host->devtype_data->eccbytes;
> host->eccsize = host->devtype_data->eccsize;
> chip->ecc.size = 512;
> - mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
>
> switch (chip->ecc.engine_type) {
> case NAND_ECC_ENGINE_TYPE_ON_HOST:
> + mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
> chip->ecc.read_page = mxc_nand_read_page;
> chip->ecc.read_page_raw = mxc_nand_read_page_raw;
> chip->ecc.read_oob = mxc_nand_read_oob;
> @@ -1465,6 +1466,54 @@ static int mxcnd_setup_interface(struct nand_chip *chip, int chipnr,
> return host->devtype_data->setup_interface(chip, chipnr, conf);
> }
>
> +static void copy_page_to_sram(struct mtd_info *mtd)
> +{
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct mxc_nand_host *host = nand_get_controller_data(this);
> + void *buf = host->data_buf;
> + unsigned int n_subpages = mtd->writesize / 512;
> + int oob_per_subpage, i;
> +
> + /* mtd->writesize is not set during ident scanning */
> + if (!n_subpages)
> + n_subpages = 1;
> +
> + oob_per_subpage = (mtd->oobsize / n_subpages) & ~1;
> +
> + for (i = 0; i < n_subpages; i++) {
> + memcpy16_toio(host->main_area0 + i * 512, buf, 512);
> + buf += 512;
> +
> + memcpy16_toio(host->spare0 + i * host->devtype_data->spare_len, buf,
> + oob_per_subpage);
> + buf += oob_per_subpage;
> + }
> +}
> +
> +static void copy_page_from_sram(struct mtd_info *mtd)
> +{
> + struct nand_chip *this = mtd_to_nand(mtd);
> + struct mxc_nand_host *host = nand_get_controller_data(this);
> + void *buf = host->data_buf;
> + unsigned int n_subpages = mtd->writesize / 512;
> + int oob_per_subpage, i;
> +
> + /* mtd->writesize is not set during ident scanning */
> + if (!n_subpages)
> + n_subpages = 1;
> +
> + oob_per_subpage = (mtd->oobsize / n_subpages) & ~1;
> +
> + for (i = 0; i < n_subpages; i++) {
> + memcpy16_fromio(buf, host->main_area0 + i * 512, 512);
> + buf += 512;
> +
> + memcpy16_fromio(buf, host->spare0 + i * host->devtype_data->spare_len,
> + oob_per_subpage);
> + buf += oob_per_subpage;
> + }
> +}
> +
> static int mxcnd_exec_op(struct nand_chip *chip,
> const struct nand_operation *op,
> bool check_only)
> @@ -1496,8 +1545,11 @@ static int mxcnd_exec_op(struct nand_chip *chip,
> */
> break;
> case NAND_OP_CMD_INSTR:
> - if (instr->ctx.cmd.opcode == NAND_CMD_PAGEPROG)
> + if (instr->ctx.cmd.opcode == NAND_CMD_PAGEPROG) {
> + if (chip->ecc.engine_type != NAND_ECC_ENGINE_TYPE_ON_HOST)
> + copy_page_to_sram(mtd);
> host->devtype_data->send_page(mtd, NFC_INPUT);
> + }
Same as before: data moves should not happen here.
>
> host->devtype_data->send_cmd(host, instr->ctx.cmd.opcode, true);
>
> @@ -1506,6 +1558,8 @@ static int mxcnd_exec_op(struct nand_chip *chip,
> if (instr->ctx.cmd.opcode == NAND_CMD_STATUS)
> statusreq = true;
>
> + host->column = 0;
> +
This is risky but maybe this is the trick to get NAND_CMD_RNDOUT
working? If yes maybe it is worth separating from this patchset, if
possible, with a comment explaining what this is all about (the
controller's SRAM being some kind of 1:1 mapping with the NAND SRAM,
thus we need to write the data at the correct offset in the controller
SRAM, I guess?).
Please write the comment above when defining the column field, and
also, if my understanding is correct, can we rename it to sram_column
or something like that?
> break;
> case NAND_OP_ADDR_INSTR:
> for (j = 0; j < instr->ctx.addr.naddrs; j++) {
> @@ -1517,9 +1571,14 @@ static int mxcnd_exec_op(struct nand_chip *chip,
> buf_write = instr->ctx.data.buf.out;
> buf_len = instr->ctx.data.len;
>
> - memcpy32_toio(host->main_area0, buf_write, buf_len);
> - if (chip->oob_poi)
> - copy_spare(mtd, false, chip->oob_poi);
> + if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST) {
> + memcpy32_toio(host->main_area0, buf_write, buf_len);
> + if (chip->oob_poi)
> + copy_spare(mtd, false, chip->oob_poi);
> + } else {
> + memcpy(host->data_buf + host->column, buf_write, buf_len);
> + host->column += buf_len;
> + }
>
> break;
> case NAND_OP_DATA_IN_INSTR:
> @@ -1552,11 +1611,18 @@ static int mxcnd_exec_op(struct nand_chip *chip,
>
> host->devtype_data->read_page(chip);
>
> - if (IS_ALIGNED(buf_len, 4)) {
> - memcpy32_fromio(buf_read, host->main_area0, buf_len);
> + if (chip->ecc.engine_type == NAND_ECC_ENGINE_TYPE_ON_HOST) {
> + if (IS_ALIGNED(buf_len, 4)) {
> + memcpy32_fromio(buf_read, host->main_area0, buf_len);
> + } else {
> + memcpy32_fromio(host->data_buf, host->main_area0, mtd->writesize);
> + memcpy(buf_read, host->data_buf, buf_len);
> + }
> } else {
> - memcpy32_fromio(host->data_buf, host->main_area0, mtd->writesize);
> - memcpy(buf_read, host->data_buf, buf_len);
> + if (!host->column)
> + copy_page_from_sram(mtd);
> + memcpy(buf_read, host->data_buf + host->column, buf_len);
> + host->column += buf_len;
> }
>
> break;
>
Thanks,
Miquèl
Powered by blists - more mailing lists