From patchwork Tue Feb 7 21:49:47 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?UmFkaW0gS3LEjW3DocWZ?= X-Patchwork-Id: 9561295 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 831DA602B1 for ; Tue, 7 Feb 2017 21:50:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 76FF228475 for ; Tue, 7 Feb 2017 21:50:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BD072848D; Tue, 7 Feb 2017 21:50:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.9 required=2.0 tests=BAYES_00,HK_RANDOM_FROM, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 15E2E28475 for ; Tue, 7 Feb 2017 21:50:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932723AbdBGVtz (ORCPT ); Tue, 7 Feb 2017 16:49:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58524 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932428AbdBGVty (ORCPT ); Tue, 7 Feb 2017 16:49:54 -0500 Received: from smtp.corp.redhat.com (int-mx16.intmail.prod.int.phx2.redhat.com [10.5.11.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DAE2E80F9A; Tue, 7 Feb 2017 21:49:49 +0000 (UTC) Received: from potion (dhcp-1-184.brq.redhat.com [10.34.1.184]) by smtp.corp.redhat.com (Postfix) with SMTP id 0995C2E667; Tue, 7 Feb 2017 21:49:47 +0000 (UTC) Received: by potion (sSMTP sendmail emulation); Tue, 07 Feb 2017 22:49:47 +0100 Date: Tue, 7 Feb 2017 22:49:47 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org Subject: Re: [PATCH 5/6] KVM: x86: do not scan IRR twice on APICv vmentry Message-ID: <20170207214946.GF31091@potion> References: <1482164232-130035-1-git-send-email-pbonzini@redhat.com> <1482164232-130035-6-git-send-email-pbonzini@redhat.com> <20170207201955.GD31091@potion> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170207201955.GD31091@potion> X-Scanned-By: MIMEDefang 2.74 on 10.5.11.28 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 07 Feb 2017 21:49:49 +0000 (UTC) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 2017-02-07 21:19+0100, Radim Krčmář: > 2016-12-19 17:17+0100, Paolo Bonzini: > > Calls to apic_find_highest_irr are scanning IRR twice, once > > in vmx_sync_pir_from_irr and once in apic_search_irr. Change > > sync_pir_from_irr to get the new maximum IRR from kvm_apic_update_irr; > > now that it does the computation, it can also do the RVI write. > > > > In order to avoid complications in svm.c, make the callback optional. > > > > Signed-off-by: Paolo Bonzini > > --- > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > > @@ -8734,20 +8736,24 @@ static void vmx_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr) > > } > > } > > > > -static void vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu) > > +static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu) > > { > > struct vcpu_vmx *vmx = to_vmx(vcpu); > > + int max_irr; > > > > - if (!pi_test_on(&vmx->pi_desc)) > > - return; > > - > > - pi_clear_on(&vmx->pi_desc); > > - /* > > - * IOMMU can write to PIR.ON, so the barrier matters even on UP. > > - * But on x86 this is just a compiler barrier anyway. > > - */ > > - smp_mb__after_atomic(); > > - kvm_apic_update_irr(vcpu, vmx->pi_desc.pir); > > + if (vcpu->arch.apicv_active && pi_test_on(&vmx->pi_desc)) { > > + pi_clear_on(&vmx->pi_desc); > > + /* > > + * IOMMU can write to PIR.ON, so the barrier matters even on UP. > > + * But on x86 this is just a compiler barrier anyway. > > + */ > > + smp_mb__after_atomic(); > > + max_irr = kvm_apic_update_irr(vcpu, vmx->pi_desc.pir); > > + } else { > > + max_irr = kvm_lapic_find_highest_irr(vcpu); > > + } > > + vmx_hwapic_irr_update(vcpu, max_irr); > > Btw. a v1 discussion revolved about the need to have > vmx_hwapic_irr_update() here when the maximal IRR should always be in > RVI, and, uh, I didn't follow up (negligible attention span) ... > > There is one place where that doesn't hold: we don't update RVI after a > EXTERNAL_INTERRUPT nested VM exit without VM_EXIT_ACK_INTR_ON_EXIT, but > IRR has likely changed. Isn't that the problem? The following patch on top of the whole series survives kvm-unit-tests and very mimimal testing: (Not sure if I have missed something.) diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index a1e9cab7d01f..8b98c1681803 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -577,10 +577,10 @@ static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu) static int apic_has_interrupt_for_ppr(struct kvm_lapic *apic, u32 ppr) { - int highest_irr; + int highest_irr = -1; if (kvm_x86_ops->sync_pir_to_irr) highest_irr = kvm_x86_ops->sync_pir_to_irr(apic->vcpu); - else + if (highest_irr == -1) highest_irr = apic_find_highest_irr(apic); if (highest_irr == -1 || (highest_irr & 0xF0) <= ppr) return -1; diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 9c8a16edf88d..637c7bd2f3ab 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -8728,18 +8728,18 @@ static int vmx_sync_pir_to_irr(struct kvm_vcpu *vcpu) struct vcpu_vmx *vmx = to_vmx(vcpu); int max_irr; - if (vcpu->arch.apicv_active && pi_test_on(&vmx->pi_desc)) { - pi_clear_on(&vmx->pi_desc); - /* - * IOMMU can write to PIR.ON, so the barrier matters even on UP. - * But on x86 this is just a compiler barrier anyway. - */ - smp_mb__after_atomic(); - max_irr = kvm_apic_update_irr(vcpu, vmx->pi_desc.pir); - } else { - max_irr = kvm_lapic_find_highest_irr(vcpu); - } + if (!vcpu->arch.apicv_active || !pi_test_on(&vmx->pi_desc)) + return -1; + + pi_clear_on(&vmx->pi_desc); + /* + * IOMMU can write to PIR.ON, so the barrier matters even on UP. + * But on x86 this is just a compiler barrier anyway. + */ + smp_mb__after_atomic(); + max_irr = kvm_apic_update_irr(vcpu, vmx->pi_desc.pir); vmx_hwapic_irr_update(vcpu, max_irr); + return max_irr; } @@ -11145,6 +11145,7 @@ static void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, /* in case we halted in L2 */ vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE; + kvm_x86_ops->hwapic_irr_update(vcpu, kvm_lapic_find_highest_irr(vcpu)); } /*