Resolve #78, EmulatedDevice::services should access the VM config #113
Conversation
There was a problem hiding this comment.
Thanks for working on this! I've not reviewed it yet, but I was hoping @dlrobertson could chime in with his reasoning around this issue. I understand that some devices may need to have additional information to know what guest portio addresses/memory regions they are responsible fore, but I'm not exactly sure why that would be determined by passing the VirtualMachineConfig to the services call. Wouldn't it make sense to pass the required information to the device during its construction, so it can determine whatever it needs to inside services? But I could definitely be missing something.
mythril/src/kmain.rs
Outdated
| @@ -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(); | |||
There was a problem hiding this comment.
Please remove this comment.
mythril/src/virtdev/mod.rs
Outdated
| @@ -497,8 +516,22 @@ mod test { | |||
| } | |||
| } | |||
|
|
|||
| impl VirtualMachineConfig { | |||
| // Default config used for testing | |||
| pub fn default() -> VirtualMachineConfig { | |||
There was a problem hiding this comment.
It doesn't look like this is used outside of this module, so rather than defining this default, I'd rather have a method in the mod test below that just returns just a VirtualMachineConfig and use that in the tests.
|
@dlrobertson do you have any comment on this? I'm not sure what your thinking was here |
Yeah, I think at this point something along these lines would be a better approach. |
|
I think #78 and this PR should now be changed to instead reduce the use of magic numbers like src/kmain.rs:79 and src/virtdev/ioapic.rs:21. Instead we should probably use a constant defined in the virtdev implementation when we construct the MADT. |
|
Thanks to you both for taking a look at our PR. We are not sure to understand what would be the best way moving forward, do you just want to remove magic numbers in the implementation or do you still want a way for devices to access the virtual machine configuration ? In what structure would you like to define the magic numbers used by the MADT ? |
Resolve issue #78
EmulatedDevice::servicesmethod to access the virtual machine configDeviceMapBuilderto be able to manipulateDeviceMapthroughVirtualMachineConfig