Repository navigation
Conversation
danbugs
added
kind/enhancement
There was a problem hiding this comment.
🔵 Needs a closer look
Memory-layout and ABI changes need cross-platform human validation, and a host-page-dependent test failure remains unresolved.
1 open issue
What changed in this PR
Enables snapshots with smaller scratch regions to reduce restore costs while preserving transport addresses.
Changes:
- Adds scratch-size selection and validation for snapshots.
- Fixes transport placement and bounds guest allocation below it.
- Updates snapshot ABI, tests, and documentation.
| File | Description |
|---|---|
| src/hyperlight_host/tests/snapshot_goldens/goldens_version.rs | Selects v7.0 goldens. |
| src/hyperlight_host/src/sandbox/snapshot/tripwires.rs | Pins ABI 7. |
| src/hyperlight_host/src/sandbox/snapshot/mod.rs | Builds snapshots with target layouts. |
| src/hyperlight_host/src/sandbox/snapshot/file/mod.rs | Validates host-page alignment. |
| src/hyperlight_host/src/sandbox/snapshot/file/media_types.rs | Sets ABI 7. |
| src/hyperlight_host/src/sandbox/snapshot/file_tests.rs | Tests alignment and ABI rejection. |
| src/hyperlight_host/src/sandbox/initialized.rs | Adds resizing API and runtime tests. |
| src/hyperlight_host/src/mem/virtq/tests.rs | Updates transport address tests. |
| src/hyperlight_host/src/mem/virtq/mod.rs | Supports resized transport snapshots. |
| src/hyperlight_host/src/mem/mgr.rs | Passes target layouts and mailbox offsets. |
| src/hyperlight_host/src/mem/layout.rs | Defines fixed transport placement and sizing. |
| src/hyperlight_guest/src/transport/mem.rs | Bounds memory access to transport ranges. |
| src/hyperlight_guest/src/transport/context.rs | Uses scratch-independent transport bounds. |
| src/hyperlight_guest/src/layout.rs | Defines the allocator’s transport boundary. |
| src/hyperlight_guest/src/arch/amd64/prim_alloc.rs | Applies the allocator boundary. |
| src/hyperlight_guest/src/arch/aarch64/prim_alloc.rs | Applies the allocator boundary. |
| src/hyperlight_common/src/layout.rs | Defines transport placement helpers. |
| docs/virtio-host-guest-communication.md | Documents transport placement. |
| docs/snapshot-versioning.md | Documents ABI 7 compatibility. |
| docs/paging-development-notes.md | Explains resizing and allocation limits. |
| CHANGELOG.md | Records the API and breaking changes. |
Review effort: Balanced
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hey! Numbers look great. Could you explain why transport needs to be moved? Would it be possible to only move pools out of arena and reserve the top region for g2h/h2g pools only? This way we could remove all the knowledge about the pool placement from the host which would simplify the code quite a bit. Currently the pools need to be pinned in scratch memory between restores. This is a bit annoying but so far it has been the only way to avoid an extra round trip on restore. This is because host request can already be queued on restore in h2g queue. IIUC, this patch doesn't change this behavior? |
Thanks! Currently, the transport arena location is calculated once at boot and cached. With scratch resizing, those values end up wrong (since the base depends on the scratch size) and would have to be recalculated. If we move the transport arena just beneath the reserved pages, the remembered addresses stay valid. We could move only the pools, which would also let the host drop the pool placement logic, but then the guest would have to re-learn the ring and mailbox addresses after a resize. I figured moving the whole arena avoids that extra work... And right, this doesn't change the pool pinning: pools and rings keep their addresses across restores, including at a different scratch size, so host requests can still be queued in the H2G queue before the guest resumes. |
syntactically
left a comment
syntactically
left a comment
There was a problem hiding this comment.
I think I generally agree with @andreiltd that the degree of knowledge of the layout in the host here is a little bit unpleasant. It would be nicer in the medium term to change things so that there is less agreement needed here---either the host does not need to know so much, or the host changes to fully understand arbitrary layouts set up by the guest. However, from my side I don't think I see doing something about that as a blocker that prevents merging this PR. (Unless you disagree @andreiltd ?)
0e23972 to
a0c5b73
Compare
This comment has been minimized.
This comment has been minimized.
a0c5b73 to
362eb1d
Compare
danbugs
changed the title
This comment has been minimized.
This comment has been minimized.
ludfjig
left a comment
ludfjig
left a comment
There was a problem hiding this comment.
Looks good! Main open point is the API shape. Rather than one snapshot_with_* method per setting, I'd suggest snapshot_with_options(&SnapshotOptions), with snapshot() calling it with defaults:
#[derive(Clone, Debug, Default)]
pub struct SnapshotOptions {
scratch_size: Option<usize>, // None keeps the sandbox's size
}
let snap = sbox.snapshot_with_options(&SnapshotOptions::default().scratch_size(8 << 20))?;This will let us add more settings without having many different methods. Thoughts?
043378c to
98ac01d
Compare
This comment has been minimized.
This comment has been minimized.
I liked the idea and had a go at it in 98ac01d. |
The guest builds its transport context once and keeps the ring addresses. For scratch to change size across a snapshot, the transport arena (virtqueue rings, pools and the checkpoint mailbox) needs an address that does not depend on the scratch size. The arena ends one guard page below the reserved top pages. The page tables start at the scratch base. The guard page is left unmapped, so an amd64 exception stack overrun faults there before it reaches the arena. aarch64 exception vectors store below SP before any check, so nested faults there can step past it. The guest's page allocator stops at the arena GPA the host publishes, and aborts if that is not between the scratch base and the end of the transport range. The guest's transport memory access is bound to the arena, which must start above the scratch base. scratch_allocator_limit_gpa is renamed scratch_reserved_top_gpa, so a guest that stops its allocator at the old limit fails to compile. The minimum scratch size grows by the guard page. This changes the snapshot layout: snapshot ABI 7, goldens v7.0. Existing snapshots must be regenerated, and guests rebuilt. Signed-off-by: danbugs <[email protected]>
Sandbox::snapshot_with_options takes a snapshot with SnapshotOptions, which leaves room for more options later. Sandbox::snapshot is the default-options case. SnapshotOptions::scratch_size gives the snapshot's layout another scratch size. The snapshot's page tables are built for that size, walking the live tables with the sandbox's own layout. Its virtqueue rings carry over unchanged, since the transport arena's address does not depend on the scratch size. Restore replaces scratch when the size differs. A size above the maximum memory size, too small for the transport, or not a whole number of host pages is rejected before the guest is entered. Room for the page tables is checked when they are built. A larger scratch window covers guest-virtual addresses below the current one, so building the snapshot fails if the guest has a mapping there. The snapshot loader refuses a scratch size this host cannot map. A guest can boot with the scratch its initialisation needs and run guest function calls from a snapshot with only the scratch they need, or with more scratch than it booted with. Less scratch makes restores that zero or replace all of it cost less. Signed-off-by: danbugs <[email protected]>
98ac01d to
991129c
Compare
Benchmark ResultsMeasured commit: kvm / amd (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
kvm / intel (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
mshv3 / amd (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
mshv3 / intel (Linux) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
hyperv-ws2025 / amd (Windows) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
hyperv-ws2025 / intel (Windows) (➖ stable)No benchmark improved or regressed. Benchmark Resultsfunction_call_codec
payload_allocation
sandboxes
slot_pool
snapshot_files
virtq_readonly
virtq_readwrite
Reported by |
Sandbox::snapshot_with_optionstakes a snapshot withSnapshotOptions, andSnapshotOptions::scratch_sizemakes it restore with a different scratch size. A guest boots with the scratch its initialisation needs and runs guest function calls from a snapshot with only what they need, so each restore resets less memory, or with more scratch than it booted with. Follows the discussion on #1901.SnapshotOptionsleaves room for more snapshot options.Sandbox::snapshotis the default-options case.snapshot_with_optionsrejects a size above the maximum memory size, too small for the transport, or not whole host pages, before the guest is entered. The snapshot loader refuses a scratch size the host cannot map.Addresses #1767