[eorry-bit]

Summary

• locate aligned GSL headers instead of assuming a fixed 0x800 offset
• validate the complete GSL, DDR parameter table, and U-Boot layout before accepting a candidate
• derive section bounds from image-tool structure fields for both 0x800 and 0x1200 layouts
• download the CV6xx GSL composite at CP_STEP1_ADDR (0x04021A00)

Some HI3519DV500 image-tool output places the valid GSL header at 0x1200; the previous fixed-offset parser read padding at 0x800 and failed with expected magic ... got 0xAAAAAAAA. Candidate scanning is bounded and requires aligned headers, valid structure and payload lengths, a bounded DDR table count and board map, in-range table data, and a valid U-Boot image immediately after the declared tables.

Validation

• uv run pytest tests/ -x -q --ignore=tests/fuzz: 731 passed, 2 skipped
• uv run pytest tests/fuzz/ -x -q --hypothesis-seed=0: 16 passed
• uv run ruff check src/ tests/
• uv run mypy src/defib/ --ignore-missing-imports
• agent C tests: 5412 passed
• web protocol tests: 86 passed
• parsed real 0x800 HI3516CV610 and 0x1200 HI3519DV500 composite images without embedding either image in the test suite
• hardware recovery reached board-ID detection, DDR training, and U-Boot on HI3519DV500 with the corrected protocol

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Parse CV6xx composite image layouts dynamically

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Discover aligned GSL headers across image-tool layouts instead of assuming offset 0x800.
• Validate complete GSL, DDR table, board mapping, and U-Boot bounds before parsing.
• Transfer GSL composites at CP_STEP1_ADDR and test 0x800/0x1200 layouts.

Diagram

graph TD
  A["Composite Image"] --> B["Aligned GSL Scan"] --> C["Header Validation"] --> D["DDR Tables"] --> E["U-Boot Image"] --> F["Boot Parts"] --> G["CV6xx Transfer"]

Loading

High-Level Assessment

The bounded aligned scan with full-chain structural validation is the appropriate approach. Enumerating known offsets would remain brittle across image-tool generations, while unbounded magic scanning would increase false-positive and resource risks; deriving boundaries from declared structures avoids both problems.

Files changed (2) +322 / -102

Bug fix (1) +195 / -55

hisilicon_cv6xx.pyDiscover and validate dynamic CV6xx composite layouts +195/-55

Discover and validate dynamic CV6xx composite layouts

• Replaces fixed GSL, DDR, and U-Boot offsets with bounded aligned candidate discovery and complete structural validation. Derives DDR table boundaries from image fields, rejects missing or ambiguous layouts, preserves dynamic headers when selecting a board table, and transfers GSL at 'CP_STEP1_ADDR' ('0x04021A00').

src/defib/protocol/hisilicon_cv6xx.py

Tests (1) +127 / -47

test_protocol_cv6xx.pyCover variable composite layouts and parser validation +127/-47

Cover variable composite layouts and parser validation

• Reworks synthetic firmware generation around declared structure fields and covers valid '0x800' and '0x1200' GSL layouts. Adds false-magic, truncated-table, dynamic DDR extraction, and corrected GSL transfer-address assertions.

tests/test_protocol_cv6xx.py

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Browser uses stale GSL address ✓ Resolved

Description

Changing GSL_LOAD_ADDR to the required CP_STEP1_ADDR leaves the browser CV6xx path sending the
same composite GSL data to the old 0x04020000 destination. Browser recovery therefore continues
using the incorrect SRAM layout while the Python path uses 0x04021A00.

Code

src/defib/protocol/hisilicon_cv6xx.py[R45-47]

+# The CV6xx GSL protocol downloads the composite image at CP_STEP1_ADDR,
+# after the BootROM/GSL stack and BSS reserved at the start of SRAM.
+GSL_LOAD_ADDR = 0x04021A00

Evidence

The changed Python constant explicitly identifies 0x04021A00 as the CV6xx composite-image
destination, and send_firmware passes it directly to the frame encoder. The browser's equivalent
CV6xx path still passes 0x04020000, while the QEMU specification also requires the stale address,
proving an implementation-level protocol mismatch introduced by changing only the Python copy.

src/defib/protocol/hisilicon_cv6xx.py[45-47]
src/defib/protocol/hisilicon_cv6xx.py[425-440]
src/defib/protocol/hisilicon_cv6xx.py[506-509]
web/index.html[447-456]
docs/qemu_hisilicon_spec.md[236-254]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The corrected CV6xx composite GSL load address is applied only to the Python protocol. The browser implementation and QEMU protocol specification still use `0x04020000`, causing protocol implementations to disagree about the required SRAM destination.
## Issue Context
The browser sends `parts.gslData` directly through the same CV6xx transfer protocol, so it must use the corrected `CP_STEP1_ADDR` (`0x04021A00`) as well. Update the checked-in protocol documentation to match.
## Fix Focus Areas
- src/defib/protocol/hisilicon_cv6xx.py[45-47]
- web/index.html[447-456]
- docs/qemu_hisilicon_spec.md[236-254]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[widgetii]

Thanks. This fixes a real bug, I checked it against every published image in the family and ran it on a CV608 board, and it's good to merge. I've rebased the branch onto master, which resolves the conflict with #165.

Parser. I compared the old and new parsers on every published boot image of this family:

Image	Result
boot-hi3516cv608-nor.bin and the five boot-hi3516cv610-{00s,10b,20g,20s}-nor.bin	byte-identical GSL (22528 B), DDR table (14336 B) and U-Boot (168448 B)
boot-hi3519dv500-dmebpro-nor.bin	master: Invalid GSL magic: expected 0x4BB4D22D, got 0xAAAAAAAA. This PR: GSL 56320 B, DDR 13312 B, U-Boot 216064 B

So nothing changes for images that already worked, and master really can't use the published dv500 dmebpro image today. (dmeb isn't published, so I couldn't check it.) The validation is sound: headers must be aligned, the scans are bounded, a board mapping that points at a missing table is rejected, and two valid layouts in one file is an error rather than a guess.

GSL load address 0x04021A00, on hardware. I ran defib burn -c hi3516cv608 --power-cycle twice from RAM on a Hi3516CV608 (DDR2 64 MB), through a vectis UART bridge. Both runs printed DDR size: 0x00000040 and reached the U-Boot console from the upload. Neither showed the flash-fallback lines (The table is incorrect / System startup), so the uploaded U-Boot ran, not the one in flash. I haven't tested a CV610 or a dv500 board.

Rebase. The only conflict was in tests/test_protocol_cv6xx.py: #165 appended a test class, and this PR appended the GSL_LOAD_ADDR assertion to test_send_firmware. I kept both. On the rebased branch: pytest 1097 passed / 3 skipped, fuzz 16 passed, ruff and mypy clean, node --test 95 passed.

Nits, not blocking, fine as follow-ups:

• build_ddr_table() reads the DDR header's structure_length from file_data again after the parser has already validated it. Carrying the first table's offset in CV6xxBootParts would keep the two from drifting.
• MAX_GSL_HEADER_OFFSET, MAX_SECTION_GAP and MAX_DDR_TABLES are reasonable bounds. A short comment saying where each comes from would help whoever meets the next image-tool generation.