bootloader checksum (#279)

* blindly implement checksum

* extra erase of checksum page

* don't need to write the sum ourselves

* maybe implement bootloader crc correctly

* minus is not plus

* modify build script to properly embed checksum

* cleanup

* use lma instead of vma

* implement bootloader blinky

* stub firmwareError

* static blinky

firmware/bootloader/bootloader_main.cpp

@@ -8,12 +8,48 @@ extern "C" {
 	#include "shared_params.h"
 }
 
+class BlinkyThread : public chibios_rt::BaseStaticThread<256> {
+protected:
+	void main(void) override {
+		Gpio yellow = getWarningLedPin();
+		Gpio blue = getCommsLedPin();
+		Gpio green = getRunningLedPin();
+
+		efiSetPadMode("yellow", yellow, PAL_MODE_OUTPUT_PUSHPULL);
+		efiSetPadMode("blue", blue, PAL_MODE_OUTPUT_PUSHPULL);
+		efiSetPadMode("green", green, PAL_MODE_OUTPUT_PUSHPULL);
+
+		auto yellowPort = getBrainPinPort(yellow);
+		auto yellowPin = getBrainPinIndex(yellow);
+		auto bluePort = getBrainPinPort(blue);
+		auto bluePin = getBrainPinIndex(blue);
+		auto greenPort = getBrainPinPort(green);
+		auto greenPin = getBrainPinIndex(green);
+
+		palSetPad(yellowPort, yellowPin);
+		palSetPad(bluePort, bluePin);
+		palSetPad(greenPort, greenPin);
+
+		while (true) {
+			palTogglePad(yellowPort, yellowPin);
+			palTogglePad(bluePort, bluePin);
+			palTogglePad(greenPort, greenPin);
+			chThdSleepMilliseconds(250);
+		}
+	}
+};
+
+static BlinkyThread blinky;
+
 int main(void) {
 	halInit();
 	chSysInit();
 
 	baseMCUInit();
 
+	// start the blinky thread
+	blinky.start(NORMALPRIO + 10);
+
 	// Init openblt shared params
 	SharedParamsInit();
 
@@ -25,7 +61,7 @@ int main(void) {
 	}
 }
 
-// very basic version, supports on chip pins only (really only used for USB)
+// very basic version, supports on chip pins only (really only used for USB, LEDs)
 void efiSetPadMode(const char* msg, brain_pin_e brainPin, iomode_t mode) {
 	ioportid_t port = getHwPort(msg, brainPin);
 	ioportmask_t pin = getHwPin(msg, brainPin);

firmware/bootloader/bootloader_stubs.cpp

@@ -14,6 +14,9 @@ void chDbgPanic3(const char* /*msg*/, const char* /*file*/, int /*line*/) {
 
 void logHardFault(uint32_t type, uintptr_t faultAddress, struct port_extctx* ctx, uint32_t csfr) { }
 
+void firmwareError(ObdCode code, const char *fmt, ...) {
+}
+
 // this is supposed to be taken from chconf_common.h but it does not work? I am not sure why :(
 // TODO: make this be defined by chconf_common.h?
 //#if ! ENABLE_PERF_TRACE

firmware/bootloader/openblt_chibios/openblt_flash.cpp

@@ -1,6 +1,8 @@
 #include "pch.h"
 #include "flash_int.h"
 
+#include <rusefi/crc.h>
+
 extern "C" {
 	#include "boot.h"
 	#include "flash.h"
@@ -20,16 +22,15 @@ blt_addr FlashGetUserProgBaseAddress() {
 
 blt_bool FlashWrite(blt_addr addr, blt_int32u len, blt_int8u *data) {
 	return (FLASH_RETURN_SUCCESS == intFlashWrite(addr, (const char*)data, len)) ? BLT_TRUE : BLT_FALSE;
-	
-	return BLT_TRUE;
 }
 
 blt_bool FlashErase(blt_addr addr, blt_int32u len) {
-	if (!intFlashIsErased(addr, len)) {
+	if (intFlashIsErased(addr, len)) {
-		return (FLASH_RETURN_SUCCESS == intFlashErase(addr, len)) ? BLT_TRUE : BLT_FALSE;
+		// Already blank, we can skip the expensive erase operation
+		return BLT_TRUE;
 	}
 
-	return BLT_TRUE;
+	return (FLASH_RETURN_SUCCESS == intFlashErase(addr, len)) ? BLT_TRUE : BLT_FALSE;
 }
 
 blt_bool FlashDone() {
@@ -42,5 +43,28 @@ blt_bool FlashWriteChecksum() {
 
 blt_bool FlashVerifyChecksum() {
 	// Naive check: if the first block is blank, there's no code there
-	return intFlashIsErased(FlashGetUserProgBaseAddress(), 4) ? BLT_FALSE : BLT_TRUE;
+	if (intFlashIsErased(FlashGetUserProgBaseAddress(), 4)) {
+		return BLT_FALSE;
+	}
+
+	static const size_t checksumOffset = 0x1C;
+
+	// Now do the actual CRC check to ensure we didn't get stuck with a half-written firmware image
+	uint8_t* start = reinterpret_cast<uint8_t*>(FlashGetUserProgBaseAddress());
+
+	size_t imageSize = *reinterpret_cast<size_t*>(start + checksumOffset + 4);
+
+	if (imageSize > 1024 * 1024) {
+		// impossibly large size, invalid
+		return BLT_FALSE;
+	}
+
+	// part before checksum+size
+	uint32_t calcChecksum = crc32(start, checksumOffset);
+	// part after checksum+size
+	calcChecksum = crc32inc(start + checksumOffset + 4, calcChecksum, imageSize - (checksumOffset + 4));
+
+	uint32_t storedChecksum = *reinterpret_cast<uint32_t*>(start + checksumOffset);
+
+	return calcChecksum == storedChecksum ? BLT_TRUE : BLT_FALSE;
 }

firmware/config/boards/common_make.sh

@@ -31,8 +31,25 @@ chmod u+x $HEX2DFU
 mkdir -p deliver
 rm -f deliver/*
 
-echo "$SCRIPT_NAME: invoking hex2dfu for incremental rusEFI image"
+# delete everything we're going to regenerate
-$HEX2DFU -i build/fome.hex -C 0x1C -o build/fome.dfu
+rm build/fome.bin build/fome.srec
+
+# Extract the firmware's base address from the elf - it may be different depending on exact CPU
+firmwareBaseAddress="$(objdump -h -j .vectors build/fome.elf | awk '/.vectors/ {print $5 }')"
+checksumAddress="$(printf "%X\n" $((0x$firmwareBaseAddress+0x1c)))"
+
+echo "Base address is 0x$firmwareBaseAddress"
+echo "Checksum address is 0x$checksumAddress"
+
+echo "$SCRIPT_NAME: invoking hex2dfu to place image checksum"
+$HEX2DFU -i build/fome.hex -c $checksumAddress -b build/fome.bin
+rm build/fome.hex
+# re-make hex, srec with the checksum in place
+objcopy -I binary -O ihex --change-addresses=0x$firmwareBaseAddress build/fome.bin build/fome.hex
+objcopy -I binary -O srec --change-addresses=0x$firmwareBaseAddress build/fome.bin build/fome.srec
+
+# make DFU
+$HEX2DFU -i build/fome.hex -o build/fome.dfu
 
 if [ "$USE_OPENBLT" = "yes" ]; then
   # this image is suitable for update through bootloader only
@@ -45,9 +62,8 @@ else
   # cp build/fome.hex  deliver/
 fi
 
-# bootloader and composite image
+# bootloader and combined image
 if [ "$USE_OPENBLT" = "yes" ]; then
-  rm -f deliver/fome_bl.dfu
   echo "$SCRIPT_NAME: invoking hex2dfu for OpenBLT"
   $HEX2DFU -i bootloader/blbuild/fome_bl.hex -o bootloader/blbuild/fome_bl.dfu
 
@@ -56,10 +72,8 @@ if [ "$USE_OPENBLT" = "yes" ]; then
   cp bootloader/blbuild/fome_bl.dfu  deliver/fome_bl.dfu
   #cp bootloader/blbuild/fome_bl.hex  deliver/fome_bl.hex
 
-  rm -f deliver/fome_openblt.dfu
+  echo "$SCRIPT_NAME: invoking hex2dfu for combined OpenBLT+FOME image"
-  echo "$SCRIPT_NAME: invoking hex2dfu for composite rusEFI+OpenBLT image"
+  $HEX2DFU -i bootloader/blbuild/fome_bl.hex -i build/fome.hex -o deliver/fome.dfu -b deliver/fome.bin
-  $HEX2DFU -i bootloader/blbuild/fome_bl.hex -i build/fome.hex -C 0x1C -o deliver/fome.dfu -b deliver/fome.bin
-  #todo: how to create 'signed' hex and srec? Do we need?
 fi
 
 echo "$SCRIPT_NAME: build folder content:"