[sonic-installer] Add dedicated SONiC-BMC U-Boot bootloader by william8545 · Pull Request #4602 · sonic-net/sonic-utilities

william8545 · 2026-06-11T05:02:57Z

Address issue - #4548

What I did

Added a dedicated BmcUbootBootloader so sonic-installer manages SONiC-BMC boot state correctly. SONiC-BMC platforms (ASPEED AST2700) run U-Boot with a two-slot environment, which the generic UbootBootloader does not model — on a BMC it previously fell through to that generic handler and operated on the wrong environment variables.

Specifically:

New sonic_installer/bootloader/bmc_uboot.py (BmcUbootBootloader, NAME = 'bmc-uboot'), selected via utilities_common.chassis.is_bmc().
Registered first in BOOTLOADERS so a stray /host/grub/grub.cfg cannot preempt it, and guarded UbootBootloader.detect() to exclude BMC.
Full two-slot lifecycle: list, set-default, set-next-boot, install, remove, cleanup, verify-next-image, set-fips/get-fips, binary-version, plus a BMC-specific verify_image_platform (fails closed).
Unit tests in tests/installer_bootloader_bmc_uboot_test.py (12 tests).

No change to sonic_installer/main.py — the work is confined to the bootloader plugin and its test.

How I did it

BmcUbootBootloader subclasses OnieInstallerBootloader and models the ASPEED AST2700 two-slot U-Boot environment:

Concept	Slot 1	Slot 2
version var	`sonic_version_1`	`sonic_version_2`
boot command	`run sonic_image_1`	`run sonic_image_2`
kernel args	`linuxargs`	`linuxargs_old`

Empty slots are written/read as the marker None (also accepts NONE/empty, case-insensitively).

Boot selection (Aboot-style contract): set-default writes the persistent boot_next and clears the one-shot boot_once; set-next-boot writes only boot_once. get_next_image() resolves boot_once first (it wins, matching U-Boot bootcmd), then boot_next, and surfaces an empty/unrecognized selector as a raw string so a broken boot state stays visible rather than being masked.
Install: install_image() runs the image's own installer (bash <image>, which performs the slot rotation and sets boot_next) and then clears any stale boot_once that would otherwise shadow the new default.
Remove: remove_image() repoints boot_once/boot_next to the surviving slot before clearing the removed slot's version + auxiliary vars and deleting its rootfs; it refuses to remove the only populated slot.
FIPS: set-fips/get-fips rewrite the sonic_fips= token in the target slot's linuxargs/linuxargs_old only (per-slot isolation).
Platform check: verify_image_platform() mirrors the GRUB approach — extracts installer/platforms_asic from the image and matches the running device_info.get_platform() with grep -Fxq (whole-line). It fails closed: compatible only when grep matches, so a malformed/manifest-less payload is rejected rather than silently accepted.

How to verify it

On a SONiC-BMC (ASPEED AST2700) board, the dedicated bootloader is auto-selected and the full command surface was exercised end-to-end, cross-checking the U-Boot environment (fw_printenv) and, across real reboots, /proc/cmdline:

sonic-installer list / verify-next-image reflect the two-slot state.
set-default <img> sets boot_next and clears boot_once; set-next-boot <img> sets a one-shot boot_once that U-Boot consumes on the next reboot and then reverts to boot_next.
install <bmc.bin> rotates the new image into a slot and boots it after reboot.
remove / cleanup clear the slot's vars, repoint the boot selectors, and delete the rootfs.
set-fips/get-fips toggle sonic_fips= in the correct per-slot args.
A wrong-platform (switch) image is rejected by verify_image_platform; --skip-platform-check overrides it.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

SONiC-BMC runs U-Boot with a fixed two-slot ASPEED environment that the shared UbootBootloader mishandles. Add BmcUbootBootloader: Aboot-style boot contract (set-default wins, boot_once one-shot), exact slot match, per-slot FIPS, and platforms_asic verification. Detect via is_bmc() and guard UbootBootloader.detect() to exclude BMC. Signed-off-by: William Tsai <willtsai@nvidia.com>

…te set-fips echo Make BmcUbootBootloader the first entry in BOOTLOADERS so a BMC is never preempted by an accidental/stale /host/grub/grub.cfg. This is safe because BmcUbootBootloader.detect() is is_bmc(), which is False on every non-BMC platform, so probing it first cannot mis-select on non-BMC systems. Drop the duplicate 'Done' echo in set_fips(); the CLI already prints the user-facing success message. Add a regression test asserting BMC wins when both BMC and GRUB detectors return true. Signed-off-by: William Tsai <willtsai@nvidia.com>

…m; add tests verify_image_platform() returned True whenever the tar extraction of installer/platforms_asic failed, so a malformed/non-tar payload or one missing the manifest silently passed platform validation. Key the result on grep (p3) instead: compatible only when the running platform is explicitly listed. This fails closed on malformed/missing manifests while still accepting an early grep match where tar is then SIGPIPE'd (which would wrongly fail if keyed on tar's return code). Add unit tests for verify_image_platform (match / mismatch / fail-closed / SIGPIPE-match / non-file), fw_printenv read-failure handling, get_installed_images empty-slot skipping, and the only-populated-slot remove guard. Signed-off-by: William Tsai <willtsai@nvidia.com>

…mment Signed-off-by: William Tsai <willtsai@nvidia.com>

…-boot Signed-off-by: William Tsai <willtsai@nvidia.com>

mssonicbld · 2026-06-11T05:03:05Z

/azp run

azure-pipelines · 2026-06-11T05:03:15Z

Azure Pipelines successfully started running 1 pipeline(s).

…fy flake8 E501 The single-line 'return ... or ... or ...' in get_next_image() was 128 characters, exceeding the 120-char limit enforced by the pre-commit flake8 check (Pretest Static Analysis). Wrap it across lines; behavior is unchanged. Signed-off-by: William Tsai <willtsai@nvidia.com>

mssonicbld · 2026-06-11T05:16:44Z

/azp run

azure-pipelines · 2026-06-11T05:16:55Z

Azure Pipelines successfully started running 1 pipeline(s).

yxieca

Reviewed the dedicated SONiC-BMC U-Boot bootloader. This is a clean, security-conscious plugin: transparent on every existing platform (no CLI/output change), the two-slot lifecycle is correct, verify_image_platform faithfully mirrors the GRUB pipeline and fails closed, and the 12 tests are genuinely thorough — including negative cases (only-slot abort, graceful fw_printenv failure, fail-closed platform check) and a test that locks in the detect() mutual-exclusion contract.

I paid particular attention to the one change with cross-platform blast radius — registering BmcUbootBootloader first in BOOTLOADERS plus the generic UbootBootloader.detect() guard. Verified it's safe: is_bmc() short-circuits False on non-BMC platforms (with a hasattr guard for older sonic_py_common), the two detect() paths are mutually exclusive, and it's regression-locked by test_detect_uses_bmc_and_generic_uboot_excludes_bmc. Existing switches see zero behavior change.

A couple of non-blocking items:

install_image relies on an implicit cross-repo contract. It assumes the image's embedded installer rotates the new image into slot 1 (boot_next = run sonic_image_1) and then just clears boot_once; it never verifies which slot actually received the image. That matches buildimage's sonic-program-uboot-env.sh today, so it's correct — but it's an undocumented coupling that would silently mis-select if the installer's slot rotation ever changed. Suggest either an inline comment naming the buildimage contract, or re-reading the resulting slot after the install instead of assuming slot 1.
Backport scope. This is new-platform enablement (ASPEED AST2700 BMC), so master-only seems right. If the BMC platform is being shipped on a release branch, this would need to go with the buildimage + is_switch_bmc pieces on that branch too. Worth a one-line confirmation.

Neither blocks. Nice work — the test coverage and the fail-closed platform check in particular.

AI agent on behalf of Ying.

shreyansh-nexthop

LGTM

chander-nexthop · 2026-06-15T09:58:44Z

IMHO, the fixes for set_fips, remove_image, verify_image_platform, and get_next_image (checking and returning the image set in boot_once) are generic to bring into UbootBootLoader itself. In fact, we had the changes for get_next _image checking boot_once locally but for some reason, this fix was missed in the upstream pr we raised.

Regarding set_default behavior to clear boot_once, IIUC, even grub.py is implemented as it's been done for uboot (default and boot_once remain orthogonal). Only Aboot seem to have taken a different approach. I am a little confused with the base Uboot class matching Grub's and the specialized Aspeed class matching Aboot's. In addition, assuming most of the existing deployments use Grub, I would prefer BMC to follow that model as customers workflow may not expect the BMC to behave differently than their grub based host (x86) or uboot based host (marvell)

Adding @pavannaregundi @antony-rheneus for their inputs too.

william8545 · 2026-06-17T01:08:37Z

chander-nexthop

UbootBootLoader

PR #4489 fixed UbootBootloader directly, but based on the discussion and feedback there: other vendors who use U-Boot claim the existing UbootBootloader works for them, and the issue only happens on BMC (which boots a FIT image), so they preferred not to modify it and risk introducing any degradation. I therefore closed #4489 and moved the work into a dedicated BmcUbootBootloader.

One-shot mechanism

BMC should behave like the operator's other hosts, and GRUB is the right reference.
That's actually the argument for clearing the one-shot on set-default — because that is what GRUB already does. The confusion is that the clearing isn't in grub.py; it's done inside the stock grub-set-default helper that grub.py shells out to:

def set_default_image(self, image):                 # sonic-installer set-default
    run_command(['grub-set-default', '--boot-directory=/host', str(idx)])
def set_next_image(self, image):                     # sonic-installer set-next-boot
    run_command(['grub-reboot', '--boot-directory=/host', str(idx)])

grub.py only delegates; grub-set-default itself unsets the one-shot.

1. GRUB source — grub-set-default unsets next_entry. The operative lines (the final three grub-editenv calls) of util/grub-set-default.in:

$grub_editenv ${grubdir}/grubenv unset prev_saved_entry
$grub_editenv ${grubdir}/grubenv unset next_entry      # <-- clears any pending grub-reboot (set-next-boot)
$grub_editenv ${grubdir}/grubenv set saved_entry="$entry"

grub-reboot (= set-next-boot), in the normal path, ultimately sets only next_entry and does not disturb the current persistent default (util/grub-reboot.in):

$grub_editenv ${grubdir}/grubenv set next_entry="$entry"   # saved_entry left as-is

So the relationship is not orthogonal — it's deliberately asymmetric: set-default supersedes a pending set-next-boot; set-next-boot does not disturb the default.

2. This is long-standing GRUB behavior, not a recent or distro-specific quirk. The next_entry one-shot model (and the unset next_entry in grub-set-default) was introduced upstream in 2013 by commit 5945c2f8 — "Reimplement grub-reboot to not depend on saved_entry. Use next_entry variable for one time boot menu entry." It is present from the grub-2.02 tag through the current release grub-2.14 and upstream master, and I verified it directly on the tested switch.

3. Verified on real hardware — a GRUB x86 switch, driving the actual sonic-installer commands and reading grub-editenv .../grubenv list between steps:

Sequence	grubenv result	Effect
`set-next-boot B` then `set-default A`	`next_entry` removed, only `saved_entry=A`	`set-default` erased the pending `set-next-boot` → next boot = A
`set-default A` then `set-next-boot B`	both `saved_entry=A` + `next_entry=B` present	`set-next-boot` kept the default → next boot = B once, then A

This matches the source exactly.

What that means for this PR

GRUB: set-default clears the one-shot.
Aboot: set_default_image() writes both SWI and SWI_DEFAULT, i.e. it also overrides the pending one-shot.
BmcUbootBootloader (this PR): set_default_image() writes boot_next and clears boot_once — the same contract as GRUB and Aboot.

So the genuinely orthogonal one is the current generic UbootBootloader (set_default_image() doesn't clear boot_once, and get_next_image() ignores boot_once) — that's the class that diverges from GRUB, Aboot and BMC.

Reference links

GRUB grub-set-default source (unsets next_entry): https://cgit.git.savannah.gnu.org/cgit/grub.git/tree/util/grub-set-default.in
GRUB grub-reboot source (sets next_entry): https://cgit.git.savannah.gnu.org/cgit/grub.git/tree/util/grub-reboot.in
Introducing commit (2013), present from grub-2.02 to HEAD: https://cgit.git.savannah.gnu.org/cgit/grub.git/commit/?id=5945c2f8496cab51b64ed9bd35d494dc04b8e5a6
SONiC writes this grub.cfg header itself (saved_entry/next_entry block): https://github.com/sonic-net/sonic-buildimage/blob/master/installer/default_platform.conf
Prior attempt to fix the generic class (closed): [sonic-installer] uboot: fix two-slot semantics, boot_once handling, and platform check #4489

chander-nexthop · 2026-06-17T04:40:55Z

@william8545 Thank you for the details. I missed checking the GRUB source code and only went with grub.py. I don't have any more comments. Thanks for fixing this.

chander-nexthop

LGTM

mssonicbld · 2026-07-01T05:36:33Z

Cherry-pick PR to msft-202608: Azure/sonic-utilities.msft#386

…t#4602) Address issue - sonic-net#4548 What I did Added a dedicated BmcUbootBootloader so sonic-installer manages SONiC-BMC boot state correctly. SONiC-BMC platforms (ASPEED AST2700) run U-Boot with a two-slot environment, which the generic UbootBootloader does not model — on a BMC it previously fell through to that generic handler and operated on the wrong environment variables. Specifically: New sonic_installer/bootloader/bmc_uboot.py (BmcUbootBootloader, NAME = 'bmc-uboot'), selected via utilities_common.chassis.is_bmc(). Registered first in BOOTLOADERS so a stray /host/grub/grub.cfg cannot preempt it, and guarded UbootBootloader.detect() to exclude BMC. Full two-slot lifecycle: list, set-default, set-next-boot, install, remove, cleanup, verify-next-image, set-fips/get-fips, binary-version, plus a BMC-specific verify_image_platform (fails closed). Unit tests in tests/installer_bootloader_bmc_uboot_test.py (12 tests). No change to sonic_installer/main.py — the work is confined to the bootloader plugin and its test. How I did it BmcUbootBootloader subclasses OnieInstallerBootloader and models the ASPEED AST2700 two-slot U-Boot environment: Concept Slot 1 Slot 2 version var sonic_version_1 sonic_version_2 boot command run sonic_image_1 run sonic_image_2 kernel args linuxargs linuxargs_old Empty slots are written/read as the marker None (also accepts NONE/empty, case-insensitively). Boot selection (Aboot-style contract): set-default writes the persistent boot_next and clears the one-shot boot_once; set-next-boot writes only boot_once. get_next_image() resolves boot_once first (it wins, matching U-Boot bootcmd), then boot_next, and surfaces an empty/unrecognized selector as a raw string so a broken boot state stays visible rather than being masked. Install: install_image() runs the image's own installer (bash <image>, which performs the slot rotation and sets boot_next) and then clears any stale boot_once that would otherwise shadow the new default. Remove: remove_image() repoints boot_once/boot_next to the surviving slot before clearing the removed slot's version + auxiliary vars and deleting its rootfs; it refuses to remove the only populated slot. FIPS: set-fips/get-fips rewrite the sonic_fips= token in the target slot's linuxargs/linuxargs_old only (per-slot isolation). Platform check: verify_image_platform() mirrors the GRUB approach — extracts installer/platforms_asic from the image and matches the running device_info.get_platform() with grep -Fxq (whole-line). It fails closed: compatible only when grep matches, so a malformed/manifest-less payload is rejected rather than silently accepted. How to verify it On a SONiC-BMC (ASPEED AST2700) board, the dedicated bootloader is auto-selected and the full command surface was exercised end-to-end, cross-checking the U-Boot environment (fw_printenv) and, across real reboots, /proc/cmdline: sonic-installer list / verify-next-image reflect the two-slot state. set-default <img> sets boot_next and clears boot_once; set-next-boot <img> sets a one-shot boot_once that U-Boot consumes on the next reboot and then reverts to boot_next. install <bmc.bin> rotates the new image into a slot and boots it after reboot. remove / cleanup clear the slot's vars, repoint the boot selectors, and delete the rootfs. set-fips/get-fips toggle sonic_fips= in the correct per-slot args. A wrong-platform (switch) image is rejected by verify_image_platform; --skip-platform-check overrides it. Previous command output (if the output of a command-line utility has changed) New command output (if the output of a command-line utility has changed) Signed-off-by: William Tsai <willtsai@nvidia.com> Signed-off-by: Yogapriya Mohankumar <ymohanku@cisco.com>

william8545 added 5 commits June 10, 2026 22:02

[sonic-installer] BMC bootloader: drop redundant get_current_image co…

8430aa1

…mment Signed-off-by: William Tsai <willtsai@nvidia.com>

[sonic-installer] doc: clarify set-default overrides pending set-next…

2163d05

…-boot Signed-off-by: William Tsai <willtsai@nvidia.com>

oleksandrivantsiv approved these changes Jun 11, 2026

View reviewed changes

This was referenced Jun 11, 2026

U-Boot sonic-installer behavior requiring fixes #4548

Open

[sonic-installer] uboot: fix two-slot semantics, boot_once handling, and platform check #4489

Closed

oleksandrivantsiv requested review from judyjoseph and yxieca June 11, 2026 23:35

yxieca approved these changes Jun 12, 2026

View reviewed changes

oleksandrivantsiv added the Request for 202605 Branch label Jun 12, 2026

shreyansh-nexthop approved these changes Jun 15, 2026

View reviewed changes

chander-nexthop approved these changes Jun 17, 2026

View reviewed changes

oleksandrivantsiv merged commit 692dcc0 into sonic-net:master Jun 26, 2026
9 checks passed

yxieca added Request for msft-202608 Branch Approved for msft-202608 Branch and removed Request for 202605 Branch labels Jun 30, 2026

mssonicbld mentioned this pull request Jul 1, 2026

[action] [PR:4602] [sonic-installer] Add dedicated SONiC-BMC U-Boot bootloader Azure/sonic-utilities.msft#386

Merged

mssonicbld added the Created PR to msft-202608 Branch label Jul 1, 2026

mssonicbld added the Included in msft-202608 Branch label Jul 1, 2026

mssonicbld removed the Created PR to msft-202608 Branch label Jul 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[sonic-installer] Add dedicated SONiC-BMC U-Boot bootloader#4602

[sonic-installer] Add dedicated SONiC-BMC U-Boot bootloader#4602
oleksandrivantsiv merged 6 commits into
sonic-net:masterfrom
william8545:shadow-pr/pr-115

william8545 commented Jun 11, 2026 •

edited

Loading

Uh oh!

mssonicbld commented Jun 11, 2026

Uh oh!

azure-pipelines Bot commented Jun 11, 2026

Uh oh!

mssonicbld commented Jun 11, 2026

Uh oh!

azure-pipelines Bot commented Jun 11, 2026

Uh oh!

yxieca left a comment

Uh oh!

shreyansh-nexthop left a comment

Uh oh!

chander-nexthop commented Jun 15, 2026 •

edited

Loading

Uh oh!

william8545 commented Jun 17, 2026

Uh oh!

chander-nexthop commented Jun 17, 2026

Uh oh!

chander-nexthop left a comment

Uh oh!

Uh oh!

mssonicbld commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

william8545 commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Uh oh!

mssonicbld commented Jun 11, 2026

Uh oh!

azure-pipelines Bot commented Jun 11, 2026

Uh oh!

mssonicbld commented Jun 11, 2026

Uh oh!

azure-pipelines Bot commented Jun 11, 2026

Uh oh!

yxieca left a comment

Choose a reason for hiding this comment

Uh oh!

shreyansh-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

chander-nexthop commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

william8545 commented Jun 17, 2026

UbootBootLoader

One-shot mechanism

What that means for this PR

Reference links

Uh oh!

chander-nexthop commented Jun 17, 2026

Uh oh!

chander-nexthop left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mssonicbld commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

william8545 commented Jun 11, 2026 •

edited

Loading

chander-nexthop commented Jun 15, 2026 •

edited

Loading