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

	F86187039: D42298.diff
	Sun, Jun 16, 5:34 PM

	Unknown Object (File)
	Tue, May 28, 10:40 PM

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

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

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

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

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

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

View All 27 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 Skipped

Unit

Tests Skipped

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
763
776	Why not checking `VM_CAP_MASK_HWINTR`?
806	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
806	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
759
765
772	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.
776
776	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.
779

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
763	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

98 lines

Diff 129523

usr.sbin/bhyve/amd64/vmexit.c

Loading...

usr.sbin/bhyve/gdb.h

Loading...

usr.sbin/bhyve/gdb.c

Loading...