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

Fix: ADI structural cleanup and ARMv8-M debug improvements #2010

Merged
merged 21 commits into from
Dec 9, 2024

Conversation

dragonmux
Copy link
Member

@dragonmux dragonmux commented Dec 6, 2024

Detailed description

In this PR we address some structural clean-up that is needed in the wake of all the ADIv6 work and some issues in Cortex-M debug of ARMv8-M targets.

This has wound up touching the ADI, Cortex-M, target and GDB layers so apologies that this PR is a little all over the place. With this however, we now have proper diagnostics on what GDB is asking us to do vs what we're doing in regards to step/next/stepi/continue/run/etc and ARMv8-M debug works properly. Tested on a Cortex-M33 and Cortex-M4 (to ensure we didn't break ARMv7-M debug in the process).

This solves what we could reproduce of the final issue raised in #1970. Any further issues in relation to ARMv8-M debug are likely to do with S vs NS modes and what debug is allowed per the DHCSR.

The GDB changes fix GDB trying to guess which "thread" (core) halted when we report halt events back to it, making the output from set debug remote on go from

[remote] wait: enter
  [remote] Packet received: T05
  [remote] select_thread_for_ambiguous_stop_reply: enter
    [remote] select_thread_for_ambiguous_stop_reply: process_wide_stop = 0
    [remote] select_thread_for_ambiguous_stop_reply: first resumed thread is Thread 1
    [remote] select_thread_for_ambiguous_stop_reply: is this guess ambiguous? = 0
  [remote] select_thread_for_ambiguous_stop_reply: exit
[remote] wait: exit

to

[remote] wait: enter
  [remote] Packet received: T05thread:1;
[remote] wait: exit

As far as we can tell this is a fully backwards compatible change, but the GDB documentation does not make this easy to determine.

Your checklist for this pull request

Closing issues

@dragonmux dragonmux added Bug Confirmed bug Enhancement General project improvement GDB Issue/PR related to GDB labels Dec 6, 2024
@dragonmux dragonmux added this to the v2.0 release milestone Dec 6, 2024
@dragonmux dragonmux requested a review from esden December 6, 2024 12:12
…me defines for the magic numbers involved and comments
…ttach()` so we can be sure we actually halted the core before doing things that require it to be halted
… are actually enabled, checking which are when the core halts
…M bits and define what all the bits actually do with comments
… with it having to guess which thread we're reporting for, causing some funky behaviour
… functions for helping nagivate target debugging issues in the future
@dragonmux dragonmux force-pushed the fix/adi-structural-cleanup branch from 75013cf to deeece3 Compare December 8, 2024 20:48
Copy link
Member

@esden esden left a comment

Choose a reason for hiding this comment

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

LGTM

@esden esden merged commit deeece3 into main Dec 9, 2024
34 of 36 checks passed
@dragonmux dragonmux deleted the fix/adi-structural-cleanup branch December 9, 2024 17:05
@florg-32
Copy link

Hey, I've tried it with my M55 dual core board, where I can still scan/attach/load fine. The first time I interrupt after continuing I get this print

(gdb) c
Continuing.
^C[New inferior 2]
warning: No executable has been specified and target does not support
determining executable automatically.  Try using the "file" command.
[New Thread 1]
Truncated register 19 in remote 'g' packet

and any next/step directly triggers a UsageFault (undefined instruction)

LMK if you want to discuss this here or elsewhere

@dragonmux
Copy link
Member Author

Please can you open an issue about this? What you're seeing suggests that we've advertised something to GDB in the target.xml that then fails to line up with what we send GDB in the all registers read packaet. To debug that we're going to need a trace from BMDA at -v 7 (INFO + GDB + TARGET diagnostic levels) and an understanding of what ARMv8-M extensions this M55 implements - FPU, Security, Vector (MVE), DSP, PAC/BTI, etc.. - and what profile it implements (Basic, Main).

BMDA should be dynamically handling whether to deal with a FPU or not and whether the caches are enabled or not, but it's entirely possible this M55 core implements something we're not yet properly dealing with on account of not having seen it in the wild yet.

@florg-32
Copy link

Thanks @dragonmux, I've opened #2018 with as much information as I could find

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug Enhancement General project improvement GDB Issue/PR related to GDB
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants