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

[David Woodhouse <dwmw2@infradead.org>]

[-- 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 --]