From f787bd7a17d14c9633bc0b9cdca2b962d6eeda24 Mon Sep 17 00:00:00 2001 From: wuxinyue Date: Thu, 11 Jun 2026 10:54:42 +0800 Subject: [PATCH] vmm: move BAR mapping registration from PciBus to DeviceManager `PciBus::register_mapping()` operates on `mmio_bus` and `io_bus` which are passed in as external parameters and have nothing to do with PciBus internal state. Move this logic into `DeviceManager::register_bar_mapping()` where it belongs, and move the `PioInsert`/`MmioInsert` error variants from `PciRootError` to `DeviceManagerError` accordingly. Signed-off-by: wuxinyue Assisted-by: Claude:Opus-4.6 --- pci/src/bus.rs | 39 +++-------------------------- vmm/src/device_manager.rs | 52 ++++++++++++++++++++++++++++----------- 2 files changed, 41 insertions(+), 50 deletions(-) diff --git a/pci/src/bus.rs b/pci/src/bus.rs index dee437bb0..52ff4bf98 100644 --- a/pci/src/bus.rs +++ b/pci/src/bus.rs @@ -13,12 +13,9 @@ use std::sync::{Arc, Barrier, Mutex}; use byteorder::{ByteOrder, LittleEndian}; use log::warn; use thiserror::Error; -use vm_device::{Bus, BusDevice, BusDeviceSync}; +use vm_device::BusDevice; -use crate::PciBarConfiguration; -use crate::configuration::{ - PciBarRegionType, PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType, -}; +use crate::configuration::{PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType}; use crate::device::{BarReprogrammingParams, DeviceRelocation, Error as PciDeviceError, PciDevice}; /// Denotes the PCI device ID of a bus' root bridge device. @@ -38,12 +35,6 @@ pub enum PciRootError { /// Could not allocate an IRQ number. #[error("Could not allocate an IRQ number")] AllocateIrq, - /// Could not add a device to the port io bus. - #[error("Could not add a device to the port io bus")] - PioInsert(#[source] vm_device::BusError), - /// Could not add a device to the mmio bus. - #[error("Could not add a device to the mmio bus")] - MmioInsert(#[source] vm_device::BusError), /// Could not find an available device slot on the PCI bus. #[error("Could not find an available device slot on the PCI bus")] NoPciDeviceSlotAvailable, @@ -145,31 +136,6 @@ impl PciBus { } } - #[expect(clippy::needless_pass_by_value)] - pub fn register_mapping( - &self, - dev: Arc, - io_bus: &Bus, - mmio_bus: &Bus, - bars: Vec, - ) -> Result<()> { - for bar in bars { - match bar.region_type() { - PciBarRegionType::IoRegion => { - io_bus - .insert(dev.clone(), bar.addr(), bar.size()) - .map_err(PciRootError::PioInsert)?; - } - PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => { - mmio_bus - .insert(dev.clone(), bar.addr(), bar.size()) - .map_err(PciRootError::MmioInsert)?; - } - } - } - Ok(()) - } - pub fn add_device(&mut self, device_id: u8, device: Arc>) -> Result<()> { self.devices.insert(device_id, device); Ok(()) @@ -559,6 +525,7 @@ mod unit_tests { use std::result::Result; use super::*; + use crate::configuration::PciBarRegionType; #[derive(Debug)] /// Helper struct that mocks the implementation of DeviceRelocation diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index d36c49320..7bca1ed78 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -74,8 +74,8 @@ use libc::{ }; use log::{debug, error, info, warn}; use pci::{ - DeviceRelocation, MmioRegion, PciBarRegionType, PciBdf, PciDevice, VfioDmaMapping, - VfioPciDevice, VfioUserDmaMapping, VfioUserPciDevice, VfioUserPciDeviceError, + DeviceRelocation, MmioRegion, PciBarConfiguration, PciBarRegionType, PciBdf, PciDevice, + VfioDmaMapping, VfioPciDevice, VfioUserDmaMapping, VfioUserPciDevice, VfioUserPciDeviceError, }; use rate_limiter::group::RateLimiterGroup; use seccompiler::SeccompAction; @@ -306,6 +306,14 @@ pub enum DeviceManagerError { #[error("Cannot add PCI device")] AddPciDevice(#[source] pci::PciRootError), + /// Could not add a device to the port io bus + #[error("Could not add a device to the port io bus")] + PioInsert(#[source] vm_device::BusError), + + /// Could not add a device to the mmio bus + #[error("Could not add a device to the mmio bus")] + MmioInsert(#[source] vm_device::BusError), + /// Cannot open persistent memory file #[error("Cannot open persistent memory file")] PmemFileOpen(#[source] io::Error), @@ -4120,25 +4128,16 @@ impl DeviceManager { ) .map_err(DeviceManagerError::AllocateBars)?; - let mut pci_bus = self.pci_segments[segment_id as usize] + self.pci_segments[segment_id as usize] .pci_bus .lock() - .unwrap(); - - pci_bus + .unwrap() .add_device(bdf.device(), pci_device) .map_err(DeviceManagerError::AddPciDevice)?; self.bus_devices.push(Arc::clone(&bus_device)); - pci_bus - .register_mapping( - bus_device, - self.address_manager.io_bus.as_ref(), - self.address_manager.mmio_bus.as_ref(), - bars.clone(), - ) - .map_err(DeviceManagerError::AddPciDevice)?; + self.register_bar_mapping(bus_device, &bars)?; let mut new_resources = Vec::new(); for bar in bars { @@ -4154,6 +4153,31 @@ impl DeviceManager { Ok(new_resources) } + #[expect(clippy::needless_pass_by_value)] + fn register_bar_mapping( + &mut self, + bus_device: Arc, + bars: &[PciBarConfiguration], + ) -> DeviceManagerResult<()> { + for bar in bars { + match bar.region_type() { + PciBarRegionType::IoRegion => { + self.address_manager + .io_bus + .insert(bus_device.clone(), bar.addr(), bar.size()) + .map_err(DeviceManagerError::PioInsert)?; + } + PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => { + self.address_manager + .mmio_bus + .insert(bus_device.clone(), bar.addr(), bar.size()) + .map_err(DeviceManagerError::MmioInsert)?; + } + } + } + Ok(()) + } + fn add_vfio_devices( &mut self, snapshot: Option<&Snapshot>,