On 11/07/2016 17:52, Radim Krčmář wrote: > 2016-07-11 16:14+0200, Paolo Bonzini: >> On 11/07/2016 15:48, Radim Krčmář wrote: >>>>> I guess the easiest solution is to replace kvm_apic_id with a field in >>>>> struct kvm_lapic, which is already shifted right by 24 in xAPIC mode. >>> >>> (I guess the fewest LOC is to look at vcpu->vcpu_id, which is equal to >>> x2apic id. xapic id cannot be greater than 255 and all of those are >>> covered by the initial value of max_id.) >> >> Yes, this would work too. Or even better perhaps, look at vcpu->vcpu_id >> in kvm_apic_id? > > APIC ID is writeable in xAPIC mode, which would make the implementation > weird without an extra variable. Always read-only APIC ID would be > best, IMO. You can do if (x2apic mode) return lapic->vcpu->vcpu_id; else return get_reg(APIC_ID) >> 24; The point is to avoid returning a shifted APIC_ID without shifting it. The alternative of course is just caching it, which at this point is not particularly harder... Paolo >>> (What makes a bit wary is that it doesn't avoid the same problem if we >>> changed KVM to reset apic id to xapic id first when disabling apic.) >> >> Yes, this is why I prefer it fixed once and for all in kvm_apic_id... > > Seems most reasonable. We'll need to be careful to have a correct value > in the apic page, but there shouldn't be any races there. > >>> Races in recalculation and APIC ID changes also lead to invalid physical >>> maps, which haven't been taken care of properly ... >> >> Hmm, true, but can be fixed separately. Probably the mutex should be >> renamed so that it can be taken outside recalculate_apic_map... > > Good point, it'll make reasoning easier and shouldn't introduce any > extra scalability issues. -- 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
