From 1140db059f44f32c69e903b687ee56e508b80af8 Mon Sep 17 00:00:00 2001 From: joaopeixoto13 Date: Sun, 25 Feb 2024 15:38:02 +0000 Subject: [PATCH] virtio-device: extend VirtioDeviceActions trait This commit enhances the VirtioDeviceActions Trait to accommodate Vhost and Vhost-User devices effectively. It introduces four new methods to the VirtioDeviceActions trait to facilitate seamless interaction with these devices: - `read_config` and `write_config`: These methods are invoked when the driver intends to read from or write to the device configuration space. Given that the device configuration space can be managed by various handlers outside of the VMM, such as vhost-user when the protocol feature CONFIG is negotiated, dedicated logic is necessary to handle these operations (e.g. GET_CONFIG/SET_CONFIG requests). - `negotiate_driver_features`: This method is called when the driver finishes the negotiation of the device features with the frontend device (selecting page 0). This method is crucial when the device handler is implemented outside of the VMM since the frontend device needs to negotiate the features with the backend device. Otherwise, the device will not be prepared to support, for example, multiple queues and configuration space reads and writes. - `interrupt_status`: When the driver requires reading the interrupt status from the device, this method is invoked. Since the responsibility for managing interrupt status lies with the frontend device, specialized logic is needed to update the interrupt status appropriately (Used Buffer Notification or Configuration Change Notification). If the device is implemented within the VMM, the interrupt status is direct management and updating by the device. Signed-off-by: joaopeixoto13 --- virtio-device/src/virtio_config.rs | 98 +++++++++++++++++++++--------- 1 file changed, 68 insertions(+), 30 deletions(-) diff --git a/virtio-device/src/virtio_config.rs b/virtio-device/src/virtio_config.rs index e30b3895..8b7deaec 100644 --- a/virtio-device/src/virtio_config.rs +++ b/virtio-device/src/virtio_config.rs @@ -76,7 +76,7 @@ pub trait VirtioDeviceType { /// Helper trait that can be implemented for objects which represent virtio devices. Together /// with `VirtioDeviceType`, it enables an automatic `VirtioDevice` implementation for objects /// that also implement `BorrowMut`. -pub trait VirtioDeviceActions { +pub trait VirtioDeviceActions: BorrowMut> { /// Type of the error that can be returned by `activate` and `reset`. type E; @@ -85,6 +85,58 @@ pub trait VirtioDeviceActions { /// Invoke the logic associated with resetting this device. fn reset(&mut self) -> result::Result<(), Self::E>; + + /// Invoke the logic associated with reading from the device configuration space. + /// + /// A default implementation is provided as we cannot expect all backends to implement + /// this function. + fn read_config(&self, offset: usize, data: &mut [u8]) { + let config_space = &self.borrow().config_space; + let config_len = config_space.len(); + if offset >= config_len { + error!("Failed to read from config space"); + return; + } + + // TODO: Are partial reads ok? + let end = cmp::min(offset.saturating_add(data.len()), config_len); + let read_len = end - offset; + // Cannot fail because the lengths are identical and we do bounds checking beforehand. + data[..read_len].copy_from_slice(&config_space[offset..end]) + } + + /// Invoke the logic associated with writing to the device configuration space. + /// + /// A default implementation is provided as we cannot expect all backends to implement + /// this function. + fn write_config(&mut self, offset: usize, data: &[u8]) { + let config_space = &mut self.borrow_mut().config_space; + let config_len = config_space.len(); + if offset >= config_len { + error!("Failed to write to config space"); + return; + } + + // TODO: Are partial writes ok? + let end = cmp::min(offset.saturating_add(data.len()), config_len); + let write_len = end - offset; + // Cannot fail because the lengths are identical and we do bounds checking beforehand. + config_space[offset..end].copy_from_slice(&data[..write_len]); + } + + /// Invoke the logic associated with negotiating the driver features. + /// + /// A default implementation is provided as we cannot expect all backends to implement + /// this function. + fn negotiate_driver_features(&mut self) {} + + /// Invoke the logic associated with the device interrupt status. + /// + /// A default implementation is provided as we cannot expect all backends to implement + /// this function. + fn interrupt_status(&self) -> &Arc { + &self.borrow().interrupt_status + } } // We can automatically implement the `VirtioDevice` trait for objects that only explicitly @@ -130,6 +182,10 @@ where 1 => ((features << 32) >> 32) + (v << 32), // Accessing an unknown page has no effect. _ => features, + }; + + if page == 0 { + ::negotiate_driver_features(self); } } @@ -150,7 +206,7 @@ where } fn interrupt_status(&self) -> &Arc { - &self.borrow().interrupt_status + ::interrupt_status(self) } fn config_generation(&self) -> u8 { @@ -158,33 +214,11 @@ where } fn read_config(&self, offset: usize, data: &mut [u8]) { - let config_space = &self.borrow().config_space; - let config_len = config_space.len(); - if offset >= config_len { - error!("Failed to read from config space"); - return; - } - - // TODO: Are partial reads ok? - let end = cmp::min(offset.saturating_add(data.len()), config_len); - let read_len = end - offset; - // Cannot fail because the lengths are identical and we do bounds checking beforehand. - data[..read_len].copy_from_slice(&config_space[offset..end]) + ::read_config(self, offset, data) } fn write_config(&mut self, offset: usize, data: &[u8]) { - let config_space = &mut self.borrow_mut().config_space; - let config_len = config_space.len(); - if offset >= config_len { - error!("Failed to write to config space"); - return; - } - - // TODO: Are partial writes ok? - let end = cmp::min(offset.saturating_add(data.len()), config_len); - let write_len = end - offset; - // Cannot fail because the lengths are identical and we do bounds checking beforehand. - config_space[offset..end].copy_from_slice(&data[..write_len]); + ::write_config(self, offset, data) } } @@ -222,6 +256,7 @@ pub(crate) mod tests { use super::*; use crate::mmio::VirtioMmioDevice; use std::borrow::Borrow; + use std::sync::atomic::Ordering; pub struct Dummy { pub cfg: VirtioConfig, @@ -320,10 +355,10 @@ pub(crate) mod tests { let mut v2 = vec![0u8; len]; // Offset to large to read anything. - d.read_config(len, v2.as_mut_slice()); + VirtioDevice::read_config(&d, len, v2.as_mut_slice()); assert_eq!(v1, v2); - d.read_config(len / 2, v2.as_mut_slice()); + VirtioDevice::read_config(&d, len / 2, v2.as_mut_slice()); for i in 0..len { if i < len / 2 { assert_eq!(v2[i], config_space[len / 2 + i]); @@ -333,10 +368,10 @@ pub(crate) mod tests { } // Offset too large to overwrite anything. - d.write_config(len, v1.as_slice()); + VirtioDevice::write_config(&mut d, len, v1.as_slice()); assert_eq!(d.cfg.config_space, config_space); - d.write_config(len / 2, v1.as_slice()); + VirtioDevice::write_config(&mut d, len / 2, v1.as_slice()); for (i, &value) in config_space.iter().enumerate().take(len) { if i < len / 2 { assert_eq!(d.cfg.config_space[i], value); @@ -345,6 +380,9 @@ pub(crate) mod tests { } } + d.cfg.interrupt_status.fetch_or(1, Ordering::SeqCst); + assert_eq!(VirtioDevice::interrupt_status(&d).load(Ordering::SeqCst), 1); + // Let's test the `WithDriverSelect` auto impl now. assert_eq!(d.queue_select(), 0); d.set_queue_select(1);