Messages in this thread | | | Date | Wed, 27 Sep 2017 15:45:19 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH v5 1/6] timekeeper: introduce extended clocksource reading callback |
| |
On Wed, 27 Sep 2017, Paolo Bonzini wrote: > On 27/09/2017 13:53, Thomas Gleixner wrote: > >> I think the hook should be specific to x86. For example it could be an > >> array of function pointers, indexed by vclock_mode, with the same > >> semantics as read_with_stamp. > > I don't think you need that. > > > > The get_time_fn() which is handed in to get_device_system_crossstamp() can > > convey that information: > > > > /* > > * Try to synchronously capture device time and a system > > * counter value calling back into the device driver > > */ > > ret = get_time_fn(&xtstamp->device, &system_counterval, ctx); > > if (ret) > > return ret; > > > > So in your case get_time_fn() would be kvmclock or hyperv clock specific > > and the actual hypercall implementation can return a failure code if the > > requirements are not met: > > > > 1) host clock source is TSC > > 2) capturing of host time and TSC is atomic > > So you are suggesting reusing the cross-timestamp hypercall to implement > nested pvclock. There are advantages and disadvantages to that. > > With read_with_stamp-like callbacks: > > + running on old KVM or on Hyper-V is supported > - pvclock_gtod_copy does not go away > > With hypercall-based callbacks on the contrary: > > + KVM can use ktime_get_snapshot for the bare metal case > - only very new KVM is supported
I don't think that it's an either or decision.
get_device_system_crossstamp(get_time_fn, ......)
So you can have specific get_time_fn() implementations for your situation:
old_kvm_fn() retrieve data from pvclock_gtod copy
new_kvm_fn() use hypercall
hyperv_fn() do what must be done
All implementations need a way to tell you:
1) Host time 2) Host TSC timestamp which corresponds to #1 3) Validity
For old_kvm_fn() pvclock_gtod_data.clock.vclock_mode == VCLOCK_TSC For new_kvm_fn() hypercall result For hyperv_fn() whatever it takes
Thanks,
tglx
| |