Skip to content

[DO NOT REVIEW] chipset: keep the sub-100 ns part of the PIT's last tick time - #5065

Merged
Pedro Henrique Penna (ppenna) merged 1 commit into
feature/microvm/devfrom
copilot/nvx-issue-1065-pit-sub-100ns-remainder
Nov 12, 2026
Merged

Pedro Henrique Penna (ppenna) merged 1 commit into
feature/microvm/devfrom
copilot/nvx-issue-1065-pit-sub-100ns-remainder

Conversation

@ppenna

@ppenna Pedro Henrique Penna (ppenna) commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Keeps the emulated 8254 PIT from counting the same time twice, for microsoft/nvx#1065 and its duplicate microsoft/nvx#1162. PitDevice::evaluate counts the whole 838 ns ticks that have elapsed since last, the VM time of the last counted tick, and advances last by them. VM time has a 100 ns resolution, so VmTime::wrapping_add dropped the 0-99 ns by which a tick generally follows a 100 ns step, and the next evaluation counted that time again. Every access to the PIT's ports or to port 0x61 evaluates the PIT, so it ran fast, and the faster the more often the guest accessed it: evaluating it every 900 ns for 90 ms counted 4.75% more ticks than evaluating it once, which misleads a guest that calibrates a clock against it. Two latched reads at one VM time could see different counts, and a restored PIT counted the dropped time again, so it read another count than the PIT that was saved, at the same VM time.

One commit on 5d656d854, the head of feature/microvm/dev (#5061, for microsoft/nvx#1063). microsoft/nvx#1557 pins this head.

Keep the sub-100 ns part of the PIT's last tick time (8eede133c)

  • Cause. evaluate advanced last with self.last.wrapping_add(Duration::from_nanos(ticks * NANOS_PER_TICK)), which keeps only whole 100 ns units, while the ticks that it counted end on the 838 ns grid. The truncated part, up to 99 ns per evaluation, was counted again by the next one.

  • Change. PitDevice keeps last_frac_ns, the nanoseconds, below 100, by which the last counted tick follows last. evaluate counts ticks from that exact time, last plus last_frac_ns, and keeps the part of the counted time below 100 ns; arm_wakeup measures the next wakeup from it; a reset clears it. The wakeup still rounds down to a 100 ns step: a VmTimeAccess timeout fires only once VM time has passed it, so that wakes the PIT at or after the tick, whereas measuring it from last alone could wake the PIT before the tick of the output change, which the 20-tick minimum would then defer.

  • Saved state. chipset.pit.SavedState gets a new uint32 field, last_frac_ns (#[mesh(3)]). Restore refuses a value of 100 or more before it changes the PIT, and treats a last tick after the current time, which a last equal to it puts there with any fraction, as InvalidLastTick, as it treats a last after it. The field is a u32 because mesh decodes a u8 field's varint as u8, which would turn 256 into a valid 0. A saved state without the field, from an earlier OpenVMM, restores with 0, so the PIT counts from last as it did, at most one tick early; an earlier OpenVMM ignores the field and counts as it did. The saved state stays compatible both ways, so no saved-state, snapshot, or time ABI version changes. The time ABI's E_PIT_ACTIVE check and the catch-up after downtime in start are unchanged, and the catch-up now counts from the exact saved tick.

  • Alternatives. Counting ticks from a fixed origin, which PIT evaluate drops sub-100ns time, so the PIT runs fast and same-instant reads differ nvx#1065 also suggests, counts the same ticks: last plus last_frac_ns stays exactly a whole number of 838 ns ticks after the last reset or restore, so the ticks counted by any VM time do not depend on how often the PIT was evaluated before. It would need an origin and a tick count in the saved state instead of one field below 100, and a mapping from the last of earlier saved states. Rounding the wakeup up would only delay it by one more 100 ns step, since the timeout already fires after VM time passes it.

  • Tests. The tests drive the PIT through its ports with [DO NOT REVIEW] chipset: start each PIT low/high count with its low byte #5061's GuestPit, which gains read, program_counter0, set_time (VM time in 100 ns units, which run_to now uses), and poll. Each of these fails at 5d656d854, before this change, with the value in parentheses:

    Two more check the change's own edges: reset_pit_counts_ticks_from_the_reset (a reset clears the fraction) and restore_rejects_an_invalid_last_tick_time (a fraction of 100 but not 99, and a fraction of 1 but not 0 with last at the current time). Reverting each part of the change, one at a time, fails the test that covers it.

Validation

At this head, 8eede133c, with Rust 1.97.1:

  • Windows: cargo check -p chipset, cargo clippy --all-targets -p chipset, cargo doc --no-deps -p chipset, and cargo nextest run --profile agent -p chipset: 110 passed, including [DO NOT REVIEW] chipset: start each PIT low/high count with its low byte #5061's tests; cargo xtask fmt --fix: clean.
  • The four tests above, run against 5d656d854's code, fail with the values in parentheses; the reset test passes there.
  • Linux, on the Azure Linux 3 KVM test VM: cargo nextest run --profile agent -p chipset: 110 passed.
  • End to end through NVX, with OpenVMM built at this head (source_clean) by NVX's build-openvmm on the Azure Linux 3 KVM and MSHV test VMs and the Windows WHP test VM: NVX's guest-boot, interrupt-map, snapshot-core, restore-processors, smp-snapshot, restore-downtime, time-abi-conformance, and time-host-correction scenarios pass with 1 and 2 VPs on all three backends. NVX's guests disable the PIT at boot, so these check that guests still boot, capture, and restore with the changed PIT and its saved state.

See microsoft/nvx#1557.

PitDevice::evaluate counts the whole 838 ns ticks that have elapsed
since `last`, the VM time of the last counted tick, and advances `last`
by them. VM time has a 100 ns resolution, so VmTime::wrapping_add
dropped the 0-99 ns by which a tick generally follows a 100 ns step, and
the next evaluation counted that time again. Every access to the PIT's
ports or to port 0x61 evaluates the PIT, so it ran fast, and the faster
the more often the guest accessed it: evaluating it every 900 ns for
90 ms counted 4.75% more ticks than evaluating it once, which misleads a
guest that calibrates a clock against it (microsoft/nvx#1162). Two reads
at one VM time could also see different counts, and a restored PIT
counted the dropped time again, so it read another count than the PIT
that was saved, at the same VM time (microsoft/nvx#1065).

The PIT now keeps the nanoseconds, below 100, by which the last counted
tick follows `last`, in last_frac_ns. evaluate counts ticks from that
exact time and keeps the part of the counted time below 100 ns,
arm_wakeup measures the next wakeup from it, and a reset clears it. The
wakeup still rounds down to a 100 ns step, which wakes the PIT at or
after the tick, since a timeout fires only once VM time has passed it;
measured from `last` alone, it could wake the PIT before the tick of the
output change, which the 20-tick minimum would then defer.

The saved state keeps it in a new uint32 field, last_frac_ns (mesh 3),
which restore requires to be below 100 before it changes the PIT; a u8
would decode 256 as 0. Restore also refuses a last tick after the
current time, which a `last` equal to it puts there with any fraction. A
saved state without the field, from an earlier OpenVMM, restores with 0,
so the PIT counts from `last` as it did, at most one tick early; an
earlier OpenVMM ignores the field and counts as it did. The saved state
stays compatible both ways, so no saved-state or snapshot version
changes.

Tests: two latched reads of channel 0, 2.5 us after it was programmed in
mode 2 with 0x50, both read 0x4f (without this change, 0x4f and then
0x4e); a PIT restored there from its encoded saved state reads 0x4f
(0x4e), and both PITs then read the count of the ticks elapsed at every
VM time up to 100 us; channel 0, in mode 2 with 0xffff, evaluated every
900 ns for 90 ms, reads 23673, as a PIT read once at 90 ms does (18572);
and the wakeup raises IRQ 0 at 18.5 us, the first VM time after the 22nd
tick, which ends a mode 0 count of 19 written at 2.5 us (19.2 us; 34.3
us with this change but an arm_wakeup that ignores last_frac_ns). A
reset PIT counts ticks from the reset, and restore refuses a
last_frac_ns of 100 but not 99, and one of 1 but not 0 with a `last`
equal to the current time.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added the release-nvx Targets a branch under feature/microvm/. label Nov 12, 2026
@ppenna
Pedro Henrique Penna (ppenna) merged commit 8eede13 into feature/microvm/dev Nov 12, 2026
73 of 74 checks passed
@github-actions

Copy link
Copy Markdown

Pedro Henrique Penna (ppenna) added a commit to microsoft/nvx that referenced this pull request Nov 12, 2026
The NVX pin moves to 6f57a6d61, position 266, 135 commits past the
mapped tip. Positions 265 and 266, which microsoft/openvmm#5067
proposes for feature/microvm/dev on its head, 8eede133c, are
unassigned, and position 266 folds into position 265. The Scope also
says that microsoft/openvmm#5065 merged position 264.

Position 265 makes PitDevice::restore reject a saved control word with
RW = 0, the counter latch command, before it changes any state, in
code that upstream main has, and adds two tests on position 263's test
helper, in a file that PR10's position 022 and positions 139, 263, and
264 also change. It changes none of the code of positions 022 and 139,
and nothing that a mapped PR adds. Replaying it on upstream main
conflicts in that file where it adds its error, its tests' import, and
its tests next to code of positions 139, 263, and 264, which upstream
main lacks; its check applies there. It follows position 264 upstream.
Position 266 changes only position 265's tests, which created PITs for
each case and so exhausted Linux's default limit of open files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-nvx Targets a branch under feature/microvm/.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant