chore: upgrade kvm-bindings to 0.8.0

Upgrade kvm-bindings to 0.8.0 to make use of upstream serialization
support.

New kvm-bindings no longer derives `Clone` on `kvm_xsave` (since this is
not correct to do for all use cases post Linux 5.17, due to the
structure now having a flexible array members). While Firecracker does
not make use of this flexible array members (according to KVM
documentation, it will only be present if `arch_prctl` is used [1]),
meaning our usage of `Clone` was not problematic, it's easier for now to
just clean up our use-sites of `kvm_xsave::clone` (which was only used
in test code)  - turns out they were either dead code, or should be
replaced with successive calls to `save_state()`.

Additionally, `VcpuFd::run` now takes `self` by mutable reference, which
is the reason for the preceding refactors. The result of `run` holds
references into the `kvm_run` structure contained by `VcpuFd`, meaning
the mutable borrow on `Vcpu::fd` will stay alive until the return value
is dropped. However, if the call to `run` is behind a function call that
takes the entirety of `Vcpu` by mutable reference, this means that we
cannot borrow any other fields of `Vcpu` mutably until we drop the
`VcpuExit` - yet we need to do exactly this to actually handle the
KVM_EXIT. So, instead inline the call to `run` so that the borrow
checker understand we only borrow `Vcpu::fd` instead of the entire
`Vcpu` object, meaning it allows us to access the _other_ fields of the
`Vcpu` struct during handling.

[1]: https://docs.kernel.org/virt/kvm/api.html#kvm-get-xsave2

Signed-off-by: Patrick Roy <[email protected]>

Author: roypat

Cargo.lock

@@ -28,6 +28,3 @@ panic = "abort"
 [profile.release]
 panic = "abort"
 lto = true
-
-[patch.crates-io]
-kvm-bindings = { git = "https://github.com/firecracker-microvm/kvm-bindings", tag = "v0.7.0-2", features = ["fam-wrappers"] }

Cargo.toml

@@ -19,7 +19,7 @@ serde_json = "1.0.116"
 thiserror = "1.0.59"
 
 vmm = { path = "../vmm" }
-vmm-sys-util = "0.12.1"
+vmm-sys-util = { version = "0.12.1", features = ["with-serde"] }
 
 [features]
 tracing = ["log-instrument", "vmm/tracing"]

src/cpu-template-helper/Cargo.toml

@@ -16,7 +16,7 @@ log-instrument = { path = "../log-instrument", optional = true }
 serde = { version = "1.0.198", features = ["derive"] }
 thiserror = "1.0.59"
 vm-memory = { version = "0.14.1", features = ["backend-mmap", "backend-bitmap"] }
-vmm-sys-util = "0.12.1"
+vmm-sys-util = { version = "0.12.1", features = ["with-serde"] }
 
 [dev-dependencies]
 serde_json = "1.0.116"

src/utils/Cargo.toml

@@ -19,8 +19,8 @@ crc64 = "2.0.0"
 derive_more = { version = "0.99.17", default-features = false, features = ["from", "display"] }
 displaydoc = "0.2.4"
 event-manager = "0.4.0"
-kvm-bindings = { version = "0.7.0", features = ["fam-wrappers"] }
-kvm-ioctls = "0.16.0"
+kvm-bindings = { version = "0.8.0", features = ["fam-wrappers", "serde"] }
+kvm-ioctls = "0.17.0"
 lazy_static = "1.4.0"
 libc = "0.2.117"
 linux-loader = "0.11.0"

src/vmm/Cargo.toml

@@ -74,7 +74,7 @@ pub struct VcpuConfig {
 }
 
 // Using this for easier explicit type-casting to help IDEs interpret the code.
-type VcpuCell = Cell<Option<*const Vcpu>>;
+type VcpuCell = Cell<Option<*mut Vcpu>>;
 
 /// Error type for [`Vcpu::start_threaded`].
 #[derive(Debug, derive_more::From, thiserror::Error)]
@@ -112,7 +112,7 @@ impl Vcpu {
             if cell.get().is_some() {
                 return Err(VcpuError::VcpuTlsInit);
             }
-            cell.set(Some(self as *const Vcpu));
+            cell.set(Some(self as *mut Vcpu));
             Ok(())
         })
     }
@@ -128,7 +128,7 @@ impl Vcpu {
         // _before_ running this, then there is nothing we can do.
         Self::TLS_VCPU_PTR.with(|cell: &VcpuCell| {
             if let Some(vcpu_ptr) = cell.get() {
-                if vcpu_ptr == self as *const Vcpu {
+                if vcpu_ptr == self as *mut Vcpu {
                     Self::TLS_VCPU_PTR.with(|cell: &VcpuCell| cell.take());
                     return Ok(());
                 }
@@ -149,13 +149,13 @@ impl Vcpu {
     /// dereferencing from pointer an already borrowed `Vcpu`.
     unsafe fn run_on_thread_local<F>(func: F) -> Result<(), VcpuError>
     where
-        F: FnOnce(&Vcpu),
+        F: FnOnce(&mut Vcpu),
     {
         Self::TLS_VCPU_PTR.with(|cell: &VcpuCell| {
             if let Some(vcpu_ptr) = cell.get() {
                 // Dereferencing here is safe since `TLS_VCPU_PTR` is populated/non-empty,
                 // and it is being cleared on `Vcpu::drop` so there is no dangling pointer.
-                let vcpu_ref: &Vcpu = &*vcpu_ptr;
+                let vcpu_ref = &mut *vcpu_ptr;
                 func(vcpu_ref);
                 Ok(())
             } else {
@@ -1003,7 +1003,7 @@ pub mod tests {
         Vcpu::register_kick_signal_handler();
         let (vm, mut vcpu, _) = setup_vcpu(0x1000);
 
-        let kvm_run =
+        let mut kvm_run =
             kvm_ioctls::KvmRunWrapper::mmap_from_fd(&vcpu.kvm_vcpu.fd, vm.fd().run_size())
                 .expect("cannot mmap kvm-run");
         let success = Arc::new(std::sync::atomic::AtomicBool::new(false));

src/vmm/src/vstate/vcpu/mod.rs

@@ -550,7 +550,7 @@ impl Peripherals {
 }
 
 /// Structure holding VCPU kvm state.
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Serialize, Deserialize)]
 pub struct VcpuState {
     /// CpuId.
     pub cpuid: CpuId,
@@ -866,11 +866,10 @@ mod tests {
         // Test `is_tsc_scaling_required` as if it were on the same
         // CPU model as the one in the snapshot state.
         let (_vm, vcpu, _) = setup_vcpu(0x1000);
-        let orig_state = vcpu.save_state().unwrap();
 
         {
             // The frequency difference is within tolerance.
-            let mut state = orig_state.clone();
+            let mut state = vcpu.save_state().unwrap();
             state.tsc_khz = Some(
                 state.tsc_khz.unwrap()
                     + state.tsc_khz.unwrap() * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR / 2,
@@ -882,7 +881,7 @@ mod tests {
 
         {
             // The frequency difference is over the tolerance.
-            let mut state = orig_state;
+            let mut state = vcpu.save_state().unwrap();
             state.tsc_khz = Some(
                 state.tsc_khz.unwrap()
                     + state.tsc_khz.unwrap() * TSC_KHZ_TOL_NUMERATOR / TSC_KHZ_TOL_DENOMINATOR * 2,

src/vmm/src/vstate/vcpu/x86_64.rs

@@ -280,7 +280,6 @@ fn test_create_and_load_snapshot() {
 #[test]
 fn test_snapshot_load_sanity_checks() {
     use vmm::persist::SnapShotStateSanityCheckError;
-    use vmm::vmm_config::machine_config::MAX_SUPPORTED_VCPUS;
 
     let mut microvm_state = get_microvm_state_from_snapshot();
 
@@ -294,13 +293,6 @@ fn test_snapshot_load_sanity_checks() {
         snapshot_state_sanity_check(&microvm_state),
         Err(SnapShotStateSanityCheckError::NoMemory)
     );
-
-    // Create MAX_SUPPORTED_VCPUS vCPUs starting from 1 vCPU.
-    for _ in 0..MAX_SUPPORTED_VCPUS.ilog2() {
-        microvm_state
-            .vcpu_states
-            .append(&mut microvm_state.vcpu_states.clone());
-    }
 }
 
 fn get_microvm_state_from_snapshot() -> MicrovmState {