Skip to content

gk7205v200/v300: failsafe rescue mode, flash-free DRAM boot counter, crash-log capture - #2396

Merged
widgetii merged 2 commits into
masterfrom
gk7205v200-failsafe-rescue
Sep 11, 2026
Merged

widgetii merged 2 commits into
masterfrom
gk7205v200-failsafe-rescue

Conversation

@widgetii

Copy link
Copy Markdown
Member

Why

The recovery + owner-diagnostics rungs, on top of the pstore region (#2392), the DTS no-map regions (OpenIPC/linux), and the bootcount arm/disarm (#2391) — which this converts off the NOR env.

What

  • Flash-free DRAM boot counter. Move the counter from the U-Boot env to a single DRAM word at 0x41f20000 (a no-map reserved region in the board DTS). S99bootok clears it through /dev/mem once majestic is proven healthy; sysupgrade arms it the same way. Writing the NOR env every boot is wear + a mid-write brick risk — a healthy boot now writes zero flash. On a U-Boot/kernel without the region the write lands in spare reserved RAM (harmless). Pairs with the DRAM bootcount in u-boot-gk7205v200#22.
  • Failsafe rescue mode. When U-Boot appends failsafe to the cmdline after bootlimit failed boots, rcS brings up only logging, mdev, networking and SSH — no SDK, no streamer — so a crashlooping camera lands reachable instead of looping. The overlay stays mounted (claim intact); the counter is reset on entry so failsafe doesn't re-escalate.
  • Crash-log capture (S98crashlog). On the normal boot after a crash, preserve the pstore log into /etc/crash (gzipped, capped, ring freed), and drop a breadcrumb when it fell into failsafe — for the WebUI to offer the owner (majestic-webui#427). Overlay-only, nothing on a clean boot.
  • gk7205v300: enable pstore, matching gk7205v200.

Proven on hardware — two Goke SoCs

  • gk7205v200 (64M, both allocators) and gk7205v300 (128M): a crashloop climbs the counter 0xB0010001→0002→ boot Dev #3Bootlimit (2) exceeded → altbootcmd → FAILSAFE (network+SSH only, breadcrumb, counter reset), then self-recovers on the next normal boot; a real panic is captured in pstore and harvested to /etc/crash; the WebUI banner + download return the actual panic dmesg; a sustained-healthy boot resets the counter to 0.

Overview for maintainers: https://claude.ai/code/artifact/71b8f759-6d64-408c-89c3-8db8227e1e07

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add Goke failsafe boot, DRAM counter, and crash capture

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Adds network-and-SSH failsafe boot after repeated unsuccessful starts.
• Replaces flash-backed boot counting with a reserved DRAM counter.
• Enables pstore capture and preserves the latest crash report for diagnostics.
Diagram

graph TD
  B["U-Boot"] --> D{"Limit exceeded?"} -->|Yes| F["Failsafe init"] --> R["Network and SSH"] --> Z["Clear counter"]
  D -->|No| N["Normal init"] --> C["Capture pstore"] --> H{"Streamer healthy?"} -->|Yes| Z
  H -->|No| B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Retain environment-backed boot counting
  • ➕ Uses existing U-Boot environment mechanisms
  • ➕ Persists across complete power loss
  • ➕ Avoids a fixed physical memory address
  • ➖ Writes NOR during recovery accounting
  • ➖ Introduces flash wear and power-loss corruption risk
  • ➖ Requires environment access from userspace
2. Use a hardware watchdog scratch register
  • ➕ Avoids NOR writes
  • ➕ Provides hardware-native reset tracking
  • ➕ Can remain isolated from kernel-managed memory
  • ➖ Depends on SoC-specific register availability and retention behavior
  • ➖ Requires additional bootloader and board-specific support
  • ➖ Reduces portability across Goke variants
3. Expose pstore records without archiving
  • ➕ Avoids copying crash data into the overlay
  • ➕ Uses the kernel pstore interface directly
  • ➖ Crash evidence is lost after a cold power cycle
  • ➖ The pstore ring remains occupied
  • ➖ WebUI availability becomes tied to volatile records

Recommendation: The reserved DRAM counter and bounded overlay archive are the best fit for warm-reset crashloop recovery. They eliminate routine NOR writes, preserve actionable crash evidence across later power loss, and degrade harmlessly on older software; environment-backed counting remains useful only where cold-boot persistence is mandatory.

Files changed (5) +116 / -49

Enhancement (4) +106 / -48
S98crashlogArchive the latest pstore crash for owner diagnostics +37/-0

Archive the latest pstore crash for owner diagnostics

• Adds an init script that packages pstore records into a validated gzip archive, writes diagnostic metadata, and removes consumed dmesg records. Archives overwrite the previous crash, while clean boots perform no writes.

general/overlay/etc/init.d/S98crashlog

S99bootokClear the DRAM boot counter after sustained streamer health +22/-25

Clear the DRAM boot counter after sustained streamer health

• Replaces U-Boot environment disarming with a flash-free devmem write to the reserved boot-counter word. Retains the background health window so unstable majestic startup continues toward failsafe escalation.

general/overlay/etc/init.d/S99bootok

rcSAdd a minimal network-and-SSH failsafe startup path +30/-0

Add a minimal network-and-SSH failsafe startup path

• Detects the failsafe kernel argument and starts only logging, pstore, device management, hostname, networking, and SSH services. It records a recovery breadcrumb, clears the DRAM counter, and skips the normal SDK and streamer startup sequence.

general/overlay/etc/init.d/rcS

sysupgradeInitialize upgrade recovery through the DRAM counter +17/-23

Initialize upgrade recovery through the DRAM counter

• Replaces environment-based boot recovery arming with a devmem counter reset before entering the flash phase. Removes obsolete environment cleanup while retaining removal of saved verify overrides for kernel CRC checking.

general/overlay/usr/sbin/sysupgrade

Other (1) +10 / -1
gk7205v300.generic.configEnable compressed ramoops-backed pstore support for gk7205v300 +10/-1

Enable compressed ramoops-backed pstore support for gk7205v300

• Enables pstore, RAM storage, console and pmsg capture for the gk7205v300 kernel. Selects panic-safe ZLIB compression and explicitly disables alternative compressors.

br-ext-chip-goke/board/gk7205v200/gk7205v300.generic.config

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Other cameras get a Goke memory write 📘 Rule violation ≡ Correctness
Description
S99bootok, the failsafe branch in rcS, and sysupgrade hard-code the Goke boot-counter address
0x41f20000 in the global overlay instead of deriving it from board-specific configuration or
checking that the camera family reserves it. Because these shared files ship with devmem enabled,
healthy-boot cleanup, upgrades, and manually requested failsafe entry can write into another
platform's physical memory, including the documented active kernel region on HiSilicon layouts.
Code

general/overlay/etc/init.d/rcS[33]

+	command -v devmem >/dev/null 2>&1 && devmem 0x41f20000 32 0 2>/dev/null
Evidence
Compliance rule 5 requires files under general/overlay/ to remain board-agnostic and obtain
per-board behavior through detection or configuration, but all three shared scripts write the same
fixed physical address without checking the board or reading board-specific configuration. The
common Buildroot fragment installs general/overlay into images and the shared BusyBox
configuration enables devmem; additionally, the HiSilicon loader documents kernel memory from
0x40000000 through 0x42000000, which places 0x41f20000 inside active OS memory and shows that
each of these shared execution paths can overwrite live memory.

Rule 5: No device-specific values in generic configuration
general/overlay/etc/init.d/S99bootok[17-18]
general/overlay/etc/init.d/S99bootok[36-36]
general/overlay/etc/init.d/rcS[33-33]
general/overlay/usr/sbin/sysupgrade[1356-1356]
general/openipc.fragment[1-6]
general/package/busybox/busybox.config[786-790]
general/package/hisilicon-osdrv-hi3516ev200/files/script/load_hisilicon[13-30]
general/package/hisilicon-osdrv-hi3516ev200/files/script/load_hisilicon[34-56]
general/overlay/etc/init.d/rcS[29-35]
general/overlay/usr/sbin/sysupgrade[1347-1356]

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 generic overlay writes the Goke-specific boot-counter address on every camera with `devmem`, even though that physical address is reserved only for the affected Goke family and may overlap active memory on other SoCs.
## Fix Focus Areas
- general/overlay/etc/init.d/S99bootok[17-36]
- general/overlay/etc/init.d/rcS[29-33]
- general/overlay/usr/sbin/sysupgrade[1347-1356]
## Recommended Fix
Remove the hard-coded `0x41f20000` operation from the generic scripts. Supply the address through board-specific configuration or a boot-counter helper installed only for the applicable Goke family, with an explicit capability check permitting the paired `gk7205v200`/`gk7205v300` platform, and make the shared boot, failsafe, and upgrade callers skip the write entirely when that configuration or capability is absent.

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



Remediation recommended

2. Healthy cameras can enter rescue mode 🐞 Bug ☼ Reliability
Description
S99bootok executes devmem once and exits without checking its status, retrying it, or reporting
that the healthy boot's counter was not cleared. If the mapping or write fails on a paired camera,
subsequent reboots continue incrementing the stale counter until the bootloader enters failsafe
despite Majestic passing the health window.
Code

general/overlay/etc/init.d/S99bootok[R33-37]

if [ ! -x /usr/bin/majestic ] || pidof majestic >/dev/null 2>&1; then
  ok=$((ok + 1))
  if [ "$ok" -ge "$need" ]; then
-				if fw_setenv upgrade_available 0 2>/dev/null &&
-					fw_setenv bootcount 0 2>/dev/null; then
-					exit 0
-				fi
-				# Environment readable but not writable: say so, or every
-				# healthy boot silently leaves U-Boot counting toward recovery.
-				logger -s -t bootok "healthy but could not clear upgrade_available (U-Boot env not writable) -- bootloader will keep counting" 2>/dev/null
+				devmem "$BOOTCOUNT_ADDR" 32 0 2>/dev/null
  	exit 0
Evidence
The health loop defines successful clearing as necessary to avoid failsafe, but after reaching eight
healthy checks it suppresses devmem errors and exits unconditionally. The upgrade and failsafe
paths use the same unchecked operation, and no environment-backed fallback remains in these paths.

general/overlay/etc/init.d/S99bootok[20-37]
general/overlay/etc/init.d/S99bootok[38-46]
general/overlay/etc/init.d/rcS[29-35]
general/overlay/usr/sbin/sysupgrade[1347-1364]

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

## Issue description
A failed physical-memory write is silently treated as a successful boot-counter clear, allowing a healthy camera to accumulate boots and enter failsafe.
## Fix Focus Areas
- general/overlay/etc/init.d/S99bootok[28-45]
- general/overlay/etc/init.d/rcS[29-35]
- general/overlay/usr/sbin/sysupgrade[1347-1356]
## Recommended Fix
Check every `devmem` exit status, retry transient failures where appropriate, and log an explicit error when the counter cannot be cleared. On the upgrade path, warn or abort according to the required recovery guarantee instead of silently proceeding without a known counter state.

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


3. Failed captures erase crash evidence 🐞 Bug ☼ Reliability
Description
S98crashlog tests the status of gzip rather than tar in its pipeline and ignores whether the
final mv succeeds. An unreadable pstore record or failed publication can therefore produce a
nonempty partial archive, log preservation as successful, and delete the original crash records.
Code

general/overlay/etc/init.d/S98crashlog[R24-26]

+if tar cf - -C "$PSTORE" . 2>/dev/null | gzip > "$CRASH/crash.tar.gz.tmp" 2>/dev/null \
+	&& [ -s "$CRASH/crash.tar.gz.tmp" ]; then
+	mv "$CRASH/crash.tar.gz.tmp" "$CRASH/crash.tar.gz" 2>/dev/null
Evidence
The shell pipeline can succeed when gzip successfully processes partial input after tar fails,
while the nonempty check only proves that gzip emitted a stream. The script then ignores mv and
metadata errors before announcing success and removing every matching dmesg record.

general/overlay/etc/init.d/S98crashlog[18-26]
general/overlay/etc/init.d/S98crashlog[27-35]

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

## Issue description
Crash records are deleted even when tar failed or the completed archive could not be published, permanently losing diagnostic evidence.
## Fix Focus Areas
- general/overlay/etc/init.d/S98crashlog[22-35]
## Recommended Fix
Create and validate the tar archive in a separate temporary file so the tar exit status is checked directly, then compress it and require the final rename and metadata publication to succeed. Delete pstore records and log success only after every preservation step has completed successfully; otherwise retain the source records and report the failure.

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


Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread general/overlay/etc/init.d/rcS Outdated
Comment thread general/package/openipc-failsafe/files/S99bootok
Comment thread general/package/openipc-failsafe/files/S98crashlog Outdated
…sh log

Builds the recovery loop on top of the pstore region (#2392) and the bootcount
arm/disarm (#2391), converting the counter off the NOR env.

- Move the boot counter from the U-Boot env to a single DRAM word at 0x41f20000
  (a no-map reserved region, board DTS): S99bootok clears it through /dev/mem
  once majestic is proven healthy, and sysupgrade arms it the same way. Writing
  the env every boot is NOR wear and a mid-write brick risk; a healthy boot now
  writes zero flash. On a U-Boot/kernel without the region the write lands in
  spare reserved RAM -- harmless.
- rcS gains a failsafe branch: when U-Boot appends "failsafe" to the cmdline
  after bootlimit failed boots, bring up only logging, mdev, networking and SSH
  -- skip the SDK and the streamer -- so a crashlooping camera lands reachable
  instead of looping. The overlay stays mounted (claim intact); the counter is
  reset on entry so failsafe does not re-escalate.
- S98crashlog preserves a crashed boot's pstore log into /etc/crash on the next
  normal boot (gzipped, capped to the latest, pstore ring then freed) and rcS
  drops a breadcrumb when it fell into failsafe, for the WebUI to offer the
  owner. Overlay-only, no flash on a clean boot.

Proven on hardware, both allocators: gk7205v200 (64M) and gk7205v300 (128M) --
crashloop escalates to failsafe and self-recovers, pstore panic is harvested,
counter resets on a healthy boot.
widgetii added a commit that referenced this pull request Sep 11, 2026
…served

S98crashlog, S99bootok and the failsafe branch's DRAM-counter write lived in
the shared overlay, so every board shipped them and cleared physical address
0x41f20000 on healthy boots, upgrades and manual failsafe entry -- even
families that never reserve that word. On a HiSilicon layout it is live
memory, not the counter, so a Goke-only write was being issued fleet-wide
(qodo review on #2396). Shipping the two scripts to every board also pushed
hi3518ev300_lite past its rootfs cap.

Move the machinery into an openipc-failsafe package selected only by the
gk7205v200/gk7205v300 defconfigs:

  - bootcount-clear derives the counter address from the board DTS
    reserved-memory node (bootcount@<addr>), so it is a safe no-op on any
    image that does not reserve it, and never touches flash;
  - failsafe-rescue is the failsafe branch, extracted from rcS;
  - S98crashlog and S99bootok move verbatim / switch to bootcount-clear.

rcS keeps only a tiny dispatcher that runs failsafe-rescue when it is present,
and sysupgrade calls bootcount-clear when present. Neither shared file writes
another platform's memory now, and boards without the package carry none of
the weight.
@widgetii
widgetii force-pushed the gk7205v200-failsafe-rescue branch from 5055121 to b7a2503 Compare September 11, 2026 13:23
…served

S98crashlog, S99bootok and the failsafe branch's DRAM-counter write lived in
the shared overlay, so every board shipped them and cleared physical address
0x41f20000 on healthy boots, upgrades and manual failsafe entry -- even
families that never reserve that word. On a HiSilicon layout it is live
memory, not the counter, so a Goke-only write was being issued fleet-wide
(qodo review on #2396). Shipping the two scripts to every board also pushed
hi3518ev300_lite past its rootfs cap.

Move the machinery into an openipc-failsafe package selected only by the
gk7205v200/gk7205v300 defconfigs:

  - bootcount-clear derives the counter address from the board DTS
    reserved-memory node (bootcount@<addr>), so it is a safe no-op on any
    image that does not reserve it, and never touches flash;
  - failsafe-rescue is the failsafe branch, extracted from rcS;
  - S98crashlog and S99bootok move here and switch to bootcount-clear.

rcS keeps only a tiny dispatcher that runs failsafe-rescue when it is present,
and sysupgrade calls bootcount-clear when present. Neither shared file writes
another platform's memory now, and boards without the package carry none of
the weight.

Also hardens the two reliability paths qodo flagged: bootcount-clear
propagates the devmem exit status and S99bootok logs a healthy-boot clear that
did not land, instead of silently letting U-Boot count toward failsafe; and
S98crashlog stages tar to a file so a partial capture cannot pass gzip's exit
status and then delete the pstore records it failed to preserve.
@widgetii
widgetii force-pushed the gk7205v200-failsafe-rescue branch from b7a2503 to 7d82a58 Compare September 11, 2026 13:27
@widgetii

Copy link
Copy Markdown
Member Author

Addressed all three findings from the Qodo review in 7d82a5837.

1 · Rule violation — "Other cameras get a Goke memory write" (Rule 5): fixed by moving the machinery out of the shared overlay into a new openipc-failsafe package, selected only by the five gk7205v200/gk7205v300 defconfigs.

  • The counter address is no longer hard-coded anywhere: bootcount-clear reads it from the board DTS reserved-memory node (bootcount@<addr>). A board whose kernel does not reserve the counter has no such node, so the write is a safe no-op there and never touches another platform's physical memory.
  • rcS now keeps only a tiny dispatcher that runs /usr/sbin/failsafe-rescue when it is present; sysupgrade calls bootcount-clear when present. Neither shared file writes the counter itself. (This also drops the two scripts off boards that don't need them, which is what put hi3518ev300_lite over its rootfs cap — that build should pass now, alongside hi3518ev300: build the crypto modules into the kernel, off the rootfs #2397.)

2 · Bug — "Healthy cameras can enter rescue mode": bootcount-clear now propagates devmem's exit status, and S99bootok logs a healthy-boot clear that did not land (could not clear the boot counter (devmem write failed) …) instead of exiting silently while U-Boot keeps counting toward failsafe.

3 · Bug — "Failed captures erase crash evidence": S98crashlog no longer pipes tar into gzip (which masked tar's status behind gzip's). It stages tar to a file and checks tar, then gzip, then the mv, and deletes the pstore records only on full success; a failed capture logs and leaves the records in place.

Off-camera gates re-run clean locally: test_shell_parse.sh (144 scripts, incl. the four new package scripts), test_strip_shell_comments.sh, ci-matrix.py --self-test, and test_kconfig_graph.py.

Hardware note: the escalation path this enables was validated end-to-end on both a gk7205v200 (64M) and a gk7205v300 (128M) — bootlimit → U-Boot altbootcmd → failsafe (network+SSH only) → counter reset, with the counter surviving the kernel/panic reboot path.

@widgetii
widgetii merged commit dac15a7 into master Sep 11, 2026
126 of 127 checks passed
@widgetii
widgetii deleted the gk7205v200-failsafe-rescue branch September 11, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant