Skip to content

Commit

Permalink
bricks/stm32: Set user flash to useful length.
Browse files Browse the repository at this point in the history
Before #109 and related
commits, the user script size was limited to about 25% of RAM. The limit on
storage was not explicitly defined.

With the aforementioned updates, the amount of space is explicitly defined and
constrained by the size of the user flash. This commit updates the size of
those regions to make them the same or larger than the previous program size
limit limit, to ensure there are no regressions in the user experience.

The size could be made a bit larger, but we'll keep the values on the lower end
so we'll still have room for the firmware to grow. See
pybricks/support#713 for an overview and the full
consideration.

For powered up hubs, this means that the user data partially overlaps with the
region scanned by the bootloader to compute the checksum. This commit adds a
checksum complement to the user data map to compensate.
  • Loading branch information
laurensvalk committed Aug 30, 2022
1 parent 6cc4aff commit c031740
Show file tree
Hide file tree
Showing 12 changed files with 85 additions and 21 deletions.
14 changes: 9 additions & 5 deletions bricks/cityhub/city_hub.ld
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
/* Specify the memory areas */
MEMORY
{
/* Non-erasable BLE bootloader (LEGO LWP3). */
/* Non-erasable BLE bootloader (LEGO LWP3). Device boots only if all words
in FLASH_FIRMWARE and FLASH_USER_0 add up to 0*/
FLASH_BOOTLOADER (rx) : ORIGIN = 0x08000000, LENGTH = 20K
/* The firmware. Installed via BLE bootloader. */
FLASH_FIRMWARE (rx) : ORIGIN = 0x08005000, LENGTH = 232K
FLASH_FIRMWARE (rx) : ORIGIN = 0x08005000, LENGTH = 220K
/* User data written by the firmware during shutdown. */
FLASH_USER (rx) : ORIGIN = 0x0803F000, LENGTH = 4K
FLASH_USER_0 (rx) : ORIGIN = 0x0803C000, LENGTH = 12K
/* As above, but this part is not counted by bootloader checksum. */
FLASH_USER_1 (rx) : ORIGIN = 0x0803F000, LENGTH = 4K
RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 32K
}

"MAGIC_OFFSET" = 0x100;
_stack_size = 4K;
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER);
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER_0);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER_0) + LENGTH(FLASH_USER_1);
_pbsys_program_load_checked_size = LENGTH(FLASH_USER_0);
14 changes: 9 additions & 5 deletions bricks/movehub/move_hub.ld
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
/* Specify the memory areas */
MEMORY
{
/* Non-erasable BLE bootloader (LEGO LWP3). */
/* Non-erasable BLE bootloader (LEGO LWP3). Device boots only if all words
in FLASH_FIRMWARE and FLASH_USER_0 add up to 0*/
FLASH_BOOTLOADER (rx) : ORIGIN = 0x08000000, LENGTH = 20K
/* The firmware. Installed via BLE bootloader. */
FLASH_FIRMWARE (rx) : ORIGIN = 0x08005000, LENGTH = 106K
FLASH_FIRMWARE (rx) : ORIGIN = 0x08005000, LENGTH = 104K
/* User data written by the firmware during shutdown. */
FLASH_USER (rx) : ORIGIN = 0x0801F800, LENGTH = 2K
FLASH_USER_0 (rx) : ORIGIN = 0x0801F000, LENGTH = 2K
/* As above, but this part is not counted by bootloader checksum. */
FLASH_USER_1 (rx) : ORIGIN = 0x0801F800, LENGTH = 2K
RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 16K
}

"MAGIC_OFFSET" = 0x100;
_stack_size = 3K;
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER);
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER_0);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER_0) + LENGTH(FLASH_USER_1);
_pbsys_program_load_checked_size = LENGTH(FLASH_USER_0);
4 changes: 2 additions & 2 deletions bricks/stm32/common.ld
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ SECTIONS
/**
* The full storage may be loaded to RAM on boot, so we need to have
* at least as much RAM available. We also want to ensure there is
* at least 4K of additional RAM for the application heap.
* at least 3K of additional RAM for the application heap.
*/
. = . + _pbdrv_block_device_storage_size + 4096;
. = . + _pbdrv_block_device_storage_size + 3072;
. = ALIGN(4);
} >RAM

Expand Down
14 changes: 9 additions & 5 deletions bricks/technichub/technic_hub.ld
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@
/* Specify the memory areas */
MEMORY
{
/* Non-erasable BLE bootloader (LEGO LWP3). */
/* Non-erasable BLE bootloader (LEGO LWP3). Device boots only if all words
in FLASH_FIRMWARE and FLASH_USER_0 add up to 0*/
FLASH_BOOTLOADER (rx) : ORIGIN = 0x08000000, LENGTH = 32K
/* The firmware. Installed via BLE bootloader. */
FLASH_FIRMWARE (rx) : ORIGIN = 0x08008000, LENGTH = 220K
FLASH_FIRMWARE (rx) : ORIGIN = 0x08008000, LENGTH = 208K
/* User data written by the firmware during shutdown. */
FLASH_USER (rx) : ORIGIN = 0x0803F000, LENGTH = 4K
FLASH_USER_0 (rx) : ORIGIN = 0x0803C000, LENGTH = 12K
/* As above, but this part is not counted by bootloader checksum. */
FLASH_USER_1 (rx) : ORIGIN = 0x0803F000, LENGTH = 4K
RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 64K
}

"MAGIC_OFFSET" = 0x200;
_stack_size = 8K;
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER);
_pbdrv_block_device_storage_start = ORIGIN(FLASH_USER_0);
_pbdrv_block_device_storage_size = LENGTH(FLASH_USER_0) + LENGTH(FLASH_USER_1);
_pbsys_program_load_checked_size = LENGTH(FLASH_USER_0);
8 changes: 4 additions & 4 deletions lib/pbio/drv/block_device/block_device_flash_stm32.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ static pbio_error_t block_device_erase_and_write(uint8_t *buffer, uint32_t size)

static const uint32_t base_address = (uint32_t)(&_pbdrv_block_device_storage_start[0]);

// Exit on invalid size.
if (size == 0 || size > pbdrv_block_device_get_size()) {
// Exit if size is 0, too big, or not a multiple of double-word size.
if (size == 0 || size > pbdrv_block_device_get_size() || size % sizeof(uint64_t)) {
return PBIO_ERROR_INVALID_ARG;
}

Expand All @@ -74,7 +74,7 @@ static pbio_error_t block_device_erase_and_write(uint8_t *buffer, uint32_t size)
return PBIO_ERROR_IO;
}

// Erase only as much as we need.
// Erase the whole user storage area.
FLASH_EraseInitTypeDef erase_init = {
#if defined(STM32F0)
.PageAddress = base_address,
Expand All @@ -84,7 +84,7 @@ static pbio_error_t block_device_erase_and_write(uint8_t *buffer, uint32_t size)
#else
#error "Unsupported target."
#endif
.NbPages = (size + (FLASH_PAGE_SIZE - 1)) / FLASH_PAGE_SIZE,
.NbPages = pbdrv_block_device_get_size() / FLASH_PAGE_SIZE,
.TypeErase = FLASH_TYPEERASE_PAGES
};

Expand Down
1 change: 1 addition & 0 deletions lib/pbio/platform/city_hub/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (0)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (1)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (0)
1 change: 1 addition & 0 deletions lib/pbio/platform/debug/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (0)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (0)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (0)
1 change: 1 addition & 0 deletions lib/pbio/platform/essential_hub/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (0)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (0)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (1)
1 change: 1 addition & 0 deletions lib/pbio/platform/move_hub/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (0)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (1)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (0)
1 change: 1 addition & 0 deletions lib/pbio/platform/prime_hub/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (1)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (0)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (1)
1 change: 1 addition & 0 deletions lib/pbio/platform/technic_hub/pbsysconfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
#define PBSYS_CONFIG_HUB_LIGHT_MATRIX (0)
#define PBSYS_CONFIG_MAIN (1)
#define PBSYS_CONFIG_PROGRAM_LOAD (1)
#define PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM (1)
#define PBSYS_CONFIG_STATUS_LIGHT (1)
#define PBSYS_CONFIG_STATUS_LIGHT_BATTERY (0)
46 changes: 46 additions & 0 deletions lib/pbio/sys/program_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ typedef struct {
* element of this structure.
*/
uint32_t write_size;
#if PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM
/**
* Checksum complement to satisfy bootloader requirements. This ensures
* that words in the scanned area still add up to precisely 0 after user
* data was written.
*/
uint32_t checksum_complement;
#endif
/**
* Size of the application program.
*/
Expand All @@ -48,6 +56,38 @@ typedef struct {
extern uint32_t _pbsys_program_load_user_ram_start;
static data_map_t *map = (data_map_t *)&_pbsys_program_load_user_ram_start;

#if PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM
// Updates checksum in data map to satisfy bootloader requirements.
static void pbsys_program_load_update_checksum(void) {

// Align writable data by a double word, to simplify checksum
// computation and storage drivers that write double words.
while (map->write_size % 8) {
*((uint8_t *)map + map->write_size++) = 0;
}

// The area scanned by the bootloader adds up to 0 when all user data
// is 0xFFFFFFFF. So the bootloader value up until just before the user
// data is always 0 + the number of words in the scanned user data.
extern uint32_t _pbsys_program_load_checked_size;
uint32_t checksize = (uint32_t)&_pbsys_program_load_checked_size;
uint32_t checksum = checksize / sizeof(uint32_t);

// Don't count existing value.
map->checksum_complement = 0;

// Add checksum for each word in the written data and empty checked size.
for (uint32_t offset = 0; offset < checksize; offset += sizeof(uint32_t)) {
uint32_t *word = (uint32_t *)((uint8_t *)map + offset);
// Assume that everything after written data is erased.
checksum += offset < map->write_size ? *word : 0xFFFFFFFF;
}

// Set the checksum complement to cancel out user data checksum.
map->checksum_complement = 0xFFFFFFFF - checksum + 1;
}
#endif // PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM

// Gets the (constant) maximum program size.
static inline uint32_t pbsys_program_load_get_max_program_size() {
return pbdrv_block_device_get_size() - sizeof(data_map_t);
Expand Down Expand Up @@ -119,6 +159,12 @@ PROCESS_THREAD(pbsys_program_load_process, ev, data) {

// Write data to storage if it was updated.
if (map->write_size) {

#if PBSYS_CONFIG_PROGRAM_LOAD_OVERLAPS_BOOTLOADER_CHECKSUM
pbsys_program_load_update_checksum();
#endif

// Write the data.
PROCESS_PT_SPAWN(&pt, pbdrv_block_device_store(&pt, (uint8_t *)map, map->write_size, &err));
}

Expand Down

0 comments on commit c031740

Please sign in to comment.