From 51a729a87454358c0f2d2c89b2baeca3b14e9aad Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Sat, 4 Apr 2026 11:05:01 -0700 Subject: [PATCH] pci: Add support for reserving but not allocating slots This can be used in a two pass approach where all configs that can hold PCI devices are evaluated to reserve any specific PCI device IDs they may need. Those device IDs will later be allocated when the devices are added to the bus. The tri-state Free, Reserved, Allocated also catches the problem of hotplugging a device with a specific, already used, device ID. Signed-off-by: Rob Bradford --- pci/src/bus.rs | 88 ++++++++++++++++++++++++++------------- vmm/src/device_manager.rs | 6 +-- vmm/src/pci_segment.rs | 19 +++++++-- 3 files changed, 76 insertions(+), 37 deletions(-) diff --git a/pci/src/bus.rs b/pci/src/bus.rs index bd0abec30..4e52ebc9b 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -114,21 +114,28 @@ impl PciDevice for PciRoot { } } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum DeviceIdState { + Free, + Reserved, + Allocated, +} + pub struct PciBus { /// Devices attached to this bus. /// Device 0 is host bridge. devices: HashMap>>, device_reloc: Arc, - device_ids: [bool; NUM_DEVICE_IDS as usize], + device_ids: [DeviceIdState; 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 = [false; NUM_DEVICE_IDS as usize]; + let mut device_ids = [DeviceIdState::Free; NUM_DEVICE_IDS as usize]; devices.insert(PCI_ROOT_DEVICE_ID, Arc::new(Mutex::new(pci_root))); - device_ids[PCI_ROOT_DEVICE_ID as usize] = true; + device_ids[PCI_ROOT_DEVICE_ID as usize] = DeviceIdState::Allocated; PciBus { devices, @@ -172,6 +179,31 @@ impl PciBus { Ok(()) } + /// Reserves a PCI device ID on the bus, marking it as in-use so + /// that automatic allocation will not use it. + /// + /// - `id`: Preferred ID to reserve on the bus. + /// + /// ## Errors + /// + /// * Returns [`PciRootError::AlreadyInUsePciDeviceSlot`] if the + /// slot is already reserved or allocated. + /// * Returns [`PciRootError::InvalidPciDeviceSlot`] if the slot + /// exceeds [`NUM_DEVICE_IDS`]. + pub fn reserve_device_id(&mut self, id: u8) -> Result { + let idx = id as usize; + if idx < NUM_DEVICE_IDS as usize { + if self.device_ids[idx] == DeviceIdState::Free { + self.device_ids[idx] = DeviceIdState::Reserved; + Ok(id) + } else { + Err(PciRootError::AlreadyInUsePciDeviceSlot(idx)) + } + } else { + Err(PciRootError::InvalidPciDeviceSlot(idx)) + } + } + /// Allocates a PCI device ID on the bus. /// /// - `id`: ID to allocate on the bus. If [`None`], the next free @@ -179,6 +211,7 @@ impl PciBus { /// allocated /// /// ## Errors + /// /// * Returns [`PciRootError::AlreadyInUsePciDeviceSlot`] in case /// the ID requested is already allocated. /// * Returns [`PciRootError::InvalidPciDeviceSlot`] in case the @@ -190,10 +223,10 @@ impl PciBus { pub fn allocate_device_id(&mut self, id: Option) -> Result { if let Some(idx) = id.map(|i| i as usize) { if idx < NUM_DEVICE_IDS as usize { - if self.device_ids[idx] { + if self.device_ids[idx] == DeviceIdState::Allocated { Err(PciRootError::AlreadyInUsePciDeviceSlot(idx)) } else { - self.device_ids[idx] = true; + self.device_ids[idx] = DeviceIdState::Allocated; Ok(idx as u8) } } else { @@ -201,8 +234,8 @@ impl PciBus { } } else { for (idx, device_id) in self.device_ids.iter_mut().enumerate() { - if !(*device_id) { - *device_id = true; + if *device_id == DeviceIdState::Free { + *device_id = DeviceIdState::Allocated; return Ok(idx as u8); } } @@ -210,22 +243,9 @@ impl PciBus { } } - pub fn get_device_id(&mut self, id: usize) -> Result<()> { - if id < NUM_DEVICE_IDS as usize { - if self.device_ids[id] { - Err(PciRootError::AlreadyInUsePciDeviceSlot(id)) - } else { - self.device_ids[id] = true; - Ok(()) - } - } else { - Err(PciRootError::InvalidPciDeviceSlot(id)) - } - } - pub fn put_device_id(&mut self, id: usize) -> Result<()> { if id < NUM_DEVICE_IDS as usize { - self.device_ids[id] = false; + self.device_ids[id] = DeviceIdState::Free; Ok(()) } else { Err(PciRootError::InvalidPciDeviceSlot(id)) @@ -577,14 +597,13 @@ mod unit_tests { } #[test] - // Test that gaps resulting from explicit allocations are filled by implicit ones, - // beginning with the first free slot + // Test that reserved IDs are skipped by automatic allocation fn allocate_device_id_fills_gaps() -> Result<(), Box> { // The first address is occupied by the root let mut bus = setup_bus(); - assert_eq!(0x01_u8, bus.allocate_device_id(Some(0x01))?); - assert_eq!(0x03_u8, bus.allocate_device_id(Some(0x03))?); - assert_eq!(0x06_u8, bus.allocate_device_id(Some(0x06))?); + bus.reserve_device_id(0x01)?; + bus.reserve_device_id(0x03)?; + bus.reserve_device_id(0x06)?; assert_eq!(0x02_u8, bus.allocate_device_id(None)?); assert_eq!(0x04_u8, bus.allocate_device_id(None)?); assert_eq!(0x05_u8, bus.allocate_device_id(None)?); @@ -593,12 +612,12 @@ mod unit_tests { } #[test] - // Test that requesting the same ID twice fails - fn allocate_device_id_request_id_twice_fails() -> Result<(), Box> { + // Test that reserving the same ID twice fails + fn reserve_device_id_twice_fails() -> Result<(), Box> { let mut bus = setup_bus(); let max_id = NUM_DEVICE_IDS - 1; - bus.allocate_device_id(Some(max_id))?; - let result = bus.allocate_device_id(Some(max_id)); + bus.reserve_device_id(max_id)?; + let result = bus.reserve_device_id(max_id); assert!(matches!( result, Err(PciRootError::AlreadyInUsePciDeviceSlot(x)) if x == usize::from(max_id), @@ -606,6 +625,15 @@ mod unit_tests { Ok(()) } + #[test] + // Test that allocating a previously reserved ID succeeds (idempotent) + fn allocate_device_id_after_reserve() -> Result<(), Box> { + let mut bus = setup_bus(); + bus.reserve_device_id(0x10)?; + assert_eq!(0x10_u8, bus.allocate_device_id(Some(0x10))?); + Ok(()) + } + #[test] // Test to request an invalid ID fn allocate_device_id_request_invalid_id_fails() -> Result<(), Box> { diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 1499f4d73..cb88fb8cc 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -498,7 +498,7 @@ pub enum DeviceManagerError { /// Could not reserve the PCI device ID. #[error("Could not reserve the PCI device ID")] - GetPciDeviceId(#[source] pci::PciRootError), + ReservePciDeviceId(#[source] pci::PciRootError), /// Could not give the PCI device ID back. #[error("Could not give the PCI device ID back")] @@ -4550,8 +4550,8 @@ impl DeviceManager { .pci_bus .lock() .unwrap() - .get_device_id(pci_device_bdf.device() as usize) - .map_err(DeviceManagerError::GetPciDeviceId)?; + .allocate_device_id(Some(pci_device_bdf.device())) + .map_err(DeviceManagerError::AllocatePciDeviceId)?; (pci_segment_id, pci_device_bdf, resources) } else { diff --git a/vmm/src/pci_segment.rs b/vmm/src/pci_segment.rs index 37cc0dcc6..6a4f10aa7 100644 --- a/vmm/src/pci_segment.rs +++ b/vmm/src/pci_segment.rs @@ -164,6 +164,17 @@ impl PciSegment { ) } + /// Reserves a device ID on this PCI segment, marking it as in-use + /// so that automatic allocation will not use it. + pub(crate) fn reserve_device_id(&self, device_id: u8) -> DeviceManagerResult<()> { + self.pci_bus + .lock() + .unwrap() + .reserve_device_id(device_id) + .map_err(DeviceManagerError::ReservePciDeviceId)?; + Ok(()) + } + /// Allocates a device's ID on this PCI segment. /// /// - `device_id`: Device ID to request for allocation @@ -613,17 +624,17 @@ mod unit_tests { } #[test] - // Test to acquire a device ID that is invalid, one that is already taken - // and one being greater than the number of allowed devices per bus. + // Test that reserving an already taken device ID fails and that + // allocating an out-of-range device ID fails. fn allocate_device_id_invalid_device_id() { // The first address is occupied by the root let already_taken_device_id = 0x0_u8; let overflow_device_id = 0xff_u8; let segment = setup(); - let bdf_res = segment.allocate_device_id(Some(already_taken_device_id)); + let bdf_res = segment.reserve_device_id(already_taken_device_id); assert!(matches!( bdf_res, - Err(DeviceManagerError::GetPciDeviceId(e)) if matches!( + Err(DeviceManagerError::ReservePciDeviceId(e)) if matches!( e, pci::PciRootError::AlreadyInUsePciDeviceSlot(0x0) )