From ae9850a28f0e5a7052809ce6fa1b7aaea5ebd6a1 Mon Sep 17 00:00:00 2001 From: Vladimir Smitka Date: Sat, 29 Aug 2026 14:55:42 +0000 Subject: [PATCH] zephyr-cp: add a partition layout check for cptools Board overlays routinely delete the partitions node their board DTS declares and rebuild it so the board gets a CIRCUITPY filesystem. Dropping the ranges; while doing so costs nothing at build time and everything at runtime: devicetree stops translating the partition addresses, so they resolve to bare offsets. On RP2040 that turns off RP2_REQUIRES_SECOND_STAGE_BOOT and builds the UF2 for the wrong address; on a board whose flash is not based at 0 it makes every flash_area offset underflow. check_partitions.py reads the edt.pickle a build has already produced, so it costs no build time -- point it at build directories after building, or run it with no arguments to check every build directory in the port. It verifies that mapped partitions resolve inside their device, that nothing overlaps or runs past the end, and reports how many partitions it inspected so a board that defines none is not mistaken for a verified one. Tests parse real device tree source, so they exercise the checks against devicetree as edtlib resolves it. --- ports/zephyr-cp/cptools/check_partitions.py | 214 +++++++++++++ .../cptools/tests/test_check_partitions.py | 294 ++++++++++++++++++ 2 files changed, 508 insertions(+) create mode 100644 ports/zephyr-cp/cptools/check_partitions.py create mode 100644 ports/zephyr-cp/cptools/tests/test_check_partitions.py diff --git a/ports/zephyr-cp/cptools/check_partitions.py b/ports/zephyr-cp/cptools/check_partitions.py new file mode 100644 index 00000000000..4d4769ec95f --- /dev/null +++ b/ports/zephyr-cp/cptools/check_partitions.py @@ -0,0 +1,214 @@ +#!/usr/bin/env python3 +"""Check the flash partition layout a board's devicetree actually resolved to. + +Board overlays in ``boards/`` routinely rebuild the ``partitions`` node so the +board gets a CIRCUITPY filesystem. Rebuilding it is easy to get subtly wrong in +a way nothing reports: drop the ``ranges;`` the board DTS declared and +devicetree stops translating partition addresses into the SoC's address space, +so every partition resolves to a bare offset instead. The build still succeeds. +What changes is anything keyed off the address -- on RP2040, +``RP2_REQUIRES_SECOND_STAGE_BOOT`` matches the code partition against +0x10000100, so a partition left at 0x100 silently turns it off and the UF2 is +built for the wrong address with no second stage bootloader linked in. + +This reads the ``edt.pickle`` a build already produced, so it costs no build +time -- point it at build directories after building, or run it with no +arguments to check every build directory in the port. + + python cptools/check_partitions.py # every build-* dir + python cptools/check_partitions.py build-raspberrypi_rpi_pico_zephyr + +Exits non-zero when a layout has problems. +""" + +import argparse +import pathlib +import pickle +import sys + +PORT_DIR = pathlib.Path(__file__).resolve().parent.parent +EDT_MODULE = PORT_DIR / "zephyr" / "scripts" / "dts" / "python-devicetree" / "src" + + +def load_edt(build_dir): + """Load the pickled devicetree a build produced, or None if there is none.""" + edt_path = build_dir / "zephyr-cp" / "zephyr" / "edt.pickle" + if not edt_path.is_file(): + edt_path = build_dir / "zephyr" / "edt.pickle" + if not edt_path.is_file(): + return None + sys.path.insert(0, str(EDT_MODULE)) + with open(edt_path, "rb") as f: + return pickle.load(f) + + +def partition_device(node): + """Walk up to the NVM device owning a partition. + + The device is the first ancestor carrying ``reg``; the ``partitions`` + grouping node in between has none. + """ + parent = getattr(node, "parent", None) + while parent is not None: + if parent.props.get("reg") is not None: + return parent + parent = getattr(parent, "parent", None) + return None + + +def device_size(node): + """Total size of an NVM device. + + External SPI/QSPI NOR carries its capacity in the ``size`` property (in + bits) and uses ``reg`` for the chip select, so reading ``reg`` alone gives + 0 for exactly the devices CIRCUITPY usually lives on. + """ + size = node.props.get("size") + if size: + return size.val // 8 + reg = node.props.get("reg") + if reg and len(reg.val) >= 2: + return reg.val[1] + return 0 + + +def is_partition_child(node): + """True for any node under a ``partitions`` grouping node. + + Membership is positional rather than by ``compatible``: an overlay may add + a partition carrying neither ``zephyr,mapped-partition`` nor a + ``fixed-partitions`` parent, and it still occupies the space and still is + what the layout means to describe. + """ + parent = getattr(node, "parent", None) + return parent is not None and getattr(parent, "name", "") == "partitions" + + +def iter_partitions(edt): + """Yield ``(node, device, offset, size, mapped)`` for every partition.""" + for node in edt.nodes: + mapped = "zephyr,mapped-partition" in getattr(node, "compats", []) + if not mapped and not is_partition_child(node): + continue + reg = node.props.get("reg") + if not reg or len(reg.val) < 2: + continue + dev = partition_device(node) + if dev is None: + continue + if mapped and (not getattr(node, "regs", None) or not getattr(dev, "regs", None)): + continue + yield node, dev, reg.val[0], reg.val[1], mapped + + +def check_layout(edt): + """Return a list of problems with the resolved layout. + + Mapped partitions are checked for address translation; every partition is + checked for overlap and for running past the end of its device. Erase-page + alignment is not checked: RP2040 deliberately puts its code partition at + 0x100, directly behind the 256-byte second stage bootloader. + """ + problems = [] + by_device = {} + for node, dev, offset, size, mapped in iter_partitions(edt): + label = node.labels[0] if node.labels else node.name + dev_label = dev.labels[0] if dev.labels else dev.name + total = device_size(dev) + if mapped: + # Test the resolved address against the device's own window rather + # than against base + reg. Both forms are in use: a partition reg + # is usually an offset, but some overlays write the absolute + # address and leave the partitions node without ranges, which + # resolves to the same correct address. What is never right is a + # partition resolving outside the device it lives in -- which is + # exactly what a rebuilt partitions node missing ranges; produces, + # since the offset is then left untranslated. + base = dev.regs[0].addr + actual = node.regs[0].addr + if total and not (base <= actual < base + total): + problems.append( + f"{label}: resolves to 0x{actual:x}, outside {dev_label} " + f"(0x{base:x}-0x{base + total:x}) -- address translation is " + f"broken; does the partitions node declare ranges;?" + ) + # Geometry below is compared in offsets from the device base, so a + # partition declared either way lands in the same space. When the + # translation is broken the difference is meaningless (and often + # negative), so keep the declared offset rather than running the + # geometry checks on nonsense. + translated = actual - base + if 0 <= translated and (not total or translated < total): + offset = translated + by_device.setdefault(dev_label, (dev, total, []))[2].append((label, offset, size)) + + for dev_label, (dev, total, parts) in sorted(by_device.items()): + ordered = sorted(parts, key=lambda p: p[1]) + for label, offset, size in ordered: + if total and offset + size > total: + problems.append( + f"{label}: ends at 0x{offset + size:x}, past the end of " + f"{dev_label} (0x{total:x})" + ) + # A running high-water mark, not neighbouring pairs: a partition + # spanning several later ones only overlaps its immediate successor in + # a pairwise walk. + high_label, high_end = None, 0 + for label, offset, size in ordered: + if offset < high_end: + problems.append( + f"{high_label} (ends 0x{high_end:x}) overlaps " + f"{label} (starts 0x{offset:x}) on {dev_label}" + ) + if offset + size > high_end: + high_label, high_end = label, offset + size + return problems + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "build_dirs", + nargs="*", + help="Build directories to check (default: every build-* in the port)", + ) + args = parser.parse_args() + + dirs = [pathlib.Path(d) for d in args.build_dirs] + if not dirs: + dirs = sorted(p for p in PORT_DIR.glob("build-*") if p.is_dir()) + if not dirs: + print("No build directories found; build a board first.", file=sys.stderr) + return 1 + + failed = [] + checked = 0 + for build_dir in dirs: + edt = load_edt(build_dir) + if edt is None: + continue + checked += 1 + n_parts = sum(1 for _ in iter_partitions(edt)) + problems = check_layout(edt) + if problems: + failed.append(build_dir.name) + print(f"{build_dir.name}:") + for problem in problems: + print(f" FAIL {problem}") + else: + # Say how much was inspected: a board whose overlay defines no + # partitions at all would otherwise be indistinguishable from a + # verified-good one. + print(f"{build_dir.name}: ok ({n_parts} partitions checked)") + + if not checked: + print("No build directory held an edt.pickle; build a board first.", file=sys.stderr) + return 1 + if failed: + print(f"\n{len(failed)} board(s) with layout problems: {', '.join(failed)}") + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/ports/zephyr-cp/cptools/tests/test_check_partitions.py b/ports/zephyr-cp/cptools/tests/test_check_partitions.py new file mode 100644 index 00000000000..12a35ba6ea9 --- /dev/null +++ b/ports/zephyr-cp/cptools/tests/test_check_partitions.py @@ -0,0 +1,294 @@ +"""Unit tests for cptools/check_partitions.py using real device tree parsing. + +Each case here is a layout that actually occurs in this port, so the checks are +exercised against devicetree as edtlib resolves it rather than against +hand-built stand-ins. +""" + +import pathlib +import sys +import tempfile + +import pytest + +portdir = pathlib.Path(__file__).parent.parent.parent +sys.path.append(str(portdir / "zephyr/scripts/dts/python-devicetree/src/")) + +from devicetree import edtlib # noqa: E402 + +sys.path.insert(0, str(pathlib.Path(__file__).parent.parent)) + +from check_partitions import check_layout, device_size, iter_partitions # noqa: E402 + +BINDINGS = [str(portdir / "zephyr/dts/bindings")] + + +def parse_dts_string(dts_content): + """Parse device tree source and return the resolved edtlib.EDT.""" + with tempfile.NamedTemporaryFile(mode="w", suffix=".dts", delete=False) as f: + f.write(dts_content) + f.flush() + temp_path = f.name + + try: + return edtlib.EDT(temp_path, BINDINGS) + finally: + pathlib.Path(temp_path).unlink() + + +def xip_flash(partitions, ranges="ranges;", size="0x200000"): + """A memory-mapped flash at 0x10000000, the RP2040 arrangement. + + ``ranges`` is what the overlays get wrong: the board DTS declares it on the + partitions node, and an overlay that deletes and rebuilds that node without + it leaves every partition untranslated. + """ + return f"""/dts-v1/; + +/ {{ + #address-cells = <1>; + #size-cells = <1>; + + soc {{ + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash0: flash@10000000 {{ + compatible = "soc-nv-flash"; + erase-block-size = <4096>; + reg = <0x10000000 {size}>; + ranges = <0x0 0x10000000 {size}>; + #address-cells = <1>; + #size-cells = <1>; + + partitions {{ + {ranges} + #address-cells = <1>; + #size-cells = <1>; +{partitions} + }}; + }}; + }}; +}}; +""" + + +MAPPED_PARTS = """ + code_partition: partition@100 { + compatible = "zephyr,mapped-partition"; + label = "code-partition"; + reg = <0x100 0x17ff00>; + }; + + circuitpy_partition: partition@180000 { + compatible = "zephyr,mapped-partition"; + label = "circuitpy"; + reg = <0x180000 0x80000>; + }; +""" + + +class TestAddressTranslation: + """The partitions node must keep translating addresses to the SoC's space.""" + + def test_missing_ranges_is_reported(self): + """This is the bug the checker exists for: without ranges; on the + rebuilt partitions node the partitions resolve to bare offsets.""" + edt = parse_dts_string(xip_flash(MAPPED_PARTS, ranges="")) + problems = check_layout(edt) + + assert len(problems) == 2 + assert any("code_partition" in p and "0x100" in p for p in problems) + assert all("ranges" in p for p in problems) + + def test_ranges_present_passes(self): + """The same layout with ranges; must be clean, or the check would only + ever be able to fail.""" + edt = parse_dts_string(xip_flash(MAPPED_PARTS)) + + assert check_layout(edt) == [] + + def test_absolute_addresses_pass(self): + """Some overlays write the absolute address into reg and leave the + partitions node without ranges. That resolves to the same correct + address, so it must not be reported.""" + parts = """ + code_partition: partition@10000100 { + compatible = "zephyr,mapped-partition"; + label = "code-partition"; + reg = <0x10000100 0x17ff00>; + }; +""" + edt = parse_dts_string(xip_flash(parts, ranges="")) + + assert check_layout(edt) == [] + + def test_flash_based_at_zero_passes(self): + """On a device based at 0 -- the nRF arrangement -- the translated and + the bare value coincide, so nothing is wrong either way.""" + dts = """/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + soc { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + flash0: flash@0 { + compatible = "soc-nv-flash"; + erase-block-size = <4096>; + reg = <0x0 0x100000>; + ranges = <0x0 0x0 0x100000>; + #address-cells = <1>; + #size-cells = <1>; + + partitions { + #address-cells = <1>; + #size-cells = <1>; + + slot0_partition: partition@10000 { + compatible = "zephyr,mapped-partition"; + label = "image-0"; + reg = <0x10000 0x10000>; + }; + }; + }; + }; +}; +""" + edt = parse_dts_string(dts) + + assert check_layout(edt) == [] + + +class TestPartitionDiscovery: + """What counts as a partition is decided by position, not by compatible.""" + + def test_partition_without_compatible_is_seen(self): + """An overlay may add a partition carrying no compatible at all. It + still occupies the space, and treating it as absent would report a + board that has a filesystem as having none.""" + parts = """ + storage_partition: partition@180000 { + label = "storage"; + reg = <0x180000 0x1000>; + }; + + circuitpy_partition: partition@181000 { + label = "circuitpy"; + reg = <0x181000 0x7f000>; + }; +""" + edt = parse_dts_string(xip_flash(parts)) + labels = [node.labels[0] for node, _dev, _off, _size, _mapped in iter_partitions(edt)] + + assert "storage_partition" in labels + assert "circuitpy_partition" in labels + + +class TestDeviceSize: + """Bus-attached flash keeps its capacity in size, not in reg.""" + + def test_size_property_wins_over_chip_select(self): + """An SPI NOR's reg is a chip select. Reading a size out of it gives 0, + which would silently skip every check on the device CIRCUITPY usually + lives on.""" + dts = """/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + soc { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + spi@0 { + compatible = "vnd,spi"; + reg = <0x0 0x100>; + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + mx25r64: mx25r6435f@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + size = <67108864>; + spi-max-frequency = <8000000>; + jedec-id = [c2 28 17]; + }; + }; + }; +}; +""" + edt = parse_dts_string(dts) + node = edt.get_node("/soc/spi@0/mx25r6435f@0") + + assert device_size(node) == 8 * 1024 * 1024 + + +class TestGeometry: + """Partitions must not overlap or leave their device.""" + + def test_partition_spanning_several_others_is_reported(self): + """Comparing neighbouring pairs only would catch the first overlap and + miss the rest, so a partition covering three others must report more + than once.""" + parts = """ + slot0_partition: partition@0 { + compatible = "zephyr,mapped-partition"; + label = "image-0"; + reg = <0x0 0x30000>; + }; + + nvm_partition: partition@10000 { + compatible = "zephyr,mapped-partition"; + label = "nvm"; + reg = <0x10000 0x1000>; + }; + + storage_partition: partition@20000 { + compatible = "zephyr,mapped-partition"; + label = "storage"; + reg = <0x20000 0x1000>; + }; +""" + edt = parse_dts_string(xip_flash(parts)) + problems = [p for p in check_layout(edt) if "overlaps" in p] + + assert len(problems) == 2 + + def test_partition_past_end_of_device_is_reported(self): + """A partition may not run past the flash it lives in.""" + parts = """ + circuitpy_partition: partition@1f0000 { + compatible = "zephyr,mapped-partition"; + label = "circuitpy"; + reg = <0x1f0000 0x20000>; + }; +""" + edt = parse_dts_string(xip_flash(parts)) + + assert any("past the end" in p for p in check_layout(edt)) + + +class TestEmptyLayout: + """A board whose overlay defines nothing must not look verified.""" + + def test_no_partitions_yields_nothing_checked(self): + """check_layout has nothing to complain about here, which is why the + caller reports how many partitions it inspected: an empty layout and a + good one are otherwise indistinguishable.""" + edt = parse_dts_string(xip_flash("")) + + assert check_layout(edt) == [] + assert list(iter_partitions(edt)) == [] + + +if __name__ == "__main__": + sys.exit(pytest.main([__file__]))