RE: [PATCH] Enabling IA32_TSC_ADJUST for guest VM

["Auld, Will" <will.auld@intel.com>]

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?

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. 

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