KVM: x86: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN
PATCH 1/1 KVM: x86: Fix guest time accounting with VIRT_CPU_ACCOUNTING_GEN
move guest_exit() after local_irq_eanbled() so that the timer interrupt hits we account that tick as spent in the guest.
--- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/x86.c +1 -1
@@ -8044,7 +8044,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) ++vcpu->stat.exits; -guest_exit_irqoff ();if (lapic_in_kernel (vcpu)) {s64 delta = vcpu->arch.apic->lapic_timer.advance_expire_delta;if (delta !=S64_MIN ) {
@@ -8054,6 +8053,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) }local_irq_enable (); +guest_exit ();preempt_enable (); vcpu->srcu_idx =srcu_read_lock (&vcpu->kvm->srcu);
Signature · 1 line
2.14.1.40.g8e62ba1
Quoted 22 lines · Wei Yang
move
guest_exit()afterlocal_irq_eanbled()so that the timer interrupt hits we account that tick as spent in the guest.Cc Paolo Bonzini <[email protected]>Signed-off-by Wei Yang <[email protected]>--- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/kvm/x86.c +0
-1@@ -8044,7 +8044,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) ++vcpu->stat.exits; -guest_exit_irqoff ();if (lapic_in_kernel (vcpu)) {s64 delta = vcpu->arch.apic->lapic_timer.advance_expire_delta;if (delta !=S64_MIN ) {
arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_vcpu *vcpu) · line 8056
Quoted 4 lines
arch/x86/kvm/x86.c +1
-0@@ -8054,6 +8053,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) }local_irq_enable (); +guest_exit ();
The tracing invoked by trace_kvm_wait_lapic_expire() needs to be done after guest exit, otherwise it will violate the RCU quiescent state. See commits:
8b89fe1f6c43 ("kvm: x86: move tracepoints outside extended quiescent state") ec0671d5684a ("KVM: LAPIC: Delay trace_kvm_wait_lapic_expire tracepoint to after vmexit")
Is this an actual issue in practice, or was this prompted by code inspection?
On SVM, this patch is essentially a nop as irqs are temporarily enabled by svm_handle_exit_irqoff(). On VMX, this only applies to ticks that occur between VM-Exit and local_irq_enable(), which is a fairly small window all things considered. Toggling irqs off and back on in guest_exit() seems like a waste of cycles, and could introduce other inaccuracies on VMX, e.g. if non-tick interrupts are taken and cause the tick to expire.
Quoted 6 lines
arch/x86/kvm/x86.c +0
-0preempt_enable (); vcpu->srcu_idx =srcu_read_lock (&vcpu->kvm->srcu);Signature · 1 line
2.14.1.40.g8e62ba1
A message that is not in the archive (sent off-list or not yet received): CANwVFYMwKqsvGhPS7ZpQRS-dJ58KMLRSx8oEk_PowMtwpfXK=A@mail.gmail.com
Quoted 5 lines · Zhang Yang
Yes, In fact we had a trouble with the small window, the system monitor found that the host machine's sys is very high now and then. Based on our's tracing, we found that timer interrupt always hit after
local_irq_enable, the guest tick would be always accounted to system time. after applying this patch, the phenomenon disappears.
Fun. Rather than switching to guest_exit(), I think it'd be better to unconditionally open an irq window in order to minimize the overhead. I'll send a patch, it's easier to explain with code :-)