Merge branch 'main' into vmclock-snapsafe

Author: bchalios

src/vmm/src/arch/x86_64/vcpu.rs

@@ -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!(
                     "{:?}",

src/vmm/src/lib.rs

@@ -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);
     }
@@ -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);

src/vmm/src/vstate/vcpu.rs

@@ -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")]
@@ -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);
@@ -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) => {
@@ -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,