2016-12-14 17:59+0100, Paolo Bonzini: > On 14/12/2016 17:15, David Hildenbrand wrote: >> >>> 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) > > Radim, what do you think? I wanted to get these in before Christmas, > but it's your call. There was a reason why it was so ugly ... it's not a hack for nothing. I can hope to make the patches/code more understandable, but the function shouldn't change, unless we want to take a different approach. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html