diff --git a/agent/README.md b/agent/README.md index c5b1b22..b359781 100644 --- a/agent/README.md +++ b/agent/README.md @@ -89,11 +89,11 @@ gk7205v500 also serves gk7205v510/v530. The V500 bootrom has no SPL stage: u-boot-xmedia image (downloaded, or `-f` for your own), whose DDR-init code runs first; the bootrom then executes the slot in place at `0x41007000`. -hi3536dv100 is the DVR/NVR part Xiongmai sells as "Hi3536E". Its UART runs -off a fixed 24 MHz clock, so `SET_BAUD` reaches 921600 with no clock -switching. The OpenIPC SPL for it doesn't bring up DDR on the Xiongmai -NBD8010S-KL-V2 board; use `-c hi3536dv100:nbd8010s` there, which uploads the -board's own SPL instead. +hi3536dv100 also covers the "Hi3536E" that Xiongmai puts in its NVRs, a +variant of the same die. Its UART runs off a fixed 24 MHz clock, so +`SET_BAUD` reaches 921600 with no clock switching. The E variant needs its +own DDR settings; OpenIPC's u-boot-hi3536dv100 applies them when +SYSCTRL+0xEE0 reads 0x02000000, so plain `-c hi3536dv100` works on both. Addresses from [qemu-hisilicon](https://github.com/OpenIPC/LoTool) hardware definitions. diff --git a/src/defib/firmware.py b/src/defib/firmware.py index 8aa897b..f7d9902 100644 --- a/src/defib/firmware.py +++ b/src/defib/firmware.py @@ -214,6 +214,28 @@ def _cached_file(name: str) -> Path | None: return None +def _has_hi3536e_ddr_config(data: bytes) -> bool: + """Built after OpenIPC/u-boot-hi3536dv100#1, whose SPL programs the + Hi3536E's DDR. Its DMC timing values sit in the SPL's literal pool + whichever registers the compiler picks; older builds lack them, and on + a Hi3536E their DDR training resets the chip.""" + spl = data[:0x5000] + return all(v.to_bytes(4, "little") in spl for v in (0x34421A12, 0x4144B48D)) + + +# Cache entries that a later published build fixed in a way recovery depends +# on. One that fails its check is downloaded again; offline it is still used +# (it works on the boards the fix is not about), with a warning. +_CACHE_CHECKS: dict[str, Callable[[bytes], bool]] = { + "u-boot-hi3536dv100-universal.bin": _has_hi3536e_ddr_config, +} + + +def _is_stale(path: Path) -> bool: + check = _CACHE_CHECKS.get(path.name) + return check is not None and not check(path.read_bytes()) + + def get_cached_path(chip: str, flash_type: str | None = None) -> Path | None: """Get the path to cached firmware, or None if not cached. @@ -236,7 +258,7 @@ def get_cached_path(chip: str, flash_type: str | None = None) -> Path | None: if not candidate: continue path = _cached_file(candidate) - if path is not None: + if path is not None and not _is_stale(path): return path return None @@ -321,9 +343,22 @@ def download_firmware( # Download name = asset_name(chip, flash_type) assert name is not None # firmware_url() is None otherwise + dest = get_cache_dir() / name + # A stale entry is about to be replaced, and _download() deletes dest + # when it fails; keep its bytes to put back if we turn out to be offline. + stale = _cached_file(name) + stale_data = stale.read_bytes() if stale is not None else None try: - return _download(url, get_cache_dir() / name, on_progress) + return _download(url, dest, on_progress) except ConnectionError: + if stale_data is not None: + dest.write_bytes(stale_data) + logger.warning( + "Download of %s failed; using the cached copy, which predates " + "a fix this chip may need", + name, + ) + return dest # A universal image cached before the per-flash-type split still boots # a NOR board, so keep it usable offline. It is never downloaded, and # never used for NAND, whose split layout it carries is retired. diff --git a/src/defib/profiles/data/hi3536dv100-nbd8010s-spl.bin b/src/defib/profiles/data/hi3536dv100-nbd8010s-spl.bin deleted file mode 100644 index e3bb63e..0000000 Binary files a/src/defib/profiles/data/hi3536dv100-nbd8010s-spl.bin and /dev/null differ diff --git a/src/defib/profiles/data/hi3536dv100.json b/src/defib/profiles/data/hi3536dv100.json index 94c3119..85caadd 100644 --- a/src/defib/profiles/data/hi3536dv100.json +++ b/src/defib/profiles/data/hi3536dv100.json @@ -1 +1 @@ -{"name": "hi3536dv100", "PRESTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 78, 87, 79, 68, 64, 1, 5, 18, 117, 106, 105, 122], "DDRSTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 120, 86, 52, 18, 64, 1, 5, 18, 117, 106, 105, 122], "ADDRESS": ["0x04012000", "0x04010500", "0x81000000"], "FILELEN": ["0x0040", "0x5000"], "STEPLEN": ["0x0040", "0x0050"], "variants": {"nbd8010s": {"SPL_BLOB": "hi3536dv100-nbd8010s-spl.bin", "FILELEN": ["0x0040", "0x5840"]}}} \ No newline at end of file +{"name": "hi3536dv100", "PRESTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 78, 87, 79, 68, 64, 1, 5, 18, 117, 106, 105, 122], "DDRSTEP0": [4, 224, 45, 229, 36, 0, 159, 229, 36, 16, 159, 229, 0, 16, 128, 229, 32, 0, 159, 229, 32, 16, 159, 229, 4, 16, 128, 228, 0, 224, 128, 229, 4, 240, 157, 228, 239, 190, 173, 222, 239, 190, 173, 222, 239, 190, 173, 222, 60, 1, 5, 18, 120, 86, 52, 18, 64, 1, 5, 18, 117, 106, 105, 122], "ADDRESS": ["0x04012000", "0x04010500", "0x81000000"], "FILELEN": ["0x0040", "0x5000"], "STEPLEN": ["0x0040", "0x0050"]} \ No newline at end of file diff --git a/tests/test_agent_binary_lookup.py b/tests/test_agent_binary_lookup.py index 52cd2e2..55ad1da 100644 --- a/tests/test_agent_binary_lookup.py +++ b/tests/test_agent_binary_lookup.py @@ -43,9 +43,6 @@ def test_a_variant_suffix_still_resolves(self, monkeypatch, tmp_path): (tmp_path / "agent-hi3516cv500.bin").write_bytes(b"\x00" * 16) assert get_agent_binary("hi3516av300:emmc") is not None - def test_a_board_variant_uses_the_chip_build(self): - assert agent_binary_for("hi3536dv100:nbd8010s") == "hi3536dv100" - def test_every_path_is_absolute(self): assert all(Path(p).is_absolute() for p in _agent_search_path("hi3516cv300")) diff --git a/tests/test_firmware.py b/tests/test_firmware.py index a8e6a86..91116be 100644 --- a/tests/test_firmware.py +++ b/tests/test_firmware.py @@ -359,3 +359,52 @@ def test_gk7205v5xx_shares_the_v500_donor_cache_entry(self, tmp_path, monkeypatc donor.write_bytes(b"D" * 4096) assert get_cached_path("gk7205v510") == donor assert get_cached_path("gk7205v510", "nand") is None + + +class TestStaleCacheRefresh: + """A cached u-boot-hi3536dv100-universal.bin from before + OpenIPC/u-boot-hi3536dv100#1 resets a Hi3536E during DDR training, so + the cache must not keep serving it once the fixed build is published.""" + + FIXED = b"\x00" * 0x100 + (0x34421A12).to_bytes(4, "little") \ + + (0x4144B48D).to_bytes(4, "little") + b"\x00" * 0x4000 + OLD = b"\x00" * 0x4108 + + def _seed(self, tmp_path, monkeypatch, data): + monkeypatch.setenv("XDG_CACHE_HOME", str(tmp_path)) + monkeypatch.setattr("sys.platform", "linux") + path = get_cache_dir() / "u-boot-hi3536dv100-universal.bin" + path.write_bytes(data) + return path + + def test_a_fixed_build_is_used_from_cache(self, tmp_path, monkeypatch): + path = self._seed(tmp_path, monkeypatch, self.FIXED) + assert get_cached_path("hi3536dv100") == path + + def test_an_old_build_is_downloaded_again(self, tmp_path, monkeypatch): + from defib import firmware + + path = self._seed(tmp_path, monkeypatch, self.OLD) + assert get_cached_path("hi3536dv100") is None + + def fake_download(url, dest, on_progress=None): + dest.write_bytes(self.FIXED) + return dest + + monkeypatch.setattr(firmware, "_download", fake_download) + assert download_firmware("hi3536dv100") == path + assert path.read_bytes() == self.FIXED + + def test_offline_the_old_build_is_still_used(self, tmp_path, monkeypatch): + from defib import firmware + + path = self._seed(tmp_path, monkeypatch, self.OLD) + + def offline(url, dest, on_progress=None): + if dest.exists(): + dest.unlink() # what _download() does on failure + raise ConnectionError("offline") + + monkeypatch.setattr(firmware, "_download", offline) + assert download_firmware("hi3536dv100") == path + assert path.read_bytes() == self.OLD diff --git a/tests/test_profiles.py b/tests/test_profiles.py index 2e8f330..77cddc9 100644 --- a/tests/test_profiles.py +++ b/tests/test_profiles.py @@ -220,19 +220,6 @@ def test_real_av300_emmc_variant_loads_spl_blob(self): # SPL extracted at 0x5000 (above the gzip boundary at ~0x5220) assert len(p.spl_data) == 0x5000 - def test_real_hi3536dv100_nbd8010s_variant_loads_spl_blob(self): - """The Xiongmai NBD8010S-KL-V2 NVR's DDR differs from the board the - OpenIPC hi3536dv100 SPL was built for, so its variant ships the SPL - from the board's own U-Boot. The blob runs right up to the LZMA - payload at 0x5840, and FILELEN grows to match so none of it is cut.""" - p = load_profile("hi3536dv100:nbd8010s", PROFILES_DIR) - assert p.spl_blob == "hi3536dv100-nbd8010s-spl.bin" - assert p.spl_data is not None - assert p.spl_data[:4] == bytes.fromhex("170800ea") - assert len(p.spl_data) == 0x5840 - assert p.spl_max_size == 0x5840 - assert load_profile("hi3536dv100", PROFILES_DIR).spl_data is None - class TestSplBlob: """SPL_BLOB lets a profile (typically a board variant) ship pre-built diff --git a/tests/test_recovery_session.py b/tests/test_recovery_session.py index fc4387c..1f8c6aa 100644 --- a/tests/test_recovery_session.py +++ b/tests/test_recovery_session.py @@ -57,7 +57,6 @@ async def test_full_standard_session(self): @pytest.mark.asyncio @pytest.mark.parametrize("chip, spl_len", [ - ("hi3536dv100:nbd8010s", 0x5840), # 0x5000 blob under an inherited FILELEN of 0x6000: must not be # padded out to the profile maximum. ("hi3516av300:emmc", 0x5000),