eboot: .RODATA, upstream uzlib, move CRC, save 112 bytes (esp8266#7844)

RODATA can be copied automatically by the bootrom, so no reason not to allow its use for strings and constants in eboot.c Revert to pfalcon's original uzlib since the single patch to remove RODATA is not required. Rationalize eboot.ld linker script, clean up BSS and init it in code. Saves 112 bytes of space in the bootloader sector by removing the extra code associated with literal loads. * Move CRC out of bootload sector We added protection to only erase the bootload sector when flashing an image when the new sector != the old sector. This was intended to minimize the chance of bricking (i.e. if there was a powerfail during flashing of the boot sector the chip would be dead). Unfortunately, by placing the CRC inside the eboot sector *every* application will have a unique eboot sector (due to the crc/len), so this protection doesn't work. Move the CRC into the first 8 bytes of IROM itself. This frees up extra space in the boot sector and ensures that eboot won't be reflashed unless there really is an eboot change.
nabeel-halo · Feb 8, 2021 · 07b4c09 · 07b4c09
1 parent 6c564c2
commit 07b4c09
Show file tree

Hide file tree

Showing 9 changed files with 67 additions and 108 deletions.
diff --git a/.gitmodules b/.gitmodules
@@ -24,4 +24,4 @@
 	url = https://github.com/arduino-libraries/Ethernet.git
 [submodule "tools/sdk/uzlib"]
 	path = tools/sdk/uzlib
-	url = https://github.com/earlephilhower/uzlib.git
+	url = https://github.com/pfalcon/uzlib.git
diff --git a/bootloaders/eboot/Makefile b/bootloaders/eboot/Makefile
@@ -40,17 +40,17 @@ APP_FW  := eboot.bin
 
 all: $(APP_OUT)
 
-tinflate.o: $(UZLIB_PATH)/tinflate.c $(UZLIB_PATH)/uzlib.h $(UZLIB_PATH)/uzlib_conf.h
+tinflate.o: $(UZLIB_PATH)/tinflate.c $(UZLIB_PATH)/uzlib.h $(UZLIB_PATH)/uzlib_conf.h Makefile
 	$(CC) $(CFLAGS) -c -o tinflate.o $(UZLIB_PATH)/tinflate.c
 
-tinfgzip.o: $(UZLIB_PATH)/tinfgzip.c $(UZLIB_PATH)/uzlib.h $(UZLIB_PATH)/uzlib_conf.h
+tinfgzip.o: $(UZLIB_PATH)/tinfgzip.c $(UZLIB_PATH)/uzlib.h $(UZLIB_PATH)/uzlib_conf.h Makefile
 	$(CC) $(CFLAGS) -c -o tinfgzip.o $(UZLIB_PATH)/tinfgzip.c
 
-$(APP_AR): $(TARGET_OBJ_PATHS) tinflate.o tinfgzip.o
+$(APP_AR): $(TARGET_OBJ_PATHS) tinflate.o tinfgzip.o Makefile
 	$(AR) cru $@ $^
 
 $(APP_OUT): $(APP_AR) eboot.ld | Makefile
-	$(LD) $(LD_SCRIPT) $(LDFLAGS) -Wl,--start-group -Wl,--whole-archive $(APP_AR) -Wl,--end-group -o $@
+	$(LD) $(LD_SCRIPT) $(LDFLAGS) -Wl,--start-group -Wl,--sort-common $(APP_AR) -Wl,--end-group -o $@
 
 clean:
 	rm -f *.o

diff --git a/bootloaders/eboot/eboot.c b/bootloaders/eboot/eboot.c
@@ -14,28 +14,19 @@
 #include "eboot_command.h"
 #include <uzlib.h>
 
-extern unsigned char _gzip_dict;
 
 #define SWRST do { (*((volatile uint32_t*) 0x60000700)) |= 0x80000000; } while(0);
 
 extern void ets_wdt_enable(void);
 extern void ets_wdt_disable(void);
 
-// Converts bit of a string into a uint32
-#define S(a,b,c,d) ( (((uint32_t)a) & 0xff) | (((uint32_t)b) << 8) | (((uint32_t)c) << 16) | (((uint32_t)d)<<24) )
-
 int print_version(const uint32_t flash_addr)
 {
     uint32_t ver;
     if (SPIRead(flash_addr + APP_START_OFFSET + sizeof(image_header_t) + sizeof(section_header_t), &ver, sizeof(ver))) {
         return 1;
     }
-    // We don't have BSS and can't print from flash, so build up string
-    // 4 chars at a time.  Smaller code than byte-wise assignment.
-    uint32_t fmt[2];
-    fmt[0] = S('v', '%', '0', '8');
-    fmt[1] = S('x', '\n', 0, 0);
-    ets_printf((const char*) fmt, ver);
+    ets_printf("v%08x\n", ver);
     return 0;
 }
 
@@ -222,6 +213,16 @@ int main()
     bool clear_cmd = false;
     struct eboot_command cmd;
 
+// BSS init commented out for now to save space.  If any static variables set
+// to 0 are used, need to uncomment it or else the BSS will not be cleared and
+// the static vars will power on with random values.
+#if 0
+    // Clear BSS ourselves, we don't have handy C runtime
+    extern char _bss_start;
+    extern char _bss_end;
+    ets_bzero(&_bss_start, &_bss_end - &_bss_start);
+#endif
+
     print_version(0);
 
     if (eboot_command_read(&cmd) == 0) {
@@ -236,32 +237,26 @@ int main()
     }
 
     if (cmd.action == ACTION_COPY_RAW) {
-        uint32_t cp = S('c', 'p', ':', 0);
-        ets_printf((const char *)&cp);
+        ets_printf("cp:");
 
         ets_wdt_disable();
         res = copy_raw(cmd.args[0], cmd.args[1], cmd.args[2], false);
         ets_wdt_enable();
 
-        cp = S('0' + res, '\n', 0, 0 );
-        ets_printf((const char *)&cp);
+        ets_printf("%d\n", res);
 #if 0
 	//devyte: this verify step below (cmp:) only works when the end of copy operation above does not overwrite the 
 	//beginning of the image in the empty area, see #7458. Disabling for now. 
         //TODO: replace the below verify with hash type, crc, or similar.
         // Verify the copy
-        uint32_t v[2];
-        v[0] = S('c', 'm', 'p', ':');
-        v[1] = 0;
-        ets_printf(const char *)v);
+        ets_printf("cmp:");
         if (res == 0) {
             ets_wdt_disable();
             res = copy_raw(cmd.args[0], cmd.args[1], cmd.args[2], true);
             ets_wdt_enable();
             }
 
-        cp = S('0' + res, '\n', 0, 0 );
-        ets_printf((const char *)&cp);
+        ets_printf("%d\n", res);
 #endif	    
         if (res == 0) {
             cmd.action = ACTION_LOAD_APP;
@@ -274,13 +269,10 @@ int main()
     }
 
     if (cmd.action == ACTION_LOAD_APP) {
-        ets_putc('l'); ets_putc('d'); ets_putc('\n');
+        ets_printf("ld\n");
         res = load_app_from_flash_raw(cmd.args[0]);
         // We will get to this only on load fail
-        uint32_t e[2];
-        e[0] = S('e', ':', '0' + res, '\n' );
-        e[1] = 0;
-        ets_printf((const char*)e);
+        ets_printf("e:%d\n", res);
     }
 
     if (res) {

diff --git a/bootloaders/eboot/eboot.elf b/bootloaders/eboot/eboot.elf
diff --git a/bootloaders/eboot/eboot.ld b/bootloaders/eboot/eboot.ld
@@ -42,53 +42,13 @@ PROVIDE(_memmap_cacheattr_reset = _memmap_cacheattr_wb_trapnull);
 SECTIONS
 {
 
-  .dport0.rodata : ALIGN(4)
-  {
-    _dport0_rodata_start = ABSOLUTE(.);
-    *(.dport0.rodata)
-    *(.dport.rodata)
-    _dport0_rodata_end = ABSOLUTE(.);
-  } >dport0_0_seg :dport0_0_phdr
-
-  .dport0.literal : ALIGN(4)
-  {
-    _dport0_literal_start = ABSOLUTE(.);
-    *(.dport0.literal)
-    *(.dport.literal)
-    _dport0_literal_end = ABSOLUTE(.);
-  } >dport0_0_seg :dport0_0_phdr
-
-  .dport0.data : ALIGN(4)
-  {
-    _dport0_data_start = ABSOLUTE(.);
-    *(.dport0.data)
-    *(.dport.data)
-    _dport0_data_end = ABSOLUTE(.);
-  } >dport0_0_seg :dport0_0_phdr
-
-  .data : ALIGN(4)
+  .globals : ALIGN(4)
   {
     *(COMMON)  /* Global vars */
-    . = ALIGN(4);
-    _heap_start = ABSOLUTE(.);
-/*    _stack_sentry = ALIGN(0x8); */
   } >dram0_0_seg :dram0_0_bss_phdr
-/* __stack = 0x3ffc8000; */
 
-  .text : ALIGN(4)
+  .data : ALIGN(4)
   {
-    _stext = .;
-    _text_start = ABSOLUTE(.);
-    *(.entry.text)
-    *(.init.literal)
-    *(.init)
-    *(.literal .text .literal.* .text.* .stub .gnu.warning .gnu.linkonce.literal.* .gnu.linkonce.t.*.literal .gnu.linkonce.t.*)
-    *(.fini.literal)
-    *(.fini)
-    *(.gnu.version)
-    _text_end = ABSOLUTE(.);
-    _etext = .;
-    . = ALIGN (8);
     _data_start = ABSOLUTE(.);
     *(.data)
     *(.data.*)
@@ -102,7 +62,10 @@ SECTIONS
     *(.gnu.linkonce.s2.*)
     *(.jcr)
     _data_end = ABSOLUTE(.);
-  . = ALIGN (8);
+  } >dram0_0_seg :dram0_0_bss_phdr
+
+  .rodata : ALIGN(4)
+  {
     _rodata_start = ABSOLUTE(.);
     *(.rodata)
     *(.rodata.*)
@@ -131,14 +94,11 @@ SECTIONS
     *(.xt_except_desc_end)
     *(.dynamic)
     *(.gnu.version_d)
-    . = ALIGN(4);   /* this table MUST be 4-byte aligned */
-    _bss_table_start = ABSOLUTE(.);
-    LONG(_bss_start)
-    LONG(_bss_end)
-    _bss_table_end = ABSOLUTE(.);
     _rodata_end = ABSOLUTE(.);
+  } >dram0_0_seg :dram0_0_bss_phdr
 
-    . = ALIGN (8);
+  .bss : ALIGN(4)
+  {
     _bss_start = ABSOLUTE(.);
     *(.dynsbss)
     *(.sbss)
@@ -152,26 +112,24 @@ SECTIONS
     *(.bss)
     *(.bss.*)
     *(.gnu.linkonce.b.*)
-    . = ALIGN (8);
     _bss_end = ABSOLUTE(.);
-    _free_space = 4096 - 17 - (. - _stext);
-/*
-The boot loader checksum must be before the CRC, which is written by elf2bin.py.
-This leaves 16 bytes after the checksum for the CRC placed at the end of the
-4096-byte sector. */
-    _cs_here = (ALIGN((. + 1), 16) == ALIGN(16)) ? (ALIGN(16) - 1) : (. + 0x0F);
+  } >dram0_0_seg :dram0_0_bss_phdr
 
-/*
-The filling (padding) and values for _crc_size and _crc_val are handled by
-elf2bin.py. With this, we give values to the symbols without explicitly
-assigning space. This avoids the linkers back *fill* operation that causes
-trouble.
 
-The CRC info is stored in last 8 bytes.    */
-    _crc_size = _stext + 4096 - 8;
-    _crc_val = _stext + 4096 - 4;
-    ASSERT((4096 > (17 + (. - _stext))), "Error: No space for CS and CRC in bootloader sector.");
-    ASSERT((_crc_size > _cs_here), "Error: CRC must be located after CS.");
+  .text : ALIGN(4)
+  {
+    _stext = .;
+    _text_start = ABSOLUTE(.);
+    *(.entry.text)
+    *(.init.literal)
+    *(.init)
+    *(.literal .text .literal.* .text.* .stub .gnu.warning .gnu.linkonce.literal.* .gnu.linkonce.t.*.literal .gnu.linkonce.t.*)
+    *(.fini.literal)
+    *(.fini)
+    *(.gnu.version)
+    _text_end = ABSOLUTE(.);
+    _etext = .;
+    . = ALIGN (4); /* Ensure 32b alignment since this is written to IRAM */
   } >iram1_0_seg :iram1_0_phdr
 
   .lit4 : ALIGN(4)

diff --git a/cores/esp8266/Esp.cpp b/cores/esp8266/Esp.cpp
@@ -444,22 +444,24 @@ bool EspClass::checkFlashConfig(bool needsEquals) {
     return false;
 }
 
+// These are defined in the linker script, and filled in by the elf2bin.py util
+extern "C" uint32_t __crc_len;
+extern "C" uint32_t __crc_val;
+
 bool EspClass::checkFlashCRC() {
-    // The CRC and total length are placed in extra space at the end of the 4K chunk
-    // of flash occupied by the bootloader.  If the bootloader grows to >4K-8 bytes,
-    // we'll need to adjust this.
-    uint32_t flashsize = *((uint32_t*)(0x40200000 + 4088)); // Start of PROGMEM plus 4K-8
-    uint32_t flashcrc = *((uint32_t*)(0x40200000 + 4092)); // Start of PROGMEM plus 4K-4
+    // Dummy CRC fill
     uint32_t z[2];
     z[0] = z[1] = 0;
 
+    uint32_t firstPart = (uintptr_t)&__crc_len - 0x40200000; // How many bytes to check before the 1st CRC val
+
     // Start the checksum
-    uint32_t crc = crc32((const void*)0x40200000, 4096-8, 0xffffffff);
+    uint32_t crc = crc32((const void*)0x40200000, firstPart, 0xffffffff);
     // Pretend the 2 words of crc/len are zero to be idempotent
     crc = crc32(z, 8, crc);
     // Finish the CRC calculation over the rest of flash
-    crc = crc32((const void*)0x40201000, flashsize-4096, crc);
-    return crc == flashcrc;
+    crc = crc32((const void*)(0x40200000 + firstPart + 8), __crc_len - (firstPart + 8), crc);
+    return crc == __crc_val;
 }
 
 

diff --git a/tools/elf2bin.py b/tools/elf2bin.py
@@ -30,8 +30,8 @@
 ffreqb = { '40': 0, '26': 1, '20': 2, '80': 15 }
 fsizeb = { '512K': 0, '256K': 1, '1M': 2, '2M': 3, '4M': 4, '8M': 8, '16M': 9 }
 
-crcsize_offset = 4088
-crcval_offset = 4092
+crcsize_offset = 4096 + 16
+crcval_offset =  4096 + 16 + 4
 
 def get_elf_entry(elf, path):
     p = subprocess.Popen([path + "/xtensa-lx106-elf-readelf", '-h', elf], stdout=subprocess.PIPE, universal_newlines=True )
@@ -188,7 +188,7 @@ def wrapper(**kwargs):
 
         wrapper(
             elf=args.eboot,
-            segments=[".text"],
+            segments=[".text", ".rodata"],
             to_addr=4096
         )
 

diff --git a/tools/sdk/ld/eagle.app.v6.common.ld.h b/tools/sdk/ld/eagle.app.v6.common.ld.h
@@ -147,6 +147,13 @@ SECTIONS
   .irom0.text : ALIGN(4)
   {
     _irom0_text_start = ABSOLUTE(.);
+
+    /* Stuff the CRC in well known symbols at a well known location */
+    __crc_len = ABSOLUTE(.);
+    LONG(0x00000000);
+    __crc_val = ABSOLUTE(.);
+    LONG(0x00000000);
+
     *(.ver_number)
     *.c.o(.literal*, .text*)
     *.cpp.o(EXCLUDE_FILE (umm_malloc.cpp.o) .literal*, EXCLUDE_FILE (umm_malloc.cpp.o) .text*)

diff --git a/tools/sdk/uzlib b/tools/sdk/uzlib