Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932493AbcJLIfc (ORCPT ); Wed, 12 Oct 2016 04:35:32 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34567 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932201AbcJLIf2 (ORCPT ); Wed, 12 Oct 2016 04:35:28 -0400 MIME-Version: 1.0 In-Reply-To: <20161011150640.GA16406@potion> References: <1476188240-3502-1-git-send-email-wanpeng.li@hotmail.com> <1476188240-3502-3-git-send-email-wanpeng.li@hotmail.com> <20161011150640.GA16406@potion> From: Wanpeng Li Date: Wed, 12 Oct 2016 16:35:26 +0800 Message-ID: Subject: Re: [PATCH RFC 2/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: "linux-kernel@vger.kernel.org" , kvm , Paolo Bonzini , Yunhong Jiang , Wanpeng Li Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9C8Zddu011454 Content-Length: 4689 Lines: 128 2016-10-11 23:06 GMT+08:00 Radim Krčmář : > 2016-10-11 20:17+0800, Wanpeng Li: >> From: Wanpeng Li >> >> Most windows guests still utilize APIC Timer periodic/oneshot mode >> instead of tsc-deadline mode, and the APIC Timer periodic/oneshot >> mode are still emulated by high overhead hrtimer on host. This patch >> converts the expected expire time of the periodic/oneshot mode to >> guest deadline tsc in order to leverage VMX preemption timer logic >> for APIC Timer tsc-deadline mode. >> >> Cc: Paolo Bonzini >> Cc: Radim Krčmář >> Cc: Yunhong Jiang >> Signed-off-by: Wanpeng Li >> --- >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> @@ -1101,13 +1101,20 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic) >> apic->lapic_timer.period == 0) >> return 0; >> >> + if (kvm_lapic_hv_timer_in_use(apic->vcpu)) { >> + u64 tscl = rdtsc(); >> + >> + tmcct = apic->lapic_timer.tscdeadline - >> + kvm_read_l1_tsc(apic->vcpu, tscl); > > Yes, this won't work. The easiest way to return a less bogus TMCCT > would be remember the timeout when setting up the timer and replace > hrtimer_get_remaining() with it -- our deliver method shouldn't change > the expiration time. Agreed. > >> + } else { >> + remaining = hrtimer_get_remaining(&apic->lapic_timer.timer); >> + if (ktime_to_ns(remaining) < 0) >> + remaining = ktime_set(0, 0); >> + >> + ns = mod_64(ktime_to_ns(remaining), apic->lapic_timer.period); >> + tmcct = div64_u64(ns, >> + (APIC_BUS_CYCLE_NS * apic->divide_count)); >> + } >> >> return tmcct; >> } >> @@ -1400,52 +1407,65 @@ bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu) >> } >> EXPORT_SYMBOL_GPL(kvm_lapic_hv_timer_in_use); >> >> -static void cancel_hv_tscdeadline(struct kvm_lapic *apic) >> +static void cancel_hv_timer(struct kvm_lapic *apic) >> { >> kvm_x86_ops->cancel_hv_timer(apic->vcpu); >> apic->lapic_timer.hv_timer_in_use = false; >> } >> >> +static bool start_hv_timer(struct kvm_lapic *apic) >> { >> + u64 tscdeadline; >> >> + if (apic_lvtt_period(apic) || apic_lvtt_oneshot(apic)) { >> + u64 tscl = rdtsc(); >> >> + apic->lapic_timer.period = (u64)kvm_lapic_get_reg(apic, APIC_TMICT) >> + * APIC_BUS_CYCLE_NS * apic->divide_count; >> + apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) + >> + nsec_to_cycles(apic->vcpu, apic->lapic_timer.period); > > start_hv_timer() is called from kvm_lapic_switch_to_hv_timer(), which > can happen mid-period. The worst case is that the timer will never > fire, because we always convert back and forth. You have to compute the > equivalent deadline only once, and carry it around. Agreed. Thanks for your review. :) Please see RFC V2. Regards, Wanpeng Li > >> + } >> + >> + tscdeadline = apic->lapic_timer.tscdeadline; >> >> if (atomic_read(&apic->lapic_timer.pending) || >> kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) { >> if (apic->lapic_timer.hv_timer_in_use) >> + cancel_hv_timer(apic); >> } else { >> apic->lapic_timer.hv_timer_in_use = true; >> hrtimer_cancel(&apic->lapic_timer.timer); >> >> /* In case the sw timer triggered in the window */ >> if (atomic_read(&apic->lapic_timer.pending)) >> + cancel_hv_timer(apic); >> } >> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id, >> apic->lapic_timer.hv_timer_in_use); >> return apic->lapic_timer.hv_timer_in_use; >> } >> >> +void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu) >> +{ >> + struct kvm_lapic *apic = vcpu->arch.apic; >> + >> + WARN_ON(!apic->lapic_timer.hv_timer_in_use); >> + WARN_ON(swait_active(&vcpu->wq)); >> + cancel_hv_timer(apic); >> + apic_timer_expired(apic); >> + >> + if (apic_lvtt_period(apic)) >> + start_hv_timer(apic); >> +} >> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer); >> + >> void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu) >> { >> struct kvm_lapic *apic = vcpu->arch.apic; >> >> WARN_ON(apic->lapic_timer.hv_timer_in_use); >> >> - if (apic_lvtt_tscdeadline(apic)) >> - start_hv_tscdeadline(apic); >> + start_hv_timer(apic); >> } >> EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer); >>