feat: implement virtio based host-guest communication - #1794
Conversation
55942a2 to
3f28f23
Compare
3f28f23 to
e88be2d
Compare
e88be2d to
a430e4e
Compare
e39b64f to
8b29c51
Compare
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
This PR migrates host/guest communication fully to the virtqueue transport (removing legacy stack I/O), updates guest dispatch to return typed ReturnValue, and extends snapshot persistence to include canonical virtqueue state.
Changes:
- Replace guest dispatch/host-call plumbing from
Vec<u8>stack buffers to typedReturnValueover virtqueues (including C guest C-API). - Add a versioned OCI “transport” layer to snapshots to persist virtqueue ring images and validate them on load.
- Remove embedded FlatBuffer byte tables/legacy OUTB actions and introduce
ExternalValueSourcesupport for virtqueue-backed decoding.
Reviewed changes
Copilot reviewed 80 out of 88 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/rust_guests/witguest/src/main.rs | Update guest dispatch return type to ReturnValue. |
| src/tests/rust_guests/simpleguest/src/main.rs | Add byte-chunk guest/host functions and migrate dispatch/returns to ReturnValue. |
| src/tests/c_guests/c_simpleguest/main.c | Add ByteChunks round-trip and error-return behaviors for C guest testing. |
| src/schema/function_types.fbs | Remove embedded byte table variants; assign explicit union tags/reservations. |
| src/hyperlight_host/tests/snapshot_goldens/fixtures.rs | Generate goldens via UninitializedSandbox + config. |
| src/hyperlight_host/tests/integration_test.rs | Update tests for virtqueue/log delivery and removed legacy stack behavior. |
| src/hyperlight_host/tests/common/mod.rs | Add helpers for UninitializedSandbox with custom config. |
| src/hyperlight_host/src/sandbox/uninitialized_evolve.rs | Remove conditional virtq attachment path in evolve. |
| src/hyperlight_host/src/sandbox/uninitialized.rs | Update snapshot-related tests for new config/layout expectations. |
| src/hyperlight_host/src/sandbox/snapshot/tripwires.rs | Add transport media-type tripwire and update OUTB action assertions. |
| src/hyperlight_host/src/sandbox/snapshot/mod.rs | Update snapshot virtq documentation to reflect validation/immutability. |
| src/hyperlight_host/src/sandbox/snapshot/file/transport.rs | New transport blob codec for persisting virtqueue ring images (with tests). |
| src/hyperlight_host/src/sandbox/snapshot/file/mod.rs | Persist/load transport layer and validate it against layout. |
| src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs | Add transport layer media type constants. |
| src/hyperlight_host/src/sandbox/snapshot/file/digest.rs | Generalize blob verification comment (now used for more blobs). |
| src/hyperlight_host/src/sandbox/snapshot/file/config.rs | Remove input/output buffer sizes from snapshot config schema. |
| src/hyperlight_host/src/sandbox/config.rs | Remove input/output region sizing; adjust transport defaults and docs. |
| src/hyperlight_host/src/sandbox/builder.rs | Remove input/output sizing APIs; add virtqueue transport tuning APIs + tests. |
| src/hyperlight_host/src/mem/virtq_mem.rs | Adjust imports and offset calculation; extend tests. |
| src/hyperlight_host/src/mem/virtq/tests.rs | New virtqueue validation/snapshot/restore tests. |
| src/hyperlight_host/src/mem/virtq/codec.rs | New host-side virtqueue codec for function calls/logs (with tests). |
| src/hyperlight_host/src/mem/shared_mem.rs | Remove legacy stack I/O helpers and related tests/errors. |
| src/hyperlight_host/src/mem/mod.rs | Stop exporting legacy virtq_mem module publicly. |
| src/hyperlight_host/src/hypervisor/hyperlight_vm/x86_64.rs | Refactor test layout creation and update FXSAVE test to use scratch. |
| src/hyperlight_host/src/error.rs | Add poisoning TransportError and VirtqError conversion. |
| src/hyperlight_host/benches/benchmarks.rs | Update benches to new transport/config + add codec benchmarks for externals. |
| src/hyperlight_guest_tracing/src/lib.rs | Avoid deadlock by returning None when trace state lock is held; add test. |
| src/hyperlight_guest_macro/src/lib.rs | Update dispatch macro/docs to return ReturnValue vs Vec<u8>. |
| src/hyperlight_guest_capi/src/error.rs | Rework C-API error setting to stash HyperlightGuestError instead of stack writing. |
| src/hyperlight_guest_capi/src/dispatch.rs | Convert C-API dispatch and host calls to typed ReturnValue over transport. |
| src/hyperlight_guest_bin/src/transport.rs | Pass mailbox address to guest transport initialization. |
| src/hyperlight_guest_bin/src/lib.rs | Initialize transport earlier (before logging/guest init); export ReturnValue. |
| src/hyperlight_guest_bin/src/host_comm.rs | Route host calls through transport and return typed ReturnValue. |
| src/hyperlight_guest_bin/src/guest_logger.rs | Emit logs via virtqueue transport (and tracing path integration). |
| src/hyperlight_guest_bin/src/guest_function/definition.rs | Keep return values typed (ReturnValue) until transport encoding. |
| src/hyperlight_guest_bin/src/guest_function/call.rs | Dispatch via virtqueue, send results, and support snapshot checkpoints. |
| src/hyperlight_guest/src/transport/mod.rs | Add guest-side codec module; rename with_context → with_ctx. |
| src/hyperlight_guest/src/transport/mem.rs | Add BufferMap-backed Bytes owner retention via GuestMapping. |
| src/hyperlight_guest/src/transport/codec.rs | New guest-side message decoding for requests/responses (with tests). |
| src/hyperlight_guest/src/lib.rs | Remove legacy guest_handle modules that depended on stack I/O. |
| src/hyperlight_guest/src/guest_handle/io.rs | Delete legacy stack-based input/output buffer I/O. |
| src/hyperlight_guest/src/guest_handle/host_comm.rs | Delete legacy host-call/logging implementation over OUTB + stack I/O. |
| src/hyperlight_guest/src/guest_handle/handle.rs | Move read_n_bytes_from_user_memory into GuestHandle. |
| src/hyperlight_guest/src/error.rs | Add conversion from GuestError and tweak bail! formatting. |
| src/hyperlight_component_util/src/guest.rs | Emit ReturnValue::VecBytes directly instead of size-prefixed flatbuffer helper. |
| src/hyperlight_common/src/virtq/ring.rs | Improve chain element storage handling; add bookkeeping error; add poll rollback support. |
| src/hyperlight_common/src/virtq/mod.rs | Normalize bookkeeping error; add zeroed_vec helper and test. |
| src/hyperlight_common/src/virtq/buffer.rs | Add Segments::append and optimize segment collection; add tests. |
| src/hyperlight_common/src/transport.rs | Add ExternalValueSource impls for RecvChain/Segments; harden header parsing. |
| src/hyperlight_common/src/outb.rs | Remove legacy stack actions and add VirtqNotify. |
| src/hyperlight_common/src/mem.rs | Remove PEB input/output stack regions. |
| src/hyperlight_common/src/layout.rs | Remove IO-buffer sizing from min scratch; add mailbox in transport arena layout. |
| src/hyperlight_common/src/flatbuffers/mod.rs | Stop including generated embedded byte payload tables. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/return_value_generated.rs | Update generated union max/values for removed variants. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/return_value_box_generated.rs | Remove accessors/verification/debug for removed return variants. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/parameter_value_generated.rs | Update generated union max/values for removed variants. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/parameter_generated.rs | Remove accessors/verification/debug for removed parameter variants. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/hlvecbytes_generated.rs | Remove generated embedded byte-vector table. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/hlsizeprefixedbytechunks_generated.rs | Remove generated embedded size-prefixed bytechunks table. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/hlsizeprefixedbuffer_generated.rs | Remove generated embedded size-prefixed buffer table. |
| src/hyperlight_common/src/flatbuffers/hyperlight/generated/hlbytechunks_generated.rs | Remove generated embedded bytechunks table. |
| src/hyperlight_common/src/flatbuffer_wrappers/util.rs | Remove legacy get_flatbuffer_result; keep capacity estimator aligned to external bytes. |
| src/hyperlight_common/src/flatbuffer_wrappers/guest_log_level.rs | Update comment referencing renamed log test. |
| src/hyperlight_common/src/flatbuffer_wrappers/codec.rs | Document ExternalValueSource implementations and expectations. |
| src/hyperlight_common/src/arch/amd64/layout.rs | Remove IO-buffer sizing from architecture minimum scratch computation. |
| src/hyperlight_common/src/arch/aarch64/layout.rs | Remove IO-buffer sizing from architecture minimum scratch computation. |
| fuzz/fuzz_targets/host_call.rs | Update fuzz harness to new sandbox initialization/config approach and new error cases. |
| fuzz/fuzz_targets/fuzz_push_pop_buffer.rs | Remove fuzz target for deleted stack push/pop I/O. |
| fuzz/Cargo.toml | Remove fuzz binary entry for deleted target. |
| docs/snapshot-versioning.md | Document added transport version marker and updated ABI scope. |
| docs/snapshot-oci-format.md | Document new OCI transport layer and its framing. |
| docs/paging-development-notes.md | Update documentation to reflect virtqueue transport arena usage. |
| docs/README.md | Link new virtqueue host/guest communication doc. |
| Justfile | Remove deleted fuzz target; update isolated/coverage test names. |
| CHANGELOG.md | Document virtqueue-only transport, snapshot transport layer, removed legacy stack I/O, etc. |
Suppressed comments (3)
src/hyperlight_common/src/transport.rs:1
- The header now derives
CheckedBitPattern/NoUninit(to validate enum discriminants), butas_bytes()still usesbytemuck::bytes_of(self). Depending on the bytemuck version,bytes_ofmay requirePod, whichMsgHeadercannot safely implement because it containsMsgKind(not allu8bit-patterns are valid). To avoid potential compilation failures or accidental UB, consider makingas_bytes()produce a[u8; MsgHeader::SIZE]via explicit little-endian encoding, and adjust callers/tests accordingly (or provide awrite_to(&mut [u8; SIZE])helper).
// SPDX-License-Identifier: Apache-2.0
src/hyperlight_common/src/transport.rs:88
- The header now derives
CheckedBitPattern/NoUninit(to validate enum discriminants), butas_bytes()still usesbytemuck::bytes_of(self). Depending on the bytemuck version,bytes_ofmay requirePod, whichMsgHeadercannot safely implement because it containsMsgKind(not allu8bit-patterns are valid). To avoid potential compilation failures or accidental UB, consider makingas_bytes()produce a[u8; MsgHeader::SIZE]via explicit little-endian encoding, and adjust callers/tests accordingly (or provide awrite_to(&mut [u8; SIZE])helper).
pub fn as_bytes(&self) -> &[u8] {
bytemuck::bytes_of(self)
}
src/hyperlight_guest_bin/src/host_comm.rs:1
- The new type-mismatch error is quite minimal, which can make debugging host/guest ABI issues harder (especially now that all calls go through virtqueues). Consider including at least the host function name and the expected return type (and optionally the actual
ReturnValuevariant) in the error message so failures can be diagnosed without adding extra tracing.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
I am curious if you have run any of the benchamrks compared to main and could share?
Question about ergonomics orthogonal to this pr: Could the call on the host accept slice like &[u8] (or &[&[u8]] and &str instead of the owned counterparts? At least 1 copy is necessary on the host I believe, so looks doable on the surface maybe? I think it could get rid of some clones if host only has access to a slice and then needs to clone it unnecessarily.
Guest functions which take a String have the bytes copied from the flatbuffer into a new String if I understand correctly. Could we consider supporting guest functions which take something similar to a &str pointing directly into the flatbuffer to get rid of copy? Flatbuffer already supports it I think. An alternative could perhaps be to document that if perf is important, use ByteChunks instead of String.
Will get back reviewing this in more detail shortly!
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
b02e4c4 to
5ea4c3c
Compare
|
Hey @ludfjig,
It looks feasible, yes. I think currently
I think this concern is valid. The preparation work in this implementation is a tradeoff: the problem is that we need to rebuild the host side state of the queue from guest controlled shared memory. If we support any arbitrary state, the complexity of validation that state blows up. Canonical state gives the host very well defined and small invariant. We could move that preparation to the restore step, but that adds a host to guest roundtrip for first call and adds delay to the restore. I think it would be great if we could at least limit the ability to call host functions during snapshot checkpoint. That would eliminate side effects on the host. I could also add tests/examples for handling spinning snapshot checkpoint, poisoning sandbox when guest fails to canonicalize queues? Let me know what you think. I also vaguely remember we did similar checkpoint for nanvix but I'm very fuzzy on the details. |
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
b99855e to
09e788c
Compare
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
09e788c to
579e7e1
Compare
Sounds good maybe we can try to do this in a follow up. And yes I was mostly concerned with the host side api ergonomics, although if a copy could be removed on the guest-side that would be bonus too (seems hard though)
Makes sense. I'm not sure there are any other alternatives to this then
Yes this is probably not idea
I think this would be useful, although it would need to be done by the host (don't accept host calls during this period) since guest is untrusted
Sounds good. I don't remember either for nanvix unfortunately 🤔 |
paging @danbugs who might have some insight |
|
@ludfjig would it be fine to merge this PR as is and address the remaining concerns as a follow up? We can create issues for them with release blocker label. What do you think? |
fine by me :) |
simongdavies
left a comment
There was a problem hiding this comment.
Previously, taking a snapshot involved only the host and didn't rely on guest. Now we enter the guest and rely (?) on the guest to do some specific work. A malicious guest could either not do the required work, do it incorrectly, but also do something else completely (spin infinitely, abort, etc). This seems like a big change that we should make sure we understand.
Would doing something like this alleviate some of the concerns here:
- Set a host-owned Checkpointing execution mode before entering the guest.
- During that mode, outb_virtq_call must reject ordinary G2H function requests before call_host_function .
- Permit only operations required by checkpointing, such as logs, completion notification, abort, and cancellation.
- Clear the mode on every exit path.
- Validate the queues and mailbox after the guest returns. Do not trust the dedicated guest routine to have behaved correctly.
I also added a couple of agent generated issues, so I dont have particularly high confidence in them
I just added a couple of comments, not sure if a couple of these are even valid since the are agent generated, happy to address these in a follow up as they don't alter the functionality, the comments on guest entry at snapshot liekwise could be addressed as a follow up |
|
I thought I would add some context to the discussion and justify the strategy for snapshoting in this PR. Each queue has two endpoints: a producer in the guest and a consumer on the host. After restore, their state must agree before communication can resume. The rings live in shared scratch memory, but they are only part of the queue state; both endpoints keep private state, including cursors and inflight chains. The guest also keeps buffer pool allocation and ownership state on the heap. H2G has an additional requirement that is before the host can send a request, the guest must post writable buffers. These fixed-size buffers tell the host which memory slots it can use. They must be available immediately after restore if we want to avoid a preparatory VM entry. Transport pools are configurable and can be several megabytes. We do not want to capture their entire capacity with every snapshot. Only payloads still held by guest code should survive snapshotting. Those payloads must remain consistent with their captured ownership state and guest pointers. This creates a tension: we want compact snapshots and fast restore, but omitting transport storage means some preparation is necessary. Where should that work happen?
The first idea is to leave the queues ready for snapshotting whenever a top-level guest call finishes. The problem is the final G2H response; tthe guest submits it before halting, but the host processes and completes it after the VM exit. The guest cannot reclaim that entry before the host completes it. With the current protocol, fully resetting the queues would require another round trip. The host would process the response, then re-enter the guest to reclaim completed entries and reset its state. This would tax every function call for snapshot preparation, even when no snapshot is requested. That cost is not acceptable.
The guest's producer and pool bookkeeping already survive through the ordinary memory snapshot as part of the heap. We could also capture the rings, both buffer pools, and the host consumer cursors. That would preserve matching state without resetting the queues but snapshot size and restore copying is scaling with configured pool capacity, including unused slots and stale payloads. This really conflicts with the goal of preserving only live transport data.
The host can rewrite shared ring memory, but that does not reset the guest's private state. Its cursors, inflight chains, and allocation ownership would still match the old rings. The host could try to modify those guest objects too. That would couple snapshot correctness to private guest data structures, rust layouts, and ownership rules. It feels super fragile. 💀
Another option is to restore guest memory and ask the guest to construct fresh queues before normal execution resumes. This keeps the snapshot compact, but adds guest execution to restore. Deferring reconstruction until the first call only moves the latency. The host still needs writable H2G buffers before it can publish that call. This does not meet the goal of keeping restore immediately usable.
Ring descriptors do not identify every live payload. Guest code can retain a buffer after its descriptor has been consumed and reclaimed. Its lifetime is then represented by guest ownership state, not by the ring. Selective capture therefore needs an ownership-aware mechanism. Scanning the rings alone cannot safely tell us which payloads to preserve or discard.
Preparation should happen when a snapshot is requested and the transport is dirty. Ordinary calls should not pay for it, and restore should not need to execute guest preparation code. The checkpoint establishes a canonical transport state:
Successful capture preserves matching guest bookkeeping and canonical rings. Pool payloads remain outside the snapshot. Restore installs the rings and attaches consumers at the agreed initial positions, without a preparatory VM entry. I would also include in this, proposed by @simongdavies, checkpoint execution restricitons. |
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
a70d6a0 to
998203f
Compare
See rendered design doc. For broader context see HIP PR: #1112
This patch replaces stack based communication with function calls transported over virtual queues. The documentation covers two current limitations: snapshots do not support retained buffers, and the host owns the transport arena. Addressing both is deferred to future work.