From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM Date: Thu, 11 Oct 2012 05:56:09 -0300 Message-ID: <20121011085607.GA24206@amt.cnet> References: <96EC5A4F3149B74492D2D9B9B1602C2728B61A0D@ORSMSX108.amr.corp.intel.com> <20121008173055.GC12735@amt.cnet> <507414A2.2090706@redhat.com> <96EC5A4F3149B74492D2D9B9B1602C2728B708F3@ORSMSX108.amr.corp.intel.com> <20121010125242.GA15137@amt.cnet> <96EC5A4F3149B74492D2D9B9B1602C2728B71E30@ORSMSX108.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Avi Kivity , "kvm@vger.kernel.org" , "Zhang, Xiantao" , "Liu, Jinsong" To: "Auld, Will" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:17104 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753498Ab2JKJIZ convert rfc822-to-8bit (ORCPT ); Thu, 11 Oct 2012 05:08:25 -0400 Content-Disposition: inline In-Reply-To: <96EC5A4F3149B74492D2D9B9B1602C2728B71E30@ORSMSX108.amr.corp.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Oct 11, 2012 at 12:47:39AM +0000, Auld, Will wrote: > Marcelo, >=20 > You are suggesting that I: > Kvm_host.h: >=20 > Struct kvm_x86_ops { >=20 > ... > change > Int (*set_msr)(struct kvm_vcpu * vcpu, u32 mrs_index, u64 data); > to > Int (*set_msr)(struct kvm_vcpu * vcpu, u32 mrs_index, u64 data, bool= from_guest); > ... > }; >=20 > and so on down the line to set_msr_common(), kvm_write_tsc()... in a = separate patch before other related patches? Yes. 'bool guest_initiated' is nicer IMO. > As far as the initialization after live migration, I will provide som= e output with explanation once I am able to again. At the moment, I hav= e hosed my system and need to figure out what's wrong and fix it first.= =20 Ok no problem. > Thanks, >=20 > Will >=20 > -----Original Message----- > From: Marcelo Tosatti [mailto:mtosatti@redhat.com]=20 > Sent: Wednesday, October 10, 2012 5:53 AM > To: Auld, Will > Cc: Avi Kivity; kvm@vger.kernel.org; Zhang, Xiantao; Liu, Jinsong > Subject: Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM >=20 > On Tue, Oct 09, 2012 at 04:10:30PM +0000, Auld, Will wrote: > > I am just testing the second version of this patch. It addresses al= l the comments so far except Marcelo's issue with breaking the function= compute_guest_tsc().=20 >=20 > Lets try to merge the missing patch from Zachary first (that'll make = it clear). >=20 > >=20 > > I needed to put the call for updating the TSC_ADJUST_MSR in kvm_wri= te_tsc() to ensure it is only called from user space. Other changes add= ed to vmcs offset should not be tracked in TSC_ADJUST_MSR.=20 >=20 > Please have a separate, earlier patch making that explicit (by passin= g a bool to kvm_x86_ops->set_msr then to kvm_set_msr_common). "that" =3D= whether msr write is guest initiated or not. >=20 > > I had some trouble with the order of initialization during live mig= ration. TSC_ADJUST is initialized first but then wiped out by multiple = initializations of tsc. The fix for this is to not update TSC_ADJUST if= the vmcs offset is not actually changing with the tsc write. So, after= migration outcome is that vmcs offset gets defined independent from th= e migrating value of TSC_ADJUST. I believe this is what we want to happ= en. >=20 > Can you please be more explicit regarding "wiped out by multiple init= ializations of tsc" ?=20 >=20 > It is probably best to maintain TSC_ADJUST separately, in software, a= nd then calculate TSC_OFFSET. >=20 > > Thanks, > >=20 > > Will > >=20 > > -----Original Message----- > > From: Avi Kivity [mailto:avi@redhat.com] > > Sent: Tuesday, October 09, 2012 5:12 AM > > To: Marcelo Tosatti > > Cc: Auld, Will; kvm@vger.kernel.org; Zhang, Xiantao > > Subject: Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM > >=20 > > On 10/08/2012 07:30 PM, Marcelo Tosatti wrote: > > >=20 > > > From Intel's manual: > > >=20 > > > =E2=80=A2 If an execution of WRMSR to the IA32_TIME_STAMP_COUNTER= MSR adds=20 > > > (or > > > subtracts) value X from the TSC, > > > the logical processor also adds (or subtracts) value X from the=20 > > > IA32_TSC_ADJUST MSR. > > >=20 > > > This is not handled in the patch.=20 > > >=20 > > > To support migration, it will be necessary to differentiate betwe= en=20 > > > guest initiated and userspace-model initiated msr write. That is,= =20 > > > only guest initiated TSC writes should affect the value of=20 > > > IA32_TSC_ADJUST MSR. > > >=20 > > > Avi, any better idea? > > >=20 > >=20 > > I think we need that anyway, since there are some read-only MSRs th= at need to be configured by the host (nvmx capabilities). So if we add= that feature it will be useful elsewhere. I don't think it's possible= to do it in any other way: > >=20 > > "Local offset value of the IA32_TSC for a logical processor. Reset = value is Zero. A write to IA32_TSC will modify the local offset in IA32= _TSC_ADJUST and the content of IA32_TSC, but does not affect the intern= al invariant TSC hardware." > >=20 > > What we want to do is affect the internal invariant TSC hardware, s= o we can't do that through the normal means. > >=20 > > btw, will tsc writes from userspace (after live migration) cause ts= c skew? If so we should think how to model a guest-wide tsc. > >=20 > > -- > > error compiling committee.c: too many arguments to function=20 > > N?????r??y????b?X??=C7=A7v?^?)=DE=BA{.n?+????h??=17??=DC=A8}???=C6=A0= z?&j:+v??? ????zZ+??+zf???h???~????i???z?=1E?w?????????&?)=DF=A2=1Bf