Re: [PATCH 3/7] KVM: x86: Allow the guest to run with dirty debug registers

Paolo Bonzini
2014-03-09T20:07:08Z

Il 09/03/2014 19:28, Radim Krčmář ha scritto:
>> > /*
>> > + * Do this here before restoring debug registers on the host. And
>> > + * since we do this before handling the vmexit, a DR access vmexit
>> > + * can (a) read the correct value of the debug registers, (b) set
>> > + * KVM_DEBUGREG_WONT_EXIT again.
> We re-enable intercepts on the next exit for the sake of simplicity?
> (Batched accesses make perfect sense, but it seems we don't have to care
> about DRs at all, without guest-debug.)

It's not just for the sake of simplicity. Synchronizing debug registers
on entry does have some cost, and it's required for running without
debug register intercepts. You would incur this cost always, since
no-guest-debug is actually the common case.

We're well into diminishing returns; Alex timed this patch vs. one that
completely ignores MOV DR exits and (to the surprise of both of us) this
patch completely restored performance despite still having a few tens of
thousands MOV DR exits/second.

In the problematic case, each context switch will do a save and restore
of debug registers; that's in total 13 debug register accesses (read 6
registers, write DR7 to disable all registers, write 6 registers
including DR7 which enables breakpoints), all very close in time. It's
quite likely that the final DR7 write is very expensive (e.g. it might
have to flush the pipeline). Also, this test case must be very heavy on
context switches, and each context switch already incurs a few exits
(for example to inject the interrupt that causes it, to read the current
time).

A different optimization could be to skip the LOAD_DEBUG_CONTROLS
vm-entry control if DR7 == host DR7 == 0x400 && MOV DR exits are
enabled. Not sure it's worthwhile though, and there's also the DEBUGCTL
MSR to take into account. Better do these kinds of tweaks if they
actually show up in the profile: Alex's testcase shows that when they
do, the impact is massive.

>> + kvm_x86_ops->sync_dirty_debug_regs(vcpu);
>
> Sneaky functionality ... it would be nicer to split 'enable DR
> intercepts' into a new kvm op.

Why? "Disable DR intercepts" is already folded into the handler for MOV
DR exits.

> And we don't have to modify DR intercepts then, which is probably the
> main reason why sync_dirty_debug_regs() does two things.

Another is that indirect calls are relatively expensive and add
complexity; in this case they would always be used back-to-back.

Paolo