pardes as P4 firmware: the seam, and a flash-mapping bug in this toolchain

The editor arrives as one freestanding OBJECT exporting a seven-function C ABI
(src/pardes/app.zig declares it, ../02-pardes-code/src/p4.zig implements it), not
as a package dependency. A build.zig.zon path dependency was built first and
reverted: merely DECLARING it nested pardes's ~30-package graph under this one and
broke every build here - std/Build.zig:2091 exceeded its 1000-branch comptime
quota via ghostty's lazyImport, seven cached tree_sitter versions use APIs removed
in 0.16, and the fetch wrote 2.6 GB across 42,736 files into this working copy.

The seam is bytes in and bytes out, which is what a serial line is anyway: the
editor owns vaxis and the ANSI encoding, this side owns the UART, the heap and the
clock, and neither names the other's types. It is versioned, because linkers do
not type-check C symbols and a drifted signature would link cleanly and then
corrupt the stack.

THE BUG WORTH THE COMMIT. .flash.text was ALIGN(64), and the image builder's
anchor makes two mapped segments share an MMU page safely - as long as rodata does
not END inside the page where text BEGINS. With a 578 KB image it does. A volatile
read of a string literal at 0x4004a1d1 returned 37 09 fa 4f, which disassembles to
"lui s2, 0x4ffa0": this image's own .flash.text. Every literal in that last shared
page read as code, so the first thing the firmware tried to print was machine code
and it died on an instruction access fault. .flash.text is now ALIGN(0x10000),
making the segments page-disjoint. The packing trick this project opened with only
ever mattered when the alternative was 64 KiB of zeros in a 1 KB image.

Two more findings, both recorded in README.md:

  * A linker symbol declared as an anyopaque OBJECT gives the optimiser a
    zero-sized object, so ordinary stores through a pointer derived from its
    address are dead code it may drop - and did, silently. The allocator's first
    block header read back as size=2988759312 next=0x14284684 and the free-list
    walk never terminated. @extern with a many-pointer has no size to lose.
    examples/memprobe.zig could not have caught it: it writes through a volatile
    pointer, which the optimiser must leave alone.

  * The RTC watchdog is armed at handover. Every example here had been resetting on
    a ten-second cycle, invisibly, because no run had ever lasted eight seconds.

State, honestly: the firmware boots, clears .bss, brings up the console, disables
the watchdog, starts the systimer, checks the ABI version, initialises the 384 KiB
heap and calls into the editor, which sets up its sink and its environment. It then
faults inside pardes_p4_init on the first allocation. The cause is measured but not
fixed: a load from .flash.rodata page 3 returns the contents of the page 0x50000
higher - exactly the vaddr distance between the rodata and text segments - while
pages 0, 2 and 4 read correctly. The bisect markers that localised it are still in
place, deliberately, because the next step needs them.


--- correction, measured after the above was written ---

Two mapped segments is NOT a choice, and the earlier comment in tools/image.zig
was right for a reason I initially got wrong and then measured.

I first read bootloader_utility.c's `#else` branch, which classifies segments by
address window with two independent ifs - and since the P4's DROM and IROM windows
are the identical range (soc.h:146-149), I concluded the last mapped segment wins
both roles and the first is never mapped. That branch does not run on this chip.
The P4 takes the SOC_MMU_DI_VADDR_SHARED branch (bootloader_utility.c:805-851),
whose own comment says it: "On chips with shared D/I external vaddr, we don't
divide them into either D or I, as essentially they are the same." It collects
mapped segments POSITIONALLY into rom_addr[2] and ends with

    assert(rom_index == 2);

Shipping a one-segment image proved it, on the board:

    Assert failed in unpack_load_app, bootloader_utility.c:842 (rom_index == 2)

So the split stays, image.zig keeps enforcing exactly two - turning that boot-time
abort into a build-time error - and both are now documented with the branch that
actually runs and the assert that actually fires.

What DOES change is alignment. .flash.text was ALIGN(64). Two mapped segments may
share a 64 KiB MMU page only if they also share a flash page, which the image
builder's anchor guarantees - and that holds right up until an application is large
enough for rodata to END inside the page where text BEGINS. With a 578 KB image it
does. Measured on the die: a volatile read of a string literal at 0x4004a1d1
returned 37 09 fa 4f, which disassembles to "lui s2, 0x4ffa0" - this image's own
.flash.text. Every literal in that shared page read as code, so the first thing the
firmware tried to print was machine code, and it died on an instruction access
fault. .flash.text is now ALIGN(0x10000), which makes the segments page-disjoint.
It costs up to 64 KiB of image padding against a 1.5 MiB partition; the packing
trick this project opened with only mattered when the alternative was 64 KiB of
zeros in a 1 KB image.

With that fixed the firmware gets much further: entry, .bss cleared, console up,
watchdog disabled, systimer running, ABI version checked, the 384 KiB heap
initialised, into the editor, its sink and environment ready - and the literal at
0x4004a1d1 now reads back correctly.

Still open, and characterised rather than guessed: pardes_p4_init faults on its
first allocation. The allocator struct crosses the seam intact (its function
pointers land in .flash.text), but the std.mem.Allocator vtable at 0x40035a1c reads
back as instruction bytes, and the dispatch at .flash.text+0xade2 jumps through it.
Ruled out with measurements: the ELF and the image agree at that address, the flash
is MD5-verified against the image, the wrong bytes are identical across three
resets and two reflashes (so not a stale cache), the corruption is a contiguous run
rather than 64-byte lines, and mmu_hal_map_region's arithmetic
(page_num = ceil(len/page), entry from vaddr) is correct for the segments as now
laid out. The next measurement is the one that settles it: read the MMU entry
registers from the running application and print vaddr -> flash for every page. The
register model in src/soc.zig can do that; the bisect markers are left in place for
it.

1 files changed, 193 insertions, 15 deletions

diff --git a/build.zig b/build.zig
index 3a22d4f..0791bc5 100644
--- a/build.zig
+++ b/build.zig
@@ -3,6 +3,7 @@
 //!   zig build            compile, link, and emit a flashable image
 //!   zig build flash      the above, then write it to the chip over the serial port
 //!   zig build monitor    open the console
+//!   zig build console    attach an interactive terminal: keystrokes in, screen out
 //!   zig build size       print where every byte of the image went
 //!
 //! There is no CMake, no ninja, no idf.py, no esptool and no external linker: Zig's own LLD does
@@ -14,6 +15,7 @@ const std = @import("std");
 const image = @import("tools/image.zig");
 const serial = @import("tools/serial.zig");
 const rom = @import("tools/rom.zig");
+const console = @import("tools/console.zig");
 
 pub fn build(b: *std.Build) void {
     // ---------------------------------------------------------------- board and target knobs
@@ -32,7 +34,12 @@ pub fn build(b: *std.Build) void {
     const min_rev = b.option(u16, "min-rev", "minimum silicon revision, major*100+minor (default 100)") orelse 100;
     const max_rev = b.option(u16, "max-rev", "maximum silicon revision (default 199)") orelse 199;
     const descriptor = b.option(DescriptorKind, "descriptor", "app descriptor: minimal (184 B) or full (256 B)") orelse .minimal;
-    const stack_size = b.option(u32, "stack", "stack size in bytes (default 8192)") orelse 8192;
+    // `-Dpardes` swaps in the editor as the application. It is a distinct option rather than just
+    // `-Dapp=src/pardes/app.zig` because it also resolves the lazy `pardes` dependency and raises
+    // the default stack: the core recurses through layout and 8 KiB is not enough for it.
+    const pardes_app = b.option(bool, "pardes", "build the pardes editor as the application (needs ../02-pardes-code)") orelse false;
+    const stack_size = b.option(u32, "stack", "stack size in bytes (default 8192, or 32768 under -Dpardes)") orelse
+        @as(u32, if (pardes_app) 32768 else 8192);
     // ReleaseSmall by default: this is firmware, and `standardOptimizeOption` would otherwise
     // hand out Debug builds - which for this target means panic machinery and formatting code
     // linked into a 500-byte image.
@@ -150,7 +157,10 @@ pub fn build(b: *std.Build) void {
         }),
     });
 
-    const app_source = b.option([]const u8, "app", "root source file (default src/main.zig)") orelse "src/main.zig";
+    // The app root still comes from `-Dapp`, so pointing that at a different shell over the same
+    // module stays possible.
+    const app_source = b.option([]const u8, "app", "root source file (default src/main.zig, or src/pardes/app.zig under -Dpardes)") orelse
+        if (pardes_app) "src/pardes/app.zig" else "src/main.zig";
     const app = b.addExecutable(.{
         .name = "app",
         .root_module = b.createModule(.{
@@ -212,6 +222,53 @@ pub fn build(b: *std.Build) void {
     });
     app.root_module.addImport("io", io_mod);
 
+    // The general-purpose allocator, its own module for exactly the reason given above for `io`: a
+    // module's imports cannot escape its root directory, so neither src/pardes/ nor examples/ can
+    // reach src/net/heap.zig as a file. Pointed at the existing file rather than copied - `Heap` is
+    // a coalescing free-list over one caller-supplied span and has nothing to do with the radio; it
+    // lives under src/net/ only because ESP-Hosted needed it first. The file has zero `export`s, so
+    // compiling it into two modules cannot collide.
+    //
+    // Added unconditionally, like `io`: an application that never imports it costs nothing, because
+    // an unreferenced module emits no code.
+    const heap_mod = b.createModule(.{
+        .root_source_file = b.path("src/net/heap.zig"),
+        .target = target,
+        .optimize = optimize,
+        .single_threaded = true,
+    });
+    app.root_module.addImport("heap", heap_mod);
+
+    if (pardes_app) {
+        // The editor arrives as a linked OBJECT, not as a package dependency, and that is a
+        // measurement rather than a preference.
+        //
+        // The obvious design was `build.zig.zon` with a path dependency on ../02-pardes-code, and
+        // `dep.module("pardes_p4")`. It was written, and it broke EVERY build in this repo -
+        // `zig build`, every example, the oracle - because merely DECLARING it nests pardes's
+        // ~30-package graph under this one. Two failures, both from just the declaration:
+        //
+        //   * std/Build.zig:2091 evaluates `mem.eql(u8, decl.name, pkg_hash)` over the whole
+        //     dependency table at comptime, and the enlarged table exceeds the 1000 backwards
+        //     branch quota. It is reached from ghostty's own build (SharedDeps.zig:874 calls
+        //     `b.lazyImport`), which is not lazy in pardes's manifest and so is always compiled.
+        //   * pardes's package cache holds seven tree_sitter versions, and the stale ones use
+        //     `Compile.addCSourceFile`/`linkLibrary`, removed in Zig 0.16. Nesting made them
+        //     reachable and their build.zig files failed to compile.
+        //
+        // Neither is fixable from this side, and both would come back the next time the editor
+        // gained a dependency. So the seam is a file instead: pardes's own build emits one
+        // freestanding object exporting a small C ABI, and this links it. The consequences are all
+        // improvements - this repo keeps having no manifest and no dependencies, the editor's
+        // renderer stays next to the vaxis it needs, and the boundary is bytes in / bytes out.
+        const obj = b.option([]const u8, "pardes-obj", "path to pardes's p4 object (default ../02-pardes-code/zig-out/pardes-p4.o)") orelse
+            "../02-pardes-code/zig-out/pardes-p4.o";
+        app.root_module.addObjectFile(if (std.fs.path.isAbsolute(obj))
+            .{ .cwd_relative = obj }
+        else
+            b.path(obj));
+    }
+
     if (hosted) {
         // The Zig half: the port table, the libc surface and the IP stack.
         //
@@ -291,6 +348,25 @@ pub fn build(b: *std.Build) void {
     run_mon.step.dependOn(&flash.step);
     b.step("run", "flash the image, then print its console output").dependOn(&run_mon.step);
 
+    // The interactive counterpart of `monitor`. `monitor` prints for N seconds and sends nothing,
+    // which is right for an application that only reports; an application the human drives needs
+    // the keyboard on the wire. `-Dconsole-baud` is separate from `-Dbaud` because they are
+    // genuinely different rates: the flasher's rate is negotiated by the ROM loader's SYNC
+    // auto-detect, while the console's is whatever the running firmware programmed into UART0.
+    const console_baud = b.option(
+        serial.Baud,
+        "console-baud",
+        "interactive console baud (default 115200, the rate the bootloader leaves UART0 at)",
+    ) orelse .b115200;
+    const con = ConsoleStep.create(b, port_path, console_baud);
+    b.step("console", "attach an interactive terminal to the running application").dependOn(&con.step);
+
+    // Ordered, for the same reason `run` is: an unordered `flash console` lets the console reset
+    // the board out from under the writer.
+    const run_con = ConsoleStep.create(b, port_path, console_baud);
+    run_con.step.dependOn(&flash.step);
+    b.step("interact", "flash the image, then attach an interactive terminal").dependOn(&run_con.step);
+
     const reset = ResetStep.create(b, port_path);
     b.step("reset", "reset the board and let the flashed application run").dependOn(&reset.step);
 
@@ -1055,27 +1131,94 @@ fn linkerScript(b: *std.Build, stack_size: u32, peripherals_ld: ?[]const u8) []c
         \\     that -Dhosted links ESP-Hosted's transport and RPC layers, and the generated protobuf
         \\     descriptors alone are 33,000 lines of .rodata. One window overflowed by 14,744 bytes.
         \\
-        \\     1 MiB now. It is a *region*, not a reservation: the image contains only the sections
-        \\     actually emitted, so a blink app is still ~1.7 KB. The ceiling that matters is the
-        \\     factory partition, 0x177000 = 1.5 MiB, and this stays comfortably inside it. Segments
-        \\     still come in the two the loader wants; they simply span more than one MMU page now,
-        \\     which the bootloader maps without complaint. */
-        \\  flash (rx) : ORIGIN = 0x40000020, LENGTH = 0xFFFE0
-        \\  l2mem (rw) : ORIGIN = 0x4FF00000, LENGTH = 0x20000
+        \\     1.5 MiB now, less 32 KiB of slack. It is a *region*, not a reservation: the image
+        \\     contains only the sections actually emitted, so a blink app is still ~1.7 KB. The
+        \\     ceiling that matters is the factory partition - `zig build monitor` reads it off the
+        \\     bootloader's own table as `factory 00010000 00177000` - and the region is deliberately
+        \\     set just under it so that an application which outgrows the partition fails at the
+        \\     LINK, with a section-overflow naming the section, rather than at the flash write or
+        \\     (worse) at boot. Segments still come in the two the loader wants; they simply span
+        \\     more than one MMU page now, which the bootloader maps without complaint. */
+        \\  flash (rx) : ORIGIN = 0x40000020, LENGTH = 0x170000
+        \\
+        \\  /* .data/.bss/.stack. This was 0x20000 for as long as nothing needed more.
+        \\
+        \\     The die was asked (examples/memprobe.zig) rather than the datasheet, because the
+        \\     relevant ESP-IDF fragment (esp_system/ld/esp32p4/memory.ld.in:18-33) is parameterised
+        \\     on CONFIG_CACHE_L2_CACHE_SIZE, whose Kconfig help says the size is set "on application
+        \\     startup" - by an application this is not. Measured, on this rev v1.3 die:
+        \\
+        \\         0x4FF03000..0x4FF3F000   240 KiB  RAM
+        \\         0x4FF3F000..0x4FF40000     4 KiB  the mask ROM's .data/.bss, never written
+        \\         0x4FF40000..0x4FFA0000   384 KiB  RAM
+        \\         0x4FFA0000..0x4FFC0000   128 KiB  NOT memory - the L2 cache lives here
+        \\
+        \\     That last line cost a bug worth recording. The first version of the probe wrote a
+        \\     pattern and read it back one page at a time, and reported the whole upper 512 KiB as
+        \\     RAM - because a store followed immediately by a load of the SAME address returns the
+        \\     stored value whether the backing store is real, an address mirror, or merely a dirty
+        \\     cache line. Writing every page before reading any page separates the three, and the
+        \\     top 128 KiB then failed. It is the L2 cache, and ESP-IDF's own arithmetic agrees
+        \\     exactly: SRAM_HIGH_SIZE = 0x80000 - CONFIG_CACHE_L2_CACHE_SIZE, with the Kconfig
+        \\     default of 128 KiB (esp_system/port/soc/esp32p4/Kconfig.cache:5,19). Handing those
+        \\     128 KiB to an allocator hung the heap on its first free-list walk.
+        \\
+        \\     This region stops at 0x4FF3F000 because `ets_printf` - the only console this image
+        \\     has - reads the ROM statics that begin at 0x4FF3FBA4, and losing them loses the
+        \\     ability to report having lost them. */
+        \\  l2mem (rw) : ORIGIN = 0x4FF00000, LENGTH = 0x3F000
+        \\
+        \\  /* The upper RAM, 384 KiB and contiguous, free once the second-stage bootloader has
+        \\     jumped away. Nothing is *linked* here: it carries no section, and the two symbols
+        \\     below hand it to the application as one span for a run-time heap. Kept out of `l2mem`
+        \\     because the ROM's statics sit between the two, and stops at 0x4FFA0000 because the
+        \\     L2 cache is above that - see the measurement in `l2mem`'s comment. */
+        \\  l2high (rw) : ORIGIN = 0x4FF40000, LENGTH = 0x60000
         \\}}
         \\
+        \\/* The heap span, from the linker rather than from a constant in Zig, so the region above is
+        \\   the single place these addresses are written down. */
+        \\__heap_start = ORIGIN(l2high);
+        \\__heap_end   = ORIGIN(l2high) + LENGTH(l2high);
+        \\
         \\SECTIONS {{
+        \\  /* Two flash-mapped segments, rodata then text, and the split is NOT optional: the
+        \\     second-stage bootloader asserts it. On a chip whose D and I external vaddr ranges are
+        \\     shared - which the P4's are (soc.h:146-149, both 0x40000000..0x44000000) - ESP-IDF
+        \\     takes the `SOC_MMU_DI_VADDR_SHARED` branch of `unpack_load_app`
+        \\     (bootloader_utility.c:805-851). That branch does not classify segments as D or I at
+        \\     all; it collects them positionally into rom_addr[2] and ends with
+        \\
+        \\         assert(rom_index == 2);
+        \\
+        \\     so one mapped segment aborts the boot with
+        \\     `Assert failed in unpack_load_app, bootloader_utility.c:842 (rom_index == 2)`, and a
+        \\     third trips `assert(rom_index < 2)` inside the loop. Measured, by trying it. */
         \\  .flash.rodata : ALIGN(16) {{
         \\    KEEP(*(.rodata.appdesc))     /* the loader reads esp_app_desc_t at image offset 0x20 */
         \\    *(.rodata .rodata.* .srodata .srodata.*)
-        \\    /* Leave exactly the 8 bytes the image builder needs for the next segment's header
-        \\       before the 64-byte boundary that .flash.text starts on. ALIGN(64) alone does not
-        \\       guarantee any hole: for one rodata length in eight the gap is 0 or 4 bytes and the
-        \\       build dies with MappedSegmentsTooClose. */
-        \\    . = ALIGN(. + 8, 64) - 8;
+        \\    /* Leave the 8 bytes the image builder needs for the next segment's header before the
+        \\       boundary .flash.text starts on. ALIGN alone does not guarantee any hole: for one
+        \\       rodata length in eight the gap is 0 or 4 bytes and the build dies with
+        \\       MappedSegmentsTooClose. */
+        \\    . = ALIGN(. + 8, 0x10000) - 8;
         \\  }} > flash
         \\
-        \\  .flash.text : ALIGN(64) {{
+        \\  /* ALIGN(0x10000), one whole MMU page, and NOT the 64 bytes this used to be.
+        \\
+        \\     Two mapped segments may share a 64 KiB MMU page only if they also share a flash page,
+        \\     which the image builder's anchor guarantees - and that reasoning holds right up until
+        \\     an application is large enough for rodata to END inside the page where text BEGINS.
+        \\     Measured on the die with a 578 KB image: a volatile read of a string literal at
+        \\     0x4004a1d1 returned `37 09 fa 4f`, which disassembles to `lui s2, 0x4ffa0` - this
+        \\     image's own .flash.text. Every literal in that shared page read as code, so the first
+        \\     thing the firmware tried to print was machine code.
+        \\
+        \\     Aligning text to a page boundary makes the segments page-disjoint, so no MMU entry is
+        \\     ever claimed by both. It costs up to 64 KiB of image padding, which against a 1.5 MiB
+        \\     partition is not worth reasoning about - the packing trick this project opened with
+        \\     only mattered when the alternative was 64 KiB of zeros in a 1 KB image. */
+        \\  .flash.text : ALIGN(0x10000) {{
         \\    *(.text.entry)
         \\    *(.text .text.*)
         \\  }} > flash
@@ -1348,6 +1491,41 @@ const MonitorStep = struct {
     }
 };
 
+/// `monitor`, but the wire runs both ways. See tools/console.zig for the size handshake, which is
+/// the only part of this that is not a straight byte copy.
+///
+/// Takes the same `port_lock` as every other step that opens the port, so `zig build flash console`
+/// cannot have the console pull the board out of download mode mid-write. It then HOLDS that lock
+/// for the whole interactive session, which is correct and worth saying out loud: the session ends
+/// when the user detaches, so any other port step named on the same command line waits for the
+/// human rather than racing them.
+const ConsoleStep = struct {
+    step: std.Build.Step,
+    port: []const u8,
+    baud: serial.Baud,
+
+    fn create(b: *std.Build, port: []const u8, baud: serial.Baud) *ConsoleStep {
+        const self = b.allocator.create(ConsoleStep) catch @panic("OOM");
+        self.* = .{
+            .step = std.Build.Step.init(.{ .id = .custom, .name = "console", .owner = b, .makeFn = make }),
+            .port = port,
+            .baud = baud,
+        };
+        return self;
+    }
+
+    fn make(step: *std.Build.Step, _: std.Build.Step.MakeOptions) anyerror!void {
+        const self: *ConsoleStep = @fieldParentPtr("step", step);
+        const b = step.owner;
+
+        port_lock.lockUncancelable(b.graph.io);
+        defer port_lock.unlock(b.graph.io);
+
+        console.attach(self.port, self.baud, .{}) catch |err|
+            return step.fail("console on {s}: {s}", .{ self.port, @errorName(err) });
+    }
+};
+
 /// Pulse the reset line and leave. One ioctl pair, but it is the difference between "did my app
 /// hang or did I forget to reset it" during development.
 const ResetStep = struct {