refactor: Move VcpuFd out of KvmVcpu

This separate emulation state from "live" data (aka the KVM file
descriptor). This is a first step towards removing the need for mocking
the `.emulate()` call in unit tests for KVM_EXIT handling.

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

Author: roypat

src/vmm/src/builder.rs

@@ -487,9 +487,9 @@ pub fn build_microvm_from_snapshot(
         if let Some(state_tsc) = microvm_state.vcpu_states[0].tsc_khz {
             // Scale the TSC frequency for all VCPUs. If a TSC frequency is not specified in the
             // snapshot, by default it uses the host frequency.
-            if vcpus[0].kvm_vcpu.is_tsc_scaling_required(state_tsc)? {
+            if vcpus[0].is_tsc_scaling_required(state_tsc)? {
                 for vcpu in &vcpus {
-                    vcpu.kvm_vcpu.set_tsc_khz(state_tsc)?;
+                    vcpu.set_tsc_khz(state_tsc)?;
                 }
             }
         }
@@ -499,8 +499,7 @@ pub fn build_microvm_from_snapshot(
     #[cfg(target_arch = "aarch64")]
     {
         for (vcpu, state) in vcpus.iter_mut().zip(microvm_state.vcpu_states.iter()) {
-            vcpu.kvm_vcpu
-                .restore_state(vmm.vm.fd(), state)
+            vcpu.restore_state(vmm.vm.fd(), state)
                 .map_err(VcpuError::VcpuResponse)
                 .map_err(BuildMicrovmFromSnapshotError::RestoreVcpus)?;
         }
@@ -511,8 +510,7 @@ pub fn build_microvm_from_snapshot(
 
     #[cfg(target_arch = "x86_64")]
     for (vcpu, state) in vcpus.iter_mut().zip(microvm_state.vcpu_states.iter()) {
-        vcpu.kvm_vcpu
-            .restore_state(state)
+        vcpu.restore_state(state)
             .map_err(VcpuError::VcpuResponse)
             .map_err(BuildMicrovmFromSnapshotError::RestoreVcpus)?;
     }
@@ -779,7 +777,6 @@ pub fn configure_system_for_boot(
             .map_err(GuestConfigError::CpuidFromKvmCpuid)?;
         let msr_index_list = cpu_template.get_msr_index_list();
         let msrs = vcpus[0]
-            .kvm_vcpu
             .get_msrs(&msr_index_list)
             .map_err(GuestConfigError::VcpuIoctl)?;
         CpuConfiguration { cpuid, msrs }
@@ -791,14 +788,13 @@ pub fn configure_system_for_boot(
         use crate::arch::aarch64::vcpu::get_registers;
 
         for vcpu in vcpus.iter_mut() {
-            vcpu.kvm_vcpu
-                .init(vmm.vm.fd(), &cpu_template.vcpu_features)
+            vcpu.init(vmm.vm.fd(), &cpu_template.vcpu_features)
                 .map_err(VmmError::VcpuInit)
                 .map_err(Internal)?;
         }
 
         let mut regs = Aarch64RegisterVec::default();
-        get_registers(&vcpus[0].kvm_vcpu.fd, &cpu_template.reg_list(), &mut regs)
+        get_registers(&vcpus[0].fd, &cpu_template.reg_list(), &mut regs)
             .map_err(GuestConfigError)?;
         CpuConfiguration { regs }
     };
@@ -814,8 +810,7 @@ pub fn configure_system_for_boot(
 
     // Configure vCPUs with normalizing and setting the generated CPU configuration.
     for vcpu in vcpus.iter_mut() {
-        vcpu.kvm_vcpu
-            .configure(vmm.guest_memory(), entry_addr, &vcpu_config)
+        vcpu.configure(vmm.guest_memory(), entry_addr, &vcpu_config)
             .map_err(VmmError::VcpuConfigure)
             .map_err(Internal)?;
     }

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

@@ -22,6 +22,7 @@ use crate::vcpu::{VcpuConfig, VcpuError};
 use crate::vstate::memory::{Address, GuestAddress, GuestMemoryMmap};
 use crate::vstate::vcpu::VcpuEmulation;
 use crate::vstate::vm::Vm;
+use crate::Vcpu;
 
 /// Errors associated with the wrappers over KVM ioctls.
 #[derive(Debug, PartialEq, Eq, thiserror::Error, displaydoc::Display)]
@@ -52,8 +53,6 @@ pub type KvmVcpuConfigureError = KvmVcpuError;
 pub struct KvmVcpu {
     /// Index of vcpu.
     pub index: u8,
-    /// KVM vcpu fd.
-    pub fd: VcpuFd,
     /// Mmio bus.
     pub mmio_bus: Option<crate::devices::Bus>,
     mpidr: u64,
@@ -67,26 +66,33 @@ 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) -> Result<Self, KvmVcpuError> {
-        let kvm_vcpu = vm
-            .fd()
-            .create_vcpu(index.into())
-            .map_err(KvmVcpuError::CreateVcpu)?;
-
-        Ok(KvmVcpu {
+    pub fn new(index: u8, _vm: &Vm) -> Self {
+        KvmVcpu {
             index,
-            fd: kvm_vcpu,
             mmio_bus: None,
             mpidr: 0,
             kvi: None,
-        })
+        }
     }
 
     /// Gets the MPIDR register value.
     pub fn get_mpidr(&self) -> u64 {
         self.mpidr
     }
 
+    /// Runs the vCPU in KVM context and handles the kvm exit reason.
+    ///
+    /// Returns error or enum specifying whether emulation was handled or interrupted.
+    pub fn run_arch_emulation(&self, exit: VcpuExit) -> Result<VcpuEmulation, VcpuError> {
+        METRICS.vcpu.failures.inc();
+        // TODO: Are we sure we want to finish running a vcpu upon
+        // receiving a vm exit that is not necessarily an error?
+        error!("Unexpected exit reason on vcpu run: {:?}", exit);
+        Err(VcpuError::UnhandledKvmExit(format!("{:?}", exit)))
+    }
+}
+
+impl Vcpu {
     /// Configures an aarch64 specific vcpu for booting Linux.
     ///
     /// # Arguments
@@ -108,13 +114,13 @@ impl KvmVcpu {
 
         setup_boot_regs(
             &self.fd,
-            self.index,
+            self.kvm_vcpu.index,
             kernel_load_addr.raw_value(),
             guest_mem,
         )
         .map_err(KvmVcpuError::ConfigureRegisters)?;
 
-        self.mpidr = get_mpidr(&self.fd).map_err(KvmVcpuError::ConfigureRegisters)?;
+        self.kvm_vcpu.mpidr = get_mpidr(&self.fd).map_err(KvmVcpuError::ConfigureRegisters)?;
 
         Ok(())
     }
@@ -129,7 +135,7 @@ impl KvmVcpu {
         vm_fd: &VmFd,
         vcpu_features: &[VcpuFeatures],
     ) -> Result<(), KvmVcpuError> {
-        let mut kvi = Self::default_kvi(vm_fd, self.index)?;
+        let mut kvi = Self::default_kvi(vm_fd, self.kvm_vcpu.index)?;
 
         for feature in vcpu_features.iter() {
             let index = feature.index as usize;
@@ -139,7 +145,7 @@ impl KvmVcpu {
         self.init_vcpu(&kvi)?;
         self.finalize_vcpu(&kvi)?;
 
-        self.kvi = if !vcpu_features.is_empty() {
+        self.kvm_vcpu.kvi = if !vcpu_features.is_empty() {
             Some(kvi)
         } else {
             None
@@ -177,17 +183,17 @@ impl KvmVcpu {
         };
         get_all_registers(&self.fd, &mut state.regs).map_err(KvmVcpuError::SaveState)?;
         state.mpidr = get_mpidr(&self.fd).map_err(KvmVcpuError::SaveState)?;
-        state.kvi = self.kvi;
+        state.kvi = self.kvm_vcpu.kvi;
         Ok(state)
     }
 
     /// Use provided state to populate KVM internal state.
     pub fn restore_state(&mut self, vm_fd: &VmFd, state: &VcpuState) -> Result<(), KvmVcpuError> {
         let kvi = match state.kvi {
             Some(kvi) => kvi,
-            None => Self::default_kvi(vm_fd, self.index)?,
+            None => Self::default_kvi(vm_fd, self.kvm_vcpu.index)?,
         };
-        self.kvi = state.kvi;
+        self.kvm_vcpu.kvi = state.kvi;
 
         self.init_vcpu(&kvi)?;
 
@@ -226,17 +232,6 @@ impl KvmVcpu {
         Ok(CpuConfiguration { regs })
     }
 
-    /// Runs the vCPU in KVM context and handles the kvm exit reason.
-    ///
-    /// Returns error or enum specifying whether emulation was handled or interrupted.
-    pub fn run_arch_emulation(&self, exit: VcpuExit) -> Result<VcpuEmulation, VcpuError> {
-        METRICS.vcpu.failures.inc();
-        // TODO: Are we sure we want to finish running a vcpu upon
-        // receiving a vm exit that is not necessarily an error?
-        error!("Unexpected exit reason on vcpu run: {:?}", exit);
-        Err(VcpuError::UnhandledKvmExit(format!("{:?}", exit)))
-    }
-
     /// Initializes internal vcpufd.
     fn init_vcpu(&self, kvi: &kvm_bindings::kvm_vcpu_init) -> Result<(), KvmVcpuError> {
         self.fd.vcpu_init(kvi).map_err(KvmVcpuError::Init)?;
@@ -301,6 +296,7 @@ mod tests {
     use std::os::unix::io::AsRawFd;
 
     use kvm_bindings::KVM_REG_SIZE_U64;
+    use utils::eventfd::EventFd;
 
     use super::*;
     use crate::arch::aarch64::regs::Aarch64RegisterRef;
@@ -311,9 +307,9 @@ mod tests {
     use crate::vstate::vm::tests::setup_vm;
     use crate::vstate::vm::Vm;
 
-    fn setup_vcpu(mem_size: usize) -> (Vm, KvmVcpu, GuestMemoryMmap) {
+    fn setup_vcpu(mem_size: usize) -> (Vm, Vcpu, GuestMemoryMmap) {
         let (mut vm, vm_mem) = setup_vm(mem_size);
-        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let mut vcpu = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vcpu.init(vm.fd(), &[]).unwrap();
         vm.setup_irqchip(1).unwrap();
 
@@ -326,7 +322,7 @@ mod tests {
 
         unsafe { libc::close(vm.fd().as_raw_fd()) };
 
-        let err = KvmVcpu::new(0, &vm);
+        let err = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap());
         assert_eq!(
             err.err().unwrap().to_string(),
             "Error creating vcpu: Bad file descriptor (os error 9)".to_string()
@@ -367,8 +363,8 @@ mod tests {
 
     #[test]
     fn test_init_vcpu() {
-        let (mut vm, _vm_mem) = setup_vm(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let (mut vm, _) = setup_vm(0x1000);
+        let mut vcpu = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         // KVM_ARM_VCPU_PSCI_0_2 is set by default.
@@ -384,7 +380,7 @@ mod tests {
 
         // Because vcpu_features vector is not empty,
         // kvi field should be non empty as well.
-        let vcpu_kvi = vcpu.kvi.unwrap();
+        let vcpu_kvi = vcpu.kvm_vcpu.kvi.unwrap();
         assert!((vcpu_kvi.features[0] & (1 << kvm_bindings::KVM_ARM_VCPU_PSCI_0_2)) == 0)
     }
 
@@ -401,8 +397,8 @@ mod tests {
 
     #[test]
     fn test_vcpu_save_restore_state() {
-        let (mut vm, _vm_mem) = setup_vm(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let (mut vm, _) = setup_vm(0x1000);
+        let mut vcpu = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         // Calling KVM_GET_REGLIST before KVM_VCPU_INIT will result in error.
@@ -440,8 +436,8 @@ mod tests {
         //
         // This should fail with ENOEXEC.
         // https://elixir.bootlin.com/linux/v5.10.176/source/arch/arm64/kvm/arm.c#L1165
-        let (mut vm, _vm_mem) = setup_vm(0x1000);
-        let vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let (mut vm, _) = setup_vm(0x1000);
+        let mut vcpu = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vm.setup_irqchip(1).unwrap();
 
         vcpu.dump_cpu_config().unwrap_err();
@@ -450,8 +446,8 @@ mod tests {
     #[test]
     fn test_dump_cpu_config_after_init() {
         // Test `dump_cpu_config()` after `KVM_VCPU_INIT`.
-        let (mut vm, _vm_mem) = setup_vm(0x1000);
-        let mut vcpu = KvmVcpu::new(0, &vm).unwrap();
+        let (mut vm, _) = setup_vm(0x1000);
+        let mut vcpu = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vm.setup_irqchip(1).unwrap();
         vcpu.init(vm.fd(), &[]).unwrap();
 
@@ -461,9 +457,9 @@ mod tests {
     #[test]
     fn test_setup_non_boot_vcpu() {
         let (vm, _) = setup_vm(0x1000);
-        let mut vcpu1 = KvmVcpu::new(0, &vm).unwrap();
+        let mut vcpu1 = Vcpu::new(0, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vcpu1.init(vm.fd(), &[]).unwrap();
-        let mut vcpu2 = KvmVcpu::new(1, &vm).unwrap();
+        let mut vcpu2 = Vcpu::new(1, &vm, EventFd::new(libc::EFD_NONBLOCK).unwrap()).unwrap();
         vcpu2.init(vm.fd(), &[]).unwrap();
     }