From 8eceba31c0d2842d8d7bfaa84253121709b1ee81 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Thu, 18 Oct 2018 16:45:13 -0700 Subject: [PATCH] devices: make PCI work in --disable-sandbox mode Make the Minijail part of the PCI device tuple optional so that an empty jail is not created for --disable-sandbox. BUG=None TEST=Boot crosvm in both --multiprocess and --disable-sandbox modes Change-Id: Ibb3f2dbf33ca19910ee7448ea823b2772e09ecc5 Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/1290289 Reviewed-by: Dylan Reid --- aarch64/src/lib.rs | 3 ++- arch/src/lib.rs | 18 ++++++++++++------ devices/src/pci/pci_root.rs | 5 ++--- devices/src/proxy.rs | 8 ++++---- src/linux.rs | 12 +++--------- x86_64/src/lib.rs | 3 ++- 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs index 7c341cf60b..88b150f23f 100644 --- a/aarch64/src/lib.rs +++ b/aarch64/src/lib.rs @@ -194,7 +194,8 @@ pub struct AArch64; impl arch::LinuxArch for AArch64 { fn build_vm(mut components: VmComponents, virtio_devs: F) -> Result where - F: FnOnce(&GuestMemory, &EventFd) -> Result, Minijail)>>, + F: FnOnce(&GuestMemory, &EventFd) + -> Result, Option)>>, { let mut resources = Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf); diff --git a/arch/src/lib.rs b/arch/src/lib.rs index c56b6f051a..565227a415 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -18,7 +18,8 @@ use std::sync::{Arc, Mutex}; use devices::virtio::VirtioDevice; use devices::{ - Bus, BusError, PciDevice, PciDeviceError, PciInterruptPin, PciRoot, ProxyDevice, Serial, + Bus, BusDevice, BusError, PciDevice, PciDeviceError, PciInterruptPin, PciRoot, ProxyDevice, + Serial, }; use io_jail::Minijail; use kvm::{Datamatch, IoeventAddress, Kvm, Vcpu, Vm}; @@ -67,7 +68,8 @@ 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, Minijail)>>; + F: FnOnce(&GuestMemory, &EventFd) + -> Result, Option)>>; } /// Errors for device manager. @@ -135,7 +137,7 @@ impl fmt::Display for DeviceRegistrationError { /// Creates a root PCI device for use by this Vm. pub fn generate_pci_root( - devices: Vec<(Box, Minijail)>, + devices: Vec<(Box, Option)>, mmio_bus: &mut Bus, resources: &mut SystemAllocator, vm: &mut Vm, @@ -172,9 +174,13 @@ pub fn generate_pci_root( .map_err(DeviceRegistrationError::RegisterIoevent)?; keep_fds.push(event.as_raw_fd()); } - let proxy = ProxyDevice::new(device, &jail, keep_fds) - .map_err(DeviceRegistrationError::ProxyDeviceCreation)?; - let arced_dev = Arc::new(Mutex::new(proxy)); + let arced_dev: Arc> = if let Some(jail) = jail { + let proxy = ProxyDevice::new(device, &jail, keep_fds) + .map_err(DeviceRegistrationError::ProxyDeviceCreation)?; + Arc::new(Mutex::new(proxy)) + } else { + Arc::new(Mutex::new(device)) + }; root.add_device(arced_dev.clone()); for range in &ranges { mmio_bus diff --git a/devices/src/pci/pci_root.rs b/devices/src/pci/pci_root.rs index d6ed5cec42..a9530743f1 100644 --- a/devices/src/pci/pci_root.rs +++ b/devices/src/pci/pci_root.rs @@ -8,7 +8,6 @@ use std::sync::{Arc, Mutex}; use byteorder::{ByteOrder, LittleEndian}; use BusDevice; -use ProxyDevice; use pci::pci_configuration::{PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType}; use pci::pci_device::PciDevice; @@ -40,7 +39,7 @@ pub struct PciRoot { /// Bus configuration for the root device. root_configuration: PciRootConfiguration, /// Devices attached to this bridge. - devices: Vec>>, + devices: Vec>>, } impl PciRoot { @@ -64,7 +63,7 @@ impl PciRoot { } /// Add a `device` to this root PCI bus. - pub fn add_device(&mut self, device: Arc>) { + pub fn add_device(&mut self, device: Arc>) { self.devices.push(device); } diff --git a/devices/src/proxy.rs b/devices/src/proxy.rs index 478625b61c..5b79d31c89 100644 --- a/devices/src/proxy.rs +++ b/devices/src/proxy.rs @@ -198,8 +198,10 @@ impl ProxyDevice { .map(|_| ()) .map_err(Error::Io) } +} - pub fn config_register_write(&mut self, reg_idx: usize, offset: u64, data: &[u8]) { +impl BusDevice for ProxyDevice { + fn config_register_write(&mut self, reg_idx: usize, offset: u64, data: &[u8]) { let res = self .send_config_cmd(Command::WriteConfig, reg_idx as u32, offset, data) .and_then(|_| self.wait()); @@ -208,7 +210,7 @@ impl ProxyDevice { } } - pub fn config_register_read(&self, reg_idx: usize) -> u32 { + fn config_register_read(&self, reg_idx: usize) -> u32 { let mut data = [0u8; 4]; let res = self .send_config_cmd(Command::ReadConfig, reg_idx as u32, 0, &[]) @@ -218,9 +220,7 @@ impl ProxyDevice { } LittleEndian::read_u32(&data) } -} -impl BusDevice for ProxyDevice { fn read(&mut self, offset: u64, data: &mut [u8]) { let res = self .send_cmd(Command::Read, offset, data.len() as u32, &[]) diff --git a/src/linux.rs b/src/linux.rs index 903b6b1868..ae3130dc16 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -249,7 +249,7 @@ fn create_virtio_devs( _exit_evt: &EventFd, wayland_device_socket: UnixDatagram, balloon_device_socket: UnixDatagram, -) -> std::result::Result, Minijail)>, Box> { +) -> std::result::Result, Option)>, Box> { static DEFAULT_PIVOT_ROOT: &'static str = "/var/empty"; let mut devs = Vec::new(); @@ -620,17 +620,11 @@ fn create_virtio_devs( devs.push(VirtioDeviceStub { dev: p9_box, jail }); } - let mut pci_devices: Vec<(Box, Minijail)> = Vec::new(); + let mut pci_devices: Vec<(Box, Option)> = 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)); + pci_devices.push((pci_dev, stub.jail)); } Ok(pci_devices) diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index ba32022fd5..700ba75cde 100644 --- a/x86_64/src/lib.rs +++ b/x86_64/src/lib.rs @@ -263,7 +263,8 @@ 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, Minijail)>>, + F: FnOnce(&GuestMemory, &EventFd) + -> Result, Option)>>, { let mut resources = Self::get_resource_allocator(components.memory_mb, components.wayland_dmabuf);