devices: make all virtio devices report version 1

Our virtio devices are all "modern" (no legacy/transitional support).
Add VIRTIO_F_VERSION_1 to the features() handler for all virtio devices
that didn't already have it.

This lets us remove the hack that forced VIRTIO_F_VERSION_1 on for all
devices.

BUG=None
TEST=build_test; boot crosvm on kevin

Change-Id: I008926a9075679aae46069aa37a14504f10e8584
Signed-off-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1313013
Reviewed-by: Zach Reizner <zachr@chromium.org>

5 files changed, 19 insertions, 14 deletions

diff --git a/devices/src/virtio/balloon.rs b/devices/src/virtio/balloon.rs
index ffb7aab..168e0c4 100644
--- a/devices/src/virtio/balloon.rs
+++ b/devices/src/virtio/balloon.rs
@@ -17,7 +17,7 @@ use sys_util::{self, EventFd, GuestAddress, GuestMemory, PollContext, PollToken}
 
 use super::{
     DescriptorChain, Queue, VirtioDevice, INTERRUPT_STATUS_CONFIG_CHANGED,
-    INTERRUPT_STATUS_USED_RING, TYPE_BALLOON,
+    INTERRUPT_STATUS_USED_RING, TYPE_BALLOON, VIRTIO_F_VERSION_1,
 };
 
 #[derive(Debug)]
@@ -296,7 +296,9 @@ impl VirtioDevice for Balloon {
     }
 
     fn features(&self) -> u64 {
-        1 << VIRTIO_BALLOON_F_MUST_TELL_HOST | 1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM
+        1 << VIRTIO_BALLOON_F_MUST_TELL_HOST
+            | 1 << VIRTIO_BALLOON_F_DEFLATE_ON_OOM
+            | 1 << VIRTIO_F_VERSION_1
     }
 
     fn ack_features(&mut self, value: u64) {
diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs
index f2ddb2c..7d5fb02 100644
--- a/devices/src/virtio/block.rs
+++ b/devices/src/virtio/block.rs
@@ -22,7 +22,10 @@ use sys_util::{
 
 use data_model::{DataInit, Le16, Le32, Le64};
 
-use super::{DescriptorChain, Queue, VirtioDevice, INTERRUPT_STATUS_USED_RING, TYPE_BLOCK};
+use super::{
+    DescriptorChain, Queue, VirtioDevice, INTERRUPT_STATUS_USED_RING, TYPE_BLOCK,
+    VIRTIO_F_VERSION_1,
+};
 
 const QUEUE_SIZE: u16 = 256;
 const QUEUE_SIZES: &'static [u16] = &[QUEUE_SIZE];
@@ -655,6 +658,7 @@ impl<T: DiskFile> Block<T> {
             avail_features |= 1 << VIRTIO_BLK_F_DISCARD;
             avail_features |= 1 << VIRTIO_BLK_F_WRITE_ZEROES;
         }
+        avail_features |= 1 << VIRTIO_F_VERSION_1;
 
         Ok(Block {
             kill_evt: None,
@@ -798,8 +802,8 @@ mod tests {
             let f = File::create(&path).unwrap();
             let b = Block::new(f, false).unwrap();
             // writable device should set VIRTIO_BLK_F_FLUSH + VIRTIO_BLK_F_DISCARD
-            // + VIRTIO_BLK_F_WRITE_ZEROES
-            assert_eq!(0x6200, b.features());
+            // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1
+            assert_eq!(0x100006200, b.features());
         }
 
         // read-only block device
@@ -807,7 +811,8 @@ mod tests {
             let f = File::create(&path).unwrap();
             let b = Block::new(f, true).unwrap();
             // read-only device should set VIRTIO_BLK_F_FLUSH and VIRTIO_BLK_F_RO
-            assert_eq!(0x220, b.features());
+            // + VIRTIO_F_VERSION_1
+            assert_eq!(0x100000220, b.features());
         }
     }
 }
diff --git a/devices/src/virtio/virtio_device.rs b/devices/src/virtio/virtio_device.rs
index a7896de..1bf0281 100644
--- a/devices/src/virtio/virtio_device.rs
+++ b/devices/src/virtio/virtio_device.rs
@@ -29,7 +29,7 @@ pub trait VirtioDevice: Send {
 
     /// The set of feature bits that this device supports.
     fn features(&self) -> u64 {
-        0
+        1 << VIRTIO_F_VERSION_1
     }
 
     /// Acknowledges that this set of features should be enabled.
diff --git a/devices/src/virtio/virtio_pci_common_config.rs b/devices/src/virtio/virtio_pci_common_config.rs
index efbfbe0..4f30b3d 100644
--- a/devices/src/virtio/virtio_pci_common_config.rs
+++ b/devices/src/virtio/virtio_pci_common_config.rs
@@ -135,14 +135,10 @@ impl VirtioPciCommonConfig {
         match offset {
             0x00 => self.device_feature_select,
             0x04 => {
-                // TODO(dverkamp): This hack (copied from MmioDevice) unconditionally
-                // reports support for VIRTIO_F_VERSION_1; once all devices have been
-                // fixed to report VIRTIO_F_VERSION_1, remove this workaround.
-                let features = device.features() | 1 << VIRTIO_F_VERSION_1;
                 // 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 {
-                    (features >> (self.device_feature_select * 32)) as u32
+                    (device.features() >> (self.device_feature_select * 32)) as u32
                 } else {
                     0
                 }
diff --git a/devices/src/virtio/wl.rs b/devices/src/virtio/wl.rs
index 9d530bb..a3a1ac0 100644
--- a/devices/src/virtio/wl.rs
+++ b/devices/src/virtio/wl.rs
@@ -67,7 +67,9 @@ use sys_util::{
 #[cfg(feature = "wl-dmabuf")]
 use sys_util::ioctl_with_ref;
 
-use super::{DescriptorChain, Queue, VirtioDevice, INTERRUPT_STATUS_USED_RING, TYPE_WL};
+use super::{
+    DescriptorChain, Queue, VirtioDevice, INTERRUPT_STATUS_USED_RING, TYPE_WL, VIRTIO_F_VERSION_1,
+};
 use vm_control::{MaybeOwnedFd, VmRequest, VmResponse};
 
 const VIRTWL_SEND_MAX_ALLOCS: usize = 28;
@@ -1604,7 +1606,7 @@ impl VirtioDevice for Wl {
     }
 
     fn features(&self) -> u64 {
-        1 << VIRTIO_WL_F_TRANS_FLAGS
+        1 << VIRTIO_WL_F_TRANS_FLAGS | 1 << VIRTIO_F_VERSION_1
     }
 
     fn ack_features(&mut self, value: u64) {