Skip to content

Commit 5cfdf92

Browse files
ppennaCopilot
andcommitted
openvmm_core: instantiate only the requested VP prefix on MSHV restore
The CLI --restore-processors option and the TTRPC restore processor count pass an explicit restore-time activation target to the VM worker as restore_vp_count, which is documented to make MSHV create and bind only VPs 0..N-1. Nothing read the field, so every MSHV restore created, restored, and ran the full VP capacity of the snapshot, as KVM and WHP do. The guest still onlined exactly N CPUs, so only the host-side saving was lost. This change belongs in fdefb05 ("openvmm: bring a restore-time processor prefix online"). That commit documented that an explicit MSHV restore target instantiates and binds only the VP prefix, while the worker still built the partition unit with the active_vp_count of None that 920ab57 ("vmm_core: instantiate an active VP prefix") set. The gRPC reference update belongs in b4eef6f ("vmservice: create, capture, and restore microVMs over the management RPC"), which added restore_processor_count. Take the target with the other restore inputs and keep it only for MSHV, which creates application processors lazily while binding them. Before any VP is bound, check the target against the topology capacity, discard the binders after the prefix, and pass the prefix to the partition unit, which validates the complete saved VP inventory before restoring only the prefix and rejects saving the reduced runtime. KVM, WHP, and untargeted MSHV restores keep the full capacity. This relies on the previous commit, which limits restored-TSC synchronization to the created VPs. Emit the startup.vp_thread_bind, startup.vp_bind_bsp, and startup.vp_bind_ap_<INDEX> profile phases that NVX documents, so lifecycle profiling shows which VPs a restore binds. On a bare-metal MSHV host, restoring a capacity-8 snapshot now binds exactly N VPs, and worker construction drops from about 97 ms to about 11 ms for one or two VPs. Unit tests cover the MSHV-only selection, discarding the suffix binders, keeping every binder without a target, and rejecting an out-of-range prefix, a prefix without saved state, and a backend that returns fewer binders than the capacity. The worker parameter documentation and the gRPC reference describe the backend-specific behavior. See microsoft/nvx#219. Fixes: fdefb05 ("openvmm: bring a restore-time processor prefix online") Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c57413b commit 5cfdf92

4 files changed

Lines changed: 190 additions & 17 deletions

File tree

‎Guide/src/reference/openvmm/management/grpc.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,9 @@ restore flow over both transports:
146146
valid only for a snapshot that advertises processor activation, implies fresh entropy and the
147147
post-restore gate, and must satisfy the snapshot's boot-online and immutable
148148
capacity bounds. `ProcessorConfig.processor_count`, when present, remains an
149-
exact capacity assertion.
149+
exact capacity assertion. On MSHV, a nonzero value instantiates and binds
150+
only that VP prefix, as for `--restore-processors`; MSHV restores without a
151+
target, and all KVM and WHP restores, instantiate the full VP capacity.
150152
* `restore_memory_bytes` selects a 128-MiB-aligned total RAM target between the
151153
snapshot base and immutable capacity. Zero selects the base. Expansion uses
152154
fresh private zeroed backing, implies fresh restore packet delivery and the

‎openvmm/openvmm_core/src/worker/dispatch.rs‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,7 +1566,7 @@ impl InitializedVm {
15661566

15671567
let Self {
15681568
partition,
1569-
vps,
1569+
mut vps,
15701570
vmtime_keeper,
15711571
vmtime_source,
15721572
memory_manager,
@@ -1583,6 +1583,12 @@ impl InitializedVm {
15831583
driver_source,
15841584
} = self;
15851585

1586+
let instantiated_vp_count = snapshot_restore.select_instantiated_vps(
1587+
&mut vps,
1588+
processor_topology.vp_count(),
1589+
saved_state.is_some(),
1590+
)?;
1591+
15861592
let mut resolver = ResourceResolver::new();
15871593

15881594
// Expose the partition reference time source, if available.
@@ -3109,7 +3115,7 @@ impl InitializedVm {
31093115
partition.clone().into_vm_partition(),
31103116
PartitionUnitParams {
31113117
processor_topology: &processor_topology,
3112-
active_vp_count: None,
3118+
active_vp_count: Some(instantiated_vp_count),
31133119
halt_vps,
31143120
halt_request_recv,
31153121
client_notify_send: halt_send,
@@ -3124,24 +3130,35 @@ impl InitializedVm {
31243130
.context("failed to create partition unit")?;
31253131

31263132
// Start the VP backing threads.
3133+
assert_eq!(
3134+
vps.len(),
3135+
vp_runners.len(),
3136+
"every instantiated VP needs exactly one binder and one runner"
3137+
);
3138+
let vp_thread_bind = openvmm_defs::profile::ProfileSpan::start();
31273139
try_join_all(vps.into_iter().zip(vp_runners).enumerate().map(
31283140
|(vp_index, (mut vp, runner))| {
31293141
let partition = partition.clone();
31303142
let chipset = chipset.clone();
31313143
let (send, recv) = mesh::oneshot();
31323144
thread::Builder::new()
31333145
.name(format!("vp-{}", vp_index))
3134-
.spawn(move || match vp.bind() {
3135-
Ok(mut vp) => {
3136-
send.send(Ok(()));
3137-
block_on_vp(
3138-
partition,
3139-
VpIndex::new(vp_index as u32),
3140-
vp.run(runner, &chipset),
3141-
)
3142-
}
3143-
Err(err) => {
3144-
send.send(Err(err));
3146+
.spawn(move || {
3147+
let vp_bind = openvmm_defs::profile::ProfileSpan::start();
3148+
let bound = vp.bind();
3149+
complete_vp_bind_profile(vp_bind, vp_index);
3150+
match bound {
3151+
Ok(mut vp) => {
3152+
send.send(Ok(()));
3153+
block_on_vp(
3154+
partition,
3155+
VpIndex::new(vp_index as u32),
3156+
vp.run(runner, &chipset),
3157+
)
3158+
}
3159+
Err(err) => {
3160+
send.send(Err(err));
3161+
}
31453162
}
31463163
})
31473164
.unwrap();
@@ -3154,6 +3171,7 @@ impl InitializedVm {
31543171
},
31553172
))
31563173
.await?;
3174+
vp_thread_bind.complete("startup", "vp_thread_bind", Default::default());
31573175

31583176
let mut this = LoadedVm {
31593177
state_units,
@@ -3235,6 +3253,19 @@ impl InitializedVm {
32353253
}
32363254
}
32373255

3256+
/// Emits the non-exclusive profile milestone of binding VP `vp_index`.
3257+
fn complete_vp_bind_profile(span: openvmm_defs::profile::ProfileSpan, vp_index: usize) {
3258+
if vp_index == 0 {
3259+
span.complete_milestone("startup", "vp_bind_bsp", Default::default());
3260+
} else if openvmm_defs::profile::enabled() {
3261+
span.complete_milestone(
3262+
"startup",
3263+
&format!("vp_bind_ap_{vp_index}"),
3264+
Default::default(),
3265+
);
3266+
}
3267+
}
3268+
32383269
impl LoadedVmInner {
32393270
fn slit_info(&self) -> Option<SlitInfo> {
32403271
let num_nodes = self.numa_cfg.nodes.len();

‎openvmm/openvmm_core/src/worker/dispatch/restore.rs‎

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,18 @@
22
// Licensed under the MIT License.
33

44
//! Snapshot restore support for the VM worker: the restore inputs taken from
5-
//! the worker parameters, the restore state kept by [`LoadedVm`], the steps
6-
//! around applying the saved state, and the post-restore start sequence
5+
//! the worker parameters, the restore state kept by [`LoadedVm`], the VP
6+
//! prefix instantiated by an explicit restore-time activation target, the
7+
//! steps around applying the saved state, and the post-restore start sequence
78
//! (input gate, readiness event, and VP release).
89
910
use super::LoadedVm;
1011
use super::clock;
1112
use super::clock::RestoreTime;
1213
use crate::partition::HvlitePartition;
1314
use anyhow::Context;
15+
use hypervisor_resources::HypervisorKind;
16+
use hypervisor_resources::MshvHandle;
1417
use membacking::FileMappingMode;
1518
use membacking::SharedMemoryBacking;
1619
use openvmm_defs::profile::ProfileSpan;
@@ -27,6 +30,7 @@ use std::fs::File;
2730
use std::future::Future;
2831
use std::io::Write as _;
2932
use std::time::Duration;
33+
use vm_resource::ResourceId;
3034
use vmm_core::partition_unit::StopGuard;
3135

3236
/// Snapshot-restore inputs taken from the [`VmWorkerParameters`].
@@ -54,6 +58,13 @@ impl RestoreParameters {
5458
parameters.restore_apic_frequency_hz,
5559
)?;
5660
tracing::debug!(?restore_time, "received snapshot restore time contract");
61+
let restore_vp_count = parameters.restore_vp_count.take();
62+
let vp_prefix = restore_vp_prefix(parameters.hypervisor.id(), restore_vp_count);
63+
tracing::debug!(
64+
?restore_vp_count,
65+
?vp_prefix,
66+
"received restore-time VP activation target"
67+
);
5768
let cpu_contract = parameters.restore_cpu_contract.take();
5869
let file_mapping_mode = if parameters.shared_memory_copy_on_write {
5970
FileMappingMode::CopyOnWrite
@@ -63,6 +74,7 @@ impl RestoreParameters {
6374
Ok(Self {
6475
state: SnapshotRestore {
6576
time: restore_time,
77+
vp_prefix,
6678
ready_sink,
6779
gate_timeout,
6880
..Default::default()
@@ -79,11 +91,52 @@ impl RestoreParameters {
7991
}
8092
}
8193

94+
/// Returns the VP prefix that a restore instantiates, or `None` for the full
95+
/// VP capacity.
96+
///
97+
/// Only MSHV instantiates an explicit restore-time activation target: it
98+
/// creates application processors lazily while binding them, so discarding
99+
/// the suffix binders avoids creating VPs that the restore will not run.
100+
/// Other backends create their VPs with the partition and keep the full
101+
/// capacity.
102+
fn restore_vp_prefix(hypervisor: &str, restore_vp_count: Option<u32>) -> Option<u32> {
103+
restore_vp_count.filter(|_| hypervisor == <MshvHandle as ResourceId<HypervisorKind>>::ID)
104+
}
105+
106+
/// Discards the binders after an explicit VP prefix and returns the number
107+
/// of VPs to instantiate. The topology and saved VP inventory keep the full
108+
/// capacity.
109+
fn select_instantiated_vps<T>(
110+
vps: &mut Vec<T>,
111+
vp_capacity: u32,
112+
vp_prefix: Option<u32>,
113+
restoring: bool,
114+
) -> anyhow::Result<u32> {
115+
anyhow::ensure!(
116+
vps.len() == vp_capacity as usize,
117+
"backend returned {} VP binders for topology capacity {vp_capacity}",
118+
vps.len()
119+
);
120+
let Some(vp_prefix) = vp_prefix else {
121+
return Ok(vp_capacity);
122+
};
123+
anyhow::ensure!(restoring, "a restore VP prefix requires saved state");
124+
anyhow::ensure!(
125+
(1..=vp_capacity).contains(&vp_prefix),
126+
"restore VP prefix {vp_prefix} is outside topology capacity 1..={vp_capacity}"
127+
);
128+
vps.truncate(vp_prefix as usize);
129+
Ok(vp_prefix)
130+
}
131+
82132
/// Snapshot-restore state of a [`LoadedVm`].
83133
#[derive(Default)]
84134
pub(super) struct SnapshotRestore {
85135
/// Saved guest-clock contract, checked and applied around the restore.
86136
time: Option<RestoreTime>,
137+
/// VP prefix instantiated for an explicit MSHV restore-time activation
138+
/// target.
139+
vp_prefix: Option<u32>,
87140
/// Whether the VM was loaded from saved state.
88141
restored_from_snapshot: bool,
89142
/// Holds the VPs stopped after a restore until the VM first starts.
@@ -101,6 +154,17 @@ pub(super) struct SnapshotRestore {
101154
}
102155

103156
impl SnapshotRestore {
157+
/// Discards the binders after the restore VP prefix, if any, before
158+
/// binding creates their VPs. Returns the number of VPs to instantiate.
159+
pub(super) fn select_instantiated_vps<T>(
160+
&self,
161+
vps: &mut Vec<T>,
162+
vp_capacity: u32,
163+
restoring: bool,
164+
) -> anyhow::Result<u32> {
165+
select_instantiated_vps(vps, vp_capacity, self.vp_prefix, restoring)
166+
}
167+
104168
/// Returns a future that completes when the armed post-restore input gate
105169
/// expires, and never completes while the gate is not armed.
106170
pub(super) fn gate_expired(&self, driver: &impl Driver) -> impl Future<Output = ()> {
@@ -260,3 +324,78 @@ impl LoadedVm {
260324
}
261325
}
262326
}
327+
328+
#[cfg(test)]
329+
mod tests {
330+
use super::*;
331+
use test_with_tracing::test;
332+
333+
fn binders() -> Vec<u32> {
334+
(0..8).collect()
335+
}
336+
337+
#[test]
338+
fn limits_only_explicit_mshv_restores() {
339+
assert_eq!(restore_vp_prefix("mshv", Some(2)), Some(2));
340+
assert_eq!(restore_vp_prefix("mshv", None), None);
341+
for hypervisor in ["kvm", "whp", "hvf"] {
342+
assert_eq!(restore_vp_prefix(hypervisor, Some(2)), None);
343+
assert_eq!(restore_vp_prefix(hypervisor, None), None);
344+
}
345+
}
346+
347+
#[test]
348+
fn restore_vp_prefix_discards_suffix_binders() {
349+
for prefix in [1, 2, 4, 8] {
350+
let mut vps = binders();
351+
assert_eq!(
352+
select_instantiated_vps(&mut vps, 8, Some(prefix), true).unwrap(),
353+
prefix
354+
);
355+
assert_eq!(vps, (0..prefix).collect::<Vec<_>>());
356+
}
357+
}
358+
359+
#[test]
360+
fn untargeted_loads_keep_every_binder() {
361+
for restoring in [false, true] {
362+
let mut vps = binders();
363+
assert_eq!(
364+
select_instantiated_vps(&mut vps, 8, None, restoring).unwrap(),
365+
8
366+
);
367+
assert_eq!(vps, binders());
368+
}
369+
}
370+
371+
#[test]
372+
fn rejects_invalid_restore_vp_prefix() {
373+
for prefix in [0, 9] {
374+
let mut vps = binders();
375+
let error = select_instantiated_vps(&mut vps, 8, Some(prefix), true).unwrap_err();
376+
assert_eq!(
377+
error.to_string(),
378+
format!("restore VP prefix {prefix} is outside topology capacity 1..=8")
379+
);
380+
assert_eq!(vps, binders());
381+
}
382+
383+
let mut vps = binders();
384+
let error = select_instantiated_vps(&mut vps, 8, Some(2), false).unwrap_err();
385+
assert_eq!(
386+
error.to_string(),
387+
"a restore VP prefix requires saved state"
388+
);
389+
assert_eq!(vps, binders());
390+
}
391+
392+
#[test]
393+
fn rejects_a_partial_backend_binder_set() {
394+
let mut vps = (0..4).collect::<Vec<u32>>();
395+
let error = select_instantiated_vps(&mut vps, 8, None, true).unwrap_err();
396+
assert_eq!(
397+
error.to_string(),
398+
"backend returned 4 VP binders for topology capacity 8"
399+
);
400+
}
401+
}

‎openvmm/openvmm_defs/src/worker.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ pub struct VmWorkerParameters {
8787
pub restore_ready_sink: Option<std::fs::File>,
8888
/// Timeout for the microVM post-restore input gate, when required.
8989
pub restore_gate_timeout: Option<std::time::Duration>,
90-
/// MSHV VP prefix instantiated for an explicit restore-time activation target.
90+
/// Explicit restore-time activation target. MSHV instantiates and binds
91+
/// only VPs `0..N-1`; other backends instantiate the full VP capacity.
9192
pub restore_vp_count: Option<u32>,
9293
/// The VM RPC channel.
9394
pub rpc: mesh::Receiver<VmRpc>,

0 commit comments

Comments
 (0)