Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM

From: Marcelo Tosatti <mtosatti@redhat.com>
To: "Auld, Will" <will.auld@intel.com>
Cc: Avi Kivity <avi@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"Zhang, Xiantao" <xiantao.zhang@intel.com>,
	"Liu, Jinsong" <jinsong.liu@intel.com>
Subject: Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM
Date: Wed, 10 Oct 2012 09:52:42 -0300	[thread overview]
Message-ID: <20121010125242.GA15137@amt.cnet> (raw)
In-Reply-To: <96EC5A4F3149B74492D2D9B9B1602C2728B708F3@ORSMSX108.amr.corp.intel.com>

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 all the comments so far except Marcelo's issue with breaking the function compute_guest_tsc(). 

Lets try to merge the missing patch from Zachary first (that'll make it
clear).

> 
> I needed to put the call for updating the TSC_ADJUST_MSR in kvm_write_tsc() to ensure it is only called from user space. Other changes added to vmcs offset should not be tracked in TSC_ADJUST_MSR. 

Please have a separate, earlier patch making that explicit (by passing a
bool to kvm_x86_ops->set_msr then to kvm_set_msr_common). "that" =
whether msr write is guest initiated or not.

> I had some trouble with the order of initialization during live migration. 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 the migrating value of TSC_ADJUST. I believe this is what we want to happen.

Can you please be more explicit regarding "wiped out by multiple
initializations of tsc" ? 

It is probably best to maintain TSC_ADJUST separately, in software, and
then calculate TSC_OFFSET.

> Thanks,
> 
> Will 
> 
> -----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
> 
> On 10/08/2012 07:30 PM, Marcelo Tosatti wrote:
> > 
> > From Intel's manual:
> > 
> > • If an execution of WRMSR to the IA32_TIME_STAMP_COUNTER MSR adds (or
> > subtracts) value X from the TSC,
> > the logical processor also adds (or subtracts) value X from the 
> > IA32_TSC_ADJUST MSR.
> > 
> > This is not handled in the patch. 
> > 
> > To support migration, it will be necessary to differentiate between 
> > guest initiated and userspace-model initiated msr write. That is, only 
> > guest initiated TSC writes should affect the value of IA32_TSC_ADJUST 
> > MSR.
> > 
> > Avi, any better idea?
> > 
> 
> I think we need that anyway, since there are some read-only MSRs that 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:
> 
> "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 internal invariant TSC hardware."
> 
> What we want to do is affect the internal invariant TSC hardware, so we can't do that through the normal means.
> 
> btw, will tsc writes from userspace (after live migration) cause tsc skew?  If so we should think how to model a guest-wide tsc.
> 
> --
> error compiling committee.c: too many arguments to function
> N?????r??y????b?X??ǧv?^?)޺{.n?+????h??\x17??ܨ}???Ơz?&j:+v???\a????zZ+??+zf???h???~????i???z?\x1e?w?????????&?)ߢ^[f