From afd155d578e0205a5beab94fe3860ca1238a876e Mon Sep 17 00:00:00 2001 From: Pascal Scholz Date: Mon, 16 Mar 2026 15:26:12 +0100 Subject: [PATCH] pci: Refactor bus.rs to better fit a PCI bus's semantics This commit refactors the PCI bus struct. It has two major focuses. First, we change the type of `device_ids` in `PciBus` to an array. A fixed-size array better reflects real PCI bus constraints, especially its limited number of PCI devices. Moreover, it can't be grown accidentally. The second focus is changing the type of the key of `devices` in `PciBus` to `u8`, since device IDs are not allowed to exceed 31. We furthermore replace magic numbers with constants and make them publicly available so we can use them in a follow-up change when parsing user input. Signed-off-by: Pascal Scholz On-behalf-of: SAP pascal.scholz@sap.com --- pci/src/bus.rs | 32 ++++++++++++++++++-------------- pci/src/lib.rs | 4 +++- vmm/src/device_manager.rs | 2 +- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/pci/src/bus.rs b/pci/src/bus.rs index 1fa7bd866..57e71551b 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -20,9 +20,13 @@ use crate::configuration::{ }; use crate::device::{BarReprogrammingParams, DeviceRelocation, Error as PciDeviceError, PciDevice}; +/// Denotes the PCI device ID of a bus' root bridge device. +pub const PCI_ROOT_DEVICE_ID: u8 = 0; +/// Denotes the maximum number of PCI devices allowed on a bus. 32 per PCI spec. +pub const NUM_DEVICE_IDS: u8 = 32; + 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(Error, Debug)] @@ -113,18 +117,18 @@ impl PciDevice for PciRoot { pub struct PciBus { /// Devices attached to this bus. /// Device 0 is host bridge. - devices: HashMap>>, + devices: HashMap>>, device_reloc: Arc, - device_ids: Vec, + device_ids: [bool; NUM_DEVICE_IDS as usize], } impl PciBus { pub fn new(pci_root: PciRoot, device_reloc: Arc) -> Self { - let mut devices: HashMap>> = HashMap::new(); - let mut device_ids: Vec = vec![false; NUM_DEVICE_IDS]; + let mut devices: HashMap>> = HashMap::new(); + let mut device_ids = [false; NUM_DEVICE_IDS as usize]; - devices.insert(0, Arc::new(Mutex::new(pci_root))); - device_ids[0] = true; + devices.insert(PCI_ROOT_DEVICE_ID, Arc::new(Mutex::new(pci_root))); + device_ids[PCI_ROOT_DEVICE_ID as usize] = true; PciBus { devices, @@ -158,7 +162,7 @@ impl PciBus { Ok(()) } - pub fn add_device(&mut self, device_id: u32, device: Arc>) -> Result<()> { + pub fn add_device(&mut self, device_id: u8, device: Arc>) -> Result<()> { self.devices.insert(device_id, device); Ok(()) } @@ -180,7 +184,7 @@ impl PciBus { } pub fn get_device_id(&mut self, id: usize) -> Result<()> { - if id < NUM_DEVICE_IDS { + if id < NUM_DEVICE_IDS as usize { if self.device_ids[id] { Err(PciRootError::AlreadyInUsePciDeviceSlot(id)) } else { @@ -193,7 +197,7 @@ impl PciBus { } pub fn put_device_id(&mut self, id: usize) -> Result<()> { - if id < NUM_DEVICE_IDS { + if id < NUM_DEVICE_IDS as usize { self.device_ids[id] = false; Ok(()) } else { @@ -240,7 +244,7 @@ impl PciConfigIo { .lock() .unwrap() .devices - .get(&(device as u32)) + .get(&(device as u8)) .map_or(0xffff_ffff, |d| { d.lock().unwrap().read_config_register(register) }) @@ -265,7 +269,7 @@ impl PciConfigIo { } let pci_bus = self.pci_bus.as_ref().lock().unwrap(); - if let Some(d) = pci_bus.devices.get(&(device as u32)) { + if let Some(d) = pci_bus.devices.get(&(device as u8)) { let mut device = d.lock().unwrap(); // Update the register value @@ -376,7 +380,7 @@ impl PciConfigMmio { .lock() .unwrap() .devices - .get(&(device as u32)) + .get(&(device as u8)) .map_or(0xffff_ffff, |d| { d.lock().unwrap().read_config_register(register) }) @@ -395,7 +399,7 @@ impl PciConfigMmio { } let pci_bus = self.pci_bus.lock().unwrap(); - if let Some(d) = pci_bus.devices.get(&(device as u32)) { + if let Some(d) = pci_bus.devices.get(&(device as u8)) { let mut device = d.lock().unwrap(); // Update the register value diff --git a/pci/src/lib.rs b/pci/src/lib.rs index 17c3ab723..c5bba16d2 100644 --- a/pci/src/lib.rs +++ b/pci/src/lib.rs @@ -21,7 +21,9 @@ use std::str::FromStr; use serde::de::Visitor; -pub use self::bus::{PciBus, PciConfigIo, PciConfigMmio, PciRoot, PciRootError}; +pub use self::bus::{ + NUM_DEVICE_IDS, PCI_ROOT_DEVICE_ID, PciBus, PciConfigIo, PciConfigMmio, PciRoot, PciRootError, +}; pub use self::configuration::{ PCI_CONFIGURATION_ID, PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, PciCapabilityId, PciClassCode, PciConfiguration, PciExpressCapabilityId, PciHeaderType, diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 97e774f98..7f96e1d08 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -4125,7 +4125,7 @@ impl DeviceManager { .unwrap(); pci_bus - .add_device(bdf.device() as u32, pci_device) + .add_device(bdf.device(), pci_device) .map_err(DeviceManagerError::AddPciDevice)?; self.bus_devices.push(Arc::clone(&bus_device));