feat: implement virtio based host-guest communication

[andreiltd]

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.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[jsturtevant]

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.

What are the implications? Is there any behavior changes for existing hyperlight consumers?

[andreiltd]

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.

What are the implications? Is there any behavior changes for existing hyperlight consumers?

There is no behavior changes for existing consumers, there is a new parameter type that is zero-copy: ByteChunks. VecBytes  still copies into guest heap memory, so retained values survive snapshots. The limitation applies only to this new type.

Adding snapshot support for retained  ByteChunks  is relatively straightforward. Buffer allocation state lives entirely in the guest, so the guest must publish a manifest listing the addresses the host should capture. The current implementation only detects outstanding  ByteChunks  borrows and returns an error if any exist. That said, the PR is already enormous so I decided to not implement it for this mvp.

The only real change for existing customers is minimum scratch and guest heap size increase as they need to account for virtual queues bookkeeping.

[ludfjig]

I haven't looked very detailed through the code yet:

You mentioned offline that separating ByteChunks into a separate PR without the virtio stuff is hard because ByteChunks dependo on virtio. Would it be possible instead to have the first pr be virtio stuff, and second be the bytechunks (just to make it easier to review)?

It's also my understanding that the new parameter type vec<Bytes> (rather than just Bytes without vec) is necessary for the guest to receive data zerocopy as it's not contiguous memory, but this type is also mirrored in the host api. I guess my question is do we expect it to be a useful for a host to provide multiple Bytes rather than just 1 contiguous one?

It looks like hlbytechunks and hlsizeprefixedbytechunks are not used anymore after recent commits, should they be removed?

[ludfjig]

just fyi I have a draft PR #1728 that will relax this requiement, and use the target snapshot as authority, requiring nothing from the sandbox's layout

[andreiltd]

Would it be possible instead to have the first pr be virtio stuff, and second be the bytechunks (just to make it easier to review)?

Possibly, but I think it would make more sense to do the other way around and maybe accept some temporary stubs for virtio. I would have to have a closer look. I understand this is quite a lot to chew, so maybe it's worth to go through additional effort to split this.

I guess my question is do we expect it to be a useful for a host to provide multiple Bytes rather than just 1 contiguous one?

Bytes would work, yes. although I think it is pretty common to collect Bytes and send it as chunks. For example hyper code (without streams) to read body till EOF could look something like this:

let mut chunks: Vec<Bytes> = Vec::new();

println!("reading request body chunks");
while let Some(next) = req.body_mut().data().await {
    let Ok(chunk) = next else {
        panic!()     
    };
    chunks.push(chunk);
}

It would be slightly annoying to require to make it contiguous before transport. To be fair I think there is also a bit surprising fact about this API, that there is no guarantee that chunking as presented on the sender side will survive transport with the same boundaries.

It looks like hlbytechunks and hlsizeprefixedbytechunks are not used anymore after recent commits, should they be removed?

Sure!

[andreiltd]

For visibility, here is the wip branch that changes the ownership of pools from host owned to guest owned, that could potentially land together with the changes in this PR:

andreiltd@18824d4

[syntactically]

I understand this is quite a lot to chew, so maybe it's worth to go through additional effort to split this.

If it's not too much effort, I think it would be useful to consider.

It would be slightly annoying to require to make it contiguous before transport

I agree; I think it's important that this support multiple input slices to get the ability to build single-copy higher-level parameter serialisation on it. (See page 5 of my serialisation design doc)

Bytes

For the host->guest direction, is there a reason to prefer bytes::Bytes to &[u8]? I think since we copy into the guest memory during the call procedure, any accesses should be nicely scoped?

[andreiltd]

I will close this one in favor of stacked PR: #1794. This PR cannot be reused in the stack because the base commit live in forks, which apparently is not supported in stacked PRs.