Skip to content

virtio: move device ownership into async task#3053

Merged
jstarks merged 7 commits intomicrosoft:mainfrom
jstarks:vhost_user_pre2
Mar 20, 2026
Merged

virtio: move device ownership into async task#3053
jstarks merged 7 commits intomicrosoft:mainfrom
jstarks:vhost_user_pre2

Conversation

@jstarks
Copy link
Member

@jstarks jstarks commented Mar 18, 2026

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.

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.
Copilot AI review requested due to automatic review settings March 18, 2026 21:39
@github-actions github-actions bot added the unsafe Related to unsafe code label Mar 18, 2026
@github-actions
Copy link

⚠️ 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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.rs with a device-task command loop and a shared TransportState machine 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.

jstarks added 2 commits March 19, 2026 02:50
- 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.
Copilot AI review requested due to automatic review settings March 19, 2026 03:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@jstarks jstarks marked this pull request as ready for review March 19, 2026 06:17
@jstarks jstarks requested a review from a team as a code owner March 19, 2026 06:17
Copilot AI review requested due to automatic review settings March 19, 2026 06:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_task command loop plus a TransportState machine 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.
@github-actions
Copy link

- 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.
Copilot AI review requested due to automatic review settings March 20, 2026 00:58
@jstarks jstarks enabled auto-merge (squash) March 20, 2026 01:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_task loop plus a shared TransportState state 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_waker takes &Waker, but waker is passed by value here. Update to pass &waker so this compiles and the Context borrows a live Waker.
        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_waker expects &Waker, but this passes the Waker by value, which won’t compile. Pass &waker instead.
        let waker = std::task::Waker::noop();
        let mut cx = std::task::Context::from_waker(waker);
        self.dev.poll_device(&mut cx);

@github-actions
Copy link

@jstarks jstarks disabled auto-merge March 20, 2026 02:59
@jstarks jstarks merged commit 722b217 into microsoft:main Mar 20, 2026
59 of 61 checks passed
@jstarks jstarks deleted the vhost_user_pre2 branch March 20, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants