opentmk: Adds vmbus guest to opentmk

[wanghenry-msft]

This adds vmbus + netvsc guest client code so that opentmk_executor is able to fuzz netvsp/vmbus

[github-actions]

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

[Steven Malis (smalis-msft)]

That is a lot of code. Is there no way we can reuse our existing vmbus infrastructure?

[wanghenry-msft]

let me pull up the analysis from a few weeks back.

[Copilot]

🟡 Changes recommended

Critical lifecycle and memory-safety issues, along with VMBus session and protocol handling defects, remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a no_std guest-side VMBus client and integrates VMBus/NetVSP fuzzing into opentmk_executor.

Changes:

• Implements SynIC, protocol messaging, channels, GPADLs, interrupts, and hypercalls.
• Adds keyboard support and raw VMBus fuzzing primitives.
• Registers handlers and updates workspace dependencies and logging.

File summaries

File	Description
opentmk/vmbus_guest/src/synic.rs	SynIC setup and page allocation
opentmk/vmbus_guest/src/protocol.rs	VMBus wire types
opentmk/vmbus_guest/src/message.rs	Message encoding and completion routing
opentmk/vmbus_guest/src/lib.rs	Public client API and documentation
opentmk/vmbus_guest/src/interrupt.rs	SIMP polling and message handling
opentmk/vmbus_guest/src/hypercalls.rs	Hypercall wrappers
opentmk/vmbus_guest/src/hvsock.rs	Hvsock helpers
opentmk/vmbus_guest/src/gpadl.rs	GPADL lifecycle
opentmk/vmbus_guest/src/fuzz.rs	Raw VMBus fuzzing primitives
opentmk/vmbus_guest/src/error.rs	Error definitions
opentmk/vmbus_guest/src/devices/mod.rs	Device module exports
opentmk/vmbus_guest/src/devices/keyboard.rs	Synthetic keyboard driver
opentmk/vmbus_guest/src/connection.rs	Version negotiation and connection state
opentmk/vmbus_guest/src/channel.rs	Channel lifecycle
opentmk/vmbus_guest/Cargo.toml	Guest crate dependencies
opentmk/opentmk_executor/src/functions/vmbus.rs	VMBus fuzz handlers
opentmk/opentmk_executor/src/functions/netvsp.rs	NetVSP fuzz handlers
opentmk/opentmk_executor/src/functions/mod.rs	Function module registration
opentmk/opentmk_executor/src/executor/mod.rs	Executor integration and reset flow
opentmk/opentmk_executor/Cargo.toml	Executor dependencies
opentmk/opentmk_core/src/tmk_logger.rs	Logging level adjustment
Cargo.toml	Workspace membership and dependencies
Cargo.lock	Dependency lock updates

Review details

Suppressed comments (13)

opentmk/opentmk_executor/src/functions/vmbus.rs:300

• conn_id == 0 is resolved before with_session initializes the VMBus connection. On first use this permanently chooses fallback ID 4, so any host-selected connection ID is ignored even though resolve_connection_id is documented to use the negotiated value. Move the resolution into the with_session closure after bring-up.

        let connection_id = fuzz::resolve_connection_id(conn_id);

opentmk/opentmk_executor/src/functions/vmbus.rs:334

• This completion variant has the same first-use ordering bug: conn_id == 0 is resolved before with_session performs negotiation, so the negotiated connection ID can be ignored. Resolve the ID inside the closure after the session exists.

        let connection_id = fuzz::resolve_connection_id(conn_id);

opentmk/vmbus_guest/src/connection.rs:343

• On timeout the old VersionResponse is still allowed to arrive, but the next loop registers the same singleton CompletionKey::VersionResponse. A delayed response from the previous request is then delivered to the new handle and can make the guest build state for the wrong version. Do not fall through on timeout without fencing or draining the old response, or restart the connection.

            Err(Error::Timeout) => {
                log::warn!("negotiate: timeout on version {version:?}, trying next in ladder");
                continue;

opentmk/vmbus_guest/src/devices/keyboard.rs:295

• write_and_signal unconditionally requests a VM_PKT_COMP, but set_leds returns immediately and never drains that completion. Repeated LED updates will accumulate completion packets in the receive ring until it fills and blocks the channel; either make LED writes fire-and-forget or consume their completions.

        let mut flags = PacketFlags::new();
        flags.set_request_completion(true);
        let need_signal = self.send.write_inband(payload, flags, 0)?;

opentmk/vmbus_guest/src/error.rs:48

• This error discards the underlying TmkError, so callers only see hypercall failed and cannot distinguish access denial, invalid input, or resource exhaustion through Display. Include the source status in the error message to make failures actionable.

    #[error("hypercall failed")]
    Hypercall(#[from] TmkError),

opentmk/vmbus_guest/src/hypercalls.rs:42

• This API rejects oversized payloads at lines 56-60; it does not truncate them. The doc comment therefore promises behavior callers will not get and should say that payloads above HV_MESSAGE_PAYLOAD_SIZE return Error::Parse.

/// The payload is truncated at `HV_MESSAGE_PAYLOAD_SIZE` (240 bytes).

opentmk/vmbus_guest/src/interrupt.rs:376

• The SIMP page is written asynchronously by the hypervisor, but this path exposes it as a normal &mut [u8] and uses ordinary byte loads/stores (slot[0], slot[5], and payload copies). Those accesses are not a shared-memory synchronization primitive and can observe stale or torn data on weakly ordered targets; the existing VMBus ring and SynIC emulator use atomic shared-memory operations for this purpose. Read, copy, and clear the slot through atomic or otherwise explicitly shared-memory-safe operations.

            let first_byte = slot[0];

opentmk/vmbus_guest/src/lib.rs:85

• OfferChannel exposes instance_id, not interface_instance, so this public example cannot be copied as written.

//!         offer.interface_instance,

opentmk/vmbus_guest/src/lib.rs:114

• This public keyboard example calls Keyboard::open, but the driver exposes Keyboard::new and requires the caller to construct the ring views. The example therefore refers to an API that does not exist.

//! let mut kbd = keyboard::Keyboard::open(&mut ctx, kbd_offer)?;

opentmk/vmbus_guest/src/lib.rs:113

• Error::NotFound is not defined by this crate's Error enum, so the public example cannot compile when copied. Use an existing error or add a deliberate missing-offer error variant.

//!     .ok_or(vmbus_guest::Error::NotFound)?;

opentmk/vmbus_guest/src/lib.rs:135

• Error::NotFound is also used here, but that variant does not exist in this crate's error surface, so the netvsp example has the same copy-and-compile failure.

//!     .ok_or(vmbus_guest::Error::NotFound)?;

opentmk/vmbus_guest/src/message.rs:286

• The process-wide table is used by every high-level UEFI path, and GPADL/open requests use new monotonically increasing keys. Once a handle is dropped, this Weak entry is never pruned unless a caller explicitly invokes pending() (the high-level paths do not), so a long fuzz campaign grows the BTreeMap without bound. Prune dead entries while registering, or make cleanup automatic.

        self.inner.entries.lock().insert(key, Arc::downgrade(&slot));

opentmk/vmbus_guest/src/synic.rs:196

• This readback is only logged; none of the four returned values is compared with the values just written. Consequently program_synic_registers still returns Ok(()) if the hypervisor silently changes or rejects a register, contrary to the verification described above. Compare the expected register values and return an error on mismatch.

    let readback = get_vp_registers(
        ctx,
        HvInputVtl::CURRENT_VTL,
        &[
            HvRegisterName(HV_REGISTER_SIMP),

• Files reviewed: 24/26 changed files
• Comments generated: 11
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wanghenry-msft]

I think the biggest issue is that vmbus client is std and we need no_std

[Steven Malis (smalis-msft)]

I would much rather we investigate what would be required to make our existing vmbus code no_std instead of duplicating all of it. That'd also provide us fuzzing coverage benefits, as we'd cover the same implementation used in OpenHCL

[Steven Malis (smalis-msft)]

We should leave this on debug for test scenarios

[Steven Malis (smalis-msft)]

Or alternatively, we wait to take this work until rust-lang/rust#100499 is available.