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

Gdb support for mshv guests #327

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

dblnz
Copy link
Contributor

@dblnz dblnz commented Mar 5, 2025

This PR address the #220 issue.
Additionally, the first 7 commits refactor the existing guest debugging code and moves code from kvm.rs to a new file that is to contain the hypervisor specific debugging functionality.

@dblnz dblnz added the kind/enhancement New feature or request label Mar 5, 2025
@dblnz dblnz force-pushed the gdb-support-mshv branch from e82833e to 0119675 Compare March 5, 2025 16:49
@ludfjig
Copy link
Contributor

ludfjig commented Mar 5, 2025

This looks great! I noticed the how-to-debug-a-hyperlgiht-guest.md only mentions KVM guests, so we might want to update that doc. Additionally we have a debugging-hyperlgiht.md doc that has no mention of gdb debugging at all. Perhaps we could add a link from that doc to how-to-debug-a-hyperlight-guest.md

@dblnz
Copy link
Contributor Author

dblnz commented Mar 5, 2025

This looks great! I noticed the how-to-debug-a-hyperlgiht-guest.md only mentions KVM guests, so we might want to update that doc. Additionally we have a debugging-hyperlgiht.md doc that has no mention of gdb debugging at all. Perhaps we could add a link from that doc to how-to-debug-a-hyperlight-guest.md

Thanks, I forgot about the documentation.
I'll add another commit that addresses the documentation

ludfjig
ludfjig previously approved these changes Mar 7, 2025
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

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

LGTM! It would be good if @syntactically could review as well

@@ -26,6 +27,7 @@ use event_loop::event_loop_thread;
use gdbstub::conn::ConnectionExt;
use gdbstub::stub::GdbStub;
use gdbstub::target::TargetError;
pub use hyp_debug::{kvm, mshv, GuestMemoryDebug, GuestVcpuDebug};
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be pub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is used in each hypervisor driver.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use pub(crate) use hyp_debug::{kvm, mshv, GuestMemoryDebug, GuestVcpuDebug}; instead?

dblnz added 13 commits March 12, 2025 11:52
- this is done in preparation of accomodating support for other hypervisors
which means some of the functionality whould be common.
- moving the debug code to a separate file can enable us to
group common behavior in traits

Signed-off-by: Doru Blânzeanu <[email protected]>
- also add a trait that contains all the needed methods for interacting
with a vCPU

Signed-off-by: Doru Blânzeanu <[email protected]>
… file

- add trait to define common behavior to be used by other hypervisors
  later

Signed-off-by: Doru Blânzeanu <[email protected]>
- it can be done using the generic way to add hw breakpoints
- also remove the entry point breakpoint after the vCPU stops to
avoid hanging there

Signed-off-by: Doru Blânzeanu <[email protected]>
- verify the debug registers that report what kind of exception was triggered
- in case there is an unknown reason, report it as a SwBp so that the gdb
client can inspect what happened

Signed-off-by: Doru Blânzeanu <[email protected]>
- use checked_sub to avoid issues from addresses that could cause
underflow

Signed-off-by: Doru Blânzeanu <[email protected]>
dblnz added 2 commits March 12, 2025 15:58
- change gdb commands in test to be usable with older gdb versions
- change Justfile to provide mshv3 feature
- change gdb test to invoke cargo test with correct features for mshv

Signed-off-by: Doru Blânzeanu <[email protected]>
@dblnz dblnz force-pushed the gdb-support-mshv branch from 4df565e to ebdbe13 Compare March 12, 2025 14:00
@dblnz dblnz requested a review from syntactically March 13, 2025 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants