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

Paolo Bonzini
2016-07-01T14:12:16Z

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