Repository navigation
Backport upstream #5764: save and restore UART state in snapshots - #36
Merged
Merged
Conversation
The VMM does not save the UART state when taking a snapshot. Upon restoring it uses a weird hack in emulate_serial_init() where it enables RX (but no TX) interrupts after a snapshot restore. This hack apart form weird is also wrong as the guest driver might have never enabled RX interrupts in the first place. A comment in the same function acknowledges that the correct thing to do is save and restore the entire device state. Because we don't re-enable TX interrupts after a snapshot restore, the guest driver might hang if it relies on those TX interrupts as reported in [1]. Fix this by saving the UART state in snapshots and restoring it from them. Bump the SNAPSHOT_VERSION appropriately. [1] - firecracker-microvm#5730 Reported-by: EJ Ciramella <ejc3@meta.com> Signed-off-by: Ilias Stamatis <ilstam@amazon.com> (cherry picked from commit 7e619f4) Backport to v1.14.4: the device manager here still has a single DevicePersistError and calls vmgenid.notify_guest() on restore, so the change is applied by hand around those. SNAPSHOT_VERSION goes from 8.0.0 to 9.0.0 (upstream went 9.0.0 to 10.0.0): snapshots taken by earlier v1.14.4 builds cannot be restored by this one, and the reverse is also refused.
Add a simple unit test that injects a few bytes into the UART's FIFO, stores it's state, create a new device from the saved state and is able to read back the correct bytes. Signed-off-by: Ilias Stamatis <ilstam@amazon.com> (cherry picked from commit 66ab0cf)
Commit 7e619f4 ("devices: serial: Save/restore UART state to/from snapshots") fixed a bug where the guest driver would hang after a snapshot restore because Firecracker didn't properly restore the state of the UART device. Add a test_serial_active_tx_snapshot test case (which mostly duplicates the existing test_serial_after_snapshot). Start an unbounded transmission before taking the snapshot. This will saturate the TX buffer of the emulated UART and will make the guest driver enable TX interrupts. Without the fix, the test case fails because the driver is stuck waiting for a TX interrupt that will never be delivered after the snapshot is restored. Note that the serial.tx("") that sends a newline to flush the buffer in the original test case isn't needed anymore since sending Ctrl+C will achieve the same result. Signed-off-by: Ilias Stamatis <ilstam@amazon.com> (cherry picked from commit 6a6e559)
dangra
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports firecracker-microvm#5764 (released in v1.16.0) onto
v1.14.4-fly.On restore, v1.14 rebuilds the serial device from scratch and turns on only its receive interrupt. If the snapshot is taken while the guest's 8250 driver is waiting for the transmit-empty interrupt, that interrupt never arrives. ttyS0 then stops transmitting for good, and every
/dev/consolewriter blocks once the tty buffer fills. With this change the UART's registers are saved in the snapshot and restored throughvm_superio::Serial::from_state, which re-raises a pending interrupt.Commits, cherry-picked with
-x:DevicePersistErrorand vmgenid notify.test_restore_from_stateunit testtest_serial_active_tx_snapshotintegration testThe upstream CHANGELOG commit is left out.
Snapshot compatibility:
SNAPSHOT_VERSIONgoes from 8.0.0 to 9.0.0, becauseDevicesStategainsserial_state. Builds with and without this change refuse each other's snapshots in both directions. A snapshot has to be resumed by the binary that took it.Testing:
cargo test -p vmm: everything passes excepttest_setup_sregs, which fails identically on the unmodified base on the nested-KVM host used./dev/console.Residual stall: with this change the console still misses a transmit interrupt about once per 1000 resumes, and recovers at the next restore. Upstream v1.17.0 shows the same rate (3 in 2000 cycles), so this backport does not introduce it.