From fedfb0b82ef6e5da6a5378284b4a89822c4d867c Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 22 Oct 2021 12:00:24 +0530 Subject: [PATCH 1/4] [i2c] Use hashmap for device_map Use hashmap instead of an array of size 1 << 7, as only a few entries of the array are used here. Signed-off-by: Viresh Kumar --- src/i2c/src/i2c.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/i2c/src/i2c.rs b/src/i2c/src/i2c.rs index b026de6..a1f8a47 100644 --- a/src/i2c/src/i2c.rs +++ b/src/i2c/src/i2c.rs @@ -6,6 +6,7 @@ // SPDX-License-Identifier: Apache-2.0 use log::info; +use std::collections::HashMap; use std::fs::{File, OpenOptions}; use std::os::unix::io::AsRawFd; @@ -444,11 +445,10 @@ impl I2cAdapter { /// I2C map and helpers pub(crate) const MAX_I2C_VDEV: usize = 1 << 7; -const I2C_INVALID_ADAPTER: u32 = 0xFFFFFFFF; pub struct I2cMap { adapters: Vec>, - device_map: [u32; MAX_I2C_VDEV], + device_map: HashMap, } impl I2cMap { @@ -456,7 +456,7 @@ impl I2cMap { where Self: Sized, { - let mut device_map: [u32; MAX_I2C_VDEV] = [I2C_INVALID_ADAPTER; MAX_I2C_VDEV]; + let mut device_map = HashMap::new(); let mut adapters: Vec> = Vec::new(); for (i, device_cfg) in device_config.inner.iter().enumerate() { @@ -466,7 +466,7 @@ impl I2cMap { // Check that all addresses corresponding to the adapter are valid. for addr in &device_cfg.addr { adapter.set_device_addr(*addr as usize)?; - device_map[*addr as usize] = i as u32; + device_map.insert(*addr, i); } info!( @@ -484,21 +484,21 @@ impl I2cMap { } pub fn transfer(&self, reqs: &mut [I2cReq]) -> Result<()> { - let device = reqs[0].addr as usize; + let device = reqs[0].addr; // identify the device in the device_map - let index = self.device_map[device]; + let index = match self.device_map.get(&device) { + Some(&index) => index, - // This can happen a lot while scanning the bus, don't print any errors. - if index == I2C_INVALID_ADAPTER { - return Err(Error::ClientAddressInvalid); - } + // This can happen a lot while scanning the bus, don't print any errors. + None => return Err(Error::ClientAddressInvalid), + }; // get the corresponding adapter based on the device config. let adapter = &self.adapters[index as usize]; // Set device's address - adapter.set_device_addr(device)?; + adapter.set_device_addr(device as usize)?; adapter.transfer(reqs) } } @@ -562,11 +562,11 @@ pub mod tests { assert_eq!(i2c_map.adapters[1].adapter_no(), 2); assert_eq!(i2c_map.adapters[2].adapter_no(), 5); - assert_eq!(i2c_map.device_map[4], 0); - assert_eq!(i2c_map.device_map[32], 1); - assert_eq!(i2c_map.device_map[21], 1); - assert_eq!(i2c_map.device_map[10], 2); - assert_eq!(i2c_map.device_map[23], 2); + assert_eq!(i2c_map.device_map.get(&4), Some(&0)); + assert_eq!(i2c_map.device_map.get(&32), Some(&1)); + assert_eq!(i2c_map.device_map.get(&21), Some(&1)); + assert_eq!(i2c_map.device_map.get(&10), Some(&2)); + assert_eq!(i2c_map.device_map.get(&23), Some(&2)); } #[test] From 91778ed00901da9ea3f538dfa1f758bfce99192c Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 8 Nov 2021 09:23:44 +0530 Subject: [PATCH 2/4] i2c: Remove unused `mem` field Removed the unused `mem` field. Signed-off-by: Viresh Kumar --- src/i2c/src/vhu_i2c.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/i2c/src/vhu_i2c.rs b/src/i2c/src/vhu_i2c.rs index 6c784da..5b0dc80 100644 --- a/src/i2c/src/vhu_i2c.rs +++ b/src/i2c/src/vhu_i2c.rs @@ -86,7 +86,6 @@ unsafe impl ByteValued for VirtioI2cInHdr {} pub struct VhostUserI2cBackend { i2c_map: Arc>, event_idx: bool, - mem: Option>, pub exit_event: EventFd, } @@ -95,7 +94,6 @@ impl VhostUserI2cBackend { Ok(VhostUserI2cBackend { i2c_map, event_idx: false, - mem: None, exit_event: EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdFailed)?, }) } @@ -249,9 +247,8 @@ impl VhostUserBackendMut fn update_memory( &mut self, - mem: GuestMemoryAtomic, + _mem: GuestMemoryAtomic, ) -> VhostUserBackendResult<()> { - self.mem = Some(mem); Ok(()) } From cf18bc7409c1f3c8067ddbd43fa601a836cfa552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 2 Nov 2021 15:24:39 +0000 Subject: [PATCH 3/4] CODEOWNERS: add myself an Mathieu to aid review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will help with review of incoming stuff although we may need to lean on the wider community when it comes to more in depth Rust stuff. Signed-off-by: Alex Bennée --- CODEOWNERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CODEOWNERS b/CODEOWNERS index ca3b677..3316996 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1,2 +1,2 @@ # Add the list of code owners here (using their GitHub username) -* gatekeeper-PullAssigner @vireshk +* gatekeeper-PullAssigner @vireshk @stsquad @mathieupoirier From 31ec556560f000728aa66adaa1d2afdcc67609ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 2 Nov 2021 15:51:48 +0000 Subject: [PATCH 4/4] README.md: document aim for separating concerns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While there may be some work to do on the testing framework to make proper use of vm-virtio we should at least document the desire for modularity. Signed-off-by: Alex Bennée --- README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/README.md b/README.md index 5abdf26..9eee6c9 100644 --- a/README.md +++ b/README.md @@ -9,3 +9,20 @@ crates. Here is the list of device backends that we support: - [I2C](https://github.com/rust-vmm/vhost-device/blob/master/src/i2c/README.md) + +## Separation of Concerns + +The binaries built by this repository can be run with any VMM which +can act as a vhost-user master. Typically they have been tested with +[QEMU](https://www.qemu.org) although the rust-vmm project does +provide a [vhost-user +master](https://github.com/rust-vmm/vhost/tree/main/src/vhost_user) +crate for rust based VMMs. + +While it's possible to implement all parts of the backend inside the +vhost-device workspace consideration should be given to separating the +VirtQueue handling and response logic to a crate in [vm-virtio +devices](https://github.com/rust-vmm/vm-virtio/tree/main/crates/devices). +This way a monolithic rust-vmm VMM implementation can reuse the core +logic to service the virtio requests directly in the application. +