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

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

On Wednesday 17 March 2010 16:51:05 Sheng Yang wrote:
> 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.

Seem not get enough update...

OK, a new flag, adjustment in Xen. Right?

-- 
regards
Yang, Sheng

> 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).
>