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

Feature: Working logs on blackpill-f4 via DebugMon#1715

Merged
dragonmux merged 2 commits into
blackmagic-debug:mainfrom
ALTracer:feature/blackpill-f4-debugmon
Jan 7, 2024
Merged

Feature: Working logs on blackpill-f4 via DebugMon#1715
dragonmux merged 2 commits into
blackmagic-debug:mainfrom
ALTracer:feature/blackpill-f4-debugmon

Conversation

@ALTracer
Copy link
Copy Markdown
Contributor

@ALTracer ALTracer commented Jan 7, 2024

Detailed description

  • This is a new feature common to other platforms but missing from this one.
  • There is an existing problem of no log traces available on blackpill-f4 family of platforms.
  • The PR solves the problem by adding required pieces and fixing linker flags.

How to reproduce the issue: build with PROBE_HOST=blackpill-f411ce ENABLE_DEBUG=1, optionally also with BMP_BOOTLOADER=1, upgrade/flash to the board, open gdb, ask for monitor debug_bmp enable (aka "mon debug en") -- Target does not support this command. Even though the binary is 40 KiB bigger.

I've been carrying the patches to use logging on my platform of choice since probably June 2023, which is when I started using the project, and I'm tired of unstashing them on top of every feature branch I'm testing just to see some important DEBUG_INFO()s. Both stlink and swlink work without such changes (but they can't fit both logs and default targets, which is a separate problem solved by an edited Makefile dirtying the working copy).

After another round through reading up on semihosting and debugging down through how it actually works, I now understand how this optional BMP feature was implemented on native all the way back in 2016 in PR151 966f360

Note two things: 1) I do not intend to downgrade to newlib-nano in BMPbootloader or in BMF, because it provides a faster memcpy and actual working mallopt+malloc_trim. 2) That means that the bootloader with 12-symbol serialno will no longer fit under 16 KiB, but I have a proposal in a separate PR to deal with it. Platform MaskROM generates a 12-symbol serial number when enumerating on USB, and I would like to enforce feature parity with it.

Your checklist for this pull request

  • I've read the Code of Conduct
  • I've read the guidelines for contributing to this repository
  • It builds for hardware native (make PROBE_HOST=native)
  • It builds as BMDA (make PROBE_HOST=hosted)
  • I've tested it to the best of my ability
  • My commit messages provide a useful short description of what the commits do

Closing issues

@dragonmux dragonmux added this to the v2.0 release milestone Jan 7, 2024
@dragonmux dragonmux added Enhancement General project improvement BMP Firmware Black Magic Probe Firmware (not PC hosted software) Foreign Host Board Non Native hardware to runing Black Magic firmware on labels Jan 7, 2024
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. We haven't the hardware to test it, so if you say this works (and hey, this explains some things in the native platform too that didn't have comments!) we'll trust you on that.

Given your PR #1716 which deals with the bootloader space complaint, we'll provisionally approve this but intend to merge it after that other PR so main doesn't wind up broken for an indeterminate amount of time for this platform.

@ALTracer
Copy link
Copy Markdown
Contributor Author

ALTracer commented Jan 7, 2024

Eh, I don't feel so confident about the Makefile.inc change. Is --nosys.specs handled right and is it better/proper way than -lnosys? There is another variant of the commit in PR1717 as third in series, but somewhy I though appending --nano.specs was a good idea (not bootloader space constraints but feature/libc/impl parity of firmware with other platforms, or making it fit below 0x08020000), and I didn't rebase it out of there.
Please review through all three PRs as they are related, and I tried to lift / rephrase arguments out of commit messages.

@dragonmux
Copy link
Copy Markdown
Member

-lnosys is implied by --specs=nosys.specs, so that change is definitely fine - and the logic here looks identical, from memory, to the native platform's for the same, so ignoring the difference of not dragging --specs=nano.specs in, it reads to us correctly per the stated goals of the PR.

* Propagate nosys for BMPBootloader into the makefile target because it conflicts with rdimon
* Switch between nosys.specs and rdimon.specs for payload firmware
* Bump DFU serial length to 12+1 to match MaskROM DFU and once utoa_upper is available
* Keep both parts full newlib (not nano) because we're not space-constrained and it's faster and has more libc features
@dragonmux
Copy link
Copy Markdown
Member

Now that the other PR is merged, if you want to check over this one still functions then we're happy to merge this and clear the way for #1717

* Flip `SCS->DEMCR |= MON_EN` for DebugMon to catch semihosting breakpoints
  when there is no upstream debugger and we want to log
* Declare PLATFORM_HAS_DEBUG to pull conditionally compiled statements
@ALTracer ALTracer force-pushed the feature/blackpill-f4-debugmon branch from f3bc806 to b82a3b4 Compare January 7, 2024 18:50
@ALTracer
Copy link
Copy Markdown
Contributor Author

ALTracer commented Jan 7, 2024

This PR of two commits provides/enables the mentioned feature. I swapped the commit order because my pre-commit hook indicated a build of one of four combinations failing to link (probably nosys/rdimon conflict) unresolved symbol: initialise_semihosting_handles(). So better keep the commits buildable.

make -C src/ clean
make -C src/ PROBE_HOST=blackpill-f411ce CROSS_COMPILE="ccache arm-none-eabi-" -j4 || exit 13
make -C src/ clean
make -C src/ PROBE_HOST=blackpill-f411ce CROSS_COMPILE="ccache arm-none-eabi-" ENABLE_DEBUG=1 -j4 || exit 14
make -C src/ clean
make -C src/ PROBE_HOST=blackpill-f411ce CROSS_COMPILE="ccache arm-none-eabi-" BMP_BOOTLOADER=1 -j4 || exit 15
make -C src/ clean
make -C src/ PROBE_HOST=blackpill-f411ce CROSS_COMPILE="ccache arm-none-eabi-" BMP_BOOTLOADER=1 ENABLE_DEBUG=1 -j4 || exit 16
  • v1.10.0-292-g6261eb5b, both normal/Release and dev-log-trace/Debug: probe operates as-if no debug_bmp is available, but binary sizes are different.
  • v1.10.0-293-g42f280a9: Flashing the build of first commit with debug enabled produces a probe hanging upon first monitor swdp_scan, ... Release build is unaffected.
  • v1.10.0-294-gb82a3b45: Flashing the build of [both commits stacked] with debug enabled produces a healthy probe printing nothing interesting to ttyBmpTarg unless asked for monitor debug_bmp enable and working otherwise. Release build is unaffected.

... probably because it goes through DEBUG_INFO(), PLATFORM_PRINTF(), printf(), _swiwrite() or _swistat(), and a breakpoint (which in absence of SCS->DHCSR & MON_EN is equivalent to a HardFault exception, not DebugMon exception), not passing further to debug_monitor_handler() or overridden _write() into debug_serial_debug_write(). By the way, this was the behaviour until now, and coincidentally on meson branches.

Update: I downgraded from ARM toolchain 12.2.Rel1 to Ubuntu/jammy GCC 10.3-2021.07.4, and because I rebuilt in-repo libnewlib-arm-none-eabi keeping the debugging symbols info (2.0 GiB for 20+ target flavours), I get to see into newlib internals when hot-attaching to a hung probe with a second BMP-compatible.

(gdb) bt
#0  blocking_handler () at ../../cm3/vector.c:104
#1  <signal handler called>
#2  0x080256a6 in do_AngelSWI (arg=<optimized out>, reason=12) at ../../../../../../../libgloss/arm/swi.h:118
#3  _swistat (fd=<optimized out>, st=0x2001fce0) at ../../../../../../../libgloss/arm/syscalls.c:746
#4  0x08024592 in _fstat_r (ptr=ptr@entry=0x20000138 <impure_data>, fd=<optimized out>, pstat=pstat@entry=0x2001fce0) at ../../../../../../../../newlib/libc/reent/fstatr.c:55
#5  0x08021c38 in __swhatbuf_r (couldbetty=<synthetic pointer>, bufsize=<synthetic pointer>, fp=0x2000048c <impure_data+852>, ptr=0x20000138 <impure_data>) at ../../../../../../../../newlib/libc/stdio/makebuf.c:96
#6  __smakebuf_r (ptr=ptr@entry=0x20000138 <impure_data>, fp=fp@entry=0x2000048c <impure_data+852>) at ../../../../../../../../newlib/libc/stdio/makebuf.c:52
#7  0x0802068e in __swsetup_r (ptr=ptr@entry=0x20000138 <impure_data>, fp=fp@entry=0x2000048c <impure_data+852>) at ../../../../../../../../newlib/libc/stdio/wsetup.c:71
#8  0x080218a2 in __sfvwrite_r (ptr=ptr@entry=0x20000138 <impure_data>, fp=fp@entry=0x2000048c <impure_data+852>, uio=uio@entry=0x2001fd94) at ../../../../../../../../newlib/libc/stdio/fvwrite.c:63
#9  0x0801c1d4 in _puts_r (ptr=0x20000138 <impure_data>, s=s@entry=0x802993c "Resetting TAP") at ../../../../../../../../newlib/libc/stdio/puts.c:91
#10 0x0801c1fa in puts (s=s@entry=0x802993c "Resetting TAP") at ../../../../../../../../newlib/libc/stdio/puts.c:129
#11 0x08009e8c in jtag_scan () at target/jtag_scan.c:89
#12 0x08005ef6 in cmd_jtag_scan (target=<optimized out>, argc=<optimized out>, argv=<optimized out>) at command.c:218
#13 0x080060c0 in command_process (t=0x0, cmd_buffer=cmd_buffer@entry=0x2001feb0 "j") at command.c:152
#14 0x0800871a in exec_q_rcmd (packet=<optimized out>, length=<optimized out>) at gdb_main.c:393
#15 0x08008c02 in exec_command (exec=0x8029034 <q_commands>, length=<optimized out>, packet=<optimized out>) at gdb_main.c:374
#16 handle_q_packet (length=<optimized out>, packet=<optimized out>) at gdb_main.c:591
#17 gdb_main_loop (tc=tc@entry=0x20000010 <gdb_controller>, pbuf=pbuf@entry=0x20000dd8 <pbuf> "qRcmd,6a", pbuf_size=pbuf_size@entry=1024, size=8, in_syscall=in_syscall@entry=false) at gdb_main.c:348
#18 0x08008ec6 in gdb_main (pbuf=pbuf@entry=0x20000dd8 <pbuf> "qRcmd,6a", pbuf_size=pbuf_size@entry=1024, size=<optimized out>) at gdb_main.c:802
#19 0x0800d8ea in bmp_poll_loop () at main.c:69
#20 main (argc=<optimized out>, argv=<optimized out>) at main.c:85

So indeed enabling DebugMon vector is the solution.

@dragonmux
Copy link
Copy Markdown
Member

That sounds like a fully working patch to us! We'll get this merged then, thank you 🙂

@dragonmux dragonmux merged commit a97e3a2 into blackmagic-debug:main Jan 7, 2024
@ALTracer ALTracer mentioned this pull request Jan 19, 2024
6 tasks
@ALTracer ALTracer deleted the feature/blackpill-f4-debugmon branch April 26, 2024 17:19
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 Foreign Host Board Non Native hardware to runing Black Magic firmware on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants