pci: Add error propagation to PCI BAR reprogramming

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

devices/src/bus.rs

@@ -9,8 +9,8 @@
 
 use std::cmp::{Ord, Ordering, PartialEq, PartialOrd};
 use std::collections::btree_map::BTreeMap;
-use std::result;
 use std::sync::{Arc, Mutex, RwLock};
+use std::{convert, error, fmt, io, result};
 
 /// Trait for devices that respond to reads or writes in an arbitrary address space.
 ///
@@ -38,6 +38,20 @@ pub enum Error {
 
 pub type Result<T> = result::Result<T, Error>;
 
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "bus_error: {:?}", self)
+    }
+}
+
+impl error::Error for Error {}
+
+impl convert::From<Error> for io::Error {
+    fn from(e: Error) -> Self {
+        io::Error::new(io::ErrorKind::Other, e)
+    }
+}
+
 /// Holds a base and length representing the address space occupied by a `BusDevice`.
 ///
 /// * base - The address at which the range start.

pci/src/bus.rs

@@ -197,13 +197,15 @@ impl PciConfigIo {
 
             // Reprogram the BAR if needed
             if let Some(params) = bar_reprog_params {
-                pci_bus.device_reloc.upgrade().unwrap().move_bar(
+                if let Err(e) = pci_bus.device_reloc.upgrade().unwrap().move_bar(
                     params.old_base,
                     params.new_base,
                     params.len,
                     device.deref_mut(),
                     params.region_type,
-                );
+                ) {
+                    error!("Failed moving device BAR: {}", e);
+                }
             }
         }
     }
@@ -313,13 +315,15 @@ impl PciConfigMmio {
 
             // Reprogram the BAR if needed
             if let Some(params) = bar_reprog_params {
-                pci_bus.device_reloc.upgrade().unwrap().move_bar(
+                if let Err(e) = pci_bus.device_reloc.upgrade().unwrap().move_bar(
                     params.old_base,
                     params.new_base,
                     params.len,
                     device.deref_mut(),
                     params.region_type,
-                );
+                ) {
+                    error!("Failed moving device BAR: {}", e);
+                }
             }
         }
     }

pci/src/device.rs

@@ -6,9 +6,9 @@ use crate::configuration::{self, PciBarRegionType};
 use crate::msix::MsixTableEntry;
 use crate::PciInterruptPin;
 use devices::BusDevice;
-use std;
 use std::fmt::{self, Display};
 use std::sync::Arc;
+use std::{self, io, result};
 use vm_allocator::SystemAllocator;
 use vm_memory::{GuestAddress, GuestUsize};
 use vmm_sys_util::eventfd::EventFd;
@@ -18,7 +18,7 @@ pub struct InterruptParameters<'a> {
 }
 
 pub type InterruptDelivery =
-    Box<dyn Fn(InterruptParameters) -> std::result::Result<(), std::io::Error> + Send + Sync>;
+    Box<dyn Fn(InterruptParameters) -> result::Result<(), io::Error> + Send + Sync>;
 
 #[derive(Debug)]
 pub enum Error {
@@ -107,7 +107,9 @@ pub trait PciDevice: BusDevice {
     /// * `data` - The data to write.
     fn write_bar(&mut self, _base: u64, _offset: u64, _data: &[u8]) {}
     /// Relocates the BAR to a different address in guest address space.
-    fn move_bar(&mut self, _old_base: u64, _new_base: u64) {}
+    fn move_bar(&mut self, _old_base: u64, _new_base: u64) -> result::Result<(), io::Error> {
+        Ok(())
+    }
 }
 
 /// This trait defines a set of functions which can be triggered whenever a
@@ -122,5 +124,5 @@ pub trait DeviceRelocation: Send + Sync {
         len: u64,
         pci_dev: &mut dyn PciDevice,
         region_type: PciBarRegionType,
-    );
+    ) -> result::Result<(), io::Error>;
 }

vfio/src/vfio_pci.rs

@@ -23,7 +23,7 @@ use pci::{
 use std::os::unix::io::AsRawFd;
 use std::ptr::null_mut;
 use std::sync::Arc;
-use std::{fmt, io};
+use std::{fmt, io, result};
 use vfio_bindings::bindings::vfio::*;
 use vm_allocator::SystemAllocator;
 use vm_memory::{Address, GuestAddress, GuestUsize};
@@ -1004,7 +1004,7 @@ impl PciDevice for VfioPciDevice {
         }
     }
 
-    fn move_bar(&mut self, old_base: u64, new_base: u64) {
+    fn move_bar(&mut self, old_base: u64, new_base: u64) -> result::Result<(), io::Error> {
         for region in self.mmio_regions.iter_mut() {
             if region.start.raw_value() == old_base {
                 region.start = GuestAddress(new_base);
@@ -1023,7 +1023,7 @@ impl PciDevice for VfioPciDevice {
                         };
                         // Safe because the guest regions are guaranteed not to overlap.
                         unsafe {
-                            self.vm_fd.set_user_memory_region(old_mem_region).unwrap();
+                            self.vm_fd.set_user_memory_region(old_mem_region)?;
                         }
 
                         // Insert new region to KVM
@@ -1036,11 +1036,13 @@ impl PciDevice for VfioPciDevice {
                         };
                         // Safe because the guest regions are guaranteed not to overlap.
                         unsafe {
-                            self.vm_fd.set_user_memory_region(new_mem_region).unwrap();
+                            self.vm_fd.set_user_memory_region(new_mem_region)?;
                         }
                     }
                 }
             }
         }
+
+        Ok(())
     }
 }

vmm/src/device_manager.rs

@@ -288,7 +288,7 @@ impl DeviceRelocation for AddressManager {
         len: u64,
         pci_dev: &mut dyn PciDevice,
         region_type: PciBarRegionType,
-    ) {
+    ) -> std::result::Result<(), std::io::Error> {
         match region_type {
             PciBarRegionType::IORegion => {
                 // Update system allocator
@@ -296,16 +296,19 @@ impl DeviceRelocation for AddressManager {
                     .lock()
                     .unwrap()
                     .free_io_addresses(GuestAddress(old_base), len as GuestUsize);
+
                 self.allocator
                     .lock()
                     .unwrap()
                     .allocate_io_addresses(Some(GuestAddress(new_base)), len as GuestUsize, None)
-                    .unwrap();
+                    .ok_or_else(|| {
+                        io::Error::new(io::ErrorKind::Other, "failed allocating new IO range")
+                    })?;
 
                 // Update PIO bus
                 self.io_bus
                     .update_range(old_base, len, new_base, len)
-                    .unwrap();
+                    .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
             }
             PciBarRegionType::Memory32BitRegion | PciBarRegionType::Memory64BitRegion => {
                 // Update system allocator
@@ -323,7 +326,12 @@ impl DeviceRelocation for AddressManager {
                             len as GuestUsize,
                             None,
                         )
-                        .unwrap();
+                        .ok_or_else(|| {
+                            io::Error::new(
+                                io::ErrorKind::Other,
+                                "failed allocating new 32 bits MMIO range",
+                            )
+                        })?;
                 } else {
                     self.allocator
                         .lock()
@@ -333,24 +341,28 @@ impl DeviceRelocation for AddressManager {
                             len as GuestUsize,
                             None,
                         )
-                        .unwrap();
+                        .ok_or_else(|| {
+                            io::Error::new(
+                                io::ErrorKind::Other,
+                                "failed allocating new 64 bits MMIO range",
+                            )
+                        })?;
                 }
 
                 // Update MMIO bus
                 self.mmio_bus
                     .update_range(old_base, len, new_base, len)
-                    .unwrap();
+                    .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
             }
         }
 
         for (event, addr, _) in pci_dev.ioeventfds() {
             let io_addr = IoEventAddress::Mmio(addr);
             self.vm_fd
-                .register_ioevent(event.as_raw_fd(), &io_addr, NoDatamatch)
+                .register_ioevent(event.as_raw_fd(), &io_addr, NoDatamatch)?;
-                .unwrap();
         }
 
-        pci_dev.move_bar(old_base, new_base);
+        pci_dev.move_bar(old_base, new_base)
     }
 }