Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757238AbcJNHMZ (ORCPT ); Fri, 14 Oct 2016 03:12:25 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:33458 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754216AbcJNHMQ (ORCPT ); Fri, 14 Oct 2016 03:12:16 -0400 Subject: Re: [PATCH 2/3] kvm: x86: do not use KVM_REQ_EVENT for APICv interrupt injection To: Paolo Bonzini , linux-kernel@vger.kernel.org, kvm@vger.kernel.org References: <1475011213-34225-1-git-send-email-pbonzini@redhat.com> <1475011213-34225-3-git-send-email-pbonzini@redhat.com> Cc: feng.wu@intel.com, mst@redhat.com, rkrcmar@redhat.com From: Yang Zhang Message-ID: Date: Fri, 14 Oct 2016 15:12:08 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1475011213-34225-3-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3653 Lines: 110 On 2016/9/28 5:20, Paolo Bonzini wrote: > Since bf9f6ac8d749 ("KVM: Update Posted-Interrupts Descriptor when vCPU > is blocked", 2015-09-18) the posted interrupt descriptor is checked > unconditionally for PIR.ON. Therefore we don't need KVM_REQ_EVENT to > trigger the scan and, if NMIs or SMIs are not involved, we can avoid > the complicated event injection path. > > However, there is a race between vmx_deliver_posted_interrupt and > vcpu_enter_guest. Fix it by disabling interrupts before vcpu->mode is > set to IN_GUEST_MODE. > > Calling kvm_vcpu_kick if PIR.ON=1 is also useless, though it has been > there since APICv was introduced. > > Signed-off-by: Paolo Bonzini > --- > arch/x86/kvm/lapic.c | 2 -- > arch/x86/kvm/vmx.c | 8 +++++--- > arch/x86/kvm/x86.c | 9 +++++++-- > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 63a442aefc12..be8b7ad56dd1 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -356,8 +356,6 @@ void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir) > struct kvm_lapic *apic = vcpu->arch.apic; > > __kvm_apic_update_irr(pir, apic->regs); > - > - kvm_make_request(KVM_REQ_EVENT, vcpu); > } > EXPORT_SYMBOL_GPL(kvm_apic_update_irr); > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index b33eee395b00..207b9aa32915 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -4844,9 +4844,11 @@ static void vmx_deliver_posted_interrupt(struct kvm_vcpu *vcpu, int vector) > if (pi_test_and_set_pir(vector, &vmx->pi_desc)) > return; > > - r = pi_test_and_set_on(&vmx->pi_desc); > - kvm_make_request(KVM_REQ_EVENT, vcpu); Hi Paolo I remember that a request is necessary before vcpu kick. Otherwise, the interrupt cannot be serviced in time. In this case, if the posted interrupt delivery occurs between a and c: vcpu_enter_guest: a. check pending interupt b. an interrupt is delivered from other vcpus(vmx_deliver_posted_interrupt() is called ) c.vcpu->mode = IN_GUEST_MODE; Previously, the vcpu will aware there is a pending request(interrupt) after c: if (vcpu->request) goto cancel_injection with this patch, since there is no request and vcpu will continue enter guest without handling the pending interrupt.(kvm_vcpu_kick does nothing since the mode isn't equal to IN_GUEST_MODE) Can this case happen? > - if (r || !kvm_vcpu_trigger_posted_interrupt(vcpu)) > + /* If a previous notification has sent the IPI, nothing to do. */ > + if (pi_test_and_set_on(&vmx->pi_desc)) > + return; > + > + if (!kvm_vcpu_trigger_posted_interrupt(vcpu)) > kvm_vcpu_kick(vcpu); > } > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 3ee8a91a78c3..604cfbfc5bee 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6658,6 +6658,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > kvm_x86_ops->prepare_guest_switch(vcpu); > if (vcpu->fpu_active) > kvm_load_guest_fpu(vcpu); > + > + /* > + * Disable IRQs before setting IN_GUEST_MODE, so that > + * posted interrupts with vcpu->mode == IN_GUEST_MODE > + * always result in virtual interrupt delivery. > + */ > + local_irq_disable(); > vcpu->mode = IN_GUEST_MODE; > > srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > @@ -6671,8 +6678,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > */ > smp_mb__after_srcu_read_unlock(); > > - local_irq_disable(); > - > if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests > || need_resched() || signal_pending(current)) { > vcpu->mode = OUTSIDE_GUEST_MODE; > -- Yang Alibaba Cloud Computing