laforge has submitted this change. (
https://gerrit.osmocom.org/c/simtrace2/+/26463 )
Change subject: firmware: add crc stub to all dfu apps to ensure reliable
loading
......................................................................
firmware: add crc stub to all dfu apps to ensure reliable loading
DFU flashing of apps sometimes aborts, and although rare this leads to
broken devices if no boot button or serial/jtag access exists, because
the bootloader will keep trying to start a half-flashed app that then
crashes at some point.
The easiest fix that works with existing bootloaders is to prepend a
small 512 byte stub that calculcates the crc and compares it with the
crc calculated at build time, and then either starts the actual app, or
sets the dfu flag and resets. This ensures we either have a working,
running app, or end up in the bootloader, ready to flash again.
For obvious reasons this only applies to dfu apps, and not to flash
targets like the actual bootloader itself.
Change-Id: Id6df0486c8b779889d21800dc2441b3aa9af8a5f
---
M contrib/prepare_upload.sh
M firmware/Makefile
M firmware/apps/blupdate/main.c
M firmware/libboard/common/resources/sam3s4/dfu.ld
M firmware/libboard/common/resources/sam3s4/flash.ld
A firmware/libcommon/source/crcstub.c
A firmware/misc/crctool.c
7 files changed, 246 insertions(+), 28 deletions(-)
Approvals:
laforge: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
diff --git a/contrib/prepare_upload.sh b/contrib/prepare_upload.sh
index 90b3278..f134d2a 100755
--- a/contrib/prepare_upload.sh
+++ b/contrib/prepare_upload.sh
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
# Create copies of binaries with -latest, -$GIT_VERSION (OS#4413, OS#3452)
cd "$(dirname "$0")/.."
@@ -9,8 +9,10 @@
cd firmware/bin
for ext in bin elf; do
for file in *."$ext"; do
- without_ext="${file%.*}"
- cp -v "$file" "$without_ext-latest.$ext"
- cp -v "$file" "$without_ext-$GIT_VERSION.$ext"
+ if ! [[ "$file" =~ ^(.*padded.*|.*nocrcstub.*)$ ]];then
+ without_ext="${file%.*}"
+ cp -v "$file" "$without_ext-latest.$ext"
+ cp -v "$file" "$without_ext-$GIT_VERSION.$ext"
+ fi
done
done
diff --git a/firmware/Makefile b/firmware/Makefile
index c30dc9a..9a2d347 100644
--- a/firmware/Makefile
+++ b/firmware/Makefile
@@ -117,7 +117,7 @@
C_LIBUSB_RT = dfu.c dfu_runtime.c
C_LIBUSB_DFU = dfu.c dfu_desc.c dfu_driver.c
C_LIBCOMMON = string.c stdio.c fputs.c usb_buf.c ringbuffer.c pseudo_talloc.c
host_communication.c \
- main_common.c stack_check.c
+ main_common.c stack_check.c crcstub.c
C_BOARD = $(notdir $(wildcard libboard/common/source/*.c))
C_BOARD += $(notdir $(wildcard libboard/$(BOARD)/source/*.c))
@@ -229,12 +229,15 @@
$(OUTPUT)-combined.bin: $(BIN)/$(BOARD)-dfu-flash-padded.bin $(OUTPUT)-dfu.bin
cat $^ > $@
-$(BIN) $(OBJ): apps/$(APP)/usb_strings_generated.h
+$(BIN) $(OBJ): apps/$(APP)/usb_strings_generated.h misc/crctool
mkdir -p $@
usbstring/usbstring: usbstring/usbstring.c
gcc $^ -o $@
+misc/crctool: misc/crctool.c
+ gcc $^ -o $@
+
.PHONY: apps/$(APP)/usb_strings.txt.patched
apps/$(APP)/usb_strings.txt.patched: apps/$(APP)/usb_strings.txt
sed "s/PRODUCT_STRING/$(shell cat
libboard/$(BOARD)/product_string.txt)/" $< > $@
@@ -251,7 +254,6 @@
$(SILENT)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_OPTIONAL)
-T"libboard/common/resources/$(CHIP)/$(1).ld" -Wl,-Map,$(OUTPUT)-$(1).map -o
$(OUTPUT)-$(1).elf $$^ $(LIBS)
$(SILENT)$(NM) $(OUTPUT)-$(1).elf >$(OUTPUT)-$(1).elf.txt
$(SILENT)$(OBJCOPY) -O binary $(OUTPUT)-$(1).elf $(OUTPUT)-$(1).bin
- $(SILENT)$(SIZE) $$^ $(OUTPUT)-$(1).elf
$$(C_OBJECTS_$(1)): $(OBJ)/$(1)_%.o: %.c Makefile $(OBJ) $(BIN)
@echo [COMPILING $$<]
@@ -261,26 +263,17 @@
@echo [ASSEMBLING $$@]
$(SILENT)$(CC) $(ASFLAGS) -DENVIRONMENT_$(1) -DENVIRONMENT=\"$(1)\" -c
-o $$@ $$<
-debug_$(1): $(1)
- $(GDB) -x "$(BOARD_LIB)/resources/gcc/$(BOARD)_$(1).gdb" -ex "reset"
-readnow -se $(OUTPUT)-$(1).elf
endef
-ALL_MEMORIES = dfu flash ram
+ALL_MEMORIES = flash ram
$(foreach MEMORY, $(ALL_MEMORIES), $(eval $(call RULES,$(MEMORY))))
# files with those names do exist..
-.PHONY: ram
.PHONY: dfu
-.PHONY: flash
+dfu: $(OUTPUT)-dfu.bin
+.PHONY: ram
ram: build_ram
-dfu: build_dfu
-ifeq ($(APP), blupdate)
- $(info updating updater section with padded bootloader file..)
- $(SILENT)dd if=/dev/zero bs=16384 count=1
of=$(BIN)/$(BOARD)-dfu-flash-padded.bin
- $(SILENT)dd if=$(BIN)/$(BOARD)-dfu-flash.bin conv=notrunc
of=$(BIN)/$(BOARD)-dfu-flash-padded.bin
- $(SILENT)$(OBJCOPY) --update-section
.blupdate=bin/$(BOARD)-dfu-flash-padded.bin bin/$(BOARD)-blupdate-dfu.elf
- $(SILENT)$(OBJCOPY) -O binary bin/$(BOARD)-blupdate-dfu.elf
bin/$(BOARD)-blupdate-dfu.bin
-endif
+.PHONY: flash
flash: build_flash
#alternate way of embedding: obj file
#ifeq ($(APP), dfu)
@@ -288,8 +281,37 @@
# $(SILENT)$(OBJCOPY) --rename-section .data=.fwupdate -I binary -O
elf32-littlearm bin/$(BOARD)-dfu-flash.bin $(OBJ)/flash_fwupdate.o
#endif
-program:
- openocd -f openocd/openocd.cfg -c "init" -c "halt" -c "flash write_bank
0 ./bin/project-flash.bin 0" -c "reset" -c "shutdown"
+C_OBJECTS_dfu = $(addprefix $(OBJ)/dfu_, $(C_OBJECTS))
+ASM_OBJECTS_dfu = $(addprefix $(OBJ)/dfu_, $(ASM_OBJECTS))
+EXTRA_OBJECTS_dfu = $(addprefix $(OBJ)/dfu_, $(EXTRA_OBJECTS))
+
+$(OUTPUT)-dfu.bin: $(OUTPUT)-dfu_nocrcstub.bin
+ $(info updating app with crc..)
+ $(SILENT)cp $< [email protected]
+ $(SILENT)misc/crctool 512 [email protected]
+ $(SILENT)mv [email protected] $@
+
+$(OUTPUT)-dfu_nocrcstub.bin: $(OUTPUT)-dfu_nocrcstub.elf
+ifeq ($(APP), blupdate)
+ $(info updating updater section with padded bootloader file..)
+ $(SILENT)dd status=none if=/dev/zero bs=16384 count=1
of=$(BIN)/$(BOARD)-dfu-flash-padded.bin
+ $(SILENT)dd status=none if=$(BIN)/$(BOARD)-dfu-flash.bin conv=notrunc
of=$(BIN)/$(BOARD)-dfu-flash-padded.bin
+ $(SILENT)$(OBJCOPY) --update-section
.blupdate=bin/$(BOARD)-dfu-flash-padded.bin $<
+endif
+ $(SILENT)$(OBJCOPY) -O binary $< $@
+
+$(OUTPUT)-dfu_nocrcstub.elf: $(ASM_OBJECTS_dfu) $(C_OBJECTS_dfu)
$(EXTRA_OBJECTS_dfu)
+ $(SILENT)$(CC) $(CFLAGS) $(LDFLAGS) $(LD_OPTIONAL)
-T"libboard/common/resources/$(CHIP)/dfu.ld"
-Wl,-Map,$(OUTPUT)-dfu_nocrcstub.map -o $@ $^ $(LIBS)
+ $(SILENT)$(NM) $@ >[email protected]
+
+$(C_OBJECTS_dfu): $(OBJ)/dfu_%.o: %.c Makefile $(OBJ) $(BIN)
+ @echo [COMPILING $<]
+ $(SILENT)$(CC) $(CFLAGS) -DENVIRONMENT_dfu -DENVIRONMENT=\"dfu\" -c -o
$@ $<
+
+$(ASM_OBJECTS_dfu): $(OBJ)/dfu_%.o: %.S Makefile $(OBJ) $(BIN)
+ @echo [ASSEMBLING $@]
+ $(SILENT)$(CC) $(ASFLAGS) -DENVIRONMENT_dfu -DENVIRONMENT=\"dfu\" -c
-o$@ $<
+
SERIAL ?= /dev/ttyUSB0
log:
diff --git a/firmware/apps/blupdate/main.c b/firmware/apps/blupdate/main.c
index e9ffe22..8c48e37 100644
--- a/firmware/apps/blupdate/main.c
+++ b/firmware/apps/blupdate/main.c
@@ -89,8 +89,14 @@
flash_cmd(EFC_FCMD_EA, 0);
#endif
flash_wait_ready();
- for (;;)
- NVIC_SystemReset();
+ for (;;) {
+ /* no functon call, since NVIC_SystemReset() might not be
inlined! */
+ SCB->AIRCR = ((0x5FA << SCB_AIRCR_VECTKEY_Pos) | (SCB->AIRCR &
SCB_AIRCR_PRIGROUP_Msk) |
+ SCB_AIRCR_SYSRESETREQ_Msk);
+ __DSB();
+ while (1)
+ ;
+ }
}
#define MAX_USB_ITER BOARD_MCK / 72 // This should be around a second
diff --git a/firmware/libboard/common/resources/sam3s4/dfu.ld
b/firmware/libboard/common/resources/sam3s4/dfu.ld
index 7d896c6..86e6622 100644
--- a/firmware/libboard/common/resources/sam3s4/dfu.ld
+++ b/firmware/libboard/common/resources/sam3s4/dfu.ld
@@ -39,7 +39,8 @@
MEMORY
{
/* reserve the first 16k (= 0x4000) for the DFU bootloader */
- rom (rx) : ORIGIN = 0x00400000 + 16K, LENGTH = 256K - 16K /* flash,
256K */
+ crcstub (rx) : ORIGIN = 0x00400000 + 16K, LENGTH = 512 /* crcstub part
*/
+ rom (rx) : ORIGIN = 0x00400000 + 16K + 512, LENGTH = 256K - 16K - 512
/* flash, 256K */
/* note: dfudata will be at the start */
ram (rwx) : ORIGIN = 0x20000000, LENGTH = 48K /* SRAM, 48K */
}
@@ -47,9 +48,17 @@
/* Section Definitions */
SECTIONS
{
- .text :
- {
+ .crcstub :
+ {
. = ALIGN(4);
+ KEEP(*(.crcstub_table))
+ KEEP(*(.crcstub_code))
+ } > crcstub = 0xff
+
+ .text :
+ {
+
+ . = ALIGN(512);
_sfixed = .;
KEEP(*(.vectors .vectors.*))
*(.text .text.* .gnu.linkonce.t.*)
@@ -94,7 +103,7 @@
. = ALIGN(4);
_efixed = .; /* End of text section */
- } > rom
+ } > rom = 0xff
/DISCARD/ :
{
diff --git a/firmware/libboard/common/resources/sam3s4/flash.ld
b/firmware/libboard/common/resources/sam3s4/flash.ld
index 50631c6..b1de154 100644
--- a/firmware/libboard/common/resources/sam3s4/flash.ld
+++ b/firmware/libboard/common/resources/sam3s4/flash.ld
@@ -97,6 +97,8 @@
/DISCARD/ :
{
*(.ARM.exidx)
+ *(.crcstub_table)
+ *(.crcstub_code)
}
. = ALIGN(4);
diff --git a/firmware/libcommon/source/crcstub.c
b/firmware/libcommon/source/crcstub.c
new file mode 100644
index 0000000..ef52521
--- /dev/null
+++ b/firmware/libcommon/source/crcstub.c
@@ -0,0 +1,86 @@
+/* SIMtrace 2 firmware crc stub
+ *
+ * (C) 2021 by sysmocom -s.f.m.c. GmbH, Author: Eric Wild <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include <stdint.h>
+#include "board.h"
+#include "core_cm3.h"
+#include "usb/device/dfu/dfu.h"
+
+/*
+* This file is a bit special, everything has to go to specific sections, and
no globals are available.
+* No external functions may be called, unless inlining is enforced!
+*/
+
+static void crc_check_stub();
+__attribute__((section(".crcstub_table"))) volatile uint32_t
crcstub_dummy_table[] = {
+ (uint32_t)0xdeadc0de, /* deliberately choose invalid value so unpatched
image will not be started */
+ (uint32_t)crc_check_stub, /* must be valid flash addr */
+ (uint32_t)0xf1, /* crc value calculated by the host */
+ (uint32_t)0xf2, /* crc calc start address */
+ (uint32_t)0xf3 /* crc calc length (byte) */
+};
+
+__attribute__((section(".crcstub_code"))) static void do_crc32(int8_t c,
uint32_t *crc_reg)
+{
+ int32_t i, mask;
+ *crc_reg ^= c;
+
+ for (unsigned int j = 0; j < 8; j++)
+ if (*crc_reg & 1)
+ *crc_reg = (*crc_reg >> 1) ^ 0xEDB88320;
+ else
+ *crc_reg = *crc_reg >> 1;
+}
+
+__attribute__((section(".crcstub_code"), noinline)) static void
crc_check_stub()
+{
+ uint32_t crc_reg = 0xffffffff;
+ uint32_t expected_crc_val = crcstub_dummy_table[2];
+ uint8_t *crc_calc_startaddr = (uint8_t *)crcstub_dummy_table[3];
+ volatile uint32_t *actual_exc_tbl = (volatile uint32_t
*)crc_calc_startaddr;
+ uint32_t crc_len = crcstub_dummy_table[4];
+
+ /* 4000ms wdt tickling */
+ WDT->WDT_MR = WDT_MR_WDRSTEN | WDT_MR_WDDBGHLT | WDT_MR_WDIDLEHLT |
(((4000UL << 8) / 1000) << 16) |
+ ((4000UL << 8) / 1000);
+
+ for (uint8_t *i = crc_calc_startaddr; i < crc_calc_startaddr + crc_len;
i++)
+ do_crc32(*i, &crc_reg);
+
+ crc_reg = ~crc_reg;
+
+ if (crc_reg == expected_crc_val) {
+ /* this looks a bit awkward because we have to ensure the bx
does not require a sp-relative load */
+ __asm__ volatile("\
+ mov r0, %0;\n\
+ mov r1, %1;\n\
+ MSR msp,r0;\n\
+ bx r1;"
+ :
+ : "r"(actual_exc_tbl[0]),
"r"(actual_exc_tbl[1]));
+ } else {
+ /* no globals ! */
+ ((struct dfudata *)0x20000000)->magic = USB_DFU_MAGIC;
+ __DSB();
+ for (;;) {
+ /* no functon call, since NVIC_SystemReset() might not
be inlined! */
+ SCB->AIRCR = ((0x5FA << SCB_AIRCR_VECTKEY_Pos) |
(SCB->AIRCR & SCB_AIRCR_PRIGROUP_Msk) |
+ SCB_AIRCR_SYSRESETREQ_Msk);
+ __DSB();
+ while (1)
+ ;
+ }
+ }
+}
diff --git a/firmware/misc/crctool.c b/firmware/misc/crctool.c
new file mode 100644
index 0000000..2602753
--- /dev/null
+++ b/firmware/misc/crctool.c
@@ -0,0 +1,91 @@
+/* SIMtrace 2 firmware crc tool
+ *
+ * (C) 2021 by sysmocom -s.f.m.c. GmbH, Author: Eric Wild <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * 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.
+ */
+
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void do_crc32(int8_t c, uint32_t *crc_reg)
+{
+ int32_t i, mask;
+ *crc_reg ^= c;
+
+ for (unsigned int j = 0; j < 8; j++)
+ if (*crc_reg & 1)
+ *crc_reg = (*crc_reg >> 1) ^ 0xEDB88320;
+ else
+ *crc_reg = *crc_reg >> 1;
+}
+
+int main(int argc, char *argv[])
+{
+ uint32_t crc_reg = 0xffffffff;
+ long fsize;
+ uint8_t *buffer;
+ uint32_t crc_start_offset;
+
+ if (argc < 3) {
+ perror("usage: crctool startoffset blupdate.bin");
+ return -1;
+ }
+
+ crc_start_offset = strtoull(argv[1], 0, 10);
+
+ FILE *blfile = fopen(argv[2], "rb+");
+ if (blfile == NULL) {
+ perror("error opening file!");
+ return -1;
+ }
+
+ fseek(blfile, 0, SEEK_END);
+ fsize = ftell(blfile);
+
+ if (fsize <= crc_start_offset) {
+ perror("file size?!");
+ return -1;
+ }
+
+ fseek(blfile, 0, SEEK_SET);
+
+ buffer = malloc(fsize);
+ fread(buffer, 1, fsize, blfile);
+
+ if (*(uint32_t *)buffer != 0xdeadc0de) {
+ perror("weird magic, not a valid blupdate file?");
+ free(buffer);
+ return -1;
+ }
+
+ uint8_t *startaddr = buffer + crc_start_offset;
+ uint8_t *endaddr = buffer + fsize;
+ for (uint8_t *i = startaddr; i < endaddr; i++) {
+ do_crc32(*i, &crc_reg);
+ }
+ crc_reg = ~crc_reg;
+
+ fprintf(stderr, "len: %ld crc: %.8x\n", fsize - crc_start_offset,
crc_reg);
+
+ ((uint32_t *)buffer)[0] = 0x2000baa0; /* fix magic to valid stack
address, checked by BL */
+ ((uint32_t *)buffer)[2] = crc_reg;
+ ((uint32_t *)buffer)[3] = 0x00400000 + 0x4000 + crc_start_offset;
+ ((uint32_t *)buffer)[4] = fsize - crc_start_offset;
+
+ fseek(blfile, 0, SEEK_SET);
+ fwrite(buffer, 4, 5, blfile);
+
+ fclose(blfile);
+ free(buffer);
+ return 0;
+}
--
To view, visit https://gerrit.osmocom.org/c/simtrace2/+/26463
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: simtrace2
Gerrit-Branch: master
Gerrit-Change-Id: Id6df0486c8b779889d21800dc2441b3aa9af8a5f
Gerrit-Change-Number: 26463
Gerrit-PatchSet: 9
Gerrit-Owner: Hoernchen <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: tsaitgaist <[email protected]>
Gerrit-MessageType: merged
