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

Cortex-M: Temptative fix for watchpoints so they work on ARMv8-M cores (eg Cortex-M33) #2008

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

mean00
Copy link

@mean00 mean00 commented Nov 30, 2024

Detailed description

The DWT block is different on Arm V8m compared to older revisions. As a result the watchpoint(s) function are not working.
The patch adds a second path dealing with the ArmV8m .
Tested on cortex m33, check setup watch some_variable (such as xTickCount on freeRtos)

  • without the patch, does not stop
  • with the patch, does stop as expected

It was partially tested, i dont have other chip with that DWT block. Only test write watchpoints.
As far as i can tell, watchpoints are still working with older chips (cortex M4)

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 is looking really good and complements our own work on ARMv8-M debug on the fix branch fix/adi-structural-cleanup. We'd love to get this merged for v2.0 so if you can take care of the small number of review notes (most are to do with nomenclature and we don't think will take that long to deal with as it's mostly just find-replace stuff), we'll get this merged ASAP.

src/target/cortexm.h Outdated Show resolved Hide resolved
src/target/cortexm.h Outdated Show resolved Hide resolved
src/target/cortexm.c Outdated Show resolved Hide resolved
src/target/cortexm.c Outdated Show resolved Hide resolved
src/target/cortexm.c Outdated Show resolved Hide resolved
@dragonmux dragonmux added this to the v2.0 release milestone Dec 2, 2024
@dragonmux dragonmux added the Bug Confirmed bug label Dec 2, 2024
@dragonmux dragonmux changed the title Arm cortexm: Temptative fix for watchpoints so they work on v8m arch ( CM33) Cortex-M: Temptative fix for watchpoints so they work on ARMv8-M cores (eg Cortex-M33) Dec 2, 2024
@mean00
Copy link
Author

mean00 commented Dec 3, 2024

Thank you for the comments, will update it as soon as i can

@mean00
Copy link
Author

mean00 commented Dec 4, 2024

I think i took all the comments into account.
If it's ok i'll squash the commits
Thank you

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.

Yeah, looks good to us. Please let us know once you've squashed the commits down (noting that there should not be a space between the suffix and its' :, so cortexm: not cortexm :) and we'll get this merged.

Thank you for the contribution! Nicely done.

@mean00
Copy link
Author

mean00 commented Dec 4, 2024

Should be okay now
Thank you

@dragonmux dragonmux merged commit b547574 into blackmagic-debug:main Dec 4, 2024
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants