virtio-device: extend VirtioDeviceActions trait

This commit enhances the VirtioDeviceActions Trait to allow
the VMM leverage the VirtIO MMIO implementation for not only
VirtIO devices but also for vhost and vhost-user devices.

Problems:
1) Since the device configuration space can be managed by various
handlers outside of the VMM, such as vhost-user devices when the
VHOST_USER_PROTOCOL_F_CONFIG feature is negotiated between the
backend and the VMM, we need to invoke dedicated logic instead
of performing the MMIO operations through the vm-virtio workspace.
2) When the driver completes the negotiation of the driver features
with the VMM, , selecting page zero within the DriverFeatures MMIO
field, we need to negotiate both the acknowledged features and the
respective protocol features with the vhost/vhost-user backend devices.
Otherwise, the device will not be prepared to support, for example,
multiple queues and configuration space reads and writes.
3) When the backend device sends an interrupt to the frontend driver,
it needs to write the interrupt status so that later the driver
determines the type of notification — whether it is a used
buffer or a configuration change notification.
Since the responsibility for managing the interrupt status lies with the
VMM through a normal MMIO write/read, if the device is not implemented
within the VMM, it cannot manage the interrupt status field.

Solution:
1) Added `read_config` and `write_config` methods to allow the VMM
to instead of executing MMIO writes and reads within the vm-virtio
workspace, using the set_config and get_config methods provided by the
Rust-VMM vhost workspace.
2) Added the `negotiate_driver_features` method to allow the VMM
to exchange the driver and protocol features with the backend
device.
3) Added the `interrupt_status` method to allow the VMM to,
for example, read the configuration space from the backend device
and check for any changes. If the device configuration space has
changed, the VMM may update the interrupt status register by setting
bit 1. Conversely, if no changes are detected, the VMM may set the
bit 0, signaling a used buffer notifcation.
This applies to the backends that do not use the
VHOST_USER_BACKEND_CONFIG_CHANGE_MSG message to notify
about configuration changes (which I think is the case for all
Rust-VMM vhost-user backends).
Otherwise, the VMM can simply handle the message and update the
interrupt status register accordingly.

Signed-off-by: João Peixoto <[email protected]>

Author: joaopeixoto13

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<VirtioConfig>`.
-pub trait VirtioDeviceActions {
+pub trait VirtioDeviceActions: BorrowMut<VirtioConfig<Queue>> {
     /// 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<AtomicU8> {
+        &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 {
+            <Self as VirtioDeviceActions>::negotiate_driver_features(self);
         }
     }
 
@@ -150,41 +206,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)
     }
 }
 
@@ -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<Queue>,
@@ -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);