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.

Sign up for GitHub

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

Jump to bottom

Support GDB on AArch64 #4355

Closed
wants to merge 7 commits into from
Closed

Support GDB on AArch64 #4355

wants to merge 7 commits into from

Conversation

michael2012z
Copy link
Member

@michael2012z michael2012z commented Jul 22, 2022 •
edited

Fixes #3980

Now this is a draft. There is still some work to do:

  • A PR for supporting AArch64 in gdbstub_arch which is on review, will update to new release of gdbstub_arch
  • Memory watch
  • Max hardware breakpoints: an API should be called to determine

@michael2012z michael2012z requested a review from a team as a code owner July 22, 2022 05:28
@michael2012z michael2012z marked this pull request as draft July 22, 2022 05:28
@michael2012z michael2012z force-pushed the aarch64-gdb branch 2 times, most recently from 73cbf65 to bd5df09 Compare July 25, 2022 15:00
@michael2012z michael2012z mentioned this pull request Jul 26, 2022
Merged
@michael2012z
Copy link
Member Author

A PR for AArch64 support on gdbstub-arch happened to be raised: daniel5151/gdbstub#109

I will rebase when it got merged.

@ptosi
Copy link

ptosi commented Jul 28, 2022

Hi, I'm the author of daniel5151/gdbstub#109, which was written for crosvm (see the CLs on Gerrit).

@michael2012z, given that you've implemented a very similar Arch, it would be very interesting for us if you could review the PR. Also, one feature missing from the CLs is support for reading MMU-translated GVAs, which I had planned to add in a future patch (see debug_read_memory which currently uses the GuestAddress as-is) given how tricky doing it properly can be but which you have already implemented; would you be interested in contributing your implementation of the PTW there?

AFAIU, the current code in this repo for supporting GDB (on x86) was derived from crosvm; would it make sense to combine our implementations there and then port the resulting code here?

@michael2012z
Copy link
Member Author

Hi, @ptosi

Thanks for connecting.

My implementation of gdbstub_arch is quite simply, I only considered the general-purposed registers, haven't figured out a solution for the system registers. I am happy to look into your PR and join the discussion there.

Regarding the code of Guest Address translation. Sorry I cannot contribute to crosvm directly now, due to the lack of legal approval, you know. :) But please feel free to port the code here by yourself. I am exciting if you can use the code in crosvm as well.

@michael2012z michael2012z force-pushed the aarch64-gdb branch 2 times, most recently from 17388e6 to d3a68f8 Compare August 1, 2022 06:31
@keiichiw keiichiw mentioned this pull request Aug 1, 2022
Closed
@michael2012z michael2012z mentioned this pull request Aug 5, 2022
Merged
5 tasks
@michael2012z
vmm: Use temporary gdbstub_arch for test
d44682d 
This is only for test purpose before `gdbstub_arch` modification is
upstreamed.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
hypervisor: Update kvm-ioctls to main branch
ef22702 
Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
vmm: Enable gdbstub on AArch64
54593fb 
The `gva_translate` function is still missing, it will be added with a
separate commit.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
vmm: Implement translate_gva on AArch64
c732b2e 
On AArch64, `translate_gva` API is not provided by KVM. We implemented
it in VMM by walking through translation tables.

Address translation is big topic, here we only focus the scenario that
happens in VMM while debugging kernel. This `translate_gva`
implementation is restricted to:
 - Exception Level 1
 - Translate high address range only (kernel space)

This implementation supports following Arm-v8a features related to
address translation:
 - FEAT_LPA
 - FEAT_LVA
 - FEAT_LPA2

The implementation supports page sizes of 4KiB, 16KiB and 64KiB.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
hypervisor: Enable gdb HW breakpoint on AArch64
5f994cc 
Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
vmm: Enable GDB HW Watchpoint
10e7cc6 
This commit setup the skeleton of HW watchpoint support.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@michael2012z
hypervisor: Implement GDB HW Watchpoint on AArch64
222a416 
Note: This is a temporary commit.
Now there is problem, still testing.

Signed-off-by: Michael Zhao <michael.zhao@arm.com>
@rbradford
Copy link
Member

@michael2012z It's cool that you got aarch64 support working with Cloud Hypervisor and it looks like that when the aarch64 support lands upstream in gdbstub_arch we can move forward quickly. In the meantime would you mind closing this PR and continuing any discussion in an issue. Unfortunately a side effect of the Jenkins "branch indexing" that cleans up old builds is also building any open PRs which consumes resources. Not a problem if we're close to merging but a bit wasteful if we've still got blockers.

Obviously you can reference your fork/branch from the github discussion!

@michael2012z
Copy link
Member Author

No problem, @rbradford . It's better close now and I will PR again when gdbstub_arch is ready.

I think it will be a while before the dependency is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GDB support on ARM64 KVM
3 participants
@michael2012z @ptosi @rbradford