diff --git a/src/pci/src/bus.rs b/src/pci/src/bus.rs index adfac6c12fb..eaa2923e8db 100644 --- a/src/pci/src/bus.rs +++ b/src/pci/src/bus.rs @@ -11,13 +11,10 @@ use std::ops::DerefMut; use std::sync::{Arc, Barrier, Mutex}; use byteorder::{ByteOrder, LittleEndian}; -use vm_device::{Bus, BusDevice, BusDeviceSync}; +use vm_device::BusDevice; -use crate::configuration::{ - PciBarRegionType, PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType, -}; +use crate::configuration::{PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType}; use crate::device::{DeviceRelocation, Error as PciDeviceError, PciDevice}; -use crate::PciBarConfiguration; const VENDOR_ID_INTEL: u16 = 0x8086; const DEVICE_ID_INTEL_VIRT_PCIE_HOST: u16 = 0x0d57; @@ -123,40 +120,11 @@ impl PciBus { } } - 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: u32, device: Arc>) -> Result<()> { self.devices.insert(device_id, device); Ok(()) } - pub fn remove_by_device(&mut self, device: &Arc>) -> Result<()> { - self.devices.retain(|_, dev| !Arc::ptr_eq(dev, device)); - Ok(()) - } - pub fn next_device_id(&mut self) -> Result { for (idx, device_id) in self.device_ids.iter_mut().enumerate() { if !(*device_id) { @@ -167,28 +135,6 @@ impl PciBus { Err(PciRootError::NoPciDeviceSlotAvailable) } - - pub fn get_device_id(&mut self, id: usize) -> Result<()> { - if id < NUM_DEVICE_IDS { - if !self.device_ids[id] { - self.device_ids[id] = true; - Ok(()) - } else { - Err(PciRootError::AlreadyInUsePciDeviceSlot(id)) - } - } else { - Err(PciRootError::InvalidPciDeviceSlot(id)) - } - } - - pub fn put_device_id(&mut self, id: usize) -> Result<()> { - if id < NUM_DEVICE_IDS { - self.device_ids[id] = false; - Ok(()) - } else { - Err(PciRootError::InvalidPciDeviceSlot(id)) - } - } } pub struct PciConfigIo { diff --git a/src/pci/src/configuration.rs b/src/pci/src/configuration.rs index c37f8026fbe..3a2639ca876 100644 --- a/src/pci/src/configuration.rs +++ b/src/pci/src/configuration.rs @@ -10,10 +10,9 @@ use std::sync::{Arc, Mutex}; use byteorder::{ByteOrder, LittleEndian}; use serde::{Deserialize, Serialize}; -use vm_device::PciBarType; use crate::device::BarReprogrammingParams; -use crate::{MsixConfig, PciInterruptPin}; +use crate::MsixConfig; // The number of 32bit registers in the config space, 4096 bytes. const NUM_CONFIGURATION_REGISTERS: usize = 1024; @@ -22,7 +21,6 @@ const STATUS_REG: usize = 1; const STATUS_REG_CAPABILITIES_USED_MASK: u32 = 0x0010_0000; const BAR0_REG: usize = 4; const ROM_BAR_REG: usize = 12; -const ROM_BAR_IDX: usize = 6; const BAR_IO_ADDR_MASK: u32 = 0xffff_fffc; const BAR_MEM_ADDR_MASK: u32 = 0xffff_fff0; const ROM_BAR_ADDR_MASK: u32 = 0xffff_f800; @@ -33,8 +31,6 @@ const CAPABILITY_LIST_HEAD_OFFSET: usize = 0x34; const FIRST_CAPABILITY_OFFSET: usize = 0x40; const CAPABILITY_MAX_OFFSET: usize = 192; -const INTERRUPT_LINE_PIN_REG: usize = 15; - pub const PCI_CONFIGURATION_ID: &str = "pci_configuration"; /// Represents the types of PCI headers allowed in the configuration registers. @@ -446,26 +442,6 @@ pub enum PciBarRegionType { Memory64BitRegion = 0x04, } -impl From for PciBarRegionType { - fn from(type_: PciBarType) -> Self { - match type_ { - PciBarType::Io => PciBarRegionType::IoRegion, - PciBarType::Mmio32 => PciBarRegionType::Memory32BitRegion, - PciBarType::Mmio64 => PciBarRegionType::Memory64BitRegion, - } - } -} - -impl From for PciBarType { - fn from(val: PciBarRegionType) -> Self { - match val { - PciBarRegionType::IoRegion => PciBarType::Io, - PciBarRegionType::Memory32BitRegion => PciBarType::Mmio32, - PciBarRegionType::Memory64BitRegion => PciBarType::Mmio64, - } - } -} - #[derive(Debug, Copy, Clone, Serialize, Deserialize)] pub enum PciBarPrefetchable { NotPrefetchable = 0, @@ -483,11 +459,11 @@ impl From for bool { #[derive(Debug, Copy, Clone, Serialize, Deserialize)] pub struct PciBarConfiguration { - addr: u64, - size: u64, - idx: usize, - region_type: PciBarRegionType, - prefetchable: PciBarPrefetchable, + pub addr: u64, + pub size: u64, + pub idx: usize, + pub region_type: PciBarRegionType, + pub prefetchable: PciBarPrefetchable, } #[derive(Debug)] @@ -797,42 +773,6 @@ impl PciConfiguration { Ok(()) } - /// Adds rom expansion BAR. - pub fn add_pci_rom_bar(&mut self, config: &PciBarConfiguration, active: u32) -> Result<()> { - let bar_idx = config.idx; - let reg_idx = ROM_BAR_REG; - - if self.rom_bar_used { - return Err(Error::RomBarInUse(bar_idx)); - } - - if !config.size.is_power_of_two() { - return Err(Error::RomBarSizeInvalid(config.size)); - } - - if bar_idx != ROM_BAR_IDX { - return Err(Error::RomBarInvalid(bar_idx)); - } - - let end_addr = config - .addr - .checked_add(config.size - 1) - .ok_or(Error::RomBarAddressInvalid(config.addr, config.size))?; - - if end_addr > u64::from(u32::MAX) { - return Err(Error::RomBarAddressInvalid(config.addr, config.size)); - } - - self.registers[reg_idx] = (config.addr as u32) | active; - self.writable_bits[reg_idx] = ROM_BAR_ADDR_MASK; - self.rom_bar_addr = self.registers[reg_idx]; - self.rom_bar_size = - encode_32_bits_bar_size(config.size as u32).ok_or(Error::Encode32BarSize)?; - self.rom_bar_used = true; - - Ok(()) - } - /// Returns the address of the given BAR region. pub fn get_bar_addr(&self, bar_num: usize) -> u64 { let bar_idx = BAR0_REG + bar_num; @@ -848,16 +788,6 @@ impl PciConfiguration { addr } - /// Configures the IRQ line and pin used by this device. - pub fn set_irq(&mut self, line: u8, pin: PciInterruptPin) { - // `pin` is 1-based in the pci config space. - let pin_idx = (pin as u32) + 1; - self.registers[INTERRUPT_LINE_PIN_REG] = (self.registers[INTERRUPT_LINE_PIN_REG] - & 0xffff_0000) - | (pin_idx << 8) - | u32::from(line); - } - /// Adds the capability `cap_data` to the list of capabilities. /// `cap_data` should include the two-byte PCI capability header (type, next), /// but not populate it. Correct values will be generated automatically based @@ -940,10 +870,6 @@ impl PciConfiguration { } } - pub fn read_config_register(&self, reg_idx: usize) -> u32 { - self.read_reg(reg_idx) - } - pub fn detect_bar_reprogramming( &mut self, reg_idx: usize, @@ -1074,73 +1000,6 @@ impl Default for PciBarConfiguration { } } -impl PciBarConfiguration { - pub fn new( - idx: usize, - size: u64, - region_type: PciBarRegionType, - prefetchable: PciBarPrefetchable, - ) -> Self { - PciBarConfiguration { - idx, - addr: 0, - size, - region_type, - prefetchable, - } - } - - #[must_use] - pub fn set_index(mut self, idx: usize) -> Self { - self.idx = idx; - self - } - - #[must_use] - pub fn set_address(mut self, addr: u64) -> Self { - self.addr = addr; - self - } - - #[must_use] - pub fn set_size(mut self, size: u64) -> Self { - self.size = size; - self - } - - #[must_use] - pub fn set_region_type(mut self, region_type: PciBarRegionType) -> Self { - self.region_type = region_type; - self - } - - #[must_use] - pub fn set_prefetchable(mut self, prefetchable: PciBarPrefetchable) -> Self { - self.prefetchable = prefetchable; - self - } - - pub fn idx(&self) -> usize { - self.idx - } - - pub fn addr(&self) -> u64 { - self.addr - } - - pub fn size(&self) -> u64 { - self.size - } - - pub fn region_type(&self) -> PciBarRegionType { - self.region_type - } - - pub fn prefetchable(&self) -> PciBarPrefetchable { - self.prefetchable - } -} - #[cfg(test)] mod tests { use vm_memory::ByteValued; diff --git a/src/pci/src/device.rs b/src/pci/src/device.rs index bf89331faa9..57f5e63eaeb 100644 --- a/src/pci/src/device.rs +++ b/src/pci/src/device.rs @@ -10,7 +10,6 @@ use std::sync::{Arc, Barrier}; use std::{io, result}; use vm_allocator::AddressAllocator; -use vm_device::Resource; use crate::configuration::{self, PciBarRegionType}; use crate::PciBarConfiguration; @@ -25,8 +24,6 @@ pub enum Error { IoRegistrationFailed(u64, configuration::Error), /// Expected resource not found. MissingResource, - /// Invalid resource - InvalidResource(Resource), } pub type Result = std::result::Result; @@ -45,7 +42,6 @@ pub trait PciDevice: Send { &mut self, _mmio32_allocator: &mut AddressAllocator, _mmio64_allocator: &mut AddressAllocator, - _resources: Option>, ) -> Result> { Ok(Vec::new()) } diff --git a/src/vmm/src/device_manager/pci_mngr.rs b/src/vmm/src/device_manager/pci_mngr.rs index 0727f76d269..578d521162b 100644 --- a/src/vmm/src/device_manager/pci_mngr.rs +++ b/src/vmm/src/device_manager/pci_mngr.rs @@ -88,32 +88,14 @@ impl PciDevices { vm: &Vm, virtio_device: &Arc>, ) -> Result<(), PciManagerError> { - for bar in &virtio_device.lock().expect("Poisoned lock").bar_regions { - match bar.region_type() { - PciBarRegionType::IoRegion => { - debug!( - "Inserting I/O BAR region: {:#x}:{:#x}", - bar.addr(), - bar.size() - ); - #[cfg(target_arch = "x86_64")] - vm.pio_bus - .insert(virtio_device.clone(), bar.addr(), bar.size())?; - #[cfg(target_arch = "aarch64")] - log::error!("pci: We do not support I/O region allocation") - } - PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => { - debug!( - "Inserting MMIO BAR region: {:#x}:{:#x}", - bar.addr(), - bar.size() - ); - vm.common - .mmio_bus - .insert(virtio_device.clone(), bar.addr(), bar.size())?; - } - } - } + let virtio_device_locked = virtio_device.lock().expect("Poisoned lock"); + let bar = &virtio_device_locked.bar_region; + assert_eq!(bar.region_type, PciBarRegionType::Memory64BitRegion); + + debug!("Inserting MMIO BAR region: {:#x}:{:#x}", bar.addr, bar.size); + vm.common + .mmio_bus + .insert(virtio_device.clone(), bar.addr, bar.size)?; Ok(()) } @@ -151,7 +133,6 @@ impl PciDevices { virtio_device.allocate_bars( &mut resource_allocator.mmio32_memory, &mut resource_allocator.mmio64_memory, - None, )?; let virtio_device = Arc::new(Mutex::new(virtio_device)); diff --git a/src/vmm/src/devices/virtio/transport/pci/device.rs b/src/vmm/src/devices/virtio/transport/pci/device.rs index 7ee580fc6a1..0c4b275bb96 100644 --- a/src/vmm/src/devices/virtio/transport/pci/device.rs +++ b/src/vmm/src/devices/virtio/transport/pci/device.rs @@ -27,7 +27,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use vm_allocator::{AddressAllocator, AllocPolicy, RangeInclusive}; use vm_device::interrupt::{InterruptIndex, InterruptSourceGroup, MsiIrqGroupConfig}; -use vm_device::{BusDevice, PciBarType, Resource}; +use vm_device::{BusDevice, PciBarType}; use vm_memory::{Address, ByteValued, GuestAddress, Le32}; use vmm_sys_util::errno; use vmm_sys_util::eventfd::EventFd; @@ -67,6 +67,9 @@ const VIRTIO_F_NOTIFICATION_DATA: u32 = 38; /// Vector value used to disable MSI for a queue. const VIRTQ_MSI_NO_VECTOR: u16 = 0xffff; +/// BAR index we are using for VirtIO configuration +const VIRTIO_BAR_INDEX: u8 = 0; + enum PciCapabilityType { Common = 1, Notify = 2, @@ -110,12 +113,12 @@ impl PciCapability for VirtioPciCap { const VIRTIO_PCI_CAP_LEN_OFFSET: u8 = 2; impl VirtioPciCap { - pub fn new(cfg_type: PciCapabilityType, pci_bar: u8, offset: u32, length: u32) -> Self { + pub fn new(cfg_type: PciCapabilityType, offset: u32, length: u32) -> Self { VirtioPciCap { cap_len: u8::try_from(std::mem::size_of::()).unwrap() + VIRTIO_PCI_CAP_LEN_OFFSET, cfg_type: cfg_type as u8, - pci_bar, + pci_bar: VIRTIO_BAR_INDEX, id: 0, padding: [0; 2], offset: Le32::from(offset), @@ -145,19 +148,13 @@ impl PciCapability for VirtioPciNotifyCap { } impl VirtioPciNotifyCap { - pub fn new( - cfg_type: PciCapabilityType, - pci_bar: u8, - offset: u32, - length: u32, - multiplier: Le32, - ) -> Self { + pub fn new(cfg_type: PciCapabilityType, offset: u32, length: u32, multiplier: Le32) -> Self { VirtioPciNotifyCap { cap: VirtioPciCap { cap_len: u8::try_from(std::mem::size_of::()).unwrap() + VIRTIO_PCI_CAP_LEN_OFFSET, cfg_type: cfg_type as u8, - pci_bar, + pci_bar: VIRTIO_BAR_INDEX, id: 0, padding: [0; 2], offset: Le32::from(offset), @@ -231,7 +228,7 @@ impl PciCapability for VirtioPciCfgCap { impl VirtioPciCfgCap { fn new() -> Self { VirtioPciCfgCap { - cap: VirtioPciCap::new(PciCapabilityType::Pci, 0, 0, 0), + cap: VirtioPciCap::new(PciCapabilityType::Pci, 0, 0), ..Default::default() } } @@ -306,7 +303,7 @@ pub struct VirtioPciDeviceState { pub pci_dev_state: VirtioPciCommonConfigState, pub msix_state: MsixConfigState, pub msi_vector_group: Vec, - pub bar_configuration: Vec, + pub bar_configuration: PciBarConfiguration, } #[derive(Debug, thiserror::Error, displaydoc::Display)] @@ -348,12 +345,6 @@ pub struct VirtioPciDevice { // Guest memory memory: GuestMemoryMmap, - // Settings PCI BAR - settings_bar: u8, - - // Whether to use 64-bit bar location or 32-bit - use_64bit_bar: bool, - // Add a dedicated structure to hold information about the very specific // virtio-pci capability VIRTIO_PCI_CAP_PCI_CFG. This is needed to support // the legacy/backward compatible mechanism of letting the guest access the @@ -362,8 +353,8 @@ pub struct VirtioPciDevice { // a device. cap_pci_cfg_info: VirtioPciCfgCapInfo, - // Details of bar regions to free - pub bar_regions: Vec, + // Details of BAR region + pub bar_region: PciBarConfiguration, } impl Debug for VirtioPciDevice { @@ -471,11 +462,9 @@ impl VirtioPciDevice { interrupt_status: Arc::new(AtomicUsize::new(0)), virtio_interrupt: Some(interrupt), memory, - settings_bar: 0, - use_64bit_bar: true, interrupt_source_group: msi_vectors, cap_pci_cfg_info: VirtioPciCfgCapInfo::default(), - bar_regions: vec![], + bar_region: PciBarConfiguration::default(), }; Ok(virtio_pci_device) @@ -524,11 +513,9 @@ impl VirtioPciDevice { interrupt_status: Arc::new(AtomicUsize::new(state.interrupt_status)), virtio_interrupt: Some(interrupt), memory: memory.clone(), - settings_bar: 0, - use_64bit_bar: true, interrupt_source_group: msi_vectors, cap_pci_cfg_info, - bar_regions: state.bar_configuration, + bar_region: state.bar_configuration, }; if state.device_activated { @@ -558,17 +545,13 @@ impl VirtioPciDevice { } pub fn config_bar_addr(&self) -> u64 { - self.configuration.get_bar_addr(self.settings_bar as usize) + self.configuration.get_bar_addr(VIRTIO_BAR_INDEX as usize) } - fn add_pci_capabilities( - &mut self, - settings_bar: u8, - ) -> std::result::Result<(), PciDeviceError> { + fn add_pci_capabilities(&mut self) -> std::result::Result<(), PciDeviceError> { // Add pointers to the different configuration structures from the PCI capabilities. let common_cap = VirtioPciCap::new( PciCapabilityType::Common, - settings_bar, COMMON_CONFIG_BAR_OFFSET.try_into().unwrap(), COMMON_CONFIG_SIZE.try_into().unwrap(), ); @@ -578,7 +561,6 @@ impl VirtioPciDevice { let isr_cap = VirtioPciCap::new( PciCapabilityType::Isr, - settings_bar, ISR_CONFIG_BAR_OFFSET.try_into().unwrap(), ISR_CONFIG_SIZE.try_into().unwrap(), ); @@ -589,7 +571,6 @@ impl VirtioPciDevice { // TODO(dgreid) - set based on device's configuration size? let device_cap = VirtioPciCap::new( PciCapabilityType::Device, - settings_bar, DEVICE_CONFIG_BAR_OFFSET.try_into().unwrap(), DEVICE_CONFIG_SIZE.try_into().unwrap(), ); @@ -599,7 +580,6 @@ impl VirtioPciDevice { let notify_cap = VirtioPciNotifyCap::new( PciCapabilityType::Notify, - settings_bar, NOTIFICATION_BAR_OFFSET.try_into().unwrap(), NOTIFICATION_SIZE.try_into().unwrap(), Le32::from(NOTIFY_OFF_MULTIPLIER), @@ -618,10 +598,10 @@ impl VirtioPciDevice { if self.msix_config.is_some() { let msix_cap = MsixCap::new( - settings_bar, + VIRTIO_BAR_INDEX, self.msix_num, MSIX_TABLE_BAR_OFFSET.try_into().unwrap(), - settings_bar, + VIRTIO_BAR_INDEX, MSIX_PBA_BAR_OFFSET.try_into().unwrap(), ); self.configuration @@ -629,7 +609,6 @@ impl VirtioPciDevice { .map_err(PciDeviceError::CapabilitiesSetup)?; } - self.settings_bar = settings_bar; Ok(()) } @@ -744,7 +723,7 @@ impl VirtioPciDevice { .expect("Poisoned lock") .state(), msi_vector_group: self.interrupt_source_group.save(), - bar_configuration: self.bar_regions.clone(), + bar_configuration: self.bar_region, } } } @@ -886,44 +865,29 @@ impl PciDevice for VirtioPciDevice { &mut self, mmio32_allocator: &mut AddressAllocator, mmio64_allocator: &mut AddressAllocator, - _resources: Option>, ) -> std::result::Result, PciDeviceError> { - let mut bars = Vec::new(); let device_clone = self.device.clone(); let device = device_clone.lock().unwrap(); // Allocate the virtio-pci capability BAR. // See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004 - let (virtio_pci_bar_addr, region_type) = if self.use_64bit_bar { - let region_type = PciBarRegionType::Memory64BitRegion; - let addr = mmio64_allocator - .allocate( - CAPABILITY_BAR_SIZE, - CAPABILITY_BAR_SIZE, - AllocPolicy::FirstMatch, - ) - .unwrap() - .start(); - (addr, region_type) - } else { - let region_type = PciBarRegionType::Memory32BitRegion; - let addr = mmio32_allocator - .allocate( - CAPABILITY_BAR_SIZE, - CAPABILITY_BAR_SIZE, - AllocPolicy::FirstMatch, - ) - .unwrap() - .start(); - (addr, region_type) + let virtio_pci_bar_addr = mmio64_allocator + .allocate( + CAPABILITY_BAR_SIZE, + CAPABILITY_BAR_SIZE, + AllocPolicy::FirstMatch, + ) + .unwrap() + .start(); + + let bar = PciBarConfiguration { + addr: virtio_pci_bar_addr, + size: CAPABILITY_BAR_SIZE, + idx: VIRTIO_COMMON_BAR_INDEX, + region_type: PciBarRegionType::Memory64BitRegion, + prefetchable: pci::PciBarPrefetchable::NotPrefetchable, }; - let bar = PciBarConfiguration::default() - .set_index(VIRTIO_COMMON_BAR_INDEX) - .set_address(virtio_pci_bar_addr) - .set_size(CAPABILITY_BAR_SIZE) - .set_region_type(region_type); - // The creation of the PCI BAR and its associated capabilities must // happen only during the creation of a brand new VM. When a VM is // restored from a known state, the BARs are already created with the @@ -933,13 +897,10 @@ impl PciDevice for VirtioPciDevice { .map_err(|e| PciDeviceError::IoRegistrationFailed(virtio_pci_bar_addr, e))?; // Once the BARs are allocated, the capabilities can be added to the PCI configuration. - self.add_pci_capabilities(VIRTIO_COMMON_BAR_INDEX.try_into().unwrap())?; - - bars.push(bar); - - self.bar_regions.clone_from(&bars); + self.add_pci_capabilities()?; + self.bar_region = bar; - Ok(bars) + Ok(vec![bar]) } fn free_bars( @@ -947,18 +908,17 @@ impl PciDevice for VirtioPciDevice { mmio32_allocator: &mut AddressAllocator, mmio64_allocator: &mut AddressAllocator, ) -> std::result::Result<(), PciDeviceError> { - for bar in self.bar_regions.drain(..) { - let range = RangeInclusive::new(bar.addr(), bar.addr() + bar.size()).unwrap(); - match bar.region_type() { - PciBarRegionType::Memory32BitRegion => { - mmio32_allocator.free(&range); - } - PciBarRegionType::Memory64BitRegion => { - mmio64_allocator.free(&range); - } - _ => error!("Unexpected PCI bar type"), - } - } + assert_eq!( + self.bar_region.region_type, + PciBarRegionType::Memory64BitRegion + ); + + let range = RangeInclusive::new( + self.bar_region.addr, + self.bar_region.addr + self.bar_region.size, + ) + .unwrap(); + mmio64_allocator.free(&range); Ok(()) } @@ -969,10 +929,8 @@ impl PciDevice for VirtioPciDevice { ) -> std::result::Result<(), std::io::Error> { // We only update our idea of the bar in order to support free_bars() above. // The majority of the reallocation is done inside DeviceManager. - for bar in self.bar_regions.iter_mut() { - if bar.addr() == old_base { - *bar = bar.set_address(new_base); - } + if self.bar_region.addr == old_base { + self.bar_region.addr = new_base; } Ok(())