Improve code and add a Flash CRC check

stm-firmware/CMakeLists.txt

@@ -45,6 +45,7 @@ endif (GIT_FOUND)
 
 set(ELFFILE ${PROJECT_NAME}.elf)
 set(HEXFILE ${PROJECT_NAME}.hex)
+set(MAPFILE ${PROJECT_NAME}.map)
 set(LINKER_SCRIPT ${CMAKE_CURRENT_SOURCE_DIR}/stm32f407vet6_flash.ld)
 
 add_compile_options(-Wall -Wextra -Wold-style-declaration -Wuninitialized -Wmaybe-uninitialized -Wunused-parameter)
@@ -62,6 +63,7 @@ include_directories(${CMAKE_CURRENT_SOURCE_DIR}/include)
 IF(CMAKE_BUILD_TYPE STREQUAL "Debug")
 	add_definitions(-DDEBUGBUILD)
 	add_compile_options(-O0 -g)
+	add_link_options(-Wl,-Map=${MAPFILE})
 ELSE()
 	add_compile_options(-O3 -g)
 	add_link_options(-Wl,--gc-sections)

stm-firmware/crc-patch-elf.py

@@ -0,0 +1,113 @@
+#!/bin/python
+
+"""
+This script patches the CRC checksums into an existing ELF file.
+
+For this, it searches the follwoing sections:
+	1) .text
+	2) .data
+	3) .ccmdata
+	4) .vectors
+
+All sections MUST be a multiple of 4 bytes long because the CRC calculation relies on whole 32 bit words.
+The sections are excrated and the CRC is calculated for each section.
+
+In the section .flashcrc, the script expects a single struct with the prototype:
+struct flash_crcs {
+	uint32_t start_magic;
+	uint32_t crc_section_text;
+	uint32_t crc_section_data;
+	uint32_t crc_section_ccm_data;
+	uint32_t crc_section_vectors;
+	uint32_t end_magic;
+};
+
+It checks, if the start magic and end magic are set to the appropriate values and then patches in the CRC values of the sections.
+The magic values checked for are: 0xA8BE53F9 and 0xFFA582FF
+"""
+from elftools.elf.elffile import ELFFile
+from elftools.elf.segments import Segment
+import elftools.elf.constants as elf_const
+import sys
+import crcmod
+import crcmod.predefined
+import struct
+
+crc_calc = crcmod.predefined.mkCrcFun('crc-32-mpeg')
+
+if len(sys.argv) < 2:
+	print("Usage:", sys.argv[0], ' <elf file>')
+	sys.exit(-1)
+
+filename=sys.argv[1]
+
+def section_calculate_crc(section):
+	data = bytearray(section.data())
+	be_data = bytearray([0 for k in range(0, len(data))])
+	# Rearrange data, because the STM controller sees it as 32 bit little endian words
+	for i in range(0, int(len(data)/4)):
+		be_data[i*4+0] = data[i*4+3]
+		be_data[i*4+1] = data[i*4+2]
+		be_data[i*4+2] = data[i*4+1]
+		be_data[i*4+3] = data[i*4+0]
+
+	return crc_calc(be_data)
+
+with open(filename, 'r+b') as f:
+	elf = ELFFile(f)
+	sections = {}
+	sections['.text'] = elf.get_section_by_name('.text')
+	sections['.data'] = elf.get_section_by_name('.data')
+	sections['.ccmdata'] = elf.get_section_by_name('.ccmdata')
+	sections['.vectors'] = elf.get_section_by_name('.vectors')
+
+	for key, sec in sections.items():
+		if sec is None:
+			print("Error! Section", key, "not found in ELF file!")
+			sys.exit(-1)
+		print('Found section', key, 'Size:',
+			  sec.data_size, 'Type:', sec['sh_type'])
+		if sec['sh_type'] != 'SHT_PROGBITS':
+			print('Error! Section must be of type SHT_PROGBITS')
+			sys.exit(-1)
+		if (sec.data_size % 4 != 0):
+			print("Section", key, "has wrong size. Must be a multiple of 4 bytes!")
+			sys.exit(-1)
+
+	text_crc = section_calculate_crc(sections['.text'])
+	print('CRC of .text section:', hex(text_crc))
+	data_crc = section_calculate_crc(sections['.data'])
+	print('CRC of .data section:', hex(data_crc))
+	ccmdata_crc = section_calculate_crc(sections['.ccmdata'])
+	print('CRC of .ccmdata section:', hex(ccmdata_crc))
+	vextors_crc = section_calculate_crc(sections['.vectors'])
+	print('CRC of .vectors section:', hex(vextors_crc))
+
+	# Check the flashcrc section
+	flashcrc_sec = elf.get_section_by_name('.flashcrc')
+	if flashcrc_sec is None:
+		print('Section for flash CRC missing!')
+		sys.exit(-1)
+	if flashcrc_sec.data_size != 6*4:
+		print("Warning!!! .flashcrc section has wrong size:",flashcrc_sec.data_size)
+
+	crc_sec_data = bytearray(flashcrc_sec.data())
+	magic1 = struct.unpack('<I'*1, bytes(crc_sec_data[0:4]))[0]
+	magic2 = struct.unpack('<I'*1, bytes(crc_sec_data[-4:]))[0]
+	print("CRC section magic values:", hex(magic1), hex(magic2))
+	if magic1 != 0xA8BE53F9 or magic2 != 0xFFA582FF:
+		print("Wrong magics in CRC section. Data misalignment?")
+		sys.exit(-2)
+
+	crc_sec_offset = flashcrc_sec['sh_offset']
+	print('CRC section ELF file offset:', hex(crc_sec_offset))
+	
+	crc_sec_data[4:8] = struct.pack('<I',text_crc)
+	crc_sec_data[8:12] = struct.pack('<I',data_crc)
+	crc_sec_data[12:16] = struct.pack('<I',ccmdata_crc)
+	crc_sec_data[16:20] = struct.pack('<I',vextors_crc)
+	f.seek(crc_sec_offset)
+	f.write(crc_sec_data)
+	print('CRCs patched successfully')
+
+	

stm-firmware/include/reflow-controller/safety/safety-config.h

@@ -56,6 +56,8 @@ enum safety_flag {
 	ERR_FLAG_SAFETY_TAB_CORRUPT = (1<<16),
 	ERR_FLAG_AMON_SUPPLY_VOLT = (1<<17),
 	ERR_FLAG_OVERTEMP = (1<<18),
+	ERR_FLAG_FLASH_CRC_CODE = (1<<19),
+	ERR_FLAG_FLASH_CRC_DATA = (1<<20),
 };
 
 /**
@@ -166,6 +168,8 @@ enum analog_value_monitor {
 					ERR_FLAG_PERSIST_ENTRY(ERR_FLAG_SAFETY_TAB_CORRUPT, true), \
 					ERR_FLAG_PERSIST_ENTRY(ERR_FLAG_AMON_SUPPLY_VOLT, false), \
 					ERR_FLAG_PERSIST_ENTRY(ERR_FLAG_OVERTEMP, false), \
+					ERR_FLAG_PERSIST_ENTRY(ERR_FLAG_FLASH_CRC_CODE, true), \
+					ERR_FLAG_PERSIST_ENTRY(ERR_FLAG_FLASH_CRC_DATA, true)
 /**
  * @brief Default config weights of safety flags. These values are loaded into the safety tables on startup
  */
@@ -188,5 +192,7 @@ enum analog_value_monitor {
 			ERR_FLAG_WEIGHT_ENTRY(ERR_FLAG_SAFETY_TAB_CORRUPT, SAFETY_FLAG_CONFIG_WEIGHT_PANIC), \
 			ERR_FLAG_WEIGHT_ENTRY(ERR_FLAG_AMON_SUPPLY_VOLT, SAFETY_FLAG_CONFIG_WEIGHT_PID), \
 			ERR_FLAG_WEIGHT_ENTRY(ERR_FLAG_OVERTEMP, SAFETY_FLAG_CONFIG_WEIGHT_PID), \
+			ERR_FLAG_WEIGHT_ENTRY(ERR_FLAG_FLASH_CRC_CODE, SAFETY_FLAG_CONFIG_WEIGHT_NONE), \
+			ERR_FLAG_WEIGHT_ENTRY(ERR_FLAG_FLASH_CRC_DATA, SAFETY_FLAG_CONFIG_WEIGHT_NONE), \
 
 #endif /* __SAFETY_CONFIG_H__ */

stm-firmware/include/reflow-controller/safety/safety-controller.h

@@ -267,6 +267,13 @@ int safety_controller_set_overtemp_limit(float over_temperature);
  */
 float safety_controller_get_overtemp_limit(void);
 
+/**
+ * @brief Perform a CRC check of the flash memory and set appropriate flags
+ * @return negative if internal error occured. Otherwise (independent from CRC check result) 0.
+ * @note This function requires the safety controller to be set up before!
+ */
+int safety_controller_trigger_flash_crc_check(void);
+
 #endif /* __SAFETY_CONTROLLER_H__ */
 
 /** @} */

stm-firmware/include/reflow-controller/safety/safety-memory.h

@@ -35,9 +35,14 @@
 #define SAFETY_MEMORY_MAGIC 0x12AA5CB7
 
 /**
- * @brief Error memory NOP entry
+ * @brief Error memory NOP entry word written to the memory
  */
-#define SAFETY_MEMORY_NOP_ENTRY 0xC1AA1222
+#define SAFETY_MEMORY_NOP_ENTRY_WORD 0xC1AA1222UL
+
+/**
+ * @brief Low Byte (byte 0) of error memory entry
+ */
+#define SAFETY_MEMORY_ERROR_ENTRY_MARKER 0x51U
 
 /**
  * @brief Offset address for the safety_memory_header.

stm-firmware/include/reflow-controller/temp-converter.h

@@ -36,6 +36,6 @@ int temp_converter_convert_resistance_to_temp(float resistance, float *temp_out)
  * @return 0 if ok, -1 if tmeperature is below the lookup table, 1 if value is above the lookup table, -1000 in case of a pointer error,
  *	   -100 if an internal error occured. This should never happen, if the lookup table is correct
  */
-int temp_convertet_convert_temp_to_resistance(float temp, float *resistance_out);
+int temp_converter_convert_temp_to_resistance(float temp, float *resistance_out);
 
 #endif /* __TEMP_CONVERTER_H__ */

stm-firmware/safety/safety-controller.c

@@ -153,6 +153,15 @@ struct overtemp_config {
 	uint32_t crc;
 };
 
+struct flash_crcs {
+	uint32_t start_magic;
+	uint32_t crc_section_text;
+	uint32_t crc_section_data;
+	uint32_t crc_section_ccm_data;
+	uint32_t crc_section_vectors;
+	uint32_t end_magic;
+};
+
 /**
  * @brief All safety error flags.
  */
@@ -176,6 +185,8 @@ static volatile struct error_flag IN_SECTION(.ccm.data) flags[] = {
 	ERR_FLAG_ENTRY(ERR_FLAG_SAFETY_TAB_CORRUPT),
 	ERR_FLAG_ENTRY(ERR_FLAG_AMON_SUPPLY_VOLT),
 	ERR_FLAG_ENTRY(ERR_FLAG_OVERTEMP),
+	ERR_FLAG_ENTRY(ERR_FLAG_FLASH_CRC_CODE),
+	ERR_FLAG_ENTRY(ERR_FLAG_FLASH_CRC_DATA),
 };
 
 /**
@@ -257,7 +268,7 @@ static void set_overtemp_config(float over_temperature)
 	int result;
 	float resistance;
 
-	result = temp_convertet_convert_temp_to_resistance(over_temperature, &resistance);
+	result = temp_converter_convert_temp_to_resistance(over_temperature, &resistance);
 	/* An error in this function is really bad... */
 	if (result < -1)
 		panic_mode();
@@ -764,6 +775,7 @@ void safety_controller_init()
 	/* This is usually done by the safety memory already. But, since this module also uses the CRC... */
 	crc_unit_init();
 
+	safety_controller_trigger_flash_crc_check();
 	stack_check_init_corruption_detect_area();
 
 	hw_rev = get_pcb_hardware_version();
@@ -1296,4 +1308,102 @@ float safety_controller_get_overtemp_limit(void)
 	return safety_controller_overtemp_config.overtemp_deg_celsius;
 }
 
+extern const uint32_t __ld_vectors_start;
+extern const uint32_t __ld_vectors_end;
+extern const uint32_t __ld_text_start;
+extern const uint32_t __ld_text_end;
+
+extern const uint32_t __ld_sdata_ccm;
+extern const uint32_t __ld_edata_ccm;
+extern const uint32_t __ld_load_ccm_data;
+
+extern const uint32_t __ld_sdata;
+extern const uint32_t __ld_edata;
+extern const uint32_t __ld_load_data;
+
+int safety_controller_trigger_flash_crc_check()
+{
+	/* This structs needs to be volatile!!
+	 * This prevents the compiler form optimizing out the reads to the crcs which will be patched in later by
+	 * a separate python script!
+	 */
+	static volatile const struct flash_crcs IN_SECTION(.flashcrc) crcs_in_flash =
+	{
+		.start_magic = 0xA8BE53F9UL,
+		.crc_section_ccm_data = 0UL,
+		.crc_section_text = 0UL,
+		.crc_section_data = 0UL,
+		.crc_section_vectors = 0UL,
+		.end_magic = 0xFFA582FFUL,
+	};
+
+	int ret = -1;
+	uint32_t len;
+	uint32_t crc;
+
+	/* Perform CRC check over vector table */
+	len = (uint32_t)((void *)&__ld_vectors_end - (void *)&__ld_vectors_start);
+	if (len % 4) {
+		safety_controller_report_error(ERR_FLAG_FLASH_CRC_CODE);
+	} else {
+		len /= 4;
+		crc_unit_reset();
+		crc_unit_input_array(&__ld_vectors_start, len);
+		crc = crc_unit_get_crc();
+		if (crc != crcs_in_flash.crc_section_vectors) {
+			safety_controller_report_error(ERR_FLAG_FLASH_CRC_CODE);
+		}
+	}
+
+	/* Perform CRC check over text section */
+	len = (uint32_t)((void *)&__ld_text_end - (void *)&__ld_text_start);
+	if (len % 4) {
+		safety_controller_report_error(ERR_FLAG_FLASH_CRC_CODE);
+	} else {
+		len /= 4;
+		crc_unit_reset();
+		crc_unit_input_array(&__ld_text_start, len);
+		crc = crc_unit_get_crc();
+		if (crc != crcs_in_flash.crc_section_text) {
+			safety_controller_report_error(ERR_FLAG_FLASH_CRC_CODE);
+		}
+	}
+
+	/* Perform CRC check over data section */
+	len = (uint32_t)((void *)&__ld_edata - (void *)&__ld_sdata);
+	if (len % 4) {
+		safety_controller_report_error(ERR_FLAG_FLASH_CRC_DATA);
+	} else {
+		len /= 4;
+		crc_unit_reset();
+		crc_unit_input_array(&__ld_load_data, len);
+		crc = crc_unit_get_crc();
+		if (crc != crcs_in_flash.crc_section_data) {
+			safety_controller_report_error(ERR_FLAG_FLASH_CRC_DATA);
+		}
+	}
+
+	/* Perform CRC check over ccm data section */
+	len = (uint32_t)((void *)&__ld_edata_ccm - (void *)&__ld_sdata_ccm);
+	if (len % 4) {
+		safety_controller_report_error(ERR_FLAG_FLASH_CRC_DATA);
+	} else {
+		len /= 4;
+		crc_unit_reset();
+		crc_unit_input_array(&__ld_load_ccm_data, len);
+		crc = crc_unit_get_crc();
+		if (crc != crcs_in_flash.crc_section_ccm_data) {
+			safety_controller_report_error(ERR_FLAG_FLASH_CRC_DATA);
+		}
+	}
+
+	crc_unit_reset();
+	crc_unit_input(0x04030201);
+	crc_unit_input(0xA0B0C0D0);
+	crc = crc_unit_get_crc();
+
+	ret = 0;
+	return ret;
+}
+
 /** @} */

stm-firmware/safety/safety-memory.c

@@ -35,11 +35,11 @@ static int word_to_error_memory_entry(uint32_t entry_data, struct error_memory_e
 	if (!out)
 		return -1002;
 
-	if (entry_data == SAFETY_MEMORY_NOP_ENTRY) {
+	if (entry_data == SAFETY_MEMORY_NOP_ENTRY_WORD) {
 		out->flag_num = 0U;
 		out->type = SAFETY_MEMORY_ERR_ENTRY_NOP;
 		out->counter = 0U;
-	} else if ((entry_data & 0xFFU) == 0x51U) {
+	} else if ((entry_data & 0xFFU) == SAFETY_MEMORY_ERROR_ENTRY_MARKER) {
 		out->flag_num = (uint8_t)((entry_data >> 8U) & 0xFFU);
 		out->type = SAFETY_MEMORY_ERR_ENTRY_FLAG;
 		out->counter = (uint16_t)((entry_data >> 16U) & 0xFFFF);
@@ -57,10 +57,10 @@ static uint32_t error_memory_entry_to_word(const struct error_memory_entry *entr
 
 	switch (entry->type) {
 	case SAFETY_MEMORY_ERR_ENTRY_NOP:
-		word = SAFETY_MEMORY_NOP_ENTRY;
+		word = SAFETY_MEMORY_NOP_ENTRY_WORD;
 		break;
 	case SAFETY_MEMORY_ERR_ENTRY_FLAG:
-		word = 0x51UL | ((uint32_t)entry->flag_num << 8U) |
+		word = (uint32_t)SAFETY_MEMORY_ERROR_ENTRY_MARKER | ((uint32_t)entry->flag_num << 8U) |
 				((uint32_t)entry->counter << 16U);
 		break;
 	}
@@ -322,9 +322,9 @@ static int safety_memory_check_error_entries()
 			return -100;
 
 		/* Valid flag entry */
-		if ((data & 0xFF) == 0x51)
+		if ((data & 0xFF) == SAFETY_MEMORY_ERROR_ENTRY_MARKER)
 			continue;
-		if (data == SAFETY_MEMORY_NOP_ENTRY)
+		if (data == SAFETY_MEMORY_NOP_ENTRY_WORD)
 			continue;
 
 		ret--;

stm-firmware/stm32f407vet6_flash.ld

@@ -44,12 +44,15 @@ SECTIONS
 {
 	.vectors : ALIGN(4)
 	{
+		__ld_vectors_start = .;
 		KEEP(*(.vectors))
 		. = ALIGN(4);
+		__ld_vectors_end = .;
 	} >FLASH =0xFF
 	
 	.text : ALIGN(4)
 	{
+		__ld_text_start = .;
 		*(.text)           /* .text sections (code) */
     		*(.text*)          /* .text* sections (code) */
     		*(.rodata)         /* .rodata sections (constants, strings, etc.) */
@@ -59,6 +62,8 @@ SECTIONS
 		*(.eh_frame)
 		KEEP(*(.init))	   /* Constructors */
 		KEEP(*(.fini))     /* Destructors  */
+		. = ALIGN(4);
+		__ld_text_end = .;
 	} >FLASH =0xFF
 	
 	.ARM.extab : ALIGN(4)
@@ -98,6 +103,13 @@ SECTIONS
     		/* Ensure LMA of ram data is at 32 bit word address in Flash */
     		. = ALIGN(4);
   	} >FLASH =0xFF
+  	
+  	.flashcrc : ALIGN(4)
+  	{
+  		KEEP(*(.flashcrc))
+  		KEEP(*(.flashcrc.*))
+  		. = ALIGN(4);
+  	} >FLASH =0xFF
 
 	/* Initialized CCM data */
 	__ld_load_ccm_data = LOADADDR(.ccmdata);

stm-firmware/temp-converter.c

@@ -75,7 +75,7 @@ return_ret_val:
 	return ret_val;
 }
 
-int temp_convertet_convert_temp_to_resistance(float temp, float *resistance_out)
+int temp_converter_convert_temp_to_resistance(float temp, float *resistance_out)
 {
 	int retcode = 0;
 	unsigned int i;