Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Flash blank check #1971

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ALTracer
Copy link
Contributor

Detailed description

  • This is a small new feature in the framework of Target Flash API.
  • The existing problem is it's hard to check how much of flash is empty (only via (gdb) x/16zw 0x08004000 etc.)
  • This PR solves it by implementing and providing a target remote monitor command to run a blank-check against a connected halted target.

Many alternative gdbservers already implement a similar mechanism -- openocd flash, pyocd, Segger JLinkGDBServer, etc.
I opted to reuse the whatever flash write buffer is allocated in BMP heap for fast block reads (like bmd_crc32). The memcmp could be optimized to run in 4 bytes per comparison, but for a quick PoC 1 byte is enough, I get I/O bounded on swdptap.c (100-150 KiB/s for FS probes).

Tested on BMDA for dual blackpills, as usual, over JTAG transport, example output is

$ ./build/blackmagic -j -f 8M -M blank_check
DPv0 detected based on JTAG IDCode
Blank 0x08020000+131072
Blank 0x08040000+131072
Blank 0x08060000+131072
Blank 0x08010000+65536
Has data at 0x08000000
Blank 0x08004000+16384
Blank 0x08008000+16384
Blank 0x0800c000+16384

Notice how it walks the singly linked flash bank list backwards, but walks eraseblocks forwards. I erased the BMF from DUT, keeping only the BMD bootloader on it (in first 16 KiB). Output in GDB is similar, but there are progress dots printed before some lines.

Needs testing on smaller probes. Future usage could be skipping vFlashErase-triggered erase delays if reading memory-mapped flash is faster than waiting on it to erase (break-even is different for genuine STM32 with true simultaneous mass-erase vs. The Compatibles which back it with some sequential slow QuadSPI-like flash). It could also have interesting interactions with mass-erase -> load (skips all slow sector erases and starts writing to empty flash immediately), eliminating some unresponsiveness.

Your checklist for this pull request

Closing issues

Copy link
Member

@dragonmux dragonmux left a comment

Choose a reason for hiding this comment

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

This has some good ideas and we like the concept. The review items are mostly all about code style and clarity.

src/target/target_flash.c Outdated Show resolved Hide resolved
src/target/target_flash.c Outdated Show resolved Hide resolved
src/target/target_flash.c Outdated Show resolved Hide resolved
src/target/target_flash.c Outdated Show resolved Hide resolved
@ALTracer ALTracer force-pushed the feature/blank-check branch 2 times, most recently from 8c8db53 to aeb352c Compare October 24, 2024 09:30
@dragonmux dragonmux added the Enhancement General project improvement label Oct 26, 2024
@ALTracer ALTracer force-pushed the feature/blank-check branch from aeb352c to c1b2c0a Compare November 21, 2024 15:56
@ALTracer ALTracer force-pushed the feature/blank-check branch from c1b2c0a to c3fbf5b Compare December 30, 2024 11:33
@dragonmux
Copy link
Member

At a quick glance, this is looking really good now. Because we're in a feature freeze for v2.0.0, we will defer this PR to the start of the v2.1 cycle as this introduces new commands and such. We'll give it a proper re-review before then though and aim to merge it at the start of that cycle.

@dragonmux dragonmux added this to the v2.1 release milestone Jan 16, 2025
* Reuse flash write buffer for block reads
* Compare against erased value and skip to next sector on mismatch
* Indicate progress via tc_printf() to gdb_if.c or BMDA stdout
@ALTracer
Copy link
Contributor Author

Because we're in a feature freeze for v2.0.0, we will defer this PR to the start of the v2.1 cycle as this introduces new commands and such.

When did v2.0.0 feature freeze begin? I had implemented and tested this more than two months ago, October 18, posted the PR 19th, and answered/applied all review items October 24th same day I saw them. CI size-diff shows about +340 bytes and I see same size increase for native locally, which now sits at 98.7% utilization. Did I fail to timely mention this (again)? And the initial idea was raised around January of 2024, stimulated by v1.10.0-295 PR1719 split-bank erase bugfix, a year ago, around time when v1.10.2 and v1.9.3 were tagged.

The single new command is in remote-monitor (and BMDA), is not debug-related (does not intend to affect execution), and responds in a similar fashion to monitor crc already present in stm32h7 driver which users may be familiar with. I am already using it to check correctness of some other flash drivers (e.g. which blocks they actually erase) both via BMP and BMDA. Having it in a separate branch slows down interactive live checks. I guess I may try to reimplement it as gdb-scripted qCRC packets per page or per TAR AutoIncr kibibyte against a fixed checksum for 0xFF.

Note that I only mention but not actually implement any interconnections with flash erase_range or vFlashErase/vFlashWrite accelerations, and I don't reorder stm32f4 banks, don't reverse (currently backwards) linked lists of target_flash_s.

Can you then please give an ETA for when v2.1 may happen, or at least post-v2.0.0? In 2026? That gives plenty time for mentioned related-changes to add / build upon this / test in synergy.

@ALTracer ALTracer force-pushed the feature/blank-check branch from c3fbf5b to d5e8780 Compare January 16, 2025 18:26
@dragonmux
Copy link
Member

When did v2.0.0 feature freeze begin?

Start of December, which is why we've been avoiding merging anything that's not a bugfix or already agreed on by that point.

Can you then please give an ETA for when v2.1 may happen, or at least post-v2.0.0? In 2026? That gives plenty time for mentioned related-changes to add / build upon this / test in synergy.

v2.0 final should be out in the next month or two, so.. not more than that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants