virtio-device: extend VirtioDeviceActions Trait

This commit enhances the VirtioDeviceActions Trait to accommodate
Vhost and Vhost-User devices. This way, this crate can be used
not only to implement virtio abstractions for devices that are
implemented inside the VMM, but it can also serve as a frontend
virtio device abstraction for Vhost and Vhost-user type devices.
This patch introduces four new methods to the VirtioDeviceActions:

- `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 or vhost-user,
dedicated logic is necessary to handle these operations.
- `negotiate_driver_features`:  This method is called when the
driver finishes the negotiation of the driver 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 some
driver features.
- `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 <[email protected]>

virtio-device/src/virtio_config.rs

@@ -3,13 +3,10 @@
 // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
 
 use std::borrow::BorrowMut;
-use std::cmp;
 use std::result;
 use std::sync::atomic::AtomicU8;
 use std::sync::Arc;
 
-use log::error;
-
 use crate::{VirtioDevice, WithDriverSelect};
 use virtio_queue::{Queue, QueueT};
 
@@ -85,6 +82,18 @@ 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.
+    fn read_config(&self, offset: usize, data: &mut [u8]);
+
+    /// Invoke the logic associated with writing to the device configuration space.
+    fn write_config(&mut self, offset: usize, data: &[u8]);
+
+    /// Invoke the logic associated with negotiating the driver features.
+    fn negotiate_driver_features(&mut self);
+
+    /// Invoke the logic associated with the device interrupt status.
+    fn interrupt_status(&self) -> &Arc<AtomicU8>;
 }
 
 // We can automatically implement the `VirtioDevice` trait for objects that only explicitly
@@ -130,6 +139,10 @@ where
             1 => ((features << 32) >> 32) + (v << 32),
             // Accessing an unknown page has no effect.
             _ => features,
+        };
+
+        if page == 0 {
+            <Self as VirtioDeviceActions>::negotiate_driver_features(self);
         }
     }
 
@@ -150,41 +163,19 @@ where
     }
 
     fn interrupt_status(&self) -> &Arc<AtomicU8> {
-        &self.borrow().interrupt_status
+        <Self as VirtioDeviceActions>::interrupt_status(self)
     }
 
     fn config_generation(&self) -> u8 {
         self.borrow().config_generation
     }
 
     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])
+        <Self as VirtioDeviceActions>::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]);
+        <Self as VirtioDeviceActions>::write_config(self, offset, data)
     }
 }
 
@@ -221,7 +212,10 @@ where
 pub(crate) mod tests {
     use super::*;
     use crate::mmio::VirtioMmioDevice;
+    use log::error;
     use std::borrow::Borrow;
+    use std::cmp;
+    use std::sync::atomic::Ordering;
 
     pub struct Dummy {
         pub cfg: VirtioConfig<Queue>,
@@ -276,6 +270,42 @@ pub(crate) mod tests {
             self.reset_count += 1;
             Ok(())
         }
+
+        fn read_config(&self, offset: usize, data: &mut [u8]) {
+            let config_space = &self.cfg.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])
+        }
+
+        fn write_config(&mut self, offset: usize, data: &[u8]) {
+            let config_space = &mut self.cfg.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]);
+        }
+
+        fn negotiate_driver_features(&mut self) {}
+
+        fn interrupt_status(&self) -> &Arc<AtomicU8> {
+            &self.cfg.interrupt_status
+        }
     }
 
     impl VirtioMmioDevice for Dummy {
@@ -320,10 +350,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 +363,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 +375,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);