From cb5aaca8095e76ac13a2e5b3030ee141957bd47d Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Thu, 23 Oct 2025 17:38:46 +0100 Subject: [PATCH] hypervisor, vmm: Remove inner Mutex protecting VcpuFd This was added in 7be69edf5179e948b892693f6c88cba9626a0795 to deal with changes to the KVM bindings that made run() and set_immediate_exit() take &mut self. Instead adopt a Box<> value in Vcpu allowing the removal of this internal Mutex. Signed-off-by: Rob Bradford --- arch/src/aarch64/mod.rs | 2 +- arch/src/riscv64/mod.rs | 2 +- arch/src/x86_64/interrupts.rs | 3 +- arch/src/x86_64/mod.rs | 2 +- arch/src/x86_64/regs.rs | 9 +- hypervisor/src/cpu.rs | 4 +- hypervisor/src/kvm/mod.rs | 179 ++++------------------------------ hypervisor/src/mshv/mod.rs | 6 +- hypervisor/src/vm.rs | 2 +- vmm/src/cpu.rs | 39 +++----- vmm/src/vm.rs | 2 +- 11 files changed, 50 insertions(+), 200 deletions(-) diff --git a/arch/src/aarch64/mod.rs b/arch/src/aarch64/mod.rs index f98942b83..cfb7e1ff5 100644 --- a/arch/src/aarch64/mod.rs +++ b/arch/src/aarch64/mod.rs @@ -66,7 +66,7 @@ pub struct EntryPoint { /// Configure the specified VCPU, and return its MPIDR. pub fn configure_vcpu( - vcpu: &Arc, + vcpu: &dyn hypervisor::Vcpu, id: u32, boot_setup: Option<(EntryPoint, &GuestMemoryAtomic)>, ) -> super::Result { diff --git a/arch/src/riscv64/mod.rs b/arch/src/riscv64/mod.rs index e6d70d38a..53aa479e2 100644 --- a/arch/src/riscv64/mod.rs +++ b/arch/src/riscv64/mod.rs @@ -63,7 +63,7 @@ pub struct EntryPoint { /// Configure the specified VCPU, and return its MPIDR. pub fn configure_vcpu( - vcpu: &Arc, + vcpu: &dyn hypervisor::Vcpu, id: u32, boot_setup: Option<(EntryPoint, &GuestMemoryAtomic)>, ) -> super::Result<()> { diff --git a/arch/src/x86_64/interrupts.rs b/arch/src/x86_64/interrupts.rs index 1ca322ec7..70534c834 100644 --- a/arch/src/x86_64/interrupts.rs +++ b/arch/src/x86_64/interrupts.rs @@ -6,7 +6,6 @@ // found in the LICENSE-BSD-3-Clause file. use std::result; -use std::sync::Arc; pub type Result = result::Result; @@ -24,7 +23,7 @@ pub fn set_apic_delivery_mode(reg: u32, mode: u32) -> u32 { /// /// # Arguments /// * `vcpu` - The VCPU object to configure. -pub fn set_lint(vcpu: &Arc) -> Result<()> { +pub fn set_lint(vcpu: &dyn hypervisor::Vcpu) -> Result<()> { let mut klapic = vcpu.get_lapic()?; let lvt_lint0 = klapic.get_klapic_reg(APIC_LVT0); diff --git a/arch/src/x86_64/mod.rs b/arch/src/x86_64/mod.rs index f1503c39e..d720e69b6 100644 --- a/arch/src/x86_64/mod.rs +++ b/arch/src/x86_64/mod.rs @@ -775,7 +775,7 @@ pub fn generate_common_cpuid( } pub fn configure_vcpu( - vcpu: &Arc, + vcpu: &dyn hypervisor::Vcpu, id: u32, boot_setup: Option<(EntryPoint, &GuestMemoryAtomic)>, cpuid: Vec, diff --git a/arch/src/x86_64/regs.rs b/arch/src/x86_64/regs.rs index 706dcd062..199398570 100644 --- a/arch/src/x86_64/regs.rs +++ b/arch/src/x86_64/regs.rs @@ -6,7 +6,6 @@ // Portions Copyright 2017 The Chromium OS Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE-BSD-3-Clause file. -use std::sync::Arc; use std::{mem, result}; use hypervisor::arch::x86::gdt::{gdt_entry, segment_from_gdt}; @@ -67,7 +66,7 @@ pub type Result = result::Result; /// # Arguments /// /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd. -pub fn setup_fpu(vcpu: &Arc) -> Result<()> { +pub fn setup_fpu(vcpu: &dyn hypervisor::Vcpu) -> Result<()> { let fpu: FpuState = FpuState { fcw: 0x37f, mxcsr: 0x1f80, @@ -82,7 +81,7 @@ pub fn setup_fpu(vcpu: &Arc) -> Result<()> { /// # Arguments /// /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd. -pub fn setup_msrs(vcpu: &Arc) -> Result<()> { +pub fn setup_msrs(vcpu: &dyn hypervisor::Vcpu) -> Result<()> { vcpu.set_msrs(&vcpu.boot_msr_entries()) .map_err(Error::SetModelSpecificRegisters)?; @@ -95,7 +94,7 @@ pub fn setup_msrs(vcpu: &Arc) -> Result<()> { /// /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd. /// * `entry_point` - Description of the boot entry to set up. -pub fn setup_regs(vcpu: &Arc, entry_point: EntryPoint) -> Result<()> { +pub fn setup_regs(vcpu: &dyn hypervisor::Vcpu, entry_point: EntryPoint) -> Result<()> { let mut regs = vcpu.create_standard_regs(); match entry_point.setup_header { None => { @@ -121,7 +120,7 @@ pub fn setup_regs(vcpu: &Arc, entry_point: EntryPoint) -> /// * `vcpu` - Structure for the VCPU that holds the VCPU's fd. pub fn setup_sregs( mem: &GuestMemoryMmap, - vcpu: &Arc, + vcpu: &dyn hypervisor::Vcpu, enable_x2_apic_mode: bool, ) -> Result<()> { let mut sregs: SpecialRegisters = vcpu.get_sregs().map_err(Error::GetStatusRegisters)?; diff --git a/hypervisor/src/cpu.rs b/hypervisor/src/cpu.rs index 2093297d8..0d52d3281 100644 --- a/hypervisor/src/cpu.rs +++ b/hypervisor/src/cpu.rs @@ -526,7 +526,7 @@ pub trait Vcpu: Send + Sync { /// /// Triggers the running of the current virtual CPU returning an exit reason. /// - fn run(&self) -> std::result::Result; + fn run(&mut self) -> std::result::Result; #[cfg(target_arch = "x86_64")] /// /// Translate guest virtual address to guest physical address @@ -542,7 +542,7 @@ pub trait Vcpu: Send + Sync { /// /// Set the "immediate_exit" state /// - fn set_immediate_exit(&self, _exit: bool) {} + fn set_immediate_exit(&mut self, _exit: bool) {} #[cfg(feature = "tdx")] /// /// Returns the details about TDX exit reason diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index ece84566c..dfdf8fa19 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -19,9 +19,11 @@ use std::os::unix::io::AsRawFd; #[cfg(feature = "tdx")] use std::os::unix::io::RawFd; use std::result; +#[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] +use std::sync::Mutex; #[cfg(target_arch = "x86_64")] use std::sync::atomic::{AtomicBool, Ordering}; -use std::sync::{Arc, Mutex, RwLock}; +use std::sync::{Arc, RwLock}; use kvm_ioctls::{NoDatamatch, VcpuFd, VmFd}; use vmm_sys_util::eventfd::EventFd; @@ -603,20 +605,20 @@ impl vm::Vm for KvmVm { &self, id: u32, vm_ops: Option>, - ) -> vm::Result> { + ) -> vm::Result> { let fd = self .fd .create_vcpu(id as u64) .map_err(|e| vm::HypervisorVmError::CreateVcpu(e.into()))?; let vcpu = KvmVcpu { - fd: Arc::new(Mutex::new(fd)), + fd, #[cfg(target_arch = "x86_64")] msrs: self.msrs.clone(), vm_ops, #[cfg(target_arch = "x86_64")] hyperv_synic: AtomicBool::new(false), }; - Ok(Arc::new(vcpu)) + Ok(Box::new(vcpu)) } #[cfg(target_arch = "aarch64")] @@ -1333,7 +1335,7 @@ impl hypervisor::Hypervisor for KvmHypervisor { /// Vcpu struct for KVM pub struct KvmVcpu { - fd: Arc>, + fd: VcpuFd, #[cfg(target_arch = "x86_64")] msrs: Vec, vm_ops: Option>, @@ -1375,8 +1377,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_regs(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_regs() .map_err(|e| cpu::HypervisorCpuError::GetStandardRegs(e.into()))? .into()) @@ -1398,8 +1398,6 @@ impl cpu::Vcpu for KvmVcpu { for i in 0..31 { let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.regs.regs[i] = u64::from_le_bytes(bytes); @@ -1411,8 +1409,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, sp); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.regs.sp = u64::from_le_bytes(bytes); @@ -1421,8 +1417,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, pc); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.regs.pc = u64::from_le_bytes(bytes); @@ -1431,8 +1425,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, pstate); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.regs.pstate = u64::from_le_bytes(bytes); @@ -1441,8 +1433,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, sp_el1); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.sp_el1 = u64::from_le_bytes(bytes); @@ -1452,8 +1442,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, elr_el1); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.elr_el1 = u64::from_le_bytes(bytes); @@ -1463,8 +1451,6 @@ impl cpu::Vcpu for KvmVcpu { for i in 0..KVM_NR_SPSR as usize { let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.spsr[i] = u64::from_le_bytes(bytes); @@ -1477,8 +1463,6 @@ impl cpu::Vcpu for KvmVcpu { for i in 0..32 { let mut bytes = [0_u8; 16]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U128, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.fp_regs.vregs[i] = u128::from_le_bytes(bytes); @@ -1489,8 +1473,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, fp_regs.fpsr); let mut bytes = [0_u8; 4]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U32, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.fp_regs.fpsr = u32::from_le_bytes(bytes); @@ -1499,8 +1481,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, fp_regs.fpcr); let mut bytes = [0_u8; 4]; self.fd - .lock() - .unwrap() .get_one_reg(arm64_core_reg_id!(KVM_REG_SIZE_U32, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetAarchCoreRegister(e.into()))?; state.fp_regs.fpcr = u32::from_le_bytes(bytes); @@ -1523,8 +1503,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_riscv_core, mode); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(riscv64_reg_id!(KVM_REG_RISCV_CORE, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetRiscvCoreRegister(e.into()))?; state.mode = u64::from_le_bytes(bytes); @@ -1533,8 +1511,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_riscv_core, regs.$reg_name); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(riscv64_reg_id!(KVM_REG_RISCV_CORE, off), &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetRiscvCoreRegister(e.into()))?; state.regs.$reg_name = u64::from_le_bytes(bytes); @@ -1585,8 +1561,6 @@ impl cpu::Vcpu for KvmVcpu { fn set_regs(&self, regs: &StandardRegisters) -> cpu::Result<()> { let regs = (*regs).into(); self.fd - .lock() - .unwrap() .set_regs(®s) .map_err(|e| cpu::HypervisorCpuError::SetStandardRegs(e.into())) } @@ -1604,8 +1578,6 @@ impl cpu::Vcpu for KvmVcpu { let mut off = offset_of!(user_pt_regs, regs); for i in 0..31 { self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.regs.regs[i].to_le_bytes(), @@ -1616,8 +1588,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, sp); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.regs.sp.to_le_bytes(), @@ -1626,8 +1596,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, pc); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.regs.pc.to_le_bytes(), @@ -1636,8 +1604,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(user_pt_regs, pstate); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.regs.pstate.to_le_bytes(), @@ -1646,8 +1612,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, sp_el1); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.sp_el1.to_le_bytes(), @@ -1656,8 +1620,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, elr_el1); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.elr_el1.to_le_bytes(), @@ -1667,8 +1629,6 @@ impl cpu::Vcpu for KvmVcpu { let mut off = offset_of!(kvm_regs, spsr); for i in 0..KVM_NR_SPSR as usize { self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, off), &kvm_regs_state.spsr[i].to_le_bytes(), @@ -1680,8 +1640,6 @@ impl cpu::Vcpu for KvmVcpu { let mut off = offset_of!(kvm_regs, fp_regs.vregs); for i in 0..32 { self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U128, off), &kvm_regs_state.fp_regs.vregs[i].to_le_bytes(), @@ -1692,8 +1650,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, fp_regs.fpsr); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U32, off), &kvm_regs_state.fp_regs.fpsr.to_le_bytes(), @@ -1702,8 +1658,6 @@ impl cpu::Vcpu for KvmVcpu { let off = offset_of!(kvm_regs, fp_regs.fpcr); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U32, off), &kvm_regs_state.fp_regs.fpcr.to_le_bytes(), @@ -1729,8 +1683,6 @@ impl cpu::Vcpu for KvmVcpu { (mode) => { let off = offset_of!(kvm_riscv_core, mode); self.fd - .lock() - .unwrap() .set_one_reg( riscv64_reg_id!(KVM_REG_RISCV_CORE, off), &kvm_regs_state.mode.to_le_bytes(), @@ -1740,8 +1692,6 @@ impl cpu::Vcpu for KvmVcpu { ($reg_name:ident) => { let off = offset_of!(kvm_riscv_core, regs.$reg_name); self.fd - .lock() - .unwrap() .set_one_reg( riscv64_reg_id!(KVM_REG_RISCV_CORE, off), &kvm_regs_state.regs.$reg_name.to_le_bytes(), @@ -1794,8 +1744,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_sregs(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_sregs() .map_err(|e| cpu::HypervisorCpuError::GetSpecialRegs(e.into()))? .into()) @@ -1808,8 +1756,6 @@ impl cpu::Vcpu for KvmVcpu { fn set_sregs(&self, sregs: &SpecialRegisters) -> cpu::Result<()> { let sregs = (*sregs).into(); self.fd - .lock() - .unwrap() .set_sregs(&sregs) .map_err(|e| cpu::HypervisorCpuError::SetSpecialRegs(e.into())) } @@ -1821,8 +1767,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_fpu(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_fpu() .map_err(|e| cpu::HypervisorCpuError::GetFloatingPointRegs(e.into()))? .into()) @@ -1835,8 +1779,6 @@ impl cpu::Vcpu for KvmVcpu { fn set_fpu(&self, fpu: &FpuState) -> cpu::Result<()> { let fpu: kvm_bindings::kvm_fpu = (*fpu).clone().into(); self.fd - .lock() - .unwrap() .set_fpu(&fpu) .map_err(|e| cpu::HypervisorCpuError::SetFloatingPointRegs(e.into())) } @@ -1852,8 +1794,6 @@ impl cpu::Vcpu for KvmVcpu { .map_err(|_| cpu::HypervisorCpuError::SetCpuid(anyhow!("failed to create CpuId")))?; self.fd - .lock() - .unwrap() .set_cpuid2(&kvm_cpuid) .map_err(|e| cpu::HypervisorCpuError::SetCpuid(e.into())) } @@ -1872,8 +1812,6 @@ impl cpu::Vcpu for KvmVcpu { ..Default::default() }; self.fd - .lock() - .unwrap() .enable_cap(&cap) .map_err(|e| cpu::HypervisorCpuError::EnableHyperVSyncIc(e.into())) } @@ -1885,8 +1823,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_cpuid2(&self, num_entries: usize) -> cpu::Result> { let kvm_cpuid = self .fd - .lock() - .unwrap() .get_cpuid2(num_entries) .map_err(|e| cpu::HypervisorCpuError::GetCpuid(e.into()))?; @@ -1902,8 +1838,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_lapic(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_lapic() .map_err(|e| cpu::HypervisorCpuError::GetlapicState(e.into()))? .into()) @@ -1916,8 +1850,6 @@ impl cpu::Vcpu for KvmVcpu { fn set_lapic(&self, klapic: &LapicState) -> cpu::Result<()> { let klapic: kvm_bindings::kvm_lapic_state = (*klapic).clone().into(); self.fd - .lock() - .unwrap() .set_lapic(&klapic) .map_err(|e| cpu::HypervisorCpuError::SetLapicState(e.into())) } @@ -1931,8 +1863,6 @@ impl cpu::Vcpu for KvmVcpu { let mut kvm_msrs = MsrEntries::from_entries(&kvm_msrs).unwrap(); let succ = self .fd - .lock() - .unwrap() .get_msrs(&mut kvm_msrs) .map_err(|e| cpu::HypervisorCpuError::GetMsrEntries(e.into()))?; @@ -1955,8 +1885,6 @@ impl cpu::Vcpu for KvmVcpu { let kvm_msrs: Vec = msrs.iter().map(|e| (*e).into()).collect(); let kvm_msrs = MsrEntries::from_entries(&kvm_msrs).unwrap(); self.fd - .lock() - .unwrap() .set_msrs(&kvm_msrs) .map_err(|e| cpu::HypervisorCpuError::SetMsrEntries(e.into())) } @@ -1967,8 +1895,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_mp_state(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_mp_state() .map_err(|e| cpu::HypervisorCpuError::GetMpState(e.into()))? .into()) @@ -1979,8 +1905,6 @@ impl cpu::Vcpu for KvmVcpu { /// fn set_mp_state(&self, mp_state: MpState) -> cpu::Result<()> { self.fd - .lock() - .unwrap() .set_mp_state(mp_state.into()) .map_err(|e| cpu::HypervisorCpuError::SetMpState(e.into())) } @@ -1992,8 +1916,6 @@ impl cpu::Vcpu for KvmVcpu { fn translate_gva(&self, gva: u64, _flags: u64) -> cpu::Result<(u64, u32)> { let tr = self .fd - .lock() - .unwrap() .translate_gva(gva) .map_err(|e| cpu::HypervisorCpuError::TranslateVirtualAddress(e.into()))?; // tr.valid is set if the GVA is mapped to valid GPA. @@ -2008,8 +1930,8 @@ impl cpu::Vcpu for KvmVcpu { /// /// Triggers the running of the current virtual CPU returning an exit reason. /// - fn run(&self) -> std::result::Result { - match self.fd.lock().unwrap().run() { + fn run(&mut self) -> std::result::Result { + match self.fd.run() { Ok(run) => match run { #[cfg(target_arch = "x86_64")] VcpuExit::IoIn(addr, data) => { @@ -2101,7 +2023,7 @@ impl cpu::Vcpu for KvmVcpu { /// potential soft lockups when being resumed. /// fn notify_guest_clock_paused(&self) -> cpu::Result<()> { - if let Err(e) = self.fd.lock().unwrap().kvmclock_ctrl() { + if let Err(e) = self.fd.kvmclock_ctrl() { // Linux kernel returns -EINVAL if the PV clock isn't yet initialised // which could be because we're still in firmware or the guest doesn't // use KVM clock. @@ -2164,8 +2086,6 @@ impl cpu::Vcpu for KvmVcpu { } } self.fd - .lock() - .unwrap() .set_guest_debug(&dbg) .map_err(|e| cpu::HypervisorCpuError::SetDebugRegs(e.into())) } @@ -2232,8 +2152,6 @@ impl cpu::Vcpu for KvmVcpu { fn vcpu_init(&self, kvi: &crate::VcpuInit) -> cpu::Result<()> { let kvm_kvi: kvm_bindings::kvm_vcpu_init = (*kvi).into(); self.fd - .lock() - .unwrap() .vcpu_init(&kvm_kvi) .map_err(|e| cpu::HypervisorCpuError::VcpuInit(e.into())) } @@ -2241,8 +2159,6 @@ impl cpu::Vcpu for KvmVcpu { #[cfg(target_arch = "aarch64")] fn vcpu_finalize(&self, feature: i32) -> cpu::Result<()> { self.fd - .lock() - .unwrap() .vcpu_finalize(&feature) .map_err(|e| cpu::HypervisorCpuError::VcpuFinalize(e.into())) } @@ -2255,8 +2171,6 @@ impl cpu::Vcpu for KvmVcpu { fn get_reg_list(&self, reg_list: &mut RegList) -> cpu::Result<()> { let mut kvm_reg_list: kvm_bindings::RegList = reg_list.clone().into(); self.fd - .lock() - .unwrap() .get_reg_list(&mut kvm_reg_list) .map_err(|e: kvm_ioctls::Error| cpu::HypervisorCpuError::GetRegList(e.into()))?; *reg_list = kvm_reg_list.into(); @@ -2291,8 +2205,6 @@ impl cpu::Vcpu for KvmVcpu { | KVM_REG_ARM64_SYSREG_OP2_MASK)) as u64); let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(id, &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetSysRegister(e.into()))?; Ok(u64::from_le_bytes(bytes)) @@ -2314,8 +2226,6 @@ impl cpu::Vcpu for KvmVcpu { // Get the register index of the PSTATE (Processor State) register. let pstate = offset_of!(kvm_regs, regs.pstate); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, pstate), ®s::PSTATE_FAULT_BITS_64.to_le_bytes(), @@ -2327,8 +2237,6 @@ impl cpu::Vcpu for KvmVcpu { // Setting the PC (Processor Counter) to the current program address (kernel address). let pc = offset_of!(kvm_regs, regs.pc); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, pc), &boot_ip.to_le_bytes(), @@ -2341,8 +2249,6 @@ impl cpu::Vcpu for KvmVcpu { // We are choosing to place it the end of DRAM. See `get_fdt_addr`. let regs0 = offset_of!(kvm_regs, regs.regs); self.fd - .lock() - .unwrap() .set_one_reg( arm64_core_reg_id!(KVM_REG_SIZE_U64, regs0), &fdt_start.to_le_bytes(), @@ -2360,8 +2266,6 @@ impl cpu::Vcpu for KvmVcpu { // Setting the A0 () to the hartid of this CPU. let a0 = offset_of!(kvm_riscv_core, regs.a0); self.fd - .lock() - .unwrap() .set_one_reg( riscv64_reg_id!(KVM_REG_RISCV_CORE, a0), &u64::from(cpu_id).to_le_bytes(), @@ -2371,8 +2275,6 @@ impl cpu::Vcpu for KvmVcpu { // Setting the PC (Processor Counter) to the current program address (kernel address). let pc = offset_of!(kvm_riscv_core, regs.pc); self.fd - .lock() - .unwrap() .set_one_reg( riscv64_reg_id!(KVM_REG_RISCV_CORE, pc), &boot_ip.to_le_bytes(), @@ -2386,8 +2288,6 @@ impl cpu::Vcpu for KvmVcpu { // not exceed 64 kilobytes in size." let a1 = offset_of!(kvm_riscv_core, regs.a1); self.fd - .lock() - .unwrap() .set_one_reg( riscv64_reg_id!(KVM_REG_RISCV_CORE, a1), &fdt_start.to_le_bytes(), @@ -2542,8 +2442,6 @@ impl cpu::Vcpu for KvmVcpu { let mut sys_regs: Vec = Vec::new(); let mut reg_list = kvm_bindings::RegList::new(500).unwrap(); self.fd - .lock() - .unwrap() .get_reg_list(&mut reg_list) .map_err(|e| cpu::HypervisorCpuError::GetRegList(e.into()))?; @@ -2562,8 +2460,6 @@ impl cpu::Vcpu for KvmVcpu { for index in indices.iter() { let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(*index, &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetSysRegister(e.into()))?; sys_regs.push(kvm_bindings::kvm_one_reg { @@ -2595,8 +2491,6 @@ impl cpu::Vcpu for KvmVcpu { let mut sys_regs: Vec = Vec::new(); let mut reg_list = kvm_bindings::RegList::new(200).unwrap(); self.fd - .lock() - .unwrap() .get_reg_list(&mut reg_list) .map_err(|e| cpu::HypervisorCpuError::GetRegList(e.into()))?; @@ -2622,8 +2516,6 @@ impl cpu::Vcpu for KvmVcpu { for index in indices.iter() { let mut bytes = [0_u8; 8]; self.fd - .lock() - .unwrap() .get_one_reg(*index, &mut bytes) .map_err(|e| cpu::HypervisorCpuError::GetSysRegister(e.into()))?; sys_regs.push(kvm_bindings::kvm_one_reg { @@ -2742,8 +2634,6 @@ impl cpu::Vcpu for KvmVcpu { // Set system registers for reg in &state.sys_regs { self.fd - .lock() - .unwrap() .set_one_reg(reg.id, ®.addr.to_le_bytes()) .map_err(|e| cpu::HypervisorCpuError::SetSysRegister(e.into()))?; } @@ -2764,8 +2654,6 @@ impl cpu::Vcpu for KvmVcpu { // Set system registers for reg in &state.non_core_regs { self.fd - .lock() - .unwrap() .set_one_reg(reg.id, ®.addr.to_le_bytes()) .map_err(|e| cpu::HypervisorCpuError::SetSysRegister(e.into()))?; } @@ -2780,20 +2668,15 @@ impl cpu::Vcpu for KvmVcpu { /// #[cfg(feature = "tdx")] fn tdx_init(&self, hob_address: u64) -> cpu::Result<()> { - tdx_command( - &self.fd.lock().unwrap().as_raw_fd(), - TdxCommand::InitVcpu, - 0, - hob_address, - ) - .map_err(cpu::HypervisorCpuError::InitializeTdx) + tdx_command(&self.fd.as_raw_fd(), TdxCommand::InitVcpu, 0, hob_address) + .map_err(cpu::HypervisorCpuError::InitializeTdx) } /// /// Set the "immediate_exit" state /// - fn set_immediate_exit(&self, exit: bool) { - self.fd.lock().unwrap().set_kvm_immediate_exit(exit.into()); + fn set_immediate_exit(&mut self, exit: bool) { + self.fd.set_kvm_immediate_exit(exit.into()); } /// @@ -2801,8 +2684,7 @@ impl cpu::Vcpu for KvmVcpu { /// #[cfg(feature = "tdx")] fn get_tdx_exit_details(&mut self) -> cpu::Result { - let mut fd = self.fd.as_ref().lock().unwrap(); - let kvm_run = fd.get_kvm_run(); + let kvm_run = self.fd.get_kvm_run(); // SAFETY: accessing a union field in a valid structure let tdx_vmcall = unsafe { &mut (*((&mut kvm_run.__bindgen_anon_1) as *mut kvm_run__bindgen_ty_1 @@ -2831,8 +2713,7 @@ impl cpu::Vcpu for KvmVcpu { /// #[cfg(feature = "tdx")] fn set_tdx_status(&mut self, status: TdxExitStatus) { - let mut fd = self.fd.as_ref().lock().unwrap(); - let kvm_run = fd.get_kvm_run(); + let kvm_run = self.fd.get_kvm_run(); // SAFETY: accessing a union field in a valid structure let tdx_vmcall = unsafe { &mut (*((&mut kvm_run.__bindgen_anon_1) as *mut kvm_run__bindgen_ty_1 @@ -2881,7 +2762,7 @@ impl cpu::Vcpu for KvmVcpu { addr: 0x0, flags: 0, }; - self.fd.lock().unwrap().has_device_attr(&cpu_attr).is_ok() + self.fd.has_device_attr(&cpu_attr).is_ok() } #[cfg(target_arch = "aarch64")] @@ -2899,13 +2780,9 @@ impl cpu::Vcpu for KvmVcpu { flags: 0, }; self.fd - .lock() - .unwrap() .set_device_attr(&cpu_attr_irq) .map_err(|_| cpu::HypervisorCpuError::InitializePmu)?; self.fd - .lock() - .unwrap() .set_device_attr(&cpu_attr) .map_err(|_| cpu::HypervisorCpuError::InitializePmu) } @@ -2915,7 +2792,7 @@ impl cpu::Vcpu for KvmVcpu { /// Get the frequency of the TSC if available /// fn tsc_khz(&self) -> cpu::Result> { - match self.fd.lock().unwrap().get_tsc_khz() { + match self.fd.get_tsc_khz() { Err(e) => { if e.errno() == libc::EIO { Ok(None) @@ -2932,7 +2809,7 @@ impl cpu::Vcpu for KvmVcpu { /// Set the frequency of the TSC if available /// fn set_tsc_khz(&self, freq: u32) -> cpu::Result<()> { - match self.fd.lock().unwrap().set_tsc_khz(freq) { + match self.fd.set_tsc_khz(freq) { Err(e) => { if e.errno() == libc::EIO { Ok(()) @@ -2949,7 +2826,7 @@ impl cpu::Vcpu for KvmVcpu { /// Trigger NMI interrupt /// fn nmi(&self) -> cpu::Result<()> { - match self.fd.lock().unwrap().nmi() { + match self.fd.nmi() { Err(e) => { if e.errno() == libc::EIO { Ok(()) @@ -2970,8 +2847,6 @@ impl KvmVcpu { fn get_xsave(&self) -> cpu::Result { Ok(self .fd - .lock() - .unwrap() .get_xsave() .map_err(|e| cpu::HypervisorCpuError::GetXsaveState(e.into()))? .into()) @@ -2987,8 +2862,6 @@ impl KvmVcpu { // when calling the kvm-ioctl library function. unsafe { self.fd - .lock() - .unwrap() .set_xsave(&xsave) .map_err(|e| cpu::HypervisorCpuError::SetXsaveState(e.into())) } @@ -3000,8 +2873,6 @@ impl KvmVcpu { /// fn get_xcrs(&self) -> cpu::Result { self.fd - .lock() - .unwrap() .get_xcrs() .map_err(|e| cpu::HypervisorCpuError::GetXcsr(e.into())) } @@ -3012,8 +2883,6 @@ impl KvmVcpu { /// fn set_xcrs(&self, xcrs: &ExtendedControlRegisters) -> cpu::Result<()> { self.fd - .lock() - .unwrap() .set_xcrs(xcrs) .map_err(|e| cpu::HypervisorCpuError::SetXcsr(e.into())) } @@ -3025,8 +2894,6 @@ impl KvmVcpu { /// fn get_vcpu_events(&self) -> cpu::Result { self.fd - .lock() - .unwrap() .get_vcpu_events() .map_err(|e| cpu::HypervisorCpuError::GetVcpuEvents(e.into())) } @@ -3038,8 +2905,6 @@ impl KvmVcpu { /// fn set_vcpu_events(&self, events: &VcpuEvents) -> cpu::Result<()> { self.fd - .lock() - .unwrap() .set_vcpu_events(events) .map_err(|e| cpu::HypervisorCpuError::SetVcpuEvents(e.into())) } @@ -3052,8 +2917,6 @@ impl KvmVcpu { let maybe_size = self .fd - .lock() - .unwrap() .get_nested_state(&mut buffer) .map_err(|e| cpu::HypervisorCpuError::GetNestedState(e.into()))?; @@ -3068,8 +2931,6 @@ impl KvmVcpu { #[cfg(target_arch = "x86_64")] fn set_nested_state(&self, state: &KvmNestedStateBuffer) -> cpu::Result<()> { self.fd - .lock() - .unwrap() .set_nested_state(state) .map_err(|e| cpu::HypervisorCpuError::GetNestedState(e.into())) } diff --git a/hypervisor/src/mshv/mod.rs b/hypervisor/src/mshv/mod.rs index 385a44462..950dd55f1 100644 --- a/hypervisor/src/mshv/mod.rs +++ b/hypervisor/src/mshv/mod.rs @@ -604,7 +604,7 @@ impl cpu::Vcpu for MshvVcpu { } #[allow(non_upper_case_globals)] - fn run(&self) -> std::result::Result { + fn run(&mut self) -> std::result::Result { match self.fd.run() { Ok(x) => match x.header.message_type { hv_message_type_HVMSG_X64_HALT => { @@ -1821,7 +1821,7 @@ impl vm::Vm for MshvVm { &self, id: u32, vm_ops: Option>, - ) -> vm::Result> { + ) -> vm::Result> { let id: u8 = id.try_into().unwrap(); let vcpu_fd = self .fd @@ -1869,7 +1869,7 @@ impl vm::Vm for MshvVm { #[cfg(feature = "sev_snp")] host_access_pages: ArcSwap::new(self.host_access_pages.load().clone()), }; - Ok(Arc::new(vcpu)) + Ok(Box::new(vcpu)) } #[cfg(target_arch = "x86_64")] diff --git a/hypervisor/src/vm.rs b/hypervisor/src/vm.rs index a2f792131..7a0d1b6af 100644 --- a/hypervisor/src/vm.rs +++ b/hypervisor/src/vm.rs @@ -316,7 +316,7 @@ pub trait Vm: Send + Sync + Any { /// Unregister an event that will, when signaled, trigger the `gsi` IRQ. fn unregister_irqfd(&self, fd: &EventFd, gsi: u32) -> Result<()>; /// Creates a new KVM vCPU file descriptor and maps the memory corresponding - fn create_vcpu(&self, id: u32, vm_ops: Option>) -> Result>; + fn create_vcpu(&self, id: u32, vm_ops: Option>) -> Result>; #[cfg(target_arch = "aarch64")] fn create_vgic(&self, config: VgicConfig) -> Result>>; #[cfg(target_arch = "riscv64")] diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index d1eb4ddaf..626ce2d83 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -392,7 +392,7 @@ macro_rules! round_up { /// A wrapper around creating and using a kvm-based VCPU. pub struct Vcpu { // The hypervisor abstracted CPU. - vcpu: Arc, + vcpu: Box, id: u32, #[cfg(target_arch = "aarch64")] mpidr: u64, @@ -450,15 +450,16 @@ impl Vcpu { #[cfg(target_arch = "aarch64")] { self.init(vm)?; - self.mpidr = arch::configure_vcpu(&self.vcpu, self.id, boot_setup) + self.mpidr = arch::configure_vcpu(self.vcpu.as_ref(), self.id, boot_setup) .map_err(Error::VcpuConfiguration)?; } #[cfg(target_arch = "riscv64")] - arch::configure_vcpu(&self.vcpu, self.id, boot_setup).map_err(Error::VcpuConfiguration)?; + arch::configure_vcpu(self.vcpu.as_ref(), self.id, boot_setup) + .map_err(Error::VcpuConfiguration)?; info!("Configuring vCPU: cpu_id = {}", self.id); #[cfg(target_arch = "x86_64")] arch::configure_vcpu( - &self.vcpu, + self.vcpu.as_ref(), self.id, boot_setup, cpuid, @@ -515,7 +516,7 @@ impl Vcpu { /// /// Note that the state of the VCPU and associated VM must be setup first for this to do /// anything useful. - pub fn run(&self) -> std::result::Result { + pub fn run(&mut self) -> std::result::Result { self.vcpu.run() } @@ -1166,12 +1167,12 @@ impl CpuManager { #[cfg(feature = "kvm")] if matches!(hypervisor_type, HypervisorType::Kvm) { - vcpu.lock().as_ref().unwrap().vcpu.set_immediate_exit(true); + vcpu.lock().unwrap().vcpu.set_immediate_exit(true); if !matches!(vcpu.lock().unwrap().run(), Ok(VmExit::Ignore)) { error!("Unexpected VM exit on \"immediate_exit\" run"); break; } - vcpu.lock().as_ref().unwrap().vcpu.set_immediate_exit(false); + vcpu.lock().unwrap().vcpu.set_immediate_exit(false); } vcpu_run_interrupted.store(true, Ordering::SeqCst); @@ -1204,10 +1205,7 @@ impl CpuManager { break; } - #[cfg(feature = "tdx")] let mut vcpu = vcpu.lock().unwrap(); - #[cfg(not(feature = "tdx"))] - let vcpu = vcpu.lock().unwrap(); // vcpu.run() returns false on a triple-fault so trigger a reset match vcpu.run() { Ok(run) => match run { @@ -1248,8 +1246,7 @@ impl CpuManager { } #[cfg(feature = "tdx")] VmExit::Tdx => { - if let Some(vcpu) = Arc::get_mut(&mut vcpu.vcpu) { - match vcpu.get_tdx_exit_details() { + match vcpu.vcpu.get_tdx_exit_details() { Ok(details) => match details { TdxExitDetails::GetQuote => warn!("TDG_VP_VMCALL_GET_QUOTE not supported"), TdxExitDetails::SetupEventNotifyInterrupt => { @@ -1258,13 +1255,7 @@ impl CpuManager { }, Err(e) => error!("Unexpected TDX VMCALL: {}", e), } - vcpu.set_tdx_status(TdxExitStatus::InvalidOperand); - } else { - // We should never reach this code as - // this means the design from the code - // is wrong. - unreachable!("Couldn't get a mutable reference from Arc as there are multiple instances"); - } + vcpu.vcpu.set_tdx_status(TdxExitStatus::InvalidOperand); } }, @@ -3011,7 +3002,7 @@ mod tests { let lint0_mode_expected = set_apic_delivery_mode(lint0, APIC_MODE_EXTINT); let lint1_mode_expected = set_apic_delivery_mode(lint1, APIC_MODE_NMI); - set_lint(&vcpu).unwrap(); + set_lint(vcpu.as_ref()).unwrap(); // Compute the value that represents LVT0 and LVT1 after set_lint. let klapic_actual: LapicState = vcpu.get_lapic().unwrap(); @@ -3028,7 +3019,7 @@ mod tests { .create_vm(HypervisorVmConfig::default()) .expect("new VM fd creation failed"); let vcpu = vm.create_vcpu(0, None).unwrap(); - setup_fpu(&vcpu).unwrap(); + setup_fpu(vcpu.as_ref()).unwrap(); let expected_fpu: FpuState = FpuState { fcw: 0x37f, @@ -3054,7 +3045,7 @@ mod tests { .create_vm(HypervisorVmConfig::default()) .expect("new VM fd creation failed"); let vcpu = vm.create_vcpu(0, None).unwrap(); - setup_msrs(&vcpu).unwrap(); + setup_msrs(vcpu.as_ref()).unwrap(); // This test will check against the last MSR entry configured (the tenth one). // See create_msr_entries for details. @@ -3089,7 +3080,7 @@ mod tests { expected_regs.set_rip(1); setup_regs( - &vcpu, + vcpu.as_ref(), arch::EntryPoint { entry_addr: vm_memory::GuestAddress(expected_regs.get_rip()), setup_header: None, @@ -3116,7 +3107,7 @@ mod tests { expected_regs.set_rsi(ZERO_PAGE_START.0); setup_regs( - &vcpu, + vcpu.as_ref(), arch::EntryPoint { entry_addr: vm_memory::GuestAddress(expected_regs.get_rip()), setup_header: Some(setup_header { diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index aaf58945e..9ea83cb05 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -3548,7 +3548,7 @@ pub fn test_vm() { mem.write_slice(&code, load_addr) .expect("Writing code to memory failed"); - let vcpu = vm.create_vcpu(0, None).expect("new Vcpu failed"); + let mut vcpu = vm.create_vcpu(0, None).expect("new Vcpu failed"); let mut vcpu_sregs = vcpu.get_sregs().expect("get sregs failed"); vcpu_sregs.cs.base = 0;