--force may have been a good idea back when only developers were using flashrom, but over the last few months I've seen too many people who incorrectly believed that --force would solve anything.
One of the problems is that --force has multiple meanings: - Force chip read by faking probe success. - Force writing even if cbtable tells us that this is the wrong image for this board. - Force chip access even if the chip is bigger than max decode size for the flash bus. - Force erase even if erase is known bad. - Force write even if write is known bad. We should kill --force and replace it with explicit --force-erase or --force-cbtable-mismatch or similar stuff, maybe -p internal:ignore_cbtable.
First step: - Remove any suggestions to use --force for probe/read from flashrom output. - Don't talk about "success" or "Found chip" if the chip is forced.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-forced_stupid/cli_classic.c =================================================================== --- flashrom-forced_stupid/cli_classic.c (Revision 992) +++ flashrom-forced_stupid/cli_classic.c (Arbeitskopie) @@ -303,6 +303,18 @@ cli_classic_usage(argv[0]); }
+ if (chip_to_probe) { + for (flash = flashchips; flash && flash->name; flash++) + if (!strcmp(flash->name, chip_to_probe)) + break; + if (!flash || !flash->name) { + fprintf(stderr, "Error: Unknown chip specified.\n"); + exit(1); + } + /* Clean up after the check. */ + flash = NULL; + } + if (programmer_init()) { fprintf(stderr, "Error: Programmer initialization failed.\n"); exit(1); @@ -329,14 +341,10 @@ } else if (!flashes[0]) { printf("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) { - printf("If you know which flash chip you have, and if this version of flashrom\n"); - printf("supports a similar flash chip, you can try to force read your chip. Run:\n"); - printf("flashrom -f -r -c similar_supported_flash_chip filename\n"); - printf("\n"); - printf("Note: flashrom can never write when the flash chip isn't found automatically.\n"); + printf("Note: flashrom can never write if the flash chip isn't found automatically.\n"); } if (force && read_it && chip_to_probe) { - printf("Force read (-f -r -c) requested, forcing chip probe success:\n"); + printf("Force read (-f -r -c) requested, pretending the chip is there:\n"); flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe); Index: flashrom-forced_stupid/flashrom.c =================================================================== --- flashrom-forced_stupid/flashrom.c (Revision 992) +++ flashrom-forced_stupid/flashrom.c (Arbeitskopie) @@ -924,7 +924,8 @@ if (!flash || !flash->name) return NULL;
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + force ? "Assuming" : "Found", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
Thanks for the try, but the laptop shut it self down while I was asleep. And it's a brick now. Shit happens, first time I managed to kill a main-board in those 17 years I've worked with computers.
A patch that changed this output would have been better if it was already in place when I tested it:
=== This flash part has status UNTESTED for operations: PROBE READ ERASE WRITE Please email a report to flashrom@flashrom.org if any of the above operations work correctly for you with this flash part. Please include the flashrom output with the additional -V option for all operations you tested (-V, -rV, -wV, -EV) , and mention which mainboard or programmer you tested. Thanks for your help! +WARNING! DON'T DO A "flashrom -EV" ON A LAPTOP THO, SINCE IT MIGHT TURN IT INTO A BRICK! ===
Cheers
2010/4/12 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net:
--force may have been a good idea back when only developers were using flashrom, but over the last few months I've seen too many people who incorrectly believed that --force would solve anything.
One of the problems is that --force has multiple meanings:
- Force chip read by faking probe success.
- Force writing even if cbtable tells us that this is the wrong image
for this board.
- Force chip access even if the chip is bigger than max decode size for
the flash bus.
- Force erase even if erase is known bad.
- Force write even if write is known bad.
We should kill --force and replace it with explicit --force-erase or --force-cbtable-mismatch or similar stuff, maybe -p internal:ignore_cbtable.
First step:
- Remove any suggestions to use --force for probe/read from flashrom output.
- Don't talk about "success" or "Found chip" if the chip is forced.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-forced_stupid/cli_classic.c
--- flashrom-forced_stupid/cli_classic.c (Revision 992) +++ flashrom-forced_stupid/cli_classic.c (Arbeitskopie) @@ -303,6 +303,18 @@ cli_classic_usage(argv[0]); }
- if (chip_to_probe) {
- for (flash = flashchips; flash && flash->name; flash++)
- if (!strcmp(flash->name, chip_to_probe))
- break;
- if (!flash || !flash->name) {
- fprintf(stderr, "Error: Unknown chip specified.\n");
- exit(1);
- }
- /* Clean up after the check. */
- flash = NULL;
- }
if (programmer_init()) { fprintf(stderr, "Error: Programmer initialization failed.\n"); exit(1); @@ -329,14 +341,10 @@ } else if (!flashes[0]) { printf("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) {
- printf("If you know which flash chip you have, and if this version of flashrom\n");
- printf("supports a similar flash chip, you can try to force read your chip. Run:\n");
- printf("flashrom -f -r -c similar_supported_flash_chip filename\n");
- printf("\n");
- printf("Note: flashrom can never write when the flash chip isn't found automatically.\n");
- printf("Note: flashrom can never write if the flash chip isn't found automatically.\n");
} if (force && read_it && chip_to_probe) {
- printf("Force read (-f -r -c) requested, forcing chip probe success:\n");
- printf("Force read (-f -r -c) requested, pretending the chip is there:\n");
flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe); Index: flashrom-forced_stupid/flashrom.c =================================================================== --- flashrom-forced_stupid/flashrom.c (Revision 992) +++ flashrom-forced_stupid/flashrom.c (Arbeitskopie) @@ -924,7 +924,8 @@ if (!flash || !flash->name) return NULL;
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
- printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
- force ? "Assuming" : "Found",
flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
On 12.04.2010 10:56, Magnus Alm wrote:
Thanks for the try, but the laptop shut it self down while I was asleep. And it's a brick now. Shit happens, first time I managed to kill a main-board in those 17 years I've worked with computers.
You can recover with a soldering iron and an external flasher (a $10 used mainboard with parallel flash on ebay might suffice).
A patch that changed this output would have been better if it was already in place when I tested it:
+WARNING! DON'T DO A "flashrom -EV" ON A LAPTOP THO, SINCE IT MIGHT TURN IT INTO A BRICK!
A better text would be: "Running pure erase (flashrom -E) on a laptop with shared flash is guaranteed to brick it. Running write (flashrom -w) on a laptop with shared flash has a high chance of bricking it. To recover, you will have to desolder the flash chip from your mainboard. You have been warned."
The fact that pure write worked for you was just luck. Really.
Every software project with the chance of bricking hardware has such warnings, and forums and mailing lists are filled with complaints from people who ignored the warnings or acted in possibly dangerous ways and then made a typo at the wrong moment. The MPlayer developers (and their software can't even brick anything AFAICS) simply wrote that everybody who doesn't read the manual or who ignores warning/error messages will be ridiculed and flamed in public. No idea if this is still the current policy, but it worked very well for them in the past. I think that such behaviour may be very effective, but it wouldn't be in line with our friendliness principle. If warning messages are too obnoxious, distributions will patch them out (see the various distribution patches for old cdrecord out there).
This problem is fundamentally unsolvable on a technical level, and needs to be addressed (and hopefully solved) as a social problem.
No offense intended.
Regards, Carl-Daniel
2010/4/12 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net:
On 12.04.2010 10:56, Magnus Alm wrote:
Thanks for the try, but the laptop shut it self down while I was asleep. And it's a brick now. Shit happens, first time I managed to kill a main-board in those 17 years I've worked with computers.
You can recover with a soldering iron and an external flasher (a $10 used mainboard with parallel flash on ebay might suffice).
A patch that changed this output would have been better if it was already in place when I tested it:
+WARNING! DON'T DO A "flashrom -EV" ON A LAPTOP THO, SINCE IT MIGHT TURN IT INTO A BRICK!
A better text would be: "Running pure erase (flashrom -E) on a laptop with shared flash is guaranteed to brick it. Running write (flashrom -w) on a laptop with shared flash has a high chance of bricking it. To recover, you will have to desolder the flash chip from your mainboard. You have been warned."
The fact that pure write worked for you was just luck. Really.
Every software project with the chance of bricking hardware has such warnings, and forums and mailing lists are filled with complaints from people who ignored the warnings or acted in possibly dangerous ways and then made a typo at the wrong moment. The MPlayer developers (and their software can't even brick anything AFAICS) simply wrote that everybody who doesn't read the manual or who ignores warning/error messages will be ridiculed and flamed in public. No idea if this is still the current policy, but it worked very well for them in the past. I think that such behaviour may be very effective, but it wouldn't be in line with our friendliness principle. If warning messages are too obnoxious, distributions will patch them out (see the various distribution patches for old cdrecord out there).
This problem is fundamentally unsolvable on a technical level, and needs to be addressed (and hopefully solved) as a social problem.
No offense intended.
No pun taken, testing such stuff is risky and I have just myself to blame.
Writing too and reading from bios worked just fine with Micheals patch tho.
Regards, Carl-Daniel
Cheers
/Magnus
Am Montag, den 12.04.2010, 21:18 +0200 schrieb Carl-Daniel Hailfinger:
You can recover with a soldering iron and an external flasher (a $10 used mainboard with parallel flash on ebay might suffice).
Correct. Although I am afraid that he would have to solder/desolder TSOP which is the most common flash chip outline used in laptops. Desoldering TSOP is probably easier with a hot-air blower (needs at least 250°C, typical hair-dryers probably are not enough).
The fact that pure write worked for you was just luck. Really.
Huh, why that? It's the same thing the vendor tool does. Is it pure luck on that tool, too? As long as you don't reenable the EC while there is no valid code, everything *should* be fine. Of course you will brick the system if you write something into the flash that is *not* valid resumable EC code.
This problem is fundamentally unsolvable on a technical level, and needs to be addressed (and hopefully solved) as a social problem.
Still I think it might be a good idea to block -E on shared-flash systems. As shared-flash systems generally need a board-enable to stop/restart the EC, it would be no problem to set a flag from that procedure that invalid flash contents will brick the system.
Of course, for the general aspect of flashrom being able to brick systems, you are right that we definitely can not catch every corner case that would brick a system, but one could still catch the most prominent cases.
Kind regards, Michael Karcher
On 12.04.2010 23:18, Michael Karcher wrote:
Am Montag, den 12.04.2010, 21:18 +0200 schrieb Carl-Daniel Hailfinger:
You can recover with a soldering iron and an external flasher (a $10 used mainboard with parallel flash on ebay might suffice).
Correct. Although I am afraid that he would have to solder/desolder TSOP which is the most common flash chip outline used in laptops. Desoldering TSOP is probably easier with a hot-air blower (needs at least 250°C, typical hair-dryers probably are not enough).
You're the electronics expert, so I'll simply agree with you.
The fact that pure write worked for you was just luck. Really.
Huh, why that? It's the same thing the vendor tool does. Is it pure luck on that tool, too? As long as you don't reenable the EC while there is no valid code, everything *should* be fine. Of course you will brick the system if you write something into the flash that is *not* valid resumable EC code.
We erase everything, then we write everything. IIRC the vendor tool doesn't erase the EC region unless it changed. If flashrom fails in the middle of erase (after it erased the EC region), the laptop is bricked because flashrom aborts. If flashrom can erase the whole chip, but fails to write the complete EC region, the machine is bricked as well because flashrom aborts. Did I overlook anything?
This problem is fundamentally unsolvable on a technical level, and needs to be addressed (and hopefully solved) as a social problem.
Still I think it might be a good idea to block -E on shared-flash systems. As shared-flash systems generally need a board-enable to stop/restart the EC, it would be no problem to set a flag from that procedure that invalid flash contents will brick the system.
How do you handle failing erase (as part of a write)? If we quit flashrom, the registered shutdown hook will be called and the machine will be a brick afterwards. Not calling the shutdown hook in such cases is a really dangerous thing to do as well because the EC remains stopped.
Of course, for the general aspect of flashrom being able to brick systems, you are right that we definitely can not catch every corner case that would brick a system, but one could still catch the most prominent cases.
True. I just fear that such a mechanism may quickly become unwieldy.
Side note: If we know that flash chip probe kills a given EC, could we probe for it and simply abort before flash chip probe?
Regards, Carl-Daniel
--force may have been a good idea back when only developers were using flashrom, but over the last few months I've seen too many people who incorrectly believed that --force would solve anything.
One of the problems is that --force has multiple meanings: - Force chip read by faking probe success. - Force chip access even if the chip is bigger than max decode size for the flash bus. - Force erase even if erase is known bad. - Force write even if write is known bad. - Force writing even if cbtable tells us that this is the wrong image for this board.
We should kill --force and replace it with explicit --force-erase or --force-chip or similar stuff like -p internal:boardmismatch=force.
First step: - Remove any suggestions to use --force for probe/read from flashrom output. - Don't talk about "success" or "Found chip" if the chip is forced. - Add a new internal programmer parameter boardmismatch=force. This overrides any mismatch detection from cbtable/image comparisons. - Add a new internal programmer parameter laptop=force_I_want_a_brick. - Adjust the documentation for --force.
Additional changes in this patch: - Add warnings about laptops to the documentation. - Abort if a laptop is detected. Can be overridden with the programmer parameter mentioned above. - Add "Portable" to the list of DMI strings indicating laptops. - Programmer parameter reliability and consistency fixes.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-forced_stupid/flashrom.8 =================================================================== --- flashrom-forced_stupid/flashrom.8 (Revision 992) +++ flashrom-forced_stupid/flashrom.8 (Arbeitskopie) @@ -77,11 +77,7 @@ coreboot table is found. .TP .B "-f, --force" -Force write without checking whether the ROM image file is really meant -to be used on this board. -.sp -Note: This check only works while coreboot is running, and only for those -boards where the coreboot code supports it. +Force something undefined. DO NOT USE! .TP .B "-l, --layout <file>" Read ROM layout from @@ -135,7 +131,7 @@ but outputs the supported hardware in MediaWiki syntax, so that it can be easily pasted into the wiki page at http://www.flashrom.org/. .TP -.B "-p, --programmer <name>[:parameters]" +.B "-p, --programmer <name>[:parameter[,parameter[,parameter]]]" Specify the programmer device. Currently supported are: .sp .BR "* internal" " (default, for in-system flashing in the mainboard)" @@ -220,6 +216,14 @@ enable is going to fail. In any case (success or failure), please report to the flashrom mailing list, see below. .sp +If flashrom detects that the image you want to write and the current board +do not match, it will refuse to write the image unless you specify +.sp +.B "flashrom -p internal:boardmismatch=force" +.sp +Note: The board match check only works while coreboot is running, and only for +those boards where the coreboot code supports it. +.sp If your mainboard uses an ITE IT87 series Super I/O for LPC<->SPI flash bus translation, flashrom should autodetect that configuration. You can use .B "flashrom -p internal:it87spiport=portnum" @@ -315,8 +319,12 @@ .SH BUGS Please report any bugs at .BR http://www.flashrom.org/trac/flashrom/newticket "," -or on the flashrom mailing list -.RB "(" http://www.flashrom.org/mailman/listinfo/flashrom ")." +or on the flashrom mailing list at +.BR http://www.flashrom.org/mailman/listinfo/flashrom "." +.sp +Do not use flashrom on laptops! The embedded controller (EC) present in many +laptops interacts badly with any flash attempts. This is a hardware limitation +and flashrom will attempt to detect it and abort immediately for safety reasons. .SH LICENCE .B flashrom is covered by the GNU General Public License (GPL), version 2. Some files are Index: flashrom-forced_stupid/flash.h =================================================================== --- flashrom-forced_stupid/flash.h (Revision 992) +++ flashrom-forced_stupid/flash.h (Arbeitskopie) @@ -385,6 +385,7 @@ #if INTERNAL_SUPPORT == 1 extern int is_laptop; extern int force_boardenable; +extern int force_boardmismatch; void probe_superio(void); int internal_init(void); int internal_shutdown(void); Index: flashrom-forced_stupid/cli_classic.c =================================================================== --- flashrom-forced_stupid/cli_classic.c (Revision 992) +++ flashrom-forced_stupid/cli_classic.c (Arbeitskopie) @@ -248,6 +248,10 @@ switch (optarg[namelen]) { case ':': programmer_param = strdup(optarg + namelen + 1); + if (!strlen(programmer_param)) { + free(programmer_param); + programmer_param = NULL; + } break; case '\0': break; @@ -303,6 +307,18 @@ cli_classic_usage(argv[0]); }
+ if (chip_to_probe) { + for (flash = flashchips; flash && flash->name; flash++) + if (!strcmp(flash->name, chip_to_probe)) + break; + if (!flash || !flash->name) { + fprintf(stderr, "Error: Unknown chip specified.\n"); + exit(1); + } + /* Clean up after the check. */ + flash = NULL; + } + if (programmer_init()) { fprintf(stderr, "Error: Programmer initialization failed.\n"); exit(1); @@ -329,14 +345,10 @@ } else if (!flashes[0]) { printf("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) { - printf("If you know which flash chip you have, and if this version of flashrom\n"); - printf("supports a similar flash chip, you can try to force read your chip. Run:\n"); - printf("flashrom -f -r -c similar_supported_flash_chip filename\n"); - printf("\n"); - printf("Note: flashrom can never write when the flash chip isn't found automatically.\n"); + printf("Note: flashrom can never write if the flash chip isn't found automatically.\n"); } if (force && read_it && chip_to_probe) { - printf("Force read (-f -r -c) requested, forcing chip probe success:\n"); + printf("Force read (-f -r -c) requested, pretending the chip is there:\n"); flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe); Index: flashrom-forced_stupid/dmi.c =================================================================== --- flashrom-forced_stupid/dmi.c (Revision 992) +++ flashrom-forced_stupid/dmi.c (Arbeitskopie) @@ -102,7 +102,8 @@ }
chassis_type = get_dmi_string("chassis-type"); - if (chassis_type && !strcmp(chassis_type, "Notebook")) { + if (chassis_type && (!strcmp(chassis_type, "Notebook") || + !strcmp(chassis_type, "Portable"))) { printf_debug("Laptop detected via DMI\n"); is_laptop = 1; } Index: flashrom-forced_stupid/flashrom.c =================================================================== --- flashrom-forced_stupid/flashrom.c (Revision 992) +++ flashrom-forced_stupid/flashrom.c (Arbeitskopie) @@ -488,7 +488,17 @@ char *param_pos, *rest, *tmp; char *dev = NULL; int devlen; + int needlelen;
+ needlelen = strlen(needle); + if (!needlelen) { + msg_gerr("%s: empty needle! Please report a bug at " + "flashrom@flashrom.org\n", __func__); + return NULL; + } + /* No programmer parameters given. */ + if (*haystack == NULL) + return NULL; param_pos = strstr(*haystack, needle); do { if (!param_pos) @@ -924,7 +934,8 @@ if (!flash || !flash->name) return NULL;
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + force ? "Assuming" : "Found", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
Index: flashrom-forced_stupid/internal.c =================================================================== --- flashrom-forced_stupid/internal.c (Revision 992) +++ flashrom-forced_stupid/internal.c (Arbeitskopie) @@ -101,6 +101,7 @@ #if INTERNAL_SUPPORT == 1 struct superio superio = {}; int force_boardenable = 0; +int force_boardmismatch = 0;
void probe_superio(void) { @@ -117,26 +118,42 @@ int internal_init(void) { int ret = 0; + int force_laptop = 0; + char *arg;
- if (programmer_param && !strlen(programmer_param)) { - free(programmer_param); - programmer_param = NULL; + arg = extract_param(&programmer_param, "boardenable=", ",:"); + if (arg && !strcmp(arg,"force")) { + force_boardenable = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for boardenable.\n"); + } else if (arg) { + msg_perr("Unknown argument for boardenable: %s\n", arg); + exit(1); } - if (programmer_param) { - char *arg; - arg = extract_param(&programmer_param, "boardenable=", ",:"); - if (arg && !strcmp(arg,"force")) - force_boardenable = 1; - else if (arg) - msg_perr("Unknown argument for boardenable: %s\n", arg); - free(arg); + free(arg);
- if (strlen(programmer_param)) - msg_perr("Unhandled programmer parameters: %s\n", - programmer_param); - free(programmer_param); - programmer_param = NULL; + arg = extract_param(&programmer_param, "boardmismatch=", ",:"); + if (arg && !strcmp(arg,"force")) { + force_boardmismatch = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for boardmismatch.\n"); + } else if (arg) { + msg_perr("Unknown argument for boardmismatch: %s\n", arg); + exit(1); } + free(arg); + + arg = extract_param(&programmer_param, "laptop=", ",:"); + if (arg && !strcmp(arg,"force_I_want_a_brick")) { + force_laptop = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for laptop.\n"); + } else if (arg) { + msg_perr("Unknown argument for laptop: %s\n", arg); + exit(1); + } + free(arg); + get_io_perms();
/* Initialize PCI access for flash enables */ @@ -155,22 +172,35 @@ probe_superio();
/* Warn if a laptop is detected. */ - if (is_laptop) - printf("========================================================================\n" - "WARNING! You seem to be running flashrom on a laptop.\n" - "Laptops, notebooks and netbooks are difficult to support and we recommend\n" - "to use the vendor flashing utility. The embedded controller (EC) in these\n" - "machines often interacts badly with flashing.\n" - "See http://www.flashrom.org/Laptops for details.\n" - "========================================================================\n"); + if (is_laptop) { + msg_perr("========================================================================\n" + "WARNING! You seem to be running flashrom on a laptop.\n" + "Laptops, notebooks and netbooks are difficult to support and we recommend\n" + "to use the vendor flashing utility. The embedded controller (EC) in these\n" + "machines often interacts badly with flashing.\n" + "See http://www.flashrom.org/Laptops for details.\n\n" + "If flash is shared with the EC, erase is guaranteed to brick your laptop\n" + "and write may brick your laptop.\n" + "Read and probe may irritate your EC and cause fan failure, backlight\n" + "failure and sudden poweroff.\n" + "You have been warned.\n" + "========================================================================\n"); + if (force_laptop) { + msg_perr("Proceeding anyway because user specified " + "laptop=force_I_want_a_brick.\n"); + } else { + msg_perr("Aborting.\n"); + exit(1); + } + }
/* try to enable it. Failure IS an option, since not all motherboards * really need this to be done, etc., etc. */ ret = chipset_flash_enable(); if (ret == -2) { - printf("WARNING: No chipset found. Flash detection " - "will most likely fail.\n"); + msg_perr("WARNING: No chipset found. Flash detection " + "will most likely fail.\n"); }
/* Probe for IT87* LPC->SPI translation unconditionally. */ @@ -182,7 +212,7 @@ * The error code might have been a warning only. * Besides that, we don't check the board enable return code either. */ - return 0; + return 0; }
int internal_shutdown(void) Index: flashrom-forced_stupid/README =================================================================== --- flashrom-forced_stupid/README (Revision 992) +++ flashrom-forced_stupid/README (Arbeitskopie) @@ -11,6 +11,11 @@ It supports a wide range of DIP32, PLCC32, DIP8, SO8/SOIC8, TSOP32, and TSOP40 chips, which use various protocols such as LPC, FWH, parallel flash, or SPI.
+Do not use flashrom on laptops! The embedded controller (EC) present in many +laptops interacts badly with any flash attempts. + +Please make a backup of your flash chip before writing to it. + Please see the flashrom(8) manpage.
Index: flashrom-forced_stupid/layout.c =================================================================== --- flashrom-forced_stupid/layout.c (Revision 992) +++ flashrom-forced_stupid/layout.c (Arbeitskopie) @@ -111,14 +111,15 @@ !strcasecmp(mainboard_part, lb_part)) { printf_debug("This firmware image matches this mainboard.\n"); } else { - if (force) { + if (force_boardmismatch) { printf("WARNING: This firmware image does not " "seem to fit to this machine - forcing it.\n"); } else { printf("ERROR: Your firmware image (%s:%s) does not " "appear to\n be correct for the detected " - "mainboard (%s:%s)\n\nOverride with --force if you " - "are absolutely sure that you\nare using a correct " + "mainboard (%s:%s)\n\nOverride with -p internal:" + "boardmismatch=force if you are absolutely sure " + "that\nyou are using a correct " "image for this mainboard or override\nthe detected " "values with --mainboard <vendor>:<mainboard>.\n\n", mainboard_vendor, mainboard_part, lb_vendor,
Am Dienstag, den 13.04.2010, 04:16 +0200 schrieb Carl-Daniel Hailfinger:
First step:
- Remove any suggestions to use --force for probe/read from flashrom output.
Good idea, best would be to have dummy chips added. So chips that can't be probed, written or erased, but read by mmap. Like dummy512k and so on.
- Don't talk about "success" or "Found chip" if the chip is forced.
Good idea.
- Add a new internal programmer parameter boardmismatch=force. This overrides any mismatch detection from cbtable/image comparisons.
OK.
- Add a new internal programmer parameter laptop=force_I_want_a_brick.
I don't like "force_I_want_a_brick", but I don't have any better idea. Something like "warranty=void" sounds appealing on the first thought, but does not really carry the information that the laptop system is the problem.
- Adjust the documentation for --force.
Excellent. Having documentation/code drift is a serious problem.
-Force write without checking whether the ROM image file is really meant -to be used on this board. -.sp -Note: This check only works while coreboot is running, and only for those -boards where the coreboot code supports it. +Force something undefined. DO NOT USE!
Maybe you should write down here what force does. You had it in the introduction text.
+If flashrom detects that the image you want to write and the current board +do not match, it will refuse to write the image unless you specify +.sp +.B "flashrom -p internal:boardmismatch=force" +.sp +Note: The board match check only works while coreboot is running, and only for +those boards where the coreboot code supports it. +.sp
Write it the other way around, like
On systems running coreboot system with support for it, flashrom checks whether the image is meant for your board...
So that the reader knows about the limitation that this checking only works on coreboot systems before he feels safe that flashrom checks.
+Do not use flashrom on laptops! The embedded controller (EC) present in many +laptops interacts badly with any flash attempts. This is a hardware limitation +and flashrom will attempt to detect it and abort immediately for safety reasons.
I am more like "Using flashrom on laptops is dangerous and may easily make your hardware unusable!" instead of "Do not use flashrom on laptops!" I think that having people making the choice not to use flashrom on laptops because they know it's dangerous is superior to just tell them not to do it.
- if (chip_to_probe) {
for (flash = flashchips; flash && flash->name; flash++)if (!strcmp(flash->name, chip_to_probe))break;if (!flash || !flash->name) {fprintf(stderr, "Error: Unknown chip specified.\n");exit(1);}/* Clean up after the check. */flash = NULL;- }
do we need this new code block...
if (force && read_it && chip_to_probe) {
printf("Force read (-f -r -c) requested, forcing chip probe success:\n");
printf("Force read (-f -r -c) requested, pretending the chip is there:\n");
good message change.
flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe);
if we have this?
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
- printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
force ? "Assuming" : "Found",
Assuming -> Forced?
- if (is_laptop) {
msg_perr("========================================================================\n""WARNING! You seem to be running flashrom on a laptop.\n""Laptops, notebooks and netbooks are difficult to support and we recommend\n""to use the vendor flashing utility. The embedded controller (EC) in these\n""machines often interacts badly with flashing.\n""See http://www.flashrom.org/Laptops for details.\n\n""If flash is shared with the EC, erase is guaranteed to brick your laptop\n""and write may brick your laptop.\n""Read and probe may irritate your EC and cause fan failure, backlight\n""failure and sudden poweroff.\n""You have been warned.\n""========================================================================\n");if (force_laptop) {msg_perr("Proceeding anyway because user specified ""laptop=force_I_want_a_brick.\n");} else {msg_perr("Aborting.\n");exit(1);}- }
I understand your motivation of not documenting the force parameter here. But I don't like the "security by obscurity" approach in general. If we provide the forcing parameter, it should be documented somewhere. Leave the undocumented stuff to closed-source software.
No real showstoppers, so
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
On 22.04.2010 16:09, Michael Karcher wrote:
Am Dienstag, den 13.04.2010, 04:16 +0200 schrieb Carl-Daniel Hailfinger:
First step:
- Remove any suggestions to use --force for probe/read from flashrom output.
Good idea, best would be to have dummy chips added. So chips that can't be probed, written or erased, but read by mmap. Like dummy512k and so on.
Hm yes. The big problem is handling programmers which only support SPI. We can't use a mmap dummy there. I have to think about this again.
- if (chip_to_probe) {
for (flash = flashchips; flash && flash->name; flash++)if (!strcmp(flash->name, chip_to_probe))break;if (!flash || !flash->name) {fprintf(stderr, "Error: Unknown chip specified.\n");exit(1);}/* Clean up after the check. */flash = NULL;- }
do we need this new code block...
if (force && read_it && chip_to_probe) {
printf("Force read (-f -r -c) requested, forcing chip probe success:\n");
printf("Force read (-f -r -c) requested, pretending the chip is there:\n");good message change.
flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe);if we have this?
Yes. This message is only triggered if you explicitly force read, and as such the message is misplaced. I've removed the message here.
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
- printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n",
force ? "Assuming" : "Found",Assuming -> Forced?
"Assuming" implies guesswork, "Forced" seems to suggest some success.
- if (is_laptop) {
[..]
if (force_laptop) {msg_perr("Proceeding anyway because user specified ""laptop=force_I_want_a_brick.\n");} else {msg_perr("Aborting.\n");exit(1);}- }
I understand your motivation of not documenting the force parameter here. But I don't like the "security by obscurity" approach in general. If we provide the forcing parameter, it should be documented somewhere. Leave the undocumented stuff to closed-source software.
OK,
No real showstoppers, so
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thank you for the review and the ack!
Here is a reworked version which incorporates your comments (except the dummy chip stuff, I still need to come up with something good for those).
--force may have been a good idea back when only developers were using flashrom, but over the last few months I've seen too many people who incorrectly believed that --force would solve anything.
One of the problems is that --force has multiple meanings: - Force chip read by faking probe success. - Force chip access even if the chip is bigger than max decode size for the flash bus. - Force erase even if erase is known bad. - Force write even if write is known bad. - Force writing even if cbtable tells us that this is the wrong image for this board.
We should kill --force and replace it with explicit --force-erase or --force-chip or similar stuff like -p internal:boardmismatch=force.
First step: - Remove any suggestions to use --force for probe/read from flashrom output. - Don't talk about "success" or "Found chip" if the chip is forced. - Add a new internal programmer parameter boardmismatch=force. This overrides any mismatch detection from cbtable/image comparisons. - Add a new internal programmer parameter laptop=force_I_want_a_brick. - Adjust the documentation for --force. - Clean up the man page a bit whereever it talks about --force or laptops.
Additional changes in this patch: - Add warnings about laptops to the documentation. - Abort if a laptop is detected. Can be overridden with the programmer parameter mentioned above. - Add "Portable" to the list of DMI strings indicating laptops. - Check if a chip specified with -c is known to flashrom. - Programmer parameter reliability and consistency fixes. - More paranoid self-checks. - Improve documentation.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-forced_stupid/flashrom.8 =================================================================== --- flashrom-forced_stupid/flashrom.8 (Revision 994) +++ flashrom-forced_stupid/flashrom.8 (Arbeitskopie) @@ -61,7 +61,10 @@ More verbose output. .TP .B "-c, --chip" <chipname> -Probe only for specified flash ROM chip. +Probe only for specified flash ROM chip. This option takes the chip name as +printed by +.B "flashrom -L" +without the vendor name. Please note that the chip name is case sensitive. .TP .B "-m, --mainboard" <[vendor:]part> Override mainboard settings. @@ -77,11 +80,16 @@ coreboot table is found. .TP .B "-f, --force" -Force write without checking whether the ROM image file is really meant -to be used on this board. +Force one or more of the following actions: .sp -Note: This check only works while coreboot is running, and only for those -boards where the coreboot code supports it. +* Force chip read and pretend the chip is there. +.sp +* Force chip access even if the chip is bigger than max decode size for\ + the flash bus. +.sp +* Force erase even if erase is known bad. +.sp +* Force write even if write is known bad. .TP .B "-l, --layout <file>" Read ROM layout from @@ -135,7 +143,7 @@ but outputs the supported hardware in MediaWiki syntax, so that it can be easily pasted into the wiki page at http://www.flashrom.org/. .TP -.B "-p, --programmer <name>[:parameters]" +.B "-p, --programmer <name>[:parameter[,parameter[,parameter]]]" Specify the programmer device. Currently supported are: .sp .BR "* internal" " (default, for in-system flashing in the mainboard)" @@ -220,6 +228,13 @@ enable is going to fail. In any case (success or failure), please report to the flashrom mailing list, see below. .sp +On systems running coreboot, flashrom checks whether the desired image matches +your mainboard. This needs some special board ID to be present in the image. +If flashrom detects that the image you want to write and the current board +do not match, it will refuse to write the image unless you specify +.sp +.B "flashrom -p internal:boardmismatch=force" +.sp If your mainboard uses an ITE IT87 series Super I/O for LPC<->SPI flash bus translation, flashrom should autodetect that configuration. You can use .B "flashrom -p internal:it87spiport=portnum" @@ -228,6 +243,24 @@ programmer section to use a non-default port for controlling the IT87 series Super I/O. In the unlikely case flashrom doesn't detect an active IT87 LPC<->SPI bridge, you can try to force recognition by using the it87spi programmer. +.sp +Using flashrom on laptops is dangerous and may easily make your hardware +unusable (see also the BUGS section). The embedded controller (EC) in these +machines often interacts badly with flashing. http://www.flashrom.org/Laptops +has more information. If flash is shared with the EC, erase is guaranteed to +brick your laptop and write is very likely to brick your laptop. +Chip read and probe may irritate your EC and cause fan failure, backlight +failure, sudden poweroff, and other nasty effects. +flashrom will attempt to detect laptops and abort immediately for safety +reasons. +If you want to proceed anyway at your own risk, use +.sp +.B "flashrom -p internal:laptop=force_I_want_a_brick" +.sp +You have been warned. +.sp +We will not help you if you force flashing on a laptop because this is a really +dumb idea. .TP .BR "dummy " programmer An optional parameter specifies the bus types it @@ -315,8 +348,14 @@ .SH BUGS Please report any bugs at .BR http://www.flashrom.org/trac/flashrom/newticket "," -or on the flashrom mailing list -.RB "(" http://www.flashrom.org/mailman/listinfo/flashrom ")." +or on the flashrom mailing list at +.BR http://www.flashrom.org/mailman/listinfo/flashrom "." +.sp +Using flashrom on laptops is dangerous and may easily make your hardware +unusable unless you can desolder the flash chip and have a full flash chip +backup. This is caused by the embedded controller (EC) present in many laptops, +which interacts badly with any flash attempts. This is a hardware limitation +and flashrom will attempt to detect it and abort immediately for safety reasons. .SH LICENCE .B flashrom is covered by the GNU General Public License (GPL), version 2. Some files are Index: flashrom-forced_stupid/flash.h =================================================================== --- flashrom-forced_stupid/flash.h (Revision 994) +++ flashrom-forced_stupid/flash.h (Arbeitskopie) @@ -385,6 +385,7 @@ #if INTERNAL_SUPPORT == 1 extern int is_laptop; extern int force_boardenable; +extern int force_boardmismatch; void probe_superio(void); int internal_init(void); int internal_shutdown(void); Index: flashrom-forced_stupid/cli_classic.c =================================================================== --- flashrom-forced_stupid/cli_classic.c (Revision 994) +++ flashrom-forced_stupid/cli_classic.c (Arbeitskopie) @@ -248,6 +248,10 @@ switch (optarg[namelen]) { case ':': programmer_param = strdup(optarg + namelen + 1); + if (!strlen(programmer_param)) { + free(programmer_param); + programmer_param = NULL; + } break; case '\0': break; @@ -303,6 +307,21 @@ cli_classic_usage(argv[0]); }
+ if (chip_to_probe) { + for (flash = flashchips; flash && flash->name; flash++) + if (!strcmp(flash->name, chip_to_probe)) + break; + if (!flash || !flash->name) { + fprintf(stderr, "Error: Unknown chip '%s' specified.\n", + chip_to_probe); + printf("Run flashrom -L to view the hardware supported " + "in this flashrom version.\n"); + exit(1); + } + /* Clean up after the check. */ + flash = NULL; + } + if (programmer_init()) { fprintf(stderr, "Error: Programmer initialization failed.\n"); exit(1); @@ -329,18 +348,14 @@ } else if (!flashes[0]) { printf("No EEPROM/flash device found.\n"); if (!force || !chip_to_probe) { - printf("If you know which flash chip you have, and if this version of flashrom\n"); - printf("supports a similar flash chip, you can try to force read your chip. Run:\n"); - printf("flashrom -f -r -c similar_supported_flash_chip filename\n"); - printf("\n"); - printf("Note: flashrom can never write when the flash chip isn't found automatically.\n"); + printf("Note: flashrom can never write if the flash chip isn't found automatically.\n"); } if (force && read_it && chip_to_probe) { - printf("Force read (-f -r -c) requested, forcing chip probe success:\n"); + printf("Force read (-f -r -c) requested, pretending the chip is there:\n"); flashes[0] = probe_flash(flashchips, 1); if (!flashes[0]) { - printf("flashrom does not support a flash chip named '%s'.\n", chip_to_probe); - printf("Run flashrom -L to view the hardware supported in this flashrom version.\n"); + printf("Probing for flash chip '%s' failed.\n", chip_to_probe); + programmer_shutdown(); exit(1); } printf("Please note that forced reads most likely contain garbage.\n"); Index: flashrom-forced_stupid/dmi.c =================================================================== --- flashrom-forced_stupid/dmi.c (Revision 994) +++ flashrom-forced_stupid/dmi.c (Arbeitskopie) @@ -102,7 +102,8 @@ }
chassis_type = get_dmi_string("chassis-type"); - if (chassis_type && !strcmp(chassis_type, "Notebook")) { + if (chassis_type && (!strcmp(chassis_type, "Notebook") || + !strcmp(chassis_type, "Portable"))) { printf_debug("Laptop detected via DMI\n"); is_laptop = 1; } Index: flashrom-forced_stupid/flashrom.c =================================================================== --- flashrom-forced_stupid/flashrom.c (Revision 994) +++ flashrom-forced_stupid/flashrom.c (Arbeitskopie) @@ -488,7 +488,17 @@ char *param_pos, *rest, *tmp; char *dev = NULL; int devlen; + int needlelen;
+ needlelen = strlen(needle); + if (!needlelen) { + msg_gerr("%s: empty needle! Please report a bug at " + "flashrom@flashrom.org\n", __func__); + return NULL; + } + /* No programmer parameters given. */ + if (*haystack == NULL) + return NULL; param_pos = strstr(*haystack, needle); do { if (!param_pos) @@ -924,7 +934,8 @@ if (!flash || !flash->name) return NULL;
- printf("Found chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + printf("%s chip "%s %s" (%d KB, %s) at physical address 0x%lx.\n", + force ? "Assuming" : "Found", flash->vendor, flash->name, flash->total_size, flashbuses_to_text(flash->bustype), base);
Index: flashrom-forced_stupid/internal.c =================================================================== --- flashrom-forced_stupid/internal.c (Revision 994) +++ flashrom-forced_stupid/internal.c (Arbeitskopie) @@ -101,6 +101,7 @@ #if INTERNAL_SUPPORT == 1 struct superio superio = {}; int force_boardenable = 0; +int force_boardmismatch = 0;
void probe_superio(void) { @@ -117,26 +118,42 @@ int internal_init(void) { int ret = 0; + int force_laptop = 0; + char *arg;
- if (programmer_param && !strlen(programmer_param)) { - free(programmer_param); - programmer_param = NULL; + arg = extract_param(&programmer_param, "boardenable=", ",:"); + if (arg && !strcmp(arg,"force")) { + force_boardenable = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for boardenable.\n"); + } else if (arg) { + msg_perr("Unknown argument for boardenable: %s\n", arg); + exit(1); } - if (programmer_param) { - char *arg; - arg = extract_param(&programmer_param, "boardenable=", ",:"); - if (arg && !strcmp(arg,"force")) - force_boardenable = 1; - else if (arg) - msg_perr("Unknown argument for boardenable: %s\n", arg); - free(arg); + free(arg);
- if (strlen(programmer_param)) - msg_perr("Unhandled programmer parameters: %s\n", - programmer_param); - free(programmer_param); - programmer_param = NULL; + arg = extract_param(&programmer_param, "boardmismatch=", ",:"); + if (arg && !strcmp(arg,"force")) { + force_boardmismatch = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for boardmismatch.\n"); + } else if (arg) { + msg_perr("Unknown argument for boardmismatch: %s\n", arg); + exit(1); } + free(arg); + + arg = extract_param(&programmer_param, "laptop=", ",:"); + if (arg && !strcmp(arg,"force_I_want_a_brick")) { + force_laptop = 1; + } else if (arg && !strlen(arg)) { + msg_perr("Missing argument for laptop.\n"); + } else if (arg) { + msg_perr("Unknown argument for laptop: %s\n", arg); + exit(1); + } + free(arg); + get_io_perms();
/* Initialize PCI access for flash enables */ @@ -155,22 +172,35 @@ probe_superio();
/* Warn if a laptop is detected. */ - if (is_laptop) - printf("========================================================================\n" - "WARNING! You seem to be running flashrom on a laptop.\n" - "Laptops, notebooks and netbooks are difficult to support and we recommend\n" - "to use the vendor flashing utility. The embedded controller (EC) in these\n" - "machines often interacts badly with flashing.\n" - "See http://www.flashrom.org/Laptops for details.\n" - "========================================================================\n"); + if (is_laptop) { + msg_perr("========================================================================\n" + "WARNING! You seem to be running flashrom on a laptop.\n" + "Laptops, notebooks and netbooks are difficult to support and we recommend\n" + "to use the vendor flashing utility. The embedded controller (EC) in these\n" + "machines often interacts badly with flashing.\n" + "See http://www.flashrom.org/Laptops for details.\n\n" + "If flash is shared with the EC, erase is guaranteed to brick your laptop\n" + "and write may brick your laptop.\n" + "Read and probe may irritate your EC and cause fan failure, backlight\n" + "failure and sudden poweroff.\n" + "You have been warned.\n" + "========================================================================\n"); + if (force_laptop) { + msg_perr("Proceeding anyway because user specified " + "laptop=force_I_want_a_brick\n"); + } else { + msg_perr("Aborting.\n"); + exit(1); + } + }
/* try to enable it. Failure IS an option, since not all motherboards * really need this to be done, etc., etc. */ ret = chipset_flash_enable(); if (ret == -2) { - printf("WARNING: No chipset found. Flash detection " - "will most likely fail.\n"); + msg_perr("WARNING: No chipset found. Flash detection " + "will most likely fail.\n"); }
/* Probe for IT87* LPC->SPI translation unconditionally. */ @@ -182,7 +212,7 @@ * The error code might have been a warning only. * Besides that, we don't check the board enable return code either. */ - return 0; + return 0; }
int internal_shutdown(void) Index: flashrom-forced_stupid/README =================================================================== --- flashrom-forced_stupid/README (Revision 994) +++ flashrom-forced_stupid/README (Arbeitskopie) @@ -11,6 +11,12 @@ It supports a wide range of DIP32, PLCC32, DIP8, SO8/SOIC8, TSOP32, and TSOP40 chips, which use various protocols such as LPC, FWH, parallel flash, or SPI.
+Do not use flashrom on laptops! The embedded controller (EC) present in many +laptops interacts badly with any flash attempts and can brick your laptop +permanently. + +Please make a backup of your flash chip before writing to it. + Please see the flashrom(8) manpage.
Index: flashrom-forced_stupid/layout.c =================================================================== --- flashrom-forced_stupid/layout.c (Revision 994) +++ flashrom-forced_stupid/layout.c (Arbeitskopie) @@ -111,14 +111,15 @@ !strcasecmp(mainboard_part, lb_part)) { printf_debug("This firmware image matches this mainboard.\n"); } else { - if (force) { + if (force_boardmismatch) { printf("WARNING: This firmware image does not " "seem to fit to this machine - forcing it.\n"); } else { printf("ERROR: Your firmware image (%s:%s) does not " "appear to\n be correct for the detected " - "mainboard (%s:%s)\n\nOverride with --force if you " - "are absolutely sure that you\nare using a correct " + "mainboard (%s:%s)\n\nOverride with -p internal:" + "boardmismatch=force if you are absolutely sure " + "that\nyou are using a correct " "image for this mainboard or override\nthe detected " "values with --mainboard <vendor>:<mainboard>.\n\n", mainboard_vendor, mainboard_part, lb_vendor,
Am Samstag, den 24.04.2010, 11:25 +0200 schrieb Carl-Daniel Hailfinger:
Thank you for the review and the ack!
Here is a reworked version which incorporates your comments (except the dummy chip stuff, I still need to come up with something good for those).
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Regards, Michael Karcher
On 28.04.2010 17:02, Michael Karcher wrote:
Am Samstag, den 24.04.2010, 11:25 +0200 schrieb Carl-Daniel Hailfinger:
Thank you for the review and the ack!
Here is a reworked version which incorporates your comments (except the dummy chip stuff, I still need to come up with something good for those).
Acked-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Thanks for the review. Committed in r996.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Magnus Alm -
Michael Karcher