From df71aaee3fbcd842ec09e87b276633ff29ce750d Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 9 Mar 2020 16:09:11 +0100 Subject: [PATCH] pci: Make the device ID allocation smarter In order to handle the case where devices are very often plugged and unplugged from a VM, we need to handle the PCI device ID allocation better. Any PCI device could be removed, which means we cannot simply rely on the vector size to give the next available PCI device ID. That's why this patch stores in memory the information about the 32 slots availability. Based on this information, whenever a new slot is needed, the code can correctly provide an available ID, or simply return an error because all slots are taken. Signed-off-by: Sebastien Boeuf --- pci/src/bus.rs | 18 ++++++++++++++++-- vmm/src/device_manager.rs | 14 ++++++++++++-- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pci/src/bus.rs b/pci/src/bus.rs index 7c7fa61d1..85df331a0 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -16,6 +16,7 @@ use vm_memory::{Address, GuestAddress, GuestUsize}; const VENDOR_ID_INTEL: u16 = 0x8086; const DEVICE_ID_INTEL_VIRT_PCIE_HOST: u16 = 0x0d57; +const NUM_DEVICE_IDS: usize = 32; /// Errors for device manager. #[derive(Debug)] @@ -28,6 +29,8 @@ pub enum PciRootError { PioInsert(devices::BusError), /// Could not add a device to the mmio bus. MmioInsert(devices::BusError), + /// Could not find an available device slot on the PCI bus. + NoPciDeviceSlotAvailable, } pub type Result = std::result::Result; @@ -81,17 +84,21 @@ pub struct PciBus { /// Device 0 is host bridge. devices: Vec>>, device_reloc: Arc, + device_ids: Vec, } impl PciBus { pub fn new(pci_root: PciRoot, device_reloc: Arc) -> Self { let mut devices: Vec>> = Vec::new(); + let mut device_ids: Vec = vec![false; NUM_DEVICE_IDS]; devices.push(Arc::new(Mutex::new(pci_root))); + device_ids[0] = true; PciBus { devices, device_reloc, + device_ids, } } @@ -129,8 +136,15 @@ impl PciBus { Ok(()) } - pub fn next_device_id(&self) -> u32 { - self.devices.len() as u32 + pub fn next_device_id(&mut self) -> Result { + for (idx, device_id) in self.device_ids.iter_mut().enumerate() { + if !(*device_id) { + *device_id = true; + return Ok(idx as u32); + } + } + + Err(PciRootError::NoPciDeviceSlotAvailable) } } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 3adf327da..5d9a201b9 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -240,6 +240,10 @@ pub enum DeviceManagerError { /// Failed to find VFIO device corresponding to the given identifier. #[cfg(feature = "pci_support")] UnknownVfioDeviceId(String), + + /// Failed to find an available PCI device ID. + #[cfg(feature = "pci_support")] + NextPciDeviceId(pci::PciRootError), } pub type DeviceManagerResult = result::Result; @@ -1474,7 +1478,10 @@ impl DeviceManager { // do multifunction. Also, because we only support one PCI // bus, the bus 0, we don't need to add anything to the // global device ID. - let pci_device_bdf = pci.next_device_id() << 3; + let pci_device_bdf = pci + .next_device_id() + .map_err(DeviceManagerError::NextPciDeviceId)? + << 3; let memory = self.memory_manager.lock().unwrap().guest_memory(); let vfio_device = VfioDevice::new( @@ -1588,7 +1595,10 @@ impl DeviceManager { // to the PCI function, and we know we don't do multifunction. // Also, because we only support one PCI bus, the bus 0, we don't need // to add anything to the global device ID. - let dev_id = pci.next_device_id() << 3; + let dev_id = pci + .next_device_id() + .map_err(DeviceManagerError::NextPciDeviceId)? + << 3; // Create the callback from the implementation of the DmaRemapping // trait. The point with the callback is to simplify the code as we