From 11e9f43305470166257cca18ee415930a5b640cf Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Fri, 15 Apr 2022 11:27:53 +0200 Subject: [PATCH] vmm: Use new Resource type PciBar Instead of defining some very generic resources as PioAddressRange or MmioAddressRange for each PCI BAR, let's move to the new Resource type PciBar in order to make things clearer. This allows the code for being more readable, but also removes the need for hard assumptions about the MMIO and PIO ranges. PioAddressRange and MmioAddressRange types can be used to describe everything except PCI BARs. BARs are very special as they can be relocated and have special information we want to carry along with them. Signed-off-by: Sebastien Boeuf --- pci/src/configuration.rs | 36 ++++++++++++++++ pci/src/device.rs | 7 +-- pci/src/vfio.rs | 21 +++++---- virtio-devices/src/transport/pci_device.rs | 23 ++++++---- vm-device/src/lib.rs | 15 +++++++ vmm/src/device_manager.rs | 50 ++++++---------------- 6 files changed, 89 insertions(+), 63 deletions(-) diff --git a/pci/src/configuration.rs b/pci/src/configuration.rs index 27072972b..60051a9a6 100644 --- a/pci/src/configuration.rs +++ b/pci/src/configuration.rs @@ -9,6 +9,7 @@ use std::fmt::{self, Display}; use std::sync::{Arc, Mutex}; use versionize::{VersionMap, Versionize, VersionizeError, VersionizeResult}; use versionize_derive::Versionize; +use vm_device::PciBarType; use vm_migration::{MigratableError, Pausable, Snapshot, Snapshottable, VersionMapped}; // The number of 32bit registers in the config space, 4096 bytes. @@ -327,12 +328,43 @@ 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, + } + } +} + +#[allow(clippy::from_over_into)] +impl Into for PciBarRegionType { + fn into(self) -> PciBarType { + match self { + PciBarRegionType::IoRegion => PciBarType::Io, + PciBarRegionType::Memory32BitRegion => PciBarType::Mmio32, + PciBarRegionType::Memory64BitRegion => PciBarType::Mmio64, + } + } +} + #[derive(Copy, Clone)] pub enum PciBarPrefetchable { NotPrefetchable = 0, Prefetchable = 0x08, } +#[allow(clippy::from_over_into)] +impl Into for PciBarPrefetchable { + fn into(self) -> bool { + match self { + PciBarPrefetchable::NotPrefetchable => false, + PciBarPrefetchable::Prefetchable => true, + } + } +} + #[derive(Copy, Clone)] pub struct PciBarConfiguration { addr: u64, @@ -982,6 +1014,10 @@ impl PciBarConfiguration { pub fn region_type(&self) -> PciBarRegionType { self.region_type } + + pub fn prefetchable(&self) -> PciBarPrefetchable { + self.prefetchable + } } #[cfg(test)] diff --git a/pci/src/device.rs b/pci/src/device.rs index 27f054605..b4743d741 100644 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -19,10 +19,8 @@ pub enum Error { IoAllocationFailed(u64), /// Registering an IO BAR failed. IoRegistrationFailed(u64, configuration::Error), - /// Not enough resources + /// Expected resource not found. MissingResource, - /// Invalid type of resource - InvalidResourceType, } pub type Result = std::result::Result; @@ -38,8 +36,7 @@ impl Display for Error { IoRegistrationFailed(addr, e) => { write!(f, "failed to register an IO BAR, addr={} err={}", addr, e) } - MissingResource => write!(f, "Missing resource"), - InvalidResourceType => write!(f, "Invalid type of resource"), + MissingResource => write!(f, "failed to find expected resource"), } } } diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 764f53cb5..8c1f9a4b3 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -376,18 +376,17 @@ impl VfioCommon { let region_size: u64; let bar_addr: GuestAddress; - let restored_bar_addr = if let Some(resources) = &resources { - match resources - .get(bars.len()) - .ok_or(PciDeviceError::MissingResource)? - { - Resource::MmioAddressRange { base, .. } => Some(GuestAddress(*base)), - Resource::PioAddressRange { base, .. } => Some(GuestAddress(*base as u64)), - _ => return Err(PciDeviceError::InvalidResourceType), + let mut restored_bar_addr = None; + if let Some(resources) = &resources { + for resource in resources { + if let Resource::PciBar { index, base, .. } = resource { + if *index == bar_id as usize { + restored_bar_addr = Some(GuestAddress(*base)); + break; + } + } } - } else { - None - }; + } let bar_offset = if bar_id == VFIO_PCI_ROM_REGION_INDEX { (PCI_ROM_EXP_BAR_INDEX * 4) as u32 diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 4112a6113..abfe03797 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -846,17 +846,22 @@ impl PciDevice for VirtioPciDevice { let mut bars = Vec::new(); let device_clone = self.device.clone(); let device = device_clone.lock().unwrap(); - let settings_bar_addr = if let Some(resources) = &resources { - if resources.is_empty() { + + let mut settings_bar_addr = None; + if let Some(resources) = &resources { + for resource in resources { + if let Resource::PciBar { index, base, .. } = resource { + if *index == VIRTIO_COMMON_BAR_INDEX { + settings_bar_addr = Some(GuestAddress(*base)); + break; + } + } + } + // Error out if no resource was matching the BAR id. + if settings_bar_addr.is_none() { return Err(PciDeviceError::MissingResource); } - match resources[VIRTIO_COMMON_BAR_INDEX] { - Resource::MmioAddressRange { base, .. } => Some(GuestAddress(base)), - _ => return Err(PciDeviceError::InvalidResourceType), - } - } else { - None - }; + } // Allocate the virtio-pci capability BAR. // See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004 diff --git a/vm-device/src/lib.rs b/vm-device/src/lib.rs index 77d19b986..d54e34fad 100644 --- a/vm-device/src/lib.rs +++ b/vm-device/src/lib.rs @@ -23,6 +23,13 @@ pub enum MsiIrqType { GenericMsi, } +#[derive(Copy, Clone, PartialEq, Serialize, Deserialize, Debug)] +pub enum PciBarType { + Io, + Mmio32, + Mmio64, +} + /// Enumeration for device resources. #[allow(missing_docs)] #[derive(Clone, Debug, Serialize, Deserialize)] @@ -31,6 +38,14 @@ pub enum Resource { PioAddressRange { base: u16, size: u16 }, /// Memory Mapped IO address range. MmioAddressRange { base: u64, size: u64 }, + /// PCI BAR + PciBar { + index: usize, + base: u64, + size: u64, + type_: PciBarType, + prefetchable: bool, + }, /// Legacy IRQ number. LegacyIrq(u32), /// Message Signaled Interrupt diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index b372b0181..9dd9bfefd 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -665,25 +665,11 @@ impl DeviceRelocation for AddressManager { if let Some(node) = self.device_tree.lock().unwrap().get_mut(&id) { let mut resource_updated = false; for resource in node.resources.iter_mut() { - match region_type { - PciBarRegionType::IoRegion => { - if let Resource::PioAddressRange { base, .. } = resource { - if *base as u64 == old_base { - *base = new_base as u16; - resource_updated = true; - break; - } - } - } - PciBarRegionType::Memory32BitRegion - | PciBarRegionType::Memory64BitRegion => { - if let Resource::MmioAddressRange { base, .. } = resource { - if *base == old_base { - *base = new_base; - resource_updated = true; - break; - } - } + if let Resource::PciBar { base, type_, .. } = resource { + if PciBarRegionType::from(*type_) == region_type && *base == old_base { + *base = new_base; + resource_updated = true; + break; } } } @@ -3222,13 +3208,6 @@ impl DeviceManager { let mut node = device_node!(vfio_name); - for region in vfio_pci_device.lock().unwrap().mmio_regions() { - node.resources.push(Resource::MmioAddressRange { - base: region.start.0, - size: region.length as u64, - }); - } - // Update the device tree with correct resource information. node.resources = new_resources; node.pci_bdf = Some(pci_device_bdf); @@ -3286,18 +3265,13 @@ impl DeviceManager { let mut new_resources = Vec::new(); for bar in bars { - match bar.region_type() { - PciBarRegionType::IoRegion => new_resources.push(Resource::PioAddressRange { - base: bar.addr() as u16, - size: bar.size() as u16, - }), - PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => { - new_resources.push(Resource::MmioAddressRange { - base: bar.addr(), - size: bar.size() as u64, - }) - } - } + new_resources.push(Resource::PciBar { + index: bar.idx(), + base: bar.addr(), + size: bar.size(), + type_: bar.region_type().into(), + prefetchable: bar.prefetchable().into(), + }); } Ok(new_resources)