From patchwork Wed May 4 11:51:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nico Huber X-Patchwork-Id: 618384 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.coreboot.org (mail.coreboot.org [80.81.252.135]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3r0Gf32SkKz9sXR for ; Wed, 4 May 2016 21:53:47 +1000 (AEST) Received: from [127.0.0.1] (helo=ra.coresystems.de) by mail.coreboot.org with esmtp (Exim 4.86_2) (envelope-from ) id 1axvLl-0006lV-KT; Wed, 04 May 2016 13:52:17 +0200 Received: from a.mx.secunet.com ([62.96.220.36]) by mail.coreboot.org with esmtps (TLSv1:DHE-RSA-AES256-SHA:256) (Exim 4.86_2) (envelope-from ) id 1axvLa-0006jX-1P for flashrom@flashrom.org; Wed, 04 May 2016 13:52:13 +0200 Received: from localhost (alg1 [127.0.0.1]) by a.mx.secunet.com (Postfix) with ESMTP id A9ECF1A065E for ; Wed, 4 May 2016 13:52:03 +0200 (CEST) X-Virus-Scanned: by secunet Received: from a.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id iIicfQzRlSER for ; Wed, 4 May 2016 13:52:02 +0200 (CEST) Received: from mail-essen-01.secunet.de (unknown [10.53.40.204]) by a.mx.secunet.com (Postfix) with ESMTP id ED1021A0674 for ; Wed, 4 May 2016 13:51:58 +0200 (CEST) Received: from schawa.localdomain (10.182.9.137) by mail-essen-01.secunet.de (10.53.40.204) with Microsoft SMTP Server (TLS) id 14.3.279.2; Wed, 4 May 2016 13:51:59 +0200 From: Nico Huber To: Date: Wed, 4 May 2016 13:51:39 +0200 Message-ID: <1462362706-31216-8-git-send-email-nico.huber@secunet.com> X-Mailer: git-send-email 2.7.0 In-Reply-To: <1462362706-31216-1-git-send-email-nico.huber@secunet.com> References: <1462362706-31216-1-git-send-email-nico.huber@secunet.com> MIME-Version: 1.0 X-Originating-IP: [10.182.9.137] X-G-Data-MailSecurity-for-Exchange-SpamLevel: 0 X-G-Data-MailSecurity-for-Exchange-SpamFilter: 0; 1; str=0001.0A0C0201.5729E25F.0265, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-G-Data-MailSecurity-for-Exchange-State: 0 X-G-Data-MailSecurity-for-Exchange-Error: 0 X-G-Data-MailSecurity-for-Exchange-Sender: 32 X-G-Data-MailSecurity-for-Exchange-Server: d65e63f7-5c15-413f-8f63-c0d707471c93 X-EXCLAIMER-MD-CONFIG: 2c86f778-e09b-4440-8b15-867914633a10 X-G-Data-MailSecurity-for-Exchange-Guid: 3CE4B04B-6344-4DCF-A5AF-459AA831951D X-Spam-Score: -1.6 (-) Subject: [flashrom] [PATCH 07/14] Kill doit() X-BeenThere: flashrom@flashrom.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: flashrom discussion and development mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: flashrom-bounces@flashrom.org Sender: "flashrom" X-Duff: Orig. Duff, Duff Lite, Duff Dry, Duff Dark, Raspberry Duff, Lady Duff, Red Duff, Tartar Control Duff No words can describe this feeling. Signed-off-by: Nico Huber --- cli_classic.c | 24 ++-- flash.h | 5 +- flashrom.c | 393 +++++++++++----------------------------------------------- 3 files changed, 90 insertions(+), 332 deletions(-) diff --git a/cli_classic.c b/cli_classic.c index ac4c6f8..d7e17f7 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -530,24 +530,28 @@ int main(int argc, char *argv[]) goto out_shutdown; } - /* Always verify write operations unless -n is used. */ - if (write_it && !dont_verify_it) - verify_it = 1; + if (layoutfile) + fl_layout_set(fill_flash, get_global_layout()); - /* Map the selected flash chip again. */ - if (map_flash(fill_flash) != 0) { - ret = 1; - goto out_shutdown; - } + fl_flag_set(fill_flash, FL_FLAG_FORCE, !!force); + fl_flag_set(fill_flash, FL_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch); + fl_flag_set(fill_flash, FL_FLAG_VERIFY_AFTER_WRITE, !dont_verify_it); + fl_flag_set(fill_flash, FL_FLAG_VERIFY_WHOLE_CHIP, true); /* FIXME: We should issue an unconditional chip reset here. This can be * done once we have a .reset function in struct flashchip. * Give the chip time to settle. */ programmer_delay(100000); - ret |= doit(fill_flash, force, filename, read_it, write_it, erase_it, verify_it); + if (read_it) + ret = do_read(fill_flash, filename); + else if (erase_it) + ret = do_erase(fill_flash); + else if (write_it) + ret = do_write(fill_flash, filename); + else if (verify_it) + ret = do_verify(fill_flash, filename); - unmap_flash(fill_flash); out_shutdown: programmer_shutdown(); out: diff --git a/flash.h b/flash.h index 3afa846..fc024ac 100644 --- a/flash.h +++ b/flash.h @@ -283,9 +283,12 @@ void print_buildinfo(void); void print_banner(void); void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); -int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename); int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); +int do_read(struct flashctx *, const char *filename); +int do_erase(struct flashctx *); +int do_write(struct flashctx *, const char *const filename); +int do_verify(struct flashctx *, const char *const filename); /* Something happened that shouldn't happen, but we can go on. */ #define ERROR_NONFATAL 0x100 diff --git a/flashrom.c b/flashrom.c index b580f0e..0bc8e6f 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1365,6 +1365,7 @@ out: #endif } +static int read_by_layout(struct flashctx *, void *); int read_flash_to_file(struct flashctx *flash, const char *filename) { unsigned long size = flash->chip->total_size * 1024; @@ -1382,7 +1383,7 @@ int read_flash_to_file(struct flashctx *flash, const char *filename) ret = 1; goto out_free; } - if (flash->chip->read(flash, buf, 0, size)) { + if (read_by_layout(flash, buf)) { msg_cerr("Read operation failed!\n"); ret = 1; goto out_free; @@ -1461,97 +1462,6 @@ static int selfcheck_eraseblocks(const struct flashchip *chip) return ret; } -static int erase_and_write_block_helper(struct flashctx *flash, - unsigned int start, unsigned int len, - uint8_t *curcontents, - uint8_t *newcontents, - int (*erasefn) (struct flashctx *flash, - unsigned int addr, - unsigned int len)) -{ - unsigned int starthere = 0, lenhere = 0; - int ret = 0, skip = 1, writecount = 0; - enum write_granularity gran = flash->chip->gran; - - /* curcontents and newcontents are opaque to walk_eraseregions, and - * need to be adjusted here to keep the impression of proper abstraction - */ - curcontents += start; - newcontents += start; - msg_cdbg(":"); - if (need_erase(curcontents, newcontents, len, gran)) { - msg_cdbg("E"); - ret = erasefn(flash, start, len); - if (ret) - return ret; - if (check_erased_range(flash, start, len)) { - msg_cerr("ERASE FAILED!\n"); - return -1; - } - /* Erase was successful. Adjust curcontents. */ - memset(curcontents, 0xff, len); - skip = 0; - } - /* get_next_write() sets starthere to a new value after the call. */ - while ((lenhere = get_next_write(curcontents + starthere, - newcontents + starthere, - len - starthere, &starthere, gran))) { - if (!writecount++) - msg_cdbg("W"); - /* Needs the partial write function signature. */ - ret = flash->chip->write(flash, newcontents + starthere, - start + starthere, lenhere); - if (ret) - return ret; - starthere += lenhere; - skip = 0; - } - if (skip) - msg_cdbg("S"); - else - all_skipped = false; - return ret; -} - -static int walk_eraseregions(struct flashctx *flash, int erasefunction, - int (*do_something) (struct flashctx *flash, - unsigned int addr, - unsigned int len, - uint8_t *param1, - uint8_t *param2, - int (*erasefn) ( - struct flashctx *flash, - unsigned int addr, - unsigned int len)), - void *param1, void *param2) -{ - int i, j; - unsigned int start = 0; - unsigned int len; - struct block_eraser eraser = flash->chip->block_erasers[erasefunction]; - - for (i = 0; i < NUM_ERASEREGIONS; i++) { - /* count==0 for all automatically initialized array - * members so the loop below won't be executed for them. - */ - len = eraser.eraseblocks[i].size; - for (j = 0; j < eraser.eraseblocks[i].count; j++) { - /* Print this for every block except the first one. */ - if (i || j) - msg_cdbg(", "); - msg_cdbg("0x%06x-0x%06x", start, - start + len - 1); - if (do_something(flash, start, len, param1, param2, - eraser.block_erase)) { - return 1; - } - start += len; - } - } - msg_cdbg("\n"); - return 0; -} - static int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; @@ -1577,71 +1487,6 @@ static int check_block_eraser(const struct flashctx *flash, int k, int log) return 0; } -int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents) -{ - int k, ret = 1; - uint8_t *curcontents; - unsigned long size = flash->chip->total_size * 1024; - unsigned int usable_erasefunctions = count_usable_erasers(flash); - - msg_cinfo("Erasing and writing flash chip... "); - curcontents = malloc(size); - if (!curcontents) { - msg_gerr("Out of memory!\n"); - exit(1); - } - /* Copy oldcontents to curcontents to avoid clobbering oldcontents. */ - memcpy(curcontents, oldcontents, size); - - for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { - if (k != 0) - msg_cinfo("Looking for another erase function.\n"); - if (!usable_erasefunctions) { - msg_cinfo("No usable erase functions left.\n"); - break; - } - msg_cdbg("Trying erase function %i... ", k); - if (check_block_eraser(flash, k, 1)) - continue; - usable_erasefunctions--; - ret = walk_eraseregions(flash, k, &erase_and_write_block_helper, - curcontents, newcontents); - /* If everything is OK, don't try another erase function. */ - if (!ret) - break; - /* Write/erase failed, so try to find out what the current chip - * contents are. If no usable erase functions remain, we can - * skip this: the next iteration will break immediately anyway. - */ - if (!usable_erasefunctions) - continue; - /* Reading the whole chip may take a while, inform the user even - * in non-verbose mode. - */ - msg_cinfo("Reading current flash chip contents... "); - if (flash->chip->read(flash, curcontents, 0, size)) { - /* Now we are truly screwed. Read failed as well. */ - msg_cerr("Can't read anymore! Aborting.\n"); - /* We have no idea about the flash chip contents, so - * retrying with another erase function is pointless. - */ - break; - } - msg_cinfo("done. "); - } - /* Free the scratchpad. */ - free(curcontents); - - if (ret) { - msg_cerr("FAILED!\n"); - } else { - if (all_skipped) - msg_cinfo("\nWarning: Chip content is identical to the requested image.\n"); - msg_cinfo("Erase/write done.\n"); - } - return ret; -} - /** @private */ static const struct fl_layout *get_layout(const struct flashctx *const flashctx, struct fl_layout_single *const fallback) @@ -2298,170 +2143,6 @@ int chip_safety_check(const struct flashctx *flash, int force, int read_it, int return 0; } -/* This function signature is horrible. We need to design a better interface, - * but right now it allows us to split off the CLI code. - * Besides that, the function itself is a textbook example of abysmal code flow. - */ -int doit(struct flashctx *flash, int force, const char *filename, int read_it, - int write_it, int erase_it, int verify_it) -{ - uint8_t *oldcontents; - uint8_t *newcontents; - int ret = 0; - unsigned long size = flash->chip->total_size * 1024; - int read_all_first = 1; /* FIXME: Make this configurable. */ - - if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) { - msg_cerr("Aborting.\n"); - return 1; - } - - if (normalize_romentries(flash)) { - msg_cerr("Requested regions can not be handled. Aborting.\n"); - return 1; - } - - /* Given the existence of read locks, we want to unlock for read, - * erase and write. - */ - if (flash->chip->unlock) - flash->chip->unlock(flash); - - if (read_it) { - return read_flash_to_file(flash, filename); - } - - oldcontents = malloc(size); - if (!oldcontents) { - msg_gerr("Out of memory!\n"); - exit(1); - } - /* Assume worst case: All bits are 0. */ - memset(oldcontents, 0x00, size); - newcontents = malloc(size); - if (!newcontents) { - msg_gerr("Out of memory!\n"); - exit(1); - } - /* Assume best case: All bits should be 1. */ - memset(newcontents, 0xff, size); - /* Side effect of the assumptions above: Default write action is erase - * because newcontents looks like a completely erased chip, and - * oldcontents being completely 0x00 means we have to erase everything - * before we can write. - */ - - if (erase_it) { - /* FIXME: Do we really want the scary warning if erase failed? - * After all, after erase the chip is either blank or partially - * blank or it has the old contents. A blank chip won't boot, - * so if the user wanted erase and reboots afterwards, the user - * knows very well that booting won't work. - */ - if (erase_and_write_flash(flash, oldcontents, newcontents)) { - emergency_help_message(); - ret = 1; - } - goto out; - } - - if (write_it || verify_it) { - if (read_buf_from_file(newcontents, size, filename)) { - ret = 1; - goto out; - } - -#if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) { - if (force_boardmismatch) { - msg_pinfo("Proceeding anyway because user forced us to.\n"); - } else { - msg_perr("Aborting. You can override this with " - "-p internal:boardmismatch=force.\n"); - ret = 1; - goto out; - } - } -#endif - } - - /* Read the whole chip to be able to check whether regions need to be - * erased and to give better diagnostics in case write fails. - * The alternative is to read only the regions which are to be - * preserved, but in that case we might perform unneeded erase which - * takes time as well. - */ - if (read_all_first) { - msg_cinfo("Reading old flash chip contents... "); - if (flash->chip->read(flash, oldcontents, 0, size)) { - ret = 1; - msg_cinfo("FAILED.\n"); - goto out; - } - } - msg_cinfo("done.\n"); - - /* Build a new image taking the given layout into account. */ - if (build_new_image(flash, read_all_first, oldcontents, newcontents)) { - msg_gerr("Could not prepare the data to be written, aborting.\n"); - ret = 1; - goto out; - } - - // //////////////////////////////////////////////////////////// - - if (write_it && erase_and_write_flash(flash, oldcontents, newcontents)) { - msg_cerr("Uh oh. Erase/write failed. "); - if (read_all_first) { - msg_cerr("Checking if anything has changed.\n"); - msg_cinfo("Reading current flash chip contents... "); - if (!flash->chip->read(flash, newcontents, 0, size)) { - msg_cinfo("done.\n"); - if (!memcmp(oldcontents, newcontents, size)) { - nonfatal_help_message(); - ret = 1; - goto out; - } - msg_cerr("Apparently at least some data has changed.\n"); - } else - msg_cerr("Can't even read anymore!\n"); - emergency_help_message(); - ret = 1; - goto out; - } else - msg_cerr("\n"); - emergency_help_message(); - ret = 1; - goto out; - } - - /* Verify only if we either did not try to write (verify operation) or actually changed something. */ - if (verify_it && (!write_it || !all_skipped)) { - msg_cinfo("Verifying flash... "); - - if (write_it) { - /* Work around chips which need some time to calm down. */ - programmer_delay(1000*1000); - ret = verify_range(flash, newcontents, 0, size); - /* If we tried to write, and verification now fails, we - * might have an emergency situation. - */ - if (ret) - emergency_help_message(); - } else { - ret = compare_range(newcontents, oldcontents, 0, size); - } - if (!ret) - msg_cinfo("VERIFIED.\n"); - } - -out: - free(oldcontents); - free(newcontents); - return ret; -} - -/** @private */ static int prepare_flash_access(struct flashctx *const flash, const bool read_it, const bool write_it, const bool erase_it, const bool verify_it) @@ -2753,3 +2434,73 @@ _free_ret: } /** @} */ /* end fl-ops */ + +int do_read(struct flashctx *const flash, const char *const filename) +{ + if (prepare_flash_access(flash, true, false, false, false)) + return 1; + + const int ret = read_flash_to_file(flash, filename); + + unmap_flash(flash); + + return ret; +} + +int do_erase(struct flashctx *const flash) +{ + const int ret = fl_flash_erase(flash); + + /* FIXME: Do we really want the scary warning if erase failed? + * After all, after erase the chip is either blank or partially + * blank or it has the old contents. A blank chip won't boot, + * so if the user wanted erase and reboots afterwards, the user + * knows very well that booting won't work. + */ + if (ret) + emergency_help_message(); + + return ret; +} + +int do_write(struct flashctx *const flash, const char *const filename) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + if (!newcontents) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + + ret = fl_image_write(flash, newcontents, flash_size); + +_free_ret: + free(newcontents); + return ret; +} + +int do_verify(struct flashctx *const flash, const char *const filename) +{ + const size_t flash_size = flash->chip->total_size * 1024; + int ret = 1; + + uint8_t *const newcontents = malloc(flash_size); + if (!newcontents) { + msg_gerr("Out of memory!\n"); + goto _free_ret; + } + + if (read_buf_from_file(newcontents, flash_size, filename)) + goto _free_ret; + + ret = fl_image_verify(flash, newcontents, flash_size); + +_free_ret: + free(newcontents); + return ret; +}