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 08:50:57 +0200 Message-ID: <20121225065057.GZ17584@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> 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]:51552 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753578Ab2LYGu7 (ORCPT ); Tue, 25 Dec 2012 01:50:59 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 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. -- Gleb.