diff --git a/CMakeLists.txt b/CMakeLists.txt index 80f0bd3..57e4948 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -183,6 +183,22 @@ if(PROJECT_IS_TOP_LEVEL) set_tests_properties(pureboot.reloc PROPERTIES TIMEOUT 180 ENVIRONMENT "PB_OBJCOPY=${CMAKE_OBJCOPY}") + # Entering the loader from a running application with no reset + # between, over a page buffer the application dirtied — the case the + # loader declines to guard and the host repairs. Hardware forbids the + # state here (SPM runs only from the boot section); simavr does not, + # which is what makes it constructible. + if(LIBAVR_MCU STREQUAL "atmega328p") + add_test(NAME pureboot.dirty + COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test/pbdirty.py + ${PB_DEVICE} $ ${PUREBOOT_SIM_MCU} ${_pb_stock_hz} + ${PUREBOOT_BASE_HEX} ${PUREBOOT_PAGE} ${_pb_stock_baud} + $.bin + ${CMAKE_CURRENT_SOURCE_DIR}/pureboot/pureboot.py + ${CMAKE_BINARY_DIR}/pbdirty-work) + set_tests_properties(pureboot.dirty PROPERTIES TIMEOUT 180) + endif() + # Re-homing: a loader mistakenly programmed at address 0 (a raw .bin # handed to a programmer) or sitting in the staging slot must heal # into the canonical slot through the ordinary --update-loader flow. diff --git a/pureboot/CMakeLists.txt b/pureboot/CMakeLists.txt index 4231c4d..104d8d7 100644 --- a/pureboot/CMakeLists.txt +++ b/pureboot/CMakeLists.txt @@ -294,6 +294,15 @@ function(pureboot_add_loader name) add_executable(${name} ${CMAKE_CURRENT_FUNCTION_LIST_DIR}/pureboot.cpp) target_link_libraries(${name} PRIVATE libavr) target_compile_definitions(${name} PRIVATE ${_defines}) + # Codegen shaping for the loader TU only, worth ~40 B on every chip and + # what carries the far-flash 1284 build under 512. At -Os GCC otherwise + # rewrites the byte-stream loops' counters into end-pointer forms that + # cost registers (-fno-ivopts, -fno-split-wide-types), leaves register + # pressure on the table with the default allocator + # (-fira-algorithm=priority), and spends bytes on rewrites a + # straight-line loader gains nothing from. + target_compile_options(${name} PRIVATE + -fno-ivopts -fira-algorithm=priority -fno-expensive-optimizations -fno-split-wide-types) target_link_options(${name} PRIVATE -nostartfiles -Wl,--section-start=.text=${_base_hex} -Wl,--defsym=pureboot_app=${_app} ${_wrap}) add_custom_command(TARGET ${name} POST_BUILD COMMAND ${CMAKE_SIZE} $) diff --git a/pureboot/README.md b/pureboot/README.md index d1d8b21..cdf0b16 100644 --- a/pureboot/README.md +++ b/pureboot/README.md @@ -2,19 +2,22 @@ A serial bootloader on [libavr](https://git.blackmark.me/avr/libavr), pure by constraint: one C++ source, no inline assembly, no global register variables -(attributes allowed), built for **every chip libavr targets — all 37 — -fitting each chip's smallest boot sector**: 512 bytes everywhere — 470 B on -the tiny13s, ~484 B on the tiny25/45/85, 458–506 B across the megas and -every configured variant of them (the m8's software-serial build is the -fattest) — except the ATmega1284/1284P, whose smallest boot sector is 1 KiB -and whose far-flash machinery (ELPM reads, RAMPZ page commands, -word-addressed wire) lands at 556–562 B in a 1 KiB slot: the 512-byte -figure is a hardware boundary those chips simply do not have, and no -implementation of this feature set fits it there. Clock, baud, serial -backend and pins are per-build configuration (below); the size matrix in -the test suite holds every combination inside its slot. The device speaks -primitives; every composite — verify, erase, reset-vector surgery, updating -the loader itself — lives in the host tool (`pureboot.py`). +(attributes and compiler flags allowed), built for **every chip libavr +targets — all 37 — in 512 bytes each**: 434 B on the tiny13s, 438–442 B on +the tiny25/45/85, 412–452 B across the megas and every configured variant of +them, and 506 B on the ATmega1284/1284P, whose far-flash machinery (ELPM +reads, RAMPZ page commands, word-addressed wire) is the heaviest. The 1284s +are the tight case: the loop-placement attributes on the byte streamers +(`pureboot.cpp`) and the codegen flags on the loader TU (`CMakeLists.txt`) +are what carry that build under the line. Clock, baud, serial backend and +pins are per-build configuration (below); the size matrix in the test suite +holds every combination inside its slot. The device speaks primitives; every +composite — verify, erase, reset-vector surgery, updating the loader itself — +lives in the host tool (`pureboot.py`). + +The 1284s still *deploy* in a 1 KiB slot, their smallest boot sector being +512 words; at 506 B the image would also fit the 644's +two-512-byte-slots-per-boot-sector geometry. The image is **position-independent**: control flow is PC-relative, the read/write paths take wire addresses, the write guard protects the slot the @@ -133,7 +136,20 @@ byte-addressed). EEPROM addresses are always bytes, counts always bytes. then erases and programs; the address must be page-aligned. Pages inside the 512-byte slot the loader is *running* in are drained but never programmed — a broken host cannot brick the running copy, and a staged copy may rewrite the -resident slot. `w` is host-paced: send the next byte only after the previous +resident slot. + +The loader never clears the SPM buffer before a fill, so **one `W` may +program the wrong bytes, and the host is what fixes it**. The buffer is +write-once per word until cleared, and two things leave words in it: a +refused page (drained, never programmed) and — where SPM runs from anywhere, +the tinies and the m48s — an application that self-programmed before +entering. The next `W` takes those stale words, and clears them: a page write +auto-erases the buffer (§26.2.1; §19.2 on the tinies), so repeating it +programs correctly. The host therefore verifies every page it writes and +rewrites what comes back wrong (three retries, then it stops); a host that +programs without reading back cannot trust the first `W` after either event. + +`w` is host-paced: send the next byte only after the previous byte's `+`. `F` returns the bytes in the hardware's Z order; on a chip without an extended fuse byte (the ATtiny13A) that slot carries no meaning. Fuse *writing* does not exist: SPM reaches flash (and, on the mega, lock @@ -283,8 +299,11 @@ update, flash (erase / program / read / verify), EEPROM (erase / program / read / verify) — then the loader hands over to the application; `--stay` keeps the session alive instead, and a later invocation reconnects into it (the knock converges there too). `--flash` and `--eeprom` verify by -read-back unless `--no-verify`; images are raw binary, or Intel HEX by -extension. `--force` overrides the refusable safety checks (today: flashing +read-back unless `--no-verify`, and a flash page that reads back wrong is +rewritten up to three times before the run stops — the loader leaves one +recoverable way for a page to land wrong (see `W` above), and rewriting is +what clears it. `--verify-flash` only reports. Images are raw binary, or +Intel HEX by extension. `--force` overrides the refusable safety checks (today: flashing application data into a mega's reset walk region). Readouts come one fact per line: `--info` prints the decoded info block @@ -320,8 +339,10 @@ regenerates the presets). Per chip preset, `ctest` runs: in the image, the info block within its first 256 bytes; - `pureboot.planner` — the host tool's pure logic: programming orders and their recovery properties, the surgery, the staging composition, the - boot-fuse decode, and the update preflight's error/warning matrix over - synthetic fuse bytes; + boot-fuse decode, the update preflight's error/warning matrix over + synthetic fuse bytes, and the repairing verify against a fake device — one + bad write repaired in a single rewrite, a page that never comes good + stopping after exactly three; - `pureboot.protocol` — end to end against a simavr device (`test/pureboot_device.c` — a hardware USART as a pty, or a cycle-timed GPIO⇄pty bridge for a software-UART build, selected with `-l` to match @@ -335,6 +356,14 @@ regenerates the presets). Per chip preset, `ctest` runs: - `pureboot.reloc` — the identical image installed one slot below the resident serves the complete command set from there (the position-independence acceptance test); +- `pureboot.dirty` (328P) — entering the loader from a running application + with no reset between, over an SPM page buffer the fixture deliberately + dirtied: the case the loader declines to guard against. A bare verify must + see the corruption, the repairing verify must fix it in one rewrite, and a + plain verify afterwards must pass. On the boot-sectioned megas hardware + forbids the state outright (SPM runs only from the boot section, and reset + erases the buffer), but simavr dispatches SPM from anywhere — which is what + makes the path constructible at all; - `pureboot.update` — the full `--update-loader` flow to a re-timed build, then every power-fail phase: the device is killed mid-write, restarted from its flash dump, and a re-run must complete the update with the diff --git a/pureboot/pureboot.cpp b/pureboot/pureboot.cpp index 42f63df..ff5d287 100644 --- a/pureboot/pureboot.cpp +++ b/pureboot/pureboot.cpp @@ -67,13 +67,11 @@ consteval std::int16_t wdrf_field() // without a hardware boot section — the tinies and the m48s, whose SPM // runs from anywhere (Atmel-8271 §26). A boot section also means the CPU // runs on while the RWW section programs; everywhere else it halts through -// the operation. The m48s still carry RWWSRE as their temporary-buffer -// discard (§26.2), so the discard picks by that bit, not by the section. +// the operation. constexpr std::uint16_t slot_bytes = spm::flash_bytes > 65536 ? 1024 : 512; constexpr std::uint32_t base = spm::flash_bytes - slot_bytes; constexpr std::uint16_t page = spm::page_bytes; constexpr bool boot_section = avr::hw::curated::has_boot_section(); -constexpr bool rww_discard = spm::detail::has_rww(); // Past 64 KiB a byte address no longer fits the wire's 16 bits, so on the // large chips every flash address on the wire — and all slot arithmetic — @@ -94,11 +92,11 @@ constexpr std::uint16_t wire_page_mask = word_flash ? (page / 2 - 1) : (page - 1 #endif constexpr std::uint8_t timeout_seconds = PUREBOOT_TIMEOUT; -// The 12-byte info block the host reads with the 'b' command; flash-resident -// (there is no crt to copy a .data image), word-aligned so its wire (word) -// address is exact on the large chips. The page byte is the wire count -// convention: 0 means 256. -[[gnu::progmem]] alignas(2) inline constexpr std::array info_data = { +// The 12-byte info block the host reads with the 'b' command, flash-resident +// through flash_table (there is no crt to copy a .data image, and its storage +// carries the word alignment 'b' needs to halve the address on the large +// chips). The page byte is the wire count convention: 0 means 256. +inline constexpr avr::flash_table{ 'P', 'B', 1, // magic, protocol version @@ -113,7 +111,8 @@ constexpr std::uint8_t timeout_seconds = PUREBOOT_TIMEOUT; // bit 0: host must patch the reset vector (no hardware boot section); // bit 1: flash wire addresses are word addresses static_cast((boot_section ? 0 : 1) | (word_flash ? 2 : 0)), -}; +}> + info_data; // The serial link. PUREBOOT_USART forces a hardware USART instance, // PUREBOOT_SOFT_SERIAL the polled software UART (no vector — the table @@ -139,16 +138,6 @@ constexpr char usart_digit = '0' + PUREBOOT_USART; constexpr char usart_digit = '0'; #endif -// Whether the chip carries the selected USART: the suffixed instance name, -// or — for instance 0 — the classic megas' un-numbered block. -consteval bool usart_exists() -{ - const char name[] = {'U', 'S', 'A', 'R', 'T', usart_digit}; - if (avr::hw::db.has_instance(std::string_view{name, sizeof(name)})) - return true; - return usart_digit == '0' && avr::hw::db.has_instance("USART"); -} - template struct hardware_link { using uart = avr::uart::usart; @@ -219,12 +208,13 @@ struct software_link { }; #if defined(PUREBOOT_USART) -static_assert(usart_exists(), "PUREBOOT_USART selects a hardware USART this chip does not have"); +static_assert(avr::uart::has_usart(), "PUREBOOT_USART selects a hardware USART this chip does not have"); using link = hardware_link; #elif defined(PUREBOOT_SOFT_SERIAL) using link = software_link; #else -using link = std::conditional_t, software_link>; +using link = + std::conditional_t(), hardware_link, software_link>; #endif // The application's entry, an absolute address the linker pins (--defsym in @@ -274,36 +264,51 @@ std::uint8_t rx_deadline() return link::rx(); } -std::uint16_t rx16() +// Inlined into its call sites: reading two bytes across a call otherwise +// strands the first in a call-saved register the caller must push/pop; folded +// into the (noreturn) command loop that cost disappears. +[[gnu::always_inline]] inline std::uint16_t rx16() { std::uint16_t low = link::rx(); return static_cast(low | (link::rx() << 8)); } // The streamers take the count in the wire's 8-bit form: 0 means 256. -// send_flash stays out of line: its two callers ('b' and 'R') otherwise each -// inline a private copy of the loop. On the large chips the address is a -// word address and the read goes through ELPM (flash_load_far). -[[gnu::noinline]] void send_flash(std::uint16_t address, std::uint8_t count) +// +// Two functions, because they want opposite placement and placement is an +// attribute: the byte-addressed loop is small enough to inline into both +// callers, the word-addressed one stays out of line but flattened — a call to +// the transmit inside it would strand the 24-bit cursor in callee-saved +// registers. `word_flash` picks at the call site. +[[maybe_unused, gnu::always_inline]] inline void send_flash_near(std::uint16_t address, std::uint8_t count) { - if constexpr (word_flash) { - // The 24-bit cursor as the machine holds it: the RAMPZ byte and a - // 16-bit Z, carried explicitly (the reassembled 32-bit address - // folds away inside the inlined far load). A single read never - // crosses a 64 KiB boundary — the protocol forbids it and the host - // splits its chunks there — so RAMPZ holds for the whole run. - std::uint8_t rampz = static_cast(address >> 15); - std::uint16_t z = static_cast(address << 1); - do { - link::tx(avr::flash_load_far((static_cast(rampz) << 16) | z)); - if (++z == 0) - ++rampz; // robustness for a host that reads across 64 KiB - } while (--count); - } else { - do - link::tx(avr::flash_load(reinterpret_cast(address++))); - while (--count); - } + do + link::tx(avr::flash_load(reinterpret_cast(address++))); + while (--count); +} + +// The 24-bit cursor as the machine holds it: the RAMPZ byte and a 16-bit Z, +// carried explicitly (the reassembled 32-bit address folds away inside the +// inlined far load). +[[maybe_unused, gnu::flatten, gnu::noinline]] void send_flash_far(std::uint16_t address, std::uint8_t count) +{ + std::uint8_t rampz = static_cast(address >> 15); + std::uint16_t z = static_cast(address << 1); + do { + link::tx(avr::flash_load_far((static_cast(rampz) << 16) | z)); + // The protocol never reads across 64 KiB, but carrying the wrap is + // smaller than the flat 32-bit cursor GCC builds without it. + if (++z == 0) + ++rampz; + } while (--count); +} + +[[gnu::always_inline]] inline void send_flash(std::uint16_t address, std::uint8_t count) +{ + if constexpr (word_flash) + send_flash_far(address, count); + else + send_flash_near(address, count); } void send_eeprom(std::uint16_t address, std::uint8_t count) @@ -331,19 +336,15 @@ void store_eeprom(std::uint16_t address, std::uint8_t count) // itself. `slot_high` is the high byte of that running slot's base (run() // derives it); a broken host thus cannot brick the running loader, and a // copy flashed one slot lower may rewrite the slot above it — how pureboot -// updates itself. On the mega the RWW section is re-enabled so reads work -// immediately. +// updates itself. void program_flash(std::uint16_t wire_address, std::uint8_t slot_high) { - // A buffer word cannot be loaded twice without an erase (§26.2.1), so a - // refused page's drained data must not linger for the next write: - // discard the buffer up front — CTPB on the tinies; on the megas - // writing RWWSRE aborts a pending load (§26.2.2 — on the m48s that - // flush is the bit's whole documented job). - if constexpr (rww_discard) - spm::rww_enable(); - else - spm::clear_buffer(); + // No discard before the fill: the buffer is write-once per word + // (§26.2.1), so filling over one a refused page or an application left + // dirty programs stale words — but a page write auto-erases the buffer + // (§26.2.1; §19.2 on the tinies), so that write clears the condition and + // the host's read-back rewrites the page. + // One induction either way. On the byte-addressed chips the wire address // itself walks the page (aligned, so the offset bits wrap to zero); on // the word-addressed large chips the wire word address becomes a 32-bit @@ -389,11 +390,14 @@ void program_flash(std::uint16_t wire_address, std::uint8_t slot_high) if constexpr (boot_section) spm::wait(); spm::write_page(address); - if constexpr (boot_section) { + if constexpr (boot_section) spm::wait(); - spm::rww_enable(); - } } + // The megas program with their RWW section disabled; reads need it back + // on. The same store discards the buffer (§26.2.2), so they never meet + // the stale-word case above. boot_section implies an RWW section. + if constexpr (boot_section) + spm::rww_enable(); } // The four fuse/lock bytes in the hardware's own Z order: low, lock, @@ -449,7 +453,7 @@ void send_fuses() // runtime address is the running slot's. Composed from the two // bytes — the high half is runtime data, so no absolute address // is ever materialized. - const auto link_low = reinterpret_cast(info_data.data()); + const auto link_low = reinterpret_cast(info_data.storage.data()); const std::uint8_t low = word_flash ? static_cast(link_low >> 1) : static_cast(link_low); send_flash(static_cast(low | (slot_high << 8)), static_cast(info_data.size())); diff --git a/pureboot/pureboot.py b/pureboot/pureboot.py index 59835b6..4c62c32 100644 --- a/pureboot/pureboot.py +++ b/pureboot/pureboot.py @@ -37,6 +37,7 @@ else: PROMPT = b"+" PROTOCOL_VERSION = 1 SLOT = 512 # the loader slot on byte-addressed chips; word-addressed ones (>64 KiB) use 1 KiB — their own smallest boot sector +RETRIES = 3 # rewrites of a page that reads back wrong, before the run stops VERBOSE = False @@ -767,9 +768,26 @@ def write_differing(loader, base, content, order=None, label=None): bar.step() if label: verbose(f"{label}: {written} of {len(offsets)} pages differed") - for at in range(0, len(content), 256): - if loader.read_flash(base + at, min(256, len(content) - at)) != content[at : at + 256]: - raise Error(f"verify failed at {base + at:#06x} after programming") + # Page-wise read-back with the same bounded repair as verify_pages: this + # is the loader-update path, where a page left wrong is a half-written + # loader slot. + for retry in range(RETRIES + 1): + bad = [ + offset + for offset in range(0, len(content), page) + if loader.read_flash(base + offset, len(content[offset : offset + page])) != content[offset : offset + page] + ] + if not bad: + break + if retry == RETRIES: + raise Error( + f"verify failed at {base + bad[0]:#06x} after programming " + f"(still wrong after {RETRIES} retries)" + ) + for offset in bad: + verbose(f"rewriting page {base + offset:#06x} (retry {retry + 1})") + loader.write_page(base + offset, content[offset : offset + page]) + written += 1 return written @@ -920,21 +938,37 @@ def op_flash(loader, path, erase, verify, fuse_bytes=None, force=False): bar.step() print(f"flash: {path}: {len(order)} pages") if verify: - verify_pages(loader, pages) + verify_pages(loader, pages, repair=True) -def verify_pages(loader, pages): +def verify_pages(loader, pages, repair=False): + """Read every page back and compare. With `repair`, a mismatched page is + rewritten and re-read, up to RETRIES times before it is raised: a page + filled over a dirty SPM buffer takes stale words, and the write that took + them cleared the buffer, so one rewrite settles it. Anything still wrong + after three is not that, and stops the run.""" + repaired = 0 with Progress("verify", len(pages)) as bar: for address in sorted(pages): - got = loader.read_flash(address, loader.info.page) - if got != pages[address]: + for retry in range(RETRIES + 1): + got = loader.read_flash(address, loader.info.page) + if got == pages[address]: + break first = next(i for i in range(len(got)) if got[i] != pages[address][i]) - raise Error( + detail = ( f"verify failed at {address + first:#06x}: " f"wrote {pages[address][first]:02x}, read {got[first]:02x}" ) + if not repair: + raise Error(detail) + if retry == RETRIES: + raise Error(f"{detail} (still wrong after {RETRIES} retries)") + verbose(f"{detail} — rewriting page {address:#06x} (retry {retry + 1})") + loader.write_page(address, pages[address]) + repaired += 1 bar.step() - print(f"verify: {len(pages)} pages ok") + note = f", {repaired} page rewrite(s)" if repaired else "" + print(f"verify: {len(pages)} pages ok{note}") def op_verify_flash(loader, path): diff --git a/test/check_pi.py b/test/check_pi.py index 40a44c0..544f286 100644 --- a/test/check_pi.py +++ b/test/check_pi.py @@ -37,11 +37,12 @@ def main(): sys.exit(1) symbols = subprocess.run([nm, "-C", elf], capture_output=True, text=True, check=True).stdout - info = [line for line in symbols.splitlines() if "info_data" in line] + info = [line for line in symbols.splitlines() if "flash_table" in line and "::storage" in line] if len(info) != 1: print(f"FAIL: expected one info-block storage symbol, found {len(info)}") sys.exit(1) - offset = int(info[0].split()[0], 16) - text_start + address = int(info[0].split()[0], 16) + offset = address - text_start if not 0 <= offset < 256: print(f"FAIL: info block at image offset {offset:#x}, must sit in the first 256 bytes") sys.exit(1) diff --git a/test/pbapp.cpp b/test/pbapp.cpp index 474a33e..02b4ddf 100644 --- a/test/pbapp.cpp +++ b/test/pbapp.cpp @@ -69,9 +69,18 @@ struct link { // 'L' hands back to the loader at the top slot — 512 bytes, or the // 1 KiB the >64 KiB chips use. constexpr std::uint32_t slot = avr::hw::db.mem.flash_size > 65536 ? 1024 : 512; - for (;;) - if (tx_t::read_blocking() == 'L') + for (;;) { + auto command = tx_t::read_blocking(); + if (command == 'L') reinterpret_cast(static_cast((avr::hw::db.mem.flash_size - slot) / 2))(); + // 'D' leaves every word of the SPM page buffer dirty, so that a + // following 'L' enters the loader with the buffer it never clears. + if (command == 'D') { + for (std::uint16_t at = 0; at < avr::spm::page_bytes; at += 2) + avr::spm::fill(at, 0xdead); + tx('D'); + } + } } }; diff --git a/test/pbdirty.py b/test/pbdirty.py new file mode 100644 index 0000000..3b1a295 --- /dev/null +++ b/test/pbdirty.py @@ -0,0 +1,90 @@ +#!/usr/bin/env python3 +"""Dirty-page-buffer acceptance test: the loader carries no buffer discard, +so a page filled over words an earlier writer left behind programs those +instead. This asserts the whole contract — the corruption is real and a bare +verify sees it, the repairing verify fixes it in one rewrite (the write that +took the stale words auto-erased the buffer), and it stays fixed. + +The state is reached the way the loader cannot prevent: an application +dirties the buffer and jumps in with no reset between. Real boot-sectioned +megas forbid that outright — SPM executes only from the boot section +(Atmel-8271 §26.2) — but simavr dispatches SPM from anywhere, which is what +makes the path constructible at all. + +Usage: pbdirty.py + +""" + +import os +import sys + + +def fail(message): + print(f"FAIL: {message}") + sys.exit(1) + + +def main(): + device_bin, elf, mcu, hz, base_hex, page, baud, app_bin, tool, workdir = sys.argv[1:] + page, baud = int(page), int(baud) + sys.path.insert(0, os.path.dirname(os.path.abspath(tool))) + sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + import pbsim + import pureboot as pb + + os.makedirs(workdir, exist_ok=True) + dump = os.path.join(workdir, "dump.bin") + + # Reset boots the application on a BOOTRST-unprogrammed mega; its 'L' is + # the loader entry this test needs, reached without a reset. + device = pbsim.Device(device_bin, elf, mcu, hz, base_hex, page, baud, dump, reset_hex="0") + try: + port = pb.Port(device.pty, baud) + loader = pb.Loader(port) + loader.connect(25) + + # Install the application and hand over to it. + pb.op_flash(loader, app_bin, erase=False, verify=True) + loader.run_application() + if port.read_exact(3, 5.0) != b"APP": + fail("the application did not start") + + port.write(b"D") + if port.read_exact(1, 5.0) != b"D": + fail("the application did not acknowledge dirtying the page buffer") + port.write(b"L") + loader = pb.Loader(port) + loader.connect(25) + + # Program by hand, so the corruption is observable before anything + # repairs it. + pages = pb.plan_flash(open(app_bin, "rb").read(), loader.info) + for address in sorted(pages): + loader.write_page(address, pages[address]) + try: + pb.verify_pages(loader, pages) + except pb.Error as error: + if "verify failed" not in str(error): + fail(f"the read-back failed, but not at verify: {error}") + else: + # Either the fixture no longer dirties the buffer, or the loader + # clears it again — in which case this test's premise is gone. + fail("programming over a dirty page buffer came back clean") + + # What the programming path uses: one rewrite settles it, and it stays + # settled. + pb.verify_pages(loader, pages, repair=True) + pb.verify_pages(loader, pages) + + # Ground truth beyond the loader's own read-back. + loader.run_application() + if port.read_exact(3, 5.0) != b"APP": + fail("the application did not start after the recovered write") + port.close() + finally: + device.stop() + print("pbdirty: a dirty page buffer is caught by verify and cleared by the retry") + + +if __name__ == "__main__": + main() diff --git a/test/pbreloc.py b/test/pbreloc.py index c1fb2be..89e85b8 100644 --- a/test/pbreloc.py +++ b/test/pbreloc.py @@ -66,8 +66,10 @@ def main(): if loader.read_eeprom(0, len(pattern)) != pattern: fail("EEPROM round-trip through the staged copy") - # The guard, both ways: its own slot refused (drained, unchanged), - # the resident slot writable. + # The guard, both ways: its own slot refused (drained, unchanged), the + # resident slot writable. The refusal leaves its drained words in the + # SPM buffer, so the write that follows may take them — and clears + # them by writing, so the retry must not. before = loader.read_flash(stage, page) loader.write_page(stage, bytes(page)) if loader.read_flash(stage, page) != before: @@ -75,7 +77,9 @@ def main(): marker = bytes((i * 3) & 0xFF for i in range(page)) loader.write_page(base, marker) if loader.read_flash(base, page) != marker: - fail("the staged copy could not write the resident slot") + loader.write_page(base, marker) + if loader.read_flash(base, page) != marker: + fail("the staged copy could not write the resident slot, even on retry") # Restore the resident image through the staged copy, then 'J' back # into it and prove it lives. diff --git a/test/test_planner.py b/test/test_planner.py index e7abd41..9e979de 100644 --- a/test/test_planner.py +++ b/test/test_planner.py @@ -204,6 +204,52 @@ def main(): pb.check_walk_region({0x7800: bytes((0xFF,)) * 128}, mega, fuses(0xFA), False) pb.check_walk_region(deep, mega, None, False) # fuses unknown: no check + # The repairing verify: a mismatched page is rewritten rather than raised, + # bounded so a fault that is not self-clearing cannot spin. + class FakeLoader: + """A device whose first `bad` writes of any page land wrong.""" + + def __init__(self, info, bad): + self.info = info + self.bad = bad + self.flash = {} + self.writes = 0 + + def write_page(self, address, data): + self.writes += 1 + self.flash[address] = bytes(len(data)) if self.bad > 0 else bytes(data) + self.bad -= 1 + + def read_flash(self, address, count): + return self.flash.get(address, bytes(count)) + + want = {0: bytes((i * 5) & 0xFF for i in range(128))} + + # One bad write, then good: repaired in place, and the caller never sees + # an error. The rewrite is counted, so a silent no-op cannot pass. + device = FakeLoader(info_of(pb, 0x7E00, 128, False, 0x8000), bad=1) + device.write_page(0, want[0]) + pb.verify_pages(device, want, repair=True) + if device.writes != 2: + fail(f"repairing verify made {device.writes} writes, expected 2") + + # Without repair the same state raises, so the repair is what fixed it. + device = FakeLoader(info_of(pb, 0x7E00, 128, False, 0x8000), bad=1) + device.write_page(0, want[0]) + expect_error("verify without repair", lambda: pb.verify_pages(device, want), "verify failed") + + # A page that never comes good stops after RETRIES rewrites, and says so. + device = FakeLoader(info_of(pb, 0x7E00, 128, False, 0x8000), bad=99) + device.write_page(0, want[0]) + expect_error( + "unrepairable page", + lambda: pb.verify_pages(device, want, repair=True), + "verify failed", + f"after {pb.RETRIES} retries", + ) + if device.writes != pb.RETRIES + 1: + fail(f"unrepairable page took {device.writes} writes, expected {pb.RETRIES + 1}") + print("test_planner: all planner and policy checks pass")