Skip to content

refactor: Eliminate double call to Vmm::stop() AND fix(vcpu): Don't treat KVM_EXIT_{SHUTDOWN,HLT} as successful termination #5611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ilstam merged 2 commits into from
Jan 9, 2026
+24 −85
Merged

refactor: Eliminate double call to Vmm::stop() AND fix(vcpu): Don't treat KVM_EXIT_{SHUTDOWN,HLT} as successful termination #5611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
673001a
refactor: Eliminate double call to Vmm::stop()
ilstam Jan 7, 2026
ad9f5bd
fix(vcpu): Don't treat KVM_EXIT_{SHUTDOWN,HLT} as successful termination
ilstam Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/vmm/src/arch/x86_64/vcpu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -746,8 +746,6 @@ impl Peripherals {
}
unexpected_exit => {
METRICS.vcpu.failures.inc();
// TODO: Are we sure we want to finish running a vcpu upon
// receiving a vm exit that is not necessarily an error?
error!("Unexpected exit reason on vcpu run: {:?}", unexpected_exit);
Err(VcpuError::UnhandledKvmExit(format!(
"{:?}",
Expand Down
62 changes: 11 additions & 51 deletions src/vmm/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -651,39 +651,8 @@ impl Vmm {

/// Signals Vmm to stop and exit.
pub fn stop(&mut self, exit_code: FcExitCode) {
// To avoid cycles, all teardown paths take the following route:
// +------------------------+----------------------------+------------------------+
// | Vmm | Action | Vcpu |
// +------------------------+----------------------------+------------------------+
// 1 | | | vcpu.exit(exit_code) |
// 2 | | | vcpu.exit_evt.write(1) |
// 3 | | <--- EventFd::exit_evt --- | |
// 4 | vmm.stop() | | |
// 5 | | --- VcpuEvent::Finish ---> | |
// 6 | | | StateMachine::finish() |
// 7 | VcpuHandle::join() | | |
// 8 | vmm.shutdown_exit_code becomes Some(exit_code) breaking the main event loop |
// +------------------------+----------------------------+------------------------+
// Vcpu initiated teardown starts from `fn Vcpu::exit()` (step 1).
// Vmm initiated teardown starts from `pub fn Vmm::stop()` (step 4).
// Once `vmm.shutdown_exit_code` becomes `Some(exit_code)`, it is the upper layer's
// responsibility to break main event loop and propagate the exit code value.
info!("Vmm is stopping.");

// We send a "Finish" event. If a VCPU has already exited, this is the only
// message it will accept... but running and paused will take it as well.
// It breaks out of the state machine loop so that the thread can be joined.
for (idx, handle) in self.vcpus_handles.iter_mut().enumerate() {
if let Err(err) = handle.send_event(VcpuEvent::Finish) {
error!("Failed to send VcpuEvent::Finish to vCPU {}: {}", idx, err);
}
}
// The actual thread::join() that runs to release the thread's resource is done in
// the VcpuHandle's Drop trait. We can trigger that to happen now by clearing the
// list of handles. Do it here instead of Vmm::Drop to avoid dependency cycles.
// (Vmm's Drop will also check if this list is empty).
self.vcpus_handles.clear();

// Break the main event loop, propagating the Vmm exit-code.
self.shutdown_exit_code = Some(exit_code);
}
Expand Down Expand Up @@ -722,26 +691,17 @@ fn construct_kvm_mpidrs(vcpu_states: &[VcpuState]) -> Vec<u64> {

impl Drop for Vmm {
fn drop(&mut self) {
// There are two cases when `drop()` is called:
// 1) before the Vmm has been mutexed and subscribed to the event manager, or
// 2) after the Vmm has been registered as a subscriber to the event manager.
//
// The first scenario is bound to happen if an error is raised during
// Vmm creation (for example, during snapshot load), before the Vmm has
// been subscribed to the event manager. If that happens, the `drop()`
// function is called right before propagating the error. In order to
// be able to gracefully exit Firecracker with the correct fault
// message, we need to prepare the Vmm contents for the tear down
// (join the vcpu threads). Explicitly calling `stop()` allows the
// Vmm to be successfully dropped and firecracker to propagate the
// error.
//
// In the second case, before dropping the Vmm object, the event
// manager calls `stop()`, which sends a `Finish` event to the vcpus
// and joins the vcpu threads. The Vmm is dropped after everything is
// ready to be teared down. The line below is a no-op, because the Vmm
// has already been stopped by the event manager at this point.
self.stop(self.shutdown_exit_code.unwrap_or(FcExitCode::Ok));
info!("Killing vCPU threads");

// Send a "Finish" event to the vCPU threads so that they terminate.
for (idx, handle) in self.vcpus_handles.iter_mut().enumerate() {
if let Err(err) = handle.send_event(VcpuEvent::Finish) {
error!("Failed to send VcpuEvent::Finish to vCPU {}: {}", idx, err);
}
}

// Join the vCPU threads by running VcpuHandle::drop().
self.vcpus_handles.clear();

if let Err(err) = std::io::stdin().lock().set_canon_mode() {
warn!("Cannot set canonical mode for the terminal. {:?}", err);
Expand Down
45 changes: 13 additions & 32 deletions src/vmm/src/vstate/vcpu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,9 @@ impl Vcpu {
Ok(VcpuEmulation::Handled) => (),
// Emulation was interrupted, check external events.
Ok(VcpuEmulation::Interrupted) => break,
// If the guest was rebooted or halted:
// - vCPU0 will always exit out of `KVM_RUN` with KVM_EXIT_SHUTDOWN or KVM_EXIT_HLT.
// - the other vCPUs won't ever exit out of `KVM_RUN`, but they won't consume CPU.
// So we pause vCPU0 and send a signal to the emulation thread to stop the VMM.
// The guest requested a SHUTDOWN or RESET. This is ARM
// specific. On x86 the i8042 emulation signals the main thread
// directly without calling Vcpu::exit().
Ok(VcpuEmulation::Stopped) => return self.exit(FcExitCode::Ok),
// If the emulation requests a pause lets do this
#[cfg(feature = "gdb")]
Expand Down Expand Up @@ -362,25 +361,9 @@ impl Vcpu {
}

// Transition to the exited state and finish on command.
// Note that this function isn't called when the guest asks for a CPU
// reset via the i8042 controller on x86.
fn exit(&mut self, exit_code: FcExitCode) -> StateMachine<Self> {
// To avoid cycles, all teardown paths take the following route:
// +------------------------+----------------------------+------------------------+
// | Vmm | Action | Vcpu |
// +------------------------+----------------------------+------------------------+
// 1 | | | vcpu.exit(exit_code) |
// 2 | | | vcpu.exit_evt.write(1) |
// 3 | | <--- EventFd::exit_evt --- | |
// 4 | vmm.stop() | | |
// 5 | | --- VcpuEvent::Finish ---> | |
// 6 | | | StateMachine::finish() |
// 7 | VcpuHandle::join() | | |
// 8 | vmm.shutdown_exit_code becomes Some(exit_code) breaking the main event loop |
// +------------------------+----------------------------+------------------------+
// Vcpu initiated teardown starts from `fn Vcpu::exit()` (step 1).
// Vmm initiated teardown starts from `pub fn Vmm::stop()` (step 4).
// Once `vmm.shutdown_exit_code` becomes `Some(exit_code)`, it is the upper layer's
// responsibility to break main event loop and propagate the exit code value.
// Signal Vmm of Vcpu exit.
if let Err(err) = self.exit_evt.write(1) {
METRICS.vcpu.failures.inc();
error!("Failed signaling vcpu exit event: {}", err);
Expand Down Expand Up @@ -456,14 +439,6 @@ fn handle_kvm_exit(
}
Ok(VcpuEmulation::Handled)
}
VcpuExit::Hlt => {
info!("Received KVM_EXIT_HLT signal");
Ok(VcpuEmulation::Stopped)
}
VcpuExit::Shutdown => {
info!("Received KVM_EXIT_SHUTDOWN signal");
Ok(VcpuEmulation::Stopped)
}
// Documentation specifies that below kvm exits are considered
// errors.
VcpuExit::FailEntry(hardware_entry_failure_reason, cpu) => {
Expand Down Expand Up @@ -713,10 +688,16 @@ pub(crate) mod tests {
fn test_handle_kvm_exit() {
let (_, _, mut vcpu) = setup_vcpu(0x1000);
let res = handle_kvm_exit(&mut vcpu.kvm_vcpu.peripherals, Ok(VcpuExit::Hlt));
assert_eq!(res.unwrap(), VcpuEmulation::Stopped);
assert!(matches!(
res,
Err(EmulationError::UnhandledKvmExit(s)) if s == "Hlt",
));

let res = handle_kvm_exit(&mut vcpu.kvm_vcpu.peripherals, Ok(VcpuExit::Shutdown));
assert_eq!(res.unwrap(), VcpuEmulation::Stopped);
assert!(matches!(
res,
Err(EmulationError::UnhandledKvmExit(s)) if s == "Shutdown",
));

let res = handle_kvm_exit(
&mut vcpu.kvm_vcpu.peripherals,
Expand Down