vmm_vhost: VhostUserMemoryRegionInfo cleanup

The mmap_handle and mmap_offset fields are not optional, so the comments
are adjusted to match reality.

Only one test function needed the Copy, Clone, and Default impls, which
can be avoided by tweaking the test (while also making it more useful,
since it tests exactly one failure instead of multiple).

BUG=b:221882601

Change-Id: Icc45577435aa27b18384cd10cb273c5152ad55ac
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5063681
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
This commit is contained in:
Daniel Verkamp 2023-11-27 15:59:33 -08:00 committed by crosvm LUCI
parent f28c1d3bc8
commit d34b36c693
2 changed files with 23 additions and 23 deletions

View file

@ -10,7 +10,6 @@
//! Common traits and structs for vhost-user backend drivers.
use base::RawDescriptor;
use base::INVALID_DESCRIPTOR;
/// Maximum number of memory regions supported.
pub const VHOST_MAX_MEMORY_REGIONS: usize = 255;
@ -52,7 +51,6 @@ impl VringConfigData {
}
/// Memory region configuration data.
#[derive(Clone, Copy)]
pub struct VhostUserMemoryRegionInfo {
/// Guest physical address of the memory region.
pub guest_phys_addr: u64,
@ -60,25 +58,12 @@ pub struct VhostUserMemoryRegionInfo {
pub memory_size: u64,
/// Virtual address in the current process.
pub userspace_addr: u64,
/// Optional offset where region starts in the mapped memory.
/// Offset where region starts in the mapped memory.
pub mmap_offset: u64,
/// Optional file descriptor for mmap.
/// File descriptor for mmap.
pub mmap_handle: RawDescriptor,
}
// We cannot derive default because windows Handle does not implement a default.
impl Default for VhostUserMemoryRegionInfo {
fn default() -> Self {
VhostUserMemoryRegionInfo {
guest_phys_addr: u64::default(),
memory_size: u64::default(),
userspace_addr: u64::default(),
mmap_offset: u64::default(),
mmap_handle: INVALID_DESCRIPTOR,
}
}
}
#[cfg(test)]
mod tests {
use super::*;

View file

@ -142,11 +142,10 @@ impl Master {
let mut ctx = VhostUserMemoryContext::new();
for region in regions.iter() {
// TODO(b/221882601): once mmap handle cross platform story exists, update this null
// check.
if region.memory_size == 0 || (region.mmap_handle as isize) < 0 {
if region.memory_size == 0 || region.mmap_handle == INVALID_DESCRIPTOR {
return Err(VhostUserError::InvalidParam);
}
let reg = VhostUserMemoryRegion {
guest_phys_addr: region.guest_phys_addr,
memory_size: region.memory_size,
@ -546,8 +545,8 @@ impl Master {
{
return Err(VhostUserError::InvalidOperation);
}
// TODO(b/221882601): once mmap handle cross platform story exists, update this null check.
if region.memory_size == 0 || (region.mmap_handle as isize) < 0 {
if region.memory_size == 0 || region.mmap_handle == INVALID_DESCRIPTOR {
return Err(VhostUserError::InvalidParam);
}
@ -790,6 +789,7 @@ impl MasterInternal {
#[cfg(test)]
mod tests {
use base::INVALID_DESCRIPTOR;
use tempfile::tempfile;
use super::*;
use crate::connection::tests::create_pair;
@ -1105,8 +1105,23 @@ mod tests {
fn test_maset_set_mem_table_failure() {
let (master, _peer) = create_pair2();
// set_mem_table() with 0 regions is invalid
master.set_mem_table(&[]).unwrap_err();
let tables = vec![VhostUserMemoryRegionInfo::default(); MAX_ATTACHED_FD_ENTRIES + 1];
// set_mem_table() with more than MAX_ATTACHED_FD_ENTRIES is invalid
let files: Vec<File> = (0..MAX_ATTACHED_FD_ENTRIES + 1)
.map(|_| tempfile().unwrap())
.collect();
let tables: Vec<VhostUserMemoryRegionInfo> = files
.iter()
.map(|f| VhostUserMemoryRegionInfo {
guest_phys_addr: 0,
memory_size: 0x100000,
userspace_addr: 0x800000,
mmap_offset: 0,
mmap_handle: f.as_raw_descriptor(),
})
.collect();
master.set_mem_table(&tables).unwrap_err();
}
}