Re: [RFC PATCH v3 0/7] Add virtio_rtc module and related changes

David Woodhouse
2024-06-21T14:02:08Z

[-- Attachment #1: Type: text/plain, Size: 3750 bytes --]

On Thu, 2024-06-20 at 14:37 +0200, Peter Hilber wrote:
> Should implement .gettimex64 instead.

Thanks. This look sane?

As noted in the code comment, in the *ideal* case we just build all
three pre/post/device timestamps from the very same counter read. So
sts->pre_ts == sts->post_ts.

In the less ideal case (which will happen on x86 when kvmclock is being
used for the system time), we use the time from ktime_get_snapshot() as
the pre_ts and take a new snapshot immediately after the get_cycles().

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index e8c65405a8f3..07a81a94d29a 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -96,9 +96,11 @@ static inline uint64_t mul_u64_u64_add_u64(uint64_t *res_hi, uint64_t delta,
}

static int vmclock_get_crosststamp(struct vmclock_state *st,
+ struct ptp_system_timestamp *sts,
struct system_counterval_t *system_counter,
struct timespec64 *tspec)
{
+ struct system_time_snapshot systime_snapshot;
uint64_t cycle, delta, seq, frac_sec;
int ret = 0;

@@ -119,7 +121,17 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
continue;
}

- cycle = get_cycles();
+ if (sts) {
+ ktime_get_snapshot(&systime_snapshot);
+
+ if (systime_snapshot.cs_id == st->cs_id) {
+ cycle = systime_snapshot.cycles;
+ } else {
+ cycle = get_cycles();
+ ptp_read_system_postts(sts);
+ }
+ } else
+ cycle = get_cycles();

delta = cycle - st->clk->counter_value;

@@ -139,6 +151,21 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
if (ret)
return ret;

+ /*
+ * When invoked for gettimex64, fill in the pre/post system times.
+ * The ideal case is when system time is based on the the same
+ * counter as st->cs_id, in which case all three pre/post/device
+ * times are derived from the *same* counter value. If cs_id does
+ * not match, then the value from ktime_get_snapshot() is used as
+ * pre_ts, and ptp_read_system_postts() was already called above
+ * for the post_ts. Those are either side of the get_cycles() call.
+ */
+ if (sts) {
+ sts->pre_ts = ktime_to_timespec64(systime_snapshot.real);
+ if (systime_snapshot.cs_id == st->cs_id)
+ sts->post_ts = sts->pre_ts;
+ }
+
if (system_counter) {
system_counter->cycles = cycle;
system_counter->cs_id = st->cs_id;
@@ -155,7 +182,7 @@ static int ptp_vmclock_get_time_fn(ktime_t *device_time,
struct timespec64 tspec;
int ret;

- ret = vmclock_get_crosststamp(st, system_counter, &tspec);
+ ret = vmclock_get_crosststamp(st, NULL, system_counter, &tspec);
if (!ret)
*device_time = timespec64_to_ktime(tspec);

@@ -198,7 +225,16 @@ static int ptp_vmclock_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts
struct vmclock_state *st = container_of(ptp, struct vmclock_state,
ptp_clock_info);

- return vmclock_get_crosststamp(st, NULL, ts);
+ return vmclock_get_crosststamp(st, NULL, NULL, ts);
+}
+
+static int ptp_vmclock_gettimex(struct ptp_clock_info *ptp, struct timespec64 *ts,
+ struct ptp_system_timestamp *sts)
+{
+ struct vmclock_state *st = container_of(ptp, struct vmclock_state,
+ ptp_clock_info);
+
+ return vmclock_get_crosststamp(st, sts, NULL, ts);
}

static int ptp_vmclock_enable(struct ptp_clock_info *ptp,
@@ -216,6 +252,7 @@ static const struct ptp_clock_info ptp_vmclock_info = {
.adjfine = ptp_vmclock_adjfine,
.adjtime = ptp_vmclock_adjtime,
.gettime64 = ptp_vmclock_gettime,
+ .gettimex64 = ptp_vmclock_gettimex,
.settime64 = ptp_vmclock_settime,
.enable = ptp_vmclock_enable,
.getcrosststamp = ptp_vmclock_getcrosststamp,

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]