Page MenuHomeFreeBSD

vmm: implement single-stepping for AMD CPUs
ClosedPublic

Authored by bnovkov on Oct 19 2023, 3:56 PM.
Tags
Referenced Files
Unknown Object (File)
Sat, Feb 24, 4:03 PM
Unknown Object (File)
Fri, Feb 16, 12:05 AM
Unknown Object (File)
Fri, Feb 16, 12:05 AM
Unknown Object (File)
Fri, Feb 16, 12:04 AM
Unknown Object (File)
Fri, Feb 16, 12:04 AM
Unknown Object (File)
Fri, Feb 16, 12:04 AM
Unknown Object (File)
Fri, Feb 16, 12:02 AM
Unknown Object (File)
Tue, Feb 13, 7:42 PM
Subscribers

Details

Summary

This patch implements single-stepping for AMD CPUs using the RFLAGS.TF single-stepping mechanism.
The GDB stub requests single-stepping using the VM_CAP_RFLAGS_TF capability. Setting this capability will set the RFLAGS.TF bit on the selected vCPU, activate DB exception intercepts, and activate POPF/PUSH instruction intercepts.
The resulting DB exception is then caught by the IDT_DB vmexit handler and bounced to userland where it is processed by the GDB stub.
This patch also makes sure that the value of the TF bit is correctly updated and that it is not erroneously propagated into memory. Stepping over PUSHF will cause the vm_handle_db function to correct the pushed RFLAGS value and stepping over POPF will update the shadowed TF bit copy.

This work was sponsored by the GSoC '22 program.

Test Plan

I've tested the changes (all four patches) on a Ryzen 5 5600X processor running -CURRENT.
The kernel and the vmm module were compiled with assertions enabled.

Setting breakpoints and single-stepping works, and stepping over POPF/PUSHF correctly preserves the TF bit.
There is, however, a chance that single-stepping will get randomly interrupted by cpu_idle_acpi.
That issue is also present on Intel CPUs and I'll look into it separately.

The virtual machine ran fine after detaching the debugger and running buildkernel.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sys/amd64/vmm/amd/svm.c
1619–1621

Can we add a function which is called from IDT_DB and from VMCB_EXIT_PUSHF to avoid this instruction restart?

sys/amd64/vmm/vmm.c
1764

You're reading rflags. So, shouldn't this be VM_PROT_RDWR?

bnovkov added inline comments.
sys/amd64/vmm/amd/svm.c
1619–1621

Sorry, I'm not sure I know what you meant by this.
Could you please elaborate a bit?

sys/amd64/vmm/amd/svm.c
1619–1621

Friendly ping, this seems to be the only issue left for this patch series so I'd like to resolve it.

sys/amd64/vmm/amd/svm.c
1440–1441

Small style nit, same with pushf below.

1444–1445
1468–1469
1477

I think you don't need this now?

sys/amd64/vmm/vmm.c
1765
1770

You could optimize this a bit perhaps to avoid doing all this if tf_shadow_val is non-zero up front as then operation is a no-op in that case?

bnovkov added inline comments.
sys/amd64/vmm/amd/svm.c
1477

Ah, that line was probably left over from a previous iteration of the patch, its removed now.

sys/amd64/vmm/vmm.c
1770

Good point, I've added a check for this.

jhb added inline comments.
sys/amd64/vmm/vmm.c
1774

This is now a no-op, but I will fix it when merging.

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

Note, there is still a hole here I think which is that the RFLAGS pushed as part of an exception frame when an exception or interrupt occurs can "leak" the shadow TF into the guest. That is, if you have enabled RFLAGS.TF to request a step and the guest takes an interrupt, the saved copy of RFLAGS on the stack will have TF set. Masking hardware interrupts during stepping prevents a good bit of this, but if the code hits an exception it will still leak the shadow TF. That isn't fatal, per se, but might be something to think about fixing at some point. I think the fix is you have to hook all exceptions and rewrite the saved RFLAGS on the stack similar to what happens now for PUSHF/POPF. You'd also need to deal with IRET as well so you can rewrite the saved RFLAGS on the stack before letting IRET continue.

This revision was automatically updated to reflect the committed changes.