Re: [PATCH 2/9] KVM: x86: dynamic kvm_apic_map

[Peter Xu <peterx@redhat.com>]

Hi, Radim,

Got several questions inline.

On Fri, May 06, 2016 at 10:53:58PM +0200, Radim Krčmář 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.

From the manual, I see x2apic only support 2^20-16 processors, not
2^32-1.  Which one is correct?

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.

[...]

> +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 = (dest_id >> 16) * 16;
> +		// XXX: phys_map must be allocated as multiples of 16
> +		if (offset < map->size) {
> +			*cluster = &map->phys_map[offset];
> +			*mask = dest_id & 0xffff;

[1]

[...]

>  static void recalculate_apic_map(struct kvm *kvm)
> @@ -156,17 +162,28 @@ static void recalculate_apic_map(struct kvm *kvm)
>  	struct kvm_apic_map *new, *old = NULL;
>  	struct kvm_vcpu *vcpu;
>  	int i;
> -
> -	new = kzalloc(sizeof(struct kvm_apic_map), GFP_KERNEL);
> +	u32 size = 255;
>  
>  	mutex_lock(&kvm->arch.apic_map_lock);
>  
> +	kvm_for_each_vcpu(i, vcpu, kvm)
> +		if (kvm_apic_present(vcpu))
> +			size = max(size, kvm_apic_id(vcpu->arch.apic));
> +
> +	size++;
> +	size += (16 - size) & 16;

Is this same as:

  size = round_up(size, 16);

?

Thanks,

-- peterx