virtio: move device ownership into async task#3053
Conversation
The virtio PCI and MMIO transports currently own the Box<dyn VirtioDevice> directly and call into it synchronously from MMIO read/write handlers. This is a problem for vhost-user support, where the "device" lives in another process and device operations are inherently asynchronous. This commit moves the VirtioDevice into a spawned async task that processes commands via a mesh channel. The transports become thin MMIO/PCI forwarders: transport-register reads and writes are handled synchronously as before, but device-config register accesses and lifecycle operations (enable, disable, stop, start, reset) are sent as RPCs to the task. The new DeviceTask struct owns the device and exposes methods for each operation. A shared TransportState state machine tracks in-flight enable/disable transitions, polled from poll_device so the chipset can observe completion asynchronously. Device-config register reads and writes use the chipset's existing deferred-IO mechanism to avoid blocking the vCPU. No changes to the VirtioDevice trait or any device implementation.
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
There was a problem hiding this comment.
Pull request overview
This PR refactors virtio PCI/MMIO transports to move Box<dyn VirtioDevice> ownership into a spawned async device task and forward lifecycle + device-config operations over a mesh RPC/channel, enabling inherently-async backends like vhost-user while keeping transport register handling synchronous.
Changes:
- Introduces
transport/task.rswith a device-task command loop and a sharedTransportStatemachine for tracking in-flight enable/disable. - Updates PCI/MMIO transports to spawn the device task, send RPCs for enable/disable/stop/start/reset, and defer device-config register accesses.
- Updates call sites and tests to provide a task driver/spawner and to account for async completion timing.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/virtio/virtio/src/transport/task.rs | Adds device-task command processing and a transport-side in-flight state machine. |
| vm/devices/virtio/virtio/src/transport/pci.rs | Spawns device task; forwards lifecycle/config IO via RPC/deferred IO; replaces synchronous device ownership. |
| vm/devices/virtio/virtio/src/transport/mmio.rs | Same as PCI: spawns device task, forwards lifecycle/config IO, and tracks async completion. |
| vm/devices/virtio/virtio/src/transport/mod.rs | Registers the new task module. |
| vm/devices/virtio/virtio/src/tests.rs | Adjusts tests for async enable completion and updated constructors. |
| vm/devices/virtio/virtio/src/resolver.rs | Passes a Spawn driver into VirtioPciDevice::new. |
| vm/devices/virtio/virtio/src/queue.rs | Enhances queue parameter inspection output. |
| openvmm/openvmm_core/src/worker/dispatch.rs | Passes a Spawn driver into virtio transport constructors. |
You can also share your feedback on Copilot code review. Take the survey.
- stop() now unconditionally sends DeviceCommand::Stop to the task, fixing a race where draining an in-flight enable left queues running because driver_ok was not yet set. - Guest reset (STATUS=0) during an in-flight enable is now tracked via a pending_reset flag in TransportState::Enabling. When the enable completes, poll_device() resets instead of setting DRIVER_OK. - Config register accesses are now byte-granular: sub-word and unaligned reads/writes to device config are deferred to the device task instead of returning 0 or being silently dropped. The task handles read-modify-write for partial writes.
There was a problem hiding this comment.
Pull request overview
This PR restructures virtio PCI/MMIO transports to support inherently-async device backends (e.g., vhost-user) by moving Box<dyn VirtioDevice> ownership into a spawned async task and communicating via mesh RPCs, while keeping transport register handling synchronous.
Changes:
- Add a new virtio transport “device task” module (
transport/task.rs) with RPC commands and a transport-side in-flight state machine. - Update PCI and MMIO transports to forward device-config and lifecycle operations to the async task and poll completion via
TransportState. - Update call sites/tests to pass a task driver for spawning the device task; improve inspect visibility for queue parameters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/virtio/virtio/src/transport/task.rs | New device-task command loop + shared transport state machine + deferred config IO helpers. |
| vm/devices/virtio/virtio/src/transport/pci.rs | Convert PCI transport to async-task ownership model; forward config/lifecycle ops via RPC; poll async completion. |
| vm/devices/virtio/virtio/src/transport/mmio.rs | Convert MMIO transport to async-task ownership model; forward config/lifecycle ops via RPC; poll async completion. |
| vm/devices/virtio/virtio/src/transport/mod.rs | Register new task submodule. |
| vm/devices/virtio/virtio/src/tests.rs | Adjust tests for async enable completion and new constructors requiring a driver spawner. |
| vm/devices/virtio/virtio/src/resolver.rs | Update PCI resolver to pass a driver spawner into VirtioPciDevice::new. |
| vm/devices/virtio/virtio/src/queue.rs | Make QueueParams inspectable with hex address fields. |
| openvmm/openvmm_core/src/worker/dispatch.rs | Update VMM device construction to pass a driver spawner into virtio device constructors. |
You can also share your feedback on Copilot code review. Take the survey.
- Remove ReadResult/WriteResult enums. Transport register handlers (read_u32_local/write_u32_local) now return plain u32/() values. Config detection happens once at the top of mmio_read/mmio_write, which defers the entire access to the device task regardless of size or alignment. - Fix pending reset: when STATUS=0 arrives during an in-flight enable, TransportState::poll() now sends a Disable RPC to the device task and transitions to Disabling, ensuring queues are actually stopped. Previously only local transport state was reset while queues kept running in the device task. - Add reset_during_enable_disables_queues tests for both MMIO and PCI transports.
There was a problem hiding this comment.
Pull request overview
This PR refactors virtio PCI/MMIO transports to move Box<dyn VirtioDevice> ownership into a spawned async device task, enabling inherently-async device backends (e.g., vhost-user) while keeping transport register handling synchronous.
Changes:
- Added an async
run_device_taskcommand loop plus aTransportStatemachine to track in-flight enable/disable transitions. - Updated PCI and MMIO transports to forward lifecycle and device-config register accesses to the device task via mesh RPC/deferred IO.
- Adjusted virtio transport tests and inspect output (queue params now inspectable) to account for async enable completion.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/virtio/virtio/src/transport/task.rs | Introduces device-task command protocol + shared transport state machine for async enable/disable/config access. |
| vm/devices/virtio/virtio/src/transport/pci.rs | Converts PCI transport to task-backed device ownership and defers device-config MMIO to the task. |
| vm/devices/virtio/virtio/src/transport/mmio.rs | Converts MMIO transport to task-backed device ownership and defers device-config MMIO to the task. |
| vm/devices/virtio/virtio/src/transport/mod.rs | Adds the new task module to the transport submodule tree. |
| vm/devices/virtio/virtio/src/tests.rs | Updates tests for async task behavior and adds helper yielding/polling utilities. |
| vm/devices/virtio/virtio/src/resolver.rs | Wires a Spawn driver into VirtioPciDevice::new for device task spawning. |
| vm/devices/virtio/virtio/src/queue.rs | Makes QueueParams inspectable and displays ring addresses in hex. |
| openvmm/openvmm_core/src/worker/dispatch.rs | Passes a driver spawner into virtio device constructors in the main VM initialization path. |
You can also share your feedback on Copilot code review. Take the survey.
- Change Enable RPC from FailableRpc to Rpc<_, bool>: errors are logged in the device task; only success/failure is signaled to the transport. - Add try_pending_reset() to atomically check-and-set pending reset, replacing the two-step is_busy() + set_pending_reset() pattern. - Move send_enable/send_disable into TransportState as start_enable() and start_disable() methods that encapsulate the state transition. - Add spec reference comments (virtio v1.2 §2.1) explaining the async reset semantics and pending_reset design. - Replace redundant TransportState::Ready assignments in restore() with asserts.
- Replace inline status resets with reset_status() in both MMIO and PCI transports for the pre-DRIVER_OK and ChangeDeviceState::reset() paths. - Rework TransportState::drain() to take a sender, handle pending_reset by chaining a disable (mirroring poll()), and return Option<TransportStateResult> so callers can apply side-effects. - Extract apply_transport_result() in both transports to deduplicate the match on TransportStateResult between poll_device() and stop(). - Add #[must_use] to TransportStateResult to catch future callers that silently discard the result. - Add regression tests for stop() during in-flight enable, stop() with pending guest reset, and stop() during failed enable.
There was a problem hiding this comment.
Pull request overview
This PR refactors virtio MMIO and PCI transports so they no longer own Box<dyn VirtioDevice> directly, instead moving device ownership into a spawned async task and interacting via mesh RPC/deferred IO. This supports inherently-async backends (e.g., vhost-user) while keeping transport register handling synchronous.
Changes:
- Introduce a
run_device_taskloop plus a sharedTransportStatestate machine to track in-flight enable/disable transitions. - Update virtio MMIO and PCI transports to forward device-config register accesses and lifecycle operations to the device task.
- Update construction call sites and unit tests to provide a task spawner/driver and to account for async enable completion.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| vm/devices/virtio/virtio/src/transport/task.rs | Adds device-task command protocol and TransportState transition tracking; implements deferred config IO forwarding. |
| vm/devices/virtio/virtio/src/transport/pci.rs | Moves PCI transport device interactions into task via mesh, adds async enable/disable sequencing, updates MMIO intercept handling. |
| vm/devices/virtio/virtio/src/transport/mmio.rs | Same refactor as PCI transport for MMIO, including deferred config IO forwarding and state machine integration. |
| vm/devices/virtio/virtio/src/transport/mod.rs | Wires in the new task module. |
| vm/devices/virtio/virtio/src/tests.rs | Adapts tests to async enable completion and new constructor signatures; adds helpers to yield/poll. |
| vm/devices/virtio/virtio/src/resolver.rs | Passes a driver/spawner to VirtioPciDevice::new. |
| vm/devices/virtio/virtio/src/queue.rs | Improves QueueParams inspectability (hex addresses). |
| openvmm/openvmm_core/src/worker/dispatch.rs | Passes a driver/spawner to virtio transport constructors. |
Comments suppressed due to low confidence (2)
vm/devices/virtio/virtio/src/tests.rs:3663
Context::from_wakertakes&Waker, butwakeris passed by value here. Update to pass&wakerso this compiles and theContextborrows a liveWaker.
let waker = std::task::Waker::noop();
let mut cx = std::task::Context::from_waker(waker);
self.dev.poll_device(&mut cx);
vm/devices/virtio/virtio/src/tests.rs:3783
Context::from_wakerexpects&Waker, but this passes theWakerby value, which won’t compile. Pass&wakerinstead.
let waker = std::task::Waker::noop();
let mut cx = std::task::Context::from_waker(waker);
self.dev.poll_device(&mut cx);
The virtio PCI and MMIO transports currently own the Box directly and call into it synchronously from MMIO read/write handlers. This is a problem for vhost-user support, where the "device" lives in another process and device operations are inherently asynchronous.
This commit moves the VirtioDevice into a spawned async task that processes commands via a mesh channel. The transports become thin MMIO/PCI forwarders: transport-register reads and writes are handled synchronously as before, but device-config register accesses and lifecycle operations (enable, disable, stop, start, reset) are sent as RPCs to the task.
The new DeviceTask struct owns the device and exposes methods for each operation. A shared TransportState state machine tracks in-flight enable/disable transitions, polled from poll_device so the chipset can observe completion asynchronously.
No changes to the VirtioDevice trait or any device implementation.