Date: Fri, 26 Jul 2013 15:14:07 -0500 From: Brooks Davis <brooks@freebsd.org> To: hackers@freebsd.org Cc: marcel@freebsd.org, imp@freebsd.org Subject: CFR: improvments to cfi(4) flash driver Message-ID: <20130726201407.GF13659@lor.one-eyed-alien.net>
next in thread | raw e-mail | index | archive | help
--Pql/uPZNXIm1JCle Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable I've made a number of improvements to the write performance of the CFI flash driver. The patch below merges the remaining changes. I've only implemented buffered write on Intel flash as that's all I currently have access to. I'm looking for review and/or testing of these changes. http://people.freebsd.org/~brooks/patches/cfi.diff -- Brooks MFP4 217312, 222008, 222052, 222053, 222673, 231484, 231491 Rework the timeout code to use actual time rather than a DELAY() loop and to use both typical and maximum to allow logging of timeout failures. Also correct the erase timeout, it is specified in milliseconds not microseconds like the other timeouts. Do not invoke DELAY() between status queries as this adds significant latency which in turn reduced write performance substantially. Implement support for buffered writes (only enabled on Intel/Sharp parts for now). This yields an order of magnitude speedup on the 64MB Intel StrataFlash parts we use. When making a copy of the block to modify, also keep a clean copy around until we are ready to commit the block and use it to avoid unnecessary erases. In the non-buffer write case, also use it to avoid unnecessary writes when the block has not been erased. This yields a significant speedup when doing things like zeroing a block. Sponsored by: DARPA, AFRL diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_bus_nexus.c ./cfi_bus_n= exus.c --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_bus_nexus.c 2013-04-29 20:40= :02.000000000 +0100 +++ ./cfi_bus_nexus.c 2013-07-26 20:36:34.000000000 +0100 @@ -4,6 +4,11 @@ * Copyright (c) 2009 Sam Leffler, Errno Consulting * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_core.c ./cfi_core.c --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_core.c 2013-06-03 15:41:18.0= 00000000 +0100 +++ ./cfi_core.c 2013-07-26 20:40:32.000000000 +0100 @@ -1,7 +1,13 @@ /*- * Copyright (c) 2007, Juniper Networks, Inc. + * Copyright (c) 2012-2013, SRI International * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: @@ -279,11 +285,31 @@ sc->sc_tag =3D rman_get_bustag(sc->sc_res); sc->sc_handle =3D rman_get_bushandle(sc->sc_res); =20 - /* Get time-out values for erase and write. */ - sc->sc_write_timeout =3D 1 << cfi_read_qry(sc, CFI_QRY_TTO_WRITE); - sc->sc_erase_timeout =3D 1 << cfi_read_qry(sc, CFI_QRY_TTO_ERASE); - sc->sc_write_timeout *=3D 1 << cfi_read_qry(sc, CFI_QRY_MTO_WRITE); - sc->sc_erase_timeout *=3D 1 << cfi_read_qry(sc, CFI_QRY_MTO_ERASE); + /* Get time-out values for erase, write, and buffer write. */ + sc->sc_typical_timeouts[CFI_TIMEOUT_ERASE] =3D + SBT_1MS * (1 << cfi_read_qry(sc, CFI_QRY_TTO_ERASE)); + sc->sc_max_timeouts[CFI_TIMEOUT_ERASE] =3D + sc->sc_typical_timeouts[CFI_TIMEOUT_ERASE] * + (1 << cfi_read_qry(sc, CFI_QRY_MTO_ERASE)); + + sc->sc_typical_timeouts[CFI_TIMEOUT_WRITE] =3D + SBT_1US * (1 << cfi_read_qry(sc, CFI_QRY_TTO_WRITE)); + sc->sc_max_timeouts[CFI_TIMEOUT_WRITE] =3D + sc->sc_typical_timeouts[CFI_TIMEOUT_WRITE] * + (1 << cfi_read_qry(sc, CFI_QRY_MTO_WRITE)); + + sc->sc_typical_timeouts[CFI_TIMEOUT_BUFWRITE] =3D + SBT_1US * (1 << cfi_read_qry(sc, CFI_QRY_TTO_BUFWRITE)); + sc->sc_max_timeouts[CFI_TIMEOUT_BUFWRITE] =3D + sc->sc_typical_timeouts[CFI_TIMEOUT_BUFWRITE] * + (1 << cfi_read_qry(sc, CFI_QRY_MTO_BUFWRITE)); + + /* Get the maximum size of a multibyte program */ + if (sc->sc_typical_timeouts[CFI_TIMEOUT_BUFWRITE] !=3D 0) + sc->sc_maxbuf =3D 1 << (cfi_read_qry(sc, CFI_QRY_MAXBUF) | + cfi_read_qry(sc, CFI_QRY_MAXBUF) << 8); + else + sc->sc_maxbuf =3D 0; =20 /* Get erase regions. */ sc->sc_regions =3D cfi_read_qry(sc, CFI_QRY_NREGIONS); @@ -351,18 +377,20 @@ } =20 static int -cfi_wait_ready(struct cfi_softc *sc, u_int ofs, u_int timeout) +cfi_wait_ready(struct cfi_softc *sc, u_int ofs, sbintime_t start, + enum cfi_wait_cmd cmd) { - int done, error; + int done, error, tto_exceeded; uint32_t st0 =3D 0, st =3D 0; + sbintime_t mend, now, tend; + + tend =3D start + sc->sc_typical_timeouts[cmd]; + mend =3D start + sc->sc_max_timeouts[cmd]; =20 done =3D 0; error =3D 0; - timeout *=3D 10; - while (!done && !error && timeout) { - DELAY(100); - timeout--; - + tto_exceeded =3D 0; + while (!done && !error) { switch (sc->sc_cmdset) { case CFI_VEND_INTEL_ECS: case CFI_VEND_INTEL_SCS: @@ -390,7 +418,25 @@ done =3D ((st & 0x40) =3D=3D (st0 & 0x40)) ? 1 : 0; break; } + + now =3D sbinuptime(); + if (tto_exceeded || now > tend) { + if (!tto_exceeded) + tto_exceeded =3D 1; + if (now > mend) { +#ifdef CFI_DEBUG_TIMEOUT + device_printf(sc->sc_dev, + "max timeout exceeded (cmd %d)", cmd); +#endif + break; + } + } } +#ifdef CFI_DEBUG_TIMEOUT + if (tto_exceeded) + device_printf(sc->sc_dev, + "typical timeout exceeded (cmd %d)", cmd); +#endif if (!done && !error) error =3D ETIMEDOUT; if (error) @@ -405,9 +451,12 @@ uint8_t *x8; uint16_t *x16; uint32_t *x32; - } ptr; + } ptr, cpyprt; register_t intr; - int error, i; + int error, i, neederase =3D 0; + uint32_t st; + u_int wlen; + sbintime_t start; =20 /* Intel flash must be unlocked before modification */ switch (sc->sc_cmdset) { @@ -419,31 +468,124 @@ break; } =20 - /* Erase the block. */ - switch (sc->sc_cmdset) { - case CFI_VEND_INTEL_ECS: - case CFI_VEND_INTEL_SCS: - cfi_write(sc, sc->sc_wrofs, CFI_BCS_BLOCK_ERASE); - cfi_write(sc, sc->sc_wrofs, CFI_BCS_CONFIRM); - break; - case CFI_VEND_AMD_SCS: - case CFI_VEND_AMD_ECS: - cfi_amd_write(sc, sc->sc_wrofs, AMD_ADDR_START, - CFI_AMD_ERASE_SECTOR); - cfi_amd_write(sc, sc->sc_wrofs, 0, CFI_AMD_BLOCK_ERASE); - break; - default: - /* Better safe than sorry... */ - return (ENODEV); - } - error =3D cfi_wait_ready(sc, sc->sc_wrofs, sc->sc_erase_timeout); - if (error) - goto out; + /* Check if an erase is required. */ + for (i =3D 0; i < sc->sc_wrbufsz; i++) + if ((sc->sc_wrbuf[i] & sc->sc_wrbufcpy[i]) !=3D sc->sc_wrbuf[i]) { + neederase =3D 1; + break; + } + + if (neederase) { + intr =3D intr_disable(); + start =3D sbinuptime(); + /* Erase the block. */ + switch (sc->sc_cmdset) { + case CFI_VEND_INTEL_ECS: + case CFI_VEND_INTEL_SCS: + cfi_write(sc, sc->sc_wrofs, CFI_BCS_BLOCK_ERASE); + cfi_write(sc, sc->sc_wrofs, CFI_BCS_CONFIRM); + break; + case CFI_VEND_AMD_SCS: + case CFI_VEND_AMD_ECS: + cfi_amd_write(sc, sc->sc_wrofs, AMD_ADDR_START, + CFI_AMD_ERASE_SECTOR); + cfi_amd_write(sc, sc->sc_wrofs, 0, CFI_AMD_BLOCK_ERASE); + break; + default: + /* Better safe than sorry... */ + intr_restore(intr); + return (ENODEV); + } + intr_restore(intr); + error =3D cfi_wait_ready(sc, sc->sc_wrofs, start,=20 + CFI_TIMEOUT_ERASE); + if (error) + goto out; + } else + error =3D 0; =20 - /* Write the block. */ + /* Write the block using a multibyte write if supported. */ ptr.x8 =3D sc->sc_wrbuf; + cpyprt.x8 =3D sc->sc_wrbufcpy; + if (sc->sc_maxbuf > sc->sc_width) { + switch (sc->sc_cmdset) { + case CFI_VEND_INTEL_ECS: + case CFI_VEND_INTEL_SCS: + for (i =3D 0; i < sc->sc_wrbufsz; i +=3D wlen) { + wlen =3D MIN(sc->sc_maxbuf, sc->sc_wrbufsz - i); + + intr =3D intr_disable(); + + start =3D sbinuptime(); + do { + cfi_write(sc, sc->sc_wrofs + i, + CFI_BCS_BUF_PROG_SETUP); + if (sbinuptime() > start + sc->sc_max_timeouts[CFI_TIMEOUT_BUFWRITE])= { + error =3D ETIMEDOUT; + goto out; + } + st =3D cfi_read(sc, sc->sc_wrofs + i); + } while (! (st & CFI_INTEL_STATUS_WSMS)); + + cfi_write(sc, sc->sc_wrofs + i, + (wlen / sc->sc_width) - 1); + switch (sc->sc_width) { + case 1: + bus_space_write_region_1(sc->sc_tag, + sc->sc_handle, sc->sc_wrofs + i, + ptr.x8 + i, wlen); + break; + case 2: + bus_space_write_region_2(sc->sc_tag, + sc->sc_handle, sc->sc_wrofs + i, + ptr.x16 + i / 2, wlen / 2); + break; + case 4: + bus_space_write_region_4(sc->sc_tag, + sc->sc_handle, sc->sc_wrofs + i, + ptr.x32 + i / 4, wlen / 4); + break; + } + + cfi_write(sc, sc->sc_wrofs + i, + CFI_BCS_CONFIRM); + + intr_restore(intr); + + error =3D cfi_wait_ready(sc, sc->sc_wrofs + i, + start, CFI_TIMEOUT_BUFWRITE); + if (error !=3D 0) + goto out; + } + goto out; + default: + /* Fall through to single word case */ + break; + } + + } + + /* Write the block one byte/word at a time. */ for (i =3D 0; i < sc->sc_wrbufsz; i +=3D sc->sc_width) { =20 + /* Avoid writing unless we are actually changing bits */ + if (!neederase) { + switch (sc->sc_width) { + case 1: + if(*(ptr.x8 + i) =3D=3D *(cpyprt.x8 + i)) + continue; + break; + case 2: + if(*(ptr.x16 + i / 2) =3D=3D *(cpyprt.x16 + i / 2)) + continue; + break; + case 4: + if(*(ptr.x32 + i / 4) =3D=3D *(cpyprt.x32 + i / 4)) + continue; + break; + } + } + /* * Make sure the command to start a write and the * actual write happens back-to-back without any @@ -451,6 +593,7 @@ */ intr =3D intr_disable(); =20 + start =3D sbinuptime(); switch (sc->sc_cmdset) { case CFI_VEND_INTEL_ECS: case CFI_VEND_INTEL_SCS: @@ -464,21 +607,22 @@ switch (sc->sc_width) { case 1: bus_space_write_1(sc->sc_tag, sc->sc_handle, - sc->sc_wrofs + i, *(ptr.x8)++); + sc->sc_wrofs + i, *(ptr.x8 + i)); break; case 2: bus_space_write_2(sc->sc_tag, sc->sc_handle, - sc->sc_wrofs + i, *(ptr.x16)++); + sc->sc_wrofs + i, *(ptr.x16 + i / 2)); break; case 4: bus_space_write_4(sc->sc_tag, sc->sc_handle, - sc->sc_wrofs + i, *(ptr.x32)++); + sc->sc_wrofs + i, *(ptr.x32 + i / 4)); break; } - + =09 intr_restore(intr); =20 - error =3D cfi_wait_ready(sc, sc->sc_wrofs, sc->sc_write_timeout); + error =3D cfi_wait_ready(sc, sc->sc_wrofs, start, + CFI_TIMEOUT_WRITE); if (error) goto out; } @@ -576,6 +720,7 @@ #ifdef CFI_ARMEDANDDANGEROUS register_t intr; int i, error; + sbintime_t start; #endif =20 if (sc->sc_cmdset !=3D CFI_VEND_INTEL_ECS) @@ -585,11 +730,12 @@ #ifdef CFI_ARMEDANDDANGEROUS for (i =3D 7; i >=3D 4; i--, id >>=3D 16) { intr =3D intr_disable(); + start =3D sbinuptime(); cfi_write(sc, 0, CFI_INTEL_PP_SETUP); cfi_put16(sc, CFI_INTEL_PR(i), id&0xffff); intr_restore(intr); - error =3D cfi_wait_ready(sc, CFI_BCS_READ_STATUS, - sc->sc_write_timeout); + error =3D cfi_wait_ready(sc, CFI_BCS_READ_STATUS, start, + CFI_TIMEOUT_WRITE); if (error) break; } @@ -629,6 +775,7 @@ #ifdef CFI_ARMEDANDDANGEROUS register_t intr; int error; + sbintime_t start; #endif if (sc->sc_cmdset !=3D CFI_VEND_INTEL_ECS) return EOPNOTSUPP; @@ -638,10 +785,12 @@ /* worthy of console msg */ device_printf(sc->sc_dev, "set PLR\n"); intr =3D intr_disable(); + binuptime(&start); cfi_write(sc, 0, CFI_INTEL_PP_SETUP); cfi_put16(sc, CFI_INTEL_PLR, 0xFFFD); intr_restore(intr); - error =3D cfi_wait_ready(sc, CFI_BCS_READ_STATUS, sc->sc_write_timeout); + error =3D cfi_wait_ready(sc, CFI_BCS_READ_STATUS, start, + CFI_TIMEOUT_WRITE); cfi_write(sc, 0, CFI_BCS_READ_ARRAY); return error; #else diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_dev.c ./cfi_dev.c --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_dev.c 2012-11-11 22:15:34.00= 0000000 +0000 +++ ./cfi_dev.c 2013-07-26 20:36:34.000000000 +0100 @@ -1,7 +1,13 @@ /*- * Copyright (c) 2007, Juniper Networks, Inc. + * Copyright (c) 2012-2013, SRI International * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: @@ -72,7 +78,8 @@ * Begin writing into a new block/sector. We read the sector into * memory and keep updating that, until we move into another sector * or the process stops writing. At that time we write the whole - * sector to flash (see cfi_block_finish). + * sector to flash (see cfi_block_finish). To avoid unneeded erase + * cycles, keep a pristine copy of the sector on hand. */ int cfi_block_start(struct cfi_softc *sc, u_int ofs) @@ -116,6 +123,8 @@ break; } } + sc->sc_wrbufcpy =3D malloc(sc->sc_wrbufsz, M_TEMP, M_WAITOK); + memcpy(sc->sc_wrbufcpy, sc->sc_wrbuf, sc->sc_wrbufsz); sc->sc_writing =3D 1; return (0); } @@ -131,6 +140,7 @@ =20 error =3D cfi_write_block(sc); free(sc->sc_wrbuf, M_TEMP); + free(sc->sc_wrbufcpy, M_TEMP); sc->sc_wrbuf =3D NULL; sc->sc_wrbufsz =3D 0; sc->sc_wrofs =3D 0; diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_disk.c ./cfi_disk.c --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_disk.c 2013-06-18 19:57:42.0= 00000000 +0100 +++ ./cfi_disk.c 2013-07-26 20:36:34.000000000 +0100 @@ -1,7 +1,13 @@ /*- * Copyright (c) 2009 Sam Leffler, Errno Consulting + * Copyright (c) 2012-2013, SRI International * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_reg.h ./cfi_reg.h --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_reg.h 2013-06-03 15:41:18.00= 0000000 +0100 +++ ./cfi_reg.h 2013-07-26 20:36:34.000000000 +0100 @@ -1,7 +1,13 @@ /*- * Copyright (c) 2007, Juniper Networks, Inc. + * Copyright (c) 2012-2013, SRI International * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: @@ -44,8 +50,8 @@ u_char max_vcc; u_char min_vpp; u_char max_vpp; - u_char tto_byte_write; /* 2**n milliseconds. */ - u_char tto_buf_write; /* 2**n milliseconds. */ + u_char tto_byte_write; /* 2**n microseconds. */ + u_char tto_buf_write; /* 2**n microseconds. */ u_char tto_block_erase; /* 2**n milliseconds. */ u_char tto_chip_erase; /* 2**n milliseconds. */ u_char mto_byte_write; /* 2**n times typical t/o. */ @@ -70,12 +76,15 @@ #define CFI_QRY_VEND offsetof(struct cfi_qry, pri_vend) =20 #define CFI_QRY_TTO_WRITE offsetof(struct cfi_qry, tto_byte_write) +#define CFI_QRY_TTO_BUFWRITE offsetof(struct cfi_qry, tto_buf_write) #define CFI_QRY_TTO_ERASE offsetof(struct cfi_qry, tto_block_erase) #define CFI_QRY_MTO_WRITE offsetof(struct cfi_qry, mto_byte_write) +#define CFI_QRY_MTO_BUFWRITE offsetof(struct cfi_qry, mto_buf_write) #define CFI_QRY_MTO_ERASE offsetof(struct cfi_qry, mto_block_erase) =20 #define CFI_QRY_SIZE offsetof(struct cfi_qry, size) #define CFI_QRY_IFACE offsetof(struct cfi_qry, iface) +#define CFI_QRY_MAXBUF offsetof(struct cfi_qry, max_buf_write_size) #define CFI_QRY_NREGIONS offsetof(struct cfi_qry, nregions) #define CFI_QRY_REGION0 offsetof(struct cfi_qry, region) #define CFI_QRY_REGION(x) (CFI_QRY_REGION0 + (x) * 4) @@ -102,6 +111,7 @@ #define CFI_BCS_ERASE_SUSPEND 0xb0 #define CFI_BCS_ERASE_RESUME 0xd0 /* Equals CONFIRM */ #define CFI_BCS_CONFIRM 0xd0 +#define CFI_BCS_BUF_PROG_SETUP 0xe8 #define CFI_BCS_READ_ARRAY 0xff =20 /* Intel commands. */ diff -ru /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_var.h ./cfi_var.h --- /home/bed22/p4/freebsd/src/sys/dev/cfi/cfi_var.h 2012-11-11 22:15:34.00= 0000000 +0000 +++ ./cfi_var.h 2013-07-26 20:36:34.000000000 +0100 @@ -1,7 +1,13 @@ /*- * Copyright (c) 2007, Juniper Networks, Inc. + * Copyright (c) 2012-2013, SRI International * All rights reserved. * + * Portions of this software were developed by SRI International and the + * University of Cambridge Computer Laboratory under DARPA/AFRL contract + * (FA8750-10-C-0237) ("CTSRD"), as part of the DARPA CRASH research + * programme. + * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions * are met: @@ -32,6 +38,12 @@ #ifndef _DEV_CFI_VAR_H_ #define _DEV_CFI_VAR_H_ =20 +enum cfi_wait_cmd { + CFI_TIMEOUT_ERASE, + CFI_TIMEOUT_WRITE, + CFI_TIMEOUT_BUFWRITE +}; + struct cfi_region { u_int r_blocks; u_int r_blksz; @@ -51,13 +63,16 @@ struct cfi_region *sc_region; /* Array of region info. */ =20 u_int sc_cmdset; - u_int sc_erase_timeout; - u_int sc_write_timeout; + sbintime_t sc_typical_timeouts[3]; + sbintime_t sc_max_timeouts[3]; + + u_int sc_maxbuf; =20 struct cdev *sc_nod; struct proc *sc_opened; /* Process that has us opened. */ =20 u_char *sc_wrbuf; + u_char *sc_wrbufcpy; u_int sc_wrbufsz; u_int sc_wrofs; u_int sc_writing; --Pql/uPZNXIm1JCle Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iD8DBQFR8tiOXY6L6fI4GtQRAoPbAJ9VrfpXnRKjnV8tnfJi7fU6q5im5ACfSIuY xoYMK9+xfA/KnHNyaCwS6hY= =APjN -----END PGP SIGNATURE----- --Pql/uPZNXIm1JCle--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130726201407.GF13659>