[PATCH v2 0/3] timekeeping: Implement and use read_snapshot() functionality

[PATCH v2 0/3] timekeeping: Implement and use read_snapshot() functionality

[David Woodhouse]

Commit ca1ec8bfac8c ("timekeeping: Add clocksource read_snapshot() 
method and hw_cycles to snapshot") provided a way for clocksources like
kvmclock and Hyper-V to expose the underlying hardware counter value
from which their clock reading is calculated.

This is useful for consumers like the KVM masterclock (when hosting
nested guests), and vmclock which needs the hardware counter value to
calculate time according to the information it's given.

Now that Thomas has beaten my initial proof of concept¹ into shape and
merged it (thanks) here are the two providers and the trivial consumer
case in the vmclock driver, reworked accordingly.

I will rework the other four KVM master clock patches into my kvmclock 
series² which I'll rebase on top of tip/timers/ptp in due course.

¹ https://lore.kernel.org/all/20260526230635.136914-1-dwmw2@infradead.org/#r
² https://lore.kernel.org/all/20260509224824.3264567-1-dwmw2@infradead.org/

David Woodhouse (3):
      clocksource/hyperv: Implement read_snapshot() for TSC page clocksource
      x86/kvmclock: Implement read_snapshot() for kvmclock clocksource
      ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing

 arch/x86/kernel/kvmclock.c         | 36 +++++++++++++++++++++++++++++-------
 drivers/clocksource/hyperv_timer.c | 37 +++++++++++++++++++++++++++----------
 drivers/ptp/ptp_vmclock.c          |  4 ++++
 3 files changed, 60 insertions(+), 17 deletions(-)

base-commit: ca1ec8bfac8c95d0fed9e3611ea21400d1f37262

[PATCH v2 1/3] clocksource/hyperv: Implement read_snapshot() for TSC page clocksource

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

Implement the read_snapshot() callback for the Hyper-V TSC page
clocksource. This returns the derived 10MHz reference time (for
timekeeping) while also providing the raw TSC value that was used
to compute it.

When the TSC page is valid, hv_read_tsc_page_tsc() atomically
captures both values from a single RDTSC inside the sequence-counter
protected read. When the TSC page is invalid (sequence == 0), the
hw_csid and hw_cycles are set to zero indicating no value is available.

This enables ktime_get_snapshot_id() to provide the raw TSC to consumers
like KVM's master clock when running nested guests under Hyper-V.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/clocksource/hyperv_timer.c | 37 ++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
index e9f5034a1bc8..df567795d175 100644
--- a/drivers/clocksource/hyperv_timer.c
+++ b/drivers/clocksource/hyperv_timer.c
@@ -444,6 +444,22 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
 	return read_hv_clock_tsc();
 }
 
+static u64 notrace read_hv_clock_tsc_cs_snapshot(struct clocksource *arg,
+						  struct clocksource_hw_snapshot *chs)
+{
+	u64 time;
+
+	if (hv_read_tsc_page_tsc(tsc_page, &chs->hw_cycles, &time)) {
+		chs->hw_csid = CSID_X86_TSC;
+	} else {
+		chs->hw_cycles = 0;
+		chs->hw_csid = CSID_GENERIC;
+		time = read_hv_clock_msr();
+	}
+
+	return time;
+}
+
 static u64 noinstr read_hv_sched_clock_tsc(void)
 {
 	return (read_hv_clock_tsc() - hv_sched_clock_offset) *
@@ -492,18 +508,19 @@ static int hv_cs_enable(struct clocksource *cs)
 #endif
 
 static struct clocksource hyperv_cs_tsc = {
-	.name	= "hyperv_clocksource_tsc_page",
-	.rating	= 500,
-	.read	= read_hv_clock_tsc_cs,
-	.mask	= CLOCKSOURCE_MASK(64),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
-	.suspend= suspend_hv_clock_tsc,
-	.resume	= resume_hv_clock_tsc,
+	.name			= "hyperv_clocksource_tsc_page",
+	.rating			= 500,
+	.read			= read_hv_clock_tsc_cs,
+	.read_snapshot		= read_hv_clock_tsc_cs_snapshot,
+	.mask			= CLOCKSOURCE_MASK(64),
+	.flags			= CLOCK_SOURCE_IS_CONTINUOUS,
+	.suspend		= suspend_hv_clock_tsc,
+	.resume			= resume_hv_clock_tsc,
 #ifdef HAVE_VDSO_CLOCKMODE_HVCLOCK
-	.enable = hv_cs_enable,
-	.vdso_clock_mode = VDSO_CLOCKMODE_HVCLOCK,
+	.enable			= hv_cs_enable,
+	.vdso_clock_mode	= VDSO_CLOCKMODE_HVCLOCK,
 #else
-	.vdso_clock_mode = VDSO_CLOCKMODE_NONE,
+	.vdso_clock_mode	= VDSO_CLOCKMODE_NONE,
 #endif
 };
 
-- 
2.54.0

Re: [PATCH v2 1/3] clocksource/hyperv: Implement read_snapshot() for TSC page clocksource

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but if `memremap` fails during Hyper-V root partition initialization, the active clocksource is not unregistered and `tsc_page` becomes NULL, leading to a guaranteed kernel panic upon the next clock read.
--

commit fd7e137ffdb89db4afcdcc509ba544a534823492
Author: David Woodhouse <dwmw@amazon.co.uk>

clocksource/hyperv: Implement read_snapshot() for TSC page clocksource

This commit implements the read_snapshot() callback for the Hyper-V TSC
page clocksource. It allows the raw TSC value and the derived time to be
atomically captured, enabling ktime_get_snapshot_id() to provide the raw
TSC to consumers like KVM's master clock.

> diff --git a/drivers/clocksource/hyperv_timer.c b/drivers/clocksource/hyperv_timer.c
> index e9f5034a1bc89..df567795d1751 100644
> --- a/drivers/clocksource/hyperv_timer.c
> +++ b/drivers/clocksource/hyperv_timer.c

[ ... ]

> @@ -444,6 +444,22 @@ static u64 notrace read_hv_clock_tsc_cs(struct clocksource *arg)
>  	return read_hv_clock_tsc();
>  }
>  
> +static u64 notrace read_hv_clock_tsc_cs_snapshot(struct clocksource *arg,
> +						  struct clocksource_hw_snapshot *chs)
> +{
> +	u64 time;
> +
> +	if (hv_read_tsc_page_tsc(tsc_page, &chs->hw_cycles, &time)) {

[Severity: High]
This isn't a bug introduced by this patch, but if memremap() fails during
Hyper-V root partition initialization, tsc_page becomes NULL:

drivers/clocksource/hyperv_timer.c:hv_remap_tsc_clocksource() {
	...
	tsc_page = memremap(tsc_pfn << HV_HYP_PAGE_SHIFT, sizeof(tsc_pg),
			    MEMREMAP_WB);
	if (!tsc_page)
		pr_err("Failed to remap Hyper-V TSC page.\n");
	...
}

Since the clocksource is not unregistered and a valid fallback pointer isn't
restored on failure, will passing this NULL tsc_page to hv_read_tsc_page_tsc()
cause a kernel panic upon the next snapshot read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604095755.64849-1-dwmw2@infradead.org?part=1

[PATCH v2 2/3] x86/kvmclock: Implement read_snapshot() for kvmclock clocksource

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

Implement the read_snapshot() callback for the kvmclock clocksource.
This returns the kvmclock nanosecond value (for timekeeping) while
also providing the raw TSC value that was used to compute it.

The TSC is read inside the pvclock seqlock-protected region, ensuring
the raw TSC and derived kvmclock value are atomically paired.

This enables ktime_get_snapshot_id() to provide the raw TSC to consumers
like the vmclock PTP driver, which currently has to do a separate call
to get_cycles() to obtain a value at *approximately* the same time, to
feed through the vmclock calculation.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
---
 arch/x86/kernel/kvmclock.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index b5991d53fc0e..cb3d0ca1fa22 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -87,6 +87,27 @@ static u64 kvm_clock_get_cycles(struct clocksource *cs)
 	return kvm_clock_read();
 }
 
+static u64 kvm_clock_get_cycles_snapshot(struct clocksource *cs,
+					 struct clocksource_hw_snapshot *chs)
+{
+	struct pvclock_vcpu_time_info *src;
+	unsigned version;
+	u64 ret, tsc;
+
+	preempt_disable_notrace();
+	src = this_cpu_pvti();
+	do {
+		version = pvclock_read_begin(src);
+		tsc = rdtsc_ordered();
+		ret = __pvclock_read_cycles(src, tsc);
+	} while (pvclock_read_retry(src, version));
+	preempt_enable_notrace();
+
+	chs->hw_cycles = tsc;
+	chs->hw_csid = CSID_X86_TSC;
+	return ret;
+}
+
 static noinstr u64 kvm_sched_clock_read(void)
 {
 	return pvclock_clocksource_read_nowd(this_cpu_pvti()) - kvm_sched_clock_offset;
@@ -156,13 +177,14 @@ static int kvm_cs_enable(struct clocksource *cs)
 }
 
 static struct clocksource kvm_clock = {
-	.name	= "kvm-clock",
-	.read	= kvm_clock_get_cycles,
-	.rating	= 400,
-	.mask	= CLOCKSOURCE_MASK(64),
-	.flags	= CLOCK_SOURCE_IS_CONTINUOUS,
-	.id     = CSID_X86_KVM_CLK,
-	.enable	= kvm_cs_enable,
+	.name		= "kvm-clock",
+	.read		= kvm_clock_get_cycles,
+	.read_snapshot	= kvm_clock_get_cycles_snapshot,
+	.rating		= 400,
+	.mask		= CLOCKSOURCE_MASK(64),
+	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.id		= CSID_X86_KVM_CLK,
+	.enable		= kvm_cs_enable,
 };
 
 static void kvm_register_clock(char *txt)
-- 
2.54.0

Re: [PATCH v2 2/3] x86/kvmclock: Implement read_snapshot() for kvmclock clocksource

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm_clock_get_cycles_snapshot() unconditionally returns raw TSC cycles and bypasses the global last_value monotonicity clamp, breaking timekeeping on hosts with unstable TSCs.
--

commit 6dc8b64e567331ab0edc9710a62a9a13cd0b8811
Author: David Woodhouse <dwmw@amazon.co.uk>

x86/kvmclock: Implement read_snapshot() for kvmclock clocksource

This commit implements the read_snapshot() callback for the kvmclock
clocksource. This allows returning both the kvmclock nanosecond value
and the raw TSC value used to compute it.

> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index b5991d53fc0ea..cb3d0ca1fa225 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c

[ ... ]

> @@ -87,6 +87,27 @@ static u64 kvm_clock_get_cycles(struct clocksource *cs)
>  	return kvm_clock_read();
>  }
>  
> +static u64 kvm_clock_get_cycles_snapshot(struct clocksource *cs,
> +					 struct clocksource_hw_snapshot *chs)
> +{
> +	struct pvclock_vcpu_time_info *src;
> +	unsigned version;
> +	u64 ret, tsc;
> +
> +	preempt_disable_notrace();
> +	src = this_cpu_pvti();
> +	do {
> +		version = pvclock_read_begin(src);
> +		tsc = rdtsc_ordered();
> +		ret = __pvclock_read_cycles(src, tsc);
> +	} while (pvclock_read_retry(src, version));
> +	preempt_enable_notrace();
> +
> +	chs->hw_cycles = tsc;
> +	chs->hw_csid = CSID_X86_TSC;

[Severity: High]
Does this open-coded read loop bypass the fallback monotonic clamping used in
the standard kvm-clock read path?

The standard path (__pvclock_clocksource_read) enforces clock monotonicity
by clamping to a globally shared last_value accumulator when
PVCLOCK_TSC_STABLE_BIT is not set.

Without this check and clamping logic, if a vCPU migrates to a host with an
unsynchronized or slower TSC, could the snapshot return a derived time that
goes backwards relative to the timekeeper's cycle_last?

Also, by unconditionally returning CSID_X86_TSC and the raw hardware cycles
even when the host marks the TSC as unstable, does this incorrectly trick
cross-timestamping consumers into assuming they have a reliable, synchronized
hardware timestamp?

> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604095755.64849-1-dwmw2@infradead.org?part=2

[PATCH v2 3/3] ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing

[David Woodhouse]

From: David Woodhouse <dwmw@amazon.co.uk>

When the system clocksource is kvmclock or Hyper-V (not the TSC
directly), vmclock_get_crosststamp() previously fell through to a
separate get_cycles() call, losing the atomic pairing between the
system time snapshot and the TSC reading.

Now that ktime_get_snapshot_id() populates hw_cycles with the
underlying TSC value for derived clocksources, use it when available.
This gives a perfect (system_time, tsc) pairing for the device time
calculation.

The SUPPORT_KVMCLOCK wrapper is still needed to convert the TSC into
kvmclock nanoseconds for system_counter->cycles, because otherwise
get_device_system_crosststamp() can't interpret the result against
the system clock.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Kiro:claude-opus-4.6-1m
---
 drivers/ptp/ptp_vmclock.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index d6a5a533164a..eebdcd5ebc08 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -140,6 +140,10 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
 			if (sts->pre_sts.cs_id == st->cs_id) {
 				cycle = sts->pre_sts.cycles;
 				sts->post_sts = sts->pre_sts;
+			} else if (sts->pre_sts.hw_csid == st->cs_id &&
+				   sts->pre_sts.hw_cycles) {
+				cycle = sts->pre_sts.hw_cycles;
+				sts->post_sts = sts->pre_sts;
 			} else {
 				cycle = get_cycles();
 				ptp_read_system_postts(sts);
-- 
2.54.0

Re: [PATCH v2 3/3] ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized stack memory read / Undefined Behavior in `vmclock_get_crosststamp()`
--

commit 9639c73bd7986c7a2cf17a27c0bf79db7385ce83
Author: David Woodhouse <dwmw@amazon.co.uk>

ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing

When the system clocksource is kvmclock or Hyper-V, vmclock_get_crosststamp()
previously fell through to get_cycles(). This patch uses hw_cycles from
ktime_get_snapshot_id() when available, giving a perfect pairing for the
device time calculation.

> diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
> index d6a5a533164aa..eebdcd5ebc080 100644
> --- a/drivers/ptp/ptp_vmclock.c
> +++ b/drivers/ptp/ptp_vmclock.c
[ ... ]
> @@ -140,6 +140,10 @@ static int vmclock_get_crosststamp(struct vmclock_state *st,
>  			if (sts->pre_sts.cs_id == st->cs_id) {
>  				cycle = sts->pre_sts.cycles;
>  				sts->post_sts = sts->pre_sts;
> +			} else if (sts->pre_sts.hw_csid == st->cs_id &&
> +				   sts->pre_sts.hw_cycles) {

[Severity: High]
This is a pre-existing issue, but by adding these checks, does this code
evaluate uninitialized stack memory?

Looking at ptp_sys_offset_extended() in drivers/ptp/ptp_chardev.c, the
struct ptp_system_timestamp sts is allocated on the stack but not
zero-initialized.

When ptp_read_system_prets(&sts) calls ktime_get_snapshot_id(), it sets
systime_snapshot->valid = false and returns early in failure cases (e.g.,
if an invalid or unavailable AUX clock ID is passed).

In these cases, the cs_id, hw_csid, and hw_cycles fields remain uninitialized
stack garbage.

vmclock_get_crosststamp() evaluates these branch conditions without first
checking if sts->pre_sts.valid is true.

Although the sample is eventually discarded by ptp_sys_offset_extended()
because sts.pre_sts.valid is false, branching on uninitialized stack memory
is undefined behavior and can trigger KMSAN warnings.

Could a check for sts->pre_sts.valid be added before evaluating these fields?

> +				cycle = sts->pre_sts.hw_cycles;
> +				sts->post_sts = sts->pre_sts;
>  			} else {
>  				cycle = get_cycles();
>  				ptp_read_system_postts(sts);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604095755.64849-1-dwmw2@infradead.org?part=3