Revert "Revert "linux: Convert all virtio devices to PCI""

This reverts commit c8986f14a8dd9f256d6faed55996d955b50ff923.

Re-land the virtio PCI conversion after the preceding fixes.

BUG=chromium:854766
TEST=Boot crosvm on nami and kevin

Change-Id: I3699e3ed1a45cecc99c51e352d0cf0c32bc4116f
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1265862
Reviewed-by: Dylan Reid <dgreid@chromium.org>

7 files changed, 45 insertions, 57 deletions

diff --git a/Cargo.lock b/Cargo.lock
index a7014e3..15720da 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",
@@ -459,6 +460,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 45201ae..5845982 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 8e00f2e..6aee2b1 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::os::unix::io::FromRawFd;
 use std::os::unix::net::UnixDatagram;
 use std::sync::{Arc, Mutex};
 
-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 resources::{AddressRanges, SystemAllocator};
 use sys_util::{EventFd, GuestAddress, GuestMemory};
 
@@ -192,7 +194,7 @@ pub struct AArch64;
 impl arch::LinuxArch for AArch64 {
     fn build_vm<F>(mut components: VmComponents, virtio_devs: F) -> Result<RunnableLinuxVm>
     where
-        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>,
+        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<(Box<PciDevice + 'static>, Minijail)>>,
     {
         let mut resources =
             Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf);
@@ -220,35 +222,19 @@ impl arch::LinuxArch for AArch64 {
 
         let mut mmio_bus = devices::Bus::new();
 
-        let (pci, pci_irqs) = arch::generate_pci_root(
-            components.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 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)));
+
         // ARM doesn't really use the io bus like x86, so just create an empty bus.
         let io_bus = devices::Bus::new();
 
-        // Create a list of mmio devices to be added.
-        let mmio_devs = virtio_devs(&mem, &exit_evt)?;
-
         let stdio_serial = 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(),
diff --git a/arch/src/lib.rs b/arch/src/lib.rs
index 3ee805e..c56b6f0 100644
--- a/arch/src/lib.rs
+++ b/arch/src/lib.rs
@@ -30,7 +30,6 @@ pub type Result<T> = result::Result<T, Box<std::error::Error>>;
 /// 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<PciDevice + 'static>, Minijail)>,
     pub memory_mb: u64,
     pub vcpu_count: u32,
     pub kernel_image: File,
@@ -68,7 +67,7 @@ pub trait LinuxArch {
     /// * `virtio_devs` - Function to generate a list of virtio devices.
     fn build_vm<F>(components: VmComponents, virtio_devs: F) -> Result<RunnableLinuxVm>
     where
-        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>;
+        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<(Box<PciDevice + 'static>, Minijail)>>;
 }
 
 /// Errors for device manager.
diff --git a/src/linux.rs b/src/linux.rs
index b45b956..4975ab9 100644
--- a/src/linux.rs
+++ b/src/linux.rs
@@ -25,7 +25,7 @@ use rand::distributions::{IndependentSample, Range};
 use rand::thread_rng;
 
 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<error::Error>),
 }
@@ -171,6 +172,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)
             }
@@ -244,7 +246,7 @@ fn create_virtio_devs(
     _exit_evt: &EventFd,
     wayland_device_socket: UnixDatagram,
     balloon_device_socket: UnixDatagram,
-) -> std::result::Result<Vec<VirtioDeviceStub>, Box<error::Error>> {
+) -> std::result::Result<Vec<(Box<PciDevice + 'static>, Minijail)>, Box<error::Error>> {
     static DEFAULT_PIVOT_ROOT: &'static str = "/var/empty";
 
     let mut devs = Vec::new();
@@ -614,7 +616,20 @@ fn create_virtio_devs(
         devs.push(VirtioDeviceStub { dev: p9_box, jail });
     }
 
-    Ok(devs)
+    let mut pci_devices: Vec<(Box<PciDevice + 'static>, 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<()> {
@@ -758,15 +773,12 @@ pub fn run_config(cfg: Config) -> Result<()> {
         info!("crosvm entering multiprocess mode");
     }
 
-    let pci_devices: Vec<(Box<PciDevice + 'static>, 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 d814814..687dec9 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 244aa45..ba32022 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 kernel_cmdline;
 extern crate kernel_loader;
 extern crate kvm;
@@ -67,10 +68,11 @@ use std::mem;
 use std::result;
 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 kvm::*;
 use resources::{AddressRanges, SystemAllocator};
 use sys_util::{EventFd, GuestAddress, GuestMemory};
@@ -261,7 +263,7 @@ fn arch_memory_regions(size: u64) -> Vec<(GuestAddress, u64)> {
 impl arch::LinuxArch for X8664arch {
     fn build_vm<F>(mut components: VmComponents, virtio_devs: F) -> Result<RunnableLinuxVm>
     where
-        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>,
+        F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<(Box<PciDevice + 'static>, Minijail)>>,
     {
         let mut resources =
             Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf);
@@ -289,35 +291,20 @@ impl arch::LinuxArch for X8664arch {
 
         let mut mmio_bus = devices::Bus::new();
 
-        let (pci, pci_irqs) = arch::generate_pci_root(
-            components.pci_devices,
-            &mut mmio_bus,
-            &mut resources,
-            &mut vm,
-        ).map_err(Error::CreatePciRoot)?;
+        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(&param).map_err(Error::Cmdline)?;
         }