Potential for putting partitions in invalid state for smt32h723

[hakanlundvall]

rustBoot/boards/hal/src/stm/stm32h723.rs

Lines 138 to 150 in 8437fd2

	// Checks if flags page.
	// STM32H7: Due to ECC functionality, it is not possible to write partition/sector
	// flags and signature more than once. This flags_cache is used to intercept write operations and
	// ensures that the sector is always erased before each write.
	if stm32h7_boot_flag_page(addr as u32) {
	self.hal_flash_lock();
	self.hal_flash_erase((STM32H7_PART_BOOT_FLAGS_PAGE_ADDRESS as usize), 1);
	self.hal_flash_unlock();
	} else if stm32h7_update_flag_page(addr as u32) {
	self.hal_flash_lock();
	self.hal_flash_erase((STM32H7_PART_UPDATE_FLAGS_PAGE_ADDRESS as usize), 1);
	self.hal_flash_unlock();
	}

If the board get rebooted immediately after this section if part of the partitions have been swapped I believe we end up with a board that won't boot.

On possible solution would be two spread out the flag and status bits over several flash words (256 bits) so that each time a new value is written only bits in a previously untouched 256 bit word is zeroed. This way we avoid erasing the sector.

[nihalpasham]

@imrank03 - thoughts on this.

[imrank03]

rustBoot/boards/hal/src/stm/stm32h723.rs

Lines 138 to 150 in 8437fd2

	// Checks if flags page.
	// STM32H7: Due to ECC functionality, it is not possible to write partition/sector
	// flags and signature more than once. This flags_cache is used to intercept write operations and
	// ensures that the sector is always erased before each write.
	if stm32h7_boot_flag_page(addr as u32) {
	self.hal_flash_lock();
	self.hal_flash_erase((STM32H7_PART_BOOT_FLAGS_PAGE_ADDRESS as usize), 1);
	self.hal_flash_unlock();
	} else if stm32h7_update_flag_page(addr as u32) {
	self.hal_flash_lock();
	self.hal_flash_erase((STM32H7_PART_UPDATE_FLAGS_PAGE_ADDRESS as usize), 1);
	self.hal_flash_unlock();
	}

If the board get rebooted immediately after this section if part of the partitions have been swapped I believe we end up with a board that won't boot.

On possible solution would be two spread out the flag and status bits over several flash words (256 bits) so that each time a new value is written only bits in a previously untouched 256 bit word is zeroed. This way we avoid erasing the sector.

@hakanlundvall Thank you for the explanation.

I'd like to seek more clarification on the suggested solution. Could you please elaborate on how spreading out the flag and status bits over several flash words (256 bits) prevents the need for sector erase? Additionally, how does this process ensure that only bits in a previously untouched 256-bit word are zeroed during the writing of a new value?

[hakanlundvall]

Sure, the ECC bits in flash are associated with 256-bit flash words. When the sector is erased, ECC bits are reset and each flash word can be written to once.
So instead of the flag values (0xff, 0x70, 0x30, and 0x00) being stored in a single byte near the end of the sector,
we could basically let the flag value be 3 * 256 bits long starting at PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET)*32 and let:

• 0xff be represented by all ones (unwritten),
• 0x70 be represented by writing 0x70 (or whatever we decide) to PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET-sector)*32
• 0x30 be represented by writing 0x30 (or whatever we decide) to PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET-sector_count - sector)*32
• 0x00 be represented by writing 0x00 (or whatever we decide) to PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET-2*sector_count - sector)*32

and read the value as:

                     ((PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET - sector)*32) & 
                      (PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET - sector_count - sector)*32) &
                      (PARTITION_START + PARTITION_SIZE - (FLAG_OFFSET - 2*sector_count - sector)*32))

Assuming the partition ends 32 bytes aligned.
The same scheme would have to be used for the partition status byte, of course.
So instead of reserving only a few bytes at the end of each partition we would need to reserve a number of 256-bit words that the firmware cannot use.

When I get time I can do a little test implementation.

[hakanlundvall]

Hmm, I did a test implementation that work for one update, then I realized what you meant by "Additionally, how does this process ensure that only bits in a previously untouched 256-bit word are zeroed during the writing of a new value?"
My solution only ensures this during one update, for the next update it would need to erase the last sector anyway...
I need to think a little bit more on this. I suppose I can get rid of the erasure during an update but I can't used the last sector for the firmware anyway. But at least getting rid of the erasure during an update should avoid getting the partitions in invalid states.

[hakanlundvall]

I have a fork at https://github.com/hakanlundvall/rustBoot/tree/flash-word where I have implemented support for a stm32h743 board and also tested out using a new 256-bit aligned flash area for every flash update. So it gets rid of the need to erase the flag page on every write. I can't use anything of the last sector of each partition for firmware though, but I doubt that is possible on the other boards anyway. It still has hardcoded values for the 256 bit flash word size, so it's not quite ready for a PR.

[imrank03]

@hakanlundvall Impressive work on implementing stm32h743 board support and exploring a new flash update approach for stm32h723. For other boards, there's no need since we're not maintaining a dedicated flash area for them.

It sounds like a promising contribution.