Skip to content
This repository was archived by the owner on Mar 7, 2026. It is now read-only.

common: remove rdimon when debug is enabled#1741

Merged
dragonmux merged 2 commits into
blackmagic-debug:mainfrom
tlyu:fix/no-rdimon
Oct 30, 2024
Merged

common: remove rdimon when debug is enabled#1741
dragonmux merged 2 commits into
blackmagic-debug:mainfrom
tlyu:fix/no-rdimon

Conversation

@tlyu
Copy link
Copy Markdown
Contributor

@tlyu tlyu commented Jan 19, 2024

Save about 1.2KiB (depending on GCC version) by redirecting _write directly in libnosys, instead of pulling in librdimon.

Detailed description

  • Improves firmware footprint on native BMP when debug output is enabled
  • There doesn't appear to be a current need to use librdimon semihosting capabilities
  • libnosys I/O functions such as _write can be retargeted directly without using semihosting
  • Initializing librdimon semihosting brings in a lot of unused code

I haven't tested on other probe platforms, so I might have unintentionally broken something.

Your checklist for this pull request

Closing issues

N/A

@tlyu tlyu changed the title common: remove rdimon common: remove rdimon when debug is enabled Jan 19, 2024
@ALTracer
Copy link
Copy Markdown
Contributor

Please rebase on main -- half of usb_serial.c moved to syscalls.c. I just tried rebasing your commit on main and got a -1344 byte size-diff for meson setup build-stlink --cross-file=cross-file/stlink.ini -Ddebug_output=true -Dbmd_bootloader=false -Dtargets="cortexm,at32f4,rp,stm", which is very nice. While writes definitely don't have to go through DebugMon handler, I expect another bonus: RDP1-locked stlinkv2 (and RDP2-locked stlinkv3) may stop hanging on their first DEBUG_INFO() call, because it no longer uses breakpoints or any (disabled) debug facilities. I can't confirm whether ST disables DebugMon, but it might be the case. Needs testing on ST_BOOTLOADER=1 stlinkv2 and stlinkv3 to confirm the benefit (of allowing users to do all the DEBUG_INFO prints on-probe without BMDA). Or any stm32f103 with normal BMD bootloader but read protection activated after confirming DEBUG_INFO works, to observe that it starts crashing.
If you don't want to handle all the Makefile.inc per-platform, I might handle it myself, checking the result on a few bluepills. f072 can't use debugmon anyways (goes into hardfault), so it's a win-win for it.

 src/platforms/common/stm32/meson.build |  2 +-
 src/platforms/common/syscalls.c        | 65 -----------------------------------------------------------------
 src/platforms/common/usb_serial.c      | 12 ------------
 3 files changed, 1 insertion(+), 78 deletions(-)

@tlyu
Copy link
Copy Markdown
Contributor Author

tlyu commented Oct 16, 2024

Thanks for taking a look! I will try to make time to work on this a bit next week.

@dragonmux
Copy link
Copy Markdown
Member

Please rebase this on main. We'll need to have a chat with Esden about debug methodologies, but principally this change seems fine to us. If we want to properly re-introduce semihosting based debug output, we can do so anyway without pulling in rdimon.spec using the lessons learned in https://github.com/blackmagic-debug/blackmagic-test-fw-archive/tree/main/semihosting/stm32f411 while preventing pulling in large amounts of unused code and the complicated spaghetti that the newlib implementation makes.

@dragonmux dragonmux added this to the v2.0 release milestone Oct 26, 2024
@dragonmux dragonmux added Enhancement General project improvement BMP Firmware Black Magic Probe Firmware (not PC hosted software) labels Oct 26, 2024
tlyu added 2 commits October 29, 2024 14:44
Save about 1.2KiB (depending on GCC version) by redirecting `_write`
directly in `libnosys`, instead of pulling in `librdimon`.
Make most syscall stubs unconditional. Remove semihosting routing for
debug output.
@tlyu
Copy link
Copy Markdown
Contributor Author

tlyu commented Oct 29, 2024

Rebased. Builds for me, haven't tested yet.

@tlyu
Copy link
Copy Markdown
Contributor Author

tlyu commented Oct 29, 2024

With -Dtargets=cortexm,nrf,rp,sam,stm,ti and ArmGNUToolchain 13.2.Rel1:

Before:

Memory region         Used Size  Region Size  %age Used
             rom:      123560 B       128 KB     94.27%
             ram:        8228 B        20 KB     40.18%

After:

Memory region         Used Size  Region Size  %age Used
             rom:      122208 B       128 KB     93.24%
             ram:        8048 B        20 KB     39.30%

@dragonmux
Copy link
Copy Markdown
Member

That's a pretty nice saving! We'll get this reviewed in the next 24h we hope, with the aim to merge it once you can confirm it still works as intended.

@tlyu
Copy link
Copy Markdown
Contributor Author

tlyu commented Oct 29, 2024

Did a quick check scanning and attaching a nRF target with BMP. Debug output to aux CDC as expected. (At least as far as I can recall; I don't remember all the details of how that debug output should look.)

Copy link
Copy Markdown
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 all LGTM, so merging. Thank you for the contribution! Sorry that it took quite so long to get to merging this.

@dragonmux dragonmux merged commit 08ff1a8 into blackmagic-debug:main Oct 30, 2024
@ALTracer ALTracer mentioned this pull request Mar 30, 2025
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

BMP Firmware Black Magic Probe Firmware (not PC hosted software) Enhancement General project improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants