From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection Date: Thu, 21 Mar 2013 14:21:30 +0200 Message-ID: <20130321122130.GS3889@redhat.com> References: <1363862964-25148-1-git-send-email-yang.z.zhang@intel.com> <1363862964-25148-4-git-send-email-yang.z.zhang@intel.com> <20130321114805.GP3889@redhat.com> <20130321120344.GQ3889@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "kvm@vger.kernel.org" , "mtosatti@redhat.com" , "Zhang, Xiantao" To: "Zhang, Yang Z" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56528 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754840Ab3CUMVc (ORCPT ); Thu, 21 Mar 2013 08:21:32 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Mar 21, 2013 at 12:12:06PM +0000, Zhang, Yang Z wrote: > Gleb Natapov wrote on 2013-03-21: > > On Thu, Mar 21, 2013 at 11:56:05AM +0000, Zhang, Yang Z wrote: > >> Gleb Natapov wrote on 2013-03-21: > >>> On Thu, Mar 21, 2013 at 06:49:21PM +0800, Yang Zhang wrote: > >>>> From: Yang Zhang > >>>> > >>>> Add a new parameter to know vcpus who received the interrupt. > >>>> > >>>> Signed-off-by: Yang Zhang > >>>> --- > >>>> arch/x86/kvm/lapic.c | 21 ++++++++++++++++----- > >>>> arch/x86/kvm/lapic.h | 5 +++-- > >>>> virt/kvm/ioapic.c | 2 +- > >>>> virt/kvm/ioapic.h | 2 +- > >>>> virt/kvm/irq_comm.c | 12 ++++++------ > >>>> 5 files changed, 27 insertions(+), 15 deletions(-) > >>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > >>>> index d3e322a..5f6b1d0 100644 > >>>> --- a/arch/x86/kvm/lapic.c > >>>> +++ b/arch/x86/kvm/lapic.c > >>>> @@ -433,10 +433,21 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu > >>> *vcpu) > >>>> static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode, > >>>> int vector, int level, int trig_mode); > >>>> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq) > >>>> +static void kvm_set_irq_dest_map(struct kvm_vcpu *vcpu, unsigned long > >>>> *dest_map) +{ + if (!kvm_lapic_enabled(vcpu)) + return; > >>> Why this check here? > >> The vcpu who didn't enable apic should not account as destination vcpu. > >> Without this check, if broadcast interrupt, all cpus will treat as > >> destination vcpu, but only those who enabled apic will receive the > >> interrupt. There are same check in __apic_accept_irq(): if > >> (unlikely(!apic_enabled(apic))) > >> break; > > I see, but you use more strict check that also checks that apic is > > emulated by the kernel and we wouldn't be here if it wasn't. Anyway lets > Do you mean the check add in here will block "userspace apic"? Shouldn't only in-kernel apic will get here? > No, it will not block. It checks for in kernel apic needlessly. Since we patch all those checks out anyway using jump labels it is not really affects performance, but I prefer to make only necessary checks for consistency. > > move bitmap update into __apic_accept_irq(). > Sure. > -- Gleb.