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? >> Or we can just simply put the assignment of apic_base to the end. > > Yes, this would work, I'd also remove recalculates from > kvm_apic_set_*apic_id() and add a compiler barrier with comment for good > measure, even though set_virtual_x2apic_mode() serves as one. Why a compiler barrier? > (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... > 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... Paolo > Having apic id stored in big endian, or "0-7,8-31" format, would be > safest. I wanted to change the apic map to do incremental updates with > with respect to the APIC that has changed, instead of being completely > recomputed, so maybe the time is now. :) >
