On 21/07/2017 17:45, Denis Plotnikov wrote:
> +static bool kvm_get_host_time_and_cycles(s64 *kernel_ns, u64 *cycle_now,
> + u64 (*get_time)(u64 *cycle_now))
> {
> - u64 ret = (u64)rdtsc_ordered();
> - u64 last = pvclock_gtod_data.clock.cycle_last;
> + unsigned int seq;
> + const seqcount_t *s = get_tk_seq();
> + bool stable;
>
> - if (likely(ret >= last))
> - return ret;
> + {
> + seq = read_seqcount_begin(s);
> + stable = clocksource_stable();
> + if (stable)
> + *kernel_ns = get_time(cycle_now);
> + } while (unlikely(read_seqcount_retry(s, seq)));
>
The clocksource change notifier seems fine to me, but this
is a layering violation. All this needs to happen in
kernel/time/timekeeping.c.
Here is a possible plan:
1) define an alternative reading function such as
int (*read_with_system_time)(ktime_t *device_time,
u64 *tstamp)
which calls into a new timekeeper function pointer.
ktime_get_with_system_counter can return -EOPNOTSUPP if the
new function pointer is not supported, and the function pointer
can also return -EOPNOTSUPP if the clock is not stable.
2) use this function from ktime_get_snapshot. This way that
function returns an actual TSC timestamp in systime_snapshot->cycles
instead of the raw value of the clocksource(*).
3) if needed, add a 'boot' field to struct system_time_snapshot.
4) use ktime_get_snapshot from KVM. Admittedly there is some
handwaving here.
(*) There is no user of systime_snapshot->cycles, or actually
the only one (get_system_device_crosststamp) is in dead code
because get_system_device_crosststamp is always called with
history_begin == NULL. But that only user seems to assume
that systime_snapshot->cycles has the same unit of measure
as system_counterval.cycles. So this counts even as a bugfix.
Paolo
