mini-boot: shrink toward the bootrom SRAM window, and gate the size - #6
Conversation
The bootrom copies this image into SRAM at 0x04010500 and SRAM ends at 0x04014000, so everything ahead of the compressed payload must fit in 0x3B00 (15104) bytes. Both images this repo builds were at 19376 — 4272 over. Past that boundary the upload writes into the bootrom's own stack, the chip stops answering mid-transfer, and the board cannot be recovered over UART at all. A user hit exactly that on a blanked hi3518ev200 in OpenIPC/firmware#2299; recovery died on the 15th 1 KB chunk, the first one to cross 0x04014000. Nothing checked the size, so the build shipped an unrecoverable image with a green tick. This does three things about that: -Os plus -ffunction-sections/-fdata-sections and --gc-sections in the link. Purely a code-size change, no behavioural difference: 19376 -> 16432 bytes of prelude. ALIGN(1024) before .image. Recovery tools find the end of the SRAM-resident program by scanning for the compressed header and rounding DOWN to 1 KB, so an unaligned payload means the tail of the last function is silently clipped. Padding to the boundary makes that rounding exact. It costs up to 1 KB, which is why the prelude lands at 17408 rather than 16432. An ASSERT in mini-boot.lds and a matching check on the shipped .bin in CI — the linker sees the ELF, but mini-boot.bin is assembled from it by dd afterwards, so both are worth checking. 17408 is still 2304 over the window, so the assert cannot yet be set to 0x3B00 without failing the build. It ratchets against the current size instead: that number may only go down. CI prints the remaining shortfall on every build so it stays visible. Closing it is #5. The obvious next lever, -mthumb for the C, does fit (14336) but does not boot: this tree's assembly is not interworking-safe (`ldr pc, _start_armboot` in start.S, `mov pc, lr` returns throughout mmu.S and the division helpers), and fixing those still leaves a fault that branches into the compressed payload. Left alone deliberately — the failure mode here is a camera that cannot be recovered, so it needs real silicon, not just a QEMU pass. Verified with the vendor arm-hisiv510 toolchain: both hi3518ev200 and hi3516cv200 build at 17408 and boot to a U-Boot prompt in the qemu-hisilicon hi3516cv200 model that build.yml already uses.
PR Summary by QodoShrink mini-boot and gate SRAM prelude size regressions
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Prelude remains 2304 bytes oversized
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| /* What this build actually achieves today. It is NOT yet inside the window | ||
| * above -- see OpenIPC/u-boot-hi3516cv200#5 -- so the assert below ratchets | ||
| * against the current size instead: it may only ever be lowered, never | ||
| * raised, so the gap cannot silently grow again while it is being closed. */ | ||
| MINI_BOOT_SIZE_CEILING = 0x4400; |
There was a problem hiding this comment.
1. Prelude remains 2304 bytes oversized 📎 Requirement gap ≡ Correctness
The live mini-boot prelude is allowed to reach 0x4400 (17408 bytes), 2304 bytes beyond the 0x3B00 UART SRAM window, because both the linker ratchet and artifact check accept this oversized result for both SoCs. Consequently, unrecoverable images still link and pass CI instead of triggering the required hard build-time rejection.
Agent Prompt
## Issue description
The mini-boot prelude remains 2304 bytes larger than the boot ROM UART SRAM window, while the linker assertion and CI artifact check permit images up to `0x4400` instead of rejecting any prelude beyond `0x3B00`.
## Issue Context
Both supported board configurations use the compressed mini-boot linker script and are checked by the workflow matrix. Keep the required DDR initialization and decompression code live, but reduce or restructure the prelude until `__image_start` is at or below `0x3B00` for both SoCs; then enforce 15104 bytes as the linker limit and make any artifact offset beyond `0x3B00` a hard CI failure so an SRAM-overrunning UART recovery image cannot ship.
## Fix Focus Areas
- arch/arm/cpu/hi3518ev200/compressed/Makefile[20-20]
- arch/arm/cpu/hi3518ev200/compressed/Makefile[93-94]
- arch/arm/cpu/hi3518ev200/compressed/mini-boot.lds[27-49]
- arch/arm/cpu/hi3518ev200/compressed/mini-boot.lds[60-61]
- .github/workflows/build.yml[94-116]
- .github/workflows/build.yml[110-116]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Review on #6 raised two things. The CI size gate found the prelude/payload boundary by scanning for an LZMA or gzip header, which is what a recovery tool has to do when all it has is the image -- but here the build knows exactly which bytes it embedded. A coincidental match in the prelude made the gate report a boundary that was not the payload's and pass, i.e. call an oversized, unrecoverable image fine. Planting a gzip magic at offset 8192 in a 17408-byte prelude reproduced it: the old check reported 8192 and passed. Locate image_data.lzma by exact bytes instead, and require it to appear exactly once. The gzip branch was wrong regardless: the Makefile only ever produces image_data.lzma and startup.c only recognises the LZMA 0x5D form. Second, the target was off by an alignment. 0x3B00 is not a multiple of 1 KB and the payload must start on a 1 KB boundary, so the largest prelude that both loads and survives the tools' round-down is 0x3800 (14336). The shortfall is 3072 bytes, not 2304. Report the remaining gap as a workflow warning annotation so it shows in the checks UI rather than only in the log. It stays non-fatal on purpose: the image boots from flash, and failing the build would stop both boards shipping a working bootloader over a defect that only affects the UART recovery path.
|
Both findings taken; one is a real bug, the other I'm answering rather than fixing. The payload scan could pass an oversized image (fixed)This is right, and it defeats the point of the gate. The check was scanning for a compression header — which is what a recovery tool has to do, having only the image — when the build knows exactly which bytes it embedded. Reproduced by planting a gzip magic at offset 8192 of a 17408-byte prelude: Now located by searching for the exact The target was also wrong, by one alignmentChasing the above turned up an error of mine. Still 3072 over (by design, for now)Not fixed, and I don't think it should be here. Of the three alternatives in the assessment, the third — gate at the true limit — fails both boards immediately, and the images it would reject boot fine from flash. Refusing to ship a working bootloader over a defect confined to the UART recovery path is a worse trade than shipping it with the defect recorded, especially as that recovery path is already broken today. The first alternative is the right one and I got some way into it; What has changed is that the gap can no longer widen unnoticed, and it is now a warning annotation on the checks UI rather than a line buried in a log. |
1 participant
Partial fix for #5. Cuts the overrun from 5040 bytes to 3072, makes the size a build failure if it ever grows again, and prints the remaining shortfall on every build. It does not yet close the gap — see "What's left" below.
The constraint
The bootrom loads this image into SRAM at
0x04010500; SRAM ends at0x04014000. So the prelude — vector table + 4 KBreg_info+ all mini-boot code, everything ahead of the compressed payload — must fit in0x3B00= 15104 bytes.That window is not a whole number of 1 KB blocks, and the payload has to start on a 1 KB boundary (see
ALIGNbelow), so the number actually to reach is the window rounded down:0x3800= 14336.Both images this repo builds were at 19376. Past the boundary the upload lands in the bootrom's own stack: the chip stops answering mid-transfer and cannot be recovered over UART at all. In OpenIPC/firmware#2299 a user's blanked hi3518ev200 died on the 15th 1 KB chunk — the first one to cross
0x04014000:Nothing checked this, so the build shipped an unrecoverable image with a green tick.
Changes
-Os,-ffunction-sections/-fdata-sections,--gc-sectionsALIGN(1024)before.imageASSERTinmini-boot.lds+ a check on the shipped.binin CImini-boot.binis assembled from it byddafterwards, so both are worth checking.The CI check finds the boundary by locating the exact
image_data.lzmathis build produced and requiring it to appear once — not by scanning for a compression header. A header scan is what a recovery tool has to do, having only the image, but here the build knows which bytes it embedded, and a coincidental match in the prelude would report a boundary that is not the payload's and pass an oversized image.Because 17408 is still over, the assert cannot be set to
0x3800without failing the build. It ratchets against the current size instead — that number may only ever go down — and CI reports the shortfall every build, as a warning annotation so it shows on the checks UI:That stays non-fatal on purpose: the image boots from flash, and failing the build would stop both boards shipping a working bootloader over a defect confined to the UART recovery path. On the currently-shipped binary the same check does fail outright:
What's left, and one dead end documented
The obvious next lever is
-mthumbfor the C. It does fit — 14336, exactly at the target — but it does not boot. This tree's assembly is not interworking-safe:start.Senters C withldr pc, _start_armboot, a raw PC load that never sets the Thumb bitmmu.Sand the_divsi3.S/_udivsi3.Shelpers return withmov pc, lr, which keeps the CPU in ARM state when returning to a Thumb callerFixing all of those (→
bx) gets much further but still ends in a prefetch abort that branches into the compressed payload. I stopped there rather than ship it: the failure mode is a camera nobody can recover, and that needs real silicon to sign off, not a QEMU pass. Recording it so the next person doesn't rediscover it.Cutting DDR training would also fit and is deliberately not done — that is the code that makes RAM work across silicon and temperature spread.
Verification
Vendor
arm-hisiv510toolchain (the onebuild.ymldownloads), both boards:QEMU is
widgetii/qemu-hisilicon'shi3516cv200machine — the same modelbuild.yml's existingqemu_smokejob uses. The unmodified binary was smoke-tested first to confirm the harness.The CI check was exercised against fixtures covering each path: honest image, payload absent, payload duplicated, unaligned payload, growth past the ratchet, a prelude at the target, and one with a coincidental gzip header planted in the prelude — the last of which the previous header-scanning version passed.
Not validated on real hardware — I have neither SoC on a bench.