Skip to content

feat: Create faster flash mechanism - #551

Open
dperezzoghbi wants to merge 12 commits into
anduril:masterfrom
dperezzoghbi:fast-flash
Open

dperezzoghbi wants to merge 12 commits into
anduril:masterfrom
dperezzoghbi:fast-flash

Conversation

@dperezzoghbi

Copy link
Copy Markdown
Contributor
Description of changes

Adds a "fast-flash" mechanism that doesn't erase the whole SPI NOR flash.
Basic idea is, a full erase/write is slow, and we can save time by keeping good blocks.

I don't want to change existing flows, so this also adds way to interact with the RCM shell. We can set the FAST_FLASH environment variable there, to change flashing behavior at runtime.

Testing

Repeated testing on trivial flashes (mostly the same) and full jp6->jp7 flash which erase 35% of the SPI.
I see ~4 mins saved on trivial flashes and ~1 min saved on significant flashes.

Comment thread overlay-with-config.nix Outdated
exec &> >(tee $ttyGS) <$ttyGS

echo "Press enter within 10s to open a console session before flashing..."
if read -t 10 -r _; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't want to add debugging functionality like this to signed flashing scripts for fused devices. Someone with access to a signed flashing script that allows access to a shell could easily use it to run arbitrary code, bypassing secure boot.

See below in this script where I only allow opening a shell if pkcFile is null (unfused device)

(Also, this adds 10 sec to all flashing?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, dropping this. I had it mainly as a way of testing/gathering perf and I didn't know if we wanted to do it always. I didn't realize the potential secure-boot bypass. Great catch, and thank you.

After getting some performance metrics, I'm settling on a different approach. I have a simple heuristic:
If 40% or more of the SPI were to be erased, then I'll just erase, then write the whole thing, as we did before, but we'll still build/diff the golden image in memory first.

We can tune this 40% metric, but that seemed to be where the performance drop were to hit.

@dperezzoghbi dperezzoghbi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes since the last review:

  1. Dropped the --interactive-pre-flash hook which basically adds a secure-boot hole. Thanks to @danielfullmer for catching this.
  2. Threshold for deciding whether to fast-flash at 40% erasure or less, from local performance metrics.
  3. Added post-write measurement to validate the image is correct.

Not sure if this is wanted, but a potential TODO:
If fast-flash is decided, we might want to invalidate/erase the first block, containing BCT, and write that block last as a special case at the end to ensure the device doesn't boot unless it has properly written everything else. This way, if flashing stops early, we dont try to boot into the unknown image.

Comment thread overlay-with-config.nix Outdated
exec &> >(tee $ttyGS) <$ttyGS

echo "Press enter within 10s to open a console session before flashing..."
if read -t 10 -r _; then

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, dropping this. I had it mainly as a way of testing/gathering perf and I didn't know if we wanted to do it always. I didn't realize the potential secure-boot bypass. Great catch, and thank you.

After getting some performance metrics, I'm settling on a different approach. I have a simple heuristic:
If 40% or more of the SPI were to be erased, then I'll just erase, then write the whole thing, as we did before, but we'll still build/diff the golden image in memory first.

We can tune this 40% metric, but that seemed to be where the performance drop were to hit.

@dperezzoghbi

Copy link
Copy Markdown
Contributor Author

Added BCT Fault tolerance from previous TODO, since I got interested in it. Also fixed future footgun with diff_granularity desync with count blocksize.

Compares two files in fixed-size chunks and prints coalesced ranges
of differing blocks as "start_block count" pairs. Used by
flash-from-device to find which erase blocks of a QSPI device
actually need to be rewritten.
The flashing initrd now prompts on the serial console for 10 seconds
before flashing begins and, once that session ends, sources
/tmp/pre-flash-hook.sh if present. This gives an external caller a
window to set environment variables (e.g. FAST_FLASH) for this
specific flash before flash-from-device runs.

Add --interactive-pre-flash to the generated initrd-flash script:
without it, the host-side expect script answers the prompt and moves
on immediately; with it, control is handed to the caller until the
console session ends. --no-timeout disables the expect script's
overall completion timeout for use alongside it.
Skipping the mandatory full-device erase before flashing QSPI is
safe as long as we verify what's actually on the device first: build
a golden image of what QSPI should contain by staging every
partition's content into a scratch file (reusing
program_spi_partition's existing placement logic, including BCT
copies and secondary_gpt end-of-disk repositioning), read the
device's current contents, and diff the two with diffblocks at
erase-block granularity. Only the erase blocks that actually differ
get erased and rewritten.

Enabled by setting FAST_FLASH in the environment before this script
runs (see the pre-flash console hook). Off by default, the full
device erase and per-partition write both happen exactly as before.
This reverts commit 4084d3f.

The pre-flash prompt handed an interactive root shell over the
console before flashing began, with no gate on
cfg.firmware.secureBoot.pkcFile the way the existing post-failure
console fallback has. On a secure-boot device that's an open
console access, not a locked-down one -- a secure-boot hole.

It could have been fixed with the same pkcFile == null guard, but
the hook only existed to let an external caller set FAST_FLASH
before this script ran. Fast-flash is now attempted unconditionally,
falling back to a full erase only when more than ~40% of the device
differs, so there's no longer a caller-set flag to gate on. Drop the
hook entirely rather than patch around it.
Remove the FAST_FLASH environment variable gate. Partition writes to
QSPI are now unconditionally staged into an in-memory golden image
(renaming spi_write to write_golden) and erase_bootdev always builds
that golden image via fast_flash_init, instead of choosing between a
full device erase/write and the golden-image path based on FAST_FLASH.

This makes diff-based flashing the only code path, in preparation for
deciding the erase strategy dynamically based on how much of the
device actually differs.
FAST_FLASH was added as an option for the flashing mechanism. I measured
the performance on my orin device with/without FAST_FLASH.

Time | Erased | Description
1:15 |   N/A  | Overhead of tegraflash + boot & everything.
1:19 |  <1%   | Trivial Reflash, only UEFI variables/ftw was erased.
4:30 |  <34%  | Major flash, JP6 -> JP7.
5:15 |  100%  | No FAST_FLASH, same whether trivial or not.

From this, I expect a threshold of ~40-50% where erasing the entire SPI is
faster, due to the CHIP_ERASE opcode being used.

Sum the diffblocks ranges to get the total bytes that would need
writing, and if that exceeds fast_flash_threshold_percentage of the
device size, erase and write the whole device instead of erasing and
writing each differing range individually.
Read back the whole mtd0 device after diff_and_program_spi runs and
compare its SHA256 against the golden image, so a partial or corrupt
write is caught immediately instead of surfacing as a boot failure.
Print bytes written, offset, and running percentage after each
erase/write range, so the diff-write path (which can take a while
across many small ranges) shows the same kind of progress as the
per-block erase output instead of going silent between steps.
Fast-flashing erases and writes each differing block individually,
as reported by diffblocks. Erasing is slow, so an interruption
partway through can leave some blocks updated and others not.

This window is wider than before: previously we did a single
CHIP_ERASE followed by a quick write of all partitions, so a
partial failure was unlikely. Splitting erases and writes into many
small steps makes a partial failure much more likely.

We can't prevent an interruption, but we can keep the device from
booting into an inconsistent state. Erase the first block, which
holds the BCT, before touching anything else, and only write it
back once every other block has been written. A partial flash then
leaves the device refusing to boot rather than booting broken.
If diff_granularity is different than 1 later, this is a potential bug.
Just ensure that count is multiplied by diff_granularity when performing
a flash_erase.
Replace the positional A_FILE B_FILE BLOCK_SIZE interface with
dd-style key=value arguments: a=, b=, bs=, count=, a-skip=, b-skip=.
Parse them by hand with a small loop over "key=value" splits --
the tool takes no dependencies and the format is simple enough not
to need one.

a-skip=/b-skip= let a caller skip a prefix of blocks in either input
before comparing, addressed independently since the two files may
need different offsets. count= caps how many blocks to compare (0
means to EOF), matching dd's convention.

This lets flash-from-device.sh skip the BCT block directly instead
of reconstructing zero-padded copies of both inputs through process
substitution just to make diffblocks ignore it. Block indices in
diffblocks' output are now relative to the skip point, so the
per-range loop in diff_and_program_spi adds the skipped block back
to recover an absolute block index.
The EOF check only looked at how many bytes were read from a, so
if b ran out first, b's read buffer kept whatever it held from the
previous block and got compared as if it were fresh data. A file
missing trailing blocks could come back byte-identical to a longer
file that happened to repeat content, silently hiding the length
difference.

Track both read lengths and let Rust's slice comparison do the
right thing: a length mismatch is never equal, so a short read is
now always reported as a differing block, regardless of what bytes
are sitting in the shorter side's buffer.

Document the other side of this: since an infinite source (e.g.
/dev/zero) never reaches EOF, comparing against one without count=
set will never terminate.
Comment thread pkgs/diffblocks/src/src/main.rs

@dperezzoghbi dperezzoghbi left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready now.

The idea is:

  1. Hijack the mtd_debug write's from before to write to a "golden" ram image
  2. Diff the golden ram image with the device state.
  3. Erase what's necessary, or perform a full erase depending on a threshold.
  4. Write the image.
  5. (new feature) validate that what we wanted to write is what's on the device.

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.

3 participants