From 66eb01adec5b2988607ed42441020d4cac78e54a Mon Sep 17 00:00:00 2001 From: Richard Date: Fri, 25 Mar 2022 14:06:35 -0700 Subject: [PATCH] virtio-block: Upstream window's block device This CL mainly splits up Window's and Unix code into their own files. seg_max is now a part of windows, but will always be set to 0 now. seg_max in virtio_blk_config was being set to 0 on windows anyways.` Bug: b:213149164 Test: cargo test and presubmit Change-Id: Icdd481dd8e05c90ac8c1f29773d51312b64d3b2b Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3553760 Tested-by: kokoro Reviewed-by: Dennis Kempin Commit-Queue: Richard Zhang --- devices/src/virtio/block/asynchronous.rs | 16 +++------- devices/src/virtio/block/block.rs | 30 ++++++++++++------- devices/src/virtio/block/common.rs | 4 +-- devices/src/virtio/block/mod.rs | 1 + devices/src/virtio/block/sys/mod.rs | 9 ++++++ devices/src/virtio/block/sys/unix.rs | 17 +++++++++++ devices/src/virtio/block/sys/windows.rs | 7 +++++ devices/src/virtio/vhost/user/device/block.rs | 12 ++------ 8 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 devices/src/virtio/block/sys/mod.rs create mode 100644 devices/src/virtio/block/sys/unix.rs create mode 100644 devices/src/virtio/block/sys/windows.rs diff --git a/devices/src/virtio/block/asynchronous.rs b/devices/src/virtio/block/asynchronous.rs index 64fc80a372..7d1565aa80 100644 --- a/devices/src/virtio/block/asynchronous.rs +++ b/devices/src/virtio/block/asynchronous.rs @@ -3,7 +3,6 @@ // found in the LICENSE file. use std::cell::RefCell; -use std::cmp::{max, min}; use std::io::{self, Write}; use std::mem::size_of; use std::rc::Rc; @@ -20,9 +19,7 @@ use thiserror::Error as ThisError; use base::Error as SysError; use base::Result as SysResult; -use base::{ - error, info, iov_max, warn, AsRawDescriptor, Event, RawDescriptor, Timer, Tube, TubeError, -}; +use base::{error, info, warn, AsRawDescriptor, Event, RawDescriptor, Timer, Tube, TubeError}; use cros_async::{ select5, sync::Mutex as AsyncMutex, AsyncError, AsyncTube, EventAsync, Executor, SelectResult, TimerAsync, @@ -34,8 +31,8 @@ use vm_memory::GuestMemory; use super::common::*; use crate::virtio::{ - async_utils, copy_config, DescriptorChain, DescriptorError, Interrupt, Queue, Reader, - SignalableInterrupt, VirtioDevice, Writer, TYPE_BLOCK, + async_utils, block::sys::*, copy_config, DescriptorChain, DescriptorError, Interrupt, Queue, + Reader, SignalableInterrupt, VirtioDevice, Writer, TYPE_BLOCK, }; const QUEUE_SIZE: u16 = 256; @@ -503,12 +500,7 @@ impl BlockAsync { let avail_features = build_avail_features(base_features, read_only, sparse, true); - let seg_max = min(max(iov_max(), 1), u32::max_value() as usize) as u32; - - // Since we do not currently support indirect descriptors, the maximum - // number of segments must be smaller than the queue size. - // In addition, the request header and status each consume a descriptor. - let seg_max = min(seg_max, u32::from(QUEUE_SIZE) - 2); + let seg_max = get_seg_max(QUEUE_SIZE); Ok(BlockAsync { kill_evt: None, diff --git a/devices/src/virtio/block/block.rs b/devices/src/virtio/block/block.rs index 2fe2a06e97..a369b07b7d 100644 --- a/devices/src/virtio/block/block.rs +++ b/devices/src/virtio/block/block.rs @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use std::cmp::{max, min}; use std::io::{self, Write}; use std::mem::size_of; use std::path::PathBuf; @@ -15,8 +14,7 @@ use std::u32; use base::Error as SysError; use base::Result as SysResult; use base::{ - error, info, iov_max, warn, AsRawDescriptor, Event, PollToken, RawDescriptor, Timer, Tube, - WaitContext, + error, info, warn, AsRawDescriptor, Event, PollToken, RawDescriptor, Timer, Tube, WaitContext, }; use data_model::DataInit; use disk::DiskFile; @@ -30,8 +28,8 @@ use vm_memory::GuestMemory; use super::common::*; use crate::virtio::{ - copy_config, DescriptorChain, DescriptorError, Interrupt, Queue, Reader, SignalableInterrupt, - VirtioDevice, Writer, TYPE_BLOCK, + block::sys::*, copy_config, DescriptorChain, DescriptorError, Interrupt, Queue, Reader, + SignalableInterrupt, VirtioDevice, Writer, TYPE_BLOCK, }; const QUEUE_SIZE: u16 = 256; @@ -440,12 +438,7 @@ impl Block { let avail_features = build_avail_features(base_features, read_only, sparse, false); - let seg_max = min(max(iov_max(), 1), u32::max_value() as usize) as u32; - - // Since we do not currently support indirect descriptors, the maximum - // number of segments must be smaller than the queue size. - // In addition, the request header and status each consume a descriptor. - let seg_max = min(seg_max, u32::from(QUEUE_SIZE) - 2); + let seg_max = get_seg_max(QUEUE_SIZE); Ok(Block { kill_evt: None, @@ -798,7 +791,12 @@ mod tests { // writable device should set VIRTIO_BLK_F_FLUSH + VIRTIO_BLK_F_DISCARD // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE // + VIRTIO_BLK_F_SEG_MAX + VIRTIO_RING_F_EVENT_IDX + #[cfg(unix)] assert_eq!(0x120006244, b.features()); + + // VIRTIO_F_SEG_MAX is not supported on Windows + #[cfg(windows)] + assert_eq!(0x120006240, b.features()); } // read-write block device, non-sparse @@ -809,7 +807,12 @@ mod tests { // writable device should set VIRTIO_BLK_F_FLUSH // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE // + VIRTIO_BLK_F_SEG_MAX + VIRTIO_RING_F_EVENT_IDX + #[cfg(unix)] assert_eq!(0x120004244, b.features()); + + // VIRTIO_F_SEG_MAX is not supported on Windows + #[cfg(windows)] + assert_eq!(0x120006240, b.features()); } // read-only block device @@ -820,7 +823,12 @@ mod tests { // read-only device should set VIRTIO_BLK_F_FLUSH and VIRTIO_BLK_F_RO // + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE + VIRTIO_BLK_F_SEG_MAX // + VIRTIO_RING_F_EVENT_IDX + #[cfg(unix)] assert_eq!(0x120000264, b.features()); + + // VIRTIO_F_SEG_MAX is not supported on Windows + #[cfg(windows)] + assert_eq!(0x120000260, b.features()); } } diff --git a/devices/src/virtio/block/common.rs b/devices/src/virtio/block/common.rs index 9d38cf3e4b..225a0c2bf5 100644 --- a/devices/src/virtio/block/common.rs +++ b/devices/src/virtio/block/common.rs @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +use crate::virtio::block::sys::*; use data_model::{DataInit, Le16, Le32, Le64}; pub const SECTOR_SHIFT: u8 = 9; @@ -33,7 +34,6 @@ pub const VIRTIO_BLK_S_OK: u8 = 0; pub const VIRTIO_BLK_S_IOERR: u8 = 1; pub const VIRTIO_BLK_S_UNSUPP: u8 = 2; -pub const VIRTIO_BLK_F_SEG_MAX: u32 = 2; pub const VIRTIO_BLK_F_RO: u32 = 5; pub const VIRTIO_BLK_F_BLK_SIZE: u32 = 6; pub const VIRTIO_BLK_F_FLUSH: u32 = 9; @@ -152,7 +152,7 @@ pub fn build_avail_features( } avail_features |= 1 << VIRTIO_BLK_F_WRITE_ZEROES; } - avail_features |= 1 << VIRTIO_BLK_F_SEG_MAX; + avail_features |= system_block_avail_features(); avail_features |= 1 << VIRTIO_BLK_F_BLK_SIZE; if multi_queue { avail_features |= 1 << VIRTIO_BLK_F_MQ; diff --git a/devices/src/virtio/block/mod.rs b/devices/src/virtio/block/mod.rs index 0909b4e1b3..d564fc4a2c 100644 --- a/devices/src/virtio/block/mod.rs +++ b/devices/src/virtio/block/mod.rs @@ -5,6 +5,7 @@ pub mod asynchronous; pub mod block; pub(crate) mod common; +pub(crate) mod sys; pub use asynchronous::{BlockAsync, DiskState}; pub use block::Block; diff --git a/devices/src/virtio/block/sys/mod.rs b/devices/src/virtio/block/sys/mod.rs new file mode 100644 index 0000000000..75209c3a72 --- /dev/null +++ b/devices/src/virtio/block/sys/mod.rs @@ -0,0 +1,9 @@ +cfg_if::cfg_if! { + if #[cfg(unix)] { + mod unix; + pub use self::unix::*; + } else if #[cfg(windows)] { + mod windows; + pub use self::windows::*; + } +} diff --git a/devices/src/virtio/block/sys/unix.rs b/devices/src/virtio/block/sys/unix.rs new file mode 100644 index 0000000000..d79fbf27ee --- /dev/null +++ b/devices/src/virtio/block/sys/unix.rs @@ -0,0 +1,17 @@ +use base::iov_max; +use std::cmp::{max, min}; + +pub const VIRTIO_BLK_F_SEG_MAX: u32 = 2; + +pub fn system_block_avail_features() -> u64 { + 1 << VIRTIO_BLK_F_SEG_MAX +} + +pub fn get_seg_max(queue_size: u16) -> u32 { + let seg_max = min(max(iov_max(), 1), u32::max_value() as usize) as u32; + + // Since we do not currently support indirect descriptors, the maximum + // number of segments must be smaller than the queue size. + // In addition, the request header and status each consume a descriptor. + min(seg_max, u32::from(queue_size) - 2) +} diff --git a/devices/src/virtio/block/sys/windows.rs b/devices/src/virtio/block/sys/windows.rs new file mode 100644 index 0000000000..3e97beb089 --- /dev/null +++ b/devices/src/virtio/block/sys/windows.rs @@ -0,0 +1,7 @@ +pub fn system_block_avail_features() -> u64 { + 0 +} + +pub fn get_seg_max(_queue_size: u16) -> u32 { + 0 +} diff --git a/devices/src/virtio/vhost/user/device/block.rs b/devices/src/virtio/vhost/user/device/block.rs index 1139cbf2ff..c8821a45ef 100644 --- a/devices/src/virtio/vhost/user/device/block.rs +++ b/devices/src/virtio/vhost/user/device/block.rs @@ -3,7 +3,6 @@ // found in the LICENSE file. use std::cell::RefCell; -use std::cmp::{max, min}; use std::fs::OpenOptions; use std::rc::Rc; use std::sync::{atomic::AtomicU64, atomic::Ordering, Arc}; @@ -14,7 +13,7 @@ use futures::future::{AbortHandle, Abortable}; use sync::Mutex; use vmm_vhost::message::*; -use base::{error, iov_max, warn, Event, Timer}; +use base::{error, warn, Event, Timer}; use cros_async::{sync::Mutex as AsyncMutex, EventAsync, Executor, TimerAsync}; use data_model::DataInit; use disk::create_async_disk_file; @@ -27,7 +26,7 @@ use crate::virtio::vhost::user::device::{ handler::{DeviceRequestHandler, Doorbell, VhostUserBackend}, vvu::pci::VvuPciDevice, }; -use crate::virtio::{self, base_features, copy_config, Queue}; +use crate::virtio::{self, base_features, block::sys::*, copy_config, Queue}; const QUEUE_SIZE: u16 = 256; const NUM_QUEUES: u16 = 16; @@ -86,12 +85,7 @@ impl BlockBackend { let avail_features = build_avail_features(base_features, read_only, sparse, true) | VhostUserVirtioFeatures::PROTOCOL_FEATURES.bits(); - let seg_max = min(max(iov_max(), 1), u32::max_value() as usize) as u32; - - // Since we do not currently support indirect descriptors, the maximum - // number of segments must be smaller than the queue size. - // In addition, the request header and status each consume a descriptor. - let seg_max = min(seg_max, u32::from(QUEUE_SIZE) - 2); + let seg_max = get_seg_max(QUEUE_SIZE); let async_image = disk_image.to_async_disk(ex)?;