Re: [PATCH v5 3/6] KVM : Calculate destination vcpu on interrupt injection

[Gleb Natapov <gleb@redhat.com>]

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 <yang.z.zhang@Intel.com>
> >>>> 
> >>>> Add a new parameter to know vcpus who received the interrupt.
> >>>> 
> >>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> ---
> >>>>  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.