[OpenWrt-Devel] [RFC/RFT] mtd resetbc: Unify "Linksys" NOR/NAND variants; Add Logging and Error Return

Jeff Kletsky lede at allycomm.com
Thu Mar 21 19:40:16 EDT 2019


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 <partition>` 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:
<https://forum.openwrt.org/t/add-support-for-linksys-ea6350-v3/18907/289>




 From aa142cb0e7b58e5c22c62be1e95325299e5acc92 Mon Sep 17 00:00:00 2001
From: Jeff Kletsky <git-commits at allycomm.com>
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 <stdio.h>
  #include <stdlib.h>
  #include <stddef.h>
@@ -29,6 +34,7 @@
  #include <string.h>
  #include <errno.h>
  #include <stdint.h>
+#include <syslog.h>

  #include <sys/ioctl.h>
  #include <mtd/mtd-user.h>
@@ -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 <jogo at openwrt.org>
- *
- * 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 <stdio.h>
-#include <stdlib.h>
-#include <stddef.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <sys/stat.h>
-#include <endian.h>
-#include <string.h>
-#include <errno.h>
-#include <stdint.h>
-
-#include <sys/ioctl.h>
-#include <mtd/mtd-user.h>
-
-#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
  }
-- 
2.11.0




_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list