From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v7 3/3] x86, apicv: add virtual x2apic support Date: Tue, 25 Dec 2012 13:58:41 +0200 Message-ID: <20121225115841.GD17584@redhat.com> References: <20121224092343.GT17584@redhat.com> <20121225063845.GY17584@redhat.com> <20121225065057.GZ17584@redhat.com> <20121225073143.GA17584@redhat.com> <20121225075224.GB17584@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "kvm@vger.kernel.org" , "Shan, Haitao" , "Tian, Kevin" To: "Zhang, Yang Z" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:28953 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753641Ab2LYL6n (ORCPT ); Tue, 25 Dec 2012 06:58:43 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Tue, Dec 25, 2012 at 08:24:43AM +0000, Zhang, Yang Z wrote: > Gleb Natapov wrote on 2012-12-25: > > On Tue, Dec 25, 2012 at 07:46:53AM +0000, Zhang, Yang Z wrote: > >> Gleb Natapov wrote on 2012-12-25: > >>> On Tue, Dec 25, 2012 at 07:25:15AM +0000, Zhang, Yang Z wrote: > >>>> Gleb Natapov wrote on 2012-12-25: > >>>>> On Tue, Dec 25, 2012 at 06:42:59AM +0000, Zhang, Yang Z wrote: > >>>>>> Gleb Natapov wrote on 2012-12-25: > >>>>>>> On Mon, Dec 24, 2012 at 11:53:37PM +0000, Zhang, Yang Z wrote: > >>>>>>>> Gleb Natapov wrote on 2012-12-24: > >>>>>>>>> On Mon, Dec 24, 2012 at 02:35:35AM +0000, Zhang, Yang Z wrote: > >>>>>>>>>> Zhang, Yang Z wrote on 2012-12-24: > >>>>>>>>>>> Gleb Natapov wrote on 2012-12-20: > >>>>>>>>>>>> On Mon, Dec 17, 2012 at 01:30:50PM +0800, Yang Zhang wrote: > >>>>>>>>>>>>> basically to benefit from apicv, we need clear MSR bitmap for > >>>>>>>>>>>>> corresponding x2apic MSRs: > >>>>>>>>>>>>> 0x800 - 0x8ff: no read intercept for apicv register > >>>>>>>>>>>>> virtualization TPR,EOI,SELF-IPI: no write intercept for > >>>>>>>>>>>>> virtual interrupt > >>> delivery > >>>>>>>>>>>> We do not set "Virtualize x2APIC mode" bit in secondary > >>>>>>>>>>>> execution control. If I read the spec correctly without that > >>>>>>>>>>>> those MSR read/writes will go straight to physical local APIC. > >>>>>>>>>>> Right. Now it cannot get benefit, but we may enable it in > >>>>>>>>>>> future and then we can benefit from it. > >>>>>>>>> Without enabling it you cannot disable MSR intercept for x2apic > >>>>>>>>> MSRs. > >>>>>>>>> > >>>>>>>>>> how about to add the following check: > >>>>>>>>>> if (apicv_enabled && virtual_x2apic_enabled) > >>>>>>>>>> clear_msr(); > >>>>>>>>>> > >>>>>>>>> I do not understand what do you mean here. > >>>>>>>> In this patch, it will clear MSR bitmap(0x800 -0x8ff) when apicv > > enabled. > >>> As > >>>>> you > >>>>>>> said, since kvm doesn't set "virtualize x2apic mode", APIC register > >>>>>>> virtualization never take effect. So we need to clear MSR bitmap only > >>>>>>> when apicv enabled and virtualize x2apic mode set. > >>>>>>>> > >>>>>>> But currently it is never set. > >>>>>> So you think the third patch is not necessary currently? Unless we > >>>>>> enabled "virtualize x2apic mode". > >>>>>> > >>>>> Without third patch vid will not work properly if a guest is in x2apic > >>>>> mode. Actually second and third patches need to be reordered to not have > >>>>> a windows where x2apic is broken. The problem is that this patch itself > >>>>> is buggy since it does not set "virtualize x2apic mode" flag. It should > >>>>> set the flag if vid is enabled and if the flag cannot be set vid should > >>>>> be forced off. > >>>> In what conditions this flag cannot be set? I think the only case is that KVM > >>> doesn't expose the x2apic capability to guest, if this is true, the > >>> guest will never use x2apic and we still can use vid. > >>>> > >>> We can indeed set "virtualize x2apic mode" unconditionally since it does > >>> not take any effect if x2apic MSRs are intercepted. > >> No. Since "Virtual APIC access" must be cleared if "virtualize x2apic mode" is set, > > and if guest still use xAPIC, then there should be lots of ept violations for apic > > access emulation. This will hurt performance. > > Stupid HW, why this pointless limitation? Can you point me where SDM says that? > Vol 3, 26.2.1.1 > Thanks. > >> We should only set "virtualize x2apic mode" when guest really uses > >> x2apic(guest set bit 11 of APIC_BASE_MSR). > >> > > Looks like SDM force us to. > > And we can disable x2apic MSR interception only after "virtualize x2apic mode" is set i.e when guest sets bit 11 of APIC_BASE_MSR. -- Gleb.