Optimize Multiboot flash handling and RAM restore progress

App/driver/mb_flash.c

@@ -41,6 +41,9 @@
 #define MB_PROGRESS_FIRST_COL     5u
 #define MB_PROGRESS_FILLED      0x2Du
 
+_Static_assert(MB_INT_APP_SIZE == MB_FLASH_PAGE * MB_PROGRESS_COLS * 4u,
+               "restore progress must advance one column every four pages");
+
 /* Number of erase/program retries per page before giving up (and resetting
  * anyway - the region is already erased, so USB recovery is the only option). */
 #define MB_PAGE_RETRIES 3u
@@ -235,8 +238,6 @@ static void MB_RamReflash(uint32_t intAddr, uint32_t extAddr, uint32_t imageSize
     uint32_t remaining = imageSize;
     uint32_t regionRemaining = MB_INT_APP_SIZE;
     uint32_t pagesDone = 0;
-    uint32_t progressAccumulator = 0;
-    uint32_t progressFilled = 0;
     uint32_t lcdEnabled = progressLine != NULL;
 
 
@@ -348,21 +349,13 @@ static void MB_RamReflash(uint32_t intAddr, uint32_t extAddr, uint32_t imageSize
         if (!success)
             goto fatal_reset;
 
-        /* Advance the gauge without division (which could call a helper
-         * in erased flash). Refresh once per 8 KiB internal sector. */
+        /* Exactly four pages per column; shifts keep all arithmetic in RAM.
+         * Refresh once per 8 KiB internal sector. */
         if (lcdEnabled)
         {
             pagesDone++;
-            progressAccumulator += MB_PROGRESS_COLS;
-            while (progressAccumulator >= (MB_INT_APP_SIZE / MB_FLASH_PAGE))
-            {
-                progressAccumulator -= (MB_INT_APP_SIZE / MB_FLASH_PAGE);
-                if (progressFilled < MB_PROGRESS_COLS)
-                {
-                    progressLine[MB_PROGRESS_FIRST_COL + progressFilled] = MB_PROGRESS_FILLED;
-                    progressFilled++;
-                }
-            }
+            if ((pagesDone & 3u) == 0u)
+                progressLine[MB_PROGRESS_FIRST_COL + (pagesDone >> 2) - 1u] = MB_PROGRESS_FILLED;
             if ((pagesDone & 31u) == 0u || regionRemaining == MB_FLASH_PAGE)
                 lcdEnabled = mb_ram_progress_blit(progressLine);
         }
@@ -800,12 +793,31 @@ uint8_t MB_SlotWrite(uint8_t slot, uint32_t offset, const uint8_t *data, uint32_
 /* All external-flash only, never brick-critical.                             */
 /* -------------------------------------------------------------------------- */
 
-_Static_assert(sizeof(mb_state_t) == 24u,
-               "FMP2/FMP3 marker layout must stay 24 bytes");
-_Static_assert(offsetof(mb_state_t, firmware_slot) == 16u,
-               "FMP2 migration reads the coupled index at byte 16");
-_Static_assert(offsetof(mb_state_t, state_crc32) == 20u,
-               "state CRC must cover the first 20 bytes (magic..bank_inv)");
+/* These records are aligned RAM objects, never casts into wire buffers. Keep
+ * every field offset fixed so existing slots, markers and UART replies agree. */
+_Static_assert(sizeof(mb_slot_header_t) == 64u && _Alignof(mb_slot_header_t) == 4u,
+               "FMB1 header must stay 64 bytes with word alignment");
+_Static_assert(offsetof(mb_slot_header_t, magic) == 0u &&
+               offsetof(mb_slot_header_t, hdr_version) == 4u &&
+               offsetof(mb_slot_header_t, flags) == 6u &&
+               offsetof(mb_slot_header_t, image_size) == 8u &&
+               offsetof(mb_slot_header_t, image_crc32) == 12u &&
+               offsetof(mb_slot_header_t, name) == 16u &&
+               offsetof(mb_slot_header_t, fw_version) == 32u &&
+               offsetof(mb_slot_header_t, reserved) == 48u,
+               "FMB1 header field offsets must not change");
+_Static_assert(sizeof(mb_state_t) == 24u && _Alignof(mb_state_t) == 4u,
+               "FMP2/FMP3 marker must stay 24 bytes with word alignment");
+_Static_assert(offsetof(mb_state_t, magic) == 0u &&
+               offsetof(mb_state_t, generation) == 4u &&
+               offsetof(mb_state_t, image_size) == 8u &&
+               offsetof(mb_state_t, image_crc32) == 12u &&
+               offsetof(mb_state_t, firmware_slot) == 16u &&
+               offsetof(mb_state_t, slot_inv) == 17u &&
+               offsetof(mb_state_t, config_bank) == 18u &&
+               offsetof(mb_state_t, bank_inv) == 19u &&
+               offsetof(mb_state_t, state_crc32) == 20u,
+               "FMP2/FMP3 marker field offsets must not change");
 
 uint32_t MB_Crc32Bytes(const uint8_t *p, uint32_t len)
 {
@@ -853,44 +865,31 @@ static mb_mark_status_t mb_read_state_copy(uint32_t base, mb_state_t *st)
         return MB_MARK_LEGACY;
     }
 
-    if (st->magic == MB_STATE_V2_MAGIC)
-    {
-        /* FMP2 stored one index for both the firmware slot and config bank at
-         * byte 16, followed by its inverse and two zeroed reserved bytes.
-         * Validate the on-flash record before normalizing it in RAM to FMP3. */
-        const uint8_t legacy_index = ((const uint8_t *)st)[16];
-        const uint8_t legacy_inv   = ((const uint8_t *)st)[17];
-        if ((uint8_t)~legacy_inv != legacy_index ||
-            legacy_index >= MB_BANK_COUNT)
-            return MB_MARK_CORRUPT;
-        if (st->image_size == 0u || st->image_size > MB_INT_APP_SIZE)
-            return MB_MARK_CORRUPT;
-        if (MB_Crc32Bytes((const uint8_t *)st,
-                           sizeof(*st) - sizeof(st->state_crc32)) != st->state_crc32)
-            return MB_MARK_CORRUPT;
-
-        st->magic         = MB_STATE_MAGIC;
-        st->firmware_slot = legacy_index;
-        st->slot_inv      = (uint8_t)~legacy_index;
-        st->config_bank   = legacy_index;
-        st->bank_inv      = (uint8_t)~legacy_index;
-        st->state_crc32   = MB_Crc32Bytes((const uint8_t *)st,
-                                           sizeof(*st) - sizeof(st->state_crc32));
-        return MB_MARK_VALID;
-    }
-
-    if (st->magic != MB_STATE_MAGIC)           return MB_MARK_CORRUPT;
+    const bool version2 = st->magic == MB_STATE_V2_MAGIC;
+    if (!version2 && st->magic != MB_STATE_MAGIC) return MB_MARK_CORRUPT;
     if ((uint8_t)~st->slot_inv != st->firmware_slot)
                                                     return MB_MARK_CORRUPT;
-    if (st->firmware_slot >= MB_SLOT_COUNT)       return MB_MARK_CORRUPT;
-    if ((uint8_t)~st->bank_inv != st->config_bank)
+    if (st->firmware_slot >= (version2 ? MB_BANK_COUNT : MB_SLOT_COUNT))
+                                                    return MB_MARK_CORRUPT;
+    /* FMP2 has no independent bank: bytes 18/19 belong to its original CRC. */
+    if (!version2 && ((uint8_t)~st->bank_inv != st->config_bank ||
+                     st->config_bank >= MB_BANK_COUNT))
                                                     return MB_MARK_CORRUPT;
-    if (st->config_bank >= MB_BANK_COUNT)      return MB_MARK_CORRUPT;
     if (st->image_size == 0u ||
         st->image_size > MB_INT_APP_SIZE)         return MB_MARK_CORRUPT;
     if (MB_Crc32Bytes((const uint8_t *)st,
                        sizeof(*st) - sizeof(st->state_crc32)) != st->state_crc32)
                                                     return MB_MARK_CORRUPT;
+
+    if (version2)
+    {
+        /* Normalize only after verifying the original FMP2 record. */
+        st->magic         = MB_STATE_MAGIC;
+        st->config_bank   = st->firmware_slot;
+        st->bank_inv      = st->slot_inv;
+        st->state_crc32   = MB_Crc32Bytes((const uint8_t *)st,
+                                           sizeof(*st) - sizeof(st->state_crc32));
+    }
     return MB_MARK_VALID;
 }
 

App/driver/mb_flash.h

@@ -59,7 +59,9 @@
 #define MB_NAME_LEN         16
 #define MB_VERSION_LEN      16
 
-typedef struct __attribute__((packed)) {
+/* Natural word alignment avoids bytewise field accesses. The on-flash layout
+ * is pinned by the size/offset assertions in mb_flash.c. */
+typedef struct {
     uint32_t magic;                    /* MB_SLOT_MAGIC                    */
     uint16_t hdr_version;              /* MB_HDR_VERSION                   */
     uint16_t flags;                    /* MB_FLAG_COMMITTED, ...           */
@@ -171,7 +173,8 @@ uint8_t MB_SlotWrite(uint8_t slot, uint32_t offset, const uint8_t *data, uint32_
 #define MB_STATE_LEGACY_MAGIC   0x31504D46u     /* "FMP1" (single 8-byte record)    */
 #define MB_STATE_V2_MAGIC       0x32504D46u     /* "FMP2" (slot == config bank)     */
 #define MB_STATE_MAGIC          0x33504D46u     /* "FMP3" (slot + bank separated)   */
-typedef struct __attribute__((packed)) {
+/* Keep this record naturally aligned in RAM, with the same serialized bytes. */
+typedef struct {
     uint32_t magic;         /* MB_STATE_MAGIC                          */
     uint32_t generation;    /* monotonically increasing record version */
     uint32_t image_size;    /* expected internal image size            */

tools/check-multiboot.mjs

@@ -0,0 +1,192 @@
+// Source-level differential checks; no firmware compilation or hardware access.
+// Run from the repository root: rtk run "node tools/check-multiboot.mjs"
+// The small C subset used by marker validation and gauge arithmetic is translated
+// to JavaScript. This does not validate target alignment or RAM code placement:
+// those still require the firmware's static assertions and the linked ELF.
+import assert from 'node:assert/strict';
+import {execFileSync} from 'node:child_process';
+import {readFileSync} from 'node:fs';
+
+const baseline = '0333d2d4c6b5aa9a16ff880ad652382f2f34ba13';
+const original = execFileSync('rtk', ['git', 'show', `${baseline}:App/driver/mb_flash.c`],
+    {encoding: 'utf8', maxBuffer: 1024 * 1024});
+const current = readFileSync('App/driver/mb_flash.c', 'utf8');
+const header = readFileSync('App/driver/mb_flash.h', 'utf8');
+const strip = text => text.replace(/\/\*[\s\S]*?\*\/|\/\/[^\n]*/g, '');
+const constants = {
+    MB_STATE_LEGACY_MAGIC: 0x31504d46, MB_STATE_V2_MAGIC: 0x32504d46,
+    MB_STATE_MAGIC: 0x33504d46, MB_BANK_COUNT: 5, MB_SLOT_COUNT: 5,
+    MB_INT_APP_SIZE: 0x1d800, MB_FLASH_PAGE: 256, MB_PROGRESS_COLS: 118,
+    MB_PROGRESS_FIRST_COL: 5, MB_PROGRESS_FILLED: 0x2d,
+    MB_MARK_VALID: 0, MB_MARK_LEGACY: 1, MB_MARK_MISSING: 2,
+    MB_MARK_CORRUPT: 3, MB_MARK_IO: 4,
+};
+
+// Guard the assumptions used by this narrow source interpreter.
+for (const [name, value] of Object.entries(constants)) {
+    const match = strip(header + current).match(new RegExp(`^#define\\s+${name}\\s+(\\S+)`, 'm'));
+    if (name.startsWith('MB_MARK_')) {
+        assert.match(header, new RegExp(`${name}\\b`));
+    } else {
+        assert(match, `Missing constant ${name}`);
+        assert.equal(constants[match[1]] ?? Number(match[1].replace(/u$/, '')), value, name);
+    }
+}
+assert.match(strip(header), /MB_MARK_VALID\s*=\s*0,\s*MB_MARK_LEGACY,\s*MB_MARK_MISSING,\s*MB_MARK_CORRUPT,\s*MB_MARK_IO,/);
+
+function block(source, anchor) {
+    const start = source.indexOf(anchor);
+    assert(start >= 0, `Missing ${anchor}`);
+    const open = source.indexOf('{', start);
+    let depth = 1;
+    let end = open + 1;
+    while (depth && end < source.length) {
+        if (source[end] === '{') depth++;
+        if (source[end] === '}') depth--;
+        end++;
+    }
+    assert.equal(depth, 0);
+    return source.slice(open + 1, end - 1);
+}
+
+function crc32(bytes, length = bytes.length) {
+    let crc = 0xffffffff;
+    for (let i = 0; i < length; i++) {
+        crc ^= bytes[i];
+        for (let bit = 0; bit < 8; bit++)
+            crc = (crc >>> 1) ^ (0xedb88320 & -(crc & 1));
+    }
+    return (crc ^ 0xffffffff) >>> 0;
+}
+assert.equal(crc32(Buffer.from('123456789')), 0xcbf43926);
+
+const stateFields = {
+    magic: [0, 4], generation: [4, 4], image_size: [8, 4], image_crc32: [12, 4],
+    firmware_slot: [16, 1], slot_inv: [17, 1], config_bank: [18, 1],
+    bank_inv: [19, 1], state_crc32: [20, 4],
+};
+function stateView(bytes) {
+    const st = {bytes};
+    for (const [field, [offset, size]] of Object.entries(stateFields)) {
+        Object.defineProperty(st, field, {
+            get: () => size === 4 ? bytes.readUInt32LE(offset) : bytes[offset],
+            set: value => size === 4 ? bytes.writeUInt32LE(value >>> 0, offset) : bytes[offset] = value & 255,
+        });
+    }
+    return st;
+}
+
+function markerReader(source) {
+    let body = block(strip(source), 'static mb_mark_status_t mb_read_state_copy(')
+        .replace(/sizeof\(\*st\)/g, '24').replace(/sizeof\(st->state_crc32\)/g, '4')
+        .replace(/\(\(const uint8_t \*\)st\)/g, 'st.bytes')
+        .replace(/\((?:const )?uint8_t \*\)st/g, 'st.bytes')
+        .replace(/st->/g, 'st.')
+        .replace(/\(uint8_t\)~([\w.]+)/g, '($1 ^ 255)')
+        .replace(/const (?:uint8_t|bool) /g, 'const ')
+        .replace(/\b(0x[\da-fA-F]+|\d+)u\b/g, '$1');
+    assert(!/->|sizeof|uint\d+_t/.test(body), 'Unsupported C construct');
+    const execute = Function('input', 'readError', 'st', 'MB_Crc32Bytes', 'memset',
+        ...Object.keys(constants), `
+        let mb_spi_err = 0;
+        const base = 0;
+        function mb_ext_read(address, bytes, size) {
+            bytes.set(input.subarray(0, size));
+            mb_spi_err = Number(readError);
+        }
+        ${body}`);
+    const st = stateView(Buffer.alloc(24));
+    return (input, readError) => ({
+        status: execute(input, readError, st, crc32,
+            (target, value, size) => target.bytes.fill(value, 0, size), ...Object.values(constants)),
+        bytes: st.bytes,
+    });
+}
+
+const before = markerReader(original);
+const after = markerReader(current);
+let markerCases = 0;
+function compare(bytes, readError = false) {
+    const a = before(bytes, readError);
+    const b = after(bytes, readError);
+    assert.equal(b.status, a.status, `Status mismatch: ${bytes.toString('hex')}`);
+    assert(a.bytes.equals(b.bytes), `Record mismatch: ${bytes.toString('hex')}`);
+    markerCases++;
+}
+function makeRecord(magic) {
+    const bytes = Buffer.alloc(24);
+    const st = stateView(bytes);
+    Object.assign(st, {magic, generation: 123, image_size: 65536, image_crc32: 0x12345678,
+        firmware_slot: 2, slot_inv: 253, config_bank: 3, bank_inv: 252});
+    if (magic === constants.MB_STATE_LEGACY_MAGIC) {
+        bytes[4] = 2;
+        bytes[5] = 253;
+    }
+    return bytes;
+}
+function checksum(bytes) { bytes.writeUInt32LE(crc32(bytes, 20), 20); }
+
+// Exhaust all index/complement pairs, including every FMP2 reserved-byte pair.
+for (const magic of [constants.MB_STATE_LEGACY_MAGIC, constants.MB_STATE_V2_MAGIC, constants.MB_STATE_MAGIC]) {
+    const bytes = makeRecord(magic);
+    for (const offset of magic === constants.MB_STATE_LEGACY_MAGIC ? [4] : [16, 18]) {
+        for (let index = 0; index < 256; index++) for (let inverse = 0; inverse < 256; inverse++) {
+            bytes[offset] = index;
+            bytes[offset + 1] = inverse;
+            checksum(bytes);
+            compare(bytes);
+        }
+        // Restore valid fields before exercising a different pair.
+        makeRecord(magic).copy(bytes);
+    }
+}
+
+for (const magic of [0, 0xffffffff, constants.MB_STATE_LEGACY_MAGIC, constants.MB_STATE_V2_MAGIC, constants.MB_STATE_MAGIC]) {
+    for (const size of [0, 1, 255, 256, 257, 0x1d7ff, 0x1d800, 0x1d801, 0xffffffff]) {
+        const bytes = makeRecord(magic);
+        bytes.writeUInt32LE(size, 8);
+        checksum(bytes);
+        compare(bytes);
+        compare(bytes, true);
+        for (let byte = 0; byte < 24; byte++) for (let bit = 0; bit < 8; bit++) {
+            bytes[byte] ^= 1 << bit;
+            compare(bytes);
+            bytes[byte] ^= 1 << bit;
+        }
+    }
+}
+
+// Execute the actual old/new gauge blocks with RAM arrays and a mocked LCD.
+function gaugeStep(source) {
+    const body = block(block(strip(source), 'static void MB_RamReflash('), 'if (lcdEnabled)')
+        .replace(/\b(\d+)u\b/g, '$1');
+    return Function('state', 'mb_ram_progress_blit', ...Object.keys(constants), `
+        let {pagesDone, progressAccumulator, progressFilled, lcdEnabled, regionRemaining, progressLine} = state;
+        if (lcdEnabled) { ${body} }
+        Object.assign(state, {pagesDone, progressAccumulator, progressFilled, lcdEnabled});`);
+}
+const oldGauge = gaugeStep(original);
+const newGauge = gaugeStep(current);
+let gaugeCases = 0;
+for (const enabled of [false, true]) for (let failPage = 0; failPage <= 472; failPage++) {
+    const init = () => ({pagesDone: 0, progressAccumulator: 0, progressFilled: 0,
+        lcdEnabled: enabled, progressLine: Buffer.alloc(128, 0x21)});
+    const a = init(), b = init();
+    for (let page = 1; page <= 472; page++) {
+        a.regionRemaining = b.regionRemaining = (473 - page) * 256;
+        let oldCalls = 0, newCalls = 0;
+        oldGauge(a, () => { oldCalls++; return page !== failPage; }, ...Object.values(constants));
+        newGauge(b, () => { newCalls++; return page !== failPage; }, ...Object.values(constants));
+        assert.equal(newCalls, oldCalls);
+        assert.equal(b.lcdEnabled, a.lcdEnabled);
+        assert.equal(b.pagesDone, a.pagesDone);
+        assert(a.progressLine.equals(b.progressLine), `Gauge mismatch at page ${page}`);
+        assert.equal(b.progressLine[4], 0x21);
+        assert.equal(b.progressLine[123], 0x21);
+        gaugeCases++;
+    }
+}
+
+console.log(`Marker status and all 24 output bytes: ${markerCases} equivalent cases.`);
+console.log(`Gauge bytes, LCD calls and failure handling: ${gaugeCases} equivalent steps.`);
+console.log('No build performed. Check layout assertions and RAM-only call targets in the next Fusion ELF.');