From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH 2/9] KVM: x86: dynamic kvm_apic_map
Date: Wed, 25 May 2016 18:15:00 +0200
Message-ID: <20160525161459.GF14795@potion>
References: <1462568045-31085-1-git-send-email-rkrcmar@redhat.com>
 <1462568045-31085-3-git-send-email-rkrcmar@redhat.com>
 <20160523080401.GC8247@pxdev.xzpeter.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	"Lan, Tianyu" <tianyu.lan@intel.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Jan Kiszka <jan.kiszka@web.de>
To: Peter Xu <peterx@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:56002 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754974AbcEYQPF (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 25 May 2016 12:15:05 -0400
Content-Disposition: inline
In-Reply-To: <20160523080401.GC8247@pxdev.xzpeter.org>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

2016-05-23 16:04+0800, Peter Xu:
> Hi, Radim,
>=20
> Got several questions inline.
>=20
> On Fri, May 06, 2016 at 10:53:58PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99=
 wrote:
>> x2APIC supports up to 2^32-1 LAPICs, but most guest in coming years =
will
>> have slighly less VCPUs.  Dynamic size saves memory at the cost of
>> turning one constant into a variable.
>=20
> From the manual, I see x2apic only support 2^20-16 processors, not
> 2^32-1.  Which one is correct?

"up to" is a crucial part.  Physical x2APIC can address 2^32-1, logical
x2APIC can address (2^16-1)*16 LAPICs and the OS can choose which mode
to use.

I think that machines with APIC IDs >2^20 will still be able to use
logical x2APIC, but higher APIC IDs are only be addressable with
physical x2APIC.

(Well, broadcasts would make even xAPIC "support" an unbounded amount o=
f
 LAPICs. :])

> From 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.
>=20
> [...]
>=20
>> +static inline bool kvm_apic_map_get_logical_dest(struct kvm_apic_ma=
p *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;
>=20
> [1]

This function is called ...get_LOGICAL_dest, so only logical addresses
are going to be passed to it.  Yes, it cannot handle APIC ID > 2^20-16.

> [...]
>=20
>>  static void recalculate_apic_map(struct kvm *kvm)
>> @@ -156,17 +162,28 @@ static void recalculate_apic_map(struct kvm *k=
vm)
>>  	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;
>=20
> Is this same as:
>=20
>   size =3D round_up(size, 16);
>=20
> ?

It is, thank you.