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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

marysaka
Copy link
Contributor

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

@marysaka marysaka force-pushed the feature/breakpoint-64bit branch from cd7f6b1 to 790ef87 Compare December 13, 2024 10:43
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.

Signed-off-by: Mary Guillemard <mary@mary.zone>
@marysaka marysaka force-pushed the feature/breakpoint-64bit branch from 790ef87 to 9086642 Compare December 15, 2024 10:17
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.

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.

Comment on lines -108 to +107
target_breakwatch_e type;
target_addr32_t addr;
target_addr64_t addr;
Copy link
Member

Choose a reason for hiding this comment

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

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 dragonmux added this to the v2.0 release milestone Dec 16, 2024
@dragonmux dragonmux added the Enhancement General project improvement label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement General project improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants