Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
30 changes: 15 additions & 15 deletions mythril/src/kmain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,45 +83,45 @@ fn build_vm(

acpi.add_sdt(madt).unwrap();

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();

Expand Down Expand Up @@ -155,7 +155,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(vm_id, config, info).expect("Failed to create vm")
}
Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/acpi.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion mythril/src/virtdev/com.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::physdev::com::*;
use crate::virtdev::{
DeviceEvent, DeviceEventResponse, DeviceRegion, EmulatedDevice, Event, Port,
};
use crate::vm::VirtualMachineConfig;
use alloc::sync::Arc;
use alloc::vec::Vec;
use core::convert::TryInto;
Expand Down Expand Up @@ -54,7 +55,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)]
}

Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/debug.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)]
}

Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/dma.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/ignore.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/ioapic.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions mythril/src/virtdev/keyboard.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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),
Expand Down
84 changes: 64 additions & 20 deletions mythril/src/virtdev/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::error::{Error, Result};
use crate::memory::{GuestAddressSpaceView, GuestPhysAddr};
use crate::vm::VirtualMachineConfig;
use alloc::collections::btree_map::BTreeMap;
use alloc::sync::Arc;
use alloc::vec::Vec;
Expand Down Expand Up @@ -159,6 +160,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 {
Expand All @@ -175,11 +192,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) => {
Expand Down Expand Up @@ -220,7 +237,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(())
Expand Down Expand Up @@ -483,7 +500,8 @@ impl<'a> fmt::Display for MemReadRequest<'a> {
#[cfg(test)]
mod test {
use super::*;
use crate::virtdev::com::*;
use crate::percore::CoreId;
use crate::{virtdev::com::*, vm::PhysicalDeviceConfig};
use core::convert::TryInto;

// This is just a dummy device so we can have arbitrary port ranges
Expand All @@ -500,8 +518,18 @@ mod test {
}
}

// Default config used for testing
pub fn get_test_config() -> VirtualMachineConfig {
let cpus = [CoreId::from(0)];
VirtualMachineConfig::new(&cpus, 1024, PhysicalDeviceConfig::default())
.expect("Couldn't create a test VirtualMachineConfig")
}

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()))
Expand All @@ -511,9 +539,13 @@ mod test {

#[test]
fn test_device_map() {
let mut map = DeviceMap::default();
let mut config = get_test_config();

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);
Expand Down Expand Up @@ -547,32 +579,38 @@ mod test {

#[test]
fn test_conflicting_portio_device() {
let mut map = DeviceMap::default();
let mut config = get_test_config();

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 = get_test_config();

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 = get_test_config();

let mut builder = config.device_map_builder();

assert!(map.register_device(dummy).is_err());
assert!(builder.register_device(dummy).is_err());
}

#[test]
Expand All @@ -581,9 +619,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 = get_test_config();

let mut builder = config.device_map_builder();

assert!(map.register_device(dummy).is_err());
assert!(builder.register_device(dummy).is_err());
}

#[test]
Expand All @@ -592,18 +632,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 = get_test_config();

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 = get_test_config();

let mut builder = config.device_map_builder();

assert!(map.register_device(dummy).is_ok());
assert!(builder.register_device(dummy).is_ok());
}
}
7 changes: 5 additions & 2 deletions mythril/src/virtdev/pci.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::error::{Error, Result};
use crate::virtdev::{DeviceEvent, DeviceRegion, EmulatedDevice, Event, Port};
use crate::{
error::{Error, Result},
vm::VirtualMachineConfig,
};
use alloc::collections::btree_map::BTreeMap;
use alloc::sync::Arc;
use alloc::vec::Vec;
Expand Down Expand Up @@ -189,7 +192,7 @@ impl PciRootComplex {
}

impl EmulatedDevice for PciRootComplex {
fn services(&self) -> Vec<DeviceRegion> {
fn services(&self, _vm_config: &VirtualMachineConfig) -> Vec<DeviceRegion> {
vec![
DeviceRegion::PortIo(
Self::PCI_CONFIG_ADDRESS..=Self::PCI_CONFIG_ADDRESS,
Expand Down
Loading