From patchwork Thu Mar 21 23:40:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jeff Kletsky X-Patchwork-Id: 1060597 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=lists.openwrt.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=allycomm.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="k9jeJW1z"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44QNZt5Lwtz9sRx for ; Fri, 22 Mar 2019 10:40:54 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Subject:MIME-Version:Date:Message-ID:From:To: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=2YkaeTpBMxJeHyRRIPXEQvHpov5tsb6H6CdCgpHgD1w=; b=k9jeJW1z9aCSYI CGtnstwApQ4cRgBROWh0h5/qVxzabzh49aSIpEmQXKpLbTcD8ndPbFzcAQjN8LF7VFmBWGt0jlK1V 4D7Y/UYeszEeMFGX19O1eTjb9Bu257cEXReLDT7DdSpRUhLERMGyCmU+RDbyvrdNOcHhkT5HZPL6B +c8KQaPfsC8RiYnvS3u1olZYbgx1ZIvb17FiZR6XscnfNgDKhu1JHx1/mHP9CwiRZnd1/mOO2KM8z pAHN50Xmt5I06qz0I9GmZXejzHX3y88H3zIZNZZGeAVY5J1S/ZqApRoapxLbg95CKUhKURvG25WVU LnLvC028EkmOpvKISeeA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1h77IN-0007OX-IR; Thu, 21 Mar 2019 23:40:23 +0000 Received: from mx.allycomm.com ([138.68.30.55]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1h77IJ-0007O7-8V for openwrt-devel@lists.openwrt.org; Thu, 21 Mar 2019 23:40:21 +0000 Received: from JKLETSKY-MBP15.local (184-23-189-107.vpn.dynamic.sonic.net [184.23.189.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx.allycomm.com (Postfix) with ESMTPSA id E3CDD38F1E; Thu, 21 Mar 2019 16:40:16 -0700 (PDT) To: openwrt-devel@lists.openwrt.org From: Jeff Kletsky Message-ID: <61fe6262-c325-c1c8-ca44-560a666e95be@wagsky.com> Date: Thu, 21 Mar 2019 16:40:16 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190321_164019_315649_3702C9C4 X-CRM114-Status: GOOD ( 28.72 ) X-Spam-Score: 0.0 (/) X-Spam-Report: SpamAssassin version 3.4.2 on bombadil.infradead.org summary: Content analysis details: (0.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [138.68.30.55 listed in list.dnswl.org] Subject: [OpenWrt-Devel] [RFC/RFT] mtd resetbc: Unify "Linksys" NOR/NAND variants; Add Logging and Error Return X-BeenThere: openwrt-devel@lists.openwrt.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "openwrt-devel" Errors-To: openwrt-devel-bounces+incoming=patchwork.ozlabs.org@lists.openwrt.org In working with porting the dual-firmware, NAND-based Linksys EA8300 to OpenWrt, one of the puzzles was why it wouldn't "take" a change of boot partition reliably. After digging into it some, I found that when a previous Linksys device was ported to the ipq40xx platform, the boot-counter reset was "hard-wired" to 16 bytes, rather than the write size of the MTD partition. This was likely due to the device booting from NOR (write size of 1), rather than NAND (typical write size of 2048) as do most of the other Linksys devices. As a result, the boot-counter reset failed. Right now, the choice of action behind `mtd resetbc ` is made on a platform-wide basis. This is questionable in general, as what happens when a device with a different boot loader is ported to one of the impacted architectures in package/system/mtd/src/Makefile ? * Does the boot-counter reset function belong as an `mtd` operation? * If so, how will future devices with different needs be accommodated? Putting those questions aside, I've: * Unified the "NOR" variant with the rest of the Linksys variants * Added logging to indicate success and failure * Provided a meaningful return value for scripting * "Protected" the use of `mtd resetbc` in start-up scripts so that   failure does not end the boot sequence As this would have impact on more devices than just adding the EA8300, I'm bringing it up now, rather than hiding it away in a device-specific patch/PR. I don't have the other impacted devices. I believe that with the exception of the EA6350v3, all are NAND-based and this should be a transparent change. Testing would be greatly appreciated! * ipq40xx   * EA8350v3 (NOR boot) * ipq806x   * EA8500 * kirkwood   * audi   * viper * mvebu   * armada-385-linksys-caiman   * armada-385-linksys-cobra   * armada-385-linksys-rango   * armada-385-linksys-shelby   * armada-385-linksys-venom   * armada-xp-linksys-mamba Thanks! Jeff See also: From aa142cb0e7b58e5c22c62be1e95325299e5acc92 Mon Sep 17 00:00:00 2001 From: Jeff Kletsky Date: Sat, 16 Mar 2019 16:52:02 -0700 Subject: [PATCH] mtd: Linksys: Add logging and NOR-detection to  linksys_bootcount.c ---  package/system/mtd/src/Makefile                    |   2 +-  package/system/mtd/src/linksys_bootcount.c         | 113 ++++++++++++++++----  package/system/mtd/src/linksys_bootcount_fix.c     | 115 ---------------------  .../base-files/etc/init.d/zlinksys_recovery        |   2 +-  .../ipq806x/base-files/etc/init.d/linksys_recovery |   2 +-  .../base-files/etc/init.d/linksys_recovery         |   2 +-  .../mvebu/base-files/etc/init.d/linksys_recovery   |   2 +-  7 files changed, 100 insertions(+), 138 deletions(-)  delete mode 100644 package/system/mtd/src/linksys_bootcount_fix.c diff --git a/package/system/mtd/src/Makefile b/package/system/mtd/src/Makefile index 08a9fb295d..e469e23ef7 100644 --- a/package/system/mtd/src/Makefile +++ b/package/system/mtd/src/Makefile @@ -16,7 +16,7 @@ obj.ramips = $(obj.seama) $(obj.tpl) $(obj.wrg)  obj.mvebu = linksys_bootcount.o  obj.kirkwood = linksys_bootcount.o  obj.ipq806x = linksys_bootcount.o -obj.ipq40xx = linksys_bootcount_fix.o +obj.ipq40xx = linksys_bootcount.o  ifdef FIS_SUPPORT    obj += fis.o diff --git a/package/system/mtd/src/linksys_bootcount.c b/package/system/mtd/src/linksys_bootcount.c index 500ede4972..9fb581ee02 100644 --- a/package/system/mtd/src/linksys_bootcount.c +++ b/package/system/mtd/src/linksys_bootcount.c @@ -18,6 +18,11 @@   *   */ +/* + * Portions Copyright (c) 2019, Jeff Kletsky + * + */ +  #include  #include  #include @@ -29,6 +34,7 @@  #include  #include  #include +#include  #include  #include @@ -37,6 +43,30 @@  #define BOOTCOUNT_MAGIC    0x20110811 +/* + * EA6350v3, and potentially other NOR-boot devices, + * use an offset increment of 16 between records, + * not mtd_info_user.writesize (often 1 on NOR devices). + */ + +#define BC_OFFSET_INCREMENT_MIN 16 + + + +#define DLOG_OPEN() + +#define DLOG_ERR(...) do {                               \ +        fprintf(stderr, "ERROR: " __VA_ARGS__); fprintf(stderr, "\n"); \ +    } while (0) + +#define DLOG_NOTICE(...) do {                        \ +        fprintf(stderr, __VA_ARGS__); fprintf(stderr, "\n");    \ +    } while (0) + +#define DLOG_DEBUG(...) + + +  struct bootcounter {      uint32_t magic;      uint32_t count; @@ -50,25 +80,51 @@ int mtd_resetbc(const char *mtd)      struct mtd_info_user mtd_info;      struct bootcounter *curr = (struct bootcounter *)page;      unsigned int i; +    unsigned int bc_offset_increment;      int last_count = 0;      int num_bc;      int fd;      int ret; +    int retval = 0; + +    DLOG_OPEN();      fd = mtd_check_open(mtd);      if (ioctl(fd, MEMGETINFO, &mtd_info) < 0) { -        fprintf(stderr, "failed to get mtd info!\n"); -        return -1; +        DLOG_ERR("Unable to obtain mtd_info for given partition name."); + +        retval = -1; +        goto out; +    } + + +    /* Detect need to override increment (for EA8350v3) */ + +    if ( mtd_info.writesize < BC_OFFSET_INCREMENT_MIN ) { + +        bc_offset_increment = BC_OFFSET_INCREMENT_MIN; +        DLOG_DEBUG("Offset increment set to %i for writesize of %i", +               bc_offset_increment, mtd_info.writesize); +    } else { + +        bc_offset_increment = mtd_info.writesize;      } -    num_bc = mtd_info.size / mtd_info.writesize; +    num_bc = mtd_info.size / bc_offset_increment;      for (i = 0; i < num_bc; i++) { -        pread(fd, curr, sizeof(*curr), i * mtd_info.writesize); +        pread(fd, curr, sizeof(*curr), i * bc_offset_increment); + +        /* Existing code assumes erase is to 0xff; left as-is (2019) */ + +        if (curr->magic != BOOTCOUNT_MAGIC && +            curr->magic != 0xffffffff) { +            DLOG_ERR("Unexpected magic %08x at offset %08x; " +                 "aborting.", +                 curr->magic, i * bc_offset_increment); -        if (curr->magic != BOOTCOUNT_MAGIC && curr->magic != 0xffffffff) { -            fprintf(stderr, "unexpected magic %08x, bailing out\n", curr->magic); +            retval = -2;              goto out;          } @@ -78,38 +134,59 @@ int mtd_resetbc(const char *mtd)          last_count = curr->count;      } -    /* no need to do writes when last boot count is already 0 */ -    if (last_count == 0) + +    if (last_count == 0) {    /* bootcount is already 0 */ + +        retval = 0;          goto out; +    }      if (i == num_bc) { +        DLOG_NOTICE("Boot-count log full with %i entries; erasing " +                "(expected occasionally).", i); +          struct erase_info_user erase_info;          erase_info.start = 0;          erase_info.length = mtd_info.size; -        /* erase block */          ret = ioctl(fd, MEMERASE, &erase_info);          if (ret < 0) { -            fprintf(stderr, "failed to erase block: %i\n", ret); -            return -1; +            DLOG_ERR("Failed to erase boot-count log MTD; " +                 "ioctl() MEMERASE returned %i", ret); + +            retval = -3; +            goto out;          }          i = 0;      } -    memset(curr, 0xff, mtd_info.writesize); +    memset(curr, 0xff, bc_offset_increment);      curr->magic = BOOTCOUNT_MAGIC;      curr->count = 0;      curr->checksum = BOOTCOUNT_MAGIC; -    ret = pwrite(fd, curr, mtd_info.writesize, i * mtd_info.writesize); -    if (ret < 0) -        fprintf(stderr, "failed to write: %i\n", ret); -    sync(); +    /* Assumes bc_offset_increment is a multiple of mtd_info.writesize */ + +    ret = pwrite(fd, curr, bc_offset_increment, i * bc_offset_increment); +    if (ret < 0) { +        DLOG_ERR( "Failed to write boot-count log entry; " +              "pwrite() returned %i", ret); +        retval = -4; +        goto out; + +    } else { +        sync(); + +        DLOG_NOTICE("Boot count reset to zero."); + +        retval = 0; +        goto out; +    } +  out:      close(fd); - -    return 0; +    return retval;  } diff --git a/package/system/mtd/src/linksys_bootcount_fix.c b/package/system/mtd/src/linksys_bootcount_fix.c deleted file mode 100644 index 3fc38012fb..0000000000 --- a/package/system/mtd/src/linksys_bootcount_fix.c +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Linksys boot counter reset code for mtd - * - * Copyright (C) 2013 Jonas Gorski - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License v2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. - * - */ - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -#include "mtd.h" - -#define BOOTCOUNT_MAGIC    0x20110811 - -struct bootcounter { -    uint32_t magic; -    uint32_t count; -    uint32_t checksum; -}; - -static char page[2048]; - -int mtd_resetbc(const char *mtd) -{ -    struct mtd_info_user mtd_info; -    struct bootcounter *curr = (struct bootcounter *)page; -    unsigned int i; -    int last_count = 0; -    int num_bc; -    int fd; -    int ret; - -    fd = mtd_check_open(mtd); - -    if (ioctl(fd, MEMGETINFO, &mtd_info) < 0) { -        fprintf(stderr, "failed to get mtd info!\n"); -        return -1; -    } - -    num_bc = mtd_info.size / 16; - -    for (i = 0; i < num_bc; i++) { -        pread(fd, curr, sizeof(*curr), i * 16); - -        if (curr->magic != (BOOTCOUNT_MAGIC) && curr->magic != 0xffffffff) { -            fprintf(stderr, "unexpected magic %08x, bailing out\n", curr->magic); -            goto out; -        } - -        if (curr->magic == 0xffffffff) -            break; - -        last_count = curr->count; -    } - -    /* no need to do writes when last boot count is already 0 */ -    if (last_count == 0) -        goto out; - - -    if (i == num_bc) { -        struct erase_info_user erase_info; -        erase_info.start = 0; -        erase_info.length = mtd_info.size; - -        /* erase block */ -        ret = ioctl(fd, MEMERASE, &erase_info); -        if (ret < 0) { -            fprintf(stderr, "failed to erase block: %i\n", ret); -            return -1; -        } - -        i = 0; -    } - -    memset(curr, 0xff, 16); - -    curr->magic = BOOTCOUNT_MAGIC; -    curr->count = 0; -    curr->checksum = BOOTCOUNT_MAGIC; - -    ret = pwrite(fd, curr, 16, i * 16); -    if (ret < 0) -        fprintf(stderr, "failed to write: %i\n", ret); -    sync(); -out: -    close(fd); - -    return 0; -} diff --git a/target/linux/ipq40xx/base-files/etc/init.d/zlinksys_recovery b/target/linux/ipq40xx/base-files/etc/init.d/zlinksys_recovery index ac6533e3fe..a284a33f1b 100755 --- a/target/linux/ipq40xx/base-files/etc/init.d/zlinksys_recovery +++ b/target/linux/ipq40xx/base-files/etc/init.d/zlinksys_recovery @@ -26,7 +26,7 @@ boot() {              fi              # reset the boot counter              fw_setenv boot_count 0 -            mtd resetbc s_env +            mtd resetbc s_env || true              echo "Linksys EA6350v3: boot counter has been reset"              echo "Linksys EA6350v3: boot_part=$(fw_printenv -n boot_part)"              ;; diff --git a/target/linux/ipq806x/base-files/etc/init.d/linksys_recovery b/target/linux/ipq806x/base-files/etc/init.d/linksys_recovery index 6b4b38ec7b..564c054623 100755 --- a/target/linux/ipq806x/base-files/etc/init.d/linksys_recovery +++ b/target/linux/ipq806x/base-files/etc/init.d/linksys_recovery @@ -13,7 +13,7 @@ case $(board_name) in              fw_setenv auto_recovery yes          fi          # reset the boot counter -        mtd resetbc s_env +        mtd resetbc s_env || true          ;;  esac  } diff --git a/target/linux/kirkwood/base-files/etc/init.d/linksys_recovery b/target/linux/kirkwood/base-files/etc/init.d/linksys_recovery index 8fd2f387ab..c7d328cca1 100755 --- a/target/linux/kirkwood/base-files/etc/init.d/linksys_recovery +++ b/target/linux/kirkwood/base-files/etc/init.d/linksys_recovery @@ -13,7 +13,7 @@ case $(board_name) in              fw_setenv auto_recovery yes          fi          # reset the boot counter -        mtd resetbc s_env +        mtd resetbc s_env || true          ;;  esac  } diff --git a/target/linux/mvebu/base-files/etc/init.d/linksys_recovery b/target/linux/mvebu/base-files/etc/init.d/linksys_recovery index 520b8aac54..a5064316bd 100755 --- a/target/linux/mvebu/base-files/etc/init.d/linksys_recovery +++ b/target/linux/mvebu/base-files/etc/init.d/linksys_recovery @@ -14,7 +14,7 @@ case $(board_name) in              fw_setenv auto_recovery yes          fi          # reset the boot counter -        mtd resetbc s_env +        mtd resetbc s_env || true          ;;  esac  }