Revert "linux: Convert all virtio devices to PCI"

This reverts commit d635acbaf348c0863bc05b8f889b2fa5f8156aaa.

This commit seems to be responsible for introducing hung tasks in tests,
so let's revert it for now to get the tests green and debug it offline.

BUG=chromium:891806
TEST=None

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

7 files changed, 42 insertions, 47 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 898dc33..8cd8374 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -16,7 +16,6 @@ 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",
@@ -440,7 +439,6 @@ 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 5845982..45201ae 100644
--- a/aarch64/Cargo.toml
+++ b/aarch64/Cargo.toml
@@ -7,7 +7,6 @@ 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 cab05d5..8d71d44 100644
--- a/aarch64/src/lib.rs
+++ b/aarch64/src/lib.rs
@@ -6,7 +6,6 @@ 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;
@@ -23,9 +22,8 @@ use std::sync::{Arc, Mutex};
 use std::os::unix::io::FromRawFd;
 use std::os::unix::net::UnixDatagram;
 
-use arch::{RunnableLinuxVm, VmComponents};
-use devices::{Bus, BusError, PciConfigMmio, PciDevice, PciInterruptPin};
-use io_jail::Minijail;
+use arch::{RunnableLinuxVm, VirtioDeviceStub, VmComponents};
+use devices::{Bus, BusError, PciConfigMmio, PciInterruptPin};
 use sys_util::{EventFd, GuestAddress, GuestMemory};
 use resources::{AddressRanges, SystemAllocator};
 
@@ -195,7 +193,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<(Box<PciDevice + 'static>, Minijail)>>
+            F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>
     {
         let mut resources = Self::get_resource_allocator(components.memory_mb,
                                                          components.wayland_dmabuf);
@@ -218,20 +216,27 @@ impl arch::LinuxArch for AArch64 {
 
         let mut mmio_bus = devices::Bus::new();
 
-        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,
+        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 (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 08e1ac5..f83e404 100644
--- a/arch/src/lib.rs
+++ b/arch/src/lib.rs
@@ -29,6 +29,7 @@ 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,
@@ -66,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<(Box<PciDevice + 'static>, Minijail)>>;
+            F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>;
 }
 
 /// Errors for device manager.
diff --git a/src/linux.rs b/src/linux.rs
index e608b22..1496998 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, VirtioPciDevice};
+use devices::{self, PciDevice};
 use io_jail::{self, Minijail};
 use kvm::*;
 use net_util::Tap;
@@ -92,7 +92,6 @@ 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>),
 }
@@ -168,7 +167,6 @@ 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)
             }
@@ -236,13 +234,12 @@ fn create_base_minijail(root: &Path, seccomp_policy: &Path) -> Result<Minijail>
     Ok(j)
 }
 
-fn create_virtio_devs(
-    cfg: VirtIoDeviceInfo,
-    mem: &GuestMemory,
-    _exit_evt: &EventFd,
-    wayland_device_socket: UnixDatagram,
-    balloon_device_socket: UnixDatagram,
-) -> std::result::Result<Vec<(Box<PciDevice + 'static>, Minijail)>, Box<error::Error>> {
+fn create_virtio_devs(cfg: VirtIoDeviceInfo,
+                      mem: &GuestMemory,
+                      _exit_evt: &EventFd,
+                      wayland_device_socket: UnixDatagram,
+                      balloon_device_socket: UnixDatagram)
+                      -> std::result::Result<Vec<VirtioDeviceStub>, Box<error::Error>> {
     static DEFAULT_PIVOT_ROOT: &'static str = "/var/empty";
 
     let mut devs = Vec::new();
@@ -581,20 +578,7 @@ fn create_virtio_devs(
         devs.push(VirtioDeviceStub {dev: p9_box, jail});
     }
 
-    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)
+    Ok(devs)
 }
 
 fn setup_vcpu_signal_handler() -> Result<()> {
@@ -711,12 +695,15 @@ 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 687dec9..d814814 100644
--- a/x86_64/Cargo.toml
+++ b/x86_64/Cargo.toml
@@ -8,7 +8,6 @@ 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 22eb2c5..a372a95 100644
--- a/x86_64/src/lib.rs
+++ b/x86_64/src/lib.rs
@@ -6,7 +6,6 @@ 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;
@@ -68,11 +67,10 @@ use std::ffi::{CStr, CString};
 use std::io::{self, stdout};
 use std::sync::{Arc, Mutex};
 
-use arch::{RunnableLinuxVm, VmComponents};
+use arch::{RunnableLinuxVm, VirtioDeviceStub, VmComponents};
 use bootparam::boot_params;
 use bootparam::E820_RAM;
-use devices::{PciConfigIo, PciDevice, PciInterruptPin};
-use io_jail::Minijail;
+use devices::{PciConfigIo, PciInterruptPin};
 use sys_util::{EventFd, GuestAddress, GuestMemory};
 use resources::{AddressRanges, SystemAllocator};
 use kvm::*;
@@ -254,7 +252,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<(Box<PciDevice + 'static>, Minijail)>>
+            F: FnOnce(&GuestMemory, &EventFd) -> Result<Vec<VirtioDeviceStub>>
     {
         let mut resources = Self::get_resource_allocator(components.memory_mb,
                                                          components.wayland_dmabuf);
@@ -277,21 +275,29 @@ impl arch::LinuxArch for X8664arch {
 
         let mut mmio_bus = devices::Bus::new();
 
-        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,
+        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(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)?;
         }