Align the guest TSC with the partition reference clock on KVM

[Robert Nowotny (bitranox)]

This replaces #4253, which I closed after review pointed out, correctly, that it froze the reference counter instead of moving the counter that had failed to move. This one fixes the half that was actually wrong, and the same root cause turns out to affect partition creation as well as reset.

The defect

A partition has two views of time, and a guest hypervisor calibrates one against the other, so they have to start together. On KVM they do not, at either of the two points where the partition's clock starts.

stimer_start() in arch/x86/kvm/hyperv.c decides whether a synthetic timer has already expired:

time_now = get_time_ref_counter(hv_stimer_to_vcpu(stimer)->kvm);
...
stimer->exp_time = stimer->count;
if (time_now >= stimer->count) {
        /* ... expire immediately ... */

The two sides of that comparison come from different clocks. stimer->count is written by the guest and computed from the guest's own counter: counter_now + horizon. time_now is the partition reference counter, which get_time_ref_counter derives from the kvmclock (the TSC page's scale and offset are computed in compute_tsc_page_parameters from hv_clock->system_time, so the reference counter follows the kvmclock whatever the TSC offset is).

So if the guest counter trails the clock by D, counter_now reads D low, every deadline lands D early, and the immediate-expire branch is taken for every horizon shorter than D. Both start points leave it trailing:

• At creation, the kernel zeroes the reference clock when the VM is created (kvm_arch_init_vm sets kvmclock_offset to minus the current base time) and fixes the guest counter's origin only when the BSP vCPU is created (kvm_arch_vcpu_postcreate calls kvm_synchronize_tsc(vcpu, NULL)). openvmm builds the whole partition between those two ioctls: the supported-CPUID query, the leaf build, capability derivation. The guest then carries that interval as a constant disagreement for its whole life.
• At machine reset, reset_all sets the reference clock to its at-reset value of zero and asks the same of each vp's Tsc. That second half silently does not happen. The write goes through MSR_IA32_TSC, and kvm_synchronize_tsc reads a host write of exactly zero as "userspace is creating or synchronizing this vcpu" rather than as a value to store, substituting kvm->arch.cur_tsc_offset for the offset the write implied. Zero, the one value a reset needs to deliver, is the one value that path cannot deliver.

What it measured

The creation gap ran 832 to 977 us over seven cold starts on the host below. It lands 1:1 in the horizon a guest hypervisor computes for a synthetic timer deadline: regressing horizon on offset across those runs gives slope 1.0107 over a 10,153-tick offset span. That shortens the 1.978 ms one-shot this guest arms to about 1.15 ms and puts roughly half the arms in the past, giving about 1.88 M past-dated re-arms a second and a nested guest that never reached a usable desktop. After the change, 0.012 to 0.024 a second, and the guest boots and stays up.

The reset half is coarser and easier to see. A guest hypervisor calibrates its reference clock off the counter and arms a one-shot at an absolute deadline on the previous boot's timeline, 38.50 s measured, while the partition counter reads 2.24 s. The deadline is then about 36 real seconds away, the clock init polls for it with a bounded budget, retires the timer when it never fires, runs out of candidates and bugchecks, which presents as a hang at the firmware logo. The MSR behaviour was confirmed directly on a live guest: counter at 5403506220, wrote 0, read back 5403522705, while a control write of 0x4000000000000000 landed.

The fix

Both halves are the same operation, so they are one function called from two places: read the clock, read the counter, write the corrected offset to every vp. At creation it runs after the add_vp loop, so the loop's own duration is not left in the answer. At reset it runs after reset_all, which has just returned the clock to zero.

The correction moves the counter to the clock and never the other way. The clock is the partition's authority on time: every synthetic timer deadline is expressed in it and GetReferenceTime reads it directly, while the counter is a per-vp view of the same instant.

The write goes through the vCPU device attribute KVM_VCPU_TSC_CTRL/KVM_VCPU_TSC_OFFSET rather than MSR_IA32_TSC, because the MSR path infers what userspace meant from the value's distance to where the counter would be anyway. A write of exactly zero is read as vCPU creation, which is the reset defect above, and a non-zero write within a second of the previous one is folded onto the existing offset by the slop branch once kvm->arch.user_set_tsc is set. The creation half needs the counter at the clock's value, which is inside that window by construction, and a reset following the previous write inside a second is inside it too. kvm_arch_tsc_set_attr has no such heuristic and stores what it is given. On a kernel without the attribute, which landed in 5.16, KVM_HAS_DEVICE_ATTR says so up front and openvmm warns and proceeds rather than failing the partition.

Every vp gets the same offset, computed once. The kernel takes unequal offsets as unsynchronized vcpus, so one pass with one value opens a single TSC generation that every vp joins, where a separately sampled value per vp would open one generation per write and drop the partition out of masterclock mode, which the reference clock is built on.

Why the counter is left ahead of the clock

The category is precedented. The placement is not, and that is the honest way to put it.

KVM already programs a timer to fire early to cover a latency it cannot remove. That is lapic_timer_advance in arch/x86/kvm/lapic.c, and its comment is this argument in other words: KVM "programs the host timer event to fire early, i.e. before the deadline expires, to account for the delay between taking the VM-Exit ... and the subsequent VM-Enter". start_sw_tscdeadline applies it as ktime_sub_ns(expire, timer_advance_ns) behind a guard of ns > timer_advance_ns, which is a do-not-schedule-closer-than rule written as code. The sizes are LAPIC_TIMER_ADVANCE_NS_INIT 1000 and LAPIC_TIMER_ADVANCE_NS_MAX 5000, tuned between 100 and 10000 cycles in eighths by adjust_lapic_timer_advance, with limit_periodic_timer_frequency as the companion floor on period.

What is new is the stage. timer_advance_ns is applied when a deadline becomes a host timer. That stage is not reachable from here: by the time stimer_start gets to it, the arm has already been sorted into past or future, and the sorting is the part that goes wrong. So the same compensation moves one step earlier, into the origin the deadline is computed from. Not a new idea, and not an already-blessed placement either.

A reviewer will find kvm_xen_start_timer (arch/x86/kvm/xen.c) sooner or later, so I would rather raise it myself. It names our defect precisely and fixes it the same way, by computing the guest's view of the clock the way the guest computes it instead of from a host-side approximation, refusing get_kvmclock_ns() for having "a systemic error in its results because it scales directly from host TSC to nanoseconds, and doesn't scale first to guest TSC and then to nanoseconds as the guest does". And then it stops. It names the residual left between its own ktime_get() and rdtsc() and accepts it uncompensated. If matching the origins and living with the residual is enough for Xen timers, why is it not enough here?

Because of where the residual leads. Xen's answer to a non-positive delta is xen_timer_callback called inline, and KVM's answer in start_sw_tscdeadline is apic_timer_expired. Both are terminal. A one-shot that fires immediately fires once, the guest loses a tick, life goes on. The Hyper-V direct-mode auto-enable one-shot re-arms on delivery. Firing it immediately does not cost a tick, it hands the guest back a deadline that the guest recomputes from the same trailing counter and re-arms just as short. Nothing in that loop terminates it. That is the storm, 1.88 M past-dated re-arms a second. So the remedy KVM accepts for a horizon shorter than the latency is structurally unavailable here, and the compensation has to land before the comparison rather than after it.

For a timer that re-arms, the kernel has already decided which way a past deadline should go, and it decided it in this same function. stimer_start's periodic branch does not fire a past-dated deadline immediately. It re-anchors it strictly into the future and throws the missed ticks away:

div64_u64_rem(time_now - stimer->exp_time, stimer->count, &remainder);
stimer->exp_time = time_now + (stimer->count - remainder);

Push forward, do not fire, when firing would only come straight back. This change does that to the one-shot's inputs, which is the only place a one-shot allows it.

Stock KVM needs none of this, because there the guest and the kernel share one origin by construction: compute_tsc_page_parameters derives the reference TSC page from the kvmclock and get_time_ref_counter reads that same struct back. The problem exists only where something else establishes the counter, which is our case.

That leaves the sign and the magnitude, which are two different kinds of claim.

The sign holds anywhere. It is read straight off time_now >= stimer->count: at a lead of exactly zero a zero-horizon arm still satisfies the comparison and fires immediately, so only a strictly positive lead is safe. The intuition runs backwards here, since a counter that trails makes deadlines read early rather than far away. A zero lead was measured rather than argued about, interleaved, 3 rounds by 2 cells by cold and warm. At 20 us: 0.027 past-dated arms a second, with no near-class arm at all across 565 s and 12 boots. At zero: 0.104 a second, and 46 of the 63 formed a near class between -8.2 and -78.9 us, median -19.5 us, none of them within 3 us of zero. So the class the lead removes is real, and its absence is not the same thing as arms that were genuinely due. The control is that the main horizon cluster moved from 2007.6 to 1987.6 us, exactly 20.0 us, so the lead was the only variable. There is a deeper class from -277 us out to -193 ms that sits at about 0.027 a second in both arms; no lead of this size touches it and none is claimed to.

The magnitude covers one delay, L, between the guest reading its counter to compute a deadline and KVM evaluating that deadline against the clock. Over L the clock advances and the guest's number does not, so a zero-horizon arm reads as past whenever the lead is at or under L. L was measured directly rather than inferred from a symptom, pairing kvm_exit(MSR_WRITE) with the set_count and the get_time_ref_counter in stimer_start: about 98k arms per run, 100% pairing, and two independent arms agreeing. p50 3.2 us, p90 3.9, p99 12 to 14, p99.9 18.4 to 18.7, max 72 to 123. So 20 us is L's p99.9 rounded up, not a multiple of its median. Anyone can rebuild that histogram on their own host and check the number, which is the main thing it has going for it. It is also the bottom of the 21-to-38 us achieved band that measured clean, so it sits at the edge of measured data rather than under it.

An adaptive lead is the obvious next question, given that timer_advance_ns is adaptive and this is not. adjust_lapic_timer_advance can close a loop because it has guest_tsc - tsc_deadline in hand on every expiry. There is no comparable signal here. This correction runs twice in a partition's life, at creation and at reset, and what it would have to converge on is a property of the host's exit latency rather than of anything the code sees afterwards. Making it adaptive means first building the measurement, which is the ftrace pairing above, and then putting it in the arm path. That is worth doing if the static floor turns out to be wrong somewhere, and not before. It is future work, and I would rather say so than imply the static value is the last word.

The floor is not the whole lead: the request adds the pairing's own worst-case error, so the achieved lead is at least the floor even at the worst residual, and a partition whose pairing is exact pays nothing for accuracy it did not need. And the result is measured rather than assumed. After the write the pair is read again under the new offset, the lead it produced is computed from that reading and classified: inside the band both measurements allow, outside it, or under the floor. The last one is an error-level line naming the lead, because a host where this goes wrong would otherwise produce a timer storm and no signal at all.

The cost is a synthetic timer delivered up to 20 us late, about 1% of the 1.978 ms period this guest arms.

The counter/clock pairing

KVM_GET_CLOCK reports host_tsc at the instant it was called and computes the clock it returns from exactly that value: __get_kvmclock ends with __pvclock_read_cycles on data->host_tsc. The guest's view of it is host_tsc + offset. Using that makes the correction exact instead of reconstructed.

The bracket, two counter reads around the ioctl, is kept but does a different job now: it is the bound the exact value has to lie inside, which turns "the guest counter is not scaled" from an assumption into a checked one. On a host that scales the counter the translation misses the bracket by orders of magnitude, and the fallback says so in the log rather than degrading quietly.

One subtlety is worth calling out, because it is what makes the field usable at partition build. KVM_CLOCK_HOST_TSC is set only on the masterclock branch, and use_master_clock is a cached bool that only pvclock_update_vm_gtod_copy writes. At build time it was computed once, in kvm_arch_init_vm, when the VM had no vcpus and the matching-TSC test could not hold. The vCPU creations since made that test true but only raised KVM_REQ_MASTERCLOCK_UPDATE, which a vp has to run to service, and none has. Measured there: 4 of 4 reads carried no host counter, and every read once the guest was running did.

KVM_SET_CLOCK is the way out, because kvm_vm_ioctl_set_clock calls pvclock_update_vm_gtod_copy on the calling thread, so writing the clock back at the value just read recomputes the flag with no vp needing to run. That rebases kvmclock_offset and rewinds the clock by the read-to-write gap, which is acceptable at that one point and nowhere else: nothing has observed the clock yet, and the counter is then put on whatever it reads afterwards. On a machine reset the caller has just written the clock anyway, so the first read there already carries the counter.

Testing

Unit tests cover the offset arithmetic (a counter behind the clock, ahead of it, already on it, a conversion too large for 64 bits, and a correction that carries the offset below zero), the bracket midpoint and its error bound against either endpoint, a bracket spanning the counter wrap, and the direction itself stated as the inequality a reader cares about: the corrected counter ends strictly ahead of the clock.

The verification's own arithmetic is tested against the production conversions rather than a test helper, comparing counters to clock + lead as exact rationals, sweeping the clock as well as the TSC rate, and carrying a negative control that fails if no swept case defeats the earlier form. That control exists because an earlier version of this test read the same clock value the correction was computed from, so the quantization cancelled and the test agreed with the write by construction.

Live: 8 boots with no shortfall reported, and the host-side error counters stayed flat across 155 further alignments.

No Guide change: nothing here is user-facing. There is no new flag and no changed command line, only different behaviour on the two paths described above.

Scope, and what has not been covered

Everything here was developed and measured on one host: a Xeon E5-2697 v2 running a nested guest hypervisor. Some of it generalises and some of it plainly does not, so here they are apart.

The sign generalises. It is derived from time_now >= stimer->count in stimer_start and from nothing else. A lead of zero admits an immediate expiry on any host, and a strictly positive lead is required on any host. No measurement of mine is load-bearing for that.

The magnitude does not. 20 us is L's p99.9 on this machine, and L is an exit-and-handle latency, so it will be a different number elsewhere. It is a floor rather than a target, which biases it the safe way: a host with a smaller L pays slightly more delivery latency than it needs, and a host with a larger L is the case to watch. That case is at least visible rather than silent, since the achieved lead is measured after the write and a shortfall is logged at error level. The 832 to 977 us creation gap is in the same category, being this host's build time between two ioctls and not a constant.

The scaled-counter path is untested, and this host cannot test it. The Xeon E5-2697 v2 has no VMX TSC scaling at all, so the guest counter is never scaled here and the only exercise the path gets is the bracket check detecting the case and taking the fallback. On current hardware the branch this machine cannot reach is the one that runs, so a reviewer's own box will exercise it the first time it runs the code. That is the part I would most like read closely.

The origin gap itself is only observable live, so the unit tests cover the arithmetic around it rather than the gap.

[github-actions]

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

[Copilot]

Pull request overview

This PR fixes a KVM timebase mismatch by actively aligning the guest TSC (guest-visible counter) to the partition reference clock during partition creation and machine reset, preventing Hyper-V synthetic timer storms and reset-time boot hangs caused by divergent clock origins.

Changes:

• Add a KVM-specific alignment routine to compute and apply a consistent vCPU TSC offset derived from a paired reference-clock/counter sample (with post-write verification).
• Invoke the alignment after vCPU creation (partition build) and after reset_all() (machine reset) to keep both partition time views consistent.
• Extend the vm/kvm wrapper to expose richer kvmclock reads (host_tsc/realtime as Option) plus vCPU TSC rate/offset attribute ioctls, and update reference-time reads to use the new API.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
vmm_core/virt_kvm/src/arch/x86_64/vm_state.rs	Switch reference-time save/restore reads to the new get_clock() API and clock_ns.
vmm_core/virt_kvm/src/arch/x86_64/mod.rs	Implement and apply guest-TSC-to-reference-clock alignment on partition creation and reset, including verification logic and unit tests for the arithmetic.
vm/kvm/src/lib.rs	Introduce ClockReading, replace get_clock_ns() with get_clock(), and add KVM vCPU TSC rate/offset support plumbing.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Robert Nowotny (bitranox)]

Fixed, though not by the path described.

lead_measurement_resolution_ns is 2 + ceil(1_000_000 / khz), so for resolution_ns to
reach the 20 us floor the counter would have to run below about 51 kHz. At any rate a host
actually reports, the floor test catches every negative lead before the cast is reached, so
the misclassification is not reachable as written.

The cast is still worth removing. Its soundness rests on a coupling between
GUEST_TSC_LEAD_FLOOR_NS and the resolution formula that nothing states: lower the floor
towards a microsecond and a small negative lead starts clearing the floor test, widens into
a large positive on the cast, and the counter trailing the clock gets reported as a harmless
overshoot. That is the dangerous direction rendered as the safe one, in the check that
exists to catch it.

The sign is now settled first, on its own terms. Added a test that fails on the previous
code with left: OutsideBand, right: BelowFloor.

[Robert Nowotny (bitranox)]

The comment was wrong and is corrected: any error is read as unsupported, not only the
ENXIO a kernel older than the attribute answers with.

Leaving the signature alone. Every caller's response to a failed probe is the same
degrade-and-warn path whatever the errno, so a Result here would hand the caller a
decision it has no way to act on differently.

I did try logging the errno, and backed it out: vm/kvm carries no logging dependency, and
neither does the sibling wrapper crate, so one debug line meant adding a dependency edge to
a crate that is deliberately pure FFI. The caller already warns that the alignment was
skipped, so what the errno would have added is detail, not the fact of it.

[Robert Nowotny (bitranox)]

Fixed. tsc_khz now rejects a rate that is not strictly positive instead of casting it,
and carries the raw signed value in the error so the zero case stays distinguishable from
the negative one.

The negative is the worse of the two. Zero at least collapses the conversions to nothing
and reads afterwards as an absent lead; a negative widens into a rate near 4.29e9 kHz and
produces an offset that looks entirely plausible, which the verification has no reason to
flag.

What the unusable rate then does to the alignment is the other thread on this PR.

[Robert Nowotny (bitranox)]

Fixed, and this was the one with teeth.

At a rate of zero the target counter falls out at zero ticks whatever the reference clock
reads, so the offset written to every vp puts the guest counter on an origin unrelated to
the clock. The verification afterwards does notice, reading the lead as absent, but only
after the write has landed, and noticing is not undoing. That makes it strictly worse than
the no-op of leaving the counter where it was, which is at least self-consistent.

The rate is now rejected at the wrapper, and the caller degrades exactly the way it already
does when the kernel cannot express the attribute at all: warn, and return without aligning.
The alignment is best effort by design, so an unreadable rate must not fail the partition
build.

The leaf conversions keep their own zero guards. Those keep the arithmetic defined; they
were never enough to keep the answer meaningful, which is why the check belongs at the
entry point as well.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

vm/kvm/src/lib.rs:31

• ioctl_none_bad is only used in an #[cfg(target_arch = "x86_64")] ioctl definition, but the use nix::ioctl_none_bad; import is unconditional. On non-x86_64 builds this becomes an unused import and can fail CI under -D warnings. Gate the import with the same cfg as the use site.

    #[cfg(target_arch = "x86_64")]
    use nix::errno::Errno;
    use nix::ioctl_none_bad;
    use nix::ioctl_read;

[Robert Nowotny (bitranox)]

Declining this one: the code compiles.

The match ending at this line is in STATEMENT position, not value position. Every arm is a
tracing::warn! / tracing::info! call, so the match evaluates to (), and Rust does not require
a semicolon after a block-like expression used as a statement. The Ok(Some(alignment)) below it
is the function's tail expression rather than a second expression sharing a statement with it.

The match this change actually added is the other one, in align_guest_tsc_to_reference_clock:
let guest_tsc_khz = match bsp.tsc_khz() { ... };. That one IS in value position, and it does
carry its semicolon.

Verified rather than argued, on this head: cargo clippy --workspace --all-targets -- -D warnings
passes, which compiles every crate in the workspace; so do cargo build -p openvmm and
cargo test -p virt_kvm (45 passed).