From patchwork Tue Apr 21 08:47:37 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 6246261 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 5C21BBF4A6 for ; Tue, 21 Apr 2015 08:48:29 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 71AE4203EB for ; Tue, 21 Apr 2015 08:48:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 8022B2028D for ; Tue, 21 Apr 2015 08:48:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753056AbbDUIsF (ORCPT ); Tue, 21 Apr 2015 04:48:05 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:34758 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751505AbbDUIrn (ORCPT ); Tue, 21 Apr 2015 04:47:43 -0400 Received: by wicmx19 with SMTP id mx19so58444425wic.1; Tue, 21 Apr 2015 01:47:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; bh=w2SOCboSgfgthAcNgeq4oz1BkT3E0oBAH7al66fT8Wo=; b=gaVbQDwYhZVKwFzYVGmX57EU4y6tZ/1HanXYlNbYOdrNBeVGZHFDM2jQjt4X6LmihY +UMWdfSSLua9y4ixz6NQNmd7AsiVcNyHzFAPGdH4pcDkApqBCeMGYtMyYlyFqQ/5Gjmu NgJk3ruvKGD85xO2aODgMVfMCSnhbuCKLpPpW/hPiuW0inkLzCk5Yw5RleUt5k4CSkB1 Zwkluc2rk5leASyOKxrtdlMkHMEnbGOdwP7Gijr5iQ8BvgN+HIGaFPNU3FNpWDznv/93 /RsZmSvEBFL54LyKKr249mpfLw6bymxvfre05btPIgBa8YVecjHNiw53JnD0XCXg4B4F o4xw== X-Received: by 10.194.60.4 with SMTP id d4mr39457158wjr.72.1429606062310; Tue, 21 Apr 2015 01:47:42 -0700 (PDT) Received: from [192.168.10.150] (net-93-66-123-41.cust.vodafonedsl.it. [93.66.123.41]) by mx.google.com with ESMTPSA id mv11sm18668931wic.23.2015.04.21.01.47.39 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Apr 2015 01:47:40 -0700 (PDT) Message-ID: <55360EA9.2030806@redhat.com> Date: Tue, 21 Apr 2015 10:47:37 +0200 From: Paolo Bonzini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: linux-kernel@vger.kernel.org, kvm@vger.kernel.org CC: saso.linux@astim.si, lists2009@fnarfbargle.com, Nadav Amit , stable@vger.kernel.org Subject: Re: [PATCH stable] KVM: x86: Fix lost interrupt on irr_pending race References: <1429602745-29882-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1429602745-29882-1-git-send-email-pbonzini@redhat.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 21/04/2015 09:52, Paolo Bonzini wrote: > From: Nadav Amit > > [ upstream commit f210f7572bedf3320599e8b2d8e8ec2d96270d0b ] > > apic_find_highest_irr assumes irr_pending is set if any vector in APIC_IRR is > set. If this assumption is broken and apicv is disabled, the injection of > interrupts may be deferred until another interrupt is delivered to the guest. > Ultimately, if no other interrupt should be injected to that vCPU, the pending > interrupt may be lost. > > commit 56cc2406d68c ("KVM: nVMX: fix "acknowledge interrupt on exit" when APICv > is in use") changed the behavior of apic_clear_irr so irr_pending is cleared > after setting APIC_IRR vector. After this commit, if apic_set_irr and > apic_clear_irr run simultaneously, a race may occur, resulting in APIC_IRR > vector set, and irr_pending cleared. In the following example, assume a single > vector is set in IRR prior to calling apic_clear_irr: > > apic_set_irr apic_clear_irr > ------------ -------------- > apic->irr_pending = true; > apic_clear_vector(...); > vec = apic_search_irr(apic); > // => vec == -1 > apic_set_vector(...); > apic->irr_pending = (vec != -1); > // => apic->irr_pending == false > > Nonetheless, it appears the race might even occur prior to this commit: > > apic_set_irr apic_clear_irr > ------------ -------------- > apic->irr_pending = true; > apic->irr_pending = false; > apic_clear_vector(...); > if (apic_search_irr(apic) != -1) > apic->irr_pending = true; > // => apic->irr_pending == false > apic_set_vector(...); > > Fixing this issue by: > 1. Restoring the previous behavior of apic_clear_irr: clear irr_pending, call > apic_clear_vector, and then if APIC_IRR is non-zero, set irr_pending. > 2. On apic_set_irr: first call apic_set_vector, then set irr_pending. > > Signed-off-by: Nadav Amit > Fixes: 33e4c68656a2e461b296ce714ec322978de85412 > Cc: stable@vger.kernel.org # 2.6.32+ > Signed-off-by: Paolo Bonzini > --- > The race was reported in 3.17+ by Brad Campbell and in > 2.6.32 by Saso Slavicic, so it qualifies for stable. Patch for kernels before 3.17: Thanks, Paolo --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 6e8ce5a1a05d..e0e5642dae41 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -341,8 +341,12 @@ EXPORT_SYMBOL_GPL(kvm_apic_update_irr); static inline void apic_set_irr(int vec, struct kvm_lapic *apic) { - apic->irr_pending = true; apic_set_vector(vec, apic->regs + APIC_IRR); + /* + * irr_pending must be true if any interrupt is pending; set it after + * APIC_IRR to avoid race with apic_clear_irr + */ + apic->irr_pending = true; } static inline int apic_search_irr(struct kvm_lapic *apic)