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 09:31:43 +0200 Message-ID: <20121225073143.GA17584@redhat.com> References: <1355722250-7122-1-git-send-email-yang.z.zhang@intel.com> <1355722250-7122-4-git-send-email-yang.z.zhang@intel.com> <20121220083106.GE17584@redhat.com> <20121224092343.GT17584@redhat.com> <20121225063845.GY17584@redhat.com> <20121225065057.GZ17584@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]:15429 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753506Ab2LYHbp (ORCPT ); Tue, 25 Dec 2012 02:31:45 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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. -- Gleb.