Maintenance5

src/vmm/src/arch/x86_64/mptable.rs

@@ -82,7 +82,7 @@ const MPC_SIGNATURE: [c_char; 4] = char_array!(c_char; 'P', 'C', 'M', 'P');
 const MPC_SPEC: i8 = 4;
 const MPC_OEM: [c_char; 8] = char_array!(c_char; 'F', 'C', ' ', ' ', ' ', ' ', ' ', ' ');
 const MPC_PRODUCT_ID: [c_char; 12] = ['0' as c_char; 12];
-const BUS_TYPE_ISA: [u8; 6] = [b'I', b'S', b'A', b' ', b' ', b' '];
+const BUS_TYPE_ISA: [u8; 6] = *b"ISA   ";
 const IO_APIC_DEFAULT_PHYS_BASE: u32 = 0xfec0_0000; // source: linux/arch/x86/include/asm/apicdef.h
 const APIC_DEFAULT_PHYS_BASE: u32 = 0xfee0_0000; // source: linux/arch/x86/include/asm/apicdef.h
 const APIC_VERSION: u8 = 0x14;

src/vmm/src/builder.rs

@@ -80,8 +80,6 @@ pub enum StartMicrovmError {
     CreateNetDevice(crate::devices::virtio::net::NetError),
     /// Cannot create pmem device: {0}
     CreatePmemDevice(#[from] crate::devices::virtio::pmem::device::PmemError),
-    /// Cannot create RateLimiter: {0}
-    CreateRateLimiter(io::Error),
     /// Error creating legacy device: {0}
     #[cfg(target_arch = "x86_64")]
     CreateLegacyDevice(device_manager::legacy::LegacyDeviceError),

src/vmm/src/device_manager/legacy.rs

@@ -12,14 +12,11 @@ use acpi_tables::aml::AmlError;
 use acpi_tables::{Aml, aml};
 
 use crate::devices::legacy::{I8042Device, SerialDevice};
-use crate::vstate::bus::BusError;
 use crate::vstate::vm::KvmVm;
 
 /// Errors corresponding to the `PortIODeviceManager`.
 #[derive(Debug, derive_more::From, thiserror::Error, displaydoc::Display)]
 pub enum LegacyDeviceError {
-    /// Failed to add legacy device to Bus: {0}
-    BusError(BusError),
     /// Failed to create EventFd: {0}
     EventFd(std::io::Error),
 }
@@ -58,12 +55,12 @@ impl PortIODeviceManager {
             self.stdio_serial.clone(),
             Self::SERIAL_PORT_ADDRESS,
             Self::SERIAL_PORT_SIZE,
-        )?;
+        );
         io_bus.insert(
             self.i8042.clone(),
             Self::I8042_KDB_DATA_REGISTER_ADDRESS,
             Self::I8042_KDB_DATA_REGISTER_SIZE,
-        )?;
+        );
 
         vm.register_irq(
             self.stdio_serial

src/vmm/src/device_manager/mmio.rs

@@ -28,7 +28,7 @@ use crate::devices::virtio::device::{VirtioDevice, VirtioDeviceId, VirtioDeviceT
 use crate::devices::virtio::transport::mmio::MmioTransport;
 #[cfg(target_arch = "x86_64")]
 use crate::logger::debug;
-use crate::vstate::bus::{Bus, BusError};
+use crate::vstate::bus::Bus;
 #[cfg(target_arch = "x86_64")]
 use crate::vstate::memory::GuestAddress;
 use crate::vstate::resources::ResourceAllocator;
@@ -39,8 +39,6 @@ use crate::vstate::vm::KvmVm;
 pub enum MmioError {
     /// Failed to allocate requested resource: {0}
     Allocator(#[from] vm_allocator::Error),
-    /// Failed to insert device on the bus: {0}
-    BusInsert(#[from] BusError),
     /// Failed to allocate requested resourc: {0}
     Cmdline(#[from] linux_loader::cmdline::Error),
     /// Could not create IRQ for MMIO device: {0}
@@ -206,7 +204,7 @@ impl MMIODeviceManager {
             device.inner.clone(),
             device.resources.addr,
             device.resources.len,
-        )?;
+        );
 
         let sub_id =
             event_manager.add_subscriber(device.inner.lock().expect("Poisoned lock").device());
@@ -311,7 +309,7 @@ impl MMIODeviceManager {
             device.inner.clone(),
             device.resources.addr,
             device.resources.len,
-        )?;
+        );
 
         self.serial = Some(device);
         Ok(())
@@ -368,7 +366,7 @@ impl MMIODeviceManager {
             device.inner.clone(),
             device.resources.addr,
             device.resources.len,
-        )?;
+        );
         self.rtc = Some(device);
         Ok(())
     }
@@ -396,7 +394,7 @@ impl MMIODeviceManager {
             device.inner.clone(),
             device.resources.addr,
             device.resources.len,
-        )?;
+        );
         self.boot_timer = Some(device);
 
         Ok(())

src/vmm/src/device_manager/mod.rs

@@ -57,7 +57,6 @@ use crate::vmm_config::drive::{BlockDeviceConfig, DriveError};
 use crate::vmm_config::mmds::MmdsConfigError;
 use crate::vmm_config::net::{NetBuilder, NetworkInterfaceConfig, NetworkInterfaceError};
 use crate::vmm_config::pmem::{PmemConfig, PmemConfigError};
-use crate::vstate::bus::BusError;
 use crate::vstate::memory::GuestMemoryMmap;
 use crate::vstate::vm::{KvmVm, Vm};
 
@@ -89,8 +88,6 @@ pub enum DeviceManagerCreateError {
 pub enum AttachDeviceError {
     /// MMIO transport error: {0}
     MmioTransport(#[from] MmioError),
-    /// Error inserting device in bus: {0}
-    Bus(#[from] BusError),
     /// Error while registering ACPI with KVM: {0}
     AttachAcpiDevice(#[from] ACPIDeviceError),
     #[cfg(target_arch = "aarch64")]
@@ -605,8 +602,7 @@ impl DeviceManager {
 
         vm.common
             .mmio_bus
-            .remove(pci_device.config_bar_addr(), CAPABILITY_BAR_SIZE)
-            .map_err(PciManagerError::Bus)?;
+            .remove(pci_device.config_bar_addr(), CAPABILITY_BAR_SIZE);
 
         self.pci_devices
             .pci_segment
@@ -657,8 +653,6 @@ pub enum DevicePersistError {
     MmioTransport,
     /// PCI Device manager: {0}
     PciDeviceManager(#[from] PciManagerError),
-    /// Bus error: {0}
-    Bus(#[from] BusError),
     #[cfg(target_arch = "aarch64")]
     /// Legacy: {0}
     Legacy(#[from] std::io::Error),
@@ -691,8 +685,6 @@ pub enum DeviceManagerPersistError {
     AcpiRestore(#[from] ACPIDeviceError),
     /// Error restoring PCI devices: {0}
     PciRestore(DevicePersistError),
-    /// Error inserting device in bus: {0}
-    Bus(#[from] BusError),
     /// Error creating DeviceManager: {0}
     DeviceManager(#[from] DeviceManagerCreateError),
 }
@@ -787,9 +779,9 @@ impl<'a> Persist<'a> for DeviceManager {
 
 #[cfg(test)]
 pub(crate) mod tests {
-    use super::*;
     use vmm_sys_util::tempfile::TempFile;
 
+    use super::*;
     use crate::builder::tests::{
         CustomBlockConfig, default_kernel_cmdline, default_vmm, insert_block_devices,
     };

src/vmm/src/device_manager/pci_mngr.rs

@@ -39,7 +39,6 @@ use crate::pci::bus::PciRootError;
 use crate::resources::VmResources;
 use crate::snapshot::Persist;
 use crate::vmm_config::memory_hotplug::MemoryHotplugConfig;
-use crate::vstate::bus::BusError;
 use crate::vstate::interrupts::InterruptError;
 use crate::vstate::memory::GuestMemoryMmap;
 use crate::vstate::vm::KvmVm;
@@ -56,8 +55,6 @@ pub struct PciDevices {
 pub enum PciManagerError {
     /// Resource allocation error: {0}
     ResourceAllocation(#[from] vm_allocator::Error),
-    /// Bus error: {0}
-    Bus(#[from] BusError),
     /// PCI root error: {0}
     PciRoot(#[from] PciRootError),
     /// MSI error: {0}
@@ -80,16 +77,13 @@ impl PciDevices {
 
         // Currently we don't assign any IRQs to PCI devices. We will be using MSI-X interrupts
         // only.
-        let pci_segment = PciSegment::new(0, vm, &[0u8; 32])?;
+        let pci_segment = PciSegment::new(0, vm, &[0u8; 32]);
         self.pci_segment = Some(pci_segment);
 
         Ok(())
     }
 
-    fn register_bars_with_bus(
-        vm: &KvmVm,
-        virtio_device: &Arc<Mutex<VirtioPciDevice>>,
-    ) -> Result<(), PciManagerError> {
+    fn register_bars_with_bus(vm: &KvmVm, virtio_device: &Arc<Mutex<VirtioPciDevice>>) {
         let virtio_device_locked = virtio_device.lock().expect("Poisoned lock");
 
         debug!(
@@ -101,9 +95,7 @@ impl PciDevices {
             virtio_device.clone(),
             virtio_device_locked.config_bar_addr(),
             CAPABILITY_BAR_SIZE,
-        )?;
-
-        Ok(())
+        );
     }
 
     fn attach_common(
@@ -127,7 +119,7 @@ impl PciDevices {
         self.virtio_devices
             .insert((device_type, id), virtio_device.clone());
 
-        Self::register_bars_with_bus(vm, &virtio_device)?;
+        Self::register_bars_with_bus(vm, &virtio_device);
 
         let mut device = virtio_device.lock().expect("Poisoned lock");
         device.register_notification_ioevents(vm)?;

src/vmm/src/devices/pci/pci_segment.rs

@@ -23,7 +23,7 @@ use crate::pci::PciSBDF;
 #[cfg(target_arch = "x86_64")]
 use crate::pci::bus::{PCI_CONFIG_IO_PORT, PCI_CONFIG_IO_PORT_SIZE};
 use crate::pci::bus::{PciBus, PciConfigIo, PciConfigMmio, PciRoot, PciRootError};
-use crate::vstate::bus::{BusDeviceSync, BusError};
+use crate::vstate::bus::BusDeviceSync;
 use crate::vstate::resources::ResourceAllocator;
 use crate::vstate::vm::KvmVm;
 
@@ -70,7 +70,7 @@ impl std::fmt::Debug for PciSegment {
 }
 
 impl PciSegment {
-    fn build(id: u16, vm: &Arc<KvmVm>, pci_irq_slots: &[u8; 32]) -> Result<PciSegment, BusError> {
+    fn build(id: u16, vm: &Arc<KvmVm>, pci_irq_slots: &[u8; 32]) -> PciSegment {
         let pci_root = PciRoot::new(None);
         let pci_bus = Arc::new(Mutex::new(PciBus::new(pci_root)));
 
@@ -81,7 +81,7 @@ impl PciSegment {
             Arc::clone(&pci_config_mmio) as Arc<dyn BusDeviceSync>,
             mmio_config_address,
             PCI_MMIO_CONFIG_SIZE_PER_SEGMENT,
-        )?;
+        );
 
         let resource_allocator = vm.resource_allocator();
 
@@ -91,7 +91,7 @@ impl PciSegment {
         let start_of_mem64_area = resource_allocator.mmio64_memory.base();
         let end_of_mem64_area = resource_allocator.mmio64_memory.end();
 
-        let segment = PciSegment {
+        PciSegment {
             id,
             pci_bus,
             pci_config_mmio,
@@ -106,25 +106,19 @@ impl PciSegment {
             start_of_mem64_area,
             end_of_mem64_area,
             pci_irq_slots: *pci_irq_slots,
-        };
-
-        Ok(segment)
+        }
     }
 
     #[cfg(target_arch = "x86_64")]
-    pub(crate) fn new(
-        id: u16,
-        vm: &Arc<KvmVm>,
-        pci_irq_slots: &[u8; 32],
-    ) -> Result<PciSegment, BusError> {
-        let mut segment = Self::build(id, vm, pci_irq_slots)?;
+    pub(crate) fn new(id: u16, vm: &Arc<KvmVm>, pci_irq_slots: &[u8; 32]) -> PciSegment {
+        let mut segment = Self::build(id, vm, pci_irq_slots);
         let pci_config_io = Arc::new(Mutex::new(PciConfigIo::new(Arc::clone(&segment.pci_bus))));
 
         vm.pio_bus.insert(
             pci_config_io.clone(),
             PCI_CONFIG_IO_PORT,
             PCI_CONFIG_IO_PORT_SIZE,
-        )?;
+        );
 
         segment.pci_config_io = Some(pci_config_io);
 
@@ -140,16 +134,12 @@ impl PciSegment {
             PCI_CONFIG_IO_PORT + PCI_CONFIG_IO_PORT_SIZE - 1
         );
 
-        Ok(segment)
+        segment
     }
 
     #[cfg(target_arch = "aarch64")]
-    pub(crate) fn new(
-        id: u16,
-        vm: &Arc<KvmVm>,
-        pci_irq_slots: &[u8; 32],
-    ) -> Result<PciSegment, BusError> {
-        let segment = Self::build(id, vm, pci_irq_slots)?;
+    pub(crate) fn new(id: u16, vm: &Arc<KvmVm>, pci_irq_slots: &[u8; 32]) -> PciSegment {
+        let segment = Self::build(id, vm, pci_irq_slots);
         info!(
             "pci: adding PCI segment: id={:#x}, PCI MMIO config address: {:#x}, mem32 area: \
              [{:#x}-{:#x}], mem64 area: [{:#x}-{:#x}]",
@@ -161,7 +151,7 @@ impl PciSegment {
             segment.end_of_mem64_area,
         );
 
-        Ok(segment)
+        segment
     }
 
     pub(crate) fn next_device_sbdf(&self) -> Result<PciSBDF, PciRootError> {
@@ -470,7 +460,7 @@ mod tests {
         let vmm = default_vmm();
         let kvm_vm = vmm.vm.as_kvm().unwrap().clone();
         let pci_irq_slots = &[0u8; 32];
-        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots).unwrap();
+        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots);
 
         assert_eq!(pci_segment.id, 0);
         assert_eq!(
@@ -502,7 +492,7 @@ mod tests {
         let vmm = default_vmm();
         let kvm_vm = vmm.vm.as_kvm().unwrap().clone();
         let pci_irq_slots = &[0u8; 32];
-        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots).unwrap();
+        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots);
 
         let mut data = [0u8; u64_to_usize(PCI_CONFIG_IO_PORT_SIZE)];
         kvm_vm.pio_bus.read(PCI_CONFIG_IO_PORT, &mut data).unwrap();
@@ -518,7 +508,7 @@ mod tests {
         let vmm = default_vmm();
         let kvm_vm = vmm.vm.as_kvm().unwrap().clone();
         let pci_irq_slots = &[0u8; 32];
-        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots).unwrap();
+        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots);
 
         let mut data = [0u8; u64_to_usize(PCI_MMIO_CONFIG_SIZE_PER_SEGMENT)];
 
@@ -539,18 +529,52 @@ mod tests {
 
     #[test]
     fn test_next_device_bdf() {
+        use crate::pci::configuration::PciConfiguration;
+        use crate::pci::{PciClassCode, PciDevice, PciMassStorageSubclass};
+
+        struct PciDevMock(PciConfiguration);
+        impl PciDevice for PciDevMock {
+            fn write_config_register(
+                &mut self,
+                reg_idx: u16,
+                offset: u8,
+                data: &[u8],
+            ) -> Option<Arc<std::sync::Barrier>> {
+                self.0.write_config_register(reg_idx, offset, data);
+                None
+            }
+            fn read_config_register(&mut self, reg_idx: u16) -> u32 {
+                self.0.read_reg(reg_idx)
+            }
+        }
+        fn mock_dev() -> Arc<Mutex<dyn PciDevice>> {
+            Arc::new(Mutex::new(PciDevMock(PciConfiguration::new_type0(
+                0x42,
+                0x0,
+                0x0,
+                PciClassCode::MassStorageController,
+                PciMassStorageSubclass::SerialScsiController as u8,
+                0x13,
+                0x12,
+            ))))
+        }
+
         let vmm = default_vmm();
         let kvm_vm = vmm.vm.as_kvm().unwrap().clone();
         let pci_irq_slots = &[0u8; 32];
-        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots).unwrap();
+        let pci_segment = PciSegment::new(0, &kvm_vm, pci_irq_slots);
 
         // Start checking from device id 1, since 0 is allocated to the Root port.
+        // `next_device_sbdf` only inspects the bus, so the caller must `add_device`
+        // before requesting the next id.
         for dev_id in 1..32 {
             let sbdf = pci_segment.next_device_sbdf().unwrap();
             // In our case we have a single Segment with id 0, which has
             // a single bus with id 0. Also, each device of ours has a
             // single function.
             assert_eq!(sbdf, PciSBDF::new(0, 0, dev_id, 0));
+            let mut segment = pci_segment.pci_bus.lock().unwrap();
+            segment.add_device(dev_id, mock_dev()).unwrap();
         }
 
         // We can only have 32 devices on a segment