From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Xu Subject: Re: [PATCH 2/9] KVM: x86: dynamic kvm_apic_map Date: Mon, 23 May 2016 16:04:01 +0800 Message-ID: <20160523080401.GC8247@pxdev.xzpeter.org> References: <1462568045-31085-1-git-send-email-rkrcmar@redhat.com> <1462568045-31085-3-git-send-email-rkrcmar@redhat.com> 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]:42773 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753279AbcEWIEF (ORCPT ); Mon, 23 May 2016 04:04:05 -0400 Content-Disposition: inline In-Reply-To: <1462568045-31085-3-git-send-email-rkrcmar@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi, Radim, Got several questions inline. On Fri, May 06, 2016 at 10:53:58PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > x2APIC supports up to 2^32-1 LAPICs, but most guest in coming years w= ill > have slighly less VCPUs. Dynamic size saves memory at the cost of > turning one constant into a variable. =46rom the manual, I see x2apic only support 2^20-16 processors, not 2^32-1. Which one is correct? =46rom below codes [1], I still see 2^20-16, since we are using high 16 bits of dest ID as cluster ID (0-0xfffe, while I guess 0xffff should be reserved), and lower 16 bits as processor/lapic mask. [...] > +static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_map= *map, > + u32 dest_id, struct kvm_lapic ***cluster, u16 *mask) { > + switch (map->mode) { > + case KVM_APIC_MODE_X2APIC: { > + u32 offset =3D (dest_id >> 16) * 16; > + // XXX: phys_map must be allocated as multiples of 16 > + if (offset < map->size) { > + *cluster =3D &map->phys_map[offset]; > + *mask =3D dest_id & 0xffff; [1] [...] > static void recalculate_apic_map(struct kvm *kvm) > @@ -156,17 +162,28 @@ static void recalculate_apic_map(struct kvm *kv= m) > struct kvm_apic_map *new, *old =3D NULL; > struct kvm_vcpu *vcpu; > int i; > - > - new =3D kzalloc(sizeof(struct kvm_apic_map), GFP_KERNEL); > + u32 size =3D 255; > =20 > mutex_lock(&kvm->arch.apic_map_lock); > =20 > + kvm_for_each_vcpu(i, vcpu, kvm) > + if (kvm_apic_present(vcpu)) > + size =3D max(size, kvm_apic_id(vcpu->arch.apic)); > + > + size++; > + size +=3D (16 - size) & 16; Is this same as: size =3D round_up(size, 16); ? Thanks, -- peterx