hypervisor, vmm: Remove shared ownership of VmmOps

This interface is used by the vCPU thread to delegate responsibility for
handling MMIO/PIO operations and to support different approaches than a
VM exit.

During profiling I found that we were spending 13.75% of the boot CPU
uage acquiring access to the object holding the VmmOps via
ArcSwap::load_full()

    13.75%     6.02%  vcpu0            cloud-hypervisor    [.] arc_swap::ArcSwapAny<T,S>::load_full
            |
            ---arc_swap::ArcSwapAny<T,S>::load_full
               |
                --13.43%--<hypervisor::kvm::KvmVcpu as hypervisor::cpu::Vcpu>::run
                          std::sys_common::backtrace::__rust_begin_short_backtrace
                          core::ops::function::FnOnce::call_once{{vtable-shim}}
                          std::sys::unix::thread::Thread::new::thread_start

However since the object implementing VmmOps does not need to be mutable
and it is only used from the vCPU side we can change the ownership to
being a simple Arc<> that is passed in when calling create_vcpu().

This completely removes the above CPU usage from subsequent profiles.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
This commit is contained in:
Rob Bradford
2020-11-18 16:37:52 +00:00
committed by Samuel Ortiz
parent 6a591ca81d
commit 0fec326582
5 changed files with 46 additions and 48 deletions

View File

@@ -19,7 +19,6 @@ use crate::hypervisor;
use crate::vm::{self, VmmOps};
#[cfg(target_arch = "aarch64")]
use crate::{arm64_core_reg_id, offset__of};
use arc_swap::ArcSwapOption;
use kvm_ioctls::{NoDatamatch, VcpuFd, VmFd};
use serde_derive::{Deserialize, Serialize};
use std::os::unix::io::{AsRawFd, RawFd};
@@ -97,7 +96,6 @@ pub struct KvmVm {
#[cfg(target_arch = "x86_64")]
msrs: MsrEntries,
state: KvmVmState,
vmmops: Arc<ArcSwapOption<Box<dyn vm::VmmOps>>>,
}
// Returns a `Vec<T>` with a size in bytes at least as large as `size_in_bytes`.
@@ -177,7 +175,11 @@ impl vm::Vm for KvmVm {
///
/// Creates a VcpuFd object from a vcpu RawFd.
///
fn create_vcpu(&self, id: u8) -> vm::Result<Arc<dyn cpu::Vcpu>> {
fn create_vcpu(
&self,
id: u8,
vmmops: Option<Arc<Box<dyn VmmOps>>>,
) -> vm::Result<Arc<dyn cpu::Vcpu>> {
let vc = self
.fd
.create_vcpu(id)
@@ -186,7 +188,7 @@ impl vm::Vm for KvmVm {
fd: vc,
#[cfg(target_arch = "x86_64")]
msrs: self.msrs.clone(),
vmmops: self.vmmops.clone(),
vmmops,
#[cfg(target_arch = "x86_64")]
hyperv_synic: AtomicBool::new(false),
};
@@ -362,13 +364,6 @@ impl vm::Vm for KvmVm {
Ok(())
}
///
/// Set the VmmOps interface
///
fn set_vmmops(&self, vmmops: Box<dyn VmmOps>) -> vm::Result<()> {
self.vmmops.store(Some(Arc::new(vmmops)));
Ok(())
}
///
/// Get dirty pages bitmap (one bit per page)
///
@@ -454,7 +449,6 @@ impl hypervisor::Hypervisor for KvmHypervisor {
fd: vm_fd,
msrs,
state: VmState {},
vmmops: Arc::new(ArcSwapOption::from(None)),
}))
}
@@ -463,7 +457,6 @@ impl hypervisor::Hypervisor for KvmHypervisor {
Ok(Arc::new(KvmVm {
fd: vm_fd,
state: VmState {},
vmmops: Arc::new(ArcSwapOption::from(None)),
}))
}
}
@@ -524,7 +517,7 @@ pub struct KvmVcpu {
fd: VcpuFd,
#[cfg(target_arch = "x86_64")]
msrs: MsrEntries,
vmmops: Arc<ArcSwapOption<Box<dyn vm::VmmOps>>>,
vmmops: Option<Arc<Box<dyn vm::VmmOps>>>,
#[cfg(target_arch = "x86_64")]
hyperv_synic: AtomicBool,
}
@@ -535,7 +528,7 @@ pub struct KvmVcpu {
/// let kvm = hypervisor::kvm::KvmHypervisor::new().unwrap();
/// let hypervisor: Arc<dyn hypervisor::Hypervisor> = Arc::new(kvm);
/// let vm = hypervisor.create_vm().expect("new VM fd creation failed");
/// let vcpu = vm.create_vcpu(0).unwrap();
/// let vcpu = vm.create_vcpu(0, None).unwrap();
/// vcpu.get/set().unwrap()
///
impl cpu::Vcpu for KvmVcpu {
@@ -723,7 +716,7 @@ impl cpu::Vcpu for KvmVcpu {
Ok(run) => match run {
#[cfg(target_arch = "x86_64")]
VcpuExit::IoIn(addr, data) => {
if let Some(vmmops) = self.vmmops.load_full() {
if let Some(vmmops) = &self.vmmops {
return vmmops
.pio_read(addr.into(), data)
.map(|_| cpu::VmExit::Ignore)
@@ -734,7 +727,7 @@ impl cpu::Vcpu for KvmVcpu {
}
#[cfg(target_arch = "x86_64")]
VcpuExit::IoOut(addr, data) => {
if let Some(vmmops) = self.vmmops.load_full() {
if let Some(vmmops) = &self.vmmops {
return vmmops
.pio_write(addr.into(), data)
.map(|_| cpu::VmExit::Ignore)
@@ -767,7 +760,7 @@ impl cpu::Vcpu for KvmVcpu {
}
VcpuExit::MmioRead(addr, data) => {
if let Some(vmmops) = self.vmmops.load_full() {
if let Some(vmmops) = &self.vmmops {
return vmmops
.mmio_read(addr, data)
.map(|_| cpu::VmExit::Ignore)
@@ -777,7 +770,7 @@ impl cpu::Vcpu for KvmVcpu {
Ok(cpu::VmExit::MmioRead(addr, data))
}
VcpuExit::MmioWrite(addr, data) => {
if let Some(vmmops) = self.vmmops.load_full() {
if let Some(vmmops) = &self.vmmops {
return vmmops
.mmio_write(addr, data)
.map(|_| cpu::VmExit::Ignore)
@@ -1136,7 +1129,7 @@ impl cpu::Vcpu for KvmVcpu {
/// let hv: Arc<dyn hypervisor::Hypervisor> = Arc::new(kvm);
/// let vm = hv.create_vm().expect("new VM fd creation failed");
/// vm.enable_split_irq().unwrap();
/// let vcpu = vm.create_vcpu(0).unwrap();
/// let vcpu = vm.create_vcpu(0, None).unwrap();
/// let state = vcpu.state().unwrap();
/// ```
fn state(&self) -> cpu::Result<CpuState> {
@@ -1282,7 +1275,7 @@ impl cpu::Vcpu for KvmVcpu {
/// let hv: Arc<dyn hypervisor::Hypervisor> = Arc::new(kvm);
/// let vm = hv.create_vm().expect("new VM fd creation failed");
/// vm.enable_split_irq().unwrap();
/// let vcpu = vm.create_vcpu(0).unwrap();
/// let vcpu = vm.create_vcpu(0, None).unwrap();
/// let state = vcpu.state().unwrap();
/// vcpu.set_state(&state).unwrap();
/// ```

View File

@@ -174,7 +174,7 @@ pub trait Vm: Send + Sync {
/// 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: u8) -> Result<Arc<dyn Vcpu>>;
fn create_vcpu(&self, id: u8, vmmops: Option<Arc<Box<dyn VmmOps>>>) -> Result<Arc<dyn Vcpu>>;
/// Registers an event to be signaled whenever a certain address is written to.
fn register_ioevent(
&self,
@@ -220,8 +220,6 @@ pub trait Vm: Send + Sync {
fn state(&self) -> Result<VmState>;
/// Set the VM state
fn set_state(&self, state: VmState) -> Result<()>;
/// Set VmmOps interface
fn set_vmmops(&self, vmmops: Box<dyn VmmOps>) -> Result<()>;
/// Get dirty pages bitmap
fn get_dirty_log(&self, slot: u32, memory_size: u64) -> Result<Vec<u64>>;
}