vm-virtio: Simplify virtio feature handling

Remove duplicated code across the different devices by handling
the virtio feature pages in VirtioDevice itself rather than
in the backends. This works as no virtio devices use feature
bits beyond 64-bits.

Signed-off-by: Cathy Zhang <cathy.zhang@intel.com>

vm-virtio/src/block.rs

@@ -893,29 +893,12 @@ impl<T: 'static + DiskFile + Send> VirtioDevice for Block<T> {
         self.queue_size.as_slice()
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/console.rs

@@ -417,29 +417,12 @@ impl VirtioDevice for Console {
         QUEUE_SIZES
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/device.rs

@@ -62,14 +62,15 @@ pub trait VirtioDevice: Send {
     /// The maximum size of each queue that this device supports.
     fn queue_max_sizes(&self) -> &[u16];
 
-    /// The set of feature bits shifted by `page * 32`.
-    fn features(&self, page: u32) -> u32 {
-        let _ = page;
+    /// The set of feature bits that this device supports.
+    fn features(&self) -> u64 {
         0
     }
 
     /// Acknowledges that this set of features should be enabled.
-    fn ack_features(&mut self, page: u32, value: u32);
+    fn ack_features(&mut self, value: u64) {
+        let _ = value;
+    }
 
     /// Reads this device configuration space at `offset`.
     fn read_config(&self, offset: u64, data: &mut [u8]);

vm-virtio/src/iommu.rs

@@ -897,29 +897,12 @@ impl VirtioDevice for Iommu {
         QUEUE_SIZES
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/net.rs

@@ -388,29 +388,12 @@ impl VirtioDevice for Net {
         &self.queue_size.as_slice()
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page: {}", page);
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page: {}", page);
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/pmem.rs

@@ -368,29 +368,12 @@ impl VirtioDevice for Pmem {
         QUEUE_SIZES
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/rng.rs

@@ -230,29 +230,12 @@ impl VirtioDevice for Rng {
         QUEUE_SIZES
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/transport/mmio.rs

@@ -189,8 +189,12 @@ impl BusDevice for MmioDevice {
                     0x08 => self.device.lock().unwrap().device_type(),
                     0x0c => VENDOR_ID, // vendor id
                     0x10 => {
-                        self.device.lock().unwrap().features(self.features_select)
-                            | if self.features_select == 1 { 0x1 } else { 0x0 }
+                        if self.features_select < 2 {
+                            (self.device.lock().unwrap().features() >> (self.features_select * 32))
+                                as u32
+                        } else {
+                            0
+                        }
                     }
                     0x34 => self.with_queue(0, |q| u32::from(q.get_max_size())),
                     0x44 => self.with_queue(0, |q| q.ready as u32),
@@ -234,11 +238,19 @@ impl BusDevice for MmioDevice {
                 let v = LittleEndian::read_u32(data);
                 match offset {
                     0x14 => self.features_select = v,
-                    0x20 => self
-                        .device
-                        .lock()
-                        .unwrap()
-                        .ack_features(self.acked_features_select, v),
+                    0x20 => {
+                        if self.acked_features_select < 2 {
+                            self.device
+                                .lock()
+                                .unwrap()
+                                .ack_features(u64::from(v) << (self.acked_features_select * 32));
+                        } else {
+                            warn!(
+                                "invalid ack_features (page {}, value 0x{:x})",
+                                self.acked_features_select, v
+                            );
+                        }
+                    }
                     0x24 => self.acked_features_select = v,
                     0x30 => self.queue_select = v,
                     0x38 => mut_q = self.with_queue_mut(|q| q.size = v as u16),

vm-virtio/src/transport/pci_common_config.rs

@@ -164,7 +164,7 @@ impl VirtioPciCommonConfig {
                 // Only 64 bits of features (2 pages) are defined for now, so limit
                 // device_feature_select to avoid shifting by 64 or more bits.
                 if self.device_feature_select < 2 {
-                    locked_device.features(self.device_feature_select)
+                    (locked_device.features() >> (self.device_feature_select * 32)) as u32
                 } else {
                     0
                 }
@@ -199,7 +199,8 @@ impl VirtioPciCommonConfig {
             0x0c => {
                 if self.driver_feature_select < 2 {
                     let mut locked_device = device.lock().unwrap();
-                    locked_device.ack_features(self.driver_feature_select, value);
+                    locked_device
+                        .ack_features(u64::from(value) << (self.driver_feature_select * 32));
                 } else {
                     warn!(
                         "invalid ack_features (page {}, value 0x{:x})",
@@ -280,11 +281,11 @@ mod tests {
             Ok(())
         }
 
-        fn features(&self, _page: u32) -> u32 {
-            DUMMY_FEATURES as u32
+        fn features(&self) -> u64 {
+            DUMMY_FEATURES
         }
 
-        fn ack_features(&mut self, _page: u32, _value: u32) {}
+        fn ack_features(&mut self, _value: u64) {}
 
         fn read_config(&self, _offset: u64, _data: &mut [u8]) {}
 

vm-virtio/src/vhost_user/blk.rs

@@ -171,27 +171,12 @@ impl VirtioDevice for Blk {
         &self.queue_sizes
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            0 => self.avail_features as u32,
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page: {}", page);
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page: {}", page);
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/vhost_user/fs.rs

@@ -284,29 +284,12 @@ impl VirtioDevice for Fs {
         &self.queue_sizes.as_slice()
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("fs: Received request for unknown features page: {}", page);
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("fs: Cannot acknowledge unknown features page: {}", page);
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/vhost_user/net.rs

@@ -177,27 +177,12 @@ impl VirtioDevice for Net {
         &self.queue_sizes
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            0 => self.avail_features as u32,
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page: {}", page);
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page: {}", page);
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {

vm-virtio/src/vsock/device.rs

@@ -439,29 +439,12 @@ where
         QUEUE_SIZES
     }
 
-    fn features(&self, page: u32) -> u32 {
-        match page {
-            // Get the lower 32-bits of the features bitfield.
-            0 => self.avail_features as u32,
-            // Get the upper 32-bits of the features bitfield.
-            1 => (self.avail_features >> 32) as u32,
-            _ => {
-                warn!("Received request for unknown features page.");
-                0u32
-            }
-        }
+    fn features(&self) -> u64 {
+        self.avail_features
     }
 
-    fn ack_features(&mut self, page: u32, value: u32) {
-        let mut v = match page {
-            0 => u64::from(value),
-            1 => u64::from(value) << 32,
-            _ => {
-                warn!("Cannot acknowledge unknown features page.");
-                0u64
-            }
-        };
-
+    fn ack_features(&mut self, value: u64) {
+        let mut v = value;
         // Check if the guest is ACK'ing a feature that we didn't claim to have.
         let unrequested_features = v & !self.avail_features;
         if unrequested_features != 0 {
@@ -619,18 +602,15 @@ mod tests {
             VirtioDeviceType::TYPE_VSOCK as u32
         );
         assert_eq!(ctx.device.queue_max_sizes(), QUEUE_SIZES);
-        assert_eq!(ctx.device.features(0), device_pages[0]);
-        assert_eq!(ctx.device.features(1), device_pages[1]);
-        assert_eq!(ctx.device.features(2), 0);
+        assert_eq!((ctx.device.features() >> (0 * 32)) as u32, device_pages[0]);
+        assert_eq!((ctx.device.features() >> (1 * 32)) as u32, device_pages[1]);
 
         // Ack device features, page 0.
-        ctx.device.ack_features(0, driver_pages[0]);
+        ctx.device
+            .ack_features(u64::from(driver_pages[0]) << (0 * 32));
         // Ack device features, page 1.
-        ctx.device.ack_features(1, driver_pages[1]);
-        // Ack some bogus page (i.e. 2). This should have no side effect.
-        ctx.device.ack_features(2, 0);
-        // Attempt to un-ack the first feature page. This should have no side effect.
-        ctx.device.ack_features(0, !driver_pages[0]);
+        ctx.device
+            .ack_features(u64::from(driver_pages[1]) << (1 * 32));
         // Check that no side effect are present, and that the acked features are exactly the same
         // as the device features.
         assert_eq!(ctx.device.acked_features, device_features & driver_features);