diff --git a/Cargo.lock b/Cargo.lock index 80689e68..a9d2e21e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -42,12 +42,6 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf1de2fe8c75bc145a2f577add951f8134889b4795d47466a54a5c846d691693" -[[package]] -name = "cfg-if" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" - [[package]] name = "clap" version = "3.2.17" @@ -94,9 +88,9 @@ dependencies = [ [[package]] name = "event-manager" -version = "0.2.1" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "377fa591135fbe23396a18e2655a6d5481bf7c5823cdfa3cc81b01a229cbe640" +checksum = "2bbb5f730c95a458654dee0afb13f1ebb4fc3d7b772789d5f30713ec68fed75d" dependencies = [ "libc", "vmm-sys-util", @@ -129,18 +123,18 @@ dependencies = [ [[package]] name = "kvm-bindings" -version = "0.5.0" +version = "0.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a78c049190826fff959994b7c1d8a2930d0a348f1b8f3aa4f9bb34cd5d7f2952" +checksum = "efe70e65a5b092161d17f5005b66e5eefe7a94a70c332e755036fc4af78c4e79" dependencies = [ "vmm-sys-util", ] [[package]] name = "kvm-ioctls" -version = "0.11.0" +version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97422ba48d7ffb66fd4d18130f72ab66f9bbbf791fb7a87b9291cdcfec437593" +checksum = "b8f8dc9c1896e5f144ec5d07169bc29f39a047686d29585a91f30489abfaeb6b" dependencies = [ "kvm-bindings", "libc", @@ -155,20 +149,17 @@ checksum = "8916b1f6ca17130ec6568feccee27c156ad12037880833a3b842a823236502e7" [[package]] name = "linux-loader" -version = "0.4.0" -source = "git+https://github.com/rust-vmm/linux-loader.git?rev=9a9f071#9a9f071219a8c74dcd703aec40ba0249e3a5993b" +version = "0.10.0" +source = "git+https://github.com/vitamin-v-software/linux-loader.git?rev=76aefef#76aefefc41c9a55217768bc0c89a55178db65b47" dependencies = [ "vm-memory", ] [[package]] name = "log" -version = "0.4.6" +version = "0.4.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c84ec4b527950aa83a329754b01dbe3f58361d1c5efacd1f6d68c494d08a17c6" -dependencies = [ - "cfg-if", -] +checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" [[package]] name = "os_str_bytes" @@ -178,18 +169,18 @@ checksum = "9ff7415e9ae3fff1225851df9e0d9e4e5479f947619774677a63572e55e80eff" [[package]] name = "proc-macro2" -version = "1.0.34" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f84e92c0f7c9d58328b85a78557813e4bd845130db68d7184635344399423b1" +checksum = "134c189feb4956b20f6f547d2cf727d4c0fe06722b20a0eec87ed445a97f92da" dependencies = [ - "unicode-xid", + "unicode-ident", ] [[package]] name = "quote" -version = "1.0.10" +version = "1.0.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "38bc8cc6a5f2e3655e0899c1b848643b2562f853f114bfec7be120678e3ace05" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" dependencies = [ "proc-macro2", ] @@ -202,13 +193,13 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" [[package]] name = "syn" -version = "1.0.82" +version = "2.0.39" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8daf5dd0bb60cbd4137b1b587d2fc0ae729bc07cf01cd70b36a1ed5ade3b9d59" +checksum = "23e78b90f2fcf45d3e842032ce32e3f2d1545ba6636271dcbf24fa306d87be7a" dependencies = [ "proc-macro2", "quote", - "unicode-xid", + "unicode-ident", ] [[package]] @@ -228,18 +219,18 @@ checksum = "b1141d4d61095b28419e22cb0bbf02755f5e54e0526f97f1e3d1d160e60885fb" [[package]] name = "thiserror" -version = "1.0.30" +version = "1.0.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "854babe52e4df1653706b98fcfc05843010039b406875930a70e4d9644e5c417" +checksum = "f9a7210f5c9a7156bb50aa36aed4c95afb51df0df00713949448cf9e97d382d2" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.30" +version = "1.0.50" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa32fd3f627f367fe16f893e2597ae3c05020f8bba2666a4e6ea73d377e5714b" +checksum = "266b2e40bc00e5a6c09c3584011e08b06f123c00362c92b975ba9843aaaa14b8" dependencies = [ "proc-macro2", "quote", @@ -247,21 +238,27 @@ dependencies = [ ] [[package]] -name = "unicode-xid" -version = "0.2.2" +name = "unicode-ident" +version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" +checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" [[package]] name = "utils" version = "0.1.0" +[[package]] +name = "virtio-bindings" +version = "0.2.2" +source = "git+https://github.com/rust-vmm/vm-virtio.git#3bbf0db2c24fe756ac3593f259d048dafc1fd3c5" + [[package]] name = "virtio-blk" version = "0.1.0" -source = "git+https://github.com/rust-vmm/vm-virtio.git#d8ef45f57b46baa99e80e555deffd3fa1ab9affc" +source = "git+https://github.com/rust-vmm/vm-virtio.git#3bbf0db2c24fe756ac3593f259d048dafc1fd3c5" dependencies = [ "log", + "virtio-bindings", "virtio-device", "virtio-queue", "vm-memory", @@ -271,19 +268,21 @@ dependencies = [ [[package]] name = "virtio-device" version = "0.1.0" -source = "git+https://github.com/rust-vmm/vm-virtio.git#d8ef45f57b46baa99e80e555deffd3fa1ab9affc" +source = "git+https://github.com/rust-vmm/vm-virtio.git#3bbf0db2c24fe756ac3593f259d048dafc1fd3c5" dependencies = [ "log", + "virtio-bindings", "virtio-queue", "vm-memory", ] [[package]] name = "virtio-queue" -version = "0.2.0" -source = "git+https://github.com/rust-vmm/vm-virtio.git#d8ef45f57b46baa99e80e555deffd3fa1ab9affc" +version = "0.10.0" +source = "git+https://github.com/rust-vmm/vm-virtio.git#3bbf0db2c24fe756ac3593f259d048dafc1fd3c5" dependencies = [ "log", + "virtio-bindings", "vm-memory", "vmm-sys-util", ] @@ -312,11 +311,12 @@ checksum = "f43fb5a6bd1a7d423ad72802801036719b7546cf847a103f8fe4575f5b0d45a6" [[package]] name = "vm-memory" -version = "0.7.0" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "339d4349c126fdcd87e034631d7274370cf19eb0e87b33166bcd956589fc72c5" +checksum = "5376c9ee5ebe2103a310d8241936cfb93c946734b0479a4fa5bdf7a64abbacd8" dependencies = [ "libc", + "thiserror", "winapi", ] @@ -385,9 +385,9 @@ dependencies = [ [[package]] name = "vmm-sys-util" -version = "0.8.0" +version = "0.11.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cf11afbc4ebc0d5c7a7748a77d19e2042677fc15faa2f4ccccb27c18a60605" +checksum = "48b7b084231214f7427041e4220d77dfe726897a6d41fddee450696e66ff2a29" dependencies = [ "bitflags", "libc", diff --git a/Cargo.toml b/Cargo.toml index aa0039fb..d3f94aca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,5 +21,6 @@ lto = true panic = "abort" [patch.crates-io] -# TODO: Using this patch until a version > 4.0 gets published. -linux-loader = { git = "https://github.com/rust-vmm/linux-loader.git", rev = "9a9f071" } +# TODO: Update with https://github.com/rust-vmm/linux-loader.git hash of commit +# "add as_string() to the Cmdline crate" +linux-loader = { git = "https://github.com/vitamin-v-software/linux-loader.git", rev = "76aefef" } diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml index 7bc3dc38..7c08e547 100644 --- a/src/api/Cargo.toml +++ b/src/api/Cargo.toml @@ -10,4 +10,4 @@ clap = "3.2.17" vmm = { path = "../vmm" } [dev-dependencies] -linux-loader = "0.4.0" +linux-loader = "0.10.0" diff --git a/src/api/src/lib.rs b/src/api/src/lib.rs index 9d23c992..c9671da0 100644 --- a/src/api/src/lib.rs +++ b/src/api/src/lib.rs @@ -211,7 +211,7 @@ mod tests { ]) .is_err()); - let mut foo_cmdline = Cmdline::new(4096); + let mut foo_cmdline = Cmdline::new(4096).unwrap(); foo_cmdline.insert_str("\"foo=bar bar=foo\"").unwrap(); // OK. diff --git a/src/arch/Cargo.toml b/src/arch/Cargo.toml index f3a2eb9b..437893c5 100644 --- a/src/arch/Cargo.toml +++ b/src/arch/Cargo.toml @@ -8,4 +8,4 @@ edition = "2018" [dependencies] vm-fdt = "0.2.0" -vm-memory = "0.7.0" +vm-memory = "0.13.1" diff --git a/src/devices/Cargo.toml b/src/devices/Cargo.toml index eb7701ee..0338787d 100644 --- a/src/devices/Cargo.toml +++ b/src/devices/Cargo.toml @@ -6,14 +6,14 @@ edition = "2018" license = "Apache-2.0 OR BSD-3-Clause" [dependencies] -event-manager = { version = "0.2.1", features = ["remote_endpoint"] } -kvm-ioctls = "0.11.0" +event-manager = { version = "0.3.0", features = ["remote_endpoint"] } +kvm-ioctls = "0.13.0" libc = "0.2.76" -linux-loader = "0.4.0" -log = "0.4.6" -vm-memory = "0.7.0" +linux-loader = "0.10.0" +log = "*" +vm-memory = "0.13.1" vm-superio = "0.5.0" -vmm-sys-util = "0.8.0" +vmm-sys-util = "0.11.2" vm-device = "0.1.0" virtio-blk = { git = "https://github.com/rust-vmm/vm-virtio.git", features = ["backend-stdio"] } @@ -23,5 +23,5 @@ virtio-queue = { git = "https://github.com/rust-vmm/vm-virtio.git"} utils = { path = "../utils" } [dev-dependencies] -vm-memory = { version = "0.7.0", features = ["backend-mmap"] } -kvm-bindings = "0.5.0" +vm-memory = { version = "0.13.1", features = ["backend-mmap"] } +kvm-bindings = "0.6.0" diff --git a/src/devices/src/virtio/block/device.rs b/src/devices/src/virtio/block/device.rs index a885a47e..b972ff74 100644 --- a/src/devices/src/virtio/block/device.rs +++ b/src/devices/src/virtio/block/device.rs @@ -9,11 +9,11 @@ use std::sync::{Arc, Mutex}; use virtio_blk::stdio_executor::StdIoBackend; use virtio_device::{VirtioConfig, VirtioDeviceActions, VirtioDeviceType, VirtioMmioDevice}; -use virtio_queue::Queue; +use virtio_queue::{Queue, QueueT}; use vm_device::bus::MmioAddress; use vm_device::device_manager::MmioManager; use vm_device::{DeviceMmio, MutDeviceMmio}; -use vm_memory::GuestAddressSpace; +use vm_memory::{GuestAddressSpace, GuestMemoryMmap}; use crate::virtio::block::{BLOCK_DEVICE_ID, VIRTIO_BLK_F_RO}; use crate::virtio::{CommonConfig, Env, SingleFdSignalQueue, QUEUE_MAX_SIZE}; @@ -25,31 +25,34 @@ use super::{build_config_space, BlockArgs, Error, Result}; // This Block device can only use the MMIO transport for now, but we plan to reuse large parts of // the functionality when we implement virtio PCI as well, for example by having a base generic // type, and then separate concrete instantiations for `MmioConfig` and `PciConfig`. -pub struct Block { +pub struct Block { cfg: CommonConfig, file_path: PathBuf, read_only: bool, // We'll prob need to remember this for state save/restore unless we pass the info from // the outside. _root_device: bool, + mem: Arc, } -impl Block -where - M: GuestAddressSpace + Clone + Send + 'static, -{ +impl Block { // Helper method that only creates a `Block` object. - fn create_block(env: &mut Env, args: &BlockArgs) -> Result { + fn create_block( + mem: Arc, + env: &mut Env, + args: &BlockArgs, + ) -> Result { let device_features = args.device_features(); // A block device has a single queue. - let queues = vec![Queue::new(env.mem.clone(), QUEUE_MAX_SIZE)]; + let queues = vec![Queue::new(QUEUE_MAX_SIZE).unwrap()]; let config_space = build_config_space(&args.file_path)?; let virtio_cfg = VirtioConfig::new(device_features, queues, config_space); let common_cfg = CommonConfig::new(virtio_cfg, env).map_err(Error::Virtio)?; Ok(Block { + mem, cfg: common_cfg, file_path: args.file_path.clone(), read_only: args.read_only, @@ -59,14 +62,18 @@ where // Create `Block` object, register it on the MMIO bus, and add any extra required info to // the kernel cmdline from the environment. - pub fn new(env: &mut Env, args: &BlockArgs) -> Result>> + pub fn new( + mem: Arc, + env: &mut Env, + args: &BlockArgs, + ) -> Result>> where // We're using this (more convoluted) bound so we can pass both references and smart // pointers such as mutex guards here. B: DerefMut, B::Target: MmioManager>, { - let block = Arc::new(Mutex::new(Self::create_block(env, args)?)); + let block = Arc::new(Mutex::new(Self::create_block(mem, env, args)?)); // Register the device on the MMIO bus. env.register_mmio_device(block.clone()) @@ -79,14 +86,14 @@ where } } -impl Borrow> for Block { - fn borrow(&self) -> &VirtioConfig { +impl Borrow> for Block { + fn borrow(&self) -> &VirtioConfig { &self.cfg.virtio } } -impl BorrowMut> for Block { - fn borrow_mut(&mut self) -> &mut VirtioConfig { +impl BorrowMut> for Block { + fn borrow_mut(&mut self) -> &mut VirtioConfig { &mut self.cfg.virtio } } @@ -131,6 +138,7 @@ impl VirtioDeviceActions for Bloc }; let handler = Arc::new(Mutex::new(QueueHandler { + mem: self.mem.clone(), inner, ioeventfd: ioevents.remove(0), })); @@ -144,7 +152,7 @@ impl VirtioDeviceActions for Bloc } } -impl VirtioMmioDevice for Block {} +impl VirtioMmioDevice for Block {} impl MutDeviceMmio for Block { fn mmio_read(&mut self, _base: MmioAddress, offset: u64, data: &mut [u8]) { @@ -177,13 +185,13 @@ mod tests { advertise_flush: true, }; - let block_mutex = Block::new(&mut env, &args).unwrap(); + let block_mutex = Block::new(env.mem.clone(), &mut env, &args).unwrap(); let block = block_mutex.lock().unwrap(); assert_eq!(block.device_type(), BLOCK_DEVICE_ID); assert_eq!( - mock.kernel_cmdline.as_str(), + mock.kernel_cmdline.as_string().unwrap(), format!( "virtio_mmio.device=4K@0x{:x}:{} root=/dev/vda ro", mock.mmio_cfg.range.base().0, diff --git a/src/devices/src/virtio/block/inorder_handler.rs b/src/devices/src/virtio/block/inorder_handler.rs index eb8f04a6..376fc221 100644 --- a/src/devices/src/virtio/block/inorder_handler.rs +++ b/src/devices/src/virtio/block/inorder_handler.rs @@ -7,8 +7,10 @@ use std::result; use log::warn; use virtio_blk::request::Request; use virtio_blk::stdio_executor::{self, StdIoBackend}; -use virtio_queue::{DescriptorChain, Queue}; -use vm_memory::{self, GuestAddressSpace}; +use virtio_queue::{DescriptorChain, Queue, QueueOwnedT, QueueT}; +use vm_memory::{self, GuestAddressSpace, GuestMemoryMmap}; + +use std::sync::Arc; use crate::virtio::SignalUsedQueue; @@ -42,18 +44,20 @@ impl From for Error { // object), but the aim is to have a way of working with generic backends and turn this into // a more flexible building block. The name comes from processing and returning descriptor // chains back to the device in the same order they are received. -pub struct InOrderQueueHandler { +pub struct InOrderQueueHandler { pub driver_notify: S, - pub queue: Queue, + pub queue: Queue, pub disk: StdIoBackend, } -impl InOrderQueueHandler +impl InOrderQueueHandler where - M: GuestAddressSpace, S: SignalUsedQueue, { - fn process_chain(&mut self, mut chain: DescriptorChain) -> result::Result<(), Error> { + fn process_chain( + &mut self, + mut chain: DescriptorChain, + ) -> result::Result<(), Error> { let used_len = match Request::parse(&mut chain) { Ok(request) => self.disk.process_request(chain.memory(), &request)?, Err(e) => { @@ -62,26 +66,28 @@ where } }; - self.queue.add_used(chain.head_index(), used_len)?; + self.queue + .add_used(chain.memory(), chain.head_index(), used_len)?; - if self.queue.needs_notification()? { + if self.queue.needs_notification(chain.memory())? { self.driver_notify.signal_used_queue(0); } Ok(()) } - pub fn process_queue(&mut self) -> result::Result<(), Error> { + pub fn process_queue(&mut self, mem: Arc) -> result::Result<(), Error> { // To see why this is done in a loop, please look at the `Queue::enable_notification` // comments in `virtio_queue`. + loop { - self.queue.disable_notification()?; + self.queue.disable_notification(mem.as_ref())?; - while let Some(chain) = self.queue.iter()?.next() { - self.process_chain(chain)?; + while let Some(chain) = self.queue.iter(mem.as_ref())?.next() { + self.process_chain::<&GuestMemoryMmap>(chain)?; } - if !self.queue.enable_notification()? { + if !self.queue.enable_notification(mem.as_ref())? { break; } } diff --git a/src/devices/src/virtio/block/queue_handler.rs b/src/devices/src/virtio/block/queue_handler.rs index 13689227..efab7d2d 100644 --- a/src/devices/src/virtio/block/queue_handler.rs +++ b/src/devices/src/virtio/block/queue_handler.rs @@ -3,7 +3,8 @@ use event_manager::{EventOps, Events, MutEventSubscriber}; use log::error; -use vm_memory::GuestAddressSpace; +use std::sync::Arc; +use vm_memory::GuestMemoryMmap; use vmm_sys_util::epoll::EventSet; use vmm_sys_util::eventfd::EventFd; @@ -16,12 +17,13 @@ const IOEVENT_DATA: u32 = 0; // signalling implementation based on `EventFd`s, and then also implements `MutEventSubscriber` // to interact with the event manager. `ioeventfd` is the `EventFd` connected to queue // notifications coming from the driver. -pub(crate) struct QueueHandler { - pub inner: InOrderQueueHandler, +pub(crate) struct QueueHandler { + pub mem: Arc, + pub inner: InOrderQueueHandler, pub ioeventfd: EventFd, } -impl MutEventSubscriber for QueueHandler { +impl MutEventSubscriber for QueueHandler { fn process(&mut self, events: Events, ops: &mut EventOps) { let mut error = true; @@ -33,7 +35,7 @@ impl MutEventSubscriber for QueueHandler { error!("unexpected events data {}", events.data()); } else if self.ioeventfd.read().is_err() { error!("ioeventfd read error") - } else if let Err(e) = self.inner.process_queue() { + } else if let Err(e) = self.inner.process_queue(self.mem.clone()) { error!("error processing block queue {:?}", e); } else { error = false; diff --git a/src/devices/src/virtio/mod.rs b/src/devices/src/virtio/mod.rs index d3cfc435..63b9e287 100644 --- a/src/devices/src/virtio/mod.rs +++ b/src/devices/src/virtio/mod.rs @@ -8,7 +8,7 @@ pub mod net; use std::convert::TryFrom; use std::io; -use std::ops::DerefMut; +use std::ops::{Deref, DerefMut}; use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::{Arc, Mutex}; @@ -19,6 +19,7 @@ use event_manager::{ use kvm_ioctls::{IoEventAddress, VmFd}; use linux_loader::cmdline::Cmdline; use virtio_device::VirtioConfig; +use virtio_queue::{Queue, QueueT}; use vm_device::bus::{self, MmioAddress, MmioRange}; use vm_device::device_manager::MmioManager; use vm_device::DeviceMmio; @@ -154,16 +155,17 @@ where } // Holds configuration objects which are common to all current devices. -pub struct CommonConfig { - pub virtio: VirtioConfig, +pub struct CommonConfig { + pub mem: M, + pub virtio: VirtioConfig, pub mmio: MmioConfig, pub endpoint: RemoteEndpoint, pub vm_fd: Arc, pub irqfd: Arc, } -impl CommonConfig { - pub fn new(virtio_cfg: VirtioConfig, env: &Env) -> Result { +impl CommonConfig { + pub fn new(virtio_cfg: VirtioConfig, env: &Env) -> Result { let irqfd = Arc::new(EventFd::new(EFD_NONBLOCK).map_err(Error::EventFd)?); env.vm_fd @@ -171,6 +173,7 @@ impl CommonConfig { .map_err(Error::RegisterIrqfd)?; Ok(CommonConfig { + mem: env.mem.clone(), virtio: virtio_cfg, mmio: env.mmio_cfg, endpoint: env.event_mgr.remote_endpoint(), @@ -183,9 +186,15 @@ impl CommonConfig { // a `Vec` that contains `EventFd`s registered as ioeventfds, which are used to convey queue // notifications coming from the driver. pub fn prepare_activate(&self) -> Result> { - if !self.virtio.queues_valid() { - return Err(Error::QueuesNotValid); - } + let m = self.mem.memory(); + let guest_mem = <::T>::deref(&m); + + self.virtio + .queues + .iter() + .all(|q| ::is_valid(q, guest_mem)) + .then_some(()) + .ok_or(Error::QueuesNotValid)?; if self.virtio.device_activated { return Err(Error::AlreadyActivated); @@ -284,7 +293,6 @@ pub(crate) mod tests { kvm_create_device, kvm_device_attr, kvm_device_type_KVM_DEV_TYPE_ARM_VGIC_V3, KVM_DEV_ARM_VGIC_CTRL_INIT, KVM_DEV_ARM_VGIC_GRP_CTRL, }; - use virtio_queue::Queue; pub type MockMem = Arc; // Can be used in other modules to test functionality that requires a `CommonArgs` struct as @@ -324,7 +332,7 @@ pub(crate) mod tests { mmio_mgr: IoManager::new(), mmio_cfg, // `4096` seems large enough for testing. - kernel_cmdline: Cmdline::new(4096), + kernel_cmdline: Cmdline::new(4096).unwrap(), } } pub fn env(&mut self) -> Env { @@ -379,7 +387,7 @@ pub(crate) mod tests { assert_eq!(bus_range.size(), range.size()); assert_eq!( - mock.kernel_cmdline.as_str(), + mock.kernel_cmdline.as_string().unwrap(), format!( "virtio_mmio.device=4K@0x{:x}:{}", range.base().0, @@ -388,7 +396,11 @@ pub(crate) mod tests { ); mock.env().insert_cmdline_str("ending_string").unwrap(); - assert!(mock.kernel_cmdline.as_str().ends_with("ending_string")); + assert!(mock + .kernel_cmdline + .as_string() + .unwrap() + .ends_with("ending_string")); } #[test] @@ -397,7 +409,7 @@ pub(crate) mod tests { let env = mock.env(); let device_features = 0; - let queues = vec![Queue::new(env.mem.clone(), 256)]; + let queues = vec![Queue::new(256).unwrap()]; let config_space = Vec::new(); let virtio_cfg = VirtioConfig::new(device_features, queues, config_space); @@ -407,8 +419,8 @@ pub(crate) mod tests { assert!(matches!(cfg.prepare_activate(), Err(Error::QueuesNotValid))); // Let's pretend the queue has been configured such that the `is_valid` check passes. - cfg.virtio.queues[0].state.ready = true; - cfg.virtio.queues[0].state.size = 256; + cfg.virtio.queues[0].set_ready(true); + cfg.virtio.queues[0].set_size(256); // This will fail because the "driver" didn't acknowledge `VIRTIO_F_VERSION_1`. assert!(matches!(cfg.prepare_activate(), Err(Error::BadFeatures(0)))); diff --git a/src/devices/src/virtio/net/device.rs b/src/devices/src/virtio/net/device.rs index ea78bc12..5d001748 100644 --- a/src/devices/src/virtio/net/device.rs +++ b/src/devices/src/virtio/net/device.rs @@ -6,11 +6,11 @@ use std::ops::DerefMut; use std::sync::{Arc, Mutex}; use virtio_device::{VirtioConfig, VirtioDeviceActions, VirtioDeviceType, VirtioMmioDevice}; -use virtio_queue::Queue; +use virtio_queue::{Queue, QueueT}; use vm_device::bus::MmioAddress; use vm_device::device_manager::MmioManager; use vm_device::{DeviceMmio, MutDeviceMmio}; -use vm_memory::GuestAddressSpace; +use vm_memory::{GuestAddressSpace, GuestMemoryMmap}; use crate::virtio::features::{VIRTIO_F_IN_ORDER, VIRTIO_F_RING_EVENT_IDX, VIRTIO_F_VERSION_1}; use crate::virtio::net::features::*; @@ -22,16 +22,24 @@ use super::queue_handler::QueueHandler; use super::simple_handler::SimpleHandler; use super::tap::Tap; -pub struct Net { +pub struct Net +where + M: GuestAddressSpace + Clone + Send + Sync + 'static, +{ + mem: Arc, cfg: CommonConfig, tap_name: String, } impl Net where - M: GuestAddressSpace + Clone + Send + 'static, + M: GuestAddressSpace + Clone + Send + Sync + 'static, { - pub fn new(env: &mut Env, args: &NetArgs) -> Result>> + pub fn new( + mem: Arc, + env: &mut Env, + args: &NetArgs, + ) -> Result>> where // We're using this (more convoluted) bound so we can pass both references and smart // pointers such as mutex guards here. @@ -52,8 +60,8 @@ where // An rx/tx queue pair. let queues = vec![ - Queue::new(env.mem.clone(), QUEUE_MAX_SIZE), - Queue::new(env.mem.clone(), QUEUE_MAX_SIZE), + Queue::new(QUEUE_MAX_SIZE).unwrap(), + Queue::new(QUEUE_MAX_SIZE).unwrap(), ]; // TODO: We'll need a minimal config space to support setting an explicit MAC addr @@ -64,6 +72,7 @@ where let common_cfg = CommonConfig::new(virtio_cfg, env).map_err(Error::Virtio)?; let net = Arc::new(Mutex::new(Net { + mem, cfg: common_cfg, tap_name: args.tap_name.clone(), })); @@ -75,25 +84,27 @@ where } } -impl VirtioDeviceType for Net { +impl VirtioDeviceType for Net { fn device_type(&self) -> u32 { NET_DEVICE_ID } } -impl Borrow> for Net { - fn borrow(&self) -> &VirtioConfig { +impl Borrow> for Net { + fn borrow(&self) -> &VirtioConfig { &self.cfg.virtio } } -impl BorrowMut> for Net { - fn borrow_mut(&mut self) -> &mut VirtioConfig { +impl BorrowMut> + for Net +{ + fn borrow_mut(&mut self) -> &mut VirtioConfig { &mut self.cfg.virtio } } -impl VirtioDeviceActions for Net { +impl VirtioDeviceActions for Net { type E = Error; fn activate(&mut self) -> Result<()> { @@ -123,7 +134,7 @@ impl VirtioDeviceActions for Net< let rxq = self.cfg.virtio.queues.remove(0); let txq = self.cfg.virtio.queues.remove(0); - let inner = SimpleHandler::new(driver_notify, rxq, txq, tap); + let inner = SimpleHandler::new(self.mem.clone(), driver_notify, rxq, txq, tap); let handler = Arc::new(Mutex::new(QueueHandler { inner, @@ -140,9 +151,9 @@ impl VirtioDeviceActions for Net< } } -impl VirtioMmioDevice for Net {} +impl VirtioMmioDevice for Net {} -impl MutDeviceMmio for Net { +impl MutDeviceMmio for Net { fn mmio_read(&mut self, _base: MmioAddress, offset: u64, data: &mut [u8]) { self.read(offset, data); } diff --git a/src/devices/src/virtio/net/queue_handler.rs b/src/devices/src/virtio/net/queue_handler.rs index 4759c2bf..90193dd8 100644 --- a/src/devices/src/virtio/net/queue_handler.rs +++ b/src/devices/src/virtio/net/queue_handler.rs @@ -1,9 +1,10 @@ // Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause +use std::os::fd::AsRawFd; + use event_manager::{EventOps, Events, MutEventSubscriber}; use log::error; -use vm_memory::GuestAddressSpace; use vmm_sys_util::epoll::EventSet; use vmm_sys_util::eventfd::EventFd; @@ -15,13 +16,13 @@ const TAPFD_DATA: u32 = 0; const RX_IOEVENT_DATA: u32 = 1; const TX_IOEVENT_DATA: u32 = 2; -pub struct QueueHandler { - pub inner: SimpleHandler, +pub struct QueueHandler { + pub inner: SimpleHandler, pub rx_ioevent: EventFd, pub tx_ioevent: EventFd, } -impl QueueHandler { +impl QueueHandler { // Helper method that receives an error message to be logged and the `ops` handle // which is used to unregister all events. fn handle_error>(&self, s: S, ops: &mut EventOps) { @@ -30,12 +31,12 @@ impl QueueHandler { .expect("Failed to remove rx ioevent"); ops.remove(Events::empty(&self.tx_ioevent)) .expect("Failed to remove tx ioevent"); - ops.remove(Events::empty(&self.inner.tap)) + ops.remove(Events::empty(&self.inner.tap.borrow().as_raw_fd())) .expect("Failed to remove tap event"); } } -impl MutEventSubscriber for QueueHandler { +impl MutEventSubscriber for QueueHandler { fn process(&mut self, events: Events, ops: &mut EventOps) { // TODO: We can also consider panicking on the errors that cannot be generated // or influenced. @@ -71,8 +72,10 @@ impl MutEventSubscriber for QueueHandler { } fn init(&mut self, ops: &mut EventOps) { + let raw_fd = &mut self.inner.tap.borrow_mut().as_raw_fd(); + ops.add(Events::with_data( - &self.inner.tap, + raw_fd, TAPFD_DATA, EventSet::IN | EventSet::EDGE_TRIGGERED, )) diff --git a/src/devices/src/virtio/net/simple_handler.rs b/src/devices/src/virtio/net/simple_handler.rs index e3df98ef..3972c88e 100644 --- a/src/devices/src/virtio/net/simple_handler.rs +++ b/src/devices/src/virtio/net/simple_handler.rs @@ -1,17 +1,19 @@ // Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause +use std::cell::RefCell; use std::cmp; use std::io::{self, Read, Write}; use std::result; use log::warn; -use virtio_queue::{DescriptorChain, Queue}; -use vm_memory::{Bytes, GuestAddressSpace}; +use virtio_queue::{DescriptorChain, Queue, QueueOwnedT, QueueT}; +use vm_memory::{Bytes, GuestMemoryMmap}; use crate::virtio::net::tap::Tap; use crate::virtio::net::{RXQ_INDEX, TXQ_INDEX}; use crate::virtio::SignalUsedQueue; +use std::sync::Arc; // According to the standard: "If the VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6 or // VIRTIO_NET_F_GUEST_UFO features are used, the maximum incoming packet will be to 65550 @@ -38,27 +40,35 @@ impl From for Error { // A simple handler implementation for a RX/TX queue pair, which does not make assumptions about // the way queue notification is implemented. The backend is not yet generic (we always assume a // `Tap` object), but we're looking at improving that going forward. -// TODO: Find a better name. -pub struct SimpleHandler { +pub struct SimpleHandler { + pub mem: Arc, pub driver_notify: S, - pub rxq: Queue, + pub rxq: Queue, pub rxbuf_current: usize, pub rxbuf: [u8; MAX_BUFFER_SIZE], - pub txq: Queue, - pub txbuf: [u8; MAX_BUFFER_SIZE], - pub tap: Tap, + pub txq: Queue, + pub txbuf: RefCell<[u8; MAX_BUFFER_SIZE]>, + pub tap: RefCell, } -impl SimpleHandler { - pub fn new(driver_notify: S, rxq: Queue, txq: Queue, tap: Tap) -> Self { +impl SimpleHandler { + pub fn new( + mem: Arc, + driver_notify: S, + rxq: Queue, + txq: Queue, + tap: Tap, + ) -> Self { SimpleHandler { + mem, driver_notify, rxq, rxbuf_current: 0, rxbuf: [0u8; MAX_BUFFER_SIZE], txq, - txbuf: [0u8; MAX_BUFFER_SIZE], - tap, + //inner: RefCell::new(SimpleHandlerInner { txbuf: [0u8; MAX_BUFFER_SIZE], tap }), + txbuf: RefCell::new([0u8; MAX_BUFFER_SIZE]), + tap: RefCell::new(tap), } } @@ -69,7 +79,7 @@ impl SimpleHandler { fn write_frame_to_guest(&mut self) -> result::Result { let num_bytes = self.rxbuf_current; - let mut chain = match self.rxq.iter()?.next() { + let mut chain = match self.rxq.iter(self.mem.as_ref())?.next() { Some(c) => c, _ => return Ok(false), }; @@ -98,7 +108,8 @@ impl SimpleHandler { warn!("rx frame too large"); } - self.rxq.add_used(chain.head_index(), count as u32)?; + self.rxq + .add_used(self.mem.as_ref(), chain.head_index(), count as u32)?; self.rxbuf_current = 0; @@ -108,7 +119,7 @@ impl SimpleHandler { pub fn process_tap(&mut self) -> result::Result<(), Error> { loop { if self.rxbuf_current == 0 { - match self.tap.read(&mut self.rxbuf) { + match self.tap.borrow_mut().read(&mut self.rxbuf) { Ok(n) => self.rxbuf_current = n, Err(_) => { // TODO: Do something (logs, metrics, etc.) in response to an error when @@ -119,12 +130,12 @@ impl SimpleHandler { } } - if !self.write_frame_to_guest()? && !self.rxq.enable_notification()? { + if !self.write_frame_to_guest()? && !self.rxq.enable_notification(self.mem.as_ref())? { break; } } - if self.rxq.needs_notification()? { + if self.rxq.needs_notification(self.mem.as_ref())? { self.driver_notify.signal_used_queue(RXQ_INDEX); } @@ -132,13 +143,13 @@ impl SimpleHandler { } fn send_frame_from_chain( - &mut self, - chain: &mut DescriptorChain, + &self, + chain: &mut DescriptorChain<&GuestMemoryMmap>, ) -> result::Result { let mut count = 0; while let Some(desc) = chain.next() { - let left = self.txbuf.len() - count; + let left = self.txbuf.borrow().len() - count; let len = desc.len() as usize; if len > left { @@ -148,39 +159,46 @@ impl SimpleHandler { chain .memory() - .read_slice(&mut self.txbuf[count..count + len], desc.addr()) + .read_slice( + &mut self.txbuf.borrow_mut()[count..count + len], + desc.addr(), + ) .map_err(Error::GuestMemory)?; count += len; } - self.tap.write(&self.txbuf[..count]).map_err(Error::Tap)?; + self.tap + .borrow_mut() + .write(&self.txbuf.borrow()[..count]) + .map_err(Error::Tap)?; Ok(count as u32) } pub fn process_txq(&mut self) -> result::Result<(), Error> { loop { - self.txq.disable_notification()?; + self.txq.disable_notification(self.mem.as_ref())?; - while let Some(mut chain) = self.txq.iter()?.next() { + while let Some(mut chain) = self.txq.iter(self.mem.as_ref())?.next() { self.send_frame_from_chain(&mut chain)?; - self.txq.add_used(chain.head_index(), 0)?; + self.txq + .add_used(self.mem.as_ref(), chain.head_index(), 0)?; - if self.txq.needs_notification()? { + if self.txq.needs_notification(self.mem.as_ref())? { self.driver_notify.signal_used_queue(TXQ_INDEX); } } - if !self.txq.enable_notification()? { + if !self.txq.enable_notification(self.mem.as_ref())? { return Ok(()); } } } pub fn process_rxq(&mut self) -> result::Result<(), Error> { - self.rxq.disable_notification()?; + self.rxq.disable_notification(self.mem.as_ref())?; self.process_tap() } } diff --git a/src/devices/src/virtio/net/tap.rs b/src/devices/src/virtio/net/tap.rs index 0db62d98..236f836a 100644 --- a/src/devices/src/virtio/net/tap.rs +++ b/src/devices/src/virtio/net/tap.rs @@ -14,7 +14,7 @@ use std::os::raw::{c_char, c_int, c_uint, c_ulong}; use std::os::unix::io::{AsRawFd, FromRawFd, RawFd}; use vmm_sys_util::ioctl::{ioctl_with_mut_ref, ioctl_with_ref, ioctl_with_val}; -use vmm_sys_util::{ioctl_expr, ioctl_ioc_nr, ioctl_iow_nr}; +use vmm_sys_util::{ioctl_ioc_nr, ioctl_iow_nr}; use super::bindings::ifreq; diff --git a/src/vm-vcpu-ref/Cargo.toml b/src/vm-vcpu-ref/Cargo.toml index 2080f4af..848554a6 100644 --- a/src/vm-vcpu-ref/Cargo.toml +++ b/src/vm-vcpu-ref/Cargo.toml @@ -11,11 +11,11 @@ keywords = ["virt", "kvm", "vm"] [dependencies] thiserror = "1.0.30" -kvm-ioctls = { version = "0.11.0" } -kvm-bindings = { version = "0.5.0", features = ["fam-wrappers"] } -vm-memory = "0.7.0" +kvm-ioctls = "0.13.0" +kvm-bindings = { version = "0.6.0", features = ["fam-wrappers"] } +vm-memory = "0.13.1" libc = "0.2.76" [dev-dependencies] -vm-memory = { version = "0.7.0", features = ["backend-mmap"] } -vmm-sys-util = "0.8.0" +vm-memory = { version = "0.13.1", features = ["backend-mmap"] } +vmm-sys-util = "0.11.2" diff --git a/src/vm-vcpu-ref/src/x86_64/mptable.rs b/src/vm-vcpu-ref/src/x86_64/mptable.rs index d8ec2c06..5fc2cb85 100644 --- a/src/vm-vcpu-ref/src/x86_64/mptable.rs +++ b/src/vm-vcpu-ref/src/x86_64/mptable.rs @@ -5,7 +5,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the THIRD-PARTY file. -use std::io; use std::mem; use std::result; use std::slice; @@ -190,7 +189,8 @@ impl MpTable { let mut checksum: u8 = 0; let max_ioapic_id = self.cpu_num + 1; - mem.read_from(base_mp, &mut io::repeat(0), mp_size) + let zero_slice: Vec = (0..mp_size).map(|_| 0u8).collect(); + mem.write_slice(zero_slice.as_slice(), base_mp) .map_err(|_| Error::Clear)?; { @@ -405,23 +405,14 @@ mod tests { let mpc_offset = GuestAddress(u64::from(mpf_intel.0.physptr)); let mpc_table: MpcTable = mem.read_obj(mpc_offset).unwrap(); - struct Sum(u8); - impl io::Write for Sum { - fn write(&mut self, buf: &[u8]) -> io::Result { - for v in buf.iter() { - self.0 = self.0.wrapping_add(*v); - } - Ok(buf.len()) - } - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } - } - - let mut sum = Sum(0); - mem.write_to(mpc_offset, &mut sum, mpc_table.0.length as usize) + let mut mpc_table: Vec = (0..mpc_table.0.length).map(|_| 0u8).collect(); + mem.read_slice(mpc_table.as_mut_slice(), mpc_offset) .unwrap(); - assert_eq!(sum.0, 0); + let mut csum: u8 = 0; + mpc_table + .iter() + .for_each(|byte| csum = csum.wrapping_add(*byte)); + assert_eq!(csum, 0); } #[test] diff --git a/src/vm-vcpu/Cargo.toml b/src/vm-vcpu/Cargo.toml index a3f878fb..f7e5e6f1 100644 --- a/src/vm-vcpu/Cargo.toml +++ b/src/vm-vcpu/Cargo.toml @@ -9,10 +9,10 @@ edition = "2018" [dependencies] thiserror = "1.0.30" libc = "0.2.76" -kvm-bindings = { version = "0.5.0", features = ["fam-wrappers"] } -kvm-ioctls = "0.11.0" -vm-memory = "0.7.0" -vmm-sys-util = ">=0.8.0" +kvm-bindings = { version = "0.6.0", features = ["fam-wrappers"] } +kvm-ioctls = "0.13.0" +vm-memory = "0.13.1" +vmm-sys-util = "0.11.2" vm-device = "0.1.0" utils = { path = "../utils" } @@ -20,4 +20,4 @@ vm-vcpu-ref = { path = "../vm-vcpu-ref" } arch = { path = "../arch" } [dev-dependencies] -vm-memory = { version = "0.7.0", features = ["backend-mmap"] } +vm-memory = { version = "0.13.1", features = ["backend-mmap"] } diff --git a/src/vm-vcpu/src/vcpu/mod.rs b/src/vm-vcpu/src/vcpu/mod.rs index 26c15ebc..7341067b 100644 --- a/src/vm-vcpu/src/vcpu/mod.rs +++ b/src/vm-vcpu/src/vcpu/mod.rs @@ -412,7 +412,7 @@ impl KvmVcpu { fn set_state(&mut self, state: VcpuState) -> Result<()> { for reg in state.regs { self.vcpu_fd - .set_one_reg(reg.id, reg.addr) + .set_one_reg(reg.id, reg.addr as u128) .map_err(Error::VcpuSetReg)?; } @@ -458,7 +458,7 @@ impl KvmVcpu { data = (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT | PSR_F_BIT | PSR_MODE_EL1h).into(); reg_id = arm64_core_reg!(pstate); self.vcpu_fd - .set_one_reg(reg_id, data) + .set_one_reg(reg_id, data as u128) .map_err(Error::VcpuSetReg)?; // Other cpus are powered off initially @@ -470,7 +470,7 @@ impl KvmVcpu { // hack -- can't get this to do offsetof(regs[0]) but luckily it's at offset 0 reg_id = arm64_core_reg!(regs); self.vcpu_fd - .set_one_reg(reg_id, data) + .set_one_reg(reg_id, data as u128) .map_err(Error::VcpuSetReg)?; } @@ -682,7 +682,7 @@ impl KvmVcpu { let data = ip.0; let reg_id = arm64_core_reg!(pc); self.vcpu_fd - .set_one_reg(reg_id, data) + .set_one_reg(reg_id, data as u128) .map_err(Error::VcpuSetReg)?; } } diff --git a/src/vm-vcpu/src/vcpu/regs.rs b/src/vm-vcpu/src/vcpu/regs.rs index 226d70d2..ffbbd111 100644 --- a/src/vm-vcpu/src/vcpu/regs.rs +++ b/src/vm-vcpu/src/vcpu/regs.rs @@ -1,5 +1,6 @@ use kvm_bindings::*; use kvm_ioctls::VcpuFd; +use std::convert::TryInto; use super::Error; @@ -60,7 +61,10 @@ pub fn get_regs_and_mpidr(vcpu_fd: &VcpuFd) -> Result<(Vec, u64), E let mut regs = Vec::with_capacity(reg_id_list.as_slice().len()); for &id in reg_id_list.as_slice() { let addr = vcpu_fd.get_one_reg(id).map_err(Error::VcpuGetReg)?; - regs.push(kvm_one_reg { id, addr }); + regs.push(kvm_one_reg { + id, + addr: addr.try_into().unwrap(), + }); if id == MPIDR_EL1 { mpidr = Some(addr); @@ -72,5 +76,5 @@ pub fn get_regs_and_mpidr(vcpu_fd: &VcpuFd) -> Result<(Vec, u64), E } // unwrap() is safe because of the is_none() check above - Ok((regs, mpidr.unwrap())) + Ok((regs, mpidr.unwrap().try_into().unwrap())) } diff --git a/src/vmm/Cargo.toml b/src/vmm/Cargo.toml index a38c174e..415eadc7 100644 --- a/src/vmm/Cargo.toml +++ b/src/vmm/Cargo.toml @@ -5,15 +5,15 @@ authors = ["rust-vmm AWS maintainers "] edition = "2018" [dependencies] -event-manager = "0.2.1" -kvm-bindings = { version = "0.5.0", features = ["fam-wrappers"] } -kvm-ioctls = "0.11.0" +event-manager = "0.3.0" +kvm-bindings = { version = "0.6.0", features = ["fam-wrappers"] } +kvm-ioctls = "0.13.0" libc = "0.2.91" -linux-loader = { version = "0.4.0", features = ["bzimage", "elf"] } +linux-loader = { version = "0.10.0", features = ["bzimage", "elf"] } vm-allocator = "0.1.0" -vm-memory = { version = "0.7.0", features = ["backend-mmap"] } +vm-memory = { version = "0.13.1", features = ["backend-mmap"] } vm-superio = "0.5.0" -vmm-sys-util = "0.8.0" +vmm-sys-util = "0.11.2" vm-device = "0.1.0" devices = { path = "../devices" } diff --git a/src/vmm/src/config/mod.rs b/src/vmm/src/config/mod.rs index 275fca06..394df8c9 100644 --- a/src/vmm/src/config/mod.rs +++ b/src/vmm/src/config/mod.rs @@ -147,7 +147,7 @@ pub struct KernelConfig { impl KernelConfig { /// Return the default kernel command line used by the Vmm. pub fn default_cmdline() -> Cmdline { - let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY); + let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY).unwrap(); // It's ok to use `unwrap` because the initial capacity of `cmdline` should be // sufficient to accommodate the default kernel cmdline. @@ -181,7 +181,7 @@ impl TryFrom<&str> for KernelConfig { .map_err(ConversionError::new_kernel)? .unwrap_or_else(|| DEFAULT_KERNEL_CMDLINE.to_string()); - let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY); + let mut cmdline = Cmdline::new(KERNEL_CMDLINE_CAPACITY).unwrap(); cmdline .insert_str(cmdline_str) .map_err(|_| ConversionError::new_kernel("Kernel cmdline capacity error"))?; @@ -282,7 +282,7 @@ mod tests { // Check that additional commas in the kernel string do not cause a panic. let kernel_str = r#"path=/foo/bar,cmdline="foo=bar",kernel_load_addr=42,"#; - let mut foo_cmdline = Cmdline::new(128); + let mut foo_cmdline = Cmdline::new(128).unwrap(); foo_cmdline.insert_str("\"foo=bar\"").unwrap(); let expected_kernel_config = KernelConfig { diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 92c2cfe6..85a56deb 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -207,7 +207,7 @@ type Net = net::Net>; pub struct Vmm { vm: KvmVm, kernel_cfg: KernelConfig, - guest_memory: GuestMemoryMmap, + guest_memory: Arc, address_allocator: AddressAllocator, irq_allocator: IrqAllocator, // The `device_mgr` is an Arc so that it can be shared between @@ -319,7 +319,7 @@ impl TryFrom for Vmm { let mut vmm = Vmm { vm, - guest_memory, + guest_memory: Arc::new(guest_memory), address_allocator, irq_allocator, device_mgr, @@ -428,14 +428,14 @@ impl Vmm { // Load the kernel into guest memory. let kernel_load = match Elf::load( - &self.guest_memory, + self.guest_memory.as_ref(), None, &mut kernel_image, Some(GuestAddress(self.kernel_cfg.load_addr)), ) { Ok(result) => result, Err(loader::Error::Elf(elf::Error::InvalidElfMagicNumber)) => BzImage::load( - &self.guest_memory, + self.guest_memory.as_ref(), None, &mut kernel_image, Some(GuestAddress(self.kernel_cfg.load_addr)), @@ -458,16 +458,17 @@ impl Vmm { // Add the kernel command line to the boot parameters. bootparams.hdr.cmd_line_ptr = CMDLINE_START as u32; - bootparams.hdr.cmdline_size = self.kernel_cfg.cmdline.as_str().len() as u32 + 1; + bootparams.hdr.cmdline_size = + String::try_from(&self.kernel_cfg.cmdline).unwrap().len() as u32 + 1; // Load the kernel command line into guest memory. - let mut cmdline = Cmdline::new(4096); + let mut cmdline = Cmdline::new(4096).unwrap(); cmdline - .insert_str(self.kernel_cfg.cmdline.as_str()) + .insert_str(String::try_from(&self.kernel_cfg.cmdline).unwrap()) .map_err(Error::Cmdline)?; load_cmdline( - &self.guest_memory, + self.guest_memory.as_ref(), GuestAddress(CMDLINE_START), // Safe because we know the command line string doesn't contain any 0 bytes. &cmdline, @@ -488,7 +489,7 @@ impl Vmm { fn load_kernel(&mut self) -> Result { let mut kernel_image = File::open(&self.kernel_cfg.path).map_err(Error::IO)?; linux_loader::loader::pe::PE::load( - &self.guest_memory, + self.guest_memory.as_ref(), Some(GuestAddress(self.kernel_cfg.load_addr)), &mut kernel_image, None, @@ -596,7 +597,6 @@ impl Vmm { // can do it after figuring out how to better separate concerns and make the VMM agnostic of // the actual device types. fn add_block_device(&mut self, cfg: &BlockConfig) -> Result<()> { - let mem = Arc::new(self.guest_memory.clone()); let range = self.address_allocator.allocate( 0x1000, DEFAULT_ADDRESSS_ALIGNEMNT, @@ -612,7 +612,7 @@ impl Vmm { let mut guard = self.device_mgr.lock().unwrap(); let mut env = Env { - mem, + mem: self.guest_memory.clone(), vm_fd: self.vm.vm_fd(), event_mgr: &mut self.event_mgr, mmio_mgr: guard.deref_mut(), @@ -628,7 +628,7 @@ impl Vmm { }; // We can also hold this somewhere if we need to keep the handle for later. - let block = Block::new(&mut env, &args).map_err(Error::Block)?; + let block = Block::new(self.guest_memory.clone(), &mut env, &args).map_err(Error::Block)?; #[cfg(target_arch = "aarch64")] self.fdt_builder .add_virtio_device(range.start(), range.len(), irq); @@ -638,7 +638,6 @@ impl Vmm { } fn add_net_device(&mut self, cfg: &NetConfig) -> Result<()> { - let mem = Arc::new(self.guest_memory.clone()); let range = self.address_allocator.allocate( 0x1000, DEFAULT_ADDRESSS_ALIGNEMNT, @@ -654,7 +653,7 @@ impl Vmm { let mut guard = self.device_mgr.lock().unwrap(); let mut env = Env { - mem, + mem: self.guest_memory.clone(), vm_fd: self.vm.vm_fd(), event_mgr: &mut self.event_mgr, mmio_mgr: guard.deref_mut(), @@ -667,7 +666,7 @@ impl Vmm { }; // We can also hold this somewhere if we need to keep the handle for later. - let net = Net::new(&mut env, &args).map_err(Error::Net)?; + let net = Net::new(self.guest_memory.clone(), &mut env, &args).map_err(Error::Net)?; self.net_devices.push(net); #[cfg(target_arch = "aarch64")] self.fdt_builder @@ -726,12 +725,12 @@ impl Vmm { let cmdline = &self.kernel_cfg.cmdline; let fdt = self .fdt_builder - .with_cmdline(cmdline.as_str().to_string()) + .with_cmdline(String::try_from(&cmdline).unwrap()) .with_num_vcpus(self.num_vcpus.try_into().unwrap()) .with_mem_size(mem_size) .create_fdt() .map_err(Error::SetupFdt)?; - fdt.write_to_mem(&self.guest_memory, fdt_offset) + fdt.write_to_mem(self.guest_memory.as_ref(), fdt_offset) .map_err(Error::SetupFdt)?; Ok(()) } @@ -748,15 +747,13 @@ mod tests { use linux_loader::elf::Elf64_Ehdr; #[cfg(target_arch = "x86_64")] use linux_loader::loader::{self, bootparam::setup_header, elf::PvhBootCapability}; + use std::fs::write; use std::io::ErrorKind; #[cfg(target_arch = "x86_64")] use std::path::Path; use std::path::PathBuf; #[cfg(target_arch = "x86_64")] - use vm_memory::{ - bytes::{ByteValued, Bytes}, - Address, GuestAddress, GuestMemory, - }; + use vm_memory::{bytes::ByteValued, Address, GuestAddress, GuestMemory}; use vmm_sys_util::{tempdir::TempDir, tempfile::TempFile}; @@ -849,7 +846,7 @@ mod tests { let irq_allocator = IrqAllocator::new(SERIAL_IRQ, vm.max_irq()).unwrap(); Vmm { vm, - guest_memory, + guest_memory: Arc::new(guest_memory), address_allocator, irq_allocator, device_mgr, @@ -868,11 +865,13 @@ mod tests { // Return the address where an ELF file should be loaded, as specified in its header. #[cfg(target_arch = "x86_64")] fn elf_load_addr(elf_path: &Path) -> GuestAddress { + use vm_memory::ReadVolatile; + let mut elf_file = File::open(elf_path).unwrap(); let mut ehdr = Elf64_Ehdr::default(); - ehdr.as_bytes() - .read_from(0, &mut elf_file, std::mem::size_of::()) - .unwrap(); + + elf_file.read_volatile(&mut ehdr.as_bytes()).unwrap(); + GuestAddress(ehdr.e_entry) } @@ -956,13 +955,30 @@ mod tests { matches!(vmm.load_kernel().unwrap_err(), Error::IO(e) if e.kind() == ErrorKind::NotFound) ); - // Test case: kernel image is invalid. + // Test case: kernel image is invalid. This test has two flavors. In the first + // we try to load an empty file, in the second a file which has all zeros. let mut vmm_config = default_vmm_config(); let temp_file = TempFile::new().unwrap(); vmm_config.kernel_config.path = PathBuf::from(temp_file.as_path()); let mut vmm = mock_vmm(vmm_config); let err = vmm.load_kernel().unwrap_err(); + #[cfg(target_arch = "x86_64")] + assert!(matches!( + err, + Error::KernelLoad(loader::Error::Elf(loader::elf::Error::ReadElfHeader)) + )); + #[cfg(target_arch = "aarch64")] + assert!(matches!( + err, + Error::KernelLoad(loader::Error::Pe(loader::pe::Error::ReadImageHeader)) + )); + + let temp_file_path = PathBuf::from(temp_file.as_path()); + let buffer: Vec = vec![0_u8; 1024]; + write(temp_file_path, buffer).unwrap(); + let err = vmm.load_kernel().unwrap_err(); + #[cfg(target_arch = "x86_64")] assert!(matches!( err, @@ -970,6 +986,7 @@ mod tests { loader::bzimage::Error::InvalidBzImage )) )); + #[cfg(target_arch = "aarch64")] assert!(matches!( err, @@ -1011,15 +1028,18 @@ mod tests { let mut vmm_config = default_vmm_config(); vmm_config.kernel_config.path = default_elf_path(); let mut vmm = mock_vmm(vmm_config); - assert_eq!(vmm.kernel_cfg.cmdline.as_str(), DEFAULT_KERNEL_CMDLINE); + assert_eq!( + String::try_from(&vmm.kernel_cfg.cmdline).unwrap(), + DEFAULT_KERNEL_CMDLINE + ); vmm.add_serial_console().unwrap(); #[cfg(target_arch = "x86_64")] - assert!(vmm.kernel_cfg.cmdline.as_str().contains("console=ttyS0")); + assert!(String::try_from(&vmm.kernel_cfg.cmdline) + .unwrap() + .contains("console=ttyS0")); #[cfg(target_arch = "aarch64")] - assert!(vmm - .kernel_cfg - .cmdline - .as_str() + assert!(String::try_from(&vmm.kernel_cfg.cmdline) + .unwrap() .contains("earlycon=uart,mmio")); } @@ -1118,7 +1138,9 @@ mod tests { assert_eq!(vmm.block_devices.len(), 1); #[cfg(target_arch = "aarch64")] assert_eq!(vmm.fdt_builder.virtio_device_len(), 1); - assert!(vmm.kernel_cfg.cmdline.as_str().contains("virtio")); + assert!(String::try_from(&vmm.kernel_cfg.cmdline) + .unwrap() + .contains("virtio")); let invalid_block_config = BlockConfig { // Let's create the tempfile directly here so that it gets out of scope immediately @@ -1151,7 +1173,9 @@ mod tests { assert_eq!(vmm.net_devices.len(), 1); #[cfg(target_arch = "aarch64")] assert_eq!(vmm.fdt_builder.virtio_device_len(), 1); - assert!(vmm.kernel_cfg.cmdline.as_str().contains("virtio")); + assert!(String::try_from(&vmm.kernel_cfg.cmdline) + .unwrap() + .contains("virtio")); } } @@ -1172,14 +1196,16 @@ mod tests { let cmdline = &vmm.kernel_cfg.cmdline; let fdt = vmm .fdt_builder - .with_cmdline(cmdline.as_str().to_string()) + .with_cmdline(String::try_from(&cmdline).unwrap()) .with_num_vcpus(vmm.num_vcpus.try_into().unwrap()) .with_mem_size(mem_size) .with_serial_console(0x40000000, 0x1000) .with_rtc(0x40001000, 0x1000) .create_fdt() .unwrap(); - assert!(fdt.write_to_mem(&vmm.guest_memory, fdt_offset).is_err()); + assert!(fdt + .write_to_mem(vmm.guest_memory.as_ref(), fdt_offset) + .is_err()); } } #[test]