Re: [PATCH 0 of 5] PV on HVM Xen

[Sheng Yang <sheng@linux.intel.com>]

On Wednesday 17 March 2010 02:37:51 Stefano Stabellini wrote:
> On Tue, 16 Mar 2010, Jeremy Fitzhardinge wrote:
> > On 03/16/2010 11:06 AM, Stefano Stabellini wrote:
> > > the following is the interesting bit of the pvclock interface:
> > >
> > > static __always_inline
> > > u64 pvclock_get_nsec_offset(const struct pvclock_vcpu_time_info *src)
> > > {
> > > 	u64 delta = __native_read_tsc() - src->tsc_timestamp;
> > > 	return scale_delta(delta, src->tsc_to_system_mul, src->tsc_shift);
> > > }
> > >
> > >
> > > xen refreshes the values in pvclock_vcpu_time_info every EPOCH (1s),
> > > therefore if the value returned by pvclock_get_nsec_offset is greater
> > > than EPOCH than the patch is not present in xen.
> > >
> > > This is a simple way of detecting if the offset is taken into account
> > > or not and it works because the offset is the value returned by rdtsc
> > > in the host when the VM was created and we can be sure it corresponds
> > > to more than 1 sec.
> >
> > That seems pretty fragile.  I don't think EPOCH is part of the ABI, and
> > I don't think we should be relying on it.
> 
> EPOCH is certainly not part of the ABI :)
> That said the difference between a correct delta and a wrong delta is so
> big that we cannot really be mistaken.
> 
> In any case I wouldn't mind a feature flag just to stay on the safe
> side, so I'll leave this decision to you and Keir (that this week is on
> vacation).

I am quite happy with a feature flag. Depends on the return of hypercall to 
determine if the feature is there seems tricky to me...

Sure, I am fine with a set/remove tsc offset. And a side effort would be we 
need more code to do it explicitly for SMP guest's vcpu in the guest kernel 
code... But it's fine if they are elegant enough(I think setup_percpu_clockev 
should be fit for it).

-- 
regards
Yang, Sheng