7
32
Fork
You've already forked blackmagic
23

target: Support 64-bit address for break/watch point #2017

Merged
marysaka merged 2 commits from feature/breakpoint-64bit into main 2025年01月11日 12:16:23 +01:00
marysaka commented 2024年12月13日 11:28:59 +01:00 (Migrated from github.com)
Copy link

This change breakwatch::addr to be 64-bit as this will be required for ARMv8 AArch64 support.

This also change the layout of breakwatch to avoid extraneous padding in 64-bit and reduce the size of the reserved array to 64 bits.

Detailed description

Your checklist for this pull request

Closing issues

This change breakwatch::addr to be 64-bit as this will be required for ARMv8 AArch64 support. This also change the layout of breakwatch to avoid extraneous padding in 64-bit and reduce the size of the reserved array to 64 bits. <!-- Filling this template is mandatory --> ## Detailed description <!-- Explain the **details** for making this change. * Is a new feature implemented? * What existing problem(s) does the pull request solve? * How does the pull request solve these problems? Please provide enough information so that others can review your pull request. Information embedded in the description part of the commits doesn't count. --> ## Your checklist for this pull request * [x] I've read the [Code of Conduct](https://github.com/blackmagic-debug/blackmagic/blob/main/CODE_OF_CONDUCT.md) * [x] I've read the [guidelines for contributing](https://github.com/blackmagic-debug/blackmagic/blob/main/CONTRIBUTING.md) to this repository * [x] It builds for hardware native (see [Building the firmware](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-firmware)) * [x] It builds as BMDA (see [Building the BMDA](https://github.com/blackmagic-debug/blackmagic?tab=readme-ov-file#building-black-magic-debug-app)) * [x] I've tested it to the best of my ability * [x] My commit messages provide a useful short description of what the commits do ## Closing issues <!-- put "fixes #XXXX" here to auto-close the issue(s) that your PR fixes (if any). -->
dragonmux left a comment
Copy link

We only have one review note and it's to do with the ordering of members in the breakwatch structure, nothing too strenuous though and with that addressed we're happy to merge this.

We only have one review note and it's to do with the ordering of members in the `breakwatch` structure, nothing too strenuous though and with that addressed we're happy to merge this.
@ -103,12 +103,18 @@ struct target_command {
typedef struct breakwatch breakwatch_s;

This reordering needs to be gated on sizeof(void *) - for the firmware, pointers are 32-bit so this mis-aligns the 64-bit int of addr, while this correctly aligns it on 64-bit platforms. Conditionally reordering the addr and type members is probably the right way forwards.

This reordering needs to be gated on `sizeof(void *)` - for the firmware, pointers are 32-bit so this mis-aligns the 64-bit int of `addr`, while this correctly aligns it on 64-bit platforms. Conditionally reordering the `addr` and `type` members is probably the right way forwards.
dragonmux left a comment
Copy link

There's one small issue we spotted in this round, and then we think this PR is there and can be merged. Good work!

There's one small issue we spotted in this round, and then we think this PR is there and can be merged. Good work!
@ -160,6 +160,12 @@ if is_firmware_build and get_option('print_memory_usage')
add_project_link_arguments('-Wl,--print-memory-usage',language:'c')

Think you mean "machine" rather than "target" here.

You will need to make a call for checking sizeof(void *) for both cc_host and cc_native, otherwise one of those two is likely going to be wrong for one of either the host or the probe. Use cc_native in the is_cross_build block.

Think you mean "machine" rather than "target" here. You will need to make a call for checking `sizeof(void *)` for both `cc_host` and `cc_native`, otherwise one of those two is likely going to be wrong for one of either the host or the probe. Use `cc_native` in the `is_cross_build` block.
dragonmux left a comment
Copy link

LGTM, merging. Thank you for the contribution!

LGTM, merging. Thank you for the contribution!
Sign in to join this conversation.
No reviewers
Labels
Clear labels
BMD App
Black Magic Debug App (aka. PC hosted) (not firmware)
BMP Firmware
Black Magic Probe Firmware (not PC hosted software)
Bug
Confirmed bug
Build system
Build system
Can't reproduce
Maintainers can't reproduce this problem
CI
Continuous Integration System
Contribution wanted
User contributions welcome
Documentation
Project documentation
Draft
Work in progress draft
Duplicate
This issue or pull request already exists
Enhancement
General project improvement
Feedback wanted
Requires additional submitter feedback
Foreign Host Board
Non Native hardware to runing Black Magic firmware on
GDB
Issue/PR related to GDB
Good first issue
Good for newcommers
HwIssue Mitigation
Solving or mitigating a Hardware issue in Software
Information Needed
Maintainers need more information
NativeHardware
Official Black Magic Debug Hardware
New Host Board
New hardware to run Black Magic firmware on
New Target
New debug target
Off Topic
Something that does not involve the project in any way
Potential Bug
A potential, unconfirmed or very special circumstance bug
Regression
Bug caused by a regression
User Interest Needed
More user interest required before consideration
User Testing Needed
Looking for user testing reports
Won't fix
Outside of the project scope or works as intended
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
blackmagic-debug/blackmagic!2017
Reference in a new issue
blackmagic-debug/blackmagic
No description provided.
Delete branch "feature/breakpoint-64bit"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?