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

[Radim KrÄmÃÅ]

2016-07-01 16:12+0200, 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.

Ok, that will make our life easier.

> 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?

Yes, the patch does it.  The only possible change from x2APIC mode is to
hardware disabled mode and re-enabled xAPIC starts with initial APIC ID
in upper 8 bits.

>                                                                (Hint: we
> want kvm-unit-tests for this).

:) Something like this?

  enable_xapic()
  id = apic_id()
  set_apic_id(id+1) // ?
  enable_x2apic()
  id == apic_id() & 0xff
  disable_apic()
  enable_xapic()
  id == apic_id()