From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752623AbcF1R4o (ORCPT ); Tue, 28 Jun 2016 13:56:44 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:53978 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752253AbcF1R4m (ORCPT ); Tue, 28 Jun 2016 13:56:42 -0400 Date: Tue, 28 Jun 2016 13:56:38 -0400 (EDT) From: Paolo Bonzini To: yunhong jiang Cc: Wanpeng Li , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Wanpeng Li , Radim =?utf-8?B?S3LEjW3DocWZ?= , Yunhong Jiang Message-ID: <640874638.2724253.1467136598812.JavaMail.zimbra@redhat.com> In-Reply-To: <20160628104547.4844fd2e@jnakajim-build> References: <1467096859-3143-1-git-send-email-wanpeng.li@hotmail.com> <20160628104547.4844fd2e@jnakajim-build> Subject: Re: [PATCH] KVM: vmx: fix underflow in TSC deadline calculation MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.4.164.1, 10.5.101.130] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF47 (Linux)/8.0.6_GA_5922) Thread-Topic: fix underflow in TSC deadline calculation Thread-Index: fkKcX5hwk4LukSzDYPQxnNt3JRcTaA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > > index fdc05ae..b15e32a 100644 > > --- a/arch/x86/kvm/lapic.c > > +++ b/arch/x86/kvm/lapic.c > > @@ -1454,11 +1454,18 @@ static void start_apic_timer(struct kvm_lapic > > *apic) /* lapic timer in tsc deadline mode */ > > u64 tscdeadline = apic->lapic_timer.tscdeadline; > > > > - if (kvm_x86_ops->set_hv_timer && > > - !kvm_x86_ops->set_hv_timer(apic->vcpu, > > tscdeadline)) { > > - apic->lapic_timer.hv_timer_in_use = true; > > - trace_kvm_hv_timer_state(apic->vcpu->vcpu_id, > > + if (kvm_x86_ops->set_hv_timer) { > > + if (kvm_x86_ops->set_hv_timer(apic->vcpu, > > Would it be better that if set_hv_timer fails, we clear the vmx timer (i.e. the > VMCS field) before return the failure? I'm not sure if it make sense to clear > the previous setup if a new setup fails, although it seems OK for me, since > we have to cancel the hv_timer anyway. Good question. I think we should abstract a little the set_hv_timer and cancel_hv_timer calls, for example with a new function start_hv_tscdeadline. It's also simpler to avoid the race window by disabling interrupts around the calls to set_hv_timer and hrtimer_cancel. I'll see what I can come up with. Paolo