Paths

Table of Contentst

Differential D42298

bhyve: refactor gdbstub to enable single-stepping on AMD CPUs
ClosedPublic
Actions

Authored by bnovkov on Oct 19 2023, 4:01 PM.

Tags

Referenced Files

	Unknown Object (File)
	Mon, May 6, 4:13 PM

	Unknown Object (File)
	Tue, Apr 30, 5:52 AM

	Unknown Object (File)
	Tue, Apr 30, 5:50 AM

	Unknown Object (File)
	Tue, Apr 30, 5:50 AM

	Unknown Object (File)
	Tue, Apr 30, 5:37 AM

	Unknown Object (File)
	Tue, Apr 30, 5:10 AM

	Unknown Object (File)
	Mon, Apr 29, 9:56 PM

	Unknown Object (File)
	Sun, Apr 28, 10:40 PM

View All 25 Files

Subscribers

Details

Reviewers

corvink
jhb

Group Reviewers

Commits

rG791e4d689520: bhyve: refactor gdbstub to enable single-stepping on AMD CPUs
rGca96a942cafb: bhyve: refactor gdbstub to enable single-stepping on AMD CPUs

Summary

This patch refactors the existing Intel-specific single-stepping mechanism in bhyve's GDB stub to work with both AMD and Intel CPUs.

This work was sponsored by the GSoC '22 program.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bnovkov created this revision.Oct 19 2023, 4:01 PM

Herald added a reviewer: bhyve. · View Herald TranscriptOct 19 2023, 4:01 PM

Herald added subscribers: bcran, rgrimes, emaste, imp. · View Herald Transcript

bnovkov requested review of this revision.Oct 19 2023, 4:01 PM

bnovkov added a parent revision: D42296: vmm: implement single-stepping for AMD CPUs.

bnovkov added a child revision: D42299: vmm: implement VM_CAP_MASK_HWINTR on AMD CPUs.Oct 19 2023, 4:05 PM

corvink added inline comments.Oct 30 2023, 2:26 PM

usr.sbin/bhyve/amd64/vmexit.c
520	Can we reuse the existing `VM_EXITCODE_DEBUG`?
usr.sbin/bhyve/gdb.c
762
775	Why not checking `VM_CAP_MASK_HWINTR`?
803	Seems unrelated. So, it should be fixed in a seperate commit. This has some benefits: We can merge such small fixes earlier The diff gets smaller The diff is more readable because it doesn't contain any noise All in all, it makes the review and bug hunting (imagine a bisect ends at this commit) easier and faster

corvink mentioned this in D42295: vmm: enable software breakpoints for AMD CPUs.Oct 30 2023, 2:31 PM

Address @corvink 's comments.

bnovkov marked 2 inline comments as done.Oct 30 2023, 5:16 PM

bnovkov added inline comments.

usr.sbin/bhyve/amd64/vmexit.c
520	As far as I can tell, that exitcode is used to suspend a vCPU. I didn't want to tie debug exception VMEXITs with this because I perceive them as two separate things + I didn't want to risk inadvertently breaking something that depends on VM_EXITCODE_DEBUG.

bnovkov marked an inline comment as done.Oct 30 2023, 5:25 PM

bnovkov added inline comments.

usr.sbin/bhyve/gdb.c
803	I've split this out into a separate revision (D42406).

This generally looks fine to me modulo some nits. I think we should probably remove checking for the VM_CAP_MASK_HWINTR capability though.

usr.sbin/bhyve/amd64/vmexit.c
520	Yeah, this is for an intercepted DB# exception similar `VM_EXITCODE_BPT` for BP#. `VM_EXITCODE_DEBUG` is for when the userland hypervisor requests a vCPU to suspend.
usr.sbin/bhyve/gdb.c
758
764
771	We should probably refactor this in a followup commit to check these capabilities once during gdb_init on the BSP (they aren't really per-vCPU but are per-VM caps) and then cache the results with some sort of `enum { NONE, MTRAP, RFLAGS_TF } stepping_support` global. That could then be used when doing a step to only set the relevant capability as well.
775
775	VM_CAP_MASK_HWINTR is not required, it just is a nice to have, so I don't think we need to require it here, we just need to require one of MTRAP or RFLAGS_TF.
778

Address @jhb 's comments.

bnovkov marked 5 inline comments as done.Dec 12 2023, 4:35 PM

jhb accepted this revision.Dec 12 2023, 10:40 PM

jhb added inline comments.

usr.sbin/bhyve/gdb.c
762	Ah, missed this earlier (but can just fix during commit). This one is not fatal and we should just ignore an error setting this capability.

This revision is now accepted and ready to land.Dec 12 2023, 10:40 PM

Closed by commit rGca96a942cafb: bhyve: refactor gdbstub to enable single-stepping on AMD CPUs (authored by bnovkov, committed by jhb). · Explain WhyDec 12 2023, 11:30 PM

This revision was automatically updated to reflect the committed changes.

jhb added a commit: rGca96a942cafb: bhyve: refactor gdbstub to enable single-stepping on AMD CPUs.

jhb added a commit: rG791e4d689520: bhyve: refactor gdbstub to enable single-stepping on AMD CPUs.Jan 5 2024, 12:29 AM

Revision Contents
Changeset List

Path

Size

usr.sbin/

bhyve/

amd64/

15 lines

1 line

94 lines

Diff 131299

usr.sbin/bhyve/amd64/vmexit.c

Loading...

usr.sbin/bhyve/gdb.h

Loading...

usr.sbin/bhyve/gdb.c

Loading...