refactor: move KVM related logic into a separate struct

Vm constructor was the only place where the `/dev/kvm` was open
and only there we could do any KVM (not VM) specific checks.
By moving this KVM logic into separate struct we can can do
KVM specific actions (like checking optional KVM capabilities)
without needing to reopen `/dev/kvm` again or storing it in `Vm`
where it does not belong.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/arch/aarch64/vcpu.rs

@@ -233,16 +233,16 @@ pub fn set_mpstate(vcpufd: &VcpuFd, state: kvm_mp_state) -> Result<(), VcpuError
 #[cfg(test)]
 mod tests {
     #![allow(clippy::undocumented_unsafe_blocks)]
-    use kvm_ioctls::Kvm;
 
     use super::*;
     use crate::arch::aarch64::layout;
     use crate::test_utils::arch_mem;
+    use crate::vstate::kvm::Kvm;
 
     #[test]
     fn test_setup_regs() {
-        let kvm = Kvm::new().unwrap();
-        let vm = kvm.create_vm().unwrap();
+        let kvm = Kvm::new(vec![]).unwrap();
+        let vm = kvm.fd.create_vm().unwrap();
         let vcpu = vm.create_vcpu(0).unwrap();
         let mem = arch_mem(layout::FDT_MAX_SIZE + 0x1000);
 
@@ -283,8 +283,8 @@ mod tests {
 
     #[test]
     fn test_read_mpidr() {
-        let kvm = Kvm::new().unwrap();
-        let vm = kvm.create_vm().unwrap();
+        let kvm = Kvm::new(vec![]).unwrap();
+        let vm = kvm.fd.create_vm().unwrap();
         let vcpu = vm.create_vcpu(0).unwrap();
         let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default();
         vm.get_preferred_target(&mut kvi).unwrap();
@@ -302,8 +302,8 @@ mod tests {
 
     #[test]
     fn test_get_set_regs() {
-        let kvm = Kvm::new().unwrap();
-        let vm = kvm.create_vm().unwrap();
+        let kvm = Kvm::new(vec![]).unwrap();
+        let vm = kvm.fd.create_vm().unwrap();
         let vcpu = vm.create_vcpu(0).unwrap();
         let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default();
         vm.get_preferred_target(&mut kvi).unwrap();
@@ -324,8 +324,8 @@ mod tests {
     fn test_mpstate() {
         use std::os::unix::io::AsRawFd;
 
-        let kvm = Kvm::new().unwrap();
-        let vm = kvm.create_vm().unwrap();
+        let kvm = Kvm::new(vec![]).unwrap();
+        let vm = kvm.fd.create_vm().unwrap();
         let vcpu = vm.create_vcpu(0).unwrap();
         let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default();
         vm.get_preferred_target(&mut kvi).unwrap();

src/vmm/src/builder.rs

@@ -68,6 +68,7 @@ use crate::utils::u64_to_usize;
 use crate::vmm_config::boot_source::BootConfig;
 use crate::vmm_config::instance_info::InstanceInfo;
 use crate::vmm_config::machine_config::{VmConfig, VmConfigError};
+use crate::vstate::kvm::Kvm;
 use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
 use crate::vstate::vcpu::{Vcpu, VcpuConfig, VcpuError};
 use crate::vstate::vm::Vm;
@@ -160,11 +161,17 @@ fn create_vmm_and_vcpus(
 ) -> Result<(Vmm, Vec<Vcpu>), StartMicrovmError> {
     use self::StartMicrovmError::*;
 
+    let kvm = Kvm::new(kvm_capabilities)
+        .map_err(VmmError::Kvm)
+        .map_err(StartMicrovmError::Internal)?;
     // Set up Kvm Vm and register memory regions.
     // Build custom CPU config if a custom template is provided.
-    let mut vm = Vm::new(kvm_capabilities)
+    let mut vm = Vm::new(&kvm)
         .map_err(VmmError::Vm)
         .map_err(StartMicrovmError::Internal)?;
+    kvm.check_memory(&guest_memory)
+        .map_err(VmmError::Kvm)
+        .map_err(StartMicrovmError::Internal)?;
     vm.memory_init(&guest_memory, track_dirty_pages)
         .map_err(VmmError::Vm)
         .map_err(StartMicrovmError::Internal)?;
@@ -186,7 +193,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(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
+        let vcpus = create_vcpus(&kvm, &vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
 
         // Make stdout non blocking.
         set_stdout_nonblocking();
@@ -218,7 +225,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(&vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
+        let vcpus = create_vcpus(&kvm, &vm, vcpu_count, &vcpus_exit_evt).map_err(Internal)?;
         setup_interrupt_controller(&mut vm, vcpu_count)?;
         vcpus
     };
@@ -227,6 +234,7 @@ fn create_vmm_and_vcpus(
         events_observer: Some(std::io::stdin()),
         instance_info: instance_info.clone(),
         shutdown_exit_code: None,
+        kvm,
         vm,
         guest_memory,
         uffd,
@@ -473,7 +481,7 @@ pub fn build_microvm_from_snapshot(
         uffd,
         vm_resources.vm_config.track_dirty_pages,
         vm_resources.vm_config.vcpu_count,
-        microvm_state.vm_state.kvm_cap_modifiers.clone(),
+        microvm_state.kvm_state.kvm_cap_modifiers.clone(),
     )?;
 
     #[cfg(target_arch = "x86_64")]
@@ -735,11 +743,16 @@ fn attach_legacy_devices_aarch64(
         .map_err(VmmError::RegisterMMIODevice)
 }
 
-fn create_vcpus(vm: &Vm, vcpu_count: u8, exit_evt: &EventFd) -> Result<Vec<Vcpu>, VmmError> {
+fn create_vcpus(
+    kvm: &Kvm,
+    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, exit_evt).map_err(VmmError::VcpuCreate)?;
+        let vcpu = Vcpu::new(cpu_idx, vm, kvm, exit_evt).map_err(VmmError::VcpuCreate)?;
         vcpus.push(vcpu);
     }
     Ok(vcpus)
@@ -762,7 +775,7 @@ pub fn configure_system_for_boot(
     #[cfg(target_arch = "x86_64")]
     let cpu_config = {
         use crate::cpu_config::x86_64::cpuid;
-        let cpuid = cpuid::Cpuid::try_from(vmm.vm.supported_cpuid().clone())
+        let cpuid = cpuid::Cpuid::try_from(vmm.kvm.supported_cpuid.clone())
             .map_err(GuestConfigError::CpuidFromKvmCpuid)?;
         let msrs = vcpus[0]
             .kvm_vcpu
@@ -1108,7 +1121,8 @@ pub(crate) mod tests {
             .map_err(StartMicrovmError::Internal)
             .unwrap();
 
-        let mut vm = Vm::new(vec![]).unwrap();
+        let kvm = Kvm::new(vec![]).unwrap();
+        let mut vm = Vm::new(&kvm).unwrap();
         vm.memory_init(&guest_memory, false).unwrap();
         let mmio_device_manager = MMIODeviceManager::new();
         let acpi_device_manager = ACPIDeviceManager::new();
@@ -1134,14 +1148,15 @@ pub(crate) mod tests {
         #[cfg(target_arch = "aarch64")]
         {
             let exit_evt = EventFd::new(libc::EFD_NONBLOCK).unwrap();
-            let _vcpu = Vcpu::new(1, &vm, exit_evt).unwrap();
+            let _vcpu = Vcpu::new(1, &vm, &kvm, exit_evt).unwrap();
             setup_interrupt_controller(&mut vm, 1).unwrap();
         }
 
         Vmm {
             events_observer: Some(std::io::stdin()),
             instance_info: InstanceInfo::default(),
             shutdown_exit_code: None,
+            kvm,
             vm,
             guest_memory,
             uffd: None,
@@ -1359,15 +1374,16 @@ pub(crate) mod tests {
         let vcpu_count = 2;
         let guest_memory = arch_mem(128 << 20);
 
+        let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
         #[allow(unused_mut)]
-        let mut vm = Vm::new(vec![]).unwrap();
+        let mut vm = Vm::new(&kvm).unwrap();
         vm.memory_init(&guest_memory, false).unwrap();
         let evfd = EventFd::new(libc::EFD_NONBLOCK).unwrap();
 
         #[cfg(target_arch = "x86_64")]
         setup_interrupt_controller(&mut vm).unwrap();
 
-        let vcpu_vec = create_vcpus(&vm, vcpu_count, &evfd).unwrap();
+        let vcpu_vec = create_vcpus(&kvm, &vm, vcpu_count, &evfd).unwrap();
         assert_eq!(vcpu_vec.len(), vcpu_count as usize);
     }
 

src/vmm/src/device_manager/legacy.rs

@@ -244,14 +244,11 @@ impl PortIODeviceManager {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::test_utils::single_region_mem;
-    use crate::Vm;
+    use crate::vstate::vm::tests::setup_vm_with_memory;
 
     #[test]
     fn test_register_legacy_devices() {
-        let guest_mem = single_region_mem(0x1000);
-        let mut vm = Vm::new(vec![]).unwrap();
-        vm.memory_init(&guest_mem, false).unwrap();
+        let (_, mut vm, _) = setup_vm_with_memory(0x1000);
         crate::builder::setup_interrupt_controller(&mut vm).unwrap();
         let mut ldm = PortIODeviceManager::new(
             Arc::new(Mutex::new(BusDevice::Serial(SerialDevice {

src/vmm/src/device_manager/mmio.rs

@@ -544,6 +544,7 @@ mod tests {
     use crate::devices::virtio::queue::Queue;
     use crate::devices::virtio::ActivateError;
     use crate::test_utils::multi_region_mem;
+    use crate::vstate::kvm::Kvm;
     use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
     use crate::{builder, Vm};
 
@@ -661,7 +662,8 @@ mod tests {
         let start_addr1 = GuestAddress(0x0);
         let start_addr2 = GuestAddress(0x1000);
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
-        let mut vm = Vm::new(vec![]).unwrap();
+        let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
+        let mut vm = Vm::new(&kvm).unwrap();
         vm.memory_init(&guest_mem, false).unwrap();
         let mut device_manager = MMIODeviceManager::new();
         let mut resource_allocator = ResourceAllocator::new().unwrap();
@@ -690,7 +692,8 @@ mod tests {
         let start_addr1 = GuestAddress(0x0);
         let start_addr2 = GuestAddress(0x1000);
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
-        let mut vm = Vm::new(vec![]).unwrap();
+        let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
+        let mut vm = Vm::new(&kvm).unwrap();
         vm.memory_init(&guest_mem, false).unwrap();
         let mut device_manager = MMIODeviceManager::new();
         let mut resource_allocator = ResourceAllocator::new().unwrap();
@@ -744,7 +747,8 @@ mod tests {
         let start_addr1 = GuestAddress(0x0);
         let start_addr2 = GuestAddress(0x1000);
         let guest_mem = multi_region_mem(&[(start_addr1, 0x1000), (start_addr2, 0x1000)]);
-        let mut vm = Vm::new(vec![]).unwrap();
+        let kvm = Kvm::new(vec![]).expect("Cannot create Kvm");
+        let mut vm = Vm::new(&kvm).unwrap();
         vm.memory_init(&guest_mem, false).unwrap();
 
         let mem_clone = guest_mem.clone();

src/vmm/src/lib.rs

@@ -127,6 +127,7 @@ use userfaultfd::Uffd;
 use vmm_sys_util::epoll::EventSet;
 use vmm_sys_util::eventfd::EventFd;
 use vmm_sys_util::terminal::Terminal;
+use vstate::kvm::Kvm;
 use vstate::vcpu::{self, KvmVcpuConfigureError, StartThreadedError, VcpuSendEventError};
 
 use crate::arch::DeviceType;
@@ -255,6 +256,8 @@ pub enum VmmError {
     VcpuSpawn(io::Error),
     /// Vm error: {0}
     Vm(vstate::vm::VmError),
+    /// Kvm error: {0}
+    Kvm(vstate::kvm::KvmError),
     /// Error thrown by observer object on Vmm initialization: {0}
     VmmObserverInit(vmm_sys_util::errno::Error),
     /// Error thrown by observer object on Vmm teardown: {0}
@@ -307,6 +310,7 @@ pub struct Vmm {
     shutdown_exit_code: Option<FcExitCode>,
 
     // Guest VM core resources.
+    kvm: Kvm,
     vm: Vm,
     guest_memory: GuestMemoryMmap,
     // Save UFFD in order to keep it open in the Firecracker process, as well.
@@ -511,6 +515,7 @@ impl Vmm {
     pub fn save_state(&mut self, vm_info: &VmInfo) -> Result<MicrovmState, MicrovmStateError> {
         use self::MicrovmStateError::SaveVmState;
         let vcpu_states = self.save_vcpu_states()?;
+        let kvm_state = self.kvm.save_state();
         let vm_state = {
             #[cfg(target_arch = "x86_64")]
             {
@@ -531,6 +536,7 @@ impl Vmm {
         Ok(MicrovmState {
             vm_info: vm_info.clone(),
             memory_state,
+            kvm_state,
             vm_state,
             vcpu_states,
             device_states,

src/vmm/src/persist.rs

@@ -36,6 +36,7 @@ use crate::vmm_config::machine_config::{HugePageConfig, MachineConfigUpdate, VmC
 use crate::vmm_config::snapshot::{
     CreateSnapshotParams, LoadSnapshotParams, MemBackendType, SnapshotType,
 };
+use crate::vstate::kvm::KvmState;
 use crate::vstate::memory::{
     GuestMemory, GuestMemoryExtension, GuestMemoryMmap, GuestMemoryState, MemoryError,
 };
@@ -77,6 +78,8 @@ pub struct MicrovmState {
     pub vm_info: VmInfo,
     /// Memory state.
     pub memory_state: GuestMemoryState,
+    /// KVM KVM state.
+    pub kvm_state: KvmState,
     /// VM KVM state.
     pub vm_state: VmState,
     /// Vcpu states.
@@ -736,6 +739,7 @@ mod tests {
             device_states: states,
             memory_state,
             vcpu_states,
+            kvm_state: Default::default(),
             vm_info: VmInfo {
                 mem_size_mib: 1u64,
                 ..Default::default()