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) )