From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH v1 06/11] KVM: x86: use hardware-compatible format for
 APIC ID register
Date: Fri, 1 Jul 2016 16:54:07 +0200
Message-ID: <20160701145407.GC2301@potion>
References: <20160630205429.16480-1-rkrcmar@redhat.com>
 <20160630205429.16480-7-rkrcmar@redhat.com>
 <733b706a-f30f-3854-5fc7-402d6b5fd79a@redhat.com>
 <20160701131119.GC27840@potion>
 <217814b5-d637-07f8-48cd-777ad498c54f@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	"Lan, Tianyu" <tianyu.lan@intel.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Jan Kiszka <jan.kiszka@web.de>, Peter Xu <peterx@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <217814b5-d637-07f8-48cd-777ad498c54f@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org

2016-07-01 16:12+0200, Paolo Bonzini:
> On 01/07/2016 15:11, Radim Kr=C4=8Dm=C3=A1=C5=99 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 =3D (u32 *)(s->regs + APIC_ID);
>>>> >> +		if (set)
>>>> >> +			*id >>=3D 24;
>>>> >> +		else
>>>> >> +			*id <<=3D 24;
>>>> >> +	}
>>> >=20
>>> > When setting, this should read from the apic_base being set.  So =
I think
>>> > you cannot use apic_x2apic_mode.
>>=20
>> 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 combinatio=
ns
>> of values/orders should work well.
>=20
> 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 t=
o
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 =3D apic_id()
  set_apic_id(id+1) // ?
  enable_x2apic()
  id =3D=3D apic_id() & 0xff
  disable_apic()
  enable_xapic()
  id =3D=3D apic_id()