vmm: Add fast path for PCI config IO port

Looking up devices on the port I/O bus is time consuming during the
boot at there is an O(lg n) tree lookup and the overhead from taking a
lock on the bus contents.

Avoid this by adding a fast path uses the hardcoded port address and
size and directs PCI config requests directly to the device.

Command line:
target/release/cloud-hypervisor --kernel ~/src/linux/vmlinux --cmdline "root=/dev/vda1 console=ttyS0" --serial tty --console off --disk path=~/workloads/focal-server-cloudimg-amd64-custom-20210609-0.raw --api-socket /tmp/api

PIO exit: 17913
PCI fast path: 17871
Percentage on fast path: 99.8%

perf before:

marvin:~/src/cloud-hypervisor (main *)$ perf report -g | grep resolve
     6.20%     6.20%  vcpu0            cloud-hypervisor    [.] vm_device:🚌:Bus::resolve

perf after:

marvin:~/src/cloud-hypervisor (2021-09-17-ioapic-fast-path *)$ perf report -g | grep resolve
     0.08%     0.08%  vcpu0            cloud-hypervisor    [.] vm_device:🚌:Bus::resolve

The compromise required to implement this fast path is bringing the
creation of the PciConfigIo device into the DeviceManager::new() so that
it can be used in the VmmOps struct which is created before
DeviceManager::create_devices() is called.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
This commit is contained in:
Rob Bradford 2021-09-17 09:59:30 +01:00
parent da8eecc797
commit 0faa7afac2
4 changed files with 75 additions and 9 deletions

View File

@ -188,20 +188,25 @@ impl PciBus {
} }
} }
#[derive(Default)]
pub struct PciConfigIo { pub struct PciConfigIo {
/// Config space register. /// Config space register.
config_address: u32, config_address: u32,
pci_bus: Arc<Mutex<PciBus>>, pci_bus: Option<Arc<Mutex<PciBus>>>,
} }
impl PciConfigIo { impl PciConfigIo {
pub fn new(pci_bus: Arc<Mutex<PciBus>>) -> Self { pub fn new() -> Self {
PciConfigIo { PciConfigIo {
pci_bus,
config_address: 0, config_address: 0,
pci_bus: None,
} }
} }
pub fn set_bus(&mut self, pci_bus: Arc<Mutex<PciBus>>) {
self.pci_bus = Some(pci_bus)
}
pub fn config_space_read(&self) -> u32 { pub fn config_space_read(&self) -> u32 {
let enabled = (self.config_address & 0x8000_0000) != 0; let enabled = (self.config_address & 0x8000_0000) != 0;
if !enabled { if !enabled {
@ -222,6 +227,8 @@ impl PciConfigIo {
} }
self.pci_bus self.pci_bus
.as_ref()
.unwrap()
.lock() .lock()
.unwrap() .unwrap()
.devices .devices
@ -249,7 +256,7 @@ impl PciConfigIo {
return None; return None;
} }
let pci_bus = self.pci_bus.lock().unwrap(); let pci_bus = self.pci_bus.as_ref().unwrap().lock().unwrap();
if let Some(d) = pci_bus.devices.get(&(device as u32)) { if let Some(d) = pci_bus.devices.get(&(device as u32)) {
let mut device = d.lock().unwrap(); let mut device = d.lock().unwrap();

View File

@ -42,3 +42,8 @@ impl PciInterruptPin {
self as u32 self as u32
} }
} }
#[cfg(target_arch = "x86_64")]
pub const PCI_CONFIG_IO_PORT: u64 = 0xcf8;
#[cfg(target_arch = "x86_64")]
pub const PCI_CONFIG_IO_PORT_SIZE: u64 = 0x8;

View File

@ -67,11 +67,12 @@ use libc::{
isatty, tcgetattr, tcsetattr, termios, ECHO, ICANON, ISIG, MAP_NORESERVE, MAP_PRIVATE, isatty, tcgetattr, tcsetattr, termios, ECHO, ICANON, ISIG, MAP_NORESERVE, MAP_PRIVATE,
MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE, TCSANOW, MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE, TCSANOW,
}; };
use pci::VfioPciDevice;
use pci::{ use pci::{
DeviceRelocation, PciBarRegionType, PciBus, PciConfigIo, PciConfigMmio, PciDevice, PciRoot, DeviceRelocation, PciBarRegionType, PciBus, PciConfigMmio, PciDevice, PciRoot, VfioPciDevice,
VfioUserPciDevice, VfioUserPciDeviceError, VfioUserPciDevice, VfioUserPciDeviceError,
}; };
#[cfg(target_arch = "x86_64")]
use pci::{PciConfigIo, PCI_CONFIG_IO_PORT, PCI_CONFIG_IO_PORT_SIZE};
use seccompiler::SeccompAction; use seccompiler::SeccompAction;
use std::collections::HashMap; use std::collections::HashMap;
use std::convert::TryInto; use std::convert::TryInto;
@ -826,6 +827,9 @@ pub struct DeviceManager {
// Keep a reference to the PCI bus // Keep a reference to the PCI bus
pci_bus: Option<Arc<Mutex<PciBus>>>, pci_bus: Option<Arc<Mutex<PciBus>>>,
#[cfg(target_arch = "x86_64")]
pci_config_io: Arc<Mutex<PciConfigIo>>,
#[cfg_attr(target_arch = "aarch64", allow(dead_code))] #[cfg_attr(target_arch = "aarch64", allow(dead_code))]
// MSI Interrupt Manager // MSI Interrupt Manager
msi_interrupt_manager: Arc<dyn InterruptManager<GroupConfig = MsiIrqGroupConfig>>, msi_interrupt_manager: Arc<dyn InterruptManager<GroupConfig = MsiIrqGroupConfig>>,
@ -959,6 +963,9 @@ impl DeviceManager {
bus_devices: Vec::new(), bus_devices: Vec::new(),
device_id_cnt: Wrapping(0), device_id_cnt: Wrapping(0),
pci_bus: None, pci_bus: None,
#[cfg(target_arch = "x86_64")]
// Create early so ready for use in `VmmOps`
pci_config_io: Arc::new(Mutex::new(PciConfigIo::new())),
msi_interrupt_manager, msi_interrupt_manager,
legacy_interrupt_manager: None, legacy_interrupt_manager: None,
passthrough_device: None, passthrough_device: None,
@ -1222,13 +1229,22 @@ impl DeviceManager {
} }
let pci_bus = Arc::new(Mutex::new(pci_bus)); let pci_bus = Arc::new(Mutex::new(pci_bus));
let pci_config_io = Arc::new(Mutex::new(PciConfigIo::new(Arc::clone(&pci_bus)))); #[cfg(target_arch = "x86_64")]
self.pci_config_io
.lock()
.unwrap()
.set_bus(Arc::clone(&pci_bus));
#[cfg(target_arch = "x86_64")]
self.bus_devices self.bus_devices
.push(Arc::clone(&pci_config_io) as Arc<Mutex<dyn BusDevice>>); .push(Arc::clone(&self.pci_config_io) as Arc<Mutex<dyn BusDevice>>);
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
self.address_manager self.address_manager
.io_bus .io_bus
.insert(pci_config_io, 0xcf8, 0x8) .insert(
self.pci_config_io.clone(),
PCI_CONFIG_IO_PORT,
PCI_CONFIG_IO_PORT_SIZE,
)
.map_err(DeviceManagerError::BusError)?; .map_err(DeviceManagerError::BusError)?;
let pci_config_mmio = Arc::new(Mutex::new(PciConfigMmio::new(Arc::clone(&pci_bus)))); let pci_config_mmio = Arc::new(Mutex::new(PciConfigMmio::new(Arc::clone(&pci_bus))));
self.bus_devices self.bus_devices
@ -3314,6 +3330,12 @@ impl DeviceManager {
.map(|ic| ic.clone() as Arc<Mutex<dyn InterruptController>>) .map(|ic| ic.clone() as Arc<Mutex<dyn InterruptController>>)
} }
#[cfg(target_arch = "x86_64")]
// Used to provide a fast path for handling PIO exits
pub fn pci_config_io(&self) -> Arc<Mutex<PciConfigIo>> {
self.pci_config_io.clone()
}
pub fn console(&self) -> &Arc<Console> { pub fn console(&self) -> &Arc<Console> {
&self.console &self.console
} }

View File

@ -65,6 +65,8 @@ use std::panic::AssertUnwindSafe;
use std::sync::{Arc, Mutex, RwLock}; use std::sync::{Arc, Mutex, RwLock};
use std::{result, str, thread}; use std::{result, str, thread};
use vm_device::Bus; use vm_device::Bus;
#[cfg(target_arch = "x86_64")]
use vm_device::BusDevice;
use vm_memory::{ use vm_memory::{
Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic, Address, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryAtomic,
GuestMemoryRegion, GuestMemoryRegion,
@ -360,6 +362,8 @@ struct VmOps {
mmio_bus: Arc<Bus>, mmio_bus: Arc<Bus>,
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
timestamp: std::time::Instant, timestamp: std::time::Instant,
#[cfg(target_arch = "x86_64")]
pci_config_io: Arc<Mutex<dyn BusDevice>>,
} }
impl VmOps { impl VmOps {
@ -417,6 +421,17 @@ impl VmmOps for VmOps {
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
fn pio_read(&self, port: u64, data: &mut [u8]) -> hypervisor::vm::Result<()> { fn pio_read(&self, port: u64, data: &mut [u8]) -> hypervisor::vm::Result<()> {
use pci::{PCI_CONFIG_IO_PORT, PCI_CONFIG_IO_PORT_SIZE};
if (PCI_CONFIG_IO_PORT..(PCI_CONFIG_IO_PORT + PCI_CONFIG_IO_PORT_SIZE)).contains(&port) {
self.pci_config_io.lock().unwrap().read(
PCI_CONFIG_IO_PORT,
port - PCI_CONFIG_IO_PORT,
data,
);
return Ok(());
}
if let Err(vm_device::BusError::MissingAddressRange) = self.io_bus.read(port, data) { if let Err(vm_device::BusError::MissingAddressRange) = self.io_bus.read(port, data) {
warn!("Guest PIO read to unregistered address 0x{:x}", port); warn!("Guest PIO read to unregistered address 0x{:x}", port);
} }
@ -425,11 +440,22 @@ impl VmmOps for VmOps {
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
fn pio_write(&self, port: u64, data: &[u8]) -> hypervisor::vm::Result<()> { fn pio_write(&self, port: u64, data: &[u8]) -> hypervisor::vm::Result<()> {
use pci::{PCI_CONFIG_IO_PORT, PCI_CONFIG_IO_PORT_SIZE};
if port == DEBUG_IOPORT as u64 && data.len() == 1 { if port == DEBUG_IOPORT as u64 && data.len() == 1 {
self.log_debug_ioport(data[0]); self.log_debug_ioport(data[0]);
return Ok(()); return Ok(());
} }
if (PCI_CONFIG_IO_PORT..(PCI_CONFIG_IO_PORT + PCI_CONFIG_IO_PORT_SIZE)).contains(&port) {
self.pci_config_io.lock().unwrap().write(
PCI_CONFIG_IO_PORT,
port - PCI_CONFIG_IO_PORT,
data,
);
return Ok(());
}
match self.io_bus.write(port, data) { match self.io_bus.write(port, data) {
Err(vm_device::BusError::MissingAddressRange) => { Err(vm_device::BusError::MissingAddressRange) => {
warn!("Guest PIO write to unregistered address 0x{:x}", port); warn!("Guest PIO write to unregistered address 0x{:x}", port);
@ -546,6 +572,10 @@ impl Vm {
let mmio_bus = Arc::clone(device_manager.lock().unwrap().mmio_bus()); let mmio_bus = Arc::clone(device_manager.lock().unwrap().mmio_bus());
// Create the VmOps structure, which implements the VmmOps trait. // Create the VmOps structure, which implements the VmmOps trait.
// And send it to the hypervisor. // And send it to the hypervisor.
#[cfg(target_arch = "x86_64")]
let pci_config_io =
device_manager.lock().unwrap().pci_config_io() as Arc<Mutex<dyn BusDevice>>;
let vm_ops: Arc<dyn VmmOps> = Arc::new(VmOps { let vm_ops: Arc<dyn VmmOps> = Arc::new(VmOps {
memory, memory,
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
@ -553,6 +583,8 @@ impl Vm {
mmio_bus, mmio_bus,
#[cfg(target_arch = "x86_64")] #[cfg(target_arch = "x86_64")]
timestamp: std::time::Instant::now(), timestamp: std::time::Instant::now(),
#[cfg(target_arch = "x86_64")]
pci_config_io,
}); });
let exit_evt_clone = exit_evt.try_clone().map_err(Error::EventFdClone)?; let exit_evt_clone = exit_evt.try_clone().map_err(Error::EventFdClone)?;