Save and restore guest debug registers.

[jhb]

Summary

Currently most of the debug registers are not saved and restored during
VM transitions allowing guest and host debug register values to leak into
the opposite context. One result is that hardware watchpoints do not work
reliably within a guest.

Due to differences in SVM and VT-x, slightly different approaches are used.

For VT-x:

• Enable debug register save/restore for VM entry/exit in the VMCS for DR7 and MSR_DEBUGCTL.
• Explicitly save DR0-3,6 of the guest.
• Explicitly save DR0-3,6-7, MSR_DEBUGCTL, and the trap flag from %rflags for the host. Note that because DR6 is "software" managed and not stored in the VMCS a kernel debugger which single steps through VM entry could corrupt the guest DR6 (since a single step trap taken after loading the guest DR6 could alter the DR6 register). To avoid this, explicitly disable single-stepping via the trace flag before loading the guest DR6. A determined debugger could still defeat this by setting a breakpoint after the guest DR6 was loaded and then single-stepping.

For SVM:

• Enable debug register caching in the VMCB for DR6/DR7.
• Explicitly save DR0-3 of the guest.
• Explicitly save DR0-3,6-7, and MSR_DEBUGCTL for the host. Since SVM saves the guest DR6 in the VMCB, the race with single-stepping described for VT-x does not exist.

For both platforms, expose all of the guest DRx values via --get-drX and
--set-drX flags to bhyvectl.

Test Plan

• I've only been able to test VT-x. The SVM bits are a guess on my part, in particular the VMCB caching stuff (I'm not sure if enabling the DR bit is required for DR6/DR7/debugctl to "work" the same way the debug controls entry/exit was required for VT-x).
• For testing I ran a simple test program under gdb with hardware watchpoints and verified the watchpoings fired while running a loop of bhyvectl with '--get-dr0 --get-dr6 --get-dr7' in another xterm to confirm the registers changed.

[grehan]

This looks fine. I'll test on AMD. Some comments:

• for completeness, DR6 and DR7 should be init'd to their reset values on guest creation (table 9-1 vol 3a, 0xFFFF0FF0 and 0x00000400 respectively)
• a glance at Linux kernel source shows that the BTF flag in IA32_DEBUGCTL MSR can be used for ptrace with the PTRACE_SINGLEBLOCK call. While it appears that gdb doesn't use this, there is at least a Linux testcase for it (https://bugzilla.redhat.com/show_bug.cgi?id=437028). This brings up the bigger issue of how to handle IA32_DEBUGCTL MSR - it looks like an MSR write exit could be necessary to verify bits being set. Something for future work :)

[rgrimes]

I have this patch downloaded, but not applied, on my AMD test system. I'll apply and build after I send this comment. Do we have any more elaborate testing than just do the watchs that jhb did?

[jhb]

In D13229#283391, @rgrimes wrote:

I have this patch downloaded, but not applied, on my AMD test system. I'll apply and build after I send this comment. Do we have any more elaborate testing than just do the watchs that jhb did?

For Intel I found that when I tried watches in GDB in a guest without these changes, the watches would not work reliably (wouldn't fire, etc.). With these fixes they worked. For AMD I think it would be useful to test the current stock code first to establish a baseline (e.g. do watchpoints not work or work sporadically?) and then see if the behavior changes with the patches applied.

[jhb]

• Set DR6 and DR7 initial values to power-on reset state.

[jhb]

It's not clear if we will have to explicitly handle writes to the DEBUGCTL MSR. I'll have to look at the SDM, but if it saves/restores the MSR during every entry/exit, then I think it would be fine to just let the guest directly access the msr. If a guest writes invalid values then the CPU would raise an exception in the guest. The only thing I'm not aware of is if we are forced to emulate that by virtue of the wrmsr always being a VM exit or if there is a way to avoid VM exits for writes to that MSR.

[avg]

First, as a baseline, I did some tests on AMD/SVM without applying this patch. I could use up to four hardware watchpoints in a guest, seemingly without any problems.
My tests weren't extensive, so I might have missed some problems.
With this patch everything works good too, I do not see any regressions and now I am able to inspect DR registers from the host.

If you would like more details or any specific tests, please let me know.

[jhb]

I am a bit surprised it worked fine without the patch. Perhaps if you used watchpoints both outside and inside at the same time you might get some inconsistent results? But I'm glad that I don't seem to have broken anything for SVM. Can someone confirm that setting "DR" in the VMCB cache flags is correct (i.e. equivalent to the VT-x changes)?

[avg]

In D13229#290214, @jhb wrote:

I am a bit surprised it worked fine without the patch. Perhaps if you used watchpoints both outside and inside at the same time you might get some inconsistent results?

That seems likely. I am not very knowledgeable in this area. It seems that the hardware provides separate DR6 and DR7 for the guest, but DR0 - DR4 are the same.
And SVM does not intercept changes to them. And without your patch it does not take care to save and restore those registers.
Thus, it must be that the guest modifies the registers in the host state?

But I'm glad that I don't seem to have broken anything for SVM. Can someone confirm that setting "DR" in the VMCB cache flags is correct (i.e. equivalent to the VT-x changes)?

As I understand, the meaning of that bit is that the guest's DR6 and DR7 have not changed between the previous #VMEXIT and the current VMRUN.
If that holds true, then the use of the bit is correct :-)

[avg]

The SVM part looks good to me (including the cache bit manipulations).

[jhb]

In D13229#290456, @avg wrote:

In D13229#290214, @jhb wrote:

But I'm glad that I don't seem to have broken anything for SVM. Can someone confirm that setting "DR" in the VMCB cache flags is correct (i.e. equivalent to the VT-x changes)?

As I understand, the meaning of that bit is that the guest's DR6 and DR7 have not changed between the previous #VMEXIT and the current VMRUN.
If that holds true, then the use of the bit is correct :-)

Ah, on VT-x the save/restore of DR7 was conditional (and the changes to VT-x turn it on). It sounds like for SVM DR6 and DR7 are saved and restored unconditionally and the dirty bit is just a way to let SVM know that the saved value in the VMCB was changed by the VMM? Also, do you know if SVM saves/restores the guest IA32_DEBUGCTL MSR (VT-x does)?

[avg]

In D13229#290619, @jhb wrote:

Ah, on VT-x the save/restore of DR7 was conditional (and the changes to VT-x turn it on). It sounds like for SVM DR6 and DR7 are saved and restored unconditionally and the dirty bit is just a way to let SVM know that the saved value in the VMCB was changed by the VMM?

That is my understanding of how the cache bits work.
More details are in section 15.15 of AMD64 Architecture Programmer’s Manual Volume 2: System Programming.

http://developer.amd.com/wordpress/media/2012/10/24593_APM_v21.pdf

Also, do you know if SVM saves/restores the guest IA32_DEBUGCTL MSR (VT-x does)?

I am not sure if I am talking about the same MSR.
Processors may support this feature, but it has to be enabled in VMCB.
Please see 15.23.

My family 10h processor does support the feature, so I'd assume that nearly all AMD processors do too.
Anyway, it seems that vmcb_init enables it unconditionally:

/* Enable Last Branch Record aka LBR for debugging */
ctrl->lbr_virt_en = 1;
state->dbgctl = BIT(0);

Not sure why the code sets bit 0 of the MSR, though. It should be up to the guest how to set up this MSR?

[jhb]

In D13229#290786, @avg wrote:

In D13229#290619, @jhb wrote:

Ah, on VT-x the save/restore of DR7 was conditional (and the changes to VT-x turn it on). It sounds like for SVM DR6 and DR7 are saved and restored unconditionally and the dirty bit is just a way to let SVM know that the saved value in the VMCB was changed by the VMM?

That is my understanding of how the cache bits work.
More details are in section 15.15 of AMD64 Architecture Programmer’s Manual Volume 2: System Programming.

Ah, thanks. (And I already had the APM saved on my Mac, should have looked at it sooner)

Also, do you know if SVM saves/restores the guest IA32_DEBUGCTL MSR (VT-x does)?

I am not sure if I am talking about the same MSR.
Processors may support this feature, but it has to be enabled in VMCB.
Please see 15.23.

My family 10h processor does support the feature, so I'd assume that nearly all AMD processors do too.
Anyway, it seems that vmcb_init enables it unconditionally:

/* Enable Last Branch Record aka LBR for debugging */
ctrl->lbr_virt_en = 1;
state->dbgctl = BIT(0);

Not sure why the code sets bit 0 of the MSR, though. It should be up to the guest how to set up this MSR?

Yeah, that seems odd. It seems like this should probably be set to 0 by default.