diff --git a/base/src/sys/unix/mmap.rs b/base/src/sys/unix/mmap.rs index 146e67af98..75dad95010 100644 --- a/base/src/sys/unix/mmap.rs +++ b/base/src/sys/unix/mmap.rs @@ -74,6 +74,12 @@ impl Protection { Protection(libc::PROT_READ) } + /// Returns Protection allowing write access. + #[inline(always)] + pub fn write() -> Protection { + Protection(libc::PROT_WRITE) + } + /// Set read events. #[inline(always)] pub fn set_read(self) -> Protection { @@ -85,6 +91,13 @@ impl Protection { pub fn set_write(self) -> Protection { Protection(self.0 | libc::PROT_WRITE) } + + /// Returns true if all access allowed by |other| is also allowed by |self|. + #[inline(always)] + pub fn allows(&self, other: &Protection) -> bool { + (self.0 & libc::PROT_READ) >= (other.0 & libc::PROT_READ) + && (self.0 & libc::PROT_WRITE) >= (other.0 & libc::PROT_WRITE) + } } impl From for Protection { diff --git a/crosvm_control/src/lib.rs b/crosvm_control/src/lib.rs index 10cfabd906..6a7c9ab5c2 100644 --- a/crosvm_control/src/lib.rs +++ b/crosvm_control/src/lib.rs @@ -22,7 +22,7 @@ use libc::{c_char, ssize_t}; use vm_control::{ client::*, BalloonControlCommand, BalloonStats, DiskControlCommand, UsbControlAttachedDevice, - UsbControlResult, VmRequest, VmResponse, + UsbControlResult, VmRequest, VmResponse, USB_CONTROL_MAX_PORTS, }; fn validate_socket_path(socket_path: *const c_char) -> Option { @@ -132,6 +132,12 @@ impl From<&UsbControlAttachedDevice> for UsbDeviceEntry { } } +/// Simply returns the maximum possible number of USB devices +#[no_mangle] +pub extern "C" fn crosvm_client_max_usb_devices() -> usize { + USB_CONTROL_MAX_PORTS +} + /// Returns all USB devices passed through the crosvm instance whose control socket is listening on `socket_path`. /// /// The function returns the amount of entries written. @@ -142,8 +148,8 @@ impl From<&UsbControlAttachedDevice> for UsbDeviceEntry { /// devices will be written to /// * `entries_length` - Amount of entries in the array specified by `entries` /// -/// Crosvm supports passing through up to 255 devices, so pasing an array with 255 entries will -/// guarantee to return all entries. +/// Use the value returned by crosvm_client_max_usb_devices() to determine the size of the input +/// array to this function. #[no_mangle] pub extern "C" fn crosvm_client_usb_list( socket_path: *const c_char, diff --git a/devices/src/virtio/iommu.rs b/devices/src/virtio/iommu.rs index f53c4bdc19..da79f64e69 100644 --- a/devices/src/virtio/iommu.rs +++ b/devices/src/virtio/iommu.rs @@ -23,8 +23,8 @@ use anyhow::Context; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] use base::warn; use base::{ - error, pagesize, AsRawDescriptor, Error as SysError, Event, RawDescriptor, Result as SysResult, - Tube, TubeError, + error, pagesize, AsRawDescriptor, Error as SysError, Event, Protection, RawDescriptor, + Result as SysResult, Tube, TubeError, }; use cros_async::{AsyncError, AsyncTube, EventAsync, Executor}; use data_model::{DataInit, Le64}; @@ -37,7 +37,7 @@ use vm_memory::{GuestAddress, GuestMemory, GuestMemoryError}; #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] use crate::pci::PciAddress; use crate::virtio::iommu::ipc_memory_mapper::*; -use crate::virtio::iommu::memory_mapper::{Error as MemoryMapperError, *}; +use crate::virtio::iommu::memory_mapper::*; use crate::virtio::iommu::protocol::*; use crate::virtio::{ async_utils, copy_config, DescriptorChain, DescriptorError, DeviceType, Interrupt, Queue, @@ -121,7 +121,7 @@ pub enum IommuError { #[error("failed to write to guest address: {0}")] GuestMemoryWrite(io::Error), #[error("memory mapper failed: {0}")] - MemoryMapper(MemoryMapperError), + MemoryMapper(anyhow::Error), #[error("Failed to read descriptor asynchronously: {0}")] ReadAsyncDesc(AsyncError), #[error("failed to read from virtio queue Event: {0}")] @@ -353,24 +353,21 @@ impl State { iova: req.virt_start.into(), gpa: GuestAddress(req.phys_start.into()), size, - perm: match write_en { - true => Permission::RW, - false => Permission::Read, + prot: match write_en { + true => Protection::read_write(), + false => Protection::read(), }, }); match vfio_map_result { - Ok(()) => (), - Err(e) => match e { - MemoryMapperError::IovaRegionOverlap => { - // If a mapping already exists in the requested range, - // the device SHOULD reject the request and set status - // to VIRTIO_IOMMU_S_INVAL. - tail.status = VIRTIO_IOMMU_S_INVAL; - return Ok(0); - } - _ => return Err(IommuError::MemoryMapper(e)), - }, + Ok(AddMapResult::Ok) => (), + Ok(AddMapResult::OverlapFailure) => { + // If a mapping already exists in the requested range, + // the device SHOULD reject the request and set status + // to VIRTIO_IOMMU_S_INVAL. + tail.status = VIRTIO_IOMMU_S_INVAL; + } + Err(e) => return Err(IommuError::MemoryMapper(e)), } } @@ -387,11 +384,18 @@ impl State { let domain: u32 = req.domain.into(); if let Some(mapper) = self.domain_map.get(&domain) { let size = u64::from(req.virt_end) - u64::from(req.virt_start) + 1; - mapper + let res = mapper .1 .lock() .remove_map(u64::from(req.virt_start), size) .map_err(IommuError::MemoryMapper)?; + if !res { + // If a mapping affected by the range is not covered in its entirety by the + // range (the UNMAP request would split the mapping), then the device SHOULD + // set the request `status` to VIRTIO_IOMMU_S_RANGE, and SHOULD NOT remove + // any mapping. + tail.status = VIRTIO_IOMMU_S_RANGE; + } } else { // If domain does not exist, the device SHOULD set the // request status to VIRTIO_IOMMU_S_NOENT diff --git a/devices/src/virtio/iommu/ipc_memory_mapper.rs b/devices/src/virtio/iommu/ipc_memory_mapper.rs index 843a378074..c62c38c6ca 100644 --- a/devices/src/virtio/iommu/ipc_memory_mapper.rs +++ b/devices/src/virtio/iommu/ipc_memory_mapper.rs @@ -4,15 +4,11 @@ //! Provide utility to communicate with an iommu in another process -use std::ops::Deref; -use std::result; - +use anyhow::{Context, Result}; use base::{AsRawDescriptor, AsRawDescriptors, RawDescriptor, Tube}; use serde::{Deserialize, Serialize}; -use crate::virtio::memory_mapper::{Error, MemRegion, Translate}; - -pub type Result = result::Result; +use crate::virtio::memory_mapper::MemRegion; #[derive(Serialize, Deserialize)] pub struct TranslateRequest { @@ -44,18 +40,19 @@ impl IpcMemoryMapper { endpoint_id, } } -} -impl Translate for IpcMemoryMapper { - fn translate(&self, iova: u64, size: u64) -> Result> { + pub fn translate(&self, iova: u64, size: u64) -> Result> { let req = TranslateRequest { endpoint_id: self.endpoint_id, iova, size, }; - self.request_tx.send(&req).map_err(Error::Tube)?; - let res: Option> = self.response_rx.recv().map_err(Error::Tube)?; - res.ok_or(Error::InvalidIOVA(iova, size)) + self.request_tx + .send(&req) + .context("error sending request")?; + let res: Option> = + self.response_rx.recv().context("error receiving reply")?; + res.context(format!("invalid iova {:x} {:x}", iova, size)) } } @@ -68,12 +65,6 @@ impl AsRawDescriptors for IpcMemoryMapper { } } -impl Translate for std::sync::MutexGuard<'_, IpcMemoryMapper> { - fn translate(&self, iova: u64, size: u64) -> Result> { - self.deref().translate(iova, size) - } -} - pub struct CreateIpcMapperRet { pub mapper: IpcMemoryMapper, pub response_tx: Tube, @@ -99,7 +90,7 @@ pub fn create_ipc_mapper(endpoint_id: u32, request_tx: Tube) -> CreateIpcMapperR #[cfg(test)] mod tests { use super::*; - use crate::virtio::memory_mapper::Permission; + use base::Protection; use std::thread; use vm_memory::GuestAddress; @@ -118,7 +109,7 @@ mod tests { .zip(&vec![MemRegion { gpa: GuestAddress(0x777), len: 1, - perm: Permission::RW, + prot: Protection::read_write(), },]) .all(|(a, b)| a == b)); }); @@ -135,7 +126,7 @@ mod tests { .send(&Some(vec![MemRegion { gpa: GuestAddress(0x777), len: 1, - perm: Permission::RW, + prot: Protection::read_write(), }])) .unwrap(); // This join needs to be here because on Windows, if `response_tx` diff --git a/devices/src/virtio/iommu/memory_mapper.rs b/devices/src/virtio/iommu/memory_mapper.rs index 6c5e4b2fa8..5bab31b990 100644 --- a/devices/src/virtio/iommu/memory_mapper.rs +++ b/devices/src/virtio/iommu/memory_mapper.rs @@ -8,87 +8,42 @@ use std::any::Any; use std::collections::BTreeMap; -use std::result; use std::sync::atomic::{AtomicU32, Ordering}; -use base::{error, AsRawDescriptors, RawDescriptor, TubeError}; -use remain::sorted; +use anyhow::{anyhow, bail, Context, Result}; +use base::{AsRawDescriptors, Protection, RawDescriptor}; use serde::{Deserialize, Serialize}; -use thiserror::Error; -use vm_memory::{GuestAddress, GuestMemoryError}; - -#[cfg(unix)] -use crate::vfio::VfioError; - -#[repr(u8)] -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub enum Permission { - Read = 1, - Write = 2, - RW = 3, -} +use vm_memory::GuestAddress; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct MemRegion { pub gpa: GuestAddress, pub len: u64, - pub perm: Permission, + pub prot: Protection, } -#[sorted] -#[derive(Error, Debug)] -pub enum Error { - #[error("address not aligned")] - AddrNotAligned, - #[error("address start ({0} is greater than or equal to end {1}")] - AddrStartGeEnd(u64, u64), - #[error("failed getting host address: {0}")] - GetHostAddress(GuestMemoryError), - #[error("integer overflow")] - IntegerOverflow, - #[error("invalid iova: {0}, length: {1}")] - InvalidIOVA(u64, u64), - #[error("iommu dma error")] - IommuDma, - #[error("iova partial overlap")] - IovaPartialOverlap, - #[error("iova region overlap")] - IovaRegionOverlap, - #[error("size is zero")] - SizeIsZero, - #[error("tube error: {0}")] - Tube(TubeError), - #[error("unimplemented")] - Unimplemented, - #[cfg(unix)] - #[error{"vfio error: {0}"}] - Vfio(VfioError), -} - -pub type Result = result::Result; - /// Manages the mapping from a guest IO virtual address space to the guest physical address space #[derive(Debug)] pub struct MappingInfo { pub iova: u64, pub gpa: GuestAddress, pub size: u64, - pub perm: Permission, + pub prot: Protection, } impl MappingInfo { #[allow(dead_code)] - fn new(iova: u64, gpa: GuestAddress, size: u64, perm: Permission) -> Result { + fn new(iova: u64, gpa: GuestAddress, size: u64, prot: Protection) -> Result { if size == 0 { - return Err(Error::SizeIsZero); + bail!("can't create 0 sized region"); } - iova.checked_add(size).ok_or(Error::IntegerOverflow)?; - gpa.checked_add(size).ok_or(Error::IntegerOverflow)?; + iova.checked_add(size).context("iova overflow")?; + gpa.checked_add(size).context("gpa overflow")?; Ok(Self { iova, gpa, size, - perm, + prot, }) } } @@ -100,10 +55,24 @@ pub struct BasicMemoryMapper { id: u32, } +#[derive(PartialEq, Debug)] +pub enum AddMapResult { + Ok, + OverlapFailure, +} + /// A generic interface for vfio and other iommu backends pub trait MemoryMapper: Send { - fn add_map(&mut self, new_map: MappingInfo) -> Result<()>; - fn remove_map(&mut self, iova_start: u64, size: u64) -> Result<()>; + /// Creates a new mapping. If the mapping overlaps with an existing + /// mapping, return Ok(false). + fn add_map(&mut self, new_map: MappingInfo) -> Result; + + /// Removes all mappings within the specified range. + /// + /// If a mapped region partially overlaps what is being unmapped, implementations + /// SHOULD return Ok(false) without removing any mappings. + fn remove_map(&mut self, iova_start: u64, size: u64) -> Result; + fn get_mask(&self) -> Result; /// Whether or not endpoints can be safely detached from this mapper. @@ -115,15 +84,15 @@ pub trait MemoryMapper: Send { /// Gets an identifier for the MemoryMapper instance. Must be unique among /// instances of the same trait implementation. fn id(&self) -> u32; -} -pub trait Translate { /// Multiple MemRegions should be returned when the gpa is discontiguous or perms are different. - fn translate(&self, iova: u64, size: u64) -> Result>; + fn translate(&self, _iova: u64, _size: u64) -> Result> { + bail!("not supported"); + } } -pub trait MemoryMapperTrait: MemoryMapper + Translate + AsRawDescriptors + Any {} -impl MemoryMapperTrait for T {} +pub trait MemoryMapperTrait: MemoryMapper + AsRawDescriptors + Any {} +impl MemoryMapperTrait for T {} impl BasicMemoryMapper { pub fn new(mask: u64) -> BasicMemoryMapper { @@ -142,42 +111,36 @@ impl BasicMemoryMapper { } impl MemoryMapper for BasicMemoryMapper { - fn add_map(&mut self, new_map: MappingInfo) -> Result<()> { + fn add_map(&mut self, new_map: MappingInfo) -> Result { if new_map.size == 0 { - return Err(Error::SizeIsZero); + bail!("can't map 0 sized region"); } let new_iova_end = new_map .iova .checked_add(new_map.size) - .ok_or(Error::IntegerOverflow)?; + .context("iova overflow")?; new_map .gpa .checked_add(new_map.size) - .ok_or(Error::IntegerOverflow)?; + .context("gpa overflow")?; let mut iter = self.maps.range(..new_iova_end); if let Some((_, map)) = iter.next_back() { if map.iova + map.size > new_map.iova { - return Err(Error::IovaRegionOverlap); + return Ok(AddMapResult::OverlapFailure); } } self.maps.insert(new_map.iova, new_map); - Ok(()) + Ok(AddMapResult::Ok) } - fn remove_map(&mut self, iova_start: u64, size: u64) -> Result<()> { - // From the virtio-iommu spec - // - // If a mapping affected by the range is not covered in its entirety by the - // range (the UNMAP request would split the mapping), then the device SHOULD - // set the request \field{status} to VIRTIO_IOMMU_S_RANGE, and SHOULD NOT - // remove any mapping. - // - // Therefore, this func checks for partial overlap first before removing the maps. + fn remove_map(&mut self, iova_start: u64, size: u64) -> Result { if size == 0 { - return Err(Error::SizeIsZero); + bail!("can't unmap 0 sized region"); } - let iova_end = iova_start.checked_add(size).ok_or(Error::IntegerOverflow)?; + let iova_end = iova_start.checked_add(size).context("iova overflow")?; + // So that we invalid requests can be rejected w/o modifying things, check + // for partial overlap before removing the maps. let mut to_be_removed = Vec::new(); for (key, map) in self.maps.range(..iova_end).rev() { let map_iova_end = map.iova + map.size; @@ -188,13 +151,13 @@ impl MemoryMapper for BasicMemoryMapper { if iova_start <= map.iova && map_iova_end <= iova_end { to_be_removed.push(*key); } else { - return Err(Error::IovaPartialOverlap); + return Ok(false); } } for key in to_be_removed { self.maps.remove(&key).expect("map should contain key"); } - Ok(()) + Ok(true) } fn get_mask(&self) -> Result { @@ -212,15 +175,13 @@ impl MemoryMapper for BasicMemoryMapper { fn id(&self) -> u32 { self.id } -} -impl Translate for BasicMemoryMapper { /// Regions of contiguous iovas and gpas, and identical permission are merged fn translate(&self, iova: u64, size: u64) -> Result> { if size == 0 { - return Err(Error::SizeIsZero); + bail!("can't translate 0 sized region"); } - let iova_end = iova.checked_add(size).ok_or(Error::IntegerOverflow)?; + let iova_end = iova.checked_add(size).context("iova overflow")?; let mut iter = self.maps.range(..iova_end); let mut last_iova = iova_end; let mut regions: Vec = Vec::new(); @@ -233,7 +194,7 @@ impl Translate for BasicMemoryMapper { // This is the last region to be inserted / first to be returned when iova >= map.iova let region_len = last_iova - std::cmp::max::(map.iova, iova); if let Some(last) = regions.last_mut() { - if map.gpa.unchecked_add(map.size) == last.gpa && map.perm == last.perm { + if map.gpa.unchecked_add(map.size) == last.gpa && map.prot == last.prot { last.gpa = map.gpa; last.len += region_len; new_region = false; @@ -250,7 +211,7 @@ impl Translate for BasicMemoryMapper { regions.push(MemRegion { gpa: map.gpa, len: region_len, - perm: map.perm, + prot: map.prot, }); } if iova >= map.iova { @@ -259,13 +220,13 @@ impl Translate for BasicMemoryMapper { regions[0].gpa = map .gpa .checked_add(iova - map.iova) - .ok_or(Error::IntegerOverflow)?; + .context("gpa overflow")?; return Ok(regions); } last_iova = map.iova; } - Err(Error::InvalidIOVA(iova, size)) + Err(anyhow!("invalid iova {:x} {:x}", iova, size)) } } @@ -283,33 +244,55 @@ mod tests { #[test] fn test_mapping_info() { // Overflow - MappingInfo::new(u64::MAX - 1, GuestAddress(1), 2, Permission::Read).unwrap_err(); - MappingInfo::new(1, GuestAddress(u64::MAX - 1), 2, Permission::Read).unwrap_err(); - MappingInfo::new(u64::MAX, GuestAddress(1), 2, Permission::Read).unwrap_err(); - MappingInfo::new(1, GuestAddress(u64::MAX), 2, Permission::Read).unwrap_err(); - MappingInfo::new(5, GuestAddress(5), u64::MAX, Permission::Read).unwrap_err(); + MappingInfo::new(u64::MAX - 1, GuestAddress(1), 2, Protection::read()).unwrap_err(); + MappingInfo::new(1, GuestAddress(u64::MAX - 1), 2, Protection::read()).unwrap_err(); + MappingInfo::new(u64::MAX, GuestAddress(1), 2, Protection::read()).unwrap_err(); + MappingInfo::new(1, GuestAddress(u64::MAX), 2, Protection::read()).unwrap_err(); + MappingInfo::new(5, GuestAddress(5), u64::MAX, Protection::read()).unwrap_err(); // size = 0 - MappingInfo::new(1, GuestAddress(5), 0, Permission::Read).unwrap_err(); + MappingInfo::new(1, GuestAddress(5), 0, Protection::read()).unwrap_err(); } #[test] fn test_map_overlap() { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(10, GuestAddress(1000), 10, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(10, GuestAddress(1000), 10, Protection::read_write()).unwrap(), + ) .unwrap(); - mapper - .add_map(MappingInfo::new(14, GuestAddress(1000), 1, Permission::RW).unwrap()) - .unwrap_err(); - mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 12, Permission::RW).unwrap()) - .unwrap_err(); - mapper - .add_map(MappingInfo::new(16, GuestAddress(1000), 6, Permission::RW).unwrap()) - .unwrap_err(); - mapper - .add_map(MappingInfo::new(5, GuestAddress(1000), 20, Permission::RW).unwrap()) - .unwrap_err(); + assert_eq!( + mapper + .add_map( + MappingInfo::new(14, GuestAddress(1000), 1, Protection::read_write()).unwrap() + ) + .unwrap(), + AddMapResult::OverlapFailure + ); + assert_eq!( + mapper + .add_map( + MappingInfo::new(0, GuestAddress(1000), 12, Protection::read_write()).unwrap() + ) + .unwrap(), + AddMapResult::OverlapFailure + ); + assert_eq!( + mapper + .add_map( + MappingInfo::new(16, GuestAddress(1000), 6, Protection::read_write()).unwrap() + ) + .unwrap(), + AddMapResult::OverlapFailure + ); + assert_eq!( + mapper + .add_map( + MappingInfo::new(5, GuestAddress(1000), 20, Protection::read_write()).unwrap() + ) + .unwrap(), + AddMapResult::OverlapFailure + ); } #[test] @@ -324,14 +307,16 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 9, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(0, GuestAddress(1000), 9, Protection::read_write()).unwrap(), + ) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); assert_eq!( @@ -339,7 +324,7 @@ mod tests { MemRegion { gpa: GuestAddress(1008), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); mapper.translate(9, 1).unwrap_err(); @@ -350,17 +335,21 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(0, GuestAddress(1000), 4, Protection::read_write()).unwrap(), + ) .unwrap(); mapper - .add_map(MappingInfo::new(5, GuestAddress(50), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(5, GuestAddress(50), 4, Protection::read_write()).unwrap(), + ) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); assert_eq!( @@ -368,7 +357,7 @@ mod tests { MemRegion { gpa: GuestAddress(51), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); mapper.remove_map(0, 9).unwrap(); @@ -379,15 +368,17 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 9, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(0, GuestAddress(1000), 9, Protection::read_write()).unwrap(), + ) .unwrap(); - mapper.remove_map(0, 4).unwrap_err(); + assert!(!mapper.remove_map(0, 4).unwrap()); assert_eq!( mapper.translate(5, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1005), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); } @@ -395,17 +386,21 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(0, GuestAddress(1000), 4, Protection::read_write()).unwrap(), + ) .unwrap(); mapper - .add_map(MappingInfo::new(5, GuestAddress(50), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(5, GuestAddress(50), 4, Protection::read_write()).unwrap(), + ) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); assert_eq!( @@ -413,7 +408,7 @@ mod tests { MemRegion { gpa: GuestAddress(50), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); mapper.remove_map(0, 4).unwrap(); @@ -424,7 +419,7 @@ mod tests { MemRegion { gpa: GuestAddress(50), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); } @@ -432,14 +427,16 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(0, GuestAddress(1000), 4, Protection::read_write()).unwrap(), + ) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); mapper.translate(9, 1).unwrap_err(); @@ -451,17 +448,19 @@ mod tests { { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(0, GuestAddress(1000), 4, Permission::Read).unwrap()) + .add_map(MappingInfo::new(0, GuestAddress(1000), 4, Protection::read()).unwrap()) .unwrap(); mapper - .add_map(MappingInfo::new(10, GuestAddress(50), 4, Permission::RW).unwrap()) + .add_map( + MappingInfo::new(10, GuestAddress(50), 4, Protection::read_write()).unwrap(), + ) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -469,7 +468,7 @@ mod tests { MemRegion { gpa: GuestAddress(1003), len: 1, - perm: Permission::Read + prot: Protection::read() } ); mapper.translate(4, 1).unwrap_err(); @@ -478,7 +477,7 @@ mod tests { MemRegion { gpa: GuestAddress(50), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); assert_eq!( @@ -486,7 +485,7 @@ mod tests { MemRegion { gpa: GuestAddress(53), len: 1, - perm: Permission::RW + prot: Protection::read_write() } ); mapper.remove_map(0, 14).unwrap(); @@ -501,28 +500,28 @@ mod tests { fn test_remove_map() { let mut mapper = BasicMemoryMapper::new(u64::MAX); mapper - .add_map(MappingInfo::new(1, GuestAddress(1000), 4, Permission::Read).unwrap()) + .add_map(MappingInfo::new(1, GuestAddress(1000), 4, Protection::read()).unwrap()) .unwrap(); mapper - .add_map(MappingInfo::new(5, GuestAddress(50), 4, Permission::RW).unwrap()) + .add_map(MappingInfo::new(5, GuestAddress(50), 4, Protection::read_write()).unwrap()) .unwrap(); mapper - .add_map(MappingInfo::new(9, GuestAddress(50), 4, Permission::RW).unwrap()) + .add_map(MappingInfo::new(9, GuestAddress(50), 4, Protection::read_write()).unwrap()) .unwrap(); assert_eq!(mapper.len(), 3); - mapper.remove_map(0, 6).unwrap_err(); + assert!(!mapper.remove_map(0, 6).unwrap()); assert_eq!(mapper.len(), 3); - mapper.remove_map(1, 5).unwrap_err(); + assert!(!mapper.remove_map(1, 5).unwrap()); assert_eq!(mapper.len(), 3); - mapper.remove_map(1, 9).unwrap_err(); + assert!(!mapper.remove_map(1, 9).unwrap()); assert_eq!(mapper.len(), 3); - mapper.remove_map(6, 4).unwrap_err(); + assert!(!mapper.remove_map(6, 4).unwrap()); assert_eq!(mapper.len(), 3); - mapper.remove_map(6, 14).unwrap_err(); + assert!(!mapper.remove_map(6, 14).unwrap()); assert_eq!(mapper.len(), 3); mapper.remove_map(5, 4).unwrap(); assert_eq!(mapper.len(), 2); - mapper.remove_map(1, 9).unwrap_err(); + assert!(!mapper.remove_map(1, 9).unwrap()); assert_eq!(mapper.len(), 2); mapper.remove_map(0, 15).unwrap(); assert_eq!(mapper.len(), 0); @@ -540,7 +539,7 @@ mod tests { let mut mapper = BasicMemoryMapper::new(u64::MAX); // [1, 5) -> [1000, 1004) mapper - .add_map(MappingInfo::new(1, GuestAddress(1000), 4, Permission::Read).unwrap()) + .add_map(MappingInfo::new(1, GuestAddress(1000), 4, Protection::read()).unwrap()) .unwrap(); mapper.translate(1, 0).unwrap_err(); assert_eq!( @@ -548,7 +547,7 @@ mod tests { MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -556,7 +555,7 @@ mod tests { MemRegion { gpa: GuestAddress(1000), len: 2, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -564,7 +563,7 @@ mod tests { MemRegion { gpa: GuestAddress(1000), len: 3, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -572,7 +571,7 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 1, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -580,13 +579,13 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 2, - perm: Permission::Read + prot: Protection::read() } ); mapper.translate(1, 5).unwrap_err(); // [1, 9) -> [1000, 1008) mapper - .add_map(MappingInfo::new(5, GuestAddress(1004), 4, Permission::Read).unwrap()) + .add_map(MappingInfo::new(5, GuestAddress(1004), 4, Protection::read()).unwrap()) .unwrap(); // Spanned across 2 maps assert_eq!( @@ -594,7 +593,7 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 5, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -602,7 +601,7 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 6, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -610,20 +609,20 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 7, - perm: Permission::Read + prot: Protection::read() } ); mapper.translate(2, 8).unwrap_err(); mapper.translate(3, 10).unwrap_err(); // [1, 9) -> [1000, 1008), [11, 17) -> [1010, 1016) mapper - .add_map(MappingInfo::new(11, GuestAddress(1010), 6, Permission::Read).unwrap()) + .add_map(MappingInfo::new(11, GuestAddress(1010), 6, Protection::read()).unwrap()) .unwrap(); // Discontiguous iova mapper.translate(3, 10).unwrap_err(); // [1, 17) -> [1000, 1016) mapper - .add_map(MappingInfo::new(9, GuestAddress(1008), 2, Permission::Read).unwrap()) + .add_map(MappingInfo::new(9, GuestAddress(1008), 2, Protection::read()).unwrap()) .unwrap(); // Spanned across 4 maps assert_eq!( @@ -631,7 +630,7 @@ mod tests { MemRegion { gpa: GuestAddress(1002), len: 10, - perm: Permission::Read + prot: Protection::read() } ); assert_eq!( @@ -639,21 +638,21 @@ mod tests { MemRegion { gpa: GuestAddress(1000), len: 16, - perm: Permission::Read + prot: Protection::read() } ); mapper.translate(1, 17).unwrap_err(); mapper.translate(0, 16).unwrap_err(); // [0, 1) -> [5, 6), [1, 17) -> [1000, 1016) mapper - .add_map(MappingInfo::new(0, GuestAddress(5), 1, Permission::Read).unwrap()) + .add_map(MappingInfo::new(0, GuestAddress(5), 1, Protection::read()).unwrap()) .unwrap(); assert_eq!( mapper.translate(0, 1).unwrap()[0], MemRegion { gpa: GuestAddress(5), len: 1, - perm: Permission::Read + prot: Protection::read() } ); // Discontiguous gpa @@ -663,12 +662,12 @@ mod tests { MemRegion { gpa: GuestAddress(5), len: 1, - perm: Permission::Read, + prot: Protection::read(), }, MemRegion { gpa: GuestAddress(1000), len: 1, - perm: Permission::Read, + prot: Protection::read(), }, ], ); @@ -678,18 +677,18 @@ mod tests { MemRegion { gpa: GuestAddress(5), len: 1, - perm: Permission::Read, + prot: Protection::read(), }, MemRegion { gpa: GuestAddress(1000), len: 15, - perm: Permission::Read, + prot: Protection::read(), }, ], ); // [0, 1) -> [5, 6), [1, 17) -> [1000, 1016), [17, 18) -> [1016, 1017) mapper - .add_map(MappingInfo::new(17, GuestAddress(1016), 2, Permission::RW).unwrap()) + .add_map(MappingInfo::new(17, GuestAddress(1016), 2, Protection::read_write()).unwrap()) .unwrap(); // Contiguous iova and gpa, but different perm assert_vec_eq( @@ -698,12 +697,12 @@ mod tests { MemRegion { gpa: GuestAddress(1000), len: 16, - perm: Permission::Read, + prot: Protection::read(), }, MemRegion { gpa: GuestAddress(1016), len: 1, - perm: Permission::RW, + prot: Protection::read_write(), }, ], ); @@ -714,12 +713,12 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 15, - perm: Permission::Read, + prot: Protection::read(), }, MemRegion { gpa: GuestAddress(1016), len: 1, - perm: Permission::RW, + prot: Protection::read_write(), }, ], ); @@ -729,12 +728,12 @@ mod tests { MemRegion { gpa: GuestAddress(1001), len: 15, - perm: Permission::Read, + prot: Protection::read(), }, MemRegion { gpa: GuestAddress(1016), len: 2, - perm: Permission::RW, + prot: Protection::read_write(), }, ], ); diff --git a/devices/src/virtio/iommu/memory_util.rs b/devices/src/virtio/iommu/memory_util.rs index c0df37d8ca..743e7faee1 100644 --- a/devices/src/virtio/iommu/memory_util.rs +++ b/devices/src/virtio/iommu/memory_util.rs @@ -9,21 +9,21 @@ use std::sync::Arc; use sync::Mutex; use anyhow::{bail, Context}; +use base::Protection; use data_model::DataInit; use vm_memory::{GuestAddress, GuestMemory}; use crate::virtio::iommu::IpcMemoryMapper; -use crate::virtio::memory_mapper::{Permission, Translate}; /// A wrapper that works with gpa, or iova and an iommu. -pub fn is_valid_wrapper( +pub fn is_valid_wrapper( mem: &GuestMemory, - iommu: &Option, + iommu: &Option>>, addr: GuestAddress, size: u64, ) -> anyhow::Result { if let Some(iommu) = iommu { - is_valid(mem, iommu, addr.offset(), size) + is_valid(mem, &iommu.lock(), addr.offset(), size) } else { Ok(addr .checked_add(size as u64) @@ -33,9 +33,9 @@ pub fn is_valid_wrapper( /// Translates `iova` into gpa regions (or 1 gpa region when it is contiguous), and check if the /// gpa regions are all valid in `mem`. -pub fn is_valid( +pub fn is_valid( mem: &GuestMemory, - iommu: &T, + iommu: &IpcMemoryMapper, iova: u64, size: u64, ) -> anyhow::Result { @@ -83,7 +83,7 @@ pub fn read_obj_from_addr( let mut buf = vec![0u8; std::mem::size_of::()]; let mut addr: usize = 0; for r in regions { - if (r.perm as u8 & Permission::Read as u8) == 0 { + if !r.prot.allows(&Protection::read()) { bail!("gpa is not readable"); } mem.read_at_addr(&mut buf[addr..(addr + r.len as usize)], r.gpa) @@ -126,7 +126,7 @@ pub fn write_obj_at_addr( let buf = val.as_slice(); let mut addr: usize = 0; for r in regions { - if (r.perm as u8 & Permission::Read as u8) == 0 { + if !r.prot.allows(&Protection::write()) { bail!("gpa is not writable"); } mem.write_at_addr(&buf[addr..(addr + (r.len as usize))], r.gpa) diff --git a/devices/src/virtio/iommu/sys/unix/vfio_wrapper.rs b/devices/src/virtio/iommu/sys/unix/vfio_wrapper.rs index b56d3b1eea..84616fa5c3 100644 --- a/devices/src/virtio/iommu/sys/unix/vfio_wrapper.rs +++ b/devices/src/virtio/iommu/sys/unix/vfio_wrapper.rs @@ -6,14 +6,13 @@ use std::sync::Arc; -use base::{AsRawDescriptor, AsRawDescriptors, RawDescriptor}; +use anyhow::Context; +use base::{AsRawDescriptor, AsRawDescriptors, Protection, RawDescriptor}; use sync::Mutex; use vm_memory::{GuestAddress, GuestMemory}; use crate::vfio::VfioError; -use crate::virtio::iommu::memory_mapper::{ - Error as MemoryMapperError, MappingInfo, MemRegion, MemoryMapper, Permission, Translate, -}; +use crate::virtio::iommu::memory_mapper::{AddMapResult, MappingInfo, MemoryMapper}; use crate::VfioContainer; pub struct VfioWrapper { @@ -49,48 +48,45 @@ impl VfioWrapper { } impl MemoryMapper for VfioWrapper { - fn add_map(&mut self, mut map: MappingInfo) -> Result<(), MemoryMapperError> { + fn add_map(&mut self, mut map: MappingInfo) -> anyhow::Result { map.gpa = GuestAddress( self.mem .get_host_address_range(map.gpa, map.size as usize) - .map_err(MemoryMapperError::GetHostAddress)? as u64, + .context("failed to find host address")? as u64, ); // Safe because both guest and host address are guaranteed by // get_host_address_range() to be valid. - unsafe { + let res = unsafe { self.container.lock().vfio_dma_map( map.iova, map.size, map.gpa.offset(), - (map.perm as u8 & Permission::Write as u8) != 0, + map.prot.allows(&Protection::write()), ) - } - .map_err(|e| { - match base::Error::last() { - err if err.errno() == libc::EEXIST => { - // A mapping already exists in the requested range, - return MemoryMapperError::IovaRegionOverlap; - } - _ => MemoryMapperError::Vfio(e), + }; + if let Err(VfioError::IommuDmaMap(err)) = res { + if err.errno() == libc::EEXIST { + // A mapping already exists in the requested range, + return Ok(AddMapResult::OverlapFailure); } - }) + } + res.context("vfio mapping error").map(|_| AddMapResult::Ok) } - fn remove_map(&mut self, iova_start: u64, size: u64) -> Result<(), MemoryMapperError> { - iova_start - .checked_add(size) - .ok_or(MemoryMapperError::IntegerOverflow)?; + fn remove_map(&mut self, iova_start: u64, size: u64) -> anyhow::Result { + iova_start.checked_add(size).context("iova overflow")?; self.container .lock() .vfio_dma_unmap(iova_start, size) - .map_err(MemoryMapperError::Vfio) + .context("vfio unmapping error") + .map(|_| true) } - fn get_mask(&self) -> Result { + fn get_mask(&self) -> anyhow::Result { self.container .lock() .vfio_get_iommu_page_size_mask() - .map_err(MemoryMapperError::Vfio) + .context("vfio get mask error") } fn supports_detach(&self) -> bool { @@ -114,12 +110,6 @@ impl MemoryMapper for VfioWrapper { } } -impl Translate for VfioWrapper { - fn translate(&self, _iova: u64, _size: u64) -> Result, MemoryMapperError> { - Err(MemoryMapperError::Unimplemented) - } -} - impl AsRawDescriptors for VfioWrapper { fn as_raw_descriptors(&self) -> Vec { vec![self.container.lock().as_raw_descriptor()] diff --git a/devices/src/virtio/queue.rs b/devices/src/virtio/queue.rs index 8d79f953e1..dccfc98b1d 100644 --- a/devices/src/virtio/queue.rs +++ b/devices/src/virtio/queue.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use sync::Mutex; use anyhow::{bail, Context}; -use base::{error, warn}; +use base::{error, warn, Protection}; use cros_async::{AsyncError, EventAsync}; use data_model::{DataInit, Le16, Le32, Le64}; use virtio_sys::virtio_ring::VIRTIO_RING_F_EVENT_IDX; @@ -18,7 +18,7 @@ use vm_memory::{GuestAddress, GuestMemory}; use super::{SignalableInterrupt, VIRTIO_MSI_NO_VECTOR}; use crate::virtio::ipc_memory_mapper::IpcMemoryMapper; -use crate::virtio::memory_mapper::{MemRegion, Permission, Translate}; +use crate::virtio::memory_mapper::MemRegion; use crate::virtio::memory_util::{ is_valid_wrapper, read_obj_from_addr_wrapper, write_obj_at_addr_wrapper, }; @@ -153,7 +153,7 @@ impl DescriptorChain { Ok(vec![MemRegion { gpa: self.addr, len: self.len.try_into().expect("u32 doesn't fit in usize"), - perm: Permission::RW, + prot: Protection::read_write(), }]) } } @@ -393,13 +393,12 @@ impl Queue { return; } - let iommu = self.iommu.as_ref().map(|i| i.lock()); for (addr, size, name) in [ (desc_table, desc_table_size, "descriptor table"), (avail_ring, avail_ring_size, "available ring"), (used_ring, used_ring_size, "used ring"), ] { - match is_valid_wrapper(mem, &iommu, addr, size as u64) { + match is_valid_wrapper(mem, &self.iommu, addr, size as u64) { Ok(valid) => { if !valid { error!( diff --git a/infra/config/generated/commit-queue.cfg b/infra/config/generated/commit-queue.cfg index 3e643b6e73..2aeff03fa3 100644 --- a/infra/config/generated/commit-queue.cfg +++ b/infra/config/generated/commit-queue.cfg @@ -34,6 +34,9 @@ config_groups { builders { name: "crosvm/try/linux_x86_64_direct" } + builders { + name: "crosvm/try/windows" + } retry_config { single_quota: 1 global_quota: 2 diff --git a/infra/config/generated/cr-buildbucket.cfg b/infra/config/generated/cr-buildbucket.cfg index e340dad229..ce441ed00d 100644 --- a/infra/config/generated/cr-buildbucket.cfg +++ b/infra/config/generated/cr-buildbucket.cfg @@ -408,5 +408,26 @@ buckets { value: 100 } } + builders { + name: "windows" + swarming_host: "chromium-swarm.appspot.com" + dimensions: "cpu:x86-64" + dimensions: "os:Windows" + dimensions: "pool:luci.crosvm.try" + exe { + cipd_package: "infra/recipe_bundles/chromium.googlesource.com/crosvm/crosvm" + cipd_version: "refs/heads/main" + cmd: "luciexe" + } + properties: + '{' + ' "recipe": "build_windows"' + '}' + service_account: "crosvm-luci-try-builder@crosvm-infra.iam.gserviceaccount.com" + experiments { + key: "luci.recipes.use_python3" + value: 100 + } + } } } diff --git a/infra/config/generated/luci-milo.cfg b/infra/config/generated/luci-milo.cfg index 01c834472d..ed9c672190 100644 --- a/infra/config/generated/luci-milo.cfg +++ b/infra/config/generated/luci-milo.cfg @@ -39,6 +39,29 @@ consoles { category: "linux" } } +consoles { + id: "Presubmit" + name: "Presubmit" + builders { + name: "buildbucket/luci.crosvm.try/linux_x86_64" + } + builders { + name: "buildbucket/luci.crosvm.try/linux_x86_64_direct" + } + builders { + name: "buildbucket/luci.crosvm.try/linux_aarch64" + } + builders { + name: "buildbucket/luci.crosvm.try/linux_armhf" + } + builders { + name: "buildbucket/luci.crosvm.try/windows" + } + builders { + name: "buildbucket/luci.crosvm.try/health_check" + } + builder_view_only: true +} consoles { id: "Infra" name: "Infra" diff --git a/infra/config/main.star b/infra/config/main.star index 1f145122dc..2984dc195b 100755 --- a/infra/config/main.star +++ b/infra/config/main.star @@ -132,6 +132,11 @@ luci.console_view( repo = "https://chromium.googlesource.com/crosvm/crosvm", ) +# View showing all presubmit builders +luci.list_view( + name = "Presubmit", +) + # View showing all infra builders luci.list_view( name = "Infra", @@ -217,6 +222,10 @@ def verify_builder( properties = props, **args ) + luci.list_view_entry( + list_view = "Presubmit", + builder = "try/%s" % name, + ) # Attach try builder to Change Verifier luci.cq_tryjob_verifier( @@ -334,7 +343,6 @@ verify_builder( executable = luci.recipe( name = "build_windows", ), - presubmit = False, category = "windows", ) diff --git a/src/main.rs b/src/main.rs index 3cd2d4f2b7..4023a8f322 100644 --- a/src/main.rs +++ b/src/main.rs @@ -405,13 +405,11 @@ fn pkg_version() -> std::result::Result<(), ()> { Ok(()) } -fn crosvm_main() -> Result { - #[cfg(not(feature = "crash-report"))] - sys::set_panic_hook(); - +// Perform transformations on `args_iter` to produce arguments suitable for parsing by `argh`. +fn prepare_argh_args>(args_iter: I) -> Vec { let mut args: Vec = Vec::default(); // http://b/235882579 - for arg in std::env::args() { + for arg in args_iter { match arg.as_str() { "--host_ip" => { eprintln!("`--host_ip` option is deprecated!"); @@ -447,12 +445,20 @@ fn crosvm_main() -> Result { ]; for arg in switch_or_option { if let Some(i) = args.iter().position(|a| a == arg) { - if i == args.len() - 1 || args[i + 1].starts_with("--") { + if i >= args.len() - 2 || args[i + 1].starts_with("-") { args.insert(i + 1, "".to_string()); } } } + args +} + +fn crosvm_main() -> Result { + #[cfg(not(feature = "crash-report"))] + sys::set_panic_hook(); + + let args = prepare_argh_args(std::env::args()); let args = args.iter().map(|s| s.as_str()).collect::>(); let args = match crosvm::cmdline::CrosvmCmdlineArgs::from_args(&args[..1], &args[1..]) { Ok(args) => args, @@ -580,3 +586,148 @@ fn main() { }; std::process::exit(exit_code); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn args_host_ip() { + assert_eq!( + prepare_argh_args( + ["crosvm", "run", "--host_ip", "1.2.3.4", "vm_kernel"].map(|x| x.to_string()) + ), + ["crosvm", "run", "--host-ip", "1.2.3.4", "vm_kernel"] + ); + } + + #[test] + fn args_balloon_bias_mib() { + assert_eq!( + prepare_argh_args( + ["crosvm", "run", "--balloon_bias_mib", "1234", "vm_kernel"].map(|x| x.to_string()) + ), + ["crosvm", "run", "--balloon-bias-mib", "1234", "vm_kernel"] + ); + } + + #[test] + fn args_h() { + assert_eq!( + prepare_argh_args(["crosvm", "run", "-h"].map(|x| x.to_string())), + ["crosvm", "run", "--help"] + ); + } + + #[test] + fn args_battery_switch() { + assert_eq!( + prepare_argh_args( + ["crosvm", "run", "--battery", "--other-args", "vm_kernel"].map(|x| x.to_string()) + ), + [ + "crosvm", + "run", + "--battery", + "", + "--other-args", + "vm_kernel" + ] + ); + } + + #[test] + fn args_battery_switch_last_arg() { + assert_eq!( + prepare_argh_args(["crosvm", "run", "--battery", "vm_kernel"].map(|x| x.to_string())), + ["crosvm", "run", "--battery", "", "vm_kernel"] + ); + } + + #[test] + fn args_battery_switch_short_arg() { + assert_eq!( + prepare_argh_args( + [ + "crosvm", + "run", + "--battery", + "-p", + "init=/bin/bash", + "vm_kernel" + ] + .map(|x| x.to_string()) + ), + [ + "crosvm", + "run", + "--battery", + "", + "-p", + "init=/bin/bash", + "vm_kernel" + ] + ); + } + + #[test] + fn args_battery_option() { + assert_eq!( + prepare_argh_args( + [ + "crosvm", + "run", + "--battery", + "type=goldfish", + "-p", + "init=/bin/bash", + "vm_kernel" + ] + .map(|x| x.to_string()) + ), + [ + "crosvm", + "run", + "--battery", + "type=goldfish", + "-p", + "init=/bin/bash", + "vm_kernel" + ] + ); + } + + #[test] + fn args_switch_multi() { + assert_eq!( + prepare_argh_args( + [ + "crosvm", + "run", + "--gpu", + "test", + "--video-decoder", + "--video-encoder", + "--battery", + "--other-switch", + "vm_kernel" + ] + .map(|x| x.to_string()) + ), + [ + "crosvm", + "run", + "--gpu", + "test", + "--video-decoder", + "", + "--video-encoder", + "", + "--battery", + "", + "--other-switch", + "vm_kernel" + ] + ); + } +}