pci, virtio: Make virtio-pci BAR restoration more generic

Updating the way of restoring BAR addresses for virtio-pci by providing
a more generic approach that will be reused for other PciDevice
implementations (i.e VfioPcidevice and VfioUserPciDevice).

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

pci/src/device.rs

@@ -8,7 +8,7 @@ use std::fmt::{self, Display};
 use std::sync::{Arc, Barrier, Mutex};
 use std::{self, io, result};
 use vm_allocator::{AddressAllocator, SystemAllocator};
-use vm_device::BusDevice;
+use vm_device::{BusDevice, Resource};
 use vm_memory::{GuestAddress, GuestUsize};
 
 #[derive(Debug)]
@@ -19,6 +19,10 @@ pub enum Error {
     IoAllocationFailed(u64),
     /// Registering an IO BAR failed.
     IoRegistrationFailed(u64, configuration::Error),
+    /// Not enough resources
+    MissingResource,
+    /// Invalid type of resource
+    InvalidResourceType,
 }
 pub type Result<T> = std::result::Result<T, Error>;
 
@@ -34,6 +38,8 @@ 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"),
         }
     }
 }
@@ -53,6 +59,7 @@ pub trait PciDevice: BusDevice {
         &mut self,
         _allocator: &Arc<Mutex<SystemAllocator>>,
         _mmio_allocator: &mut AddressAllocator,
+        _resources: Option<Vec<Resource>>,
     ) -> Result<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>> {
         Ok(Vec::new())
     }

pci/src/vfio.rs

@@ -23,7 +23,7 @@ use vm_allocator::{AddressAllocator, SystemAllocator};
 use vm_device::interrupt::{
     InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig,
 };
-use vm_device::BusDevice;
+use vm_device::{BusDevice, Resource};
 use vm_memory::{Address, GuestAddress, GuestUsize};
 use vmm_sys_util::eventfd::EventFd;
 
@@ -363,6 +363,7 @@ impl VfioCommon {
         allocator: &Arc<Mutex<SystemAllocator>>,
         mmio_allocator: &mut AddressAllocator,
         vfio_wrapper: &dyn Vfio,
+        _resources: Option<Vec<Resource>>,
     ) -> Result<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>, PciDeviceError> {
         let mut ranges = Vec::new();
         let mut bar_id = VFIO_PCI_BAR0_REGION_INDEX as u32;
@@ -1326,9 +1327,10 @@ impl PciDevice for VfioPciDevice {
         &mut self,
         allocator: &Arc<Mutex<SystemAllocator>>,
         mmio_allocator: &mut AddressAllocator,
+        resources: Option<Vec<Resource>>,
     ) -> Result<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>, PciDeviceError> {
         self.common
-            .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper)
+            .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper, resources)
     }
 
     fn free_bars(

pci/src/vfio_user.rs

@@ -21,7 +21,7 @@ use vfio_user::{Client, Error as VfioUserError};
 use vm_allocator::{AddressAllocator, SystemAllocator};
 use vm_device::dma_mapping::ExternalDmaMapping;
 use vm_device::interrupt::{InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig};
-use vm_device::BusDevice;
+use vm_device::{BusDevice, Resource};
 use vm_memory::bitmap::AtomicBitmap;
 use vm_memory::{
     Address, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryRegion, GuestRegionMmap,
@@ -392,9 +392,10 @@ impl PciDevice for VfioUserPciDevice {
         &mut self,
         allocator: &Arc<Mutex<SystemAllocator>>,
         mmio_allocator: &mut AddressAllocator,
+        resources: Option<Vec<Resource>>,
     ) -> Result<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>, PciDeviceError> {
         self.common
-            .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper)
+            .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper, resources)
     }
 
     fn free_bars(

virtio-devices/src/transport/pci_device.rs

@@ -35,7 +35,7 @@ use vm_device::dma_mapping::ExternalDmaMapping;
 use vm_device::interrupt::{
     InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig,
 };
-use vm_device::BusDevice;
+use vm_device::{BusDevice, Resource};
 use vm_memory::{Address, ByteValued, GuestAddress, GuestMemoryAtomic, GuestUsize, Le32};
 use vm_migration::{
     Migratable, MigratableError, Pausable, Snapshot, Snapshottable, Transportable, VersionMapped,
@@ -318,7 +318,6 @@ pub struct VirtioPciDevice {
 
     // Settings PCI BAR
     settings_bar: u8,
-    settings_bar_addr: Option<GuestAddress>,
 
     // Whether to use 64-bit bar location or 32-bit
     use_64bit_bar: bool,
@@ -452,7 +451,6 @@ impl VirtioPciDevice {
             queue_evts,
             memory: Some(memory),
             settings_bar: 0,
-            settings_bar_addr: None,
             use_64bit_bar,
             interrupt_source_group,
             cap_pci_cfg_info: VirtioPciCfgCapInfo::default(),
@@ -542,12 +540,6 @@ impl VirtioPciDevice {
         self.common_config.driver_status == DEVICE_INIT as u8
     }
 
-    // This function is used by the caller to provide the expected base address
-    // for the virtio-pci configuration BAR.
-    pub fn set_config_bar_addr(&mut self, bar_addr: u64) {
-        self.settings_bar_addr = Some(GuestAddress(bar_addr));
-    }
-
     pub fn config_bar_addr(&self) -> u64 {
         self.configuration.get_bar_addr(self.settings_bar as usize)
     }
@@ -847,11 +839,23 @@ impl PciDevice for VirtioPciDevice {
         &mut self,
         allocator: &Arc<Mutex<SystemAllocator>>,
         mmio_allocator: &mut AddressAllocator,
+        resources: Option<Vec<Resource>>,
     ) -> std::result::Result<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>, PciDeviceError>
     {
         let mut ranges = 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() {
+                return Err(PciDeviceError::MissingResource);
+            }
+            match resources[0] {
+                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
@@ -859,7 +863,7 @@ impl PciDevice for VirtioPciDevice {
             let region_type = PciBarRegionType::Memory64BitRegion;
             let addr = mmio_allocator
                 .allocate(
-                    self.settings_bar_addr,
+                    settings_bar_addr,
                     CAPABILITY_BAR_SIZE,
                     Some(CAPABILITY_BAR_SIZE),
                 )
@@ -872,7 +876,7 @@ impl PciDevice for VirtioPciDevice {
                 .lock()
                 .unwrap()
                 .allocate_mmio_hole_addresses(
-                    self.settings_bar_addr,
+                    settings_bar_addr,
                     CAPABILITY_BAR_SIZE,
                     Some(CAPABILITY_BAR_SIZE),
                 )

vmm/src/device_manager.rs

@@ -399,9 +399,6 @@ pub enum DeviceManagerError {
     /// Resource was already found.
     ResourceAlreadyExists,
 
-    /// Expected resources for virtio-pci could not be found.
-    MissingVirtioPciResources,
-
     /// Expected resources for virtio-pmem could not be found.
     MissingVirtioPmemResources,
 
@@ -3204,6 +3201,7 @@ impl DeviceManager {
             vfio_pci_device.clone(),
             pci_segment_id,
             pci_device_bdf,
+            None,
         )?;
 
         vfio_pci_device
@@ -3240,6 +3238,7 @@ impl DeviceManager {
         pci_device: Arc<Mutex<dyn PciDevice>>,
         segment_id: u16,
         bdf: PciBdf,
+        resources: Option<Vec<Resource>>,
     ) -> DeviceManagerResult<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>> {
         let bars = pci_device
             .lock()
@@ -3250,6 +3249,7 @@ impl DeviceManager {
                     .allocator
                     .lock()
                     .unwrap(),
+                resources,
             )
             .map_err(DeviceManagerError::AllocateBars)?;
 
@@ -3378,6 +3378,7 @@ impl DeviceManager {
             vfio_user_pci_device.clone(),
             pci_segment_id,
             pci_device_bdf,
+            None,
         )?;
 
         let mut node = device_node!(vfio_user_name);
@@ -3424,7 +3425,7 @@ impl DeviceManager {
 
         // Look for the id in the device tree. If it can be found, that means
         // the device is being restored, otherwise it's created from scratch.
-        let (pci_segment_id, pci_device_bdf, config_bar_addr) = if let Some(node) =
+        let (pci_segment_id, pci_device_bdf, resources) = if let Some(node) =
             self.device_tree.lock().unwrap().get(&id)
         {
             info!("Restoring virtio-pci {} resources", id);
@@ -3440,21 +3441,7 @@ impl DeviceManager {
                 .get_device_id(pci_device_bdf.device() as usize)
                 .map_err(DeviceManagerError::GetPciDeviceId)?;
 
-            if node.resources.is_empty() {
+            (pci_segment_id, pci_device_bdf, Some(node.resources.clone()))
-                return Err(DeviceManagerError::MissingVirtioPciResources);
-            }
-
-            // We know the configuration BAR address is stored on the first
-            // resource in the list.
-            let config_bar_addr = match node.resources[0] {
-                Resource::MmioAddressRange { base, .. } => Some(base),
-                _ => {
-                    error!("Unexpected resource {:?} for {}", node.resources[0], id);
-                    return Err(DeviceManagerError::MissingVirtioPciResources);
-                }
-            };
-
-            (pci_segment_id, pci_device_bdf, config_bar_addr)
         } else {
             let pci_device_bdf = self.pci_segments[pci_segment_id as usize].next_device_bdf()?;
 
@@ -3529,38 +3516,34 @@ impl DeviceManager {
         }
 
         let device_type = virtio_device.lock().unwrap().device_type();
-        let mut virtio_pci_device = VirtioPciDevice::new(
+        let virtio_pci_device = Arc::new(Mutex::new(
-            id.clone(),
+            VirtioPciDevice::new(
-            memory,
+                id.clone(),
-            virtio_device,
+                memory,
-            msix_num,
+                virtio_device,
-            access_platform,
+                msix_num,
-            &self.msi_interrupt_manager,
+                access_platform,
-            pci_device_bdf.into(),
+                &self.msi_interrupt_manager,
-            self.activate_evt
+                pci_device_bdf.into(),
-                .try_clone()
+                self.activate_evt
-                .map_err(DeviceManagerError::EventFd)?,
+                    .try_clone()
-            // All device types *except* virtio block devices should be allocated a 64-bit bar
+                    .map_err(DeviceManagerError::EventFd)?,
-            // The block devices should be given a 32-bit BAR so that they are easily accessible
+                // All device types *except* virtio block devices should be allocated a 64-bit bar
-            // to firmware without requiring excessive identity mapping.
+                // The block devices should be given a 32-bit BAR so that they are easily accessible
-            // The exception being if not on the default PCI segment.
+                // to firmware without requiring excessive identity mapping.
-            pci_segment_id > 0 || device_type != VirtioDeviceType::Block as u32,
+                // The exception being if not on the default PCI segment.
-            dma_handler,
+                pci_segment_id > 0 || device_type != VirtioDeviceType::Block as u32,
-        )
+                dma_handler,
-        .map_err(DeviceManagerError::VirtioDevice)?;
+            )
+            .map_err(DeviceManagerError::VirtioDevice)?,
+        ));
 
-        // This is important as this will set the BAR address if it exists,
-        // which is mandatory on the restore path.
-        if let Some(addr) = config_bar_addr {
-            virtio_pci_device.set_config_bar_addr(addr);
-        }
-
-        let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device));
         let bars = self.add_pci_device(
             virtio_pci_device.clone(),
             virtio_pci_device.clone(),
             pci_segment_id,
             pci_device_bdf,
+            resources,
         )?;
 
         let bar_addr = virtio_pci_device.lock().unwrap().config_bar_addr();
@@ -3573,12 +3556,14 @@ impl DeviceManager {
         }
 
         // Update the device tree with correct resource information.
+        let mut new_resources = Vec::new();
         for pci_bar in bars.iter() {
-            node.resources.push(Resource::MmioAddressRange {
+            new_resources.push(Resource::MmioAddressRange {
                 base: pci_bar.0.raw_value(),
                 size: pci_bar.1 as u64,
             });
         }
+        node.resources = new_resources;
         node.migratable = Some(Arc::clone(&virtio_pci_device) as Arc<Mutex<dyn Migratable>>);
         node.pci_bdf = Some(pci_device_bdf);
         node.pci_device_handle = Some(PciDeviceHandle::Virtio(virtio_pci_device));