Re: [PATCH] Enabling IA32_TSC_ADJUST for guest VM

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Oct 11, 2012 at 12:47:39AM +0000, Auld, Will wrote:
> Marcelo,
> 
> You are suggesting that I:
> Kvm_host.h:
> 
> Struct kvm_x86_ops {
> 
> ...
> 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);
> ...
> };
> 
> 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 some output with explanation once I am able to again. At the moment, I have hosed my system and need to figure out what's wrong and fix it first. 

Ok no problem.

> Thanks,
> 
> Will
> 
> -----Original Message-----
> From: Marcelo Tosatti [mailto:mtosatti@redhat.com] 
> 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
> 
> 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??? ????zZ+??+zf???h???~????i???z?\x1e?w?????????&?)ߢ^[f