Re: [PATCH v1 06/11] KVM: x86: use hardware-compatible format for APIC ID register

[Paolo Bonzini]

On 01/07/2016 15:11, Radim KrÄmÃÅ wrote:
>>> >> +static void __kvm_apic_state_fixup(struct kvm_vcpu *vcpu,
>>> >> +		struct kvm_lapic_state *s, bool set)
>>> >> +{
>>> >> +	if (apic_x2apic_mode(vcpu->arch.apic)) {
>>> >> +		u32 *id = (u32 *)(s->regs + APIC_ID);
>>> >> +		if (set)
>>> >> +			*id >>= 24;
>>> >> +		else
>>> >> +			*id <<= 24;
>>> >> +	}
>> > 
>> > When setting, this should read from the apic_base being set.  So I think
>> > you cannot use apic_x2apic_mode.
> 
> apic_x2apic_mode uses apic_base MSR, so its value does not depend on
> LAPIC_SET/GET.  I don't like the dependency much, but all combinations
> of values/orders should work well.

You're right of course.  However it should be documented in the
KVM_GET_LAPIC/KVM_SET_LAPIC ioctl docs that KVM_SET_MSR for apic_base
should be performed first.

Should kvm_lapic_set_base change the value of the ID register when
x2apic mode is left just like we do for entering x2apic mode?  (Hint: we
want kvm-unit-tests for this).

Paolo