Re: [PATCH 1/9] KVM: x86: add kvm_apic_map_get_dest_lapic

[Peter Xu <peterx@redhat.com>]

On Wed, May 25, 2016 at 06:02:46PM +0200, Radim Krčmář wrote:

[...]

> kvm_apic_map_get_dest_lapic() returns false if it doesn't know how to
> handle the irq, so another function has to be used to get destinations
> of the interrupt in question.
> 
> 'irq->dest_id >= ARRAY_SIZE(map->phys_map)' is a case where we know that
> the destination doesn't exist, so asking another function to get
> destinations would be pure waste. but the destination doesn't exist, so
> *bitmap has to be 0 to avoid invalid accesses.
> 
> "return true;" means that **dst offsets encoded in *bitmap are correct
> destination(s).  Ah, it is horribly convoluted.  I am not sure if I can
> improve the comment, though ...

Thanks for the explanation.

How about: "Return true if the interrupt can be handled by fast path,
false otherwise (in which case we still need to go through the slow
path). Note: we may have zero kvm_lapic candidate even when we return
true, which means the interrupt should be dropped. In this case,
*bitmap would be zero."

> 
> > [...]
> > 
> > > @@ -821,69 +835,16 @@ bool kvm_intr_is_single_vcpu_fast(struct kvm *kvm, struct kvm_lapic_irq *irq,
> > >  	rcu_read_lock();
> > >  	map = rcu_dereference(kvm->arch.apic_map);
> > >  
> > > -	if (!map)
> > > -		goto out;
> > > +	if (kvm_apic_map_get_dest_lapic(kvm, NULL, irq, map, &dst, &bitmap) &&
> > > +			hweight16(bitmap) == 1) {
> > 
> > If we follow the rule in comment above (return true only if there
> > are valid candidates), we may avoid the hweight16(bitmap) == 1
> > check as well.
> 
> hweight16(bitmap) ranges from 0 to 16.  Logical destination mode has
> multicast (the maximal addressible unit is one cluster, which has 16
> LAPICs), but multicast cannot be optimized by hardware, so we have a
> special handling of cases where the destination is just one VCPU.
> 
> e.g. for bitmap = 0b100101, the interrupt is directed to dst[0], dst[2]
> and dst[5].

Yes, thanks!

-- peterx