mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-24 12:34:31 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

hypervisor: pass IoOperation data as slices

Browse source 
Previously, the Vcpu handle_io() and handle_mmio() functions used an
IoOperation containing a fixed-length data array to represent a write
and returning a fixed-length data array to represent a read, along with
a separate size field to indicate how much of the fixed-length array
should be read/written.

This change uses Rust slices to represent the I/O data instead:
- Write contains a &[u8] of data to be written.
- Read contains a &mut [u8] to be filled with the read data.

The new IoOperation matches the Bus read()/write() APIs more closely,
and avoids the need for hypervisors and callers to convert between
fixed-size arrays and slices.

The Bus::read() function now always initializes the data slice before
(potentially) calling a device's read() function. This ensures
consistent results even if a device does not always fill out every data
byte (for example, the default BusDevice read() handler that is a no-op)
or if no device is found. This replaces the previous zeroing that would
happen when initializing the read data array to return from handle_fn.
Without this, the data slice may have had stale data from the previous
MMIO/IO exit, depending on the hypervisor implementation.

No functional change intended.

BUG=b:359382839
TEST=tools/dev_container tools/presubmit

Change-Id: Id88ebfa7ece5cc7466c010db2cbde303aeb97bf8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5913962
Reviewed-by: Vaibhav Nagarnaik <vnagarnaik@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This commit is contained in:
Daniel Verkamp 2024-10-07 12:24:36 -07:00 committed by crosvm LUCI
parent 787240bce2
commit 40920e7278
16 changed files with 317 additions and 507 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
devices/src/bus.rs
View file

@ -700,11 +700,15 @@ impl Bus {
/// Reads data from the device that owns the range containing `addr` and puts it into `data`.
///
/// Returns true on success, otherwise `data` is untouched.
/// Returns true on success, otherwise `data` is filled with zeroes.
pub fn read(&self, addr: u64, data: &mut [u8]) -> bool {
#[cfg(feature = "stats")]
let start = self.stats.lock().start_stat();
// Initialize `data` with all zeroes to ensure consistent results even if device `read()`
// implementations don't always fill every byte.
data.fill(0);
let device_index = if let Some((offset, address, entry)) = self.get_device(addr) {
let io = BusAccessInfo {
address,

7
devices/tests/irqchip/userspace.rs
View file

@ -674,13 +674,10 @@ impl Vcpu for FakeVcpu {
unimplemented!()
}
fn handle_mmio(
&self,
_handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, _handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
unimplemented!()
}
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
unimplemented!()
}
fn on_suspend(&self) -> Result<()> {

28
hypervisor/src/geniezone/mod.rs
View file

@ -1210,10 +1210,7 @@ impl Vcpu for GeniezoneVcpu {
}
}
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the gzvm_vcpu_run struct because the
// kernel told us how large it was. The pointer is page aligned so casting to a different
@ -1227,29 +1224,22 @@ impl Vcpu for GeniezoneVcpu {
// union field to use.
let mmio = unsafe { &mut run.__bindgen_anon_1.mmio };
let address = mmio.phys_addr;
let size = mmio.size as usize;
let data = &mut mmio.data[..mmio.size as usize];
if mmio.is_write != 0 {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Write { data: mmio.data },
})?;
Ok(())
} else if let Some(data) = handle_fn(IoParams {
address,
size,
operation: IoOperation::Read,
})? {
mmio.data[..size].copy_from_slice(&data[..size]);
Ok(())
operation: IoOperation::Write(data),
})
} else {
Err(Error::new(EINVAL))
handle_fn(IoParams {
address,
operation: IoOperation::Read(data),
})
}
}
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
Err(Error::new(EINVAL))
}
}

28
hypervisor/src/gunyah/mod.rs
View file

@ -6,7 +6,6 @@
mod aarch64;
mod gunyah_sys;
use std::cmp::min;
use std::cmp::Reverse;
use std::collections::BTreeMap;
use std::collections::BinaryHeap;
@ -810,10 +809,7 @@ impl Vcpu for GunyahVcpu {
}
}
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the gh_vcpu_run struct because the
// kernel told us how large it was. The pointer is page aligned so casting to a different
@ -826,27 +822,21 @@ impl Vcpu for GunyahVcpu {
// union field to use.
let mmio = unsafe { &mut run.__bindgen_anon_1.mmio };
let address = mmio.phys_addr;
let size = min(mmio.len as usize, mmio.data.len());
let data = &mut mmio.data[..mmio.len as usize];
if mmio.is_write != 0 {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Write { data: mmio.data },
})?;
Ok(())
} else if let Some(data) = handle_fn(IoParams {
address,
size,
operation: IoOperation::Read,
})? {
mmio.data[..size].copy_from_slice(&data[..size]);
Ok(())
operation: IoOperation::Write(data),
})
} else {
Err(Error::new(EINVAL))
handle_fn(IoParams {
address,
operation: IoOperation::Read(data),
})
}
}
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, _handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
unreachable!()
}

82
hypervisor/src/haxm/vcpu.rs
View file

@ -5,7 +5,6 @@
use core::ffi::c_void;
use std::arch::x86_64::CpuidResult;
use std::collections::BTreeMap;
use std::intrinsics::copy_nonoverlapping;
use std::mem::size_of;
use base::errno_result;
@ -176,10 +175,7 @@ impl Vcpu for HaxmVcpu {
/// Once called, it will determine whether a mmio read or mmio write was the reason for the mmio
/// exit, call `handle_fn` with the respective IoOperation to perform the mmio read or
/// write, and set the return data in the vcpu so that the vcpu can resume running.
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the hax_tunnel struct because the
// kernel told us how large it was.
@ -193,39 +189,34 @@ impl Vcpu for HaxmVcpu {
// Safe because the exit_reason (which comes from the kernel) told us which
// union field to use.
unsafe { ((*mmio).gpa, (*mmio).size as usize, (*mmio).direction) };
// SAFETY:
// Safe because the exit_reason (which comes from the kernel) told us which
// union field to use. We use `addr_of_mut!()` to get a potentially unaligned u64 pointer,
// but it is then cast via a u8 pointer to a u8 slice, which has no alignment requirements.
let data = unsafe {
assert!(size <= size_of::<u64>());
std::slice::from_raw_parts_mut(
std::ptr::addr_of_mut!((*mmio).__bindgen_anon_1.value) as *mut u8,
size,
)
};
match direction {
HAX_EXIT_DIRECTION_MMIO_READ => {
if let Some(data) = handle_fn(IoParams {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Read,
operation: IoOperation::Read(data),
})
// We have to unwrap/panic here because HAXM doesn't have a
// facility to inject a GP fault here. Once HAXM can do that, we
// should inject a GP fault & bubble the error.
.unwrap()
{
let data = u64::from_ne_bytes(data);
// SAFETY:
// Safe because we know this is an mmio read, so we need to put data into the
// "value" field of the hax_fastmmio.
unsafe {
(*mmio).__bindgen_anon_1.value = data;
}
}
.unwrap();
Ok(())
}
HAX_EXIT_DIRECTION_MMIO_WRITE => {
// SAFETY:
// safe because we trust haxm to fill in the union properly.
let data = unsafe { (*mmio).__bindgen_anon_1.value };
handle_fn(IoParams {
address,
size,
operation: IoOperation::Write {
data: data.to_ne_bytes(),
},
operation: IoOperation::Write(data),
})
// Similarly to the read direction, we MUST panic here.
.unwrap();
@ -241,7 +232,7 @@ impl Vcpu for HaxmVcpu {
/// call `handle_fn` with the respective IoOperation to perform the io in or io out,
/// and set the return data in the vcpu so that the vcpu can resume running.
#[allow(clippy::cast_ptr_alignment)]
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the hax_tunnel struct because the
// kernel told us how large it was.
@ -256,40 +247,29 @@ impl Vcpu for HaxmVcpu {
let address = io.port.into();
let size = io.size as usize;
let count = io.count as usize;
let mut io_buffer_ptr = self.io_buffer as *mut u8;
let data_len = count * size;
// SAFETY:
// Safe because the exit_reason (which comes from the kernel) told us that this is port io,
// where the iobuf can be treated as a *u8
let buffer: &mut [u8] =
unsafe { std::slice::from_raw_parts_mut(self.io_buffer as *mut u8, data_len) };
let data_chunks = buffer.chunks_mut(size);
match io.direction as u32 {
HAX_EXIT_DIRECTION_PIO_IN => {
for _ in 0..count {
if let Some(data) = handle_fn(IoParams {
for data in data_chunks {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Read,
}) {
// SAFETY:
// Safe because the exit_reason (which comes from the kernel) told us that
// this is port io, where the iobuf can be treated as a *u8
unsafe {
copy_nonoverlapping(data.as_ptr(), io_buffer_ptr, size);
io_buffer_ptr = io_buffer_ptr.add(size);
}
}
operation: IoOperation::Read(data),
});
}
Ok(())
}
HAX_EXIT_DIRECTION_PIO_OUT => {
for _ in 0..count {
let mut data = [0; 8];
// SAFETY:
// safe because we check the size, from what the kernel told us is the max to
// copy.
unsafe {
copy_nonoverlapping(io_buffer_ptr, data.as_mut_ptr(), size);
io_buffer_ptr = io_buffer_ptr.add(size);
}
for data in data_chunks {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Write { data },
operation: IoOperation::Write(data),
});
}
Ok(())

99
hypervisor/src/kvm/mod.rs
View file

@ -16,7 +16,6 @@ mod riscv64;
#[cfg(target_arch = "x86_64")]
mod x86_64;
use std::cmp::min;
use std::cmp::Reverse;
use std::collections::BTreeMap;
use std::collections::BinaryHeap;
@ -27,7 +26,6 @@ use std::os::raw::c_ulong;
use std::os::raw::c_void;
use std::os::unix::prelude::OsStrExt;
use std::path::Path;
use std::ptr::copy_nonoverlapping;
use std::sync::Arc;
use base::errno_result;
@ -1039,14 +1037,10 @@ impl Vcpu for KvmVcpu {
}
}
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the kvm_run struct because the
// kernel told us how large it was. The pointer is page aligned so casting to a different
// type is well defined, hence the clippy allow attribute.
// kernel told us how large it was.
let run = unsafe { &mut *(self.run_mmap.as_ptr() as *mut kvm_run) };
// Verify that the handler is called in the right context.
assert!(run.exit_reason == KVM_EXIT_MMIO);
@ -1055,31 +1049,24 @@ impl Vcpu for KvmVcpu {
// union field to use.
let mmio = unsafe { &mut run.__bindgen_anon_1.mmio };
let address = mmio.phys_addr;
let size = min(mmio.len as usize, mmio.data.len());
let data = &mut mmio.data[..mmio.len as usize];
if mmio.is_write != 0 {
handle_fn(IoParams {
address,
size,
operation: IoOperation::Write { data: mmio.data },
})?;
Ok(())
} else if let Some(data) = handle_fn(IoParams {
address,
size,
operation: IoOperation::Read,
})? {
mmio.data[..size].copy_from_slice(&data[..size]);
Ok(())
operation: IoOperation::Write(data),
})
} else {
Err(Error::new(EINVAL))
handle_fn(IoParams {
address,
operation: IoOperation::Read(data),
})
}
}
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
// SAFETY:
// Safe because we know we mapped enough memory to hold the kvm_run struct because the
// kernel told us how large it was. The pointer is page aligned so casting to a different
// type is well defined, hence the clippy allow attribute.
// kernel told us how large it was.
let run = unsafe { &mut *(self.run_mmap.as_ptr() as *mut kvm_run) };
// Verify that the handler is called in the right context.
assert!(run.exit_reason == KVM_EXIT_IO);
@ -1087,52 +1074,42 @@ impl Vcpu for KvmVcpu {
// Safe because the exit_reason (which comes from the kernel) told us which
// union field to use.
let io = unsafe { run.__bindgen_anon_1.io };
let address = u64::from(io.port);
let size = usize::from(io.size);
let count = io.count as usize;
let data_len = count * size;
let data_offset = io.data_offset as usize;
assert!(data_offset + data_len <= self.run_mmap.size());
// SAFETY:
// The data_offset is defined by the kernel to be some number of bytes into the kvm_run
// structure, which we have fully mmap'd.
let mut data_ptr = unsafe { (run as *mut kvm_run as *mut u8).add(io.data_offset as usize) };
let buffer: &mut [u8] = unsafe {
std::slice::from_raw_parts_mut(
(run as *mut kvm_run as *mut u8).add(data_offset),
data_len,
)
};
let data_chunks = buffer.chunks_mut(size);
match io.direction as u32 {
KVM_EXIT_IO_IN => {
for _ in 0..io.count {
if let Some(data) = handle_fn(IoParams {
address: io.port.into(),
size,
operation: IoOperation::Read,
}) {
// TODO(b/315998194): Add safety comment
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
copy_nonoverlapping(data.as_ptr(), data_ptr, size);
data_ptr = data_ptr.add(size);
}
} else {
return Err(Error::new(EINVAL));
}
}
Ok(())
if io.direction == KVM_EXIT_IO_IN as u8 {
for data in data_chunks {
handle_fn(IoParams {
address,
operation: IoOperation::Read(data),
});
}
KVM_EXIT_IO_OUT => {
for _ in 0..io.count {
let mut data = [0; 8];
// TODO(b/315998194): Add safety comment
#[allow(clippy::undocumented_unsafe_blocks)]
unsafe {
copy_nonoverlapping(data_ptr, data.as_mut_ptr(), min(size, data.len()));
data_ptr = data_ptr.add(size);
}
handle_fn(IoParams {
address: io.port.into(),
size,
operation: IoOperation::Write { data },
});
}
Ok(())
} else {
debug_assert_eq!(io.direction, KVM_EXIT_IO_OUT as u8);
for data in data_chunks {
handle_fn(IoParams {
address,
operation: IoOperation::Write(data),
});
}
_ => Err(Error::new(EINVAL)),
}
Ok(())
}
}

32
hypervisor/src/lib.rs
View file

@ -268,23 +268,22 @@ pub trait Vm: Send {
}
/// Operation for Io and Mmio
#[derive(Copy, Clone, Debug)]
pub enum IoOperation {
Read,
Write {
/// Data to be written.
///
/// For 64 bit architecture, Mmio and Io only work with at most 8 bytes of data.
data: [u8; 8],
},
#[derive(Debug)]
pub enum IoOperation<'a> {
/// Data to be read from a device on the bus.
///
/// The `handle_fn` should fill the entire slice with the read data.
Read(&'a mut [u8]),
/// Data to be written to a device on the bus.
Write(&'a [u8]),
}
/// Parameters describing an MMIO or PIO from the guest.
#[derive(Copy, Clone, Debug)]
pub struct IoParams {
#[derive(Debug)]
pub struct IoParams<'a> {
pub address: u64,
pub size: usize,
pub operation: IoOperation,
pub operation: IoOperation<'a>,
}
/// Handle to a virtual CPU that may be used to request a VM exit from within a signal handler.
@ -350,10 +349,7 @@ pub trait Vcpu: downcast_rs::DowncastSync {
/// Once called, it will determine whether a MMIO read or MMIO write was the reason for the MMIO
/// exit, call `handle_fn` with the respective IoParams to perform the MMIO read or write, and
/// set the return data in the vcpu so that the vcpu can resume running.
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()>;
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()>;
/// Handles an incoming PIO from the guest.
///
@ -363,7 +359,7 @@ pub trait Vcpu: downcast_rs::DowncastSync {
/// Once called, it will determine whether an input or output was the reason for the Io exit,
/// call `handle_fn` with the respective IoParams to perform the input/output operation, and set
/// the return data in the vcpu so that the vcpu can resume running.
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()>;
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams)) -> Result<()>;
/// Signals to the hypervisor that this Vcpu is being paused by userspace.
fn on_suspend(&self) -> Result<()>;

122
hypervisor/src/whpx/vcpu.rs
View file

@ -7,6 +7,7 @@ use std::arch::x86_64::CpuidResult;
use std::collections::BTreeMap;
use std::convert::TryInto;
use std::mem::size_of;
use std::mem::size_of_val;
use std::sync::Arc;
use base::Error;
@ -105,8 +106,8 @@ trait InstructionEmulatorCallbacks {
struct InstructionEmulatorContext<'a> {
vm_partition: Arc<SafePartition>,
index: u32,
handle_mmio: Option<&'a mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>>,
handle_io: Option<&'a mut dyn FnMut(IoParams) -> Option<[u8; 8]>>,
handle_mmio: Option<&'a mut dyn FnMut(IoParams) -> Result<()>>,
handle_io: Option<&'a mut dyn FnMut(IoParams)>,
}
impl InstructionEmulatorCallbacks for SafeInstructionEmulator {
@ -117,46 +118,33 @@ impl InstructionEmulatorCallbacks for SafeInstructionEmulator {
// unsafe because windows could decide to call this at any time.
// However, we trust the kernel to call this while the vm/vcpu is valid.
let ctx = unsafe { &mut *(context as *mut InstructionEmulatorContext) };
let Some(handle_io) = &mut ctx.handle_io else {
return E_UNEXPECTED;
};
// safe because we trust the kernel to fill in the io_access
let io_access_info = unsafe { &mut *io_access };
let address = io_access_info.Port.into();
let size = io_access_info.AccessSize as usize;
// SAFETY: We trust the kernel to fill in the io_access
let data: &mut [u8] = unsafe {
assert!(size <= size_of_val(&io_access_info.Data));
std::slice::from_raw_parts_mut(&mut io_access_info.Data as *mut u32 as *mut u8, size)
};
match io_access_info.Direction {
WHPX_EXIT_DIRECTION_PIO_IN => {
if let Some(handle_io) = &mut ctx.handle_io {
if let Some(data) = handle_io(IoParams {
address,
size,
operation: IoOperation::Read,
}) {
// Safe because we know this is an io_access_info field of u32,
// so casting as a &mut [u8] of len 4 is safe.
let buffer = unsafe {
std::slice::from_raw_parts_mut(
&mut io_access_info.Data as *mut u32 as *mut u8,
4,
)
};
buffer[..size].copy_from_slice(&data[..size]);
}
S_OK
} else {
E_UNEXPECTED
}
handle_io(IoParams {
address,
operation: IoOperation::Read(data),
});
S_OK
}
WHPX_EXIT_DIRECTION_PIO_OUT => {
if let Some(handle_io) = &mut ctx.handle_io {
handle_io(IoParams {
address,
size,
operation: IoOperation::Write {
data: (io_access_info.Data as u64).to_ne_bytes(),
},
});
S_OK
} else {
E_UNEXPECTED
}
handle_io(IoParams {
address,
operation: IoOperation::Write(data),
});
S_OK
}
_ => E_UNEXPECTED,
}
@ -168,49 +156,38 @@ impl InstructionEmulatorCallbacks for SafeInstructionEmulator {
// unsafe because windows could decide to call this at any time.
// However, we trust the kernel to call this while the vm/vcpu is valid.
let ctx = unsafe { &mut *(context as *mut InstructionEmulatorContext) };
let Some(handle_mmio) = &mut ctx.handle_mmio else {
return E_UNEXPECTED;
};
// safe because we trust the kernel to fill in the memory_access
let memory_access_info = unsafe { &mut *memory_access };
let address = memory_access_info.GpaAddress;
let size = memory_access_info.AccessSize as usize;
let data = &mut memory_access_info.Data[..size];
match memory_access_info.Direction {
WHPX_EXIT_DIRECTION_MMIO_READ => {
ctx.handle_mmio
.as_mut()
.map_or(E_UNEXPECTED, |handle_mmio| {
handle_mmio(IoParams {
address,
size,
operation: IoOperation::Read,
})
.map_err(|e| {
error!("handle_mmio failed with {e}");
e
})
.ok()
.flatten()
.map_or(E_UNEXPECTED, |data| {
memory_access_info.Data = data;
S_OK
})
})
if let Err(e) = handle_mmio(IoParams {
address,
operation: IoOperation::Read(data),
}) {
error!("handle_mmio failed with {e}");
E_UNEXPECTED
} else {
S_OK
}
}
WHPX_EXIT_DIRECTION_MMIO_WRITE => {
ctx.handle_mmio
.as_mut()
.map_or(E_UNEXPECTED, |handle_mmio| {
handle_mmio(IoParams {
address,
size,
operation: IoOperation::Write {
data: memory_access_info.Data,
},
})
.map_err(|e| {
error!("handle_mmio failed with {e}");
e
})
.map_or(E_UNEXPECTED, |_| S_OK)
})
if let Err(e) = handle_mmio(IoParams {
address,
operation: IoOperation::Write(data),
}) {
error!("handle_mmio write with {e}");
E_UNEXPECTED
} else {
S_OK
}
}
_ => E_UNEXPECTED,
}
@ -559,10 +536,7 @@ impl Vcpu for WhpxVcpu {
/// Once called, it will determine whether a mmio read or mmio write was the reason for the mmio
/// exit, call `handle_fn` with the respective IoOperation to perform the mmio read or
/// write, and set the return data in the vcpu so that the vcpu can resume running.
fn handle_mmio(
&self,
handle_fn: &mut dyn FnMut(IoParams) -> Result<Option<[u8; 8]>>,
) -> Result<()> {
fn handle_mmio(&self, handle_fn: &mut dyn FnMut(IoParams) -> Result<()>) -> Result<()> {
let mut status: WHV_EMULATOR_STATUS = Default::default();
let mut ctx = InstructionEmulatorContext {
vm_partition: self.vm_partition.clone(),
@ -596,7 +570,7 @@ impl Vcpu for WhpxVcpu {
/// Once called, it will determine whether an io in or io out was the reason for the io exit,
/// call `handle_fn` with the respective IoOperation to perform the io in or io out,
/// and set the return data in the vcpu so that the vcpu can resume running.
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams) -> Option<[u8; 8]>) -> Result<()> {
fn handle_io(&self, handle_fn: &mut dyn FnMut(IoParams)) -> Result<()> {
let mut status: WHV_EMULATOR_STATUS = Default::default();
let mut ctx = InstructionEmulatorContext {
vm_partition: self.vm_partition.clone(),

112
hypervisor/tests/hypervisor_virtualization.rs
View file

@ -884,27 +884,20 @@ fn test_io_exit_handler() {
let exit_matcher =
move |_, exit: &VcpuExit, vcpu: &mut dyn VcpuX86_64, _: &mut dyn Vm| match exit {
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_io(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
IoOperation::Read(data) => {
assert_eq!(address, 0x20);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
// The original number written below will be doubled and
// passed back.
data[0] = cached_byte.load(Ordering::SeqCst) * 2;
Some(data)
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x10);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
assert_eq!(data[0], 0x34);
cached_byte.fetch_add(data[0], Ordering::SeqCst);
None
}
}
})
@ -964,27 +957,20 @@ fn test_io_rep_string() {
let exit_matcher =
move |_, exit: &VcpuExit, vcpu: &mut dyn VcpuX86_64, vm: &mut dyn Vm| match exit {
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_io(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
IoOperation::Read(data) => {
assert_eq!(address, 0x80);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
// Return 0, 1, 2, 3, 4 for subsequent reads.
data[0] = read_data.fetch_add(1, Ordering::SeqCst);
Some(data)
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x80);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
// Expect 0, 1, 2, 3, 4 to be written.
let expected_write = write_data.fetch_add(1, Ordering::SeqCst);
assert_eq!(data[0], expected_write);
None
}
}
})
@ -1046,14 +1032,10 @@ fn test_mmio_exit_cross_page() {
let exit_matcher = |_, exit: &VcpuExit, vcpu: &mut dyn VcpuX86_64, _: &mut dyn Vm| match exit {
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_mmio(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
match (address, size) {
IoOperation::Read(data) => {
match (address, data.len()) {
// First MMIO read asks to load the first 8 bytes
// of a new execution page, when an instruction
// crosses page boundary.
@ -1062,21 +1044,25 @@ fn test_mmio_exit_cross_page() {
(0x1000, 8) => {
// Ensure this instruction is the first read
// in the sequence.
Ok(Some([0x88, 0x03, 0x67, 0x8a, 0x01, 0xf4, 0, 0]))
data.copy_from_slice(&[0x88, 0x03, 0x67, 0x8a, 0x01, 0xf4, 0, 0]);
Ok(())
}
// Second MMIO read is a regular read from an
// unmapped memory (pointed to by initial EAX).
(0x3010, 1) => Ok(Some([0x66, 0, 0, 0, 0, 0, 0, 0])),
(0x3010, 1) => {
data.copy_from_slice(&[0x66]);
Ok(())
}
_ => {
panic!("invalid address({:#x})/size({})", address, size)
panic!("invalid address({:#x})/size({})", address, data.len())
}
}
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x3000);
assert_eq!(data[0], 0x33);
assert_eq!(size, 1);
Ok(None)
assert_eq!(data.len(), 1);
Ok(())
}
}
})
@ -1159,19 +1145,15 @@ fn test_mmio_exit_readonly_memory() {
let exit_matcher = |_, exit: &VcpuExit, vcpu: &mut dyn VcpuX86_64, _: &mut dyn Vm| match exit {
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| match operation {
IoOperation::Read => {
vcpu.handle_mmio(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(_) => {
panic!("unexpected mmio read call");
}
IoOperation::Write { data } => {
assert_eq!(size, 1);
IoOperation::Write(data) => {
assert_eq!(data.len(), 1);
assert_eq!(address, 0x5000);
assert_eq!(data[0], 0x67);
Ok(None)
Ok(())
}
})
.expect("failed to set the data");
@ -2638,7 +2620,6 @@ fn test_interrupt_ready_when_normally_not_interruptible() {
}
instrumentation_traces.borrow_mut().push(instrumentation);
// We are always handling out IO port, so no data to return.
None
})
.expect("should handle IO successfully");
if should_inject_interrupt {
@ -2696,7 +2677,6 @@ fn test_interrupt_ready_when_interrupt_enable_flag_not_set() {
vcpu.handle_io(&mut |io_params| {
addr = io_params.address;
// We are always handling out IO port, so no data to return.
None
})
.expect("should handle IO successfully");
let regs = vcpu
@ -2942,7 +2922,7 @@ fn test_request_interrupt_window() {
VcpuExit::Intr => false,
VcpuExit::Io => {
// We are always handling out IO port, so no data to return.
vcpu.handle_io(&mut |_| None)
vcpu.handle_io(&mut |_| {})
.expect("should handle IO successfully");
assert!(!vcpu.ready_for_interrupt());
@ -3098,7 +3078,7 @@ fn test_mmx_state_is_preserved_by_hypervisor() {
false
}
VcpuExit::Io => {
vcpu.handle_io(&mut |_| None)
vcpu.handle_io(&mut |_| {})
.expect("should handle IO successfully");
// kaiyili@ pointed out we should check the XSAVE state exposed by the hypervisor via
@ -3238,7 +3218,7 @@ fn test_avx_state_is_preserved_by_hypervisor() {
false
}
VcpuExit::Io => {
vcpu.handle_io(&mut |_| None)
vcpu.handle_io(&mut |_| {})
.expect("should handle IO successfully");
// kaiyili@ pointed out we should check the XSAVE state exposed by the hypervisor via
@ -3660,7 +3640,7 @@ fn test_slat_on_region_removal_is_mmio() {
//
// We strictly don't care what this data is, since the VM exits before running any
// further instructions.
vcpu.handle_io(&mut |_| None)
vcpu.handle_io(&mut |_| {})
.expect("should handle IO successfully");
// Remove the test memory region to cause a SLAT fault (in the passing case).
@ -3672,21 +3652,18 @@ fn test_slat_on_region_removal_is_mmio() {
false
}
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_mmio(&mut |IoParams { address, operation }| {
assert_eq!(address, 0x20000, "MMIO for wrong address");
assert_eq!(size, 1);
assert!(
matches!(operation, IoOperation::Read),
"got unexpected IO operation {:?}",
operation
);
// We won't vmenter again, so there's no need to actually satisfy the MMIO by
// returning data; however, some hypervisors (WHPX) require it.
Ok(Some([0u8; 8]))
match operation {
IoOperation::Read(data) => {
assert_eq!(data.len(), 1);
data[0] = 0;
Ok(())
}
IoOperation::Write(_) => {
panic!("got unexpected IO operation {:?}", operation);
}
}
})
.unwrap();
true
@ -3891,7 +3868,7 @@ fn test_interrupt_injection_when_not_ready() {
VcpuExit::FailEntry { .. } | VcpuExit::Shutdown(..) | VcpuExit::Hlt => true,
VcpuExit::Io => {
// We are always handling out IO port, so no data to return.
vcpu.handle_io(&mut |_| None)
vcpu.handle_io(&mut |_| {})
.expect("should handle IO successfully");
assert!(!vcpu.ready_for_interrupt());
// We don't care whether we inject the interrupt successfully or not.
@ -3959,7 +3936,6 @@ fn test_ready_for_interrupt_for_intercepted_instructions() {
vcpu.handle_io(&mut |params| {
io_port = params.address;
// We are always handling out IO port, so no data to return.
None
})
.expect("should handle port IO successfully");
match io_port {

109
hypervisor/tests/mmio_and_pio.rs
View file

@ -111,54 +111,42 @@ where
loop {
match vcpu.run().expect("run failed") {
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_mmio(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
IoOperation::Read(data) => {
assert_eq!(address, 0x3010);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
exits.fetch_add(1, Ordering::SeqCst);
// this number will be read into al register
data.copy_from_slice(&0x66_u64.to_ne_bytes());
Ok(Some(data))
data.copy_from_slice(&[0x66]);
Ok(())
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x3000);
assert_eq!(data[0], 0x33);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
exits.fetch_add(1, Ordering::SeqCst);
Ok(None)
Ok(())
}
}
})
.expect("failed to set the data");
}
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_io(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
IoOperation::Read(data) => {
assert_eq!(address, 0x20);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
exits.fetch_add(1, Ordering::SeqCst);
// this number will be read into the al register
data.copy_from_slice(&0x77_u64.to_ne_bytes());
Some(data)
data.copy_from_slice(&[0x77]);
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x19);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
assert_eq!(data[0], 0x66);
exits.fetch_add(1, Ordering::SeqCst);
None
}
}
})
@ -282,27 +270,20 @@ where
loop {
match vcpu.run().expect("run failed") {
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| {
match operation {
IoOperation::Read => panic!("unexpected PIO read"),
IoOperation::Write { data } => {
assert!((1..=4).contains(&address));
if address % 2 == 0 {
assert_eq!(size, 1);
assert_eq!(data[0], address as u8);
} else {
assert_eq!(size, 2);
assert_eq!(data[0], address as u8);
assert_eq!(data[1], 0);
}
exit_bits.fetch_or(1 << (address - 1), Ordering::SeqCst);
exit_count.fetch_add(1, Ordering::SeqCst);
None
vcpu.handle_io(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(_) => panic!("unexpected PIO read"),
IoOperation::Write(data) => {
assert!((1..=4).contains(&address));
if address % 2 == 0 {
assert_eq!(data.len(), 1);
assert_eq!(data[0], address as u8);
} else {
assert_eq!(data.len(), 2);
assert_eq!(data[0], address as u8);
assert_eq!(data[1], 0);
}
exit_bits.fetch_or(1 << (address - 1), Ordering::SeqCst);
exit_count.fetch_add(1, Ordering::SeqCst);
}
})
.expect("failed to set the data");
@ -427,31 +408,23 @@ where
loop {
match vcpu.run().expect("run failed") {
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
assert!((1..=4).contains(&address));
vcpu.handle_io(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(data) => {
assert!((1..=4).contains(&address));
if address % 2 == 0 {
assert_eq!(size, 1);
data[0] = address as u8;
} else {
assert_eq!(size, 2);
data[0] = address as u8;
data[1] = address as u8;
}
exit_bits.fetch_or(1 << (address - 1), Ordering::SeqCst);
exit_count.fetch_add(1, Ordering::SeqCst);
Some(data)
if address % 2 == 0 {
assert_eq!(data.len(), 1);
data[0] = address as u8;
} else {
assert_eq!(data.len(), 2);
data[0] = address as u8;
data[1] = address as u8;
}
IoOperation::Write { .. } => panic!("unexpected PIO write"),
exit_bits.fetch_or(1 << (address - 1), Ordering::SeqCst);
exit_count.fetch_add(1, Ordering::SeqCst);
}
IoOperation::Write(_) => panic!("unexpected PIO write"),
})
.expect("failed to set the data");
}

28
hypervisor/tests/mmio_fetch_memory.rs
View file

@ -80,15 +80,11 @@ fn test_whpx_mmio_fetch_memory() {
match vcpu.run().expect("run failed") {
VcpuExit::Mmio => {
exits.fetch_add(1, Ordering::SeqCst);
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| {
vcpu.handle_mmio(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
IoOperation::Read(data) => {
memory_reads.fetch_add(1, Ordering::SeqCst);
match (address, size) {
match (address, data.len()) {
// First MMIO read from the WHV_EMULATOR asks to
// load the first 8 bytes of a new execution
// page, when an instruction crosses page
@ -99,22 +95,28 @@ fn test_whpx_mmio_fetch_memory() {
// Ensure this instruction is the first read
// in the sequence.
assert_eq!(memory_reads.load(Ordering::SeqCst), 1);
Ok(Some([0x88, 0x03, 0x67, 0x8a, 0x01, 0xf4, 0, 0]))
data.copy_from_slice(&[
0x88, 0x03, 0x67, 0x8a, 0x01, 0xf4, 0, 0,
]);
Ok(())
}
// Second MMIO read is a regular read from an
// unmapped memory.
(0x3010, 1) => Ok(Some([0x66, 0, 0, 0, 0, 0, 0, 0])),
(0x3010, 1) => {
data.copy_from_slice(&[0x66]);
Ok(())
}
_ => {
panic!("invalid address({:#x})/size({})", address, size)
panic!("invalid address({:#x})/size({})", address, data.len())
}
}
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x3000);
assert_eq!(data[0], 0x33);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
memory_writes.fetch_add(1, Ordering::SeqCst);
Ok(None)
Ok(())
}
}
})

14
hypervisor/tests/read_only_memory.rs
View file

@ -163,20 +163,16 @@ where
VcpuExit::Intr => continue,
VcpuExit::Hlt => break,
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| match operation {
IoOperation::Read => {
vcpu.handle_mmio(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(_) => {
panic!("unexpected mmio read call");
}
IoOperation::Write { data } => {
assert_eq!(size, 1);
IoOperation::Write(data) => {
assert_eq!(data.len(), 1);
assert_eq!(address, vcpu_sregs.es.base);
assert_eq!(data[0], 0x67);
exits.fetch_add(1, Ordering::SeqCst);
Ok(None)
Ok(())
}
})
.expect("failed to set the data");

13
hypervisor/tests/real_run_addr.rs
View file

@ -108,19 +108,14 @@ where
loop {
match vcpu.run().expect("run failed") {
VcpuExit::Io => {
vcpu.handle_io(&mut |IoParams {
address,
size,
operation,
}| match operation {
IoOperation::Read => {
vcpu.handle_io(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(_) => {
panic!("unexpected io in call");
}
IoOperation::Write { data } => {
IoOperation::Write(data) => {
assert_eq!(address, 0x3f8);
assert_eq!(size, 1);
assert_eq!(data.len(), 1);
out.lock().push(data[0] as char);
None
}
})
.expect("failed to set the data");

17
hypervisor/tests/remove_memory.rs
View file

@ -168,19 +168,14 @@ where
VcpuExit::Intr => continue,
VcpuExit::Hlt => break,
VcpuExit::Mmio => {
vcpu.handle_mmio(&mut |IoParams {
address,
size,
operation,
}| match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
vcpu.handle_mmio(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(data) => {
assert_eq!(address, 0x3000);
assert_eq!(size, 1);
data.copy_from_slice(&0x44_u64.to_ne_bytes());
Ok(Some(data))
assert_eq!(data.len(), 1);
data.copy_from_slice(&[0x44]);
Ok(())
}
IoOperation::Write { .. } => {
IoOperation::Write(_) => {
panic!("unexpected mmio write");
}
})

51
src/crosvm/sys/linux/vcpu.rs
View file

@ -64,35 +64,6 @@ const SCHED_FLAG_UTIL_CLAMP_MIN: u64 = 0x20;
const SCHED_SCALE_CAPACITY: u32 = 1024;
const SCHED_FLAG_KEEP_ALL: u64 = SCHED_FLAG_KEEP_POLICY | SCHED_FLAG_KEEP_PARAMS;
fn bus_io_handler(bus: &Bus) -> impl FnMut(IoParams) -> Option<[u8; 8]> + '_ {
|IoParams {
address,
mut size,
operation: direction,
}| match direction {
IoOperation::Read => {
let mut data = [0u8; 8];
if size > data.len() {
error!("unsupported Read size of {} bytes", size);
size = data.len();
}
// Ignore the return value of `read()`. If no device exists on the bus at the given
// location, return the initial value of data, which is all zeroes.
let _ = bus.read(address, &mut data[..size]);
Some(data)
}
IoOperation::Write { data } => {
if size > data.len() {
error!("unsupported Write size of {} bytes", size);
size = data.len()
}
let data = &data[..size];
bus.write(address, data);
None
}
}
}
/// Set the VCPU thread affinity and other per-thread scheduler properties.
/// This function will be called from each VCPU thread at startup.
#[allow(clippy::unnecessary_cast)]
@ -399,13 +370,31 @@ where
if !interrupted_by_signal {
match vcpu.run() {
Ok(VcpuExit::Io) => {
if let Err(e) = vcpu.handle_io(&mut bus_io_handler(&io_bus)) {
if let Err(e) =
vcpu.handle_io(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(data) => {
io_bus.read(address, data);
}
IoOperation::Write(data) => {
io_bus.write(address, data);
}
})
{
error!("failed to handle io: {}", e)
}
}
Ok(VcpuExit::Mmio) => {
if let Err(e) =
vcpu.handle_mmio(&mut |io_params| Ok(bus_io_handler(&mmio_bus)(io_params)))
vcpu.handle_mmio(&mut |IoParams { address, operation }| match operation {
IoOperation::Read(data) => {
mmio_bus.read(address, data);
Ok(())
}
IoOperation::Write(data) => {
mmio_bus.write(address, data);
Ok(())
}
})
{
error!("failed to handle mmio: {}", e);
}

76
src/sys/windows/run_vcpu.rs
View file

@ -732,71 +732,47 @@ where
match exit {
Ok(VcpuExit::Io) => {
let _trace_event = trace_event!(crosvm, "VcpuExit::Io");
vcpu.handle_io(&mut |IoParams { address, mut size, operation}| {
vcpu.handle_io(&mut |IoParams { address, operation}| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
if size > data.len() {
error!("unsupported IoIn size of {} bytes", size);
size = data.len();
}
io_bus.read(address, &mut data[..size]);
Some(data)
IoOperation::Read(data) => {
io_bus.read(address, data);
}
IoOperation::Write { data } => {
if size > data.len() {
error!("unsupported IoOut size of {} bytes", size);
size = data.len()
}
vm.handle_io_events(IoEventAddress::Pio(address), &data[..size])
IoOperation::Write(data) => {
vm.handle_io_events(IoEventAddress::Pio(address), data)
.unwrap_or_else(|e| error!(
"failed to handle ioevent for pio write to {} on vcpu {}: {}",
address, context.cpu_id, e
));
io_bus.write(address, &data[..size]);
None
io_bus.write(address, data);
}
}
}).unwrap_or_else(|e| error!("failed to handle io: {}", e));
}
Ok(VcpuExit::Mmio) => {
let _trace_event = trace_event!(crosvm, "VcpuExit::Mmio");
vcpu.handle_mmio(&mut |IoParams { address, mut size, operation }| {
vcpu.handle_mmio(&mut |IoParams { address, operation }| {
match operation {
IoOperation::Read => {
let mut data = [0u8; 8];
if size > data.len() {
error!("unsupported MmioRead size of {} bytes", size);
size = data.len();
}
{
let data = &mut data[..size];
if !mmio_bus.read(address, data) {
info!(
"mmio read failed: {:x}; trying memory read..",
address
);
vm.get_memory()
.read_exact_at_addr(
data,
vm_memory::GuestAddress(address),
IoOperation::Read(data) => {
if !mmio_bus.read(address, data) {
info!(
"mmio read failed: {:x}; trying memory read..",
address
);
vm.get_memory()
.read_exact_at_addr(
data,
vm_memory::GuestAddress(address),
)
.unwrap_or_else(|e| {
error!(
"guest memory read failed at {:x}: {}",
address, e
)
.unwrap_or_else(|e| {
error!(
"guest memory read failed at {:x}: {}",
address, e
)
});
}
});
}
Ok(Some(data))
Ok(())
}
IoOperation::Write { data } => {
if size > data.len() {
error!("unsupported MmioWrite size of {} bytes", size);
size = data.len()
}
let data = &data[..size];
IoOperation::Write(data) => {
vm.handle_io_events(IoEventAddress::Mmio(address), data)
.unwrap_or_else(|e| error!(
"failed to handle ioevent for mmio write to {} on vcpu {}: {}",
@ -814,7 +790,7 @@ where
address, e
));
}
Ok(None)
Ok(())
}
}
}).unwrap_or_else(|e| error!("failed to handle mmio: {}", e));