From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04963C10F14 for ; Tue, 16 Apr 2019 17:49:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D69352087C for ; Tue, 16 Apr 2019 17:49:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730255AbfDPRtH (ORCPT ); Tue, 16 Apr 2019 13:49:07 -0400 Received: from mga06.intel.com ([134.134.136.31]:25058 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730236AbfDPRtE (ORCPT ); Tue, 16 Apr 2019 13:49:04 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Apr 2019 10:49:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,358,1549958400"; d="scan'208";a="224069834" Received: from sjchrist-coffee.jf.intel.com ([10.54.74.181]) by orsmga001.jf.intel.com with ESMTP; 16 Apr 2019 10:49:02 -0700 From: Sean Christopherson To: Paolo Bonzini , =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= Cc: kvm@vger.kernel.org, Liran Alon , Wanpeng Li Subject: [PATCH v2 6/7] KVM: lapic: Refactor start_hv_timer()'s semantics and usage Date: Tue, 16 Apr 2019 10:48:56 -0700 Message-Id: <20190416174857.506-7-sean.j.christopherson@intel.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190416174857.506-1-sean.j.christopherson@intel.com> References: <20190416174857.506-1-sean.j.christopherson@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org start_hv_timer() currently returns a bool, with %false interpreted as "failure" and %true as "success". But the reality is far more complex, as the %false is also returned when programming was successful, but must be canceled because there was already a pending timer interrupt. It can also return %false when programming was successful but the timer was marked expired because the deadline already passed, in which case the hv timer must also be canceled to avoid double injection. Functionally, the existing code is correct in the sense that it doesn't doing anything visibily wrong, e.g. generate spurious interrupts or miss an interrupt. But it's extremely confusing and inefficient, e.g. there are multiple extraneous calls to apic_timer_expired() that effectively get dropped due to @timer_pending being %true. Refactor start_hv_timer() to return an 'int' and move the handling of the various combinations of success vs. expiration vs. periodic up a level to restart_apic_timer(), which eliminates the needs to overload the meaning of %false. Cc: Wanpeng Li Suggested-by: Liran Alon Suggested-by: Paolo Bonzini Signed-off-by: Sean Christopherson --- arch/x86/kvm/lapic.c | 59 ++++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index fa2dbca5ad6a..b867569107d5 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1678,41 +1678,35 @@ static void cancel_hv_timer(struct kvm_lapic *apic) apic->lapic_timer.hv_timer_in_use = false; } -static bool start_hv_timer(struct kvm_lapic *apic) +/* + * Return: + * Negative errno on failure, + * '1' on success and the timer's deadline has passed, + * '0' on success and the timer's deadline is some time in the future. + */ +static int start_hv_timer(struct kvm_lapic *apic) { struct kvm_timer *ktimer = &apic->lapic_timer; int r; WARN_ON(preemptible()); if (!kvm_x86_ops->set_hv_timer) - return false; + return -ENOENT; if (!apic_lvtt_period(apic) && atomic_read(&ktimer->pending)) - return false; + return -EBUSY; if (!ktimer->tscdeadline) - return false; + return -EINVAL; r = kvm_x86_ops->set_hv_timer(apic->vcpu, ktimer->tscdeadline); if (r < 0) - return false; + return r; ktimer->hv_timer_in_use = true; hrtimer_cancel(&ktimer->timer); - /* - * Also recheck ktimer->pending, in case the sw timer triggered in - * the window. For periodic timer, leave the hv timer running for - * simplicity, and the deadline will be recomputed on the next vmexit. - */ - if (!apic_lvtt_period(apic) && (r || atomic_read(&ktimer->pending))) { - if (r) - apic_timer_expired(apic); - return false; - } - - trace_kvm_hv_timer_state(apic->vcpu->vcpu_id, true); - return true; + return r; } static void start_sw_timer(struct kvm_lapic *apic) @@ -1734,9 +1728,36 @@ static void start_sw_timer(struct kvm_lapic *apic) static void restart_apic_timer(struct kvm_lapic *apic) { + int r; + preempt_disable(); - if (!start_hv_timer(apic)) + r = start_hv_timer(apic); + if (r < 0) { start_sw_timer(apic); + } else if (!apic_lvtt_period(apic)) { + /* + * For a periodic timer, leave the timer running for simplicity + * so that the deadline will be recomputed on the next VM-Exit. + */ + if (atomic_read(&apic->lapic_timer.pending)) { + /* + * Cancel the hv timer if the sw timer fired while the + * hv timer was being programmed. + */ + cancel_hv_timer(apic); + } else if (r) { + /* + * start_hv_timer() returns '1' if the timer's deadline + * has already passed. Again, go through with the full + * programming in the periodic case as the VM-Exit is + * needed to recompute the periodic timer deadline. + */ + apic_timer_expired(apic); + cancel_hv_timer(apic); + } + } + trace_kvm_hv_timer_state(apic->vcpu->vcpu_id, + apic->lapic_timer.hv_timer_in_use); preempt_enable(); } -- 2.21.0