Skip to content

sysupgrade: arm bootloader boot-count recovery, and disarm it once healthy - #2391

Merged
widgetii merged 2 commits into
masterfrom
sysupgrade-bootcount-arm
Sep 10, 2026
Merged

widgetii merged 2 commits into
masterfrom
sysupgrade-bootcount-arm

Conversation

@widgetii

Copy link
Copy Markdown
Member

The problem

A U-Boot with boot-count recovery (the gk7205v200 bootloader just gained it) can reflash a known-good image when a freshly-written one never reaches a working userspace — the field brick where an interrupted upgrade left an image only a UART reflash could recover. But the bootloader half is inert until userspace tells it (a) a flash is in flight and (b), later, that the flash actually worked. This adds those two signals.

Change

  • sysupgrade sets upgrade_available=1 (and bootcount=0) just before the flash — on the still-normal system, before the ramfs pivot moves the MTD env out of reach. It also drops any saved verify=n so a camera that once ran the pre-verify U-Boot gets its kernel CRC-checked again.
  • S99bootok (new init hook) disarms it (upgrade_available=0) once the camera is healthy — majestic up, or no majestic on the image at all. If majestic never comes up it leaves the flag set, so the bootloader escalates to recovery on the following boots instead of looping on an image that boots but doesn't stream. The wait runs in the background (never holds up boot); an unarmed boot is a single fw_printenv with no write.

Safe on older / vendor U-Boot

On a camera whose U-Boot has no boot-count logic, upgrade_available and bootcount are just unused env vars it ignores — arming/disarming them changes nothing about how those cameras boot. The recovery only activates where the bootloader knows what the flag means. This is the coupled userspace half of the gk7205v200 bootloader boot-count escalation.

Hardware tested on

gk7205v200 (lab). The camera's U-Boot predates the boot-count logic, so the env vars are inert there — which is exactly the "safe on old U-Boot" case. Verified the arm → disarm cycle on the running camera:

# armed (as sysupgrade does before flashing)
upgrade_available=1  bootcount=0
# S99bootok start, majestic up  ->  disarms
after: upgrade_available=0  bootcount=0
# unarmed re-run  ->  no-op, no write
still: upgrade_available=0

The camera booted and streamed normally throughout. The bootloader escalation this flag drives is verified in QEMU on the u-boot side; the full end-to-end on a camera running the boot-count U-Boot is the remaining integration step.

Scope

Shared overlay (sysupgrade + a new S99bootok), so it builds for every board; ci-matrix --self-test passes and test_shell_parse / test_strip_shell_comments pass (140 scripts, incl. these, parse clean stripped). Behaviour is inert on every board until a boot-count-capable U-Boot is present.

…althy

A U-Boot that supports boot-count recovery (OpenIPC/u-boot-gk7205v200) can
reflash a known-good image when a freshly written one never reaches a working
userspace -- the failure mode behind a field brick, where an interrupted
upgrade left an image that a soldering iron was the only way back from. But the
bootloader half is inert until userspace tells it a flash is in flight and,
later, that the flash worked. This adds those two signals:

- sysupgrade sets upgrade_available=1 (and bootcount=0) just before the flash,
  on the still-normal system before the ramfs pivot moves the MTD env out of
  reach. It also drops a saved verify=n key, so a camera that once ran the
  pre-verify U-Boot gets its kernel CRC-checked again.
- S99bootok, a new init hook, disarms it (upgrade_available=0) once the camera
  is healthy -- majestic up, or no majestic on the image at all. If majestic
  never comes up it leaves the flag set, so the bootloader escalates to
  recovery on the following boots instead of looping on an image that boots but
  does not stream. It runs its wait in the background so a slow or failing
  start never holds up boot, and an unarmed (normal) boot costs a single
  fw_printenv and writes nothing.

Safe on cameras with an older OpenIPC or a vendor-supplied U-Boot that has no
boot-count logic: upgrade_available and bootcount are then just unused
environment variables it ignores, so arming and disarming them changes nothing
about how those cameras boot. The recovery only activates where the bootloader
knows what the flag means.

Hardware tested on: gk7205v200 (lab). Verified the arm -> disarm cycle on the
running camera -- fw_setenv upgrade_available 1, then S99bootok with majestic up
clears it to 0; an unarmed run is a no-op; the camera boots normally throughout
(its U-Boot predates the boot-count logic, so the vars are inert, which is the
point). The bootloader escalation the flag drives is verified in QEMU on the
u-boot side; the end-to-end on a camera running the boot-count U-Boot is the
remaining integration step.

Evidence:
  # armed
  upgrade_available=1  bootcount=0
  # S99bootok start (majestic up)
  after: upgrade_available=0  bootcount=0
  # unarmed re-run: no write
  still: upgrade_available=0
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Add boot-count recovery arming and healthy-boot disarming

✨ Enhancement 🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Arms U-Boot boot-count recovery before flashing makes environment storage unavailable.
• Disarms recovery asynchronously only after upgraded userspace reaches a healthy camera state.
• Removes legacy verify=n overrides to restore kernel CRC verification.
Diagram

sequenceDiagram
    actor Operator
    participant Upgrade as sysupgrade
    participant Env as U-Boot Env
    participant Boot as U-Boot
    participant Hook as S99bootok
    participant Camera as Majestic
    Operator->>Upgrade: Start firmware flash
    Upgrade->>Env: Arm recovery and reset count
    Upgrade->>Env: Remove verify override
    Boot->>Env: Read recovery state
    Boot->>Hook: Start upgraded userspace
    Hook->>Env: Check armed state
    Hook->>Camera: Poll camera health
    alt Camera healthy or absent
        Hook->>Env: Disarm and reset count
    else Camera remains unhealthy
        Env-->>Boot: Recovery stays armed
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Mark healthy from Majestic startup
  • ➕ Uses a direct service-readiness signal
  • ➕ Avoids polling when Majestic is installed
  • ➖ Cannot handle images built without Majestic
  • ➖ Couples bootloader recovery state to one application
  • ➖ May miss failures occurring immediately after service startup
2. Enable only on supported boards
  • ➕ Avoids environment reads and writes on legacy bootloaders
  • ➕ Limits behavior to explicitly validated hardware
  • ➖ Requires maintaining board capability metadata
  • ➖ Prevents compatible bootloaders on other boards from benefiting automatically
  • ➖ Fragments otherwise shared upgrade behavior

Recommendation: Keep the shared sysupgrade and late-init hook approach. It provides backward-compatible recovery signaling without board-specific configuration, handles images both with and without Majestic, and places arming before the ramfs pivot while keeping health waits off the boot-critical path. Service-integrated signaling would be more direct but is too tightly coupled to Majestic.

Files changed (2) +51 / -0

Enhancement (2) +51 / -0
S99bootokDisarm boot-count recovery after userspace health validation +34/-0

Disarm boot-count recovery after userspace health validation

• Adds a non-blocking late-init hook that checks whether recovery is armed, then waits up to 60 seconds for Majestic. It clears 'upgrade_available' and 'bootcount' when Majestic is healthy or absent, while leaving recovery armed if the camera service never starts.

general/overlay/etc/init.d/S99bootok

sysupgradeArm boot-count recovery before entering ramfs +17/-0

Arm boot-count recovery before entering ramfs

• Sets 'upgrade_available=1' and resets 'bootcount' before flashing moves the MTD environment out of reach. It also removes a persisted 'verify=n' override so U-Boot resumes kernel CRC verification.

general/overlay/usr/sbin/sysupgrade

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

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

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Starting streamers disable recovery ✓ Resolved 🐞 Bug ≡ Correctness
Description
S99bootok treats a successful pidof majestic as proof of health and clears both bootloader
variables immediately. S95majestic returns at the fork and the repository documents that pidof
sees the process before initialization completes, so an image whose streamer crashes during startup
loses recovery protection.
Code

general/overlay/etc/init.d/S99bootok[R26-28]

+		if [ ! -x /usr/bin/majestic ] || pidof majestic >/dev/null 2>&1; then
+			fw_setenv upgrade_available 0 2>/dev/null
+			fw_setenv bootcount 0 2>/dev/null
Evidence
The new script clears recovery solely on process-name visibility. Majestic is launched
asynchronously, and the existing utility documentation explicitly records that the process becomes
visible to pidof before it has completed initialization.

general/overlay/etc/init.d/S99bootok[23-33]
general/package/majestic/files/S95majestic[41-51]
general/overlay/usr/sbin/extutils[48-65]

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 recovery flag is cleared as soon as a majestic process exists, even though that process may still be initializing or may immediately exit.
## Fix Focus Areas
- general/overlay/etc/init.d/S99bootok[23-33]
- general/package/majestic/files/S95majestic[41-51]
- general/overlay/usr/sbin/extutils[48-65]
## Recommended Fix
Replace the bare `pidof` condition with a meaningful majestic readiness check and clear the recovery flag only after that check succeeds. Keep the timeout behavior so failure to reach readiness leaves recovery armed.

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



Remediation recommended

2. Failed upgrades arm unrelated boots ✓ Resolved 🐞 Bug ☼ Reliability
Description
sysupgrade sets upgrade_available=1 before pre-flash verification and version checks determine
whether anything will be written. If verification aborts before an in-place flash or every requested
image already matches under --no_reboot, the running system is restored without clearing the flag
and a later boot is still treated as an upgrade boot.
Code

general/overlay/usr/sbin/sysupgrade[R1352-1354]

+if command -v fw_setenv >/dev/null 2>&1; then
+	fw_setenv upgrade_available 1 2>/dev/null
+	fw_setenv bootcount 0 2>/dev/null
Evidence
Arming precedes flash_and_reboot, while rootfs verification can call die() before the first
write and equal versions can skip both write functions. The pre-flash die() and successful
--no_reboot paths restore services but do not undo the newly added environment state.

general/overlay/usr/sbin/sysupgrade[78-108]
general/overlay/usr/sbin/sysupgrade[145-153]
general/overlay/usr/sbin/sysupgrade[220-269]
general/overlay/usr/sbin/sysupgrade[1074-1088]
general/overlay/usr/sbin/sysupgrade[1109-1128]
general/overlay/usr/sbin/sysupgrade[1344-1359]

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

## Issue description
Recovery remains armed when the post-arm flash path returns without touching flash, causing subsequent boots to inherit upgrade state from an unsuccessful or no-op run.
## Fix Focus Areas
- general/overlay/usr/sbin/sysupgrade[78-108]
- general/overlay/usr/sbin/sysupgrade[1074-1088]
- general/overlay/usr/sbin/sysupgrade[1344-1359]
## Recommended Fix
Track whether recovery was successfully armed and clear `upgrade_available` before every return that restores the original running system without touching flash. Preserve the flag once flash has actually been modified or the process has entered the ramfs phase.

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


3. Upgrades proceed without recovery ✓ Resolved 🐞 Bug ☼ Reliability
Description
sysupgrade checks only that fw_setenv exists, then discards all three statuses before entering
the flash path. If environment detection or a write fails, the new image is committed without the
recovery arm, with a stale boot count, or with a persisted verify=n, while the operator receives
no indication.
Code

general/overlay/usr/sbin/sysupgrade[R1352-1354]

+if command -v fw_setenv >/dev/null 2>&1; then
+	fw_setenv upgrade_available 1 2>/dev/null
+	fw_setenv bootcount 0 2>/dev/null
Evidence
All output and statuses from the safety-critical writes are discarded, after which flashing proceeds
unconditionally. The repository's environment-tool implementation explicitly returns errors when it
cannot detect, open, or write its environment configuration.

general/overlay/usr/sbin/sysupgrade[1352-1359]
general/overlay/usr/sbin/sysupgrade[1361-1380]
general/package/all-patches/uboot-tools/0011-env-partition-autosearch.patch[130-159]

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 upgrade proceeds after failed bootloader-environment writes, silently losing recovery protection or retaining unsafe stale values.
## Fix Focus Areas
- general/overlay/usr/sbin/sysupgrade[1352-1359]
- general/package/all-patches/uboot-tools/0011-env-partition-autosearch.patch[130-159]
## Recommended Fix
Check and report every `fw_setenv` result. If arming succeeds but resetting `bootcount` fails, undo the arm and stop before flashing; if arming is unavailable on an unsupported device, emit a prominent diagnostic rather than presenting the upgrade as recovery-protected.

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


4. Healthy cameras remain recovery-armed ✓ Resolved 🐞 Bug ☼ Reliability
Description
S99bootok suppresses both fw_setenv errors and exits zero regardless of whether
upgrade_available was cleared. When Linux can read the environment but cannot write it, each
otherwise healthy boot leaves U-Boot counting and provides neither a retry nor a diagnostic before
recovery escalation.
Code

general/overlay/etc/init.d/S99bootok[R27-29]

+			fw_setenv upgrade_available 0 2>/dev/null
+			fw_setenv bootcount 0 2>/dev/null
+			exit 0
Evidence
The script has already proved that the environment is readable, but it ignores both subsequent write
results and exits immediately. The environment-tool implementation has explicit configuration
failure paths, while the new script's own protocol states that U-Boot continues counting until
upgrade_available is cleared.

general/overlay/etc/init.d/S99bootok[14-29]
general/package/all-patches/uboot-tools/0011-env-partition-autosearch.patch[130-159]

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 disarm write is treated as success, leaving healthy firmware marked as an unsuccessful upgrade without retries or diagnostics.
## Fix Focus Areas
- general/overlay/etc/init.d/S99bootok[23-33]
- general/package/all-patches/uboot-tools/0011-env-partition-autosearch.patch[130-159]
## Recommended Fix
Check the result of clearing `upgrade_available`, log failures visibly, and retry within a bounded interval instead of exiting successfully. Reset `bootcount` only after the flag is cleared, and report a failure if either value cannot be persisted.

ⓘ 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 turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread general/overlay/etc/init.d/S99bootok Outdated
Comment thread general/overlay/usr/sbin/sysupgrade Outdated
Comment thread general/overlay/usr/sbin/sysupgrade Outdated
Comment thread general/overlay/etc/init.d/S99bootok Outdated
Four robustness fixes from review of the arm/disarm pair:

- S99bootok required only a single `pidof majestic` to declare the image
  healthy, but majestic is visible to pidof the moment S95majestic forks,
  before its pipeline is up -- a streamer that crashed during startup would be
  seen "healthy" for an instant and disarm recovery. Now it requires majestic
  to stay up across a sustained window (8 consecutive checks, ~16s); if it dies
  the window restarts, and if it never holds the flag stays armed so U-Boot
  escalates. (findings 1)

- S99bootok ignored the fw_setenv result and exited 0 regardless, so a camera
  whose env is readable but not writable would silently leave the flag set on
  every healthy boot with no diagnostic. It now logs (logger -s) when the
  disarm write fails, and when majestic never stays up. (finding 4)

- sysupgrade armed before the flash but never cleared the flag on an abort that
  wrote nothing. die() now drops the arm on its no-write path (nothing touched,
  camera unchanged). A failure after enter_ramfs still reboots, and S99bootok
  disarms on the next healthy boot -- bootcount advances only one per reboot and
  the limit is several, so an aborted upgrade never escalates falsely.
  (finding 2)

- the arming fw_setenv calls were silent; they now warn if the recovery arm
  cannot be set, or if a stale verify=n cannot be cleared, so the operator is
  not left thinking recovery is in place when it is not. (finding 3)

Re-verified on gk7205v200: the sustained-health disarm holds the flag until
majestic is stably up, then clears it; an unarmed boot is still a no-op.
Shell-parse and strip-comment gates pass.
@widgetii
widgetii merged commit 1d89e5f into master Sep 10, 2026
120 of 121 checks passed
@widgetii
widgetii deleted the sysupgrade-bootcount-arm branch September 10, 2026 12:36
widgetii added a commit that referenced this pull request Sep 11, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant