fix: Call KVM_KVMCLOCK_CTRL not only after pause but also 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 in addition to after
pausing a VM.

[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/arch/x86_64/vcpu.rs

@@ -272,6 +272,19 @@ impl KvmVcpu {
         self.peripherals.pio_bus = Some(pio_bus);
     }
 
+    /// Calls KVM_KVMCLOCK_CTRL to avoid guest soft lockup watchdog panics on resume.
+    /// See https://docs.kernel.org/virt/kvm/api.html .
+    pub fn kvmclock_ctrl(&self) {
+        // We do not want to fail if the call is not successful, because that may be acceptable
+        // depending on the workload. For example, EINVAL is returned if kvm-clock is not
+        // activated (e.g., no-kvmclock is specified in the guest kernel parameter).
+        // https://elixir.bootlin.com/linux/v6.17.5/source/arch/x86/kvm/x86.c#L5736-L5737
+        if let Err(err) = self.fd.kvmclock_ctrl() {
+            METRICS.vcpu.kvmclock_ctrl_fails.inc();
+            warn!("KVM_KVMCLOCK_CTRL call failed {}", err);
+        }
+    }
+
     /// Get the current XSAVE state for this vCPU.
     ///
     /// The C `kvm_xsave` struct was extended by adding a flexible array member (FAM) in the end
@@ -699,6 +712,8 @@ impl KvmVcpu {
         self.fd
             .set_vcpu_events(&state.vcpu_events)
             .map_err(KvmVcpuError::VcpuSetVcpuEvents)?;
+
+        self.kvmclock_ctrl();
         Ok(())
     }
 }

src/vmm/src/vstate/vcpu.rs

@@ -234,18 +234,10 @@ impl Vcpu {
                 // 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);
-                }
+                    self.kvm_vcpu.kvmclock_ctrl();
+                    return StateMachine::next(Self::paused)
+                },
                 // Emulation errors lead to vCPU exit.
                 Err(_) => return self.exit(FcExitCode::GenericError),
             }
@@ -263,15 +255,8 @@ 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);
-                }
+                self.kvm_vcpu.kvmclock_ctrl();
 
                 // Move to 'paused' state.
                 state = StateMachine::next(Self::paused);
@@ -322,7 +307,6 @@ impl Vcpu {
                     );
                     self.kvm_vcpu.fd.set_kvm_immediate_exit(0);
                 }
-                // Nothing special to do.
                 self.response_sender
                     .send(VcpuResponse::Resumed)
                     .expect("vcpu channel unexpectedly closed");