pbio/sys: Move block device read to driver level.

This moves the platform-specific checksum and saving logic to the driver level.

This is a better fit generally, and it is needed for EV3 where the flash shares
the SPI bus with the ADC driver. When both reside at the pbdrv level, they can
run sequentially without holding up other drivers.

This also renames the public functions to more accurately represent what they
do. This can only be used to save the disk in one go, so we can't call them
generic storage functions.

Fixes pybricks/support#2264

Author: laurensvalk

lib/pbio/drv/block_device/block_device_ev3.c

@@ -32,6 +32,34 @@
 #include <pbio/error.h>
 #include <pbio/int_math.h>
 
+static pbio_os_process_t pbdrv_block_device_ev3_init_process;
+
+static struct {
+    /**
+     * How much data to write on shutdown and load on the next boot. Includes
+     * the size of this field, because it is also saved.
+     */
+    uint32_t saved_size;
+    /**
+     * A copy of the data loaded from flash and application heap. The first
+     * portion of this, up to pbdrv_block_device_get_writable_size() bytes,
+     * gets saved to flash at shutdown.
+     */
+    uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE];
+} ramdisk __attribute__((section(".noinit"), used));
+
+uint32_t pbdrv_block_device_get_writable_size(void) {
+    return PBDRV_CONFIG_BLOCK_DEVICE_EV3_SIZE - sizeof(ramdisk.saved_size);
+}
+
+pbio_error_t pbdrv_block_device_get_data(uint8_t **data) {
+    *data = ramdisk.data;
+
+    // Higher level code can use the ramdisk data if initialization completed
+    // successfully. Otherwise it should reset to factory default data.
+    return pbdrv_block_device_ev3_init_process.err;
+}
+
 /**
  * SPI bus state.
  */
@@ -522,7 +550,7 @@ static uint8_t write_address[4] = {FLASH_CMD_WRITE_DATA};
 // Request sector erase at address. Buffer: erase command + address.
 static uint8_t erase_address[4] = {FLASH_CMD_ERASE_BLOCK};
 
-pbio_error_t pbdrv_block_device_read(pbio_os_state_t *state, uint32_t offset, uint8_t *buffer, uint32_t size) {
+static pbio_error_t pbdrv_block_device_read(pbio_os_state_t *state, uint32_t offset, uint8_t *buffer, uint32_t size) {
 
     static uint32_t size_done;
     static uint32_t size_now;
@@ -573,21 +601,29 @@ static pbio_error_t flash_wait_write(pbio_os_state_t *state) {
     PBIO_OS_ASYNC_END(PBIO_SUCCESS);
 }
 
-pbio_error_t pbdrv_block_device_store(pbio_os_state_t *state, uint8_t *buffer, uint32_t size) {
+pbio_error_t pbdrv_block_device_write_all(pbio_os_state_t *state, uint32_t used_data_size) {
 
     static pbio_os_state_t sub;
     static uint32_t offset;
     static uint32_t size_now;
     static uint32_t size_done;
     pbio_error_t err;
 
+    // We're going to write the used portion of the ramdisk to flash. Includes
+    // the size field itself, so add it to the total write size.
+    uint8_t *buffer = (void *)&ramdisk;
+    uint32_t size = used_data_size + sizeof(ramdisk.saved_size);
+
     PBIO_OS_ASYNC_BEGIN(state);
 
     // Exit on invalid size.
     if (size == 0 || size > PBDRV_CONFIG_BLOCK_DEVICE_EV3_SIZE) {
         return PBIO_ERROR_INVALID_ARG;
     }
 
+    // Store the new size so we know how much to load on next boot.
+    ramdisk.saved_size = size;
+
     #if PBDRV_CONFIG_ADC_EV3
     // HACK
     // We only store on shutdown. Block ADC.
@@ -648,10 +684,9 @@ pbio_error_t pbdrv_block_device_store(pbio_os_state_t *state, uint8_t *buffer, u
     PBIO_OS_ASYNC_END(PBIO_SUCCESS);
 }
 
-static pbio_os_process_t pbdrv_block_device_ev3_init_process;
-
 pbio_error_t pbdrv_block_device_ev3_init_process_thread(pbio_os_state_t *state, void *context) {
     pbio_error_t err;
+    static pbio_os_state_t sub;
 
     PBIO_OS_ASYNC_BEGIN(state);
 
@@ -667,10 +702,24 @@ pbio_error_t pbdrv_block_device_ev3_init_process_thread(pbio_os_state_t *state,
         return PBIO_ERROR_FAILED;
     }
 
-    // Initialization done.
+    // Read size of stored data.
+    PBIO_OS_AWAIT(state, &sub, err = pbdrv_block_device_read(&sub, 0, (uint8_t *)&ramdisk.saved_size, sizeof(ramdisk.saved_size)));
+    if (err != PBIO_SUCCESS) {
+        return err;
+    }
+
+    // Read the available data into RAM.
+    PBIO_OS_AWAIT(state, &sub, err = pbdrv_block_device_read(&sub, 0, (uint8_t *)&ramdisk, ramdisk.saved_size));
+
+    // Reading may fail with PBIO_ERROR_INVALID_ARG if the size is too big.
+    // This happens when the size value was uninitialized or another firmware
+    // was used before. We still want to proceed with the boot process. The
+    // higher level code sees this error when requesting the RAM disk. On
+    // failure, it can reset the user data to factory defaults, and save it
+    // properly on shutdown.
     pbdrv_init_busy_down();
 
-    PBIO_OS_ASYNC_END(PBIO_SUCCESS);
+    PBIO_OS_ASYNC_END(err);
 }
 
 void pbdrv_block_device_init(void) {

lib/pbio/drv/block_device/block_device_flash_stm32.c

@@ -21,39 +21,104 @@
 
 #include STM32_HAL_H
 
-void pbdrv_block_device_init(void) {
+extern uint8_t _pbdrv_block_device_storage_start[];
+
+static struct {
+    /**
+     * How much data to write on shutdown and load on the next boot. Includes
+     * the size of this field, because it is also saved.
+     */
+    uint32_t saved_size;
+    /**
+     * 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. Needs to be volatile as it gets optimized out when it
+     * is never read by our code.
+     */
+    volatile uint32_t checksum_complement;
+    /**
+     * A copy of the data loaded from flash and application heap. The first
+     * portion of this, up to pbdrv_block_device_get_writable_size() bytes,
+     * gets saved to flash at shutdown.
+     */
+    uint8_t data[PBDRV_CONFIG_BLOCK_DEVICE_RAM_SIZE];
+} ramdisk __attribute__((section(".noinit"), used));
+
+const uint32_t header_size = sizeof(ramdisk.saved_size) + sizeof(ramdisk.checksum_complement);
+
+uint32_t pbdrv_block_device_get_writable_size(void) {
+    return PBDRV_CONFIG_BLOCK_DEVICE_FLASH_STM32_SIZE - header_size;
 }
 
-extern uint8_t _pbdrv_block_device_storage_start[];
+static pbio_error_t init_err;
 
-pbio_error_t pbdrv_block_device_read(pbio_os_state_t *state, uint32_t offset, uint8_t *buffer, uint32_t size) {
+void pbdrv_block_device_init(void) {
 
-    // NB: This function is called as an awaitable for compatibility with other
-    // external storage mediums. It blocks during the memcpy, but we only use
-    // this at boot.
+    uint32_t size;
+
+    memcpy((void *)&size, _pbdrv_block_device_storage_start, sizeof(size));
 
     // Exit on invalid size.
-    if (size == 0 || offset + size > PBDRV_CONFIG_BLOCK_DEVICE_FLASH_STM32_SIZE) {
-        return PBIO_ERROR_INVALID_ARG;
+    if (size == 0 || size > PBDRV_CONFIG_BLOCK_DEVICE_FLASH_STM32_SIZE) {
+        // This error will be retrieved when higher level code requests the
+        // ramdisk, so that it can reset data to firmware defaults.
+        init_err = PBIO_ERROR_INVALID_ARG;
+        return;
     }
 
-    // Copy requested data to RAM.
-    memcpy(buffer, _pbdrv_block_device_storage_start + offset, size);
+    // Load requested amount of data to RAM. Also re-reads the size value.
+    memcpy((void *)&ramdisk, _pbdrv_block_device_storage_start, size);
+}
 
-    return PBIO_SUCCESS;
+pbio_error_t pbdrv_block_device_get_data(uint8_t **data) {
+    *data = ramdisk.data;
+    return init_err;
 }
 
-typedef union _double_word_t {
-    uint8_t data[8];
-    uint64_t dword;
-} double_word_t;
+// Updates checksum in data map to satisfy bootloader requirements.
+static void pbdrv_block_device_update_ramdisk_size_and_checksum(uint32_t used_data_size) {
+
+    // Total size includes used data and header.
+    ramdisk.saved_size = used_data_size + header_size;
 
-pbio_error_t pbdrv_block_device_store(pbio_os_state_t *state, uint8_t *buffer, uint32_t size) {
+    // Align writable data by a double word, to simplify checksum
+    // computation and storage drivers that write double words.
+    while (ramdisk.saved_size % 8) {
+        *((uint8_t *)&ramdisk + ramdisk.saved_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_storage_checked_size;
+    uint32_t checksize = (uint32_t)&_pbsys_storage_checked_size;
+    uint32_t checksum = checksize / sizeof(uint32_t);
+
+    // Don't count existing value.
+    ramdisk.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 *)&ramdisk + offset);
+        // Assume that everything after written data is erased by the block
+        // device driver prior to writing.
+        checksum += offset < ramdisk.saved_size ? *word : 0xFFFFFFFF;
+    }
+
+    // Set the checksum complement to cancel out user data checksum.
+    ramdisk.checksum_complement = 0xFFFFFFFF - checksum + 1;
+}
+
+pbio_error_t pbdrv_block_device_write_all(pbio_os_state_t *state, uint32_t used_data_size) {
 
     // NB: This function is called as an awaitable for compatibility with other
     // external storage mediums. This implementation is blocking, but we only
     // use it during shutdown so this is acceptable.
 
+    // Account for header size and make valid checksum.
+    pbdrv_block_device_update_ramdisk_size_and_checksum(used_data_size);
+    uint32_t size = ramdisk.saved_size;
+
     static const uint32_t base_address = (uint32_t)(&_pbdrv_block_device_storage_start[0]);
 
     // Exit if size is 0, too big, or not a multiple of double-word size.
@@ -103,15 +168,15 @@ pbio_error_t pbdrv_block_device_store(pbio_os_state_t *state, uint8_t *buffer, u
         __disable_irq();
 
         // Write the data and re-enable interrupts.
-        hal_err = HAL_FLASH_Program(FLASH_TYPEPROGRAM_DOUBLEWORD, base_address + done, *(uint64_t *)(buffer + done));
+        hal_err = HAL_FLASH_Program(FLASH_TYPEPROGRAM_DOUBLEWORD, base_address + done, *(uint64_t *)((void *)&ramdisk + done));
         __set_PRIMASK(irq);
         if (hal_err != HAL_OK) {
             HAL_FLASH_Lock();
             return PBIO_ERROR_IO;
         }
 
         // Update write progress.
-        done += sizeof(double_word_t);
+        done += sizeof(uint32_t) * 2;
     }
 
     // Lock flash on completion.

lib/pbio/drv/block_device/block_device_test.c

@@ -10,14 +10,10 @@
 #include <stdint.h>
 #include <string.h>
 
-#include <contiki.h>
-
 #include <pbdrv/block_device.h>
 
 #include <pbio/version.h>
 
-#include <pbsys/storage.h>
-
 /**
 The following script is compiled using pybricksdev compile hello.py in MULTI_MPY_V6.
 
@@ -58,43 +54,42 @@ static const uint8_t _program_data[] = {
     0x63,
 };
 
+// Information from MicroPython should not be used in the pbdrv drivers but it
+// is permissible for this test. It ensures we can place the expected git
+// version at the right place. FIXME: Move the git version to pybricks build
+// system, since it isn't actually the micropython git version.
+#include "genhdr/mpversion.h"
+#include <pbsys/storage.h>
+
 static struct {
-    uint32_t write_size;
     uint8_t user_data[PBSYS_CONFIG_STORAGE_USER_DATA_SIZE];
     char stored_firmware_hash[8];
     pbsys_storage_settings_t settings;
     uint32_t program_offset;
     uint32_t program_size;
     uint8_t program_data[sizeof(_program_data)];
-} blockdev = { 0 };
+} ramdisk __attribute__((section(".noinit"), used)) = { 0 };
 
-// Information from MicroPython should not be used in the pbdrv drivers but it
-// is permissible for this test. It ensures we can place the expected git
-// version at the right place. FIXME: Move the git version to pybricks build
-// system, since it isn't actually the micropython git version.
-#include "genhdr/mpversion.h"
-
-void pbdrv_block_device_init(void) {
-    blockdev.write_size = sizeof(blockdev) + sizeof(_program_data);
-    blockdev.program_size = sizeof(_program_data);
-    memcpy(&blockdev.stored_firmware_hash[0], MICROPY_GIT_HASH, sizeof(blockdev.stored_firmware_hash));
-    memcpy(&blockdev.program_data[0], _program_data, sizeof(_program_data));
+uint32_t pbdrv_block_device_get_writable_size(void) {
+    return 0;
 }
 
-pbio_error_t pbdrv_block_device_read(pbio_os_state_t *state, uint32_t offset, uint8_t *buffer, uint32_t size) {
-
-    // Exit on invalid size.
-    if (size == 0 || offset + size > PBDRV_CONFIG_BLOCK_DEVICE_TEST_SIZE) {
-        return PBIO_ERROR_INVALID_ARG;
-    }
+pbio_error_t pbdrv_block_device_get_data(uint8_t **data) {
+    *data = (void *)&ramdisk;
 
-    // Copy requested data to RAM.
-    memcpy(buffer, (uint8_t *)&blockdev + offset, size);
+    // Higher level code can use the ramdisk data if initialization completed
+    // successfully. Otherwise it should reset to factory default data.
     return PBIO_SUCCESS;
 }
 
+void pbdrv_block_device_init(void) {
+    ramdisk.program_size = sizeof(_program_data);
+    memcpy(&ramdisk.stored_firmware_hash[0], MICROPY_GIT_HASH, sizeof(ramdisk.stored_firmware_hash));
+    memcpy(&ramdisk.program_data[0], _program_data, sizeof(_program_data));
+}
+
 // Don't store any data in this implementation.
-pbio_error_t pbdrv_block_device_store(pbio_os_state_t *state, uint8_t *buffer, uint32_t size) {
+pbio_error_t pbdrv_block_device_write_all(pbio_os_state_t *state, uint32_t used_data_size) {
     return PBIO_ERROR_NOT_IMPLEMENTED;
 }