From c01e583597173a064f7e89db0de0fbc5c2d3df41 Mon Sep 17 00:00:00 2001 From: BlackMark Date: Fri, 31 Jul 2026 01:58:09 +0200 Subject: [PATCH] console: the terminal is the original's again, and the way out is a jump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six things the port had dropped or got wrong, and the one that matters is the last. The help is a table again — name, dots, description, one command per line — instead of a single line of bare words that said nothing about what any of them did. The layout is the original's, colons at column 12, which `bootloader` at ten characters is what sets. Abbreviations are back, and they were a feature: any prefix resolves to the first command it matches, so `up` is uptime and `st` is statistics. Order does the disambiguating, which is why the table is in the original's dispatch order and new entries go on the end — appending cannot take an abbreviation that already meant something. `reset` keeps the original's exception and must be typed in full: `r` should not be able to clear the histogram. The histogram gets its resolution back. The bar was capped at 40 columns where the original scaled to 100, and on a distribution this narrow that threw away most of the difference between neighbouring buckets. Same normalisation as before: divide by whatever makes the tallest bucket fit. The sample count moves to a fixed ten-column field before the bar, so the numbers read as a table instead of trailing off the ragged right end. `version` exists again, and this is 2.1 — 2.0 being the port as it stood. Added while here: `save`, to force the writeback that otherwise waits up to thirty minutes; the resistance in `show`, which is the one number that says *why* a temperature is wrong and which the original printed; a report when a line overflows the buffer rather than silently acting on its head; "no data yet" where there is none; and a blank line after each command's output. And the way out. `bootloader` now jumps rather than resetting, because pureboot hands straight back on WDRF by design — so the legacy watchdog-reset hand-over reaches it and opens no window, which on a board with no reset line is a board that cannot be reflashed. Two more bugs in the same three lines: the target was 0x7800, a 2 KB boot section's base, which on this board's 512-byte section reads erased and made the check false and the command a no-op; and UCSR0B was left set, which mutes a loader that bit-bangs the pin the USART still owns. All three are now read back out of the emitted image by ctest, the address and the watchdog red-proven against exactly the legacy behaviour they exist to catch. libavr advances to 71cfb2f. Verified on the board: FanTemp v2.1, min 0 C / max 74 C matching what 1.8b reported off the same EEPROM, the fan curve within one percentage point of the legacy double-precision one at every 5 C from 15 to 60, and `bootloader` -> pureboot 7 -> back to a running application. Co-Authored-By: Claude Opus 5 --- CMakeLists.txt | 12 +- CMakePresets.json | 102 ++++++++------ README.md | 39 +++++- libavr | 2 +- src/bootloader.hpp | 75 ++++++++-- src/terminal.hpp | 280 ++++++++++++++++++++++++++++++------- test/CMakeLists.txt | 12 ++ test/check_reachability.py | 132 +++++++++++++++++ 8 files changed, 547 insertions(+), 107 deletions(-) create mode 100644 test/CMakeLists.txt create mode 100644 test/check_reachability.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 3a9bbaa..eb6fc15 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -18,4 +18,14 @@ add_subdirectory(${LIBAVR_ROOT} libavr-build) add_executable(fantemp src/main.cpp) target_link_libraries(fantemp PRIVATE libavr) -add_custom_command(TARGET fantemp POST_BUILD COMMAND ${CMAKE_SIZE} $) +# The raw image is what the loader takes, and what the reachability check +# measures the boot-section clearance against. +add_custom_command(TARGET fantemp POST_BUILD + COMMAND ${CMAKE_SIZE} $ + COMMAND ${CMAKE_OBJCOPY} -O binary -R .eeprom + $ $/fantemp.bin + COMMAND ${CMAKE_OBJCOPY} -O ihex -R .eeprom + $ $/fantemp.hex) + +enable_testing() +add_subdirectory(test) diff --git a/CMakePresets.json b/CMakePresets.json index 67dce93..9b8ff0c 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -1,43 +1,59 @@ -{ - "version": 8, - "configurePresets": [ - { - "name": "base", - "hidden": true, - "generator": "Ninja", - "binaryDir": "${sourceDir}/build/${presetName}", - "toolchainFile": "${sourceDir}/libavr/cmake/avr-toolchain.cmake", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", - "CMAKE_COLOR_DIAGNOSTICS": "ON" - } - }, - { - "name": "atmega328p-generated", - "inherits": "base", - "cacheVariables": { - "LIBAVR_MCU": "atmega328p", - "LIBAVR_REFLECT": "OFF" - } - }, - { - "name": "atmega328p-reflect", - "inherits": "base", - "cacheVariables": { - "LIBAVR_MCU": "atmega328p", - "LIBAVR_REFLECT": "ON" - } - } - ], - "buildPresets": [ - { - "name": "atmega328p-generated", - "configurePreset": "atmega328p-generated" - }, - { - "name": "atmega328p-reflect", - "configurePreset": "atmega328p-reflect" - } - ] -} \ No newline at end of file +{ + "version": 8, + "configurePresets": [ + { + "name": "base", + "hidden": true, + "generator": "Ninja", + "binaryDir": "${sourceDir}/build/${presetName}", + "toolchainFile": "${sourceDir}/libavr/cmake/avr-toolchain.cmake", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON", + "CMAKE_COLOR_DIAGNOSTICS": "ON" + } + }, + { + "name": "atmega328p-generated", + "inherits": "base", + "cacheVariables": { + "LIBAVR_MCU": "atmega328p", + "LIBAVR_REFLECT": "OFF" + } + }, + { + "name": "atmega328p-reflect", + "inherits": "base", + "cacheVariables": { + "LIBAVR_MCU": "atmega328p", + "LIBAVR_REFLECT": "ON" + } + } + ], + "buildPresets": [ + { + "name": "atmega328p-generated", + "configurePreset": "atmega328p-generated" + }, + { + "name": "atmega328p-reflect", + "configurePreset": "atmega328p-reflect" + } + ], + "testPresets": [ + { + "name": "atmega328p-generated", + "configurePreset": "atmega328p-generated", + "output": { + "outputOnFailure": true + } + }, + { + "name": "atmega328p-reflect", + "configurePreset": "atmega328p-reflect", + "output": { + "outputOnFailure": true + } + } + ] +} diff --git a/README.md b/README.md index 9d8ab2b..5164a64 100644 --- a/README.md +++ b/README.md @@ -1,11 +1,44 @@ # fantemp -Temperature-controlled fan firmware (ATmega328P, 16 MHz), rewritten on +**v2.1.** Temperature-controlled fan firmware (ATmega328P, 16 MHz), rewritten on [libavr](https://git.blackmark.me/avr/libavr): thermistor on ADC0 sampled free-running and averaged over 1000 conversions, fan on OC0B at 50 kHz, 115200 Bd serial console (`help` lists the commands), temperature -histogram persisted to EEPROM, watchdog-reset path into a boot-section -bootloader. +histogram persisted to EEPROM, and a direct jump into a boot-section +bootloader at `0x7e00`. + +The EEPROM format is the legacy firmware's, unchanged: 100 little-endian +`uint32` buckets at address 0, one per °C. A board carrying years of history +from FanTemp 1.8b keeps every count — verified on hardware, all 67 non-empty +buckets byte-identical across the conversion. + +## The console + +Commands may be abbreviated to any unambiguous-by-order prefix, as the legacy +firmware allowed: `up` is `uptime`, `st` is `statistics`, `sa` is `save`. The +table order resolves ties, so `s` is `show` — and `reset` is deliberately the one +command that cannot be abbreviated, because `r` should not be able to wipe the +histogram. `save` (new) forces a writeback, which otherwise happens every 30 +minutes and on the way into the bootloader. + +## Reaching the bootloader + +`bootloader` **jumps**; it does not reset. That is not a style choice: + +- **pureboot hands straight back on WDRF**, by design — an unattended board that + watchdog-resets in a loop must not sit in a loader. So the legacy + watchdog-reset hand-over arrives and opens no window at all, and on a board + with no reset line that is a board that cannot be reflashed. +- The address is `0x7e00`, the top 512 bytes. The legacy firmware used `0x7800`, + a 2 KB boot section's base, which on a board with a 512-byte boot section reads + erased — so its `bootloader` command silently never arrived anywhere. +- `UCSR0B` is cleared first. While `TXEN0` is set the USART owns PD1, so a loader + that bit-bangs the same pin receives perfectly and answers into nothing. + +`ctest` reads all three back out of the emitted image (`test/check_reachability.py`), +because none of them is visible from the source alone and the failure mode is an +unreflashable board. Both the address and the watchdog checks are red-proven +against the legacy behaviour they exist to catch. The Steinhart–Hart math of the legacy firmware (runtime doubles + libm log) is gone: the Beta equation and the cubic fan curve are evaluated diff --git a/libavr b/libavr index c21ed31..71cfb2f 160000 --- a/libavr +++ b/libavr @@ -1 +1 @@ -Subproject commit c21ed3171efd9081a77194a1f3c96d0e62202984 +Subproject commit 71cfb2f1aeab19b0ef6f4bd5fe6285bec987c932 diff --git a/src/bootloader.hpp b/src/bootloader.hpp index 7cbb4c5..4a73131 100644 --- a/src/bootloader.hpp +++ b/src/bootloader.hpp @@ -6,10 +6,24 @@ #include "board.hpp" -// Reset-into-bootloader: `bootloader` on the console arms the watchdog -// and hangs; the next boot sees WDRF and jumps to the boot section at -// 0x7800 (byte address) — if one is flashed there — before anything else -// runs. +// Reaching the resident bootloader from the console. +// +// The legacy firmware did this with a watchdog reset: `bootloader` armed the +// watchdog and hung, and the next boot noticed WDRF and jumped to the boot +// section. That works for TinySafeBoot and **does not work for pureboot**, which +// deliberately hands straight back to the application on WDRF — an unattended +// board that watchdog-resets in a loop must not sit in a loader instead of +// running. So a reset-based route into pureboot opens no window at all, and on a +// board whose only way in is the firmware that is a lockout. +// +// This route therefore never resets. It jumps, with the reset flags already +// clear, so the loader starts as if from a clean power-on and opens its window. +// +// The address is this board's, and it is not the legacy one: the loader lives in +// the top 512 bytes at 0x7e00 (`hfuse d4` puts the boot section at 0x7c00 with +// pureboot's staging slot below its own slot). The legacy firmware probed 0x7800 +// — a 2 KB boot section's base — which on this board reads erased, so its check +// was always false and its `bootloader` command never actually arrived anywhere. namespace app { class bootloader { @@ -17,28 +31,65 @@ class bootloader { using guard = dev::watchdog<{.timeout = 16_ms}>; + // The top 512 bytes. An erased slot reads 0xffff, which is not an + // instruction any loader begins with — so this asks "is a loader installed" + // rather than "is it the one I expect", which is the check the legacy + // firmware got wrong in the other direction by testing one specific byte. + static constexpr std::uint16_t base = 0x7e00; + static bool present() { - return pgm_read_byte(0x7800) != 0xff; + return pgm_read_word(base) != 0xffff; + } + + // A function pointer holds a word address on AVR, so the byte address + // halves. [[gnu::noipa]] keeps the call indirect: a constant target folds + // into a relative call that cannot reach across flash. + [[gnu::noipa, noreturn]] static void call(jump_fn target) + { + target(); + __builtin_unreachable(); } public: - // Call first thing in main: reset_cause() clears MCUSR (a lingering - // WDRF would re-arm the watchdog), then a watchdog reset diverts into - // the bootloader when one is flashed. + // Call first thing in main. reset_cause() reads *and clears* MCUSR, which + // matters on its own: a lingering WDRF forces the watchdog back on at its + // shortest timeout. The diversion below is a leftover of the legacy route + // and is kept only because it is free and cannot hurt — with BOOTRST + // programmed the loader has already run before this line, so nothing + // normally reaches it. static void handle_reset() { auto cause = avr::power::reset_cause(); guard::disable(); if (cause.watchdog && present()) - reinterpret_cast(0x7800 / 2)(); + call(reinterpret_cast(base / 2)); } + // Hand over for real: no reset, so no WDRF for the loader to refuse. [[noreturn]] static void enter() { - guard::init(); - while (true) { - } + // Interrupts first — the receive vector and the timer live in this + // application's vector table, and once the loader is running there is no + // application to vector into. + avr::irq::disable(); + guard::disable(); + + // Release the USART. While TXEN0 is set the peripheral owns PD1, not the + // port register, so a loader that bit-bangs the same pin receives + // perfectly and answers into nothing — mute, not deaf, and unverifiable + // from the host. pureboot clears this itself; TinySafeBoot, which is what + // this board still carries, does not. Four bytes make the hand-over work + // for either one, which is the only reason this route can be tested + // before the loader is replaced. + avr::hw::ucsr0b::write(0); + + call(reinterpret_cast(base / 2)); + } + + static bool available() + { + return present(); } }; diff --git a/src/terminal.hpp b/src/terminal.hpp index 203600a..f6c0edb 100644 --- a/src/terminal.hpp +++ b/src/terminal.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -10,6 +11,7 @@ #include "controller.hpp" #include "curve.hpp" #include "statistics.hpp" +#include "thermistor.hpp" // The serial console: line-buffered commands over the hardware UART. // `help` lists everything; `monitor` streams until any key. @@ -19,19 +21,114 @@ class terminal { static constexpr std::uint8_t line_max = 24; static inline char line[line_max]{}; static inline std::uint8_t at = 0; + static inline bool overflowed = false; static inline bool monitoring = false; static inline std::uint64_t last_monitor = 0; + // Commands, in the order they are matched — which is the order the original + // firmware matched them in, and that order is load-bearing. An abbreviation + // resolves to the *first* entry it prefixes, so `s` is show (not statistics, + // not set) exactly as it always was, and anything appended to this list + // cannot steal an abbreviation that already meant something else. + struct command { + std::string_view name; + bool exact; // reset only: an abbreviation must not be able to wipe data + }; + + static constexpr std::array commands{{ + {"help", false}, + {"show", false}, + {"curve", false}, + {"monitor", false}, + {"bootloader", false}, + {"uptime", false}, + {"statistics", false}, + {"histogram", false}, + {"reset", true}, + {"set", false}, + {"auto", false}, + {"version", false}, + {"save", false}, + }}; + + // Column the descriptions' colons line up in, counted from the start of the + // name. The longest name is `bootloader` at 10, so 12 leaves it a space and + // one dot — the original's layout exactly. + static constexpr std::uint8_t help_column = 12; + static void prompt() { serial << "> "_P; } + // `name ....: ` — the dots are what make a dozen descriptions readable in a + // terminal, and they cost nothing but a loop. + static void help_row(std::string_view name) + { + serial << name << ' '; + for (auto i = name.size() + 1; i < help_column; ++i) + serial << '.'; + serial << ": "_P; + } + + static void help() + { + serial << "\r\nFanTemp "_P << version << " command overview\r\n"_P; + help_row(commands[0].name); + serial << "prints this help message\r\n"_P; + help_row(commands[1].name); + serial << "shows current temperature and fan speed\r\n"_P; + help_row(commands[2].name); + serial << "shows mapping from temperature to fan speed\r\n"_P; + help_row(commands[3].name); + serial << "loops the show command until a key is pressed\r\n"_P; + help_row(commands[4].name); + serial << "enters the bootloader\r\n"_P; + help_row(commands[5].name); + serial << "shows system uptime\r\n"_P; + help_row(commands[6].name); + serial << "prints overall statistics like min and max temp\r\n"_P; + help_row(commands[7].name); + serial << "prints a histogram of the temperature\r\n"_P; + help_row(commands[8].name); + serial << "resets statistics to 0 in EEPROM and RAM (no abbreviation)\r\n"_P; + help_row(commands[9].name); + serial << "sets the fan speed to the provided value, 0-100\r\n"_P; + help_row(commands[10].name); + serial << "turns on automatic fan control\r\n"_P; + help_row(commands[11].name); + serial << "displays firmware version\r\n"_P; + help_row(commands[12].name); + serial << "writes the statistics to EEPROM now\r\n"_P; + serial << "commands may be abbreviated: 'up' is uptime\r\n"_P; + } + + // The thermistor's resistance from the divider, in whole ohms. The original + // printed this beside the reading and it is the one number that says *why* a + // temperature is wrong: an open sensor rails the ADC and the resistance goes + // to the tens of megohms, a shorted one to zero. + static std::uint32_t resistance() + { + auto adc = controller::last_adc(); + if (adc >= 1023) + return 0xffffffff; // open circuit: the divider has no solution + return static_cast(thermistor::series_resistor) * adc / (1023u - adc); + } + static void show() { - serial << "temperature "_P << controller::temperature_quarters() / 4 << '.' - << (controller::temperature_quarters() % 4) * 25 << " C, adc "_P << controller::last_adc() << ", fan "_P - << controller::fan_percent() << " %, "_P; + if (!controller::data_available()) { + serial << "no data yet\r\n"_P; + return; + } + auto quarters = controller::temperature_quarters(); + serial << "temperature "_P << quarters / 4 << '.' << (quarters % 4) * 25 << " C, adc "_P + << controller::last_adc() << ", resistance "_P; + if (auto ohms = resistance(); ohms == 0xffffffff) + serial << "open"_P; + else + serial << ohms << " Ohm"_P; + serial << ", fan "_P << controller::fan_percent() << " %, "_P; if (controller::automatic()) serial << "auto"_P; else @@ -54,6 +151,10 @@ class terminal { static void print_statistics() { + if (statistics::total_samples() == 0) { + serial << "no data yet\r\n"_P; + return; + } serial << "min "_P << statistics::min_temperature() << " C, max "_P << statistics::max_temperature() << " C, samples "_P << static_cast(statistics::total_samples()) << "\r\n"_P; } @@ -62,77 +163,149 @@ class terminal { { auto highest = statistics::highest_bucket(); if (highest == 0) { - serial << "empty\r\n"_P; + serial << "no data yet\r\n"_P; return; } + // The original's normalisation, and its resolution: divide by whatever + // makes the tallest bucket fit in a hundred columns, not forty. A bar + // that tops out at 40 throws away most of the difference between + // neighbouring buckets, which on a distribution this narrow is the whole + // picture. + std::uint32_t factor = highest / bar_max > 1 ? highest / bar_max : 1; + while (highest / factor > bar_max) + ++factor; + for (std::uint8_t t = statistics::min_temperature(); t <= statistics::max_temperature(); ++t) { - serial << t << " C |"_P; - auto width = static_cast((statistics::bucket(t) * 40) / highest); - for (std::uint8_t i = 0; i < width; ++i) + auto count = statistics::bucket(t); + // Count first, in a fixed column, so the numbers read as a table + // instead of trailing off the ragged right-hand end of the bars. + serial << avr::dec<{.width = 2, .fill = '0'}>(t) << " C : "_P << avr::dec<{.width = 10, .fill = ' '}>(count) + << " |"_P; + for (std::uint32_t i = 0; i < count / factor; ++i) serial << '#'; - serial << ' ' << statistics::bucket(t) << "\r\n"_P; + serial << "\r\n"_P; } } - static void help() + // Abbreviations: the input matches a command when it is a non-empty prefix + // of it. `reset` is the exception and must be typed in full. + // + // starts_with, not substr: substr throws std::out_of_range, and one + // potentially-throwing call is enough to pull in std::terminate, which does + // not exist in a freestanding AVR build. The link fails rather than the + // firmware, so this is a build-time trap rather than a runtime one — but it + // is a trap, and the whole file avoids substr for that reason. + static bool matches(std::string_view input, const command &c) { - serial << "help show curve monitor uptime statistics histogram reset set <0-100> auto version bootloader\r\n"_P; + if (input.empty()) + return false; + if (c.exact) + return input == c.name; + return c.name.starts_with(input); } - static void dispatch(std::string_view cmd) + static void dispatch(std::string_view input) { - if (cmd.empty()) { - } else if (cmd == "help") { + // A line that overflowed the buffer is not a command — it is the tail of + // one. Acting on it is how a truncated `reset` becomes a surprise. + if (overflowed) { + serial << "input too long, ignored\r\n"_P; + overflowed = false; + return; + } + + // Split on the first space with the (pointer, length) constructor rather + // than substr, which throws — see matches(). + const auto space = input.find(' '); + const auto word = space == std::string_view::npos ? input : std::string_view{input.data(), space}; + const auto rest = space == std::string_view::npos + ? std::string_view{} + : std::string_view{input.data() + space + 1, input.size() - space - 1}; + if (word.empty()) + return; + + std::uint8_t which = commands.size(); + for (std::uint8_t i = 0; i < commands.size(); ++i) + if (matches(word, commands[i])) { + which = i; + break; + } + + switch (which) { + case 0: help(); - } else if (cmd == "show") { + return; + case 1: show(); - } else if (cmd == "curve") { + return; + case 2: print_curve(); - } else if (cmd == "monitor") { + return; + case 3: monitoring = true; - } else if (cmd == "uptime") { - print_uptime(); - } else if (cmd == "statistics") { - print_statistics(); - } else if (cmd == "histogram") { - print_histogram(); - } else if (cmd == "reset") { - statistics::reset(); - serial << "statistics cleared\r\n"_P; - } else if (cmd == "auto") { - controller::set_automatic(); - serial << "auto\r\n"_P; - } else if (cmd == "version") { - serial << "fantemp on libavr\r\n"_P; - } else if (cmd == "bootloader") { + return; + case 4: serial << "entering bootloader\r\n"_P; statistics::save(); + serial.drain(); bootloader::enter(); - } else if (cmd.starts_with("set ")) { - std::uint8_t percent = 0; - bool valid = cmd.size() > 4; - for (std::size_t i = 4; i < cmd.size(); ++i) { - if (cmd[i] < '0' || cmd[i] > '9') { + case 5: + print_uptime(); + return; + case 6: + print_statistics(); + return; + case 7: + print_histogram(); + return; + case 8: + statistics::reset(); + serial << "statistics cleared in EEPROM and RAM\r\n"_P; + return; + case 9: { + std::uint16_t percent = 0; + bool valid = !rest.empty(); + for (char c : rest) { + if (c < '0' || c > '9') { valid = false; break; } - percent = static_cast(percent * 10 + (cmd[i] - '0')); + percent = static_cast(percent * 10 + (c - '0')); + if (percent > 100) + valid = false; } - if (valid && percent <= 100) { - controller::set_manual(percent); - serial << "fan "_P << percent << " %\r\n"_P; + if (valid) { + controller::set_manual(static_cast(percent)); + serial << "fan "_P << percent << " %, manual\r\n"_P; } else { serial << "set 0..100\r\n"_P; } - } else { - serial << "? (help)\r\n"_P; + return; + } + case 10: + controller::set_automatic(); + serial << "automatic fan control\r\n"_P; + return; + case 11: + serial << "FanTemp "_P << version << " on libavr\r\n"_P; + return; + case 12: + statistics::save(); + serial << "statistics written to EEPROM\r\n"_P; + return; + default: + serial << '\'' << word << "' is not a command; 'help' for the list\r\n"_P; + return; } } public: + static constexpr std::string_view version = "v2.1"; + static constexpr std::uint8_t bar_max = 100; + static void init() { - serial << "\r\nfantemp on libavr — help for commands\r\n"_P; + serial << "\r\nFanTemp "_P << version << " on libavr -- 'help' for commands\r\n"_P; prompt(); } @@ -153,18 +326,31 @@ class terminal { char c = static_cast(*in); if (c == '\r' || c == '\n') { serial << "\r\n"_P; + if (at == 0 && !overflowed) { + prompt(); // a bare Enter just reprompts, no gap needed + continue; + } dispatch(std::string_view{line, at}); at = 0; - if (!monitoring) + if (!monitoring) { + // A blank line between a command's output and the next + // prompt: without it the answer and the thing you type + // next run together and a screen of them is unreadable. + serial << "\r\n"_P; prompt(); + } } else if (c == 0x7f || c == 0x08) { if (at) { --at; serial << "\b \b"_P; } - } else if (at < line_max && c >= ' ') { - line[at++] = c; - serial << c; // echo + } else if (c >= ' ') { + if (at < line_max) { + line[at++] = c; + serial << c; // echo + } else { + overflowed = true; // reported when the line is submitted + } } } } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt new file mode 100644 index 0000000..c89daf1 --- /dev/null +++ b/test/CMakeLists.txt @@ -0,0 +1,12 @@ +# The board has no reset line and no programming header, so the loader-entry +# route in the emitted image is the only thing standing between a firmware change +# and an unreflashable board. It has been wrong before — see the script. +find_package(Python3 COMPONENTS Interpreter) +if(Python3_FOUND) + add_test(NAME fantemp.reachability + COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/check_reachability.py + --objdump ${CMAKE_OBJDUMP} --elf $ + --image $/fantemp.bin) +else() + message(STATUS "Python not found — the reachability check is skipped") +endif() diff --git a/test/check_reachability.py b/test/check_reachability.py new file mode 100644 index 0000000..96a5d02 --- /dev/null +++ b/test/check_reachability.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +"""The board's only way in, checked in the emitted image. + +This board has no reset line and no programming header. The single route to the +bootloader is the running firmware's `bootloader` command, so a firmware that +gets that route wrong is a board that cannot be reflashed — and the failure is +silent, because everything else still works. + +It has been wrong before. The firmware this one replaces probed and jumped to +`0x7800`, the base of a 2 KB boot section, while the board's loader sits at +`0x7e00`; `check()` therefore read an erased byte, was false, and the command +never arrived anywhere. Nothing about that is visible short of trying it on the +hardware, which is what this replaces. + +Three properties, all read out of the disassembly rather than the source: + +1. The image ends below the boot section. `hfuse d4` puts that at 0x7c00, so an + application reaching into it would be overwritten by the loader — or worse, + executed at reset, since BOOTRST points there. +2. The hand-over targets the loader base. A word address of 0x3f00 is byte + 0x7e00; anything else is the 0x7800 bug again. +3. The hand-over does not arm the watchdog. pureboot hands straight back on + WDRF by design, so a reset-based route reaches it and opens no window. The + legacy firmware's route was exactly that, and it is the one change that + cannot be walked back from the host. + + check_reachability.py --objdump avr-objdump --elf fantemp --image fantemp.bin +""" + +from __future__ import annotations + +import argparse +import pathlib +import re +import subprocess +import sys + +BOOT_SECTION = 0x7C00 # hfuse d4: BOOTSZ 512 words +LOADER_BASE = 0x7E00 # pureboot's 512-byte slot, at the top +WDTCSR = 0x60 + + +def main() -> int: + parser = argparse.ArgumentParser() + parser.add_argument("--objdump", required=True) + parser.add_argument("--elf", type=pathlib.Path, required=True) + parser.add_argument("--image", type=pathlib.Path, required=True) + args = parser.parse_args() + + failures = [] + + size = args.image.stat().st_size + if size >= BOOT_SECTION: + failures.append(f"the image is {size} B and reaches 0x{size - 1:04x}, " + f"into the boot section at 0x{BOOT_SECTION:04x}") + else: + print(f" ok image {size} B, ends 0x{size - 1:04x}, " + f"{BOOT_SECTION - size} B clear of the boot section") + + text = subprocess.run([args.objdump, "-d", str(args.elf)], + capture_output=True, text=True, check=True).stdout + + # The address the hand-over actually targets, read at its call sites — not + # "does the image contain this byte somewhere", which proves nothing: 0x3f is + # an ordinary constant that appears in the curve tables, so a check like that + # passes just as happily on the 0x7800 bug it is supposed to catch. + # + # bootloader::call() takes the target as a function pointer, so each call site + # loads the *word* address into a register pair immediately before it. + lines = text.splitlines() + helper = re.compile(r"\b(?:r?call)\b.*<_ZN3app10bootloader4call") + sites = [] + for index, line in enumerate(lines): + if not helper.search(line): + continue + held: dict[str, int] = {} + for back in lines[max(0, index - 8):index]: + if m := re.search(r"\bldi\s+(r\d+),\s*0x([0-9A-Fa-f]{2})", back): + held[m.group(1)] = int(m.group(2), 16) + # The AVR ABI passes the pointer in r25:r24. + if "r24" in held and "r25" in held: + sites.append(held["r25"] << 8 | held["r24"]) + + want = LOADER_BASE // 2 + if not sites: + failures.append("no call to bootloader::call with a loaded target — the " + "hand-over could not be read out of the image") + elif wrong := [a for a in sites if a != want]: + failures.append(f"the hand-over targets word {[hex(a) for a in wrong]} " + f"(byte {[hex(a * 2) for a in wrong]}), not the loader at " + f"0x{LOADER_BASE:04x}") + else: + print(f" ok all {len(sites)} hand-over site(s) target word 0x{want:04x} " + f"(byte 0x{LOADER_BASE:04x})") + + # An icall/ijmp has to exist for that address to be jumped to indirectly. + if not re.search(r"\b(icall|ijmp)\b", text): + failures.append("no icall/ijmp — the hand-over cannot reach across flash") + else: + print(" ok an indirect call exists (a relative one cannot reach)") + + # What actually reaches WDTCSR, not what the image happens to load somewhere. + # A timed disable writes WDCE|WDE (0x18) and then zero. Arming writes WDE + # *without* WDCE — including 0x08, a 16 ms timeout with every prescaler bit + # clear, which is precisely what the legacy route used and is why this cannot + # be a check for "a prescaler is present". + WDCE, WDE = 0x10, 0x08 + values, held = [], {} + for line in text.splitlines(): + if m := re.search(r"\bldi\s+(r\d+),\s*0x([0-9A-Fa-f]{2})", line): + held[m.group(1)] = int(m.group(2), 16) + elif m := re.search(rf"\bsts\s+0x00{WDTCSR:02X},\s*(r\d+)", line, re.I): + reg = m.group(1) + values.append(0 if reg == "r1" else held.get(reg)) + armed = [v for v in values if v is not None and (v & WDE) and not (v & WDCE)] + if armed: + failures.append(f"WDTCSR is written {[hex(v) for v in armed]} — WDE without " + f"WDCE is arming the watchdog, and a reset-based hand-over " + f"opens no pureboot window") + elif not values: + print(" ok the watchdog is never written") + else: + print(f" ok WDTCSR writes are {[hex(v) if v is not None else '?' for v in values]}" + f" — unlock and clear, never an arm") + + for line in failures: + print(f" FAIL {line}") + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main())