From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [PATCH 1/9] KVM: x86: add kvm_apic_map_get_dest_lapic Date: Thu, 26 May 2016 19:58:17 +0800 Message-ID: <20160526115817.GA10883@pxdev.xzpeter.org> References: <1462568045-31085-1-git-send-email-rkrcmar@redhat.com> <1462568045-31085-2-git-send-email-rkrcmar@redhat.com> <20160519063606.GA8574@pxdev.xzpeter.org> <20160525160245.GE14795@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Paolo Bonzini , "Lan, Tianyu" , Igor Mammedov , Jan Kiszka To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mx1.redhat.com ([209.132.183.28]:40264 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752083AbcEZL6Z (ORCPT ); Thu, 26 May 2016 07:58:25 -0400 Content-Disposition: inline In-Reply-To: <20160525160245.GE14795@potion> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, May 25, 2016 at 06:02:46PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: [...] > 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 destination= s > of the interrupt in question. >=20 > 'irq->dest_id >=3D 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. >=20 > "return true;" means that **dst offsets encoded in *bitmap are correc= t > destination(s). Ah, it is horribly convoluted. I am not sure if I c= an > 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." >=20 > > [...] > >=20 > > > @@ -821,69 +835,16 @@ bool kvm_intr_is_single_vcpu_fast(struct kv= m *kvm, struct kvm_lapic_irq *irq, > > > rcu_read_lock(); > > > map =3D rcu_dereference(kvm->arch.apic_map); > > > =20 > > > - if (!map) > > > - goto out; > > > + if (kvm_apic_map_get_dest_lapic(kvm, NULL, irq, map, &dst, &bit= map) && > > > + hweight16(bitmap) =3D=3D 1) { > >=20 > > If we follow the rule in comment above (return true only if there > > are valid candidates), we may avoid the hweight16(bitmap) =3D=3D 1 > > check as well. >=20 > 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. >=20 > e.g. for bitmap =3D 0b100101, the interrupt is directed to dst[0], ds= t[2] > and dst[5]. Yes, thanks! -- peterx