refactor: Store msrs_to_save in struct Vm instead of Kvm

This allows us to not need to feed `struct Kvm` all the way down to Vcpu
creation.

While we're at it, abstract away the `MsrList` type, and just have a
helper that returns a slice of u32 (as every call site converts the
MsrList to this anyway).

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

Author: roypat

src/vmm/src/builder.rs

@@ -192,7 +192,7 @@ fn create_vmm_and_vcpus(
     #[cfg(target_arch = "x86_64")]
     let (vcpus, pio_device_manager) = {
         setup_interrupt_controller(&mut vm)?;
-        let vcpus = create_vcpus(&kvm, &vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
+        let vcpus = create_vcpus(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
 
         // Make stdout non blocking.
         set_stdout_nonblocking();
@@ -224,7 +224,7 @@ fn create_vmm_and_vcpus(
     // Search for `kvm_arch_vcpu_create` in arch/arm/kvm/arm.c.
     #[cfg(target_arch = "aarch64")]
     let vcpus = {
-        let vcpus = create_vcpus(&kvm, &vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
+        let vcpus = create_vcpus(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
         setup_interrupt_controller(&mut vm, vcpu_count)?;
         vcpus
     };
@@ -746,16 +746,11 @@ fn attach_legacy_devices_aarch64(
         .map_err(VmmError::RegisterMMIODevice)
 }
 
-fn create_vcpus(
-    kvm: &Kvm,
-    vm: &Vm,
-    vcpu_count: u8,
-    exit_evt: &EventFd,
-) -> Result<Vec<Vcpu>, VmmError> {
+fn create_vcpus(vm: &Vm, vcpu_count: u8, exit_evt: &EventFd) -> Result<Vec<Vcpu>, VmmError> {
     let mut vcpus = Vec::with_capacity(vcpu_count as usize);
     for cpu_idx in 0..vcpu_count {
         let exit_evt = exit_evt.try_clone().map_err(VmmError::EventFd)?;
-        let vcpu = Vcpu::new(cpu_idx, vm, kvm, exit_evt).map_err(VmmError::VcpuCreate)?;
+        let vcpu = Vcpu::new(cpu_idx, vm, exit_evt).map_err(VmmError::VcpuCreate)?;
         vcpus.push(vcpu);
     }
     Ok(vcpus)
@@ -1164,7 +1159,7 @@ pub(crate) mod tests {
         #[cfg(target_arch = "aarch64")]
         {
             let exit_evt = EventFd::new(libc::EFD_NONBLOCK).unwrap();
-            let _vcpu = Vcpu::new(1, &vm, &kvm, exit_evt).unwrap();
+            let _vcpu = Vcpu::new(1, &vm, exit_evt).unwrap();
             setup_interrupt_controller(&mut vm, 1).unwrap();
         }
 
@@ -1399,7 +1394,7 @@ pub(crate) mod tests {
         #[cfg(target_arch = "x86_64")]
         setup_interrupt_controller(&mut vm).unwrap();
 
-        let vcpu_vec = create_vcpus(&kvm, &vm, vcpu_count, &evfd).unwrap();
+        let vcpu_vec = create_vcpus(&vm, vcpu_count, &evfd).unwrap();
         assert_eq!(vcpu_vec.len(), vcpu_count as usize);
     }
 

src/vmm/src/vstate/kvm.rs

@@ -23,9 +23,6 @@ pub enum KvmError {
     configured on the /dev/kvm file's ACL. */
     Kvm(kvm_ioctls::Error),
     #[cfg(target_arch = "x86_64")]
-    /// Failed to get MSR index list to save into snapshots: {0}
-    GetMsrsToSave(crate::arch::x86_64::msr::MsrError),
-    #[cfg(target_arch = "x86_64")]
     /// Failed to get supported cpuid: {0}
     GetSupportedCpuId(kvm_ioctls::Error),
     /// The number of configured slots is bigger than the maximum reported by KVM
@@ -45,9 +42,6 @@ pub struct Kvm {
     #[cfg(target_arch = "x86_64")]
     /// Supported CpuIds.
     pub supported_cpuid: CpuId,
-    #[cfg(target_arch = "x86_64")]
-    /// Msrs needed to be saved on snapshot creation.
-    pub msrs_to_save: MsrList,
 }
 
 impl Kvm {
@@ -82,19 +76,22 @@ impl Kvm {
             let supported_cpuid = kvm_fd
                 .get_supported_cpuid(KVM_MAX_CPUID_ENTRIES)
                 .map_err(KvmError::GetSupportedCpuId)?;
-            let msrs_to_save = crate::arch::x86_64::msr::get_msrs_to_save(&kvm_fd)
-                .map_err(KvmError::GetMsrsToSave)?;
 
             Ok(Kvm {
                 fd: kvm_fd,
                 max_memslots,
                 kvm_cap_modifiers,
                 supported_cpuid,
-                msrs_to_save,
             })
         }
     }
 
+    /// Msrs needed to be saved on snapshot creation.
+    #[cfg(target_arch = "x86_64")]
+    pub fn msrs_to_save(&self) -> Result<MsrList, crate::arch::x86_64::msr::MsrError> {
+        crate::arch::x86_64::msr::get_msrs_to_save(&self.fd)
+    }
+
     /// Check guest memory does not have more regions than kvm allows.
     pub fn check_memory(&self, guest_mem: &GuestMemoryMmap) -> Result<(), KvmError> {
         if guest_mem.num_regions() > self.max_memslots {

src/vmm/src/vstate/vcpu/aarch64.rs

@@ -22,7 +22,7 @@ use crate::cpu_config::aarch64::custom_cpu_template::VcpuFeatures;
 use crate::cpu_config::templates::CpuConfiguration;
 use crate::logger::{error, IncMetric, METRICS};
 use crate::vcpu::{VcpuConfig, VcpuError};
-use crate::vstate::kvm::{Kvm, OptionalCapabilities};
+use crate::vstate::kvm::OptionalCapabilities;
 use crate::vstate::memory::{Address, GuestAddress, GuestMemoryMmap};
 use crate::vstate::vcpu::VcpuEmulation;
 use crate::vstate::vm::Vm;
@@ -78,7 +78,7 @@ impl KvmVcpu {
     ///
     /// * `index` - Represents the 0-based CPU index between [0, max vcpus).
     /// * `vm` - The vm to which this vcpu will get attached.
-    pub fn new(index: u8, vm: &Vm, _: &Kvm) -> Result<Self, KvmVcpuError> {
+    pub fn new(index: u8, vm: &Vm) -> Result<Self, KvmVcpuError> {
         let kvm_vcpu = vm
             .fd()
             .create_vcpu(index.into())
@@ -315,7 +315,7 @@ mod tests {
 
     fn setup_vcpu(mem_size: usize) -> (Kvm, Vm, KvmVcpu, GuestMemoryMmap) {
         let (kvm, mut vm, vm_mem) = setup_vm_with_memory(mem_size);
-        let mut vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vcpu.init(&[]).unwrap();
         vm.setup_irqchip(1).unwrap();
 
@@ -324,11 +324,11 @@ mod tests {
 
     #[test]
     fn test_create_vcpu() {
-        let (kvm, vm, _) = setup_vm_with_memory(0x1000);
+        let (_, vm, _) = setup_vm_with_memory(0x1000);
 
         unsafe { libc::close(vm.fd().as_raw_fd()) };
 
-        let err = KvmVcpu::new(0, &vm, &kvm);
+        let err = KvmVcpu::new(0, &vm);
         assert_eq!(
             err.err().unwrap().to_string(),
             "Error creating vcpu: Bad file descriptor (os error 9)".to_string()
@@ -378,8 +378,8 @@ mod tests {
 
     #[test]
     fn test_init_vcpu() {
-        let (kvm, mut vm, _) = setup_vm_with_memory(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         // KVM_ARM_VCPU_PSCI_0_2 is set by default.
@@ -397,8 +397,8 @@ mod tests {
 
     #[test]
     fn test_vcpu_save_restore_state() {
-        let (kvm, mut vm, _) = setup_vm_with_memory(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         // Calling KVM_GET_REGLIST before KVM_VCPU_INIT will result in error.
@@ -441,8 +441,8 @@ mod tests {
         //
         // This should fail with ENOEXEC.
         // https://elixir.bootlin.com/linux/v5.10.176/source/arch/arm64/kvm/arm.c#L1165
-        let (kvm, mut vm, _) = setup_vm_with_memory(0x1000);
-        let vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         vcpu.dump_cpu_config().unwrap_err();
@@ -451,8 +451,8 @@ mod tests {
     #[test]
     fn test_dump_cpu_config_after_init() {
         // Test `dump_cpu_config()` after `KVM_VCPU_INIT`.
-        let (kvm, mut vm, _) = setup_vm_with_memory(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
+        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
         vm.setup_irqchip(1).unwrap();
         vcpu.init(&[]).unwrap();
 
@@ -461,10 +461,10 @@ mod tests {
 
     #[test]
     fn test_setup_non_boot_vcpu() {
-        let (kvm, vm, _) = setup_vm_with_memory(0x1000);
-        let mut vcpu1 = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, vm, _) = setup_vm_with_memory(0x1000);
+        let mut vcpu1 = KvmVcpu::new(0, &vm).unwrap();
         vcpu1.init(&[]).unwrap();
-        let mut vcpu2 = KvmVcpu::new(1, &vm, &kvm).unwrap();
+        let mut vcpu2 = KvmVcpu::new(1, &vm).unwrap();
         vcpu2.init(&[]).unwrap();
     }
 

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

@@ -44,8 +44,6 @@ pub use aarch64::{KvmVcpuError, *};
 #[cfg(target_arch = "x86_64")]
 pub use x86_64::{KvmVcpuError, *};
 
-use super::kvm::Kvm;
-
 /// Signal number (SIGRTMIN) used to kick Vcpus.
 pub const VCPU_RTSIG_OFFSET: i32 = 0;
 
@@ -214,10 +212,10 @@ impl Vcpu {
     /// * `index` - Represents the 0-based CPU index between [0, max vcpus).
     /// * `vm` - The vm to which this vcpu will get attached.
     /// * `exit_evt` - An `EventFd` that will be written into when this vcpu exits.
-    pub fn new(index: u8, vm: &Vm, kvm: &Kvm, exit_evt: EventFd) -> Result<Self, VcpuError> {
+    pub fn new(index: u8, vm: &Vm, exit_evt: EventFd) -> Result<Self, VcpuError> {
         let (event_sender, event_receiver) = channel();
         let (response_sender, response_receiver) = channel();
-        let kvm_vcpu = KvmVcpu::new(index, vm, kvm).unwrap();
+        let kvm_vcpu = KvmVcpu::new(index, vm).unwrap();
 
         Ok(Vcpu {
             exit_evt,
@@ -787,6 +785,7 @@ pub(crate) mod tests {
     use crate::devices::BusDevice;
     use crate::seccomp::get_empty_filters;
     use crate::utils::signal::validate_signal_num;
+    use crate::vstate::kvm::Kvm;
     use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
     use crate::vstate::vcpu::VcpuError as EmulationError;
     use crate::vstate::vm::tests::setup_vm_with_memory;
@@ -937,15 +936,15 @@ pub(crate) mod tests {
 
         #[cfg(target_arch = "aarch64")]
         let vcpu = {
-            let mut vcpu = Vcpu::new(1, &vm, &kvm, exit_evt).unwrap();
+            let mut vcpu = Vcpu::new(1, &vm, exit_evt).unwrap();
             vcpu.kvm_vcpu.init(&[]).unwrap();
             vm.setup_irqchip(1).unwrap();
             vcpu
         };
         #[cfg(target_arch = "x86_64")]
         let vcpu = {
             vm.setup_irqchip().unwrap();
-            Vcpu::new(1, &vm, &kvm, exit_evt).unwrap()
+            Vcpu::new(1, &vm, exit_evt).unwrap()
         };
         (kvm, vm, vcpu, gm)
     }

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

@@ -23,7 +23,6 @@ use crate::arch::x86_64::msr::{create_boot_msr_entries, MsrError};
 use crate::arch::x86_64::regs::{SetupFpuError, SetupRegistersError, SetupSpecialRegistersError};
 use crate::cpu_config::x86_64::{cpuid, CpuConfiguration};
 use crate::logger::{IncMetric, METRICS};
-use crate::vstate::kvm::Kvm;
 use crate::vstate::memory::{Address, GuestAddress, GuestMemoryMmap};
 use crate::vstate::vcpu::{VcpuConfig, VcpuEmulation};
 use crate::vstate::vm::Vm;
@@ -165,7 +164,7 @@ impl KvmVcpu {
     ///
     /// * `index` - Represents the 0-based CPU index between [0, max vcpus).
     /// * `vm` - The vm to which this vcpu will get attached.
-    pub fn new(index: u8, vm: &Vm, kvm: &Kvm) -> Result<Self, KvmVcpuError> {
+    pub fn new(index: u8, vm: &Vm) -> Result<Self, KvmVcpuError> {
         let kvm_vcpu = vm
             .fd()
             .create_vcpu(index.into())
@@ -175,7 +174,7 @@ impl KvmVcpu {
             index,
             fd: kvm_vcpu,
             peripherals: Default::default(),
-            msrs_to_save: kvm.msrs_to_save.as_slice().to_vec(),
+            msrs_to_save: vm.msrs_to_save().to_vec(),
         })
     }
 
@@ -726,6 +725,7 @@ mod tests {
         StaticCpuTemplate,
     };
     use crate::cpu_config::x86_64::cpuid::{Cpuid, CpuidEntry, CpuidKey};
+    use crate::vstate::kvm::Kvm;
     use crate::vstate::vm::tests::{setup_vm, setup_vm_with_memory};
     use crate::vstate::vm::Vm;
 
@@ -750,7 +750,7 @@ mod tests {
     fn setup_vcpu(mem_size: usize) -> (Kvm, Vm, KvmVcpu, GuestMemoryMmap) {
         let (kvm, vm, vm_mem) = setup_vm_with_memory(mem_size);
         vm.setup_irqchip().unwrap();
-        let vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let vcpu = KvmVcpu::new(0, &vm).unwrap();
         (kvm, vm, vcpu, vm_mem)
     }
 
@@ -1168,11 +1168,11 @@ mod tests {
     #[test]
     fn test_get_msr_chunks_preserved_order() {
         // Regression test for #4666
-        let (kvm, vm) = setup_vm();
-        let vcpu = KvmVcpu::new(0, &vm, &kvm).unwrap();
+        let (_, vm) = setup_vm();
+        let vcpu = KvmVcpu::new(0, &vm).unwrap();
 
         // The list of supported MSR indices, in the order they were returned by KVM
-        let msrs_to_save = kvm.msrs_to_save;
+        let msrs_to_save = vm.msrs_to_save();
         // The MSRs after processing. The order should be identical to the one returned by KVM, with
         // the exception of deferred MSRs, which should be moved to the end (but show up in the same
         // order as they are listed in [`DEFERRED_MSRS`].
@@ -1185,7 +1185,6 @@ mod tests {
             .flat_map(|chunk| chunk.as_slice().iter())
             .zip(
                 msrs_to_save
-                    .as_slice()
                     .iter()
                     .filter(|&idx| !DEFERRED_MSRS.contains(idx))
                     .chain(DEFERRED_MSRS.iter()),

src/vmm/src/vstate/vm/aarch64.rs

@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
 
 use super::VmError;
 use crate::arch::aarch64::gic::GicState;
-use crate::vstate::kvm::Kvm;
+use crate::Kvm;
 
 /// Structure representing the current architecture's understand of what a "virtual machine" is.
 #[derive(Debug)]

src/vmm/src/vstate/vm/x86_64.rs

@@ -4,12 +4,13 @@
 use std::fmt;
 
 use kvm_bindings::{
-    kvm_clock_data, kvm_irqchip, kvm_pit_config, kvm_pit_state2, KVM_CLOCK_TSC_STABLE,
+    kvm_clock_data, kvm_irqchip, kvm_pit_config, kvm_pit_state2, MsrList, KVM_CLOCK_TSC_STABLE,
     KVM_IRQCHIP_IOAPIC, KVM_IRQCHIP_PIC_MASTER, KVM_IRQCHIP_PIC_SLAVE, KVM_PIT_SPEAKER_DUMMY,
 };
 use kvm_ioctls::VmFd;
 use serde::{Deserialize, Serialize};
 
+use crate::arch::x86_64::msr::MsrError;
 use crate::vstate::vm::VmError;
 
 /// Error type for [`Vm::restore_state`]
@@ -39,19 +40,23 @@ pub enum ArchVmError {
     VmSetClock(kvm_ioctls::Error),
     /// Failed to set KVM vm irqchip: {0}
     VmSetIrqChip(kvm_ioctls::Error),
+    /// Failed to get MSR index list to save into snapshots: {0}
+    GetMsrsToSave(MsrError),
 }
 
 /// Structure representing the current architecture's understand of what a "virtual machine" is.
 #[derive(Debug)]
 pub struct ArchVm {
     pub(super) fd: VmFd,
+    msrs_to_save: MsrList,
 }
 
 impl ArchVm {
     /// Create a new `Vm` struct.
     pub fn new(kvm: &crate::vstate::kvm::Kvm) -> Result<ArchVm, VmError> {
         let fd = kvm.fd.create_vm().map_err(VmError::VmFd)?;
-        Ok(ArchVm { fd })
+        let msrs_to_save = kvm.msrs_to_save().map_err(ArchVmError::GetMsrsToSave)?;
+        Ok(ArchVm { fd, msrs_to_save })
     }
 
     /// Restores the KVM VM state.
@@ -139,6 +144,11 @@ impl ArchVm {
             ioapic,
         })
     }
+
+    /// Gets the list of MSRs to save when creating snapshots
+    pub fn msrs_to_save(&self) -> &[u32] {
+        self.msrs_to_save.as_slice()
+    }
 }
 
 #[derive(Default, Deserialize, Serialize)]