From 2bb0b22cc1d3003e487632593d68c43ff69d34d9 Mon Sep 17 00:00:00 2001 From: Jing Liu Date: Tue, 4 Jun 2019 14:51:00 +0800 Subject: [PATCH] pci: Refine pci topology PciConfigIo is a legacy pci bus dispatcher, which manages all pci devices including a pci root bridge. However, it is unnecessary to design a complex hierarchy which redirects every access by PciRoot. Since pci root bridge is also a pci device instance, and only contains easy config space read/write, and PciConfigIo actually acts as a pci bus to dispatch resource based resolving when VMExit, we re-arrange to make the pci hierarchy clean. Signed-off-by: Jing Liu --- pci/src/{root.rs => bus.rs} | 219 ++++++++++++++++++------------------ pci/src/device.rs | 4 +- pci/src/lib.rs | 9 +- vmm/src/vm.rs | 23 ++-- 4 files changed, 125 insertions(+), 130 deletions(-) rename pci/src/{root.rs => bus.rs} (67%) mode change 100755 => 100644 diff --git a/pci/src/root.rs b/pci/src/bus.rs old mode 100755 new mode 100644 similarity index 67% rename from pci/src/root.rs rename to pci/src/bus.rs index 133fe1b21..54e779df4 --- a/pci/src/root.rs +++ b/pci/src/bus.rs @@ -26,25 +26,20 @@ pub enum PciRootError { } pub type Result = std::result::Result; -/// Emulates the PCI Root bridge. +/// Emulates the PCI Root bridge device. pub struct PciRoot { - /// Bus configuration for the root device. - configuration: PciConfiguration, - /// Devices attached to this bridge. - devices: Vec>>, + /// Configuration space. + config: PciConfiguration, } impl PciRoot { - /// Create an empty PCI root bus. - pub fn new(configuration: Option) -> Self { - if let Some(config) = configuration { - PciRoot { - configuration: config, - devices: Vec::new(), - } + /// Create an empty PCI root bridge. + pub fn new(config: Option) -> Self { + if let Some(config) = config { + PciRoot { config } } else { PciRoot { - configuration: PciConfiguration::new( + config: PciConfiguration::new( VENDOR_ID_INTEL, DEVICE_ID_INTEL_VIRT_PCIE_HOST, PciClassCode::BridgeDevice, @@ -55,126 +50,102 @@ impl PciRoot { 0, None, ), - devices: Vec::new(), - } - } - } - - /// Add a `device` to this root PCI bus. - pub fn add_device(&mut self, pci_device: Arc>) -> Result<()> { - self.devices.push(pci_device); - Ok(()) - } - - /// Register Guest Address mapping of a `device` to IO bus. - pub fn register_mapping( - &self, - device: Arc>, - bus: &mut devices::Bus, - bars: Vec<(GuestAddress, GuestUsize)>, - ) -> Result<()> { - for (address, size) in bars { - bus.insert(device.clone(), address.raw_value(), size) - .map_err(PciRootError::MmioInsert)?; - } - Ok(()) - } - pub fn config_space_read( - &self, - bus: usize, - device: usize, - _function: usize, - register: usize, - ) -> u32 { - // Only support one bus. - if bus != 0 { - return 0xffff_ffff; - } - - match device { - 0 => { - // If bus and device are both zero, then read from the root config. - self.configuration.read_config_register(register) - } - dev_num => self.devices.get(dev_num - 1).map_or(0xffff_ffff, |d| { - d.lock().unwrap().read_config_register(register) - }), - } - } - - pub fn config_space_write( - &mut self, - bus: usize, - device: usize, - _function: usize, - register: usize, - offset: u64, - data: &[u8], - ) { - if offset as usize + data.len() > 4 { - return; - } - - // Only support one bus. - if bus != 0 { - return; - } - - match device { - 0 => { - // If bus and device are both zero, then read from the root config. - self.configuration - .write_config_register(register, offset, data); - } - dev_num => { - if let Some(d) = self.devices.get(dev_num - 1) { - d.lock() - .unwrap() - .write_config_register(register, offset, data); - } } } } } -/// Emulates PCI configuration access mechanism #1 (I/O ports 0xcf8 and 0xcfc). +impl BusDevice for PciRoot {} + +impl PciDevice for PciRoot { + fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) { + self.config.write_config_register(reg_idx, offset, data); + } + + fn read_config_register(&self, reg_idx: usize) -> u32 { + self.config.read_reg(reg_idx) + } +} + pub struct PciConfigIo { - /// PCI root bridge. - pci_root: PciRoot, - /// Current address to read/write from (0xcf8 register, litte endian). + /// Devices attached to this bus. + /// Device 0 is host bridge. + devices: Vec>>, + /// Config space register. config_address: u32, } impl PciConfigIo { pub fn new(pci_root: PciRoot) -> Self { + let mut devices: Vec>> = Vec::new(); + devices.push(Arc::new(Mutex::new(pci_root))); + PciConfigIo { - pci_root, + devices, config_address: 0, } } - fn config_space_read(&self) -> u32 { + pub fn register_mapping( + &self, + dev: Arc>, + bus: &mut devices::Bus, + bars: Vec<(GuestAddress, GuestUsize)>, + ) -> Result<()> { + for (address, size) in bars { + bus.insert(dev.clone(), address.raw_value(), size) + .map_err(PciRootError::MmioInsert)?; + } + Ok(()) + } + + pub fn add_device(&mut self, device: Arc>) -> Result<()> { + self.devices.push(device); + Ok(()) + } + + pub fn config_space_read(&self) -> u32 { let enabled = (self.config_address & 0x8000_0000) != 0; if !enabled { return 0xffff_ffff; } - let (bus, device, function, register) = + let (bus, device, _function, register) = parse_config_address(self.config_address & !0x8000_0000); - self.pci_root - .config_space_read(bus, device, function, register) + + // Only support one bus. + if bus != 0 { + return 0xffff_ffff; + } + + self.devices.get(device).map_or(0xffff_ffff, |d| { + d.lock().unwrap().read_config_register(register) + }) } - fn config_space_write(&mut self, offset: u64, data: &[u8]) { + pub fn config_space_write(&mut self, offset: u64, data: &[u8]) { + if offset as usize + data.len() > 4 { + return; + } + let enabled = (self.config_address & 0x8000_0000) != 0; if !enabled { return; } - let (bus, device, function, register) = + let (bus, device, _function, register) = parse_config_address(self.config_address & !0x8000_0000); - self.pci_root - .config_space_write(bus, device, function, register, offset, data) + + // Only support one bus. + if bus != 0 { + return; + } + + if let Some(d) = self.devices.get(device) { + d.lock() + .unwrap() + .write_config_register(register, offset, data); + } } fn set_config_address(&mut self, offset: u64, data: &[u8]) { @@ -232,25 +203,49 @@ impl BusDevice for PciConfigIo { /// Emulates PCI memory-mapped configuration access mechanism. pub struct PciConfigMmio { - /// PCI root bridge. - pci_root: PciRoot, + /// Devices attached to this bus. + /// Device 0 is host bridge. + devices: Vec>>, } impl PciConfigMmio { pub fn new(pci_root: PciRoot) -> Self { - PciConfigMmio { pci_root } + let mut devices: Vec>> = Vec::new(); + + devices.push(Arc::new(Mutex::new(pci_root))); + PciConfigMmio { devices } } fn config_space_read(&self, config_address: u32) -> u32 { - let (bus, device, function, register) = parse_config_address(config_address); - self.pci_root - .config_space_read(bus, device, function, register) + let (bus, device, _function, register) = parse_config_address(config_address); + + // Only support one bus. + if bus != 0 { + return 0xffff_ffff; + } + + self.devices.get(device).map_or(0xffff_ffff, |d| { + d.lock().unwrap().read_config_register(register) + }) } fn config_space_write(&mut self, config_address: u32, offset: u64, data: &[u8]) { - let (bus, device, function, register) = parse_config_address(config_address); - self.pci_root - .config_space_write(bus, device, function, register, offset, data) + if offset as usize + data.len() > 4 { + return; + } + + let (bus, device, _function, register) = parse_config_address(config_address); + + // Only support one bus. + if bus != 0 { + return; + } + + if let Some(d) = self.devices.get(device) { + d.lock() + .unwrap() + .write_config_register(register, offset, data); + } } } diff --git a/pci/src/device.rs b/pci/src/device.rs index 25111700a..24e04da75 100755 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -90,11 +90,11 @@ pub trait PciDevice: BusDevice { /// Reads from a BAR region mapped in to the device. /// * `addr` - The guest address inside the BAR. /// * `data` - Filled with the data from `addr`. - fn read_bar(&mut self, base: u64, offset: u64, data: &mut [u8]); + fn read_bar(&mut self, _base: u64, _offset: u64, _data: &mut [u8]) {} /// Writes to a BAR region mapped in to the device. /// * `addr` - The guest address inside the BAR. /// * `data` - The data to write. - fn write_bar(&mut self, base: u64, offset: u64, data: &[u8]); + fn write_bar(&mut self, _base: u64, _offset: u64, _data: &[u8]) {} /// Invoked when the device is sandboxed. fn on_device_sandboxed(&mut self) {} } diff --git a/pci/src/lib.rs b/pci/src/lib.rs index 05cb03d38..f58cd8bda 100644 --- a/pci/src/lib.rs +++ b/pci/src/lib.rs @@ -10,20 +10,21 @@ extern crate kvm_ioctls; extern crate vm_memory; extern crate vmm_sys_util; +mod bus; mod configuration; mod device; mod msix; -mod root; +pub use self::bus::{PciConfigIo, PciConfigMmio, PciRoot, PciRootError}; pub use self::configuration::{ PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciHeaderType, PciMassStorageSubclass, PciNetworkControllerSubclass, PciProgrammingInterface, PciSerialBusSubClass, PciSubclass, }; -pub use self::device::Error as PciDeviceError; -pub use self::device::{InterruptDelivery, InterruptParameters, PciDevice}; +pub use self::device::{ + Error as PciDeviceError, InterruptDelivery, InterruptParameters, PciDevice, +}; pub use self::msix::{MsixCap, MsixConfig, MsixTableEntry}; -pub use self::root::{PciConfigIo, PciConfigMmio, PciRoot, PciRootError}; /// PCI has four interrupt pins A->D. #[derive(Copy, Clone)] diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 66cff3c57..68c674f30 100755 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -525,7 +525,8 @@ impl DeviceManager { exit_evt.try_clone().map_err(DeviceManagerError::EventFd)?, ))); - let mut pci_root = PciRoot::new(None); + let pci_root = PciRoot::new(None); + let mut pci = PciConfigIo::new(pci_root); for disk_cfg in &vm_cfg.disks { // Open block device path @@ -559,7 +560,7 @@ impl DeviceManager { memory.clone(), allocator, vm_fd, - &mut pci_root, + &mut pci, &mut mmio_bus, &interrupt_info, )?; @@ -584,7 +585,7 @@ impl DeviceManager { memory.clone(), allocator, vm_fd, - &mut pci_root, + &mut pci, &mut mmio_bus, &interrupt_info, )?; @@ -600,7 +601,7 @@ impl DeviceManager { memory.clone(), allocator, vm_fd, - &mut pci_root, + &mut pci, &mut mmio_bus, &interrupt_info, )?; @@ -623,7 +624,7 @@ impl DeviceManager { memory.clone(), allocator, vm_fd, - &mut pci_root, + &mut pci, &mut mmio_bus, &interrupt_info, )?; @@ -688,14 +689,14 @@ impl DeviceManager { memory.clone(), allocator, vm_fd, - &mut pci_root, + &mut pci, &mut mmio_bus, &interrupt_info, )?; } } - let pci = Arc::new(Mutex::new(PciConfigIo::new(pci_root))); + let pci = Arc::new(Mutex::new(pci)); Ok(DeviceManager { io_bus, @@ -713,7 +714,7 @@ impl DeviceManager { memory: GuestMemoryMmap, allocator: &mut SystemAllocator, vm_fd: &Arc, - pci_root: &mut PciRoot, + pci: &mut PciConfigIo, mmio_bus: &mut devices::Bus, interrupt_info: &InterruptInfo, ) -> DeviceManagerResult<()> { @@ -804,12 +805,10 @@ impl DeviceManager { let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device)); - pci_root - .add_device(virtio_pci_device.clone()) + pci.add_device(virtio_pci_device.clone()) .map_err(DeviceManagerError::AddPciDevice)?; - pci_root - .register_mapping(virtio_pci_device.clone(), mmio_bus, bars) + pci.register_mapping(virtio_pci_device.clone(), mmio_bus, bars) .map_err(DeviceManagerError::AddPciDevice)?; Ok(())