Re: [PATCH kvm-unit-tests v2] KVM: x86: add hyperv clock test case

[Marcelo Tosatti <mtosatti@redhat.com>]

On Wed, May 25, 2016 at 09:30:02PM +0300, Roman Kagan wrote:
> On Mon, May 23, 2016 at 08:44:03PM -0300, Marcelo Tosatti wrote:
> > On Mon, May 23, 2016 at 06:55:06PM -0300, Marcelo Tosatti wrote:
> > > On Thu, Apr 21, 2016 at 08:21:25PM +0300, Roman Kagan wrote:
> > > > +	if (t >= end) {
> > > > +		long long ref = rdmsr(HV_X64_MSR_TIME_REF_COUNT);
> > > > +		if (i == 0)
> > > > +			printf("Time reference MSR drift: %lld\n\n", ref - end);
> > > > +		ok[i] &= (ref - end) > -5 && (ref - end) < 5;
> > > 
> > > This is prone to fail: guest can be scheduled at "***" above and 
> > > test will fail.
> > 
> > I suppose this would be a better test
> > 
> >         do {
> >                 u64 now_refpage; tsc_ref_read();
> >                 u64 now_refcount;
> > 
> >                 now_refpage = tsc_ref_read();
> >                 now_refcount = rdmsr(HV_X64_MSR_TIME_REF_COUNT);
> > 
> >                 if (now_refpage > now_refcount) {
> >                         printf("now_refpage %ld > now_refcount %ld\n", 
> >                                 now_refpage, now_refcount);
> 
> Yes that's similar to what I posted in my other message on this thread.
> I'd also test that now_refpage >= pref_refcount, too.
> 
> I'm cooking a patch with these changes, will post soonish.
> 
> > which fails once you run 
> > 
> > void main(void)
> > {
> >         int ret;
> >         struct timex tx;
> >         char *ptr;
> > 
> >         memset((void*)&tx, 0, sizeof(tx));
> > 
> >         tx.freq = -6553600;
> >         //tx.freq = -237507;
> >         tx.modes = ADJ_FREQUENCY;
> >         ret = adjtimex(&tx);
> 
> Right, which is a problem with kvm-clock too (as I wrote in another
> thread, pvclock_gtod_data updates don't currently trigger per-VM
> masterclock updates).  I'm still struggling through the multiple lengthy
> discussions trying to figure out if it's a bug or a feature...
> 
> Roman.

Its a bug, i am writing an improvement based on Paolo's change to
update the multiplier. 

But the hyperv tsc reference page patches should not depend on it.