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. Paolo > >> - 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. > -- 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