From da0e0f939b731d89d067fb9382bbdc05e47f4067 Mon Sep 17 00:00:00 2001 From: Tomasz Jeznach Date: Wed, 29 Apr 2020 12:58:11 -0700 Subject: devices: pci: refactor PCI devices to use PciAddress. Simple refactor of PCI device addressing to use PciAddress type providing bus:device.function number. BUG=None TEST=build_test & tast run crostini.Sanity Change-Id: I7755ad6b31aa8c882475cd8212630e1cc86ef49e Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2172766 Reviewed-by: Daniel Verkamp Tested-by: kokoro Commit-Queue: Tomasz Jeznach --- devices/src/pci/ac97.rs | 36 ++++++++++++++++++++--------- devices/src/pci/pci_device.rs | 10 ++++----- devices/src/pci/vfio_pci.rs | 32 +++++++++++++------------- devices/src/usb/xhci/xhci_controller.rs | 25 ++++++++++++--------- devices/src/virtio/gpu/mod.rs | 11 +++++---- devices/src/virtio/virtio_device.rs | 4 ++-- devices/src/virtio/virtio_pci_device.rs | 40 +++++++++++++++++++-------------- 7 files changed, 94 insertions(+), 64 deletions(-) (limited to 'devices') diff --git a/devices/src/pci/ac97.rs b/devices/src/pci/ac97.rs index 228346d..8a6748d 100644 --- a/devices/src/pci/ac97.rs +++ b/devices/src/pci/ac97.rs @@ -23,7 +23,7 @@ use crate::pci::pci_configuration::{ PciBarConfiguration, PciClassCode, PciConfiguration, PciHeaderType, PciMultimediaSubclass, }; use crate::pci::pci_device::{self, PciDevice, Result}; -use crate::pci::PciInterruptPin; +use crate::pci::{PciAddress, PciInterruptPin}; // Use 82801AA because it's what qemu does. const PCI_DEVICE_ID_INTEL_82801AA_5: u16 = 0x2415; @@ -82,7 +82,7 @@ pub struct Ac97Parameters { pub struct Ac97Dev { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, // The irq events are temporarily saved here. They need to be passed to the device after the // jail forks. This happens when the bus is first written. irq_evt: Option, @@ -108,7 +108,7 @@ impl Ac97Dev { Ac97Dev { config_regs, - pci_bus_dev: None, + pci_address: None, irq_evt: None, irq_resample_evt: None, bus_master: Ac97BusMaster::new(mem, audio_server), @@ -216,8 +216,8 @@ impl PciDevice for Ac97Dev { "AC97".to_owned() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn assign_irq( @@ -233,15 +233,20 @@ impl PciDevice for Ac97Dev { } fn allocate_io_bars(&mut self, resources: &mut SystemAllocator) -> Result> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); let mut ranges = Vec::new(); let mixer_regs_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( MIXER_REGS_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, "ac97-mixer_regs".to_string(), MIXER_REGS_SIZE, ) @@ -259,7 +264,12 @@ impl PciDevice for Ac97Dev { .mmio_allocator(MmioType::Low) .allocate_with_align( MASTER_REGS_SIZE, - Alloc::PciBar { bus, dev, bar: 1 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 1, + }, "ac97-master_regs".to_string(), MASTER_REGS_SIZE, ) @@ -338,7 +348,11 @@ mod tests { .add_high_mmio_addresses(0x3000_0000, 0x1000_0000) .create_allocator(5, false) .unwrap(); - ac97_dev.assign_bus_dev(0, 0); + ac97_dev.assign_address(PciAddress { + bus: 0, + dev: 0, + func: 0, + }); assert!(ac97_dev.allocate_io_bars(&mut allocator).is_ok()); } } diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index 2f74ea1..244edbd 100644 --- a/devices/src/pci/pci_device.rs +++ b/devices/src/pci/pci_device.rs @@ -10,7 +10,7 @@ use resources::{Error as SystemAllocatorFaliure, SystemAllocator}; use sys_util::EventFd; use crate::pci::pci_configuration; -use crate::pci::PciInterruptPin; +use crate::pci::{PciAddress, PciInterruptPin}; use crate::BusDevice; #[derive(Debug)] @@ -48,8 +48,8 @@ impl Display for Error { pub trait PciDevice: Send { /// Returns a label suitable for debug output. fn debug_label(&self) -> String; - /// Assign a unique bus and device number to this device. - fn assign_bus_dev(&mut self, _bus: u8, _device: u8 /*u5*/) {} + /// Assign a unique bus, device and function number to this device. + fn assign_address(&mut self, _address: PciAddress) {} /// A vector of device-specific file descriptors that must be kept open /// after jailing. Must be called before the process is jailed. fn keep_fds(&self) -> Vec; @@ -148,8 +148,8 @@ impl PciDevice for Box { fn debug_label(&self) -> String { (**self).debug_label() } - fn assign_bus_dev(&mut self, bus: u8, device: u8 /*u5*/) { - (**self).assign_bus_dev(bus, device) + fn assign_address(&mut self, address: PciAddress) { + (**self).assign_address(address) } fn keep_fds(&self) -> Vec { (**self).keep_fds() diff --git a/devices/src/pci/vfio_pci.rs b/devices/src/pci/vfio_pci.rs index d1b6f2b..7b5c233 100644 --- a/devices/src/pci/vfio_pci.rs +++ b/devices/src/pci/vfio_pci.rs @@ -22,7 +22,7 @@ use crate::pci::msix::{ }; use crate::pci::pci_device::{Error as PciDeviceError, PciDevice}; -use crate::pci::{PciClassCode, PciInterruptPin}; +use crate::pci::{PciAddress, PciClassCode, PciInterruptPin}; use crate::vfio::{VfioDevice, VfioIrqType}; @@ -457,7 +457,7 @@ enum DeviceData { pub struct VfioPciDevice { device: Arc, config: VfioPciConfig, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, interrupt_evt: Option, interrupt_resample_evt: Option, mmio_regions: Vec, @@ -522,7 +522,7 @@ impl VfioPciDevice { VfioPciDevice { device: dev, config, - pci_bus_dev: None, + pci_address: None, interrupt_evt: None, interrupt_resample_evt: None, mmio_regions: Vec::new(), @@ -776,8 +776,8 @@ impl PciDevice for VfioPciDevice { "vfio pci device".to_string() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -816,9 +816,9 @@ impl PciDevice for VfioPciDevice { ) -> Result, PciDeviceError> { let mut ranges = Vec::new(); let mut i = VFIO_PCI_BAR0_REGION_INDEX; - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); while i <= VFIO_PCI_ROM_REGION_INDEX { let mut low: u32 = 0xffffffff; @@ -857,8 +857,9 @@ impl PciDevice for VfioPciDevice { .allocate_with_align( size, Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: i as u8, }, "vfio_bar".to_string(), @@ -917,16 +918,17 @@ impl PciDevice for VfioPciDevice { VFIO_REGION_TYPE_PCI_VENDOR_TYPE | (INTEL_VENDOR_ID as u32), VFIO_REGION_SUBTYPE_INTEL_IGD_OPREGION, ) { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_device_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_device_bars"); let bar_addr = resources .mmio_allocator(MmioType::Low) .allocate( size, Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: (index * 4) as u8, }, "vfio_bar".to_string(), diff --git a/devices/src/usb/xhci/xhci_controller.rs b/devices/src/usb/xhci/xhci_controller.rs index b77a73a..e357ae3 100644 --- a/devices/src/usb/xhci/xhci_controller.rs +++ b/devices/src/usb/xhci/xhci_controller.rs @@ -3,8 +3,8 @@ // found in the LICENSE file. use crate::pci::{ - PciBarConfiguration, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, - PciInterruptPin, PciProgrammingInterface, PciSerialBusSubClass, + PciAddress, PciBarConfiguration, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, + PciHeaderType, PciInterruptPin, PciProgrammingInterface, PciSerialBusSubClass, }; use crate::register_space::{Register, RegisterSpace}; use crate::usb::host_backend::host_backend_device_provider::HostBackendDeviceProvider; @@ -94,7 +94,7 @@ enum XhciControllerState { /// xHCI PCI interface implementation. pub struct XhciController { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, mem: GuestMemory, state: XhciControllerState, } @@ -114,7 +114,7 @@ impl XhciController { ); XhciController { config_regs, - pci_bus_dev: None, + pci_address: None, mem, state: XhciControllerState::Created { device_provider: usb_provider, @@ -166,8 +166,8 @@ impl PciDevice for XhciController { "xhci controller".to_owned() } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -206,15 +206,20 @@ impl PciDevice for XhciController { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); // xHCI spec 5.2.1. let bar0_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( XHCI_BAR0_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, "xhci_bar0".to_string(), XHCI_BAR0_SIZE, ) diff --git a/devices/src/virtio/gpu/mod.rs b/devices/src/virtio/gpu/mod.rs index e8e7f4a..7ba800e 100644 --- a/devices/src/virtio/gpu/mod.rs +++ b/devices/src/virtio/gpu/mod.rs @@ -42,7 +42,9 @@ use self::virtio_2d_backend::Virtio2DBackend; use self::virtio_3d_backend::Virtio3DBackend; #[cfg(feature = "gfxstream")] use self::virtio_gfxstream_backend::VirtioGfxStreamBackend; -use crate::pci::{PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability}; +use crate::pci::{ + PciAddress, PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, PciCapability, +}; use vm_control::VmMemoryControlRequestSocket; @@ -1206,10 +1208,11 @@ impl VirtioDevice for Gpu { } // Require 1 BAR for mapping 3D buffers - fn get_device_bars(&mut self, bus: u8, dev: u8) -> Vec { + fn get_device_bars(&mut self, address: PciAddress) -> Vec { self.pci_bar = Some(Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: GPU_BAR_NUM, }); vec![PciBarConfiguration::new( diff --git a/devices/src/virtio/virtio_device.rs b/devices/src/virtio/virtio_device.rs index 6eb5548..7c07651 100644 --- a/devices/src/virtio/virtio_device.rs +++ b/devices/src/virtio/virtio_device.rs @@ -7,7 +7,7 @@ use std::os::unix::io::RawFd; use sys_util::{EventFd, GuestMemory}; use super::*; -use crate::pci::{MsixStatus, PciBarConfiguration, PciCapability}; +use crate::pci::{MsixStatus, PciAddress, PciBarConfiguration, PciCapability}; /// Trait for virtio devices to be driven by a virtio transport. /// @@ -75,7 +75,7 @@ pub trait VirtioDevice: Send { } /// Returns any additional BAR configuration required by the device. - fn get_device_bars(&mut self, _bus: u8, _dev: u8) -> Vec { + fn get_device_bars(&mut self, _address: PciAddress) -> Vec { Vec::new() } diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index 61409f3..eb91e58 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/devices/src/virtio/virtio_pci_device.rs @@ -15,10 +15,10 @@ use sys_util::{warn, EventFd, GuestMemory, Result}; use super::*; use crate::pci::{ - MsixCap, MsixConfig, PciBarConfiguration, PciCapability, PciCapabilityID, PciClassCode, - PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, PciSubclass, + MsixCap, MsixConfig, PciAddress, PciBarConfiguration, PciCapability, PciCapabilityID, + PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, + PciSubclass, }; - use vm_control::VmIrqRequestSocket; use self::virtio_pci_common_config::VirtioPciCommonConfig; @@ -211,7 +211,7 @@ const VIRTIO_PCI_DEVICE_ID_BASE: u16 = 0x1040; // Add to device type to get devi /// transport for virtio devices. pub struct VirtioPciDevice { config_regs: PciConfiguration, - pci_bus_dev: Option<(u8, u8)>, + pci_address: Option, device: Box, device_activated: bool, @@ -269,7 +269,7 @@ impl VirtioPciDevice { Ok(VirtioPciDevice { config_regs, - pci_bus_dev: None, + pci_address: None, device, device_activated: false, interrupt_status: Arc::new(AtomicUsize::new(0)), @@ -392,8 +392,8 @@ impl PciDevice for VirtioPciDevice { format!("virtio-pci ({})", self.device.debug_label()) } - fn assign_bus_dev(&mut self, bus: u8, device: u8) { - self.pci_bus_dev = Some((bus, device)); + fn assign_address(&mut self, address: PciAddress) { + self.pci_address = Some(address); } fn keep_fds(&self) -> Vec { @@ -425,16 +425,21 @@ impl PciDevice for VirtioPciDevice { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_io_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_io_bars"); // Allocate one bar for the structures pointed to by the capability structures. let mut ranges = Vec::new(); let settings_config_addr = resources .mmio_allocator(MmioType::Low) .allocate_with_align( CAPABILITY_BAR_SIZE, - Alloc::PciBar { bus, dev, bar: 0 }, + Alloc::PciBar { + bus: address.bus, + dev: address.dev, + func: address.func, + bar: 0, + }, format!( "virtio-{}-cap_bar", type_to_str(self.device.device_type()).unwrap_or("?") @@ -463,18 +468,19 @@ impl PciDevice for VirtioPciDevice { &mut self, resources: &mut SystemAllocator, ) -> std::result::Result, PciDeviceError> { - let (bus, dev) = self - .pci_bus_dev - .expect("assign_bus_dev must be called prior to allocate_device_bars"); + let address = self + .pci_address + .expect("assign_address must be called prior to allocate_device_bars"); let mut ranges = Vec::new(); - for config in self.device.get_device_bars(bus, dev) { + for config in self.device.get_device_bars(address) { let device_addr = resources .mmio_allocator(MmioType::High) .allocate_with_align( config.get_size(), Alloc::PciBar { - bus, - dev, + bus: address.bus, + dev: address.dev, + func: address.func, bar: config.get_register_index() as u8, }, format!( -- cgit 1.4.1