diff --git a/vm-virtio/src/block.rs b/vm-virtio/src/block.rs index de6169294..58b1df139 100755 --- a/vm-virtio/src/block.rs +++ b/vm-virtio/src/block.rs @@ -893,29 +893,12 @@ impl VirtioDevice for Block { 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 { diff --git a/vm-virtio/src/console.rs b/vm-virtio/src/console.rs index 87270c95d..e08f1f6a2 100755 --- a/vm-virtio/src/console.rs +++ b/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 { diff --git a/vm-virtio/src/device.rs b/vm-virtio/src/device.rs index 7a1fa0050..1a372eea4 100644 --- a/vm-virtio/src/device.rs +++ b/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]); diff --git a/vm-virtio/src/iommu.rs b/vm-virtio/src/iommu.rs index e40ef39b8..6343731d0 100644 --- a/vm-virtio/src/iommu.rs +++ b/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 { diff --git a/vm-virtio/src/net.rs b/vm-virtio/src/net.rs index c03a245ae..c0c6d2dd2 100644 --- a/vm-virtio/src/net.rs +++ b/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 { diff --git a/vm-virtio/src/pmem.rs b/vm-virtio/src/pmem.rs index 63974ab12..f09e48158 100644 --- a/vm-virtio/src/pmem.rs +++ b/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 { diff --git a/vm-virtio/src/rng.rs b/vm-virtio/src/rng.rs index 0b5727eb5..4c4062bea 100755 --- a/vm-virtio/src/rng.rs +++ b/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 { diff --git a/vm-virtio/src/transport/mmio.rs b/vm-virtio/src/transport/mmio.rs index 993afe2cb..259142faf 100644 --- a/vm-virtio/src/transport/mmio.rs +++ b/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), diff --git a/vm-virtio/src/transport/pci_common_config.rs b/vm-virtio/src/transport/pci_common_config.rs index d6cf39995..16e88ed92 100644 --- a/vm-virtio/src/transport/pci_common_config.rs +++ b/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]) {} diff --git a/vm-virtio/src/vhost_user/blk.rs b/vm-virtio/src/vhost_user/blk.rs index 080045ce7..d53a21ac9 100644 --- a/vm-virtio/src/vhost_user/blk.rs +++ b/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 { diff --git a/vm-virtio/src/vhost_user/fs.rs b/vm-virtio/src/vhost_user/fs.rs index 04099ff7a..28cd7a730 100644 --- a/vm-virtio/src/vhost_user/fs.rs +++ b/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 { diff --git a/vm-virtio/src/vhost_user/net.rs b/vm-virtio/src/vhost_user/net.rs index 5fc730d8f..954db0865 100644 --- a/vm-virtio/src/vhost_user/net.rs +++ b/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 { diff --git a/vm-virtio/src/vsock/device.rs b/vm-virtio/src/vsock/device.rs index a84daed3d..634a45915 100644 --- a/vm-virtio/src/vsock/device.rs +++ b/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);