Re: [PATCH] nand: nandsim: Implement approximation mode

From: Richard Weinberger
Date: Thu May 12 2016 - 08:47:23 EST

Next message: Michal Hocko: "Re: [RFC 03/13] mm, page_alloc: don't retry initial attempt in slowpath"
Previous message: Ingo Molnar: "Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well"
In reply to: Boris Brezillon: "Re: [PATCH] nand: nandsim: Implement approximation mode"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Boris,

Am 12.05.2016 um 14:36 schrieb Boris Brezillon:
> Hi Richard,
>
> On Thu, 12 May 2016 13:32:45 +0200
> Richard Weinberger <richard@xxxxxx> wrote:
>
>> Sometimes all you need is a NAND with a given page, erase and chip size
>> to load and inspect a certain image.
>> OOB, ECC sizes and other metrics don't matter much then.
>> In such a situation I find myself often fiddling around with NAND IDs
>> to get what I need, especially when the given NAND ID from the datasheet
>> decode to something else than the NAND advertises via ONFI.
>> Using the new nandsim.approx module parameter it is possible to specify
>> the page size, number of pages per block and the total number of blocks.
>> Nandsim will then emulate a SLC NAND with 128 bytes OOB and the given
>> sizes. Nandsim achieves this by providing ONFI parameters to NAND core.
>
> The current nandsim implementation is indeed incomplete and does not
> allow for the full NAND chain emulation.
> I first considered dispatching the emulation bit in the different
> components (the NAND controller, the NAND chip and the generic/standard
> code), but that means introduction a new infrastructure and I'm now
> unsure that this is appropriate.

Yep.
But a facelifting for nandsim would not harm. ;)

> The other question we should ask ourselves is whether this NAND
> emulation layer belongs here. I mean, we could emulate a bunch of NAND
> chips and NAND controller directly in Qemu instead of trying to teach
> nandsim how to emulate advanced chips.

I like nandsim a lot because I can use it without visualization.
Especially for stress testing UBI/UBIFS nandsim on bare metal is very
useful.

>>
>> Signed-off-by: Richard Weinberger <richard@xxxxxx>
>> ---
>> drivers/mtd/nand/nand_base.c | 12 +++++----
>> drivers/mtd/nand/nandsim.c | 62 +++++++++++++++++++++++++++++++++++++++-----
>> include/linux/mtd/nand.h | 3 +++
>> 3 files changed, 66 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 557b846..25432c2 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -3208,7 +3208,7 @@ static void sanitize_string(uint8_t *s, size_t len)
>> strim(s);
>> }
>>
>> -static u16 onfi_crc16(u16 crc, u8 const *p, size_t len)
>> +u16 nand_onfi_crc16(u16 crc, u8 const *p, size_t len)
>> {
>> int i;
>> while (len--) {
>> @@ -3246,7 +3246,7 @@ static int nand_flash_detect_ext_param_page(struct mtd_info *mtd,
>>
>> /* Read out the Extended Parameter Page. */
>> chip->read_buf(mtd, (uint8_t *)ep, len);
>> - if ((onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2)
>> + if ((nand_onfi_crc16(ONFI_CRC_BASE, ((uint8_t *)ep) + 2, len - 2)
>> != le16_to_cpu(ep->crc))) {
>> pr_debug("fail in the CRC.\n");
>> goto ext_out;
>> @@ -3328,14 +3328,16 @@ static int nand_flash_detect_onfi(struct mtd_info *mtd, struct nand_chip *chip,
>> /* Try ONFI for unknown chip or LP */
>> chip->cmdfunc(mtd, NAND_CMD_READID, 0x20, -1);
>> if (chip->read_byte(mtd) != 'O' || chip->read_byte(mtd) != 'N' ||
>> - chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I')
>> + chip->read_byte(mtd) != 'F' || chip->read_byte(mtd) != 'I') {
>> +
>> return 0;
>> + }
>
> Hm, this change seems unnecessary.

Whops, there was a debug printk.

>>
>> chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
>> for (i = 0; i < 3; i++) {
>> for (j = 0; j < sizeof(*p); j++)
>> ((uint8_t *)p)[j] = chip->read_byte(mtd);
>> - if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
>> + if (nand_onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) ==
>> le16_to_cpu(p->crc)) {
>> break;
>> }
>> @@ -3442,7 +3444,7 @@ static int nand_flash_detect_jedec(struct mtd_info *mtd, struct nand_chip *chip,
>> for (j = 0; j < sizeof(*p); j++)
>> ((uint8_t *)p)[j] = chip->read_byte(mtd);
>>
>> - if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) ==
>> + if (nand_onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 510) ==
>> le16_to_cpu(p->crc))
>> break;
>> }
>> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
>> index a58169a2..b5be38a 100644
>> --- a/drivers/mtd/nand/nandsim.c
>> +++ b/drivers/mtd/nand/nandsim.c
>> @@ -114,6 +114,15 @@ static u_char id_bytes[8] = {
>> [3] = CONFIG_NANDSIM_FOURTH_ID_BYTE,
>> [4 ... 7] = 0xFF,
>> };
>> +static unsigned int approx[3];
>> +
>> +static struct nand_onfi_params id_onfi = {
>> + .sig = {'O', 'N', 'F', 'I'},
>> + .manufacturer = "nandsim ",
>> + .model = "nandsim ",
>> +};
>> +
>> +static unsigned char *id_p;
>>
>> module_param_array(id_bytes, byte, NULL, 0400);
>> module_param_named(first_id_byte, id_bytes[0], byte, 0400);
>> @@ -139,6 +148,7 @@ module_param(overridesize, uint, 0400);
>> module_param(cache_file, charp, 0400);
>> module_param(bbt, uint, 0400);
>> module_param(bch, uint, 0400);
>> +module_param_array(approx, uint, NULL, 0400);
>>
>> MODULE_PARM_DESC(id_bytes, "The ID bytes returned by NAND Flash 'read ID' command");
>> MODULE_PARM_DESC(first_id_byte, "The first byte returned by NAND Flash 'read ID' command (manufacturer ID) (obsolete)");
>> @@ -174,6 +184,10 @@ MODULE_PARM_DESC(cache_file, "File to use to cache nand pages instead of mem
>> MODULE_PARM_DESC(bbt, "0 OOB, 1 BBT with marker in OOB, 2 BBT with marker in data area");
>> MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits should "
>> "be correctable in 512-byte blocks");
>> +MODULE_PARM_DESC(approx, "Approximation mode, simulate a good enough NAND that satisfies"
>> + " page size, block size and number of blocks."
>> + " e.g. 4096,128,8192 will give you a NAND with 4KiB page size, 128 pages per"
>> + " block and 8192 blocks in total.");
>>
>> /* The largest possible page size */
>> #define NS_LARGEST_PAGE_SIZE 4096
>> @@ -229,6 +243,7 @@ MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits should "
>> #define STATE_CMD_RESET 0x0000000C /* reset */
>> #define STATE_CMD_RNDOUT 0x0000000D /* random output command */
>> #define STATE_CMD_RNDOUTSTART 0x0000000E /* random output start command */
>> +#define STATE_CMD_PARAM 0x0000000F /* param output start command */
>> #define STATE_CMD_MASK 0x0000000F /* command states mask */
>>
>> /* After an address is input, the simulator goes to one of these states */
>> @@ -245,7 +260,8 @@ MODULE_PARM_DESC(bch, "Enable BCH ecc and set how many bits should "
>> #define STATE_DATAOUT 0x00001000 /* waiting for page data output */
>> #define STATE_DATAOUT_ID 0x00002000 /* waiting for ID bytes output */
>> #define STATE_DATAOUT_STATUS 0x00003000 /* waiting for status output */
>> -#define STATE_DATAOUT_MASK 0x00007000 /* data output states mask */
>> +#define STATE_DATAOUT_ONFI 0x00007000 /* waiting for ONFI output */
>> +#define STATE_DATAOUT_MASK 0x0000F000 /* data output states mask */
>>
>> /* Previous operation is done, ready to accept new requests */
>> #define STATE_READY 0x00000000
>> @@ -307,7 +323,6 @@ struct nandsim {
>> unsigned int nbparts;
>>
>> uint busw; /* flash chip bus width (8 or 16) */
>> - u_char ids[8]; /* chip's ID bytes */
>> uint32_t options; /* chip's characteristic bits */
>> uint32_t state; /* current chip state */
>> uint32_t nxstate; /* next expected state */
>> @@ -408,6 +423,8 @@ static struct nandsim_operations {
>> {OPT_ANY, {STATE_CMD_STATUS, STATE_DATAOUT_STATUS, STATE_READY}},
>> /* Read ID */
>> {OPT_ANY, {STATE_CMD_READID, STATE_ADDR_ZERO, STATE_DATAOUT_ID, STATE_READY}},
>> + /* Read param */
>> + {OPT_ANY, {STATE_CMD_PARAM, STATE_ADDR_ZERO, STATE_DATAOUT_ONFI, STATE_READY}},
>> /* Large page devices read page */
>> {OPT_LARGEPAGE, {STATE_CMD_READ0, STATE_ADDR_PAGE, STATE_CMD_READSTART | ACTION_CPY,
>> STATE_DATAOUT, STATE_READY}},
>> @@ -1077,6 +1094,8 @@ static char *get_state_name(uint32_t state)
>> return "STATE_CMD_RNDOUT";
>> case STATE_CMD_RNDOUTSTART:
>> return "STATE_CMD_RNDOUTSTART";
>> + case STATE_CMD_PARAM:
>> + return "STATE_CMD_PARAM";
>> case STATE_ADDR_PAGE:
>> return "STATE_ADDR_PAGE";
>> case STATE_ADDR_SEC:
>> @@ -1125,6 +1144,7 @@ static int check_command(int cmd)
>> case NAND_CMD_RESET:
>> case NAND_CMD_RNDOUT:
>> case NAND_CMD_RNDOUTSTART:
>> + case NAND_CMD_PARAM:
>> return 0;
>>
>> default:
>> @@ -1164,6 +1184,8 @@ static uint32_t get_state_by_command(unsigned command)
>> return STATE_CMD_RNDOUT;
>> case NAND_CMD_RNDOUTSTART:
>> return STATE_CMD_RNDOUTSTART;
>> + case NAND_CMD_PARAM:
>> + return STATE_CMD_PARAM;
>> }
>>
>> NS_ERR("get_state_by_command: unknown command, BUG\n");
>> @@ -1856,9 +1878,17 @@ static void switch_state(struct nandsim *ns)
>> break;
>>
>> case STATE_DATAOUT_ID:
>> - ns->regs.num = ns->geom.idbytes;
>> + if (ns->regs.row == 0x20) {
>> + ns->regs.num = sizeof(id_onfi.sig);
>> + id_p = (unsigned char *)id_onfi.sig;
>> + } else {
>> + ns->regs.num = ns->geom.idbytes;
>> + id_p = (unsigned char *)id_bytes;
>> + }
>> + break;
>> + case STATE_DATAOUT_ONFI:
>> + ns->regs.num = sizeof(id_onfi);
>> break;
>> -

Another whitespace damage^^.

>> case STATE_DATAOUT_STATUS:
>> ns->regs.count = ns->regs.num = 0;
>> break;
>> @@ -1951,7 +1981,11 @@ static u_char ns_nand_read_byte(struct mtd_info *mtd)
>> break;
>> case STATE_DATAOUT_ID:
>> NS_DBG("read_byte: read ID byte %d, total = %d\n", ns->regs.count, ns->regs.num);
>> - outb = ns->ids[ns->regs.count];
>> + outb = id_p[ns->regs.count];
>> + ns->regs.count += 1;
>> + break;
>> + case STATE_DATAOUT_ONFI:
>> + outb = ((unsigned char *)&id_onfi)[ns->regs.count];
>> ns->regs.count += 1;
>> break;
>> default:
>> @@ -2289,10 +2323,26 @@ static int __init ns_init_module(void)
>> nand->geom.idbytes = 4;
>> else
>> nand->geom.idbytes = 2;
>> +
>> + if (approx[0] && approx[1] && approx[2]) {
>> + id_onfi.byte_per_page = cpu_to_le32(approx[0]);
>> + id_onfi.pages_per_block = cpu_to_le32(approx[1]);
>> + id_onfi.blocks_per_lun = cpu_to_le32(approx[2]);
>> +
>> + id_onfi.spare_bytes_per_page = cpu_to_le32(128);
>> + id_onfi.lun_count = cpu_to_le32(1);
>> + id_onfi.bits_per_cell = cpu_to_le32(1);
>> + id_onfi.revision = cpu_to_le32(1 << 5);
>> + id_onfi.crc = cpu_to_le16(nand_onfi_crc16(ONFI_CRC_BASE, (uint8_t *)&id_onfi,
>> + sizeof(id_onfi) - sizeof(id_onfi.crc)));
>> +
>> + nand->geom.idbytes = 2;
>> + id_bytes[0] = id_bytes[1] = 0xFF;
>> + }
>> +
>> nand->regs.status = NS_STATUS_OK(nand);
>> nand->nxstate = STATE_UNKNOWN;
>> nand->options |= OPT_PAGE512; /* temporary value */
>> - memcpy(nand->ids, id_bytes, sizeof(nand->ids));
>> if (bus_width == 16) {
>> nand->busw = 16;
>> chip->options |= NAND_BUSWIDTH_16;
>> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
>> index 56574ba..82f6db2 100644
>> --- a/include/linux/mtd/nand.h
>> +++ b/include/linux/mtd/nand.h
>> @@ -50,6 +50,9 @@ extern int nand_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>> /* unlocks specified locked blocks */
>> extern int nand_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len);
>>
>> +/* calculate ONFI CRC */
>> +extern u16 nand_onfi_crc16(u16 crc, u8 const *p, size_t len);
>> +
>> /* The maximum number of NAND chips in an array */
>> #define NAND_MAX_CHIPS 8
>>
>
> Just a general feedback on the whole approach. If what you really want
> is a solution to skip the chip detection and manually set the chip
> info, then you shouldn't call nand_scan_ident() and manually fill
> the mtd and nand_chip fields instead.

Okay, that would work too. I was not sure whether I'm allowed to
do so.

Thanks,
//richard

Next message: Michal Hocko: "Re: [RFC 03/13] mm, page_alloc: don't retry initial attempt in slowpath"
Previous message: Ingo Molnar: "Re: [tip:perf/core] perf/x86/intel/pt: Generate PMI in the STOP region as well"
In reply to: Boris Brezillon: "Re: [PATCH] nand: nandsim: Implement approximation mode"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]