[Pedro Henrique Penna (ppenna)]

Makes the emulated 8254 PIT reject a saved state in which a counter's control word has RW = 0, for microsoft/nvx#1066 and its duplicate microsoft/nvx#1147. RW = 0, bits 5:4 of an 8254 control word, encodes the counter latch command, which latches a counter's count without programming the counter, so no counter holds it: Timer::set_control latches the count and never stores it, and Timer::read and Timer::write handle only RW = 1, 2, and 3. PitDevice::restore installed each saved control word with ControlWord::from, which takes any byte, so a modified snapshot whose PIT state held RW = 0 restored, and the guest's next read or write of that counter's port reached unreachable!() and panicked the VM worker.

Two commits on 8eede133c, the head of feature/microvm/dev, after #5061 and #5065 changed the same file. The second changes only the first's tests, and can fold into it.

Reject a saved PIT control word with RW = 0 (607407fa9)

• Cause. PitDevice::restore built each timer's TimerState with control: ControlWord::from(control), without the check that Timer::set_control makes of every control word that a guest writes. Counter 0's control word 0x10 changed to 0x00 in a fresh PIT's saved state, then a write to port 0x40, panicked in Timer::write (chipset: PIT restore accepts a control word with rw = 0, then hits unreachable!() on the next access nvx#1066), and any counter's control word with bits 5:4 cleared, then a read of its port, panicked in Timer::read (openvmm: PIT restore accepts RW=0 and subsequent counter I/O can panic nvx#1147).
• Change. restore checks every timer's saved control word, after the fraction of the last tick and before it changes any state, and fails with RestoreError::InvalidSavedState, "timer N control word 0xNN has no read/write mode", when one has RW = 0, so a rejected restore leaves the device as it was. The other bits of a control word need no check: every mode decodes, 6 and 7 as the 8254's aliases of modes 2 and 3, and the counter-select bits of a stored control word affect nothing. Timer::set_control has never stored RW = 0, and a reset counter holds RW = 1, so every state that the PIT saved still restores; no field, format, or version changes.
• Alternatives. The patch attached to openvmm: PIT restore accepts RW=0 and subsequent counter I/O can panic nvx#1147 checks each control word in the loop that installs the timers, so rejecting counter 1's or 2's leaves the earlier counters' saved state installed; checking every timer first rejects the state before anything changes, as chipset: PIT restore accepts a control word with rw = 0, then hits unreachable!() on the next access nvx#1066 expects. Making Timer::read and Timer::write tolerate RW = 0 would keep the invalid state rather than reject it where it enters.
• Tests.
  • restore_rejects_rw0_control_word_without_changing_state: for each counter and each of the 64 control words with RW = 0, restoring a fresh PIT's saved state with that control word, as both issues do, fails with InvalidSavedState, into a PIT whose counters are reset and into one whose three counters run, and leaves the PIT's saved state equal to that of an identical PIT that never restored; the counter's port then reads, takes a write, and reads as in that PIT.
  • restore_accepts_every_programmable_control_word: for each counter, every control word that a guest can program, with RW = 1, 2, or 3, modes 0 to 7, and binary or BCD counts, survives a save and restore, and the restored PIT reads and writes the counter as the saved one does.

Keep the PIT restore tests within Linux's open file limit (6f57a6d61)

• Cause. The first commit's tests created a GuestPit for each case, three for each of 384 cases and two for each of 144. A GuestPit leaves some of its files open on Linux after it is dropped, so restore_rejects_rw0_control_word_without_changing_state failed there with "failed to create event: Too many open files (os error 24)" at the default limit of 1024 open files per process, and passed with a limit of 16384. Windows, where the tests passed, has no such limit.
• Change. The tests create five GuestPits in all, and each case resets the PITs that it uses, so that every case starts from the same state. The negative test's saved state also holds a fraction of its last tick that the reset PITs lack, as the PITs that it created for each case had one that the saved state lacked, so that a failed restore that installed only the fraction still shows.
• Tests. Without the restore check, the negative test fails at counter 0's control word 0x00, which restores. With the check in the loop that installs the timers, as in openvmm: PIT restore accepts RW=0 and subsequent counter I/O can panic nvx#1147's patch, it fails at counter 1, whose rejection leaves counter 0's saved state installed. With a failed restore that installs only the fraction, it fails at counter 0's control word 0x00. With a restore that keeps the PIT's output instead of the saved one, the positive test fails at control word 0x12.

Not in this change, found while auditing what restored state the PIT accepts, neither of which panics:

• A restored read_high set while the counter's control word has RW = 1 or 2, which no guest programming produces, keeps Timer::read from releasing a latched count, so the counter's latched reads repeat one count until the guest writes a control word for the counter.
• The existing InvalidLastTick and E_PIT_ACTIVE checks run after restore installs the saved timers, last, and last_frac_ns, so a restore that fails them leaves that state installed. A failed restore ends the VM before any processor runs.

Validation

With Rust 1.97.1, on Windows, for chipset, at 6f57a6d61: cargo check, cargo clippy --all-targets (clean apart from the environment's LNK4199 linker notices), cargo doc --no-deps, and cargo nextest run --profile agent (112 passed), then cargo xtask fmt --fix (rustfmt, lints, verify-fuzzers, and verify-flowey complete, no changes). On Linux, on the Azure Linux 3 KVM test VM, at the default limit of 1024 open files: cargo nextest run --profile agent -p chipset and cargo test -p chipset --lib, 112 passed each.

At 8eede133c, as at c2c21ddb2, the reproductions of both issues restore, and then the next write of port 0x40 panics with "internal error: entered unreachable code" in Timer::write, and the next read of port 0x40, 0x41, or 0x42 in Timer::read. With these commits, each restore fails with "timer N control word 0x00 has no read/write mode", and the ports read and write. A scratch test that restored 300,000 random saved states, with random timers, last up to the current time, and last_frac_ns, and then accessed every port, in a debug build, panicked at once without the check and passed with it; with an audit of the device's panics and arithmetic, it finds no other restored state that panics the PIT.

The PIT is OpenVMM's own on every backend, so this change is backend-independent; NVX's microVM tests on KVM, MSHV, and WHP run with it through microsoft/nvx#1558, which pins this head.

[github-actions]

29 Petri tests failed (28 unstable)