Elimination of unneeded generics and lots of .map_err()

src/firecracker/src/api_server/request/machine_configuration.rs

@@ -123,6 +123,8 @@ mod tests {
                 cpu_template: None,
                 track_dirty_pages: Some(false),
                 huge_pages: Some(expected),
+                #[cfg(feature = "gdb")]
+                gdb_socket_path: None,
             };
             assert_eq!(
                 vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
@@ -142,6 +144,8 @@ mod tests {
             cpu_template: Some(StaticCpuTemplate::None),
             track_dirty_pages: Some(false),
             huge_pages: Some(HugePageConfig::None),
+            #[cfg(feature = "gdb")]
+            gdb_socket_path: None,
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
@@ -161,6 +165,8 @@ mod tests {
             cpu_template: None,
             track_dirty_pages: Some(true),
             huge_pages: Some(HugePageConfig::None),
+            #[cfg(feature = "gdb")]
+            gdb_socket_path: None,
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
@@ -184,6 +190,8 @@ mod tests {
                 cpu_template: Some(StaticCpuTemplate::T2),
                 track_dirty_pages: Some(true),
                 huge_pages: Some(HugePageConfig::None),
+                #[cfg(feature = "gdb")]
+                gdb_socket_path: None,
             };
             assert_eq!(
                 vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),
@@ -209,6 +217,8 @@ mod tests {
             cpu_template: None,
             track_dirty_pages: Some(true),
             huge_pages: Some(HugePageConfig::None),
+            #[cfg(feature = "gdb")]
+            gdb_socket_path: None,
         };
         assert_eq!(
             vmm_action_from_request(parse_put_machine_config(&Body::new(body)).unwrap()),

src/vmm/src/arch/aarch64/fdt.rs

@@ -15,6 +15,7 @@ use vm_memory::GuestMemoryError;
 use super::super::{DeviceType, InitrdConfig};
 use super::cache_info::{CacheEntry, read_cache_config};
 use super::gic::GICDevice;
+use crate::device_manager::mmio::MMIODeviceInfo;
 use crate::devices::acpi::vmgenid::{VMGENID_MEM_SIZE, VmGenId};
 use crate::vstate::memory::{Address, GuestMemory, GuestMemoryMmap};
 
@@ -42,16 +43,6 @@ const GIC_FDT_IRQ_TYPE_PPI: u32 = 1;
 const IRQ_TYPE_EDGE_RISING: u32 = 1;
 const IRQ_TYPE_LEVEL_HI: u32 = 4;
 
-/// Trait for devices to be added to the Flattened Device Tree.
-pub trait DeviceInfoForFDT {
-    /// Returns the address where this device will be loaded.
-    fn addr(&self) -> u64;
-    /// Returns the associated interrupt for this device.
-    fn irq(&self) -> u32;
-    /// Returns the amount of memory that needs to be reserved for this device.
-    fn length(&self) -> u64;
-}
-
 /// Errors thrown while configuring the Flattened Device Tree for aarch64.
 #[derive(Debug, thiserror::Error, displaydoc::Display)]
 pub enum FdtError {
@@ -64,11 +55,11 @@ pub enum FdtError {
 }
 
 /// Creates the flattened device tree for this aarch64 microVM.
-pub fn create_fdt<T: DeviceInfoForFDT + Clone + Debug>(
+pub fn create_fdt(
     guest_mem: &GuestMemoryMmap,
     vcpu_mpidr: Vec<u64>,
     cmdline: CString,
-    device_info: &HashMap<(DeviceType, String), T>,
+    device_info: &HashMap<(DeviceType, String), MMIODeviceInfo>,
     gic_device: &GICDevice,
     vmgenid: &Option<VmGenId>,
     initrd: &Option<InitrdConfig>,
@@ -361,69 +352,68 @@ fn create_psci_node(fdt: &mut FdtWriter) -> Result<(), FdtError> {
     Ok(())
 }
 
-fn create_virtio_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut FdtWriter,
-    dev_info: &T,
-) -> Result<(), FdtError> {
-    let virtio_mmio = fdt.begin_node(&format!("virtio_mmio@{:x}", dev_info.addr()))?;
+fn create_virtio_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> {
+    let virtio_mmio = fdt.begin_node(&format!("virtio_mmio@{:x}", dev_info.addr))?;
 
     fdt.property_string("compatible", "virtio,mmio")?;
-    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?;
     fdt.property_array_u32(
         "interrupts",
-        &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING],
+        &[
+            GIC_FDT_IRQ_TYPE_SPI,
+            dev_info.irq.unwrap().into(),
+            IRQ_TYPE_EDGE_RISING,
+        ],
     )?;
     fdt.property_u32("interrupt-parent", GIC_PHANDLE)?;
     fdt.end_node(virtio_mmio)?;
 
     Ok(())
 }
 
-fn create_serial_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut FdtWriter,
-    dev_info: &T,
-) -> Result<(), FdtError> {
-    let serial = fdt.begin_node(&format!("uart@{:x}", dev_info.addr()))?;
+fn create_serial_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> {
+    let serial = fdt.begin_node(&format!("uart@{:x}", dev_info.addr))?;
 
     fdt.property_string("compatible", "ns16550a")?;
-    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?;
     fdt.property_u32("clocks", CLOCK_PHANDLE)?;
     fdt.property_string("clock-names", "apb_pclk")?;
     fdt.property_array_u32(
         "interrupts",
-        &[GIC_FDT_IRQ_TYPE_SPI, dev_info.irq(), IRQ_TYPE_EDGE_RISING],
+        &[
+            GIC_FDT_IRQ_TYPE_SPI,
+            dev_info.irq.unwrap().into(),
+            IRQ_TYPE_EDGE_RISING,
+        ],
     )?;
     fdt.end_node(serial)?;
 
     Ok(())
 }
 
-fn create_rtc_node<T: DeviceInfoForFDT + Clone + Debug>(
-    fdt: &mut FdtWriter,
-    dev_info: &T,
-) -> Result<(), FdtError> {
+fn create_rtc_node(fdt: &mut FdtWriter, dev_info: &MMIODeviceInfo) -> Result<(), FdtError> {
     // Driver requirements:
     // https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/rtc/arm,pl031.yaml
     // We do not offer the `interrupt` property because the device
     // does not implement interrupt support.
     let compatible = b"arm,pl031\0arm,primecell\0";
 
-    let rtc = fdt.begin_node(&format!("rtc@{:x}", dev_info.addr()))?;
+    let rtc = fdt.begin_node(&format!("rtc@{:x}", dev_info.addr))?;
     fdt.property("compatible", compatible)?;
-    fdt.property_array_u64("reg", &[dev_info.addr(), dev_info.length()])?;
+    fdt.property_array_u64("reg", &[dev_info.addr, dev_info.len])?;
     fdt.property_u32("clocks", CLOCK_PHANDLE)?;
     fdt.property_string("clock-names", "apb_pclk")?;
     fdt.end_node(rtc)?;
 
     Ok(())
 }
 
-fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildHasher>(
+fn create_devices_node(
     fdt: &mut FdtWriter,
-    dev_info: &HashMap<(DeviceType, String), T, S>,
+    dev_info: &HashMap<(DeviceType, String), MMIODeviceInfo>,
 ) -> Result<(), FdtError> {
     // Create one temp Vec to store all virtio devices
-    let mut ordered_virtio_device: Vec<&T> = Vec::new();
+    let mut ordered_virtio_device: Vec<&MMIODeviceInfo> = Vec::new();
 
     for ((device_type, _device_id), info) in dev_info {
         match device_type {
@@ -437,7 +427,7 @@ fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildH
     }
 
     // Sort out virtio devices by address from low to high and insert them into fdt table.
-    ordered_virtio_device.sort_by_key(|&a| a.addr());
+    ordered_virtio_device.sort_by_key(|a| a.addr);
     for ordered_device_info in ordered_virtio_device.drain(..) {
         create_virtio_node(fdt, ordered_device_info)?;
     }
@@ -448,6 +438,7 @@ fn create_devices_node<T: DeviceInfoForFDT + Clone + Debug, S: std::hash::BuildH
 #[cfg(test)]
 mod tests {
     use std::ffi::CString;
+    use std::num::NonZeroU32;
 
     use kvm_ioctls::Kvm;
 
@@ -460,23 +451,6 @@ mod tests {
 
     const LEN: u64 = 4096;
 
-    #[derive(Clone, Debug)]
-    pub struct MMIODeviceInfo {
-        addr: u64,
-        irq: u32,
-    }
-
-    impl DeviceInfoForFDT for MMIODeviceInfo {
-        fn addr(&self) -> u64 {
-            self.addr
-        }
-        fn irq(&self) -> u32 {
-            self.irq
-        }
-        fn length(&self) -> u64 {
-            LEN
-        }
-    }
     // The `load` function from the `device_tree` will mistakenly check the actual size
     // of the buffer with the allocated size. This works around that.
     fn set_size(buf: &mut [u8], pos: usize, val: u32) {
@@ -493,17 +467,26 @@ mod tests {
         let dev_info: HashMap<(DeviceType, std::string::String), MMIODeviceInfo> = [
             (
                 (DeviceType::Serial, DeviceType::Serial.to_string()),
-                MMIODeviceInfo { addr: 0x00, irq: 1 },
+                MMIODeviceInfo {
+                    addr: 0x00,
+                    irq: NonZeroU32::new(1),
+                    len: LEN,
+                },
             ),
             (
                 (DeviceType::Virtio(1), "virtio".to_string()),
-                MMIODeviceInfo { addr: LEN, irq: 2 },
+                MMIODeviceInfo {
+                    addr: LEN,
+                    irq: NonZeroU32::new(2),
+                    len: LEN,
+                },
             ),
             (
                 (DeviceType::Rtc, "rtc".to_string()),
                 MMIODeviceInfo {
                     addr: 2 * LEN,
-                    irq: 3,
+                    irq: NonZeroU32::new(3),
+                    len: LEN,
                 },
             ),
         ]

src/vmm/src/arch/aarch64/mod.rs

@@ -23,9 +23,9 @@ use std::fmt::Debug;
 
 use vm_memory::GuestMemoryError;
 
-pub use self::fdt::DeviceInfoForFDT;
 use self::gic::GICDevice;
 use crate::arch::DeviceType;
+use crate::device_manager::mmio::MMIODeviceInfo;
 use crate::devices::acpi::vmgenid::VmGenId;
 use crate::vstate::memory::{Address, Bytes, GuestAddress, GuestMemory, GuestMemoryMmap};
 
@@ -63,11 +63,11 @@ pub fn arch_memory_regions(size: usize) -> Vec<(GuestAddress, usize)> {
 /// * `device_info` - A hashmap containing the attached devices for building FDT device nodes.
 /// * `gic_device` - The GIC device.
 /// * `initrd` - Information about an optional initrd.
-pub fn configure_system<T: DeviceInfoForFDT + Clone + Debug>(
+pub fn configure_system(
     guest_mem: &GuestMemoryMmap,
     cmdline_cstring: CString,
     vcpu_mpidr: Vec<u64>,
-    device_info: &HashMap<(DeviceType, String), T>,
+    device_info: &HashMap<(DeviceType, String), MMIODeviceInfo>,
     gic_device: &GICDevice,
     vmgenid: &Option<VmGenId>,
     initrd: &Option<super::InitrdConfig>,

src/vmm/src/arch/x86_64/mod.rs

@@ -41,7 +41,7 @@ use linux_loader::loader::elf::start_info::{
 
 use crate::arch::{BootProtocol, InitrdConfig, SYSTEM_MEM_SIZE, SYSTEM_MEM_START};
 use crate::device_manager::resources::ResourceAllocator;
-use crate::utils::u64_to_usize;
+use crate::utils::{mib_to_bytes, u64_to_usize};
 use crate::vstate::memory::{
     Address, GuestAddress, GuestMemory, GuestMemoryMmap, GuestMemoryRegion,
 };
@@ -73,10 +73,11 @@ pub enum ConfigurationError {
     StartInfoSetup,
 }
 
-const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32;
+/// First address that cannot be addressed using 32 bit anymore.
+pub const FIRST_ADDR_PAST_32BITS: u64 = 1 << 32;
 
 /// Size of MMIO gap at top of 32-bit address space.
-pub const MEM_32BIT_GAP_SIZE: u64 = 768 << 20;
+pub const MEM_32BIT_GAP_SIZE: u64 = mib_to_bytes(768) as u64;
 /// The start of the memory area reserved for MMIO devices.
 pub const MMIO_MEM_START: u64 = FIRST_ADDR_PAST_32BITS - MEM_32BIT_GAP_SIZE;
 /// The size of the memory area reserved for MMIO devices.
@@ -402,7 +403,7 @@ mod tests {
         );
 
         // Now assigning some memory that falls before the 32bit memory hole.
-        let mem_size = 128 << 20;
+        let mem_size = mib_to_bytes(128);
         let gm = arch_mem(mem_size);
         let mut resource_allocator = ResourceAllocator::new().unwrap();
         configure_system(
@@ -427,7 +428,7 @@ mod tests {
         .unwrap();
 
         // Now assigning some memory that is equal to the start of the 32bit memory hole.
-        let mem_size = 3328 << 20;
+        let mem_size = mib_to_bytes(3328);
         let gm = arch_mem(mem_size);
         let mut resource_allocator = ResourceAllocator::new().unwrap();
         configure_system(
@@ -452,7 +453,7 @@ mod tests {
         .unwrap();
 
         // Now assigning some memory that falls after the 32bit memory hole.
-        let mem_size = 3330 << 20;
+        let mem_size = mib_to_bytes(3330);
         let gm = arch_mem(mem_size);
         let mut resource_allocator = ResourceAllocator::new().unwrap();
         configure_system(