From d635acbaf348c0863bc05b8f889b2fa5f8156aaa Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Mon, 24 Sep 2018 15:00:36 -0700 Subject: [PATCH] linux: Convert all virtio devices to PCI Change the main create_virtio_devs() function to create virtio devices using the PCI transport rather than MMIO. BUG=chromium:854766 TEST=Boot crosvm and verify that all virtio devices still work Change-Id: I9a6e60b21edea1e5ac2b3ae5c91793d45cf5063a Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/1241541 Reviewed-by: Dylan Reid --- Cargo.lock | 2 ++ aarch64/Cargo.toml | 1 + aarch64/src/lib.rs | 23 +++++++++-------------- arch/src/lib.rs | 3 +-- src/linux.rs | 35 ++++++++++++++++++++++++----------- x86_64/Cargo.toml | 1 + x86_64/src/lib.rs | 24 +++++++++--------------- 7 files changed, 47 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8cd8374987..898dc33ed7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16,6 +16,7 @@ dependencies = [ "byteorder 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "data_model 0.1.0", "devices 0.1.0", + "io_jail 0.1.0", "kernel_cmdline 0.1.0", "kvm 0.1.0", "kvm_sys 0.1.0", @@ -439,6 +440,7 @@ dependencies = [ "cc 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)", "data_model 0.1.0", "devices 0.1.0", + "io_jail 0.1.0", "kernel_cmdline 0.1.0", "kernel_loader 0.1.0", "kvm 0.1.0", diff --git a/aarch64/Cargo.toml b/aarch64/Cargo.toml index 45201aefb7..58459823bf 100644 --- a/aarch64/Cargo.toml +++ b/aarch64/Cargo.toml @@ -7,6 +7,7 @@ authors = ["The Chromium OS Authors"] arch = { path = "../arch" } data_model = { path = "../data_model" } devices = { path = "../devices" } +io_jail = { path = "../io_jail" } kernel_cmdline = { path = "../kernel_cmdline" } kvm_sys = { path = "../kvm_sys" } kvm = { path = "../kvm" } diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs index 8d71d4489a..cab05d5835 100644 --- a/aarch64/src/lib.rs +++ b/aarch64/src/lib.rs @@ -6,6 +6,7 @@ extern crate arch; extern crate byteorder; extern crate data_model; extern crate devices; +extern crate io_jail; extern crate kernel_cmdline; extern crate kvm; extern crate kvm_sys; @@ -22,8 +23,9 @@ use std::sync::{Arc, Mutex}; use std::os::unix::io::FromRawFd; use std::os::unix::net::UnixDatagram; -use arch::{RunnableLinuxVm, VirtioDeviceStub, VmComponents}; -use devices::{Bus, BusError, PciConfigMmio, PciInterruptPin}; +use arch::{RunnableLinuxVm, VmComponents}; +use devices::{Bus, BusError, PciConfigMmio, PciDevice, PciInterruptPin}; +use io_jail::Minijail; use sys_util::{EventFd, GuestAddress, GuestMemory}; use resources::{AddressRanges, SystemAllocator}; @@ -193,7 +195,7 @@ pub struct AArch64; impl arch::LinuxArch for AArch64 { fn build_vm(mut components: VmComponents, virtio_devs: F) -> Result where - F: FnOnce(&GuestMemory, &EventFd) -> Result> + F: FnOnce(&GuestMemory, &EventFd) -> Result, Minijail)>> { let mut resources = Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf); @@ -216,27 +218,20 @@ impl arch::LinuxArch for AArch64 { let mut mmio_bus = devices::Bus::new(); - let (pci, pci_irqs) = arch::generate_pci_root(components.pci_devices, + let exit_evt = EventFd::new().map_err(Error::CreateEventFd)?; + + let pci_devices = virtio_devs(&mem, &exit_evt)?; + let (pci, pci_irqs) = arch::generate_pci_root(pci_devices, &mut mmio_bus, &mut resources, &mut vm) .map_err(Error::CreatePciRoot)?; let pci_bus = Arc::new(Mutex::new(PciConfigMmio::new(pci))); - let exit_evt = EventFd::new().map_err(Error::CreateEventFd)?; let (io_bus, stdio_serial) = Self::setup_io_bus()?; - // Create a list of mmio devices to be added. - let mmio_devs = virtio_devs(&mem, &exit_evt)?; - Self::add_arch_devs(&mut vm, &mut mmio_bus)?; - for stub in mmio_devs { - arch::register_mmio(&mut mmio_bus, &mut vm, stub.dev, stub.jail, - &mut resources, &mut cmdline) - .map_err(Error::RegisterVsock)?; - } - mmio_bus.insert(pci_bus.clone(), AARCH64_PCI_CFG_BASE, AARCH64_PCI_CFG_SIZE, false) .map_err(Error::RegisterPci)?; diff --git a/arch/src/lib.rs b/arch/src/lib.rs index f83e404fbd..08e1ac555a 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -29,7 +29,6 @@ pub type Result = result::Result>; /// Holds the pieces needed to build a VM. Passed to `build_vm` in the `LinuxArch` trait below to /// create a `RunnableLinuxVm`. pub struct VmComponents { - pub pci_devices: Vec<(Box, Minijail)>, pub memory_mb: u64, pub vcpu_count: u32, pub kernel_image: File, @@ -67,7 +66,7 @@ pub trait LinuxArch { /// * `virtio_devs` - Function to generate a list of virtio devices. fn build_vm(components: VmComponents, virtio_devs: F) -> Result where - F: FnOnce(&GuestMemory, &EventFd) -> Result>; + F: FnOnce(&GuestMemory, &EventFd) -> Result, Minijail)>>; } /// Errors for device manager. diff --git a/src/linux.rs b/src/linux.rs index 1496998ca8..e608b22901 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -25,7 +25,7 @@ use rand::thread_rng; use rand::distributions::{IndependentSample, Range}; use byteorder::{ByteOrder, LittleEndian}; -use devices::{self, PciDevice}; +use devices::{self, PciDevice, VirtioPciDevice}; use io_jail::{self, Minijail}; use kvm::*; use net_util::Tap; @@ -92,6 +92,7 @@ pub enum Error { TimerFd(sys_util::Error), VhostNetDeviceNew(devices::virtio::vhost::Error), VhostVsockDeviceNew(devices::virtio::vhost::Error), + VirtioPciDev(sys_util::Error), WaylandDeviceNew(sys_util::Error), LoadKernel(Box), } @@ -167,6 +168,7 @@ impl fmt::Display for Error { &Error::VhostVsockDeviceNew(ref e) => { write!(f, "failed to set up virtual socket device: {:?}", e) } + &Error::VirtioPciDev(ref e) => write!(f, "failed to create virtio pci dev: {}", e), &Error::WaylandDeviceNew(ref e) => { write!(f, "failed to create wayland device: {:?}", e) } @@ -234,12 +236,13 @@ fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result Ok(j) } -fn create_virtio_devs(cfg: VirtIoDeviceInfo, - mem: &GuestMemory, - _exit_evt: &EventFd, - wayland_device_socket: UnixDatagram, - balloon_device_socket: UnixDatagram) - -> std::result::Result, Box> { +fn create_virtio_devs( + cfg: VirtIoDeviceInfo, + mem: &GuestMemory, + _exit_evt: &EventFd, + wayland_device_socket: UnixDatagram, + balloon_device_socket: UnixDatagram, +) -> std::result::Result, Minijail)>, Box> { static DEFAULT_PIVOT_ROOT: &'static str = "/var/empty"; let mut devs = Vec::new(); @@ -578,7 +581,20 @@ fn create_virtio_devs(cfg: VirtIoDeviceInfo, devs.push(VirtioDeviceStub {dev: p9_box, jail}); } - Ok(devs) + let mut pci_devices: Vec<(Box, Minijail)> = Vec::new(); + for stub in devs { + let pci_dev = + Box::new(VirtioPciDevice::new((*mem).clone(), stub.dev).map_err(Error::VirtioPciDev)?); + + // TODO(dverkamp): Make this work in non-multiprocess mode without creating an empty jail + let jail = match stub.jail { + Some(j) => j, + None => Minijail::new().unwrap(), + }; + pci_devices.push((pci_dev, jail)); + } + + Ok(pci_devices) } fn setup_vcpu_signal_handler() -> Result<()> { @@ -695,15 +711,12 @@ pub fn run_config(cfg: Config) -> Result<()> { info!("crosvm entering multiprocess mode"); } - let pci_devices: Vec<(Box, Minijail)> = Vec::new(); - // Masking signals is inherently dangerous, since this can persist across clones/execs. Do this // before any jailed devices have been spawned, so that we can catch any of them that fail very // quickly. let sigchld_fd = SignalFd::new(libc::SIGCHLD).map_err(Error::CreateSignalFd)?; let components = VmComponents { - pci_devices, memory_mb: (cfg.memory.unwrap_or(256) << 20) as u64, vcpu_count: cfg.vcpu_count.unwrap_or(1), kernel_image: File::open(cfg.kernel_path.as_path()) diff --git a/x86_64/Cargo.toml b/x86_64/Cargo.toml index d8148146c8..687dec939f 100644 --- a/x86_64/Cargo.toml +++ b/x86_64/Cargo.toml @@ -8,6 +8,7 @@ build = "build.rs" arch = { path = "../arch" } data_model = { path = "../data_model" } devices = { path = "../devices" } +io_jail = { path = "../io_jail" } kvm_sys = { path = "../kvm_sys" } kvm = { path = "../kvm" } sys_util = { path = "../sys_util" } diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index a372a95ca7..22eb2c538a 100644 --- a/x86_64/src/lib.rs +++ b/x86_64/src/lib.rs @@ -6,6 +6,7 @@ extern crate arch; extern crate byteorder; extern crate data_model; extern crate devices; +extern crate io_jail; extern crate kvm; extern crate kvm_sys; extern crate libc; @@ -67,10 +68,11 @@ use std::ffi::{CStr, CString}; use std::io::{self, stdout}; use std::sync::{Arc, Mutex}; -use arch::{RunnableLinuxVm, VirtioDeviceStub, VmComponents}; +use arch::{RunnableLinuxVm, VmComponents}; use bootparam::boot_params; use bootparam::E820_RAM; -use devices::{PciConfigIo, PciInterruptPin}; +use devices::{PciConfigIo, PciDevice, PciInterruptPin}; +use io_jail::Minijail; use sys_util::{EventFd, GuestAddress, GuestMemory}; use resources::{AddressRanges, SystemAllocator}; use kvm::*; @@ -252,7 +254,7 @@ fn arch_memory_regions(size: u64) -> Vec<(GuestAddress, u64)> { impl arch::LinuxArch for X8664arch { fn build_vm(mut components: VmComponents, virtio_devs: F) -> Result where - F: FnOnce(&GuestMemory, &EventFd) -> Result> + F: FnOnce(&GuestMemory, &EventFd) -> Result, Minijail)>> { let mut resources = Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf); @@ -275,29 +277,21 @@ impl arch::LinuxArch for X8664arch { let mut mmio_bus = devices::Bus::new(); - let (pci, pci_irqs) = arch::generate_pci_root(components.pci_devices, + let exit_evt = EventFd::new().map_err(Error::CreateEventFd)?; + + let pci_devices = virtio_devs(&mem, &exit_evt)?; + let (pci, pci_irqs) = arch::generate_pci_root(pci_devices, &mut mmio_bus, &mut resources, &mut vm) .map_err(Error::CreatePciRoot)?; let pci_bus = Arc::new(Mutex::new(PciConfigIo::new(pci))); - let exit_evt = EventFd::new().map_err(Error::CreateEventFd)?; let (io_bus, stdio_serial) = Self::setup_io_bus( &mut vm, exit_evt.try_clone().map_err(Error::CloneEventFd)?, Some(pci_bus.clone()))?; - - // Create a list of mmio devices to be added. - let mmio_devs = virtio_devs(&mem, &exit_evt)?; - - for stub in mmio_devs { - arch::register_mmio(&mut mmio_bus, &mut vm, stub.dev, stub.jail, - &mut resources, &mut cmdline) - .map_err(Error::RegisterVsock)?; - } - for param in components.extra_kernel_params { cmdline.insert_str(¶m).map_err(Error::Cmdline)?; }