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

[Paolo Bonzini <pbonzini@redhat.com>]

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