fix: Move KVM_KVMCLOCK_CTRL from after pause to before resume

KVM_KVMCLOCK_CTRL ioctl sets `pvclock_set_guest_stopped_request` flag of
`kvm_vcpu_arch` [1]. On the next guest time update, if the flag is set,
KVM ORs in `PVCLOCK_GUEST_STOPPED` and `kvm_setup_guest_pvclock()`
pushes the `hv_clock` into the guest's pvclock page [2]. If the
`hv_clock` has not been written to the guest's pvclock page when taking
a snapshot, it is not saved in the snapshot memory (i.e.
`PVCLOCK_GUEST_STOPPED` isn't set in resumed VMs). So we should call
KVM_KVMCLOCK_CTRL ioctl before resuming a VM rather than after pausing a
VM. That covers both the pause-and-resume case and the
restore-and-resume case.

[1]: https://elixir.bootlin.com/linux/v6.16.3/source/arch/x86/kvm/x86.c#L5734
[2]: https://elixir.bootlin.com/linux/v6.16.3/source/arch/x86/kvm/x86.c#L3286-L3295
Signed-off-by: Takahiro Itazuri <[email protected]>

Author: zulinx86

CHANGELOG.md

@@ -43,6 +43,10 @@ and this project adheres to
   bug causing a read/write from an iovec to be duplicated when receiving an
   error on an iovec other than the first. This caused a data corruption issue in
   the vsock device starting from guest kernel 6.17.
+- [#5494](https://github.com/firecracker-microvm/firecracker/pull/5494): Fixed a
+  bug where soft lockups were detected by watchdog on microVMs restored from
+  snapshots. This moved KVM_KVMCLOCK_CTRL ioctl call, which notifies watchdog
+  that the microVMs were paused, from after pause to before resume.
 
 ## [1.13.0]
 

src/vmm/src/vstate/vcpu.rs

@@ -233,19 +233,7 @@ impl Vcpu {
                 Ok(VcpuEmulation::Stopped) => return self.exit(FcExitCode::Ok),
                 // If the emulation requests a pause lets do this
                 #[cfg(feature = "gdb")]
-                Ok(VcpuEmulation::Paused) => {
-                    // Calling `KVM_KVMCLOCK_CTRL` to make sure the guest softlockup watchdog
-                    // does not panic on resume, see https://docs.kernel.org/virt/kvm/api.html .
-                    // We do not want to fail if the call is not successful, because depending
-                    // that may be acceptable depending on the workload.
-                    #[cfg(target_arch = "x86_64")]
-                    if let Err(err) = self.kvm_vcpu.fd.kvmclock_ctrl() {
-                        METRICS.vcpu.kvmclock_ctrl_fails.inc();
-                        warn!("KVM_KVMCLOCK_CTRL call failed {}", err);
-                    }
-
-                    return StateMachine::next(Self::paused);
-                }
+                Ok(VcpuEmulation::Paused) => return StateMachine::next(Self::paused),
                 // Emulation errors lead to vCPU exit.
                 Err(_) => return self.exit(FcExitCode::GenericError),
             }
@@ -263,16 +251,6 @@ impl Vcpu {
                     .send(VcpuResponse::Paused)
                     .expect("vcpu channel unexpectedly closed");
 
-                // Calling `KVM_KVMCLOCK_CTRL` to make sure the guest softlockup watchdog
-                // does not panic on resume, see https://docs.kernel.org/virt/kvm/api.html .
-                // We do not want to fail if the call is not successful, because depending
-                // that may be acceptable depending on the workload.
-                #[cfg(target_arch = "x86_64")]
-                if let Err(err) = self.kvm_vcpu.fd.kvmclock_ctrl() {
-                    METRICS.vcpu.kvmclock_ctrl_fails.inc();
-                    warn!("KVM_KVMCLOCK_CTRL call failed {}", err);
-                }
-
                 // Move to 'paused' state.
                 state = StateMachine::next(Self::paused);
             }
@@ -322,7 +300,15 @@ impl Vcpu {
                     );
                     self.kvm_vcpu.fd.set_kvm_immediate_exit(0);
                 }
-                // Nothing special to do.
+                // Calling `KVM_KVMCLOCK_CTRL` to make sure the guest softlockup watchdog
+                // does not panic on resume, see https://docs.kernel.org/virt/kvm/api.html .
+                // We do not want to fail if the call is not successful, because depending
+                // that may be acceptable depending on the workload.
+                #[cfg(target_arch = "x86_64")]
+                if let Err(err) = self.kvm_vcpu.fd.kvmclock_ctrl() {
+                    METRICS.vcpu.kvmclock_ctrl_fails.inc();
+                    warn!("KVM_KVMCLOCK_CTRL call failed {}", err);
+                }
                 self.response_sender
                     .send(VcpuResponse::Resumed)
                     .expect("vcpu channel unexpectedly closed");