Merge with upstream 2022-07-12

93b6042f3 crosvm_control: Expose USB_CONTROL_MAX_PORTS
7ec522a1b devices: iommu: remove Translate trait
eb077b850 devices: iommu: remove custom permission enum
c206d1a3d devices: iommu: replace memory_mapper::Error with anyhow
92fabf70b infra: Add CQ list view and enable windows in CQ
4a764d2c8 main: add tests for argh compatibility code
a44e8a03e main: fix switch-or-option argh wrapper nits

de7835d9b0..93b6042f33

BUG=b:237334804
BUG=b:188858559
BUG=b:237620529

Change-Id: I03eadabb9a9382c00f643a3671c2137cf75110cb
This commit is contained in:
crosvm-luci-ci-builder 2022-07-12 22:01:05 -07:00
commit 3b2bbe7fce
13 changed files with 468 additions and 260 deletions

View file

@ -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<c_int> for Protection {

View file

@ -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<PathBuf> {
@ -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,

View file

@ -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

View file

@ -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<T> = result::Result<T, Error>;
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<Vec<MemRegion>> {
pub fn translate(&self, iova: u64, size: u64) -> Result<Vec<MemRegion>> {
let req = TranslateRequest {
endpoint_id: self.endpoint_id,
iova,
size,
};
self.request_tx.send(&req).map_err(Error::Tube)?;
let res: Option<Vec<MemRegion>> = 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<Vec<MemRegion>> =
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<Vec<MemRegion>> {
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`

View file

@ -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<T> = result::Result<T, Error>;
/// 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<Self> {
fn new(iova: u64, gpa: GuestAddress, size: u64, prot: Protection) -> Result<Self> {
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<AddMapResult>;
/// 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<bool>;
fn get_mask(&self) -> Result<u64>;
/// 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<Vec<MemRegion>>;
fn translate(&self, _iova: u64, _size: u64) -> Result<Vec<MemRegion>> {
bail!("not supported");
}
}
pub trait MemoryMapperTrait: MemoryMapper + Translate + AsRawDescriptors + Any {}
impl<T: MemoryMapper + Translate + AsRawDescriptors + Any> MemoryMapperTrait for T {}
pub trait MemoryMapperTrait: MemoryMapper + AsRawDescriptors + Any {}
impl<T: MemoryMapper + AsRawDescriptors + Any> 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<AddMapResult> {
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<bool> {
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<u64> {
@ -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<Vec<MemRegion>> {
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<MemRegion> = 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::<u64>(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) <RW>
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(),
},
],
);

View file

@ -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<T: Translate>(
pub fn is_valid_wrapper(
mem: &GuestMemory,
iommu: &Option<T>,
iommu: &Option<Arc<Mutex<IpcMemoryMapper>>>,
addr: GuestAddress,
size: u64,
) -> anyhow::Result<bool> {
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<T: Translate>(
/// 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<T: Translate>(
pub fn is_valid(
mem: &GuestMemory,
iommu: &T,
iommu: &IpcMemoryMapper,
iova: u64,
size: u64,
) -> anyhow::Result<bool> {
@ -83,7 +83,7 @@ pub fn read_obj_from_addr<T: DataInit>(
let mut buf = vec![0u8; std::mem::size_of::<T>()];
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<T: DataInit>(
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)

View file

@ -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<AddMapResult> {
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<bool> {
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<u64, MemoryMapperError> {
fn get_mask(&self) -> anyhow::Result<u64> {
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<Vec<MemRegion>, MemoryMapperError> {
Err(MemoryMapperError::Unimplemented)
}
}
impl AsRawDescriptors for VfioWrapper {
fn as_raw_descriptors(&self) -> Vec<RawDescriptor> {
vec![self.container.lock().as_raw_descriptor()]

View file

@ -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!(

View file

@ -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

View file

@ -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
}
}
}
}

View file

@ -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"

View file

@ -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",
)

View file

@ -405,13 +405,11 @@ fn pkg_version() -> std::result::Result<(), ()> {
Ok(())
}
fn crosvm_main() -> Result<CommandStatus> {
#[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<I: IntoIterator<Item = String>>(args_iter: I) -> Vec<String> {
let mut args: Vec<String> = 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<CommandStatus> {
];
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<CommandStatus> {
#[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::<Vec<_>>();
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"
]
);
}
}