Resolve #78, EmulatedDevice::services should access the VM config

mythril/src/kmain.rs

@@ -79,45 +79,46 @@ fn build_vm(
 
     acpi.add_sdt(madt).unwrap();
 
-    let device_map = config.virtual_devices_mut();
+    // let device_map = config.virtual_devices_mut();
+    let mut builder = config.device_map_builder();
 
-    device_map
+    builder
         .register_device(virtdev::acpi::AcpiRuntime::new(0x600).unwrap())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::debug::DebugPort::new(0x402))
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::com::Uart8250::new(0x3F8))
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::vga::VgaController::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::dma::Dma8237::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::ignore::IgnoredDevice::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::pci::PciRootComplex::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::pic::Pic8259::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::keyboard::Keyboard8042::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::pit::Pit8254::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::pos::ProgrammableOptionSelect::new())
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::rtc::CmosRtc::new(cfg.memory))
         .unwrap();
-    device_map
+    builder
         .register_device(virtdev::ioapic::IoApic::new())
         .unwrap();
 
@@ -151,7 +152,7 @@ fn build_vm(
     )
     .unwrap();
 
-    device_map.register_device(fw_cfg_builder.build()).unwrap();
+    builder.register_device(fw_cfg_builder.build()).unwrap();
 
     vm::VirtualMachine::new(cfg.cpus[0].raw, config, info)
         .expect("Failed to create vm")

mythril/src/virtdev/acpi.rs

@@ -1,6 +1,6 @@
-use crate::error::Result;
 use crate::time;
 use crate::virtdev::{DeviceEvent, DeviceRegion, EmulatedDevice, Event, Port};
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use spin::RwLock;
@@ -42,7 +42,7 @@ impl AcpiRuntime {
 }
 
 impl EmulatedDevice for AcpiRuntime {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![
             DeviceRegion::PortIo(
                 Self::FADT_SMI_COMMAND..=Self::FADT_SMI_COMMAND,

mythril/src/virtdev/com.rs

@@ -1,9 +1,9 @@
-use crate::error::Result;
 use crate::physdev::com::*;
 use crate::vcpu;
 use crate::virtdev::{
     DeviceEvent, DeviceEventResponse, DeviceRegion, EmulatedDevice, Event, Port,
 };
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use core::convert::TryInto;
@@ -54,7 +54,7 @@ impl Uart8250 {
 }
 
 impl EmulatedDevice for Uart8250 {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![DeviceRegion::PortIo(self.base_port..=self.base_port + 7)]
     }
 

mythril/src/virtdev/debug.rs

@@ -1,7 +1,7 @@
-use crate::error::Result;
 use crate::virtdev::{
     DeviceEvent, DeviceEventResponse, DeviceRegion, EmulatedDevice, Event, Port,
 };
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use core::convert::TryInto;
@@ -18,7 +18,7 @@ impl DebugPort {
 }
 
 impl EmulatedDevice for DebugPort {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![DeviceRegion::PortIo(self.port..=self.port)]
     }
 

mythril/src/virtdev/dma.rs

@@ -1,5 +1,5 @@
-use crate::error::Result;
 use crate::virtdev::{DeviceRegion, EmulatedDevice, Event, Port};
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use spin::RwLock;
@@ -31,7 +31,7 @@ impl Dma8237 {
 }
 
 impl EmulatedDevice for Dma8237 {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![
             DeviceRegion::PortIo(
                 Self::DMA1_CHAN2_ADDR..=Self::DMA1_MASTER_CLEAR,

mythril/src/virtdev/ignore.rs

@@ -1,5 +1,5 @@
-use crate::error::Result;
 use crate::virtdev::{DeviceRegion, EmulatedDevice, Event};
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use spin::RwLock;
@@ -17,7 +17,7 @@ impl IgnoredDevice {
 }
 
 impl EmulatedDevice for IgnoredDevice {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![
             // Ignore #IGNNE stuff
             DeviceRegion::PortIo(241..=241),

mythril/src/virtdev/ioapic.rs

@@ -1,6 +1,6 @@
-use crate::error::Result;
 use crate::memory::GuestPhysAddr;
 use crate::virtdev::{DeviceRegion, EmulatedDevice, Event};
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use spin::RwLock;
@@ -15,7 +15,7 @@ impl IoApic {
 }
 
 impl EmulatedDevice for IoApic {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![
             DeviceRegion::MemIo(
                 GuestPhysAddr::new(0xfec00000)..=GuestPhysAddr::new(0xfec010f0),

mythril/src/virtdev/keyboard.rs

@@ -1,5 +1,5 @@
-use crate::error::Result;
 use crate::virtdev::{DeviceEvent, DeviceRegion, EmulatedDevice, Event, Port};
+use crate::{error::Result, vm::VirtualMachineConfig};
 use alloc::sync::Arc;
 use alloc::vec::Vec;
 use spin::RwLock;
@@ -17,7 +17,7 @@ impl Keyboard8042 {
 }
 
 impl EmulatedDevice for Keyboard8042 {
-    fn services(&self) -> Vec<DeviceRegion> {
+    fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
         vec![
             DeviceRegion::PortIo(Self::PS2_DATA..=Self::PS2_DATA),
             DeviceRegion::PortIo(Self::PS2_STATUS..=Self::PS2_STATUS),

mythril/src/virtdev/mod.rs

@@ -1,6 +1,9 @@
-use crate::error::{Error, Result};
 use crate::memory::{GuestAddressSpaceViewMut, GuestPhysAddr};
 use crate::vcpu;
+use crate::{
+    error::{Error, Result},
+    vm::VirtualMachineConfig,
+};
 use alloc::collections::btree_map::BTreeMap;
 use alloc::sync::Arc;
 use alloc::vec::Vec;
@@ -160,6 +163,22 @@ impl DeviceInteraction for GuestPhysAddr {
     }
 }
 
+pub struct DeviceMapBuilder<'config> {
+    pub vm_config: &'config mut VirtualMachineConfig,
+}
+
+impl<'config> DeviceMapBuilder<'config> {
+    pub fn register_device(
+        &mut self,
+        dev: Arc<RwLock<dyn EmulatedDevice>>,
+    ) -> Result<()> {
+        let services = dev.read().services(&self.vm_config);
+        self.vm_config
+            .virtual_devices_mut()
+            .register_services(services, dev)
+    }
+}
+
 /// A structure for looking up `EmulatedDevice`s by port or address
 #[derive(Default)]
 pub struct DeviceMap {
@@ -176,11 +195,11 @@ impl DeviceMap {
         op.find_device(self)
     }
 
-    pub fn register_device(
+    pub fn register_services(
         &mut self,
+        services: Vec<DeviceRegion>,
         dev: Arc<RwLock<dyn EmulatedDevice>>,
     ) -> Result<()> {
-        let services = dev.read().services();
         for region in services.into_iter() {
             match region {
                 DeviceRegion::PortIo(val) => {
@@ -221,7 +240,7 @@ impl DeviceMap {
 }
 
 pub trait EmulatedDevice: Send + Sync {
-    fn services(&self) -> Vec<DeviceRegion>;
+    fn services(&self, vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion>;
 
     fn on_event(&mut self, _event: Event) -> Result<()> {
         Ok(())
@@ -480,7 +499,7 @@ impl<'a> fmt::Display for MemReadRequest<'a> {
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::virtdev::com::*;
+    use crate::{virtdev::com::*, vm::PhysicalDeviceConfig};
     use core::convert::TryInto;
 
     // This is just a dummy device so we can have arbitrary port ranges
@@ -497,8 +516,22 @@ mod test {
         }
     }
 
+    impl VirtualMachineConfig {
+        // Default config used for testing
+        pub fn default() -> VirtualMachineConfig {
+            VirtualMachineConfig::new(
+                vec![0.into()],
+                1024,
+                PhysicalDeviceConfig::default(),
+            )
+        }
+    }
+
     impl EmulatedDevice for DummyDevice {
-        fn services(&self) -> Vec<DeviceRegion> {
+        fn services(
+            &self,
+            _vm_config: &VirtualMachineConfig,
+        ) -> Vec<DeviceRegion> {
             self.services
                 .iter()
                 .map(|x| DeviceRegion::PortIo(x.clone()))
@@ -508,9 +541,13 @@ mod test {
 
     #[test]
     fn test_device_map() {
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
+
+        let mut builder = config.device_map_builder();
         let com = Uart8250::new(0);
-        map.register_device(com).unwrap();
+        builder.register_device(com).unwrap();
+
+        let map = config.virtual_devices();
         let _dev = map.find_device(0u16).unwrap();
 
         assert_eq!(map.find_device(10u16).is_none(), true);
@@ -544,32 +581,38 @@ mod test {
 
     #[test]
     fn test_conflicting_portio_device() {
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
+
+        let mut builder = config.device_map_builder();
         let com = Uart8250::new(0);
-        map.register_device(com).unwrap();
+        builder.register_device(com).unwrap();
         let com = Uart8250::new(0);
 
-        assert!(map.register_device(com).is_err());
+        assert!(builder.register_device(com).is_err());
     }
 
     #[test]
     fn test_fully_overlapping_portio_device() {
         // region 2 fully inside region 1
         let services = vec![0..=10, 2..=8];
         let dummy = DummyDevice::new(services);
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
 
-        assert!(map.register_device(dummy).is_err());
+        let mut builder = config.device_map_builder();
+
+        assert!(builder.register_device(dummy).is_err());
     }
 
     #[test]
     fn test_fully_encompassing_portio_device() {
         // region 1 fully inside region 2
         let services = vec![2..=8, 0..=10];
         let dummy = DummyDevice::new(services);
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
+
+        let mut builder = config.device_map_builder();
 
-        assert!(map.register_device(dummy).is_err());
+        assert!(builder.register_device(dummy).is_err());
     }
 
     #[test]
@@ -578,9 +621,11 @@ mod test {
         // the start of region 2
         let services = vec![0..=4, 3..=8];
         let dummy = DummyDevice::new(services);
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
+
+        let mut builder = config.device_map_builder();
 
-        assert!(map.register_device(dummy).is_err());
+        assert!(builder.register_device(dummy).is_err());
     }
 
     #[test]
@@ -589,18 +634,22 @@ mod test {
         // the tail of region 2
         let services = vec![3..=8, 0..=4];
         let dummy = DummyDevice::new(services);
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
 
-        assert!(map.register_device(dummy).is_err());
+        let mut builder = config.device_map_builder();
+
+        assert!(builder.register_device(dummy).is_err());
     }
 
     #[test]
     fn test_non_overlapping_portio_device() {
         // region 1 and region 2 don't overlap
         let services = vec![0..=3, 4..=8];
         let dummy = DummyDevice::new(services);
-        let mut map = DeviceMap::default();
+        let mut config = VirtualMachineConfig::default();
+
+        let mut builder = config.device_map_builder();
 
-        assert!(map.register_device(dummy).is_ok());
+        assert!(builder.register_device(dummy).is_ok());
     }
 }