diff --git a/Cargo.lock b/Cargo.lock index 8ba4f24..2ca4c77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -287,6 +287,7 @@ dependencies = [ "vhost", "vhost-user-backend", "virtio-bindings", + "virtio-queue", "vm-memory 0.7.0", "vmm-sys-util", ] diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index d6ba776..861b91e 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 58.4, + "coverage_score": 74.1, "exclude_path": "", "crate_features": "" } diff --git a/src/i2c/Cargo.toml b/src/i2c/Cargo.toml index 2738dda..4340251 100644 --- a/src/i2c/Cargo.toml +++ b/src/i2c/Cargo.toml @@ -19,5 +19,10 @@ thiserror = "1.0" vhost = { version = "0.2", features = ["vhost-user-slave"] } vhost-user-backend = { git = "https://github.com/rust-vmm/vhost-user-backend", rev = "4047c697470cc6c37e8e1835025b091d2b59c2f7" } virtio-bindings = ">=0.1" +virtio-queue = { git = "https://github.com/rust-vmm/vm-virtio", rev = "66cda80" } vm-memory = "0.7" vmm-sys-util = "=0.9.0" + +[dev-dependencies] +virtio-queue = { git = "https://github.com/rust-vmm/vm-virtio", rev = "66cda80", features = ["test-utils"] } +vm-memory = { version = "0.7.0", features = ["backend-mmap", "backend-atomic"] } diff --git a/src/i2c/src/i2c.rs b/src/i2c/src/i2c.rs index 4662270..946a756 100644 --- a/src/i2c/src/i2c.rs +++ b/src/i2c/src/i2c.rs @@ -574,6 +574,20 @@ pub mod tests { use std::convert::TryFrom; use vmm_sys_util::tempfile::TempFile; + // Update read-buffer of each write-buffer with index + 1 value. + pub fn update_rdwr_buf(buf: &mut Vec) { + for (i, byte) in buf.iter_mut().enumerate() { + *byte = i as u8 + 1; + } + } + + // Verify the write-buffer passed to us + pub fn verify_rdwr_buf(buf: &[u8]) { + for (i, byte) in buf.iter().enumerate() { + assert_eq!(*byte, i as u8 + 1); + } + } + #[derive(Debug)] pub struct DummyDevice { funcs_result: Result, @@ -611,12 +625,15 @@ pub mod tests { } fn rdwr(&self, reqs: &mut [I2cReq]) -> Result<()> { - // Update buffer of each write-buffer with index + 1 value. for req in reqs { + if req.len == 0 { + return Err(Error::I2cTransferInvalid(0)); + } + if (req.flags & I2C_M_RD) != 0 { - for (j, byte) in req.buf.iter_mut().enumerate() { - *byte = j as u8 + 1; - } + update_rdwr_buf(&mut req.buf); + } else { + verify_rdwr_buf(&req.buf); } } @@ -644,9 +661,7 @@ pub mod tests { // Match what's done by DummyDevice::rdwr() for req in reqs { if (req.flags & I2C_M_RD) != 0 { - for (i, byte) in req.buf.iter().enumerate() { - assert_eq!(*byte, i as u8 + 1); - } + verify_rdwr_buf(&req.buf); } } } @@ -735,6 +750,12 @@ pub mod tests { }, ]; + for req in &mut reqs { + if (req.flags & I2C_M_RD) == 0 { + update_rdwr_buf(&mut req.buf); + } + } + i2c_map.transfer(&mut *reqs).unwrap(); verify_rdwr_data(&reqs); } diff --git a/src/i2c/src/vhu_i2c.rs b/src/i2c/src/vhu_i2c.rs index f5ff3ea..9582afd 100644 --- a/src/i2c/src/vhu_i2c.rs +++ b/src/i2c/src/vhu_i2c.rs @@ -17,6 +17,7 @@ use virtio_bindings::bindings::virtio_net::{VIRTIO_F_NOTIFY_ON_EMPTY, VIRTIO_F_V use virtio_bindings::bindings::virtio_ring::{ VIRTIO_RING_F_EVENT_IDX, VIRTIO_RING_F_INDIRECT_DESC, }; +use virtio_queue::DescriptorChain; use vm_memory::{ByteValued, Bytes, GuestMemoryAtomic, GuestMemoryMmap, Le16, Le32}; use vmm_sys_util::epoll::EventSet; use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK}; @@ -32,21 +33,21 @@ const NUM_QUEUES: usize = 1; type Result = std::result::Result; type VhostUserBackendResult = std::result::Result; -#[derive(Debug, ThisError)] +#[derive(Copy, Clone, Debug, PartialEq, ThisError)] /// Errors related to vhost-device-i2c daemon. pub enum Error { #[error("Failed to handle event, didn't match EPOLLIN")] HandleEventNotEpollIn, #[error("Failed to handle unknown event")] HandleEventUnknown, - #[error("Received unexpected write only descriptor")] - UnexpectedWriteOnlyDescriptor, - #[error("Received unexpected readable descriptor")] - UnexpectedReadableDescriptor, - #[error("Invalid descriptor count")] - UnexpectedDescriptorCount, - #[error("Invalid descriptor size")] - UnexpectedDescriptorSize, + #[error("Received unexpected write only descriptor at index {0}")] + UnexpectedWriteOnlyDescriptor(usize), + #[error("Received unexpected readable descriptor at index {0}")] + UnexpectedReadableDescriptor(usize), + #[error("Invalid descriptor count {0}")] + UnexpectedDescriptorCount(usize), + #[error("Invalid descriptor size, expected: {0}, found: {1}")] + UnexpectedDescriptorSize(usize, u32), #[error("Descriptor not found")] DescriptorNotFound, #[error("Descriptor read failed")] @@ -55,8 +56,8 @@ pub enum Error { DescriptorWriteFailed, #[error("Failed to send notification")] NotificationFailed, - #[error("Failed to create new EventFd: {0:?}")] - EventFdFailed(std::io::Error), + #[error("Failed to create new EventFd")] + EventFdFailed, } impl convert::From for io::Error { @@ -96,26 +97,25 @@ pub struct VhostUserI2cBackend { pub exit_event: EventFd, } +type I2cDescriptorChain = DescriptorChain>>; + impl VhostUserI2cBackend { pub fn new(i2c_map: Arc>) -> Result { Ok(VhostUserI2cBackend { i2c_map, event_idx: false, - exit_event: EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdFailed)?, + exit_event: EventFd::new(EFD_NONBLOCK).map_err(|_| Error::EventFdFailed)?, }) } /// Process the requests in the vring and dispatch replies - fn process_queue(&self, vring: &VringRwLock) -> Result { + fn process_requests( + &self, + requests: Vec, + vring: Option<&VringRwLock>, + ) -> Result { let mut reqs: Vec = Vec::new(); - let requests: Vec<_> = vring - .get_mut() - .get_queue_mut() - .iter() - .map_err(|_| Error::DescriptorNotFound)? - .collect(); - if requests.is_empty() { return Ok(true); } @@ -125,16 +125,20 @@ impl VhostUserI2cBackend { let descriptors: Vec<_> = desc_chain.clone().collect(); if (descriptors.len() != 2) && (descriptors.len() != 3) { - return Err(Error::UnexpectedDescriptorCount); + return Err(Error::UnexpectedDescriptorCount(descriptors.len())); } let desc_out_hdr = descriptors[0]; + if desc_out_hdr.is_write_only() { - return Err(Error::UnexpectedWriteOnlyDescriptor); + return Err(Error::UnexpectedWriteOnlyDescriptor(0)); } if desc_out_hdr.len() as usize != size_of::() { - return Err(Error::UnexpectedDescriptorSize); + return Err(Error::UnexpectedDescriptorSize( + size_of::(), + desc_out_hdr.len(), + )); } let out_hdr = desc_chain @@ -149,11 +153,14 @@ impl VhostUserI2cBackend { let desc_in_hdr = descriptors[descriptors.len() - 1]; if !desc_in_hdr.is_write_only() { - return Err(Error::UnexpectedReadableDescriptor); + return Err(Error::UnexpectedReadableDescriptor(descriptors.len() - 1)); } if desc_in_hdr.len() as usize != size_of::() { - return Err(Error::UnexpectedDescriptorSize); + return Err(Error::UnexpectedDescriptorSize( + size_of::(), + desc_in_hdr.len(), + )); } let (buf, len) = match descriptors.len() { @@ -163,13 +170,13 @@ impl VhostUserI2cBackend { let len = desc_buf.len(); if len == 0 { - return Err(Error::UnexpectedDescriptorSize); + return Err(Error::UnexpectedDescriptorSize(1, len)); } let mut buf = vec![0; len as usize]; if flags != I2C_M_RD { if desc_buf.is_write_only() { - return Err(Error::UnexpectedWriteOnlyDescriptor); + return Err(Error::UnexpectedWriteOnlyDescriptor(1)); } desc_chain @@ -177,7 +184,7 @@ impl VhostUserI2cBackend { .read(&mut buf, desc_buf.addr()) .map_err(|_| Error::DescriptorReadFailed)?; } else if !desc_buf.is_write_only() { - return Err(Error::UnexpectedReadableDescriptor); + return Err(Error::UnexpectedReadableDescriptor(1)); } (buf, len) @@ -230,15 +237,32 @@ impl VhostUserI2cBackend { .write_obj::(in_hdr, desc_in_hdr.addr()) .map_err(|_| Error::DescriptorWriteFailed)?; - if vring.add_used(desc_chain.head_index(), len).is_err() { - warn!("Couldn't return used descriptors to the ring"); + if let Some(vring) = vring { + if vring.add_used(desc_chain.head_index(), len).is_err() { + warn!("Couldn't return used descriptors to the ring"); + } } } - // Send notification once all the requests are processed - vring - .signal_used_queue() - .map_err(|_| Error::NotificationFailed)?; + Ok(true) + } + + /// Process the requests in the vring and dispatch replies + fn process_queue(&self, vring: &VringRwLock) -> Result { + let requests: Vec<_> = vring + .get_mut() + .get_queue_mut() + .iter() + .map_err(|_| Error::DescriptorNotFound)? + .collect(); + + if self.process_requests(requests, Some(vring))? { + // Send notification once all the requests are processed + vring + .signal_used_queue() + .map_err(|_| Error::NotificationFailed)?; + } + Ok(true) } } @@ -328,11 +352,365 @@ impl VhostUserBackendMut #[cfg(test)] mod tests { - use super::*; - use crate::i2c::tests::DummyDevice; - use crate::AdapterConfig; use std::convert::TryFrom; + use virtio_queue::defs::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE}; + use virtio_queue::{mock::MockSplitQueue, Descriptor}; + use vm_memory::{Address, GuestAddress, GuestMemoryAtomic, GuestMemoryMmap}; + + use super::Error; + use super::*; + use crate::i2c::tests::{update_rdwr_buf, verify_rdwr_buf, DummyDevice}; + use crate::AdapterConfig; + + // Prepares a single chain of descriptors + fn prepare_desc_chain( + start_addr: GuestAddress, + buf: &mut Vec, + flag: u32, + client_addr: u16, + ) -> I2cDescriptorChain { + let mem = GuestMemoryMmap::<()>::from_ranges(&[(start_addr, 0x1000)]).unwrap(); + let vq = MockSplitQueue::new(&mem, 16); + let mut next_addr = vq.desc_table().total_size() + 0x100; + let mut index = 0; + + // Out header descriptor + let out_hdr = VirtioI2cOutHdr { + addr: From::from(client_addr << 1), + padding: From::from(0x0), + flags: From::from(flag), + }; + + let desc_out = Descriptor::new( + next_addr, + size_of::() as u32, + VIRTQ_DESC_F_NEXT, + index + 1, + ); + + mem.write_obj::(out_hdr, desc_out.addr()) + .unwrap(); + vq.desc_table().store(index, desc_out); + next_addr += desc_out.len() as u64; + index += 1; + + // Buf descriptor: optional + if !buf.is_empty() { + // Set buffer is write-only or not + let flag = if (flag & VIRTIO_I2C_FLAGS_M_RD) == 0 { + update_rdwr_buf(buf); + 0 + } else { + VIRTQ_DESC_F_WRITE + }; + + let desc_buf = Descriptor::new( + next_addr, + buf.len() as u32, + flag | VIRTQ_DESC_F_NEXT, + index + 1, + ); + mem.write(buf, desc_buf.addr()).unwrap(); + vq.desc_table().store(index, desc_buf); + next_addr += desc_buf.len() as u64; + index += 1; + } + + // In response descriptor + let desc_in = Descriptor::new(next_addr, size_of::() as u32, VIRTQ_DESC_F_WRITE, 0); + vq.desc_table().store(index, desc_in); + + // Put the descriptor index 0 in the first available ring position. + mem.write_obj(0u16, vq.avail_addr().unchecked_add(4)) + .unwrap(); + + // Set `avail_idx` to 1. + mem.write_obj(1u16, vq.avail_addr().unchecked_add(2)) + .unwrap(); + + // Create descriptor chain from pre-filled memory + vq.create_queue(GuestMemoryAtomic::::new(mem.clone())) + .iter() + .unwrap() + .next() + .unwrap() + } + + // Validate descriptor chains after processing them, checks pass/failure of + // operation and the value of the buffers updated by the `DummyDevice`. + fn validate_desc_chains(desc_chains: Vec, status: u8) { + for desc_chain in desc_chains { + let descriptors: Vec<_> = desc_chain.clone().collect(); + + let in_hdr = desc_chain + .memory() + .read_obj::(descriptors[descriptors.len() - 1].addr()) + .unwrap(); + + // Operation result should match expected status. + assert_eq!(in_hdr.status, status); + + let out_hdr = desc_chain + .memory() + .read_obj::(descriptors[0].addr()) + .unwrap(); + + if (out_hdr.flags.to_native() & VIRTIO_I2C_FLAGS_M_RD) != 0 && descriptors.len() == 3 { + let mut buf = vec![0; descriptors[1].len() as usize]; + desc_chain + .memory() + .read(&mut buf, descriptors[1].addr()) + .unwrap(); + + // Verify the content of the read-buffer + verify_rdwr_buf(&buf); + } + } + } + + // Prepares list of dummy descriptors, their content isn't significant + fn prepare_desc_chain_dummy( + addr: Option>, + flags: Vec, + len: Vec, + ) -> I2cDescriptorChain { + let mem = GuestMemoryMmap::<()>::from_ranges(&[(GuestAddress(0), 0x1000)]).unwrap(); + let vq = MockSplitQueue::new(&mem, 16); + + for (i, flag) in flags.iter().enumerate() { + let mut f = if i == flags.len() - 1 { + 0 + } else { + VIRTQ_DESC_F_NEXT + }; + f |= flag; + + let offset = match addr { + Some(ref addr) => addr[i], + _ => 0x100, + }; + + let desc = Descriptor::new(offset, len[i], f, (i + 1) as u16); + vq.desc_table().store(i as u16, desc); + } + + // Put the descriptor index 0 in the first available ring position. + mem.write_obj(0u16, vq.avail_addr().unchecked_add(4)) + .unwrap(); + + // Set `avail_idx` to 1. + mem.write_obj(1u16, vq.avail_addr().unchecked_add(2)) + .unwrap(); + + // Create descriptor chain from pre-filled memory + vq.create_queue(GuestMemoryAtomic::::new(mem.clone())) + .iter() + .unwrap() + .next() + .unwrap() + } + + #[test] + fn process_requests_success() { + let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap(); + let i2c_map = I2cMap::::new(&device_config).unwrap(); + let backend = VhostUserI2cBackend::new(Arc::new(i2c_map)).unwrap(); + + // Descriptor chain size zero, shouldn't fail + backend + .process_requests(Vec::::new(), None) + .unwrap(); + + // Valid single read descriptor + let mut buf: Vec = vec![0; 30]; + let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, VIRTIO_I2C_FLAGS_M_RD, 4); + let desc_chains = vec![desc_chain]; + + backend.process_requests(desc_chains.clone(), None).unwrap(); + validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK); + + // Valid single write descriptor + let mut buf: Vec = vec![0; 30]; + let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, 0, 4); + let desc_chains = vec![desc_chain]; + + backend.process_requests(desc_chains.clone(), None).unwrap(); + validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK); + + // Valid mixed read-write descriptors + let mut buf: Vec> = vec![vec![0; 30]; 6]; + let desc_chains = vec![ + // Write + prepare_desc_chain(GuestAddress(0), &mut buf[0], 0, 4), + // Read + prepare_desc_chain(GuestAddress(0), &mut buf[1], VIRTIO_I2C_FLAGS_M_RD, 4), + // Write + prepare_desc_chain(GuestAddress(0), &mut buf[2], 0, 4), + // Read + prepare_desc_chain(GuestAddress(0), &mut buf[3], VIRTIO_I2C_FLAGS_M_RD, 4), + // Write + prepare_desc_chain(GuestAddress(0), &mut buf[4], 0, 4), + // Read + prepare_desc_chain(GuestAddress(0), &mut buf[5], VIRTIO_I2C_FLAGS_M_RD, 4), + ]; + + backend.process_requests(desc_chains.clone(), None).unwrap(); + validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_OK); + } + + #[test] + fn process_requests_failure() { + let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap(); + let i2c_map = I2cMap::::new(&device_config).unwrap(); + let backend = VhostUserI2cBackend::new(Arc::new(i2c_map)).unwrap(); + + // One descriptors + let flags: Vec = vec![0]; + let len: Vec = vec![0]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedDescriptorCount(1) + ); + + // Four descriptors + let flags: Vec = vec![0, 0, 0, 0]; + let len: Vec = vec![0, 0, 0, 0]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedDescriptorCount(4) + ); + + // Write only out hdr + let flags: Vec = vec![VIRTQ_DESC_F_WRITE, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![ + size_of::() as u32, + 1, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedWriteOnlyDescriptor(0) + ); + + // Invalid out hdr length + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![100, 1, size_of::() as u32]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedDescriptorSize(size_of::(), 100) + ); + + // Invalid out hdr address + let addr: Vec = vec![0x10000, 0, 0]; + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![ + size_of::() as u32, + 1, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::DescriptorReadFailed + ); + + // Read only in hdr + let flags: Vec = vec![0, 0, 0]; + let len: Vec = vec![ + size_of::() as u32, + 1, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedReadableDescriptor(2) + ); + + // Invalid in hdr length + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![size_of::() as u32, 1, 100]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedDescriptorSize(size_of::(), 100) + ); + + // Invalid in hdr address + let addr: Vec = vec![0, 0, 0x10000]; + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![ + size_of::() as u32, + 1, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::DescriptorWriteFailed + ); + + // Invalid buf length + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![ + size_of::() as u32, + 0, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(None, flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::UnexpectedDescriptorSize(1, 0) + ); + + // Invalid buf address + let addr: Vec = vec![0, 0x10000, 0]; + let flags: Vec = vec![0, 0, VIRTQ_DESC_F_WRITE]; + let len: Vec = vec![ + size_of::() as u32, + 1, + size_of::() as u32, + ]; + let desc_chain = prepare_desc_chain_dummy(Some(addr), flags, len); + assert_eq!( + backend + .process_requests(vec![desc_chain], None) + .unwrap_err(), + Error::DescriptorReadFailed + ); + + // Missing buffer for I2C rdwr transfer + let mut buf = Vec::::new(); + let desc_chain = prepare_desc_chain(GuestAddress(0), &mut buf, VIRTIO_I2C_FLAGS_M_RD, 4); + let desc_chains = vec![desc_chain]; + + backend.process_requests(desc_chains.clone(), None).unwrap(); + validate_desc_chains(desc_chains, VIRTIO_I2C_MSG_ERR); + } + #[test] fn verify_backend() { let device_config = AdapterConfig::try_from("1:4,2:32:21,5:10:23").unwrap();