From fbab2abc79b7cd055376f431e81500aefa7637b3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Thu, 21 Oct 2021 12:26:55 +0530 Subject: [PATCH] [i2c] Allow zero-length requests Commands like SMBUS Quick don't require a buffer for the request and are called as zero-length requests. The specification allows such requests under the VIRTIO_I2C_F_ZERO_LENGTH_REQUEST feature, which is mandatory to be implemented by the devices now. Add support for zero-length requests. Signed-off-by: Viresh Kumar --- coverage_config_x86_64.json | 2 +- src/i2c/src/i2c.rs | 22 ++++++++- src/i2c/src/vhu_i2c.rs | 95 ++++++++++++++++++++++++------------- 3 files changed, 82 insertions(+), 37 deletions(-) diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 1e0172f..d6ba776 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 59.8, + "coverage_score": 58.4, "exclude_path": "", "crate_features": "" } diff --git a/src/i2c/src/i2c.rs b/src/i2c/src/i2c.rs index cb0e6a6..4662270 100644 --- a/src/i2c/src/i2c.rs +++ b/src/i2c/src/i2c.rs @@ -34,6 +34,8 @@ pub enum Error { SMBusCommandInvalid(u32), #[error("Invalid SMBus transfer, request-count: {0}, req[0].len: {1}, req[1].len: {2}")] SMBusTransferInvalid(usize, u16, u16), + #[error("Invalid I2C transfer, request with invalid count: {0}")] + I2cTransferInvalid(usize), #[error("Failed to open adapter at /dev/i2c-{0}")] DeviceOpenFailed(u32), #[error("Ioctl command failed for {0} operation: {1}")] @@ -194,7 +196,7 @@ impl SmbusMsg { // Special Read requests 0 => Ok(SmbusMsg { read_write, - command: reqs[0].buf[0], + command: 0, size: I2C_SMBUS_QUICK, data: None, }), @@ -356,6 +358,10 @@ impl I2cDevice for PhysDevice { let len = reqs.len(); for req in reqs { + if req.len == 0 { + return Err(Error::I2cTransferInvalid(0)); + } + msgs.push(I2cMsg { addr: req.addr, flags: req.flags, @@ -745,7 +751,7 @@ pub mod tests { addr: 0x3, flags: 0, len: 0, - buf: vec![0], + buf: Vec::::new(), }]; i2c_map.transfer(&mut reqs).unwrap(); @@ -1026,6 +1032,18 @@ pub mod tests { Error::IoctlFailure("rdwr", IoError::last()) ); + // rdwr failure - missing buffer + let mut reqs = [I2cReq { + addr: 0x4, + flags: 0, + len: 0, + buf: Vec::::new(), + }]; + assert_eq!( + dev.rdwr(&mut reqs).unwrap_err(), + Error::I2cTransferInvalid(0) + ); + // smbus failure let mut data = SmbusMsg { read_write: 0, diff --git a/src/i2c/src/vhu_i2c.rs b/src/i2c/src/vhu_i2c.rs index 85f9140..f5ff3ea 100644 --- a/src/i2c/src/vhu_i2c.rs +++ b/src/i2c/src/vhu_i2c.rs @@ -23,6 +23,9 @@ use vmm_sys_util::eventfd::{EventFd, EFD_NONBLOCK}; use crate::i2c::*; +/// Virtio I2C Feature bits +const VIRTIO_I2C_F_ZERO_LENGTH_REQUEST: u16 = 0; + const QUEUE_SIZE: usize = 1024; const NUM_QUEUES: usize = 1; @@ -77,6 +80,9 @@ struct VirtioI2cOutHdr { } unsafe impl ByteValued for VirtioI2cOutHdr {} +/// VirtioI2cOutHdr Flags +const VIRTIO_I2C_FLAGS_M_RD: u32 = 1 << 1; + #[derive(Copy, Clone, Default)] #[repr(C)] struct VirtioI2cInHdr { @@ -118,7 +124,7 @@ impl VhostUserI2cBackend { for desc_chain in requests.clone() { let descriptors: Vec<_> = desc_chain.clone().collect(); - if descriptors.len() != 3 { + if (descriptors.len() != 2) && (descriptors.len() != 3) { return Err(Error::UnexpectedDescriptorCount); } @@ -131,12 +137,17 @@ impl VhostUserI2cBackend { return Err(Error::UnexpectedDescriptorSize); } - let desc_buf = descriptors[1]; - if desc_buf.len() == 0 { - return Err(Error::UnexpectedDescriptorSize); - } + let out_hdr = desc_chain + .memory() + .read_obj::(desc_out_hdr.addr()) + .map_err(|_| Error::DescriptorReadFailed)?; - let desc_in_hdr = descriptors[2]; + let flags = match out_hdr.flags.to_native() & VIRTIO_I2C_FLAGS_M_RD { + VIRTIO_I2C_FLAGS_M_RD => I2C_M_RD, + _ => 0, + }; + + let desc_in_hdr = descriptors[descriptors.len() - 1]; if !desc_in_hdr.is_write_only() { return Err(Error::UnexpectedReadableDescriptor); } @@ -145,28 +156,40 @@ impl VhostUserI2cBackend { return Err(Error::UnexpectedDescriptorSize); } - let mut buf: Vec = vec![0; desc_buf.len() as usize]; + let (buf, len) = match descriptors.len() { + // Buffer is available + 3 => { + let desc_buf = descriptors[1]; + let len = desc_buf.len(); - let mut flags: u16 = 0; + if len == 0 { + return Err(Error::UnexpectedDescriptorSize); + } + let mut buf = vec![0; len as usize]; - if desc_buf.is_write_only() { - flags = I2C_M_RD; - } else { - desc_chain - .memory() - .read(&mut buf, desc_buf.addr()) - .map_err(|_| Error::DescriptorReadFailed)?; - } + if flags != I2C_M_RD { + if desc_buf.is_write_only() { + return Err(Error::UnexpectedWriteOnlyDescriptor); + } - let out_hdr = desc_chain - .memory() - .read_obj::(desc_out_hdr.addr()) - .map_err(|_| Error::DescriptorReadFailed)?; + desc_chain + .memory() + .read(&mut buf, desc_buf.addr()) + .map_err(|_| Error::DescriptorReadFailed)?; + } else if !desc_buf.is_write_only() { + return Err(Error::UnexpectedReadableDescriptor); + } + + (buf, len) + } + + _ => (Vec::::new(), 0), + }; reqs.push(I2cReq { addr: out_hdr.addr.to_native() >> 1, flags, - len: desc_buf.len() as u16, + len: len as u16, buf, }); } @@ -182,16 +205,23 @@ impl VhostUserI2cBackend { for (i, desc_chain) in requests.iter().enumerate() { let descriptors: Vec<_> = desc_chain.clone().collect(); - let desc_buf = descriptors[1]; - let desc_in_hdr = descriptors[2]; + let desc_in_hdr = descriptors[descriptors.len() - 1]; let mut len = size_of::() as u32; - // Write the data read from the I2C device - if desc_buf.is_write_only() { - desc_chain - .memory() - .write(&reqs[i].buf, desc_buf.addr()) - .map_err(|_| Error::DescriptorWriteFailed)?; + if descriptors.len() == 3 { + let desc_buf = descriptors[1]; + + // Write the data read from the I2C device + if reqs[i].flags == I2C_M_RD { + desc_chain + .memory() + .write(&reqs[i].buf, desc_buf.addr()) + .map_err(|_| Error::DescriptorWriteFailed)?; + } + + if in_hdr.status == VIRTIO_I2C_MSG_OK { + len += desc_buf.len(); + } } // Write the transfer status @@ -200,10 +230,6 @@ impl VhostUserI2cBackend { .write_obj::(in_hdr, desc_in_hdr.addr()) .map_err(|_| Error::DescriptorWriteFailed)?; - if in_hdr.status == VIRTIO_I2C_MSG_OK { - len += desc_buf.len(); - } - if vring.add_used(desc_chain.head_index(), len).is_err() { warn!("Couldn't return used descriptors to the ring"); } @@ -235,6 +261,7 @@ impl VhostUserBackendMut | 1 << VIRTIO_F_NOTIFY_ON_EMPTY | 1 << VIRTIO_RING_F_INDIRECT_DESC | 1 << VIRTIO_RING_F_EVENT_IDX + | 1 << VIRTIO_I2C_F_ZERO_LENGTH_REQUEST | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits() } @@ -314,7 +341,7 @@ mod tests { assert_eq!(backend.num_queues(), NUM_QUEUES); assert_eq!(backend.max_queue_size(), QUEUE_SIZE); - assert_eq!(backend.features(), 0x171000000); + assert_eq!(backend.features(), 0x171000001); assert_eq!(backend.protocol_features(), VhostUserProtocolFeatures::MQ); assert_eq!(backend.queues_per_thread(), vec![0xffff_ffff]);