Re: [PATCH v8 05/17] KVM: x86: Avoid NTP frequency skew for KVM clock on 32-bit host

On Wed, Aug 05, 2026, [email protected] wrote:
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 3107878a6fe5d..8f469fd9863f9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> 
> [ ... ]
> 
> > @@ -915,13 +914,6 @@ static s64 get_kvmclock_base_ns(void)
> >  	/* Count up from boot time, but with the frequency of the raw clock.  */
> >  	return ktime_to_ns(ktime_add(ktime_get_raw(), pvclock_gtod_data.offs_boot));
> 
> [Severity: Low]
> Will reading the 64-bit pvclock_gtod_data.offs_boot without seqcount
> protection or data_race() annotations trigger KCSAN data race warnings on
> 32-bit systems?
> 
> By removing the ktime_get_boottime_ns() fallback, this read now executes on
> 32-bit architectures where it compiles to two non-atomic 32-bit accesses.
> If a KVM vCPU thread calls get_kvmclock_base_ns() while a timer interrupt
> runs timekeeping_update(), it overwrites offs_boot.
> 
> Even though the value only actually changes during suspend when the freezer
> subsystem guarantees vCPU threads are frozen (preventing functional tearing),
> overwriting the identical value concurrently with an unprotected read still
> introduces a formal C11 data race.

Huh.  And strictly speaking, 64-bit could tear the store/load.  Stealing heavily
from ktime_mono_to_any(), this as a prep patch plus fixup (not yet tested)?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b6e1dfd6db6a..57679d871581 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -921,7 +921,8 @@ static void update_pvclock_gtod(struct timekeeper *tk)
 
        vdata->wall_time_sec            = tk->xtime_sec;
 
-       vdata->offs_boot                = tk->offs_boot;
+       /* Pairs with the READ_ONCE() in get_kvmclock_base_ns(). */
+       WRITE_ONCE(vdata->offs_boot, tk->offs_boot);
 
        write_seqcount_end(&vdata->seq);
 }
@@ -929,7 +930,26 @@ static void update_pvclock_gtod(struct timekeeper *tk)
 static s64 get_kvmclock_base_ns(void)
 {
        /* Count up from boot time, but with the frequency of the raw clock.  */
-       return ktime_to_ns(ktime_add(ktime_get_raw(), pvclock_gtod_data.offs_boot));
+       struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
+       ktime_t raw = ktime_get_raw();
+       ktime_t now;
+
+       /*
+        * Synchronization with clock updates isn't required on 64-bit as only
+        * one field is being consume
+        * */
+#ifdef CONFIG_X86_64
+       now = ktime_add(raw, READ_ONCE(gtod->offs_boot));
+#else
+       unsigned int seq;
+
+       do {
+               seq = read_seqcount_begin(&gtod->seq);
+               now = ktime_add(raw, *offset);
+       } while (read_seqcount_retry(gtod->seq, seq));
+#endif
+
+       return ktime_to_ns(now);
 }
 
 static uint32_t div_frac(uint32_t dividend, uint32_t divisor)