Re: [PATCH v2 2/4] KVM: x86: replace kvm_apic_id with kvm_{x,x2}apic_id

[David Hildenbrand]

>  	kvm_for_each_vcpu(i, vcpu, kvm)
>  		if (kvm_apic_present(vcpu))
> -			max_id = max(max_id, kvm_apic_id(vcpu->arch.apic));
> +			max_id = max(max_id, kvm_x2apic_id(vcpu->arch.apic));
>
>  	new = kvm_kvzalloc(sizeof(struct kvm_apic_map) +
>  	                   sizeof(struct kvm_lapic *) * ((u64)max_id + 1));
> @@ -179,16 +189,23 @@ static void recalculate_apic_map(struct kvm *kvm)
>  		struct kvm_lapic *apic = vcpu->arch.apic;
>  		struct kvm_lapic **cluster;
>  		u16 mask;
> -		u32 ldr, aid;
> +		u32 ldr;
> +		u8 xapic_id;
> +		u32 x2apic_id;
>
>  		if (!kvm_apic_present(vcpu))
>  			continue;
>
> -		aid = kvm_apic_id(apic);

think I'd even prefer here a simple

aid = kvm_xapic_id(apic);
if (apic_x2apic_mode(apic))
	aid = kvm_x2apic_id(apic);

that would keep changes minimal and I don't really see any benefit in 
the code when splitting handling up.

Patch 4 then simply can fixup setting code

if (aid <= new->max_apic_id && !new->phys_map[aid])
	new->phys_map[aid] = apic;

(if I am not missing some important corner case here)


> -		ldr = kvm_lapic_get_reg(apic, APIC_LDR);
> +		xapic_id = kvm_xapic_id(apic);
> +		x2apic_id = kvm_x2apic_id(apic);
>
> -		if (aid <= new->max_apic_id)
> -			new->phys_map[aid] = apic;
> +		if (apic_x2apic_mode(apic) &&
> +				x2apic_id <= new->max_apic_id)
> +			new->phys_map[x2apic_id] = apic;
> +		else if (!apic_x2apic_mode(apic))


This looks good to me.

--

David